Hi Martin, Thanks for the comments. As the author of #374, I obviously didn't think it so clearly unnecessary, but glad to hear your thoughts. Hopefully we can get on the same page as to what the issues are and/or a better solution. (Always happy to replace something with a simpler option, provided it also works!) To that end, I'd second Chris's suggestion of working through an alternate PR so we have something concrete.
Going through some of these in detail (inline) On Wed, Mar 31, 2021 at 9:58 PM Martin Thomson <m...@lowentropy.net> wrote: > # Matching Inner and Outer Values > > When we get right down to it, there are a very small number of things that > truly change in response to HelloRetryRequest. And all of these changes > are to values that do not need confidentiality protection. > > The draft lists three fields that change: ciphersuites, key_share, and > version. From my perspective, changing cipher suites, supported groups, or > versions would be a big mistake. So what changes is even more limited. > Just the shares in key_share. > > On this basis, a client that offers cipher suites, groups, versions, and > key shares that are identical in both inner and outer ClientHello messages > will always receive a HelloRetryRequest that applies equally to both > messages. > > The only adjustment that is acceptable with respect to these fields being > identical is the addition of TLS 1.2-only options to the outer ClientHello > (or the removal of the same from the inner ClientHello if you prefer it > that way around). This is a fine optimization on the basis that accepting > ECH represents a commitment to support TLS 1.3 (or higher). But it is > really just an optimization (the draft makes this mandatory, which is > silly). The client can therefore remove options from the inner ClientHello > only if it is impossible to select them with TLS 1.3 or higher. > > For new extensions, if they define a means of adjustment or correction via > HelloRetryRequest (there is currently just one: password_salt, which I > haven't examined), then they too need to follow this restriction. It's > not an onerous one. > > Follow this simple constraint and HelloRetryRequest will always apply > equally to both inner and outer ClientHello and everything works. > Conveniently, this is more or less exactly what the current draft says. > Almost. > This was a previous attempt at this, but it seems to have been confusing. I do agree that having ClientHelloInner vs ClientHelloOuter differences that result in different HelloRetryRequest acceptance criteria is a bad plan: in general, I think a client implementation should be judicious about which difference it exposes because each difference needs special logic to make sure it's using the right set as you evaluate the handshake. At the same time, I worry that *mandating* it in the extension will make this decision process tricky to navigate and get us in a rough corner. PR #316 was trying to capture exactly this, but it seems we didn't get on the same page. (Maybe we can now?) See this discussion: https://github.com/tlswg/draft-ietf-tls-esni/pull/316#discussion_r509880007 And so the text ended up with something self-contradictory. It says: "[...] clients offering ECH MUST ensure that all extensions or parameters that might change in response to receiving a HelloRetryRequest have the same values in ClientHelloInner and ClientHelloOuter." https://tlswg.org/draft-ietf-tls-esni/draft-ietf-tls-esni.html#name-handling-helloretryrequest And then it goes to list things that differ from this criteria! Indeed the list is right and the criteria is wrong. The criteria is neither sufficient (we do need cipher suites to align), nor sufficient (as you note, the key share *values* don't strictly need to match). I concluded from this exercise that trying to formulate the rules is a bit of a mess. As you allude to with password_salt, we need to formulate something that can capture future HRR extensions. (I'd also gotten that sense from one of the past WG meetings: land the HRR-matching rule as a bandaid, but we didn't have consensus for it as an actual solution.) I actually had not heard of password_salt, so thanks for the pointer. I've only skimmed it but, at a glance, this seems exactly like something where you'd want ClientHelloInner vs ClientHelloOuter to differ. It seems to be about some password-based TLS authentication, but any client authentication should only apply to the true ClientHelloInner handshake and not the fallback ClientHelloOuter handshake. > The draft currently allows inner and outer ClientHello to have different > types of key share. The way it handles this is terrible: it recommends > breaking the transcript. To me, that seems like it would only serve to > open the protocol up to downgrade attack. > I'm not sure I follow this point. Do you mean the draft as-is, or the PR? I don't believe the draft as-is does, and I don't think the PR does anything to the transcript that draft as-is doesn't already do. The transcript behaviors are largely forced by a combination of backwards compatibility for ClientHelloOuter and split mode for ClientHelloInner. Incidentally, I don't see a problem with having a different key share > *value* in inner and outer ClientHello. There's no point in doing that > unless you believe that these values leak information (they really > shouldn't), but it wouldn't break this model if a client decided to do that. > > https://github.com/tlswg/draft-ietf-tls-esni/issues/333 appears to be > concerned about the cookie only applying to one or other ClientHello. I > don't see how is the case, so I'm just going to say that this is fixed by > having HelloRetryRequest apply to both inner and outer ClientHello > messages. If the client receives HelloRetryRequest that applies to just > one of the two, then the problem is that the client is faulty. That would > be treated as a programming error as normal (crash, open a bug report, send > an internal_error alert, etc...). > I think you may have misunderstood #333. That or I'm misunderstanding your response. Imagine a client-facing server that wants to support HRR statelessly. When that client-facing server accepts ECH and gets an HRR from the backend server to forward, it has no opportunity to inject its own cookie. Maybe our answer is client-facing servers always do HRR on ECH accept. I'm perfectly fine with that, but if that's our answer, we should explicitly make that decision. > Then there are the things that more or less have to change in response to > HelloRetryRequest, but really only because the ClientHello changes: > padding, pre_shared_key, and ECH itself. For those, we need to address a > minor inconsistency problem at the level of the core protocol itself. > > # Addressing Minor HelloRetryRequest Problems > > We do need to fix RFC 8446 rules regarding HelloRetryRequest. David > already suggested some minute adjustments for that problem in > https://github.com/tlswg/draft-ietf-tls-esni/issues/358 . The short > version is that extensions can define their own rules for how they change > after HelloRetryRequest. This is a good amendment, especially as it > relates to extensions that are not known to the server. > > That tweak does have deployment issues, because the original rules have > been interpreted too literally in some cases, but that should not affect > ECH specifically. Servers that have this bug won't be able to deploy ECH > without fixing the bug and that's OK. Other servers will only see grease. > > The draft currently mandates that greasing values not change after > HelloRetryRequest, which will avoid this compatibility bug, but also reveal > the fraud. I can tolerate that small amount of leakage. > Independent of what we do in ECH, I like that adjustment to HRR rules. If we want to do that as part of resolving the ECH issue, we should make that decision in rfc8446bis. I filed https://github.com/tlswg/tls13-spec/issues/1223, but it's not clear that has consensus yet. In terms of sticking out, Ben Schwartz also pointed out another issue with the current situation. It's not just that, where HelloRetryRequest occurs naturally, this quirk reveals GREASE. A forged HelloRetryRequest also reveals GREASE. This PR resolves this by authenticating the ECH acceptance signal in HelloRetryRequest. > # Avoiding HelloRetryRequest > > I think that Nick's suggestion for helping avoid HelloRetryRequest by > placing hints about key shares in DNS SVCB/HTTPS records is a fine one. > > I see the arguments about this being about the configuration needing to > speak for backend servers when the record relates to frontend servers. But > my perspective here is that you already need to ensure that backend servers > have a consistent cryptographic support profile; adding a small number of > frontend servers to the set that need to be made consistent isn't that > difficult. If this consistency is not possible in some deployments, that's > understandable, but then it is an optional enhancement that won't be > available to those deployments, that's all. > > Of course, this is an extension that we can pursue separately. > I think trying to avoid HRR, provided we actually make it optional, can be pursued independent of this question. If we say it's optional, we still need to make ECH + HRR work. If we make it mandatory, I don't see how we can make it deployable on the server. In particular: - Servers can't guarantee client behavior a priori, so it'd need a binding "hint". - We'd need a story for mismatch between DNS configs and the server, to keep ECH deployable at all. - We'd need to be comfortable that every HRR reason, past and present, can be hinted away by DNS. I suspect cookie and password_hint already break this. > # Conclusion > > I'm firmly opposed to splitting HelloRetryRequest. I would like to deploy > ECH and this doesn't really help with that. > > I don't agree that there is a problem that needs to be fixed with the > current draft. > > On the other hand, I can guarantee that this change will delay Firefox > deployment significantly (that is, for an indefinite period). It would > require rearchitecting a piece of code that is rarely used already (despite > being a source of significant complexity) and replacing it with code that > is even more complex and would include paths that are even more lightly > used. > I'd be very interested to understand the architectural constraints here. This didn't seem likely to be particularly challenging to implement, but I only know our own stack and certainly could have mispredicted. David
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
