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

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Hubert Kario]

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:

Comment 1

[Hubert Kario]

Attachment: capture.pcap

packet capture of the above mentioned interaction

Comment 2

[Hubert Kario]

Attachment: openssl_keylog.txt

key log file for the capture.pcap

Comment 3

[Eric Rescorla (:ekr)]

Marking security until we sort this out.

Comment 4

[Hubert Kario]

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

Comment 5

[Eric Rescorla (:ekr)]

Thanks, that's helpful. Any chance you have the keylogfile for NSS?

Comment 6

[Eric Rescorla (:ekr)]

OK, I am able to reproduce this.

Comment 7

[Eric Rescorla (:ekr)]

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.

Comment 8

[Eric Rescorla (:ekr)]

MT, this is a defect, but I don't think it's a security-relevant defect. Do you disagree?

Comment 9

[Martin Thomson [:mt:]]

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.

Comment 10

[Eric Rescorla (:ekr)]

Actually, I think it will cause connection failures because we don't retry.

Comment 11

[Eric Rescorla (:ekr)]

Franziskus, can you take a crack at fixing this (confirming my diagnosis first, please)

Comment 12

[Martin Thomson [:mt:]]

> it will cause connection failures because we don't retry

Ahh right, that's not a retry-relevant error code.

Comment 13

[Franziskus Kiefer [:franziskus]]

Attachment: Bug 1481271 - recalculate binding keys on 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.

Comment 14

[Eric Rescorla (:ekr)]

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.

Comment 15

[Daiki Ueno [:ueno]]

Attachment: Bug 1481271, resend the same ticket in ClientHello after HRR

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.

Comment 16

[Daiki Ueno [:ueno]]

Pushed as:
https://hg.mozilla.org/projects/nss/rev/6fa77de9d93b