Closed Bug 1481271 Opened 3 years ago Closed 3 years ago

[TLS1.3] NSS includes PSK identifiers from a different ticket after HRR

Categories

(NSS :: Libraries, defect)

3.38
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: hkario, Assigned: ueno)

Details

(Keywords: sec-other)

Attachments

(3 files, 1 obsolete file)

When running the strsclnt against gnutls or openssl server, some connection are rejected by server. Both gnutls and openssl error messages indicate that the calculated binder values are incorrect.

Using openssl-1.1.1-pre8.

openssl s_server -www -groups P-256 -CAfile trust.pem -build_chain -cert rsa-server/cert.pem -key rsa-server/key.pem -keylogfile openssl_keylog.txt -ciphersuites TLS_AES_256_GCM_SHA384

strsclnt -c 10 -P 20 -p 4433 -C :1302 -d sql:./ca-db/ -V tls1.3:tls1.3 localhost



strsclnt log:
strsclnt: -- SSL: Server Certificate Validated.
strsclnt: 0 cache hits; 0 cache misses, 0 cache not reusable
          0 stateless resumes
strsclnt: -- SSL: Server Certificate Validated.
strsclnt: PR_Send returned error -12226, OS error 0: SSL peer rejected a handshake message for unacceptable content.
strsclnt: PR_Send returned error -12226, OS error 0: SSL peer rejected a handshake message for unacceptable content.
strsclnt: 5 cache hits; 0 cache misses, 0 cache not reusable
          5 stateless resumes
strsclnt: PR_Send returned error -12226, OS error 0: SSL peer rejected a handshake message for unacceptable content.


OpenSSL log:
140315557500736:error:141FA0FD:SSL routines:tls_psk_do_binder:binder does not verify:ssl/statem/extensions.c:1586:
140315557500736:error:141FA0FD:SSL routines:tls_psk_do_binder:binder does not verify:ssl/statem/extensions.c:1586:
140315557500736:error:141FA0FD:SSL routines:tls_psk_do_binder:binder does not verify:ssl/statem/extensions.c:1586:
Attached file capture.pcap
packet capture of the above mentioned interaction
Attached file openssl_keylog.txt
key log file for the capture.pcap
Marking security until we sort this out.
Group: crypto-core-security
I think it may be HRR related - with both GnuTLS and OpenSSL I have configured the server to support just P-256, while the default key_share for NSS is X25519, with no way to override in strsclnt
Thanks, that's helpful. Any chance you have the keylogfile for NSS?
OK, I am able to reproduce this.
I think I have the problem.

When we generate the CH after HRR, we look in the session cache again. However, we do not *redo* tls13_SetupClientHello, which computes the keys. Consider the following timeline:


Conn 1: <handshake>
Conn 1: S->C: NST=X
Conn 2: C->S: CH [PSK_ID=X]
Conn 1: S->C: NST=Y
Conn 2: S->C: HRR
Conn 2: C->S: CH [PSK_ID=Y]


This already violates the spec, but the reason the binder fails is that we don't recompute the keys, so we use the key for X but the ID of Y.
MT, this is a defect, but I don't think it's a security-relevant defect. Do you disagree?
Flags: needinfo?(martin.thomson)
Based on your description, yes.  It's not even much of a noticeable defect period.  At worst it means that we suffer a few more signature verifications and lose some 0-RTT.  So it's a performance hit.  We should fix it quickly, but security is not something to worry about.
Flags: needinfo?(martin.thomson)
Actually, I think it will cause connection failures because we don't retry.
Franziskus, can you take a crack at fixing this (confirming my diagnosis first, please)
> it will cause connection failures because we don't retry

Ahh right, that's not a retry-relevant error code.
Summary: [TLS1.3] NSS sometimes generates binder than can not be verified by OpenSSL or GnuTLS → [TLS1.3] NSS regenerates PSK binders from a different ticket after HRR
Summary: [TLS1.3] NSS regenerates PSK binders from a different ticket after HRR → [TLS1.3] NSS includes PSK identifiers from a different ticket after HRR
This should fix the issue by making sure that the binding keys are re-calculated. I still have to write a test that actually checks this. But in case someone wants test the patch.
Unfortunately, this is the wrong fix, as noted in c7. The relevant text in S 4.1.2 forbids adding new PSKs in the new CH,

   When a client first connects to a server, it is REQUIRED to send the
   ClientHello as its first TLS message.  The client will also send a
   ClientHello when the server has responded to its ClientHello with a
   HelloRetryRequest.  In that case, the client MUST send the same
   ClientHello without modification, except as follows:


...

   -  Updating the "pre_shared_key" extension if present by recomputing
      the "obfuscated_ticket_age" and binder values and (optionally)
      removing any PSKs which are incompatible with the server's
      indicated cipher suite.
Keywords: sec-other
This is an another attempt to fix the issue: store the sent session ticket in `ssl3.hs` until the client receives ServerHello.
Test is not ready as I couldn't find any easy way to establish multiple connections in gtests to reproduce the scenario described in comment 7.
Attachment #8999202 - Attachment is obsolete: true
QA Contact: franziskuskiefer
Pushed as:
https://hg.mozilla.org/projects/nss/rev/6fa77de9d93b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.41
Assignee: nobody → dueno
Group: crypto-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.