tlswg / draft-ietf-tls-svcb-ech

Other
1 stars 0 forks source link

WGLC: Comments from Stephen Farrell #7

Open enygren opened 3 months ago

enygren commented 3 months ago

From Stephen Farrell:

I (strongly:-) support publication. I do have comments but really am ok if those are ignored.

Given that the onus should be on ECH aware clients to reject such dodgy stuff, perhaps we'd be better off to just say that publishers MUST check the presentation syntax is empty or valid base64 and SHOULD be a base64 encoded ECHConfigList? Then we could leave all the details of checks before publication to [1] which may be a better approach.

Again though, I'm fine that this be published as-is and prefer that to spending loads of time arguing, so do feel free to ignore any of the above,

Cheers, S.

[1] https://datatracker.ietf.org/doc/html/draft-ietf-tls-wkech [2] https://datatracker.ietf.org/doc/draft-pauly-v6ops-happy-eyeballs-v3/ [3] https://datatracker.ietf.org/doc/draft-farrell-tls-pemesni/

enygren commented 3 months ago

Inserting Mikes responses:

Mike Bishop via ietf.org

Jun 25, 2024, 8:31 AM

to Stephen, Sean, TLS Responses to some of these in-line below. More generally, I think several of these arise from the question of whether requirements on "publishers" apply specifically to a tool which is automatically generating these records or generally to the operating entity causing the records to be created. I believe Section 3 is attempting to describe a state that needs to exist for safe deployment, not attempting to specify a tool per se. Thus I generally support leaving and/or moving requirements on automation to [1].

-----Original Message----- From: Stephen Farrell [stephen.farrell@cs.tcd.ie](mailto:stephen.farrell@cs.tcd.ie) Sent: Monday, June 24, 2024 8:00 PM To: Sean Turner [sean@sn3rd.com](mailto:sean@sn3rd.com); TLS List [tls@ietf.org](mailto:tls@ietf.org) Subject: [TLS]Re: Working Group Last Call for Bootstrapping TLS Encrypted ClientHello with DNS Service Bindings

Hiya,

On 21/06/2024 17:26, Sean Turner wrote:

Gentle reminder this WGLC is still ongoing.

I (strongly:-) support publication. I do have comments but really am ok if those are ignored.

  • section 3 has a MUST for checking all IP addresses are for servers that have access to the relevant ECH private key material. We go into some more detail on how to do that in [1] so perhaps some of that text ought be here, or (given the best HOWTO may change) that could be via a reference to [1]. OTOH, that'd maybe be a normative ref so could delay things, so not sure how best to proceed. (See also the "bind" comment at the end.)

I'm wary about adding a normative reference to an I-D that's not finalized yet; that's how we wound up with this document to begin with. In [1] you note that your mechanisms aren't intended to be universal, and I'm not sure that pulling it into this doc is required. I think the statement in Section 3 describes the desired state -- each IP the name might resolve to needs access to either the indicated key(s) or a certificate for the public name.

  • A particular case of the above is if an ECHConfigList has more than one ECHConfig (e.g. for different algs). I'm not sure if the text here implies that all public values must "work" or if it means that at least one public value must "work." (In publication code I've written I check all public keys and only publish if all public keys are ok, but one could do otherwise and not suffer disaster:-)

I'm inclined to agree that all keys should ideally be supported by all servers identified by the TargetName, but being authoritative for the private name cures any gaps. Perhaps this needs to be more granular? MUST be authoritative for the public name and SHOULD have access to the private key seems reasonable at first glance, but is there a deployment scenario where the front-end will not be authoritative for the public name?

  • I'm not clear if section 3 implies a server has to check that all relevant A/AAAA and ipv[46]hint values are present and correct before publication. (Test publication code I've done currently ignores hints at publication time, and just checks one of A/AAAA works, but I'd be ok with being more picky.)

  • I think it'd be better (but not necessary) if section 3 had some guidance on when

    1 HTTPS RRdata value is reasonable or to be expected for ECH-specific reasons. (Basically, if one has >1 CDN I guess?)

Typically multi-CDN works by having a DNS server that issues different responses at different times, not by listing them all in a single RRSet. More likely this is if you have endpoints with different capabilities that aren't on the same server -- say, HTTP/2 and HTTP/3 are served off of different machines or at different port numbers.

  • I wonder if there's a need to consider the new happy eyeballs stuff [2] before declaring victory here. I've no strong opinion but this and [2] probably should at least not contradict one another. (I don't know if that's been checked.)

HEv3 says that ECH records should be sorted ahead of non-ECH records, but doesn't say anything about switching to SVCB-reliant per 4.1. We should probably file that feedback with HEv3, but I don't believe this document needs to change.

  • Does 4.1 correspond to what browsers do now with ECH? It's not clear to me that it does, but I'm not sure. (I'm currently writing a test generator to figure that out but it'll be some weeks before I could answer the question myself.)

  • 4.2 implies but does not say that parts of the ech= value that affect the outer CH need to be encoded as extensions within the ECHConfigList, or else are at the whim of the client code. That's probably ok, but worth a bit of thought. (Personally, I'd prefer we disparage extensions within ECHConfigList values but I think I've lost that argument before:-)

  • 4.3 has a MUST NOT for clients - do we need to say that that means normal clients and not the special-case client that is needed to do checks before publication?

I think that seems clear enough, since the publisher isn't a "client" in this sense. But if it's not clear, that could be added.

  • section 5 says: "Origins that publish an "ech" SvcParam in their HTTPS record SHOULD also publish an HTTPS record with the "ech" SvcParam for every alt-authority offered in its Alt-Svc Field Values. Otherwise, clients might reveal the name of the server in an unencrypted ClientHello to an alt-authority." I think some examples of that would really help. More generally some examples would be good to add as an appendix.

  • I'd like to (yet again:-) suggest that defining a PEM format for ECH stuff would be worthwhile, and could be done as an appendix of this document. (I.e. include [3] as an appendix.) When I suggested this before, there was some opposition so it;s ok if people still don't want that, but IMO it'd be good were that defined somewhere.

I don't feel like that's connected enough to this document to define here, particularly not as a new addition at WGLC.

enygren commented 3 months ago

From Stephen Farrell:

On 25/06/2024 16:30, Mike Bishop wrote:

Responses to some of these in-line below. More generally, I think several of these arise from the question of whether requirements on "publishers" apply specifically to a tool which is automatically generating these records or generally to the operating entity causing the records to be created. I believe Section 3 is attempting to describe a state that needs to exist for safe deployment, not attempting to specify a tool per se. Thus I generally support leaving and/or moving requirements on automation to [1].

I tend to agree and am fine with whatever wording tweaks the authors prefer, but it'd likely be good to not say "MUST" (in this doc) in cases where the text is guidance for what's a sane thing to do, rather than asking developers to reject other inputs.