1714245 - Remove PSKs with incompatible hashes after HRR

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P4)

Description

[Martin Thomson [:mt:]]

When we receive a TLS HelloRetryRequest, that fixes the choice of cipher suite. That means that the PSK we have might not be valid if it uses a different hash.

Rather than include the PSK in this case, we can save a bunch of effort by dropping it. This is permitted by the protocol.

Saving effort also means that we don't need to think too hard about how the transcript for the PSK binder is constructed with mismatched hash functions (one for the ClientHello compression, another for the binder). It looks like our code is correct in this regard, but it would need careful validation to be sure. As this is a nonsensical situation it is better to avoid the question entirely.

Comment 1

[Eric Rescorla (:ekr)]

Marking security until Benjamin has a chance to take a look, just to be sure.

Comment 2

[Benjamin Beurdouche [:beurdouche]]

Dennis, when you have a few mins could you look at this bug please? : ) Thanks!

Comment 3

[Dennis Jackson]

RFC 8446 §4.1.4

In addition, in its updated ClientHello, the client SHOULD NOT offer any pre-shared keys associated with a hash other than that of the selected cipher suite. This allows the client to avoid having to compute partial hash transcripts for multiple hashes in the second ClientHello.

If I have understood correctly, dropping PSKs with the wrong hash function is required by the spec, so we should definitely do this.

Comment 4

[Martin Thomson [:mt:]]

Right. We don't have to drop them and it was easier to keep them at the time. There are lots of reasons to do the work though.

Separately, if you can work out whether we have any concrete problems in existing code, we can work out whether to keep this as a security bug. We were cautious initially, but I haven't seen any cause for concern. Confirmation that it is not a security bug would be helpful.

Comment 5

[Leander Schwarz]

Attachment: Bug 1714245 - On HRR skip PSK incompatible with negotiated ciphersuites hash algorithm. r?djackson

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:djackson, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit auto_nag documentation.

Comment 7

[Dennis Jackson]

https://hg.mozilla.org/projects/nss/rev/ab31b109e13e3b550556ec984d22a31b61a08a4a