Invalid names end in .invalid. Fixes #630

[ekr]

@bemasc

[ekr]

@letoams

[ekr]

@paulwouters: PTAL

[martinthomson]

Suggested change

	configurations SHOULD have invalid keys, and invalid public names,
	configurations can have invalid keys, and invalid public names,

I can't think of an argument for not allowing configurations that are otherwise syntactically valid. They shouldn't be used, which might mean that they might not go anywhere, but I don't see why you need multiple layers of signaling.

[martinthomson]

See my comment on #637 for why I am not enthusiastic about this general direction. I've suggested a different structure there.

[ekr]

I don't have a strong opinion about the substance here; the part about invalid keys was in the previous text.

[bemasc]

@martinthomson

The point here is that if server grease (as described here) is coupled with a valid key and public_name, then the grease doesn't work. Misbehaving clients will still connect successfully.

Similarly, if the public_name is syntactically invalid, grease may still fail, if the client rejects the ECHConfig due to a syntax error.

For grease to work, the key must be wrong and the public_name must be syntactically valid but also wrong. This text is recommending a specific way to be wrong, because the obvious alternative (setting the public_name to some random stranger's domain) seems pretty weird/insecure.

[martinthomson]

Yeah, I don't think that any of those arguments hold.

For a connection to be established, the server needs to hold the corresponding private key and be willing to use it. The client needs to use the public name and (somehow) validate it successfully. The client also needs to ignore the greased codepoint, despite not understanding it and knowing that it means that the entire config should be ignored. Sure, the client doesn't know that the key pair is or isn't good, but it has multiple cues to go from.

We owe clients that are beyond incompetent no such affordance in specifications.

To take a step back, you only need one clear and unambiguous way to signal that a config should be ignored. That's the presence of a mandatory, unknown extension. Adding in name checks a) complicates the validation process and b) prevents me from doing my new, totally awesome thing that might rely on invalid names. Which I realize that I'm going to have to share now. Here, it's incomplete, but it might at least make sense: https://github.com/martinthomson/ech-pnmasq

[sftcd]

I scanned Martin's document and it seems quite complex. I do agree we ought revisit the problem, but not until after the ECH RFC has issued so can we rather try get this thing out the door?

[bemasc]

Adding in name checks a) complicates the validation process and b) prevents me from doing my new, totally awesome thing ...

The text here is not attempting to add name checks. In fact, this text assumes that clients do not perform any name checks (beyond syntax). If client applies special treatment to ".invalid" names, the client has "special-cased the grease" and the procedure is broken.

Would you be happier with an additional sentence like "Clients MUST NOT apply special treatment to names based on the ".invalid" TLD."?

[paulwouters]

It seems Martin's raised point is a new issue. I am fine if we go that way and omit the specifics like how to formulate an invalid hostname. If text will remain about hostnames, it would still be good to suggest using .invalid as an example, without any 2119 enforcement.

[ekr]

I tend to agree with @bemasc that in order for the GREASE to induce a failure (which is the point!) we need the ECHConfig to actually be invalid, not just have an unexpected extension. I don't see how this creates a future compatibility issue. @martinthomson is there a specific change here that will avoid creating a problem for your draft? If not, I'd prefer to land this (happy to change the SHOULD to can if you feel strongly).

[martinthomson]

The specific thing that I was concerned about is the requirement to check the name. I believe that it should be possible to have a valid ECH config with a garbage public name in it. That said, an extension can override the need to run any checks in the main spec, so I can live with a "SHOULD" if pressed.

However, greasing only needs to trip one check. The mandatory extension one seemed fine for that, particularly since you can reserve a mandatory-to-understand codepoint specifically for that purpose. Forcing the configuration to be bad is unnecessary as a requirement: that is, it is not necessary to guarantee interoperability, so is a misuse of 2119 language.

[bemasc]

@martinthomson I'm confused. This PR doesn't introduce any requirement for either endpoint to check a name.

However, greasing only needs to trip one check. The mandatory extension one seemed fine for that, particularly since you can reserve a mandatory-to-understand codepoint specifically for that purpose. Forcing the configuration to be bad is unnecessary as a requirement: that is, it is not necessary to guarantee interoperability, so is a misuse of 2119 language.

If the public name is not "bad", then the grease will "fail to fail". Broken client implementations (i.e. those that ignore unrecognized mandatory extensions instead of rejecting the config) will trigger the recovery flow, resulting in a successful connection (using a non-grease ECHConfig retrieved in-band) instead of angry users filing bug reports against the client (which is the point of this grease).

[martinthomson]

I'm objecting to this:

These extraneous ECH configurations SHOULD have invalid keys, and invalid public names,

And this claim:

If the public name is not "bad", then the grease will "fail to fail".

Which is untrue. It is only necessary to have one thing wrong. Operating on the assumption that clients will deliberately violate a clear and well-specified rule in the specification is sometimes appropriate, but only when there is evidence that this has happened.

The goal is achieved when the rule is inoperable. A strong recommendation (i.e., "SHOULD") does not improve interoperability.

[0x9fff00]

I'm objecting to this:

These extraneous ECH configurations SHOULD have invalid keys, and invalid public names,

As far as I can tell, the Server Greasing section that this is part of is completely irrelevant to client implementations and is purely a recommendation on how to construct a grease ECH config on the server side. I think “invalid public names” just means names that if used would not result in a successful connection, not that the client should attempt to detect and reject configs with “invalid” names. I don’t see how the client could do so, since it cannot know if the server would accept a name before trying it.

[bemasc]

@martinthomson Is this closer to what you're looking for?

Suggested change

	configurations SHOULD have invalid keys, and invalid public names,
	configurations SHOULD have keys and public names that the server does not accept.
	Otherwise, incorrectly implemented clients may not fail to connect as intended.
	For example, the public name could end in ".invalid" (see {{?RFC2606}}).

[martinthomson]

I'm still stuck on the "SHOULD" and this notion that we need to concern ourselves with incorrectly implemented clients.

[ekr]

Isn't the entire purpose of this piece of greasing to detect incorrectly implemented clients?

[martinthomson]

So let them connect and collect their IP addresses?

I guess that I'm struggling with the use of 2119 language here. Grease is discretionary and serves no direct interoperability purpose. I don't see why you would so forcibly recommend action like this when this could be a simple "we reserved some codepoints and other stuff, which you can use this way if you are interested".

[martinthomson]

I should have said, the greased ECH ClientHello extension serves a purpose that depends on it being used, but this one doesn't.

[ekr]

So let them connect and collect their IP addresses?

The reasoning here is as follows:

• You provide an ECHConfig with a mandatory unknown extension which should cause a compliant client to skip the ECHConfig
• The ECHConfig is otherwise invalid (but in a way that is not readily apparent), so if it doesn't skip it, it will experience a connection failure.
• The user will report the connection failure and that will help get the client fixed.

I guess that I'm struggling with the use of 2119 language here. Grease is discretionary and serves no direct interoperability purpose. I don't see why you would so forcibly recommend action like this when this could be a simple "we reserved some codepoints and other stuff, which you can use this way if you are interested".

I don't have a strong opinion either way about the SHOULD, but I do think we should explain the logic above. Perhaps:

The reserved values with the high-order bit set are mandatory, as
defined in {{config-extensions}}. In order to GREASE correct client
behavior in ignoring these values, servers can randomly select from these values and include them in extraneous ECH configurations which have invalid keys, and invalid public names,
Correctly-implemented clients will ignore these configurations because
they do not recognize the mandatory extension. Incorrectly implemented
clients which skip the extension will attempt to connect to the server
using the ECHConfig, but fail to connect because the server will not
be able to decrypt the encrypted ClientHello or perform the recovery
flow, thus causing a failure on the client.

[martinthomson]

Why not invalid IP addresses? There's no place like 127.0.0.1.

My point here is that you don't have a good measure for exactly how stupid the implementation needs to be to be caught out this way. A perfectly valid approach would include a completely valid configuration, aside from the mandatory extension that no one can support. Then, if a connection attempt is made, the server can log everything, including the inner ClientHello. I don't know what you /do/ with that information, but it's a trap for a very particular profile of stupid.

You can make the config more and more obviously silly, but who then are you planning to trap? For instance, I don't see why you couldn't connect with "foo.invalid" and expect it to work, aside from any retry.

[bemasc]

I think we are talking past each other slightly (the text assumes that clients MUST NOT special-case .invalid!), but I also think @martinthomson is onto something here. We don't have to rely on "natural" failure modes (as in the current text). The server can explicitly recognize the GREASE configs and inject failures. This allows better GREASE (the config can be otherwise valid) and richer failure modes (e.g. injecting a warning message into webpage content).

I've added a suggestion below.

[bemasc]

I think we are talking past each other slightly (the text assumes that clients MUST NOT special-case .invalid!), but I also think @martinthomson is onto something here. We don't have to rely on "natural" failure modes (as in the current text). The server can explicitly recognize the GREASE configs and inject failures. This allows better GREASE (the config can be otherwise valid) and richer failure modes (e.g. injecting a warning message into webpage content).

I've added a suggestion below.

[ekr]

@martinthomson @bemasc how does this look?