search

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 58 forks source link

Require that someone validate public_name correctly. #456

Closed chris-wood closed 3 years ago

chris-wood commented 3 years ago

This combines #436 and #447, requiring that validation of public_name happen somewhere, providing a reference parser (if one does not exist), and allowing it to happen when parsing ECHConfig.

Closes #396, #405, #424

cc @martinthomson, @davidben, @cjpatton, @sftcd

sftcd commented 3 years ago

On 14/06/2021 20:26, Christopher Wood wrote:

@chris-wood commented on this pull request.

+: This value MUST NOT begin or end with an ASCII dot and MUST be parsable as a +dot-separated sequence of LDH labels, as defined in +{{!RFC5890, Section 2.3.1}}. Clients MUST ignore any ECHConfig structure +whose public_name does not meet these criteria. +

The public_name field is already limited to 255 characters, so I think we're fine.

The max for a label in a DNS name is 63 octets though isn't it?

chris-wood commented 3 years ago

The max for a label in a DNS name is 63 octets though isn't it?

Yeah, but RFC6066 has no similar requirements (that I can tell), so I don't see we need to impose anything new here. Maybe I'm missing something? And if so, can you please propose text?

sftcd commented 3 years ago

On 14/06/2021 20:33, Christopher Wood wrote:

The max for a label in a DNS name is 63 octets though isn't it?

Yeah, but RFC6066 has no similar requirements (that I can tell), so I don't see we need to impose anything new here. Maybe I'm missing something? And if so, can you please propose text?

Fair ask:-)

I guess the public_name mostly has to go in a cert SAN so would it help to use or refer to rfc5280 text?

The most relevant bit there seems to be:

When the subjectAltName extension contains a domain name
system label, the domain name MUST be stored in the
dNSName (an IA5String). The name MUST be in the "preferred
name syntax", as specified by Section 3.5 of [RFC1034] and
as modified by Section 2.1 of [RFC1123].  Note that while
uppercase and lowercase letters are allowed in domain
names, no significance is attached to the case.  In
addition, while the string " " is a legal domain name,
subjectAltName extensions with a dNSName of " " MUST NOT
be used.

(That text is near [1]).

That said, that " " is a legal domain name surprises me (and I'm a co-author of 5280!)

BTW, I'm fine that that all gets fixed later if that's better.

S.

[1] https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6

chris-wood commented 3 years ago

BTW, I'm fine that that all gets fixed later if that's better.

Sounds good -- I dropped an OPEN ISSUE to note this for later. @sftcd, would you have cycles to take that issue and propose a PR to address it?

sftcd commented 3 years ago

On 14/06/2021 20:52, Christopher Wood wrote:

BTW, I'm fine that that all gets fixed later if that's better.

Sounds good -- I dropped an OPEN ISSUE to note this for later. @sftcd, would you have cycles to take that issue and propose a PR to address it?

Sneaky eh:-)

Sure, I can look at it this week but don't wait on me.

S

chris-wood commented 3 years ago

Sure, I can look at it this week but don't wait on me.

Yeah, there's no rush here, I think. This seems like a smaller issue to resolve at some point.