Closed Bug 1309054 Opened 4 years ago Closed 4 years ago

Client shouldn't offer PSK if the PSK cipher suite isn't enabled

Categories

(NSS :: Libraries, defect)

3.28
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mt, Unassigned)

References

Details

In review of bug 1308389, ekr noted that we should be more cautious about offering PSK.

1. The client shouldn't bother to attempt resumption if the cipher suite doesn't match.
2. Similarly, the server shouldn't bother to attempt resumption if it doesn't negotiate the same cipher suite.  (Subset of this: it shouldn't resume if the client doesn't offer the right cipher suite.)

This likely requires a small tweak to the code, but it's mostly just new tests.
Note that TLS 1.2 requires that resumption use the same cipher suite too.  I'm not sure that it's safe to do the same sort of enforcement that was added in bug 1308389, but we can avoid offering resumption from the client if the cipher suite isn't enabled.  Similarly, the server can avoid resuming if it doesn't pick the same cipher suite.

In other words, this isn't a 1.3-only test (whereas the other might have to be).
(In reply to Martin Thomson [:mt:] from comment #0)
> In review of bug 1308389, ekr noted that we should be more cautious about
> offering PSK.
> 
> 1. The client shouldn't bother to attempt resumption if the cipher suite
> doesn't match.

I think this may be missing. It's not "doesn't match" but rather "is in our list".
It should be in ssl3_SendClientHello()


> 2. Similarly, the server shouldn't bother to attempt resumption if it
> doesn't negotiate the same cipher suite.  (Subset of this: it shouldn't
> resume if the client doesn't offer the right cipher suite.)

This is checked here:
http://searchfox.org/nss/source/lib/ssl/tls13con.c#960


> This likely requires a small tweak to the code, but it's mostly just new
> tests.
OK, both had bugs, both found and fixed:
For point 1: https://nss-dev.phacility.com/D84
For point 2: https://nss-dev.phacility.com/D80
You need to log in before you can comment on or make changes to this bug.