tlswg / draft-ietf-tls-esni

TLS Encrypted Client Hello
https://tlswg.github.io/draft-ietf-tls-esni/#go.draft-ietf-tls-esni.html
Other
229 stars 56 forks source link

Correct `ECHConfigList` length bounds #614

Closed ctz closed 4 months ago

ctz commented 5 months ago

Draft 18.

ECHConfig is:

       struct {
           uint16 version;
           uint16 length;
           select (ECHConfig.version) {
             case 0xfe0d: ECHConfigContents contents;
           }
       } ECHConfig;

4 bytes each minimum.

ECHConfigList is:

       ECHConfig ECHConfigList<1..2^16-1>;

That implies an ECHConfig could be encoded in one byte? I think this should be:

       ECHConfig ECHConfigList<4..2^16-4>;
davidben commented 5 months ago

Where are you getting 2^16-4 from? That seems wrong. It seems like you are confusing minimum of 4 with multiple of 4.

Beyond that, agreed that 4 is a valid value, but 1 is also just as valid, as is 2 or 3. It makes no difference whether you reject 1 bytes at this layer of parser, or immediately afterwards when you actually go to parse the ECHConfigs. The only thing we're actually saying with any of these values is "there must be at least one ECHConfig".

To be honest, I think the TLSWG's habit of trying to compute the theoretical minimum value here, instead of just 1, is a waste of energy. At best it is confusing, and at worse we get the value wrong on accident and say the wrong thing. https://mailarchive.ietf.org/arch/msg/tls/kA1C6C0Wluwp6ui1VDmrFbaEm1U/ https://mailarchive.ietf.org/arch/msg/tls/Vlcshsrq1QmWDgIqJO3se8K25jA/

So while I'm not opposed to this PR, since it is a habit we have in other specifications, the lack of opposition only comes from the pre-existing habit. On its own, I think the habit is a bad one and ultimately makes our documents less readable.

ctz commented 5 months ago

Where are you getting 2^16-4 from? That seems wrong. It seems like you are confusing minimum of 4 with multiple of 4.

You're right. Corrected.

Beyond that, agreed that 4 is a valid value, but 1 is also just as valid, as is 2 or 3. It makes no difference whether you reject 1 bytes at this layer of parser, or immediately afterwards when you actually go to parse the ECHConfigs. The only thing we're actually saying with any of these values is "there must be at least one ECHConfig".

Well I agree. I'm not sure who the TLS presentation language is for, but it always seems to me to detract from the descriptive text and take oxygen from more useful artifacts like test vectors or a reference implementation -- but I realise the TLSWG doesn't have a culture of either of those.