1311213 - Assertion failure: ss->sec.peerCert, at ../../lib/ssl/tls13con.c:2082 when enabling resume bogo tests

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Jed Davis [:jld] ⟨⏰|UTC-8⟩ ⟦he/him⟧]

Attachment: bugNNNNNNN-nss-bogo-resume.diff

With the attached patch to make the BoGo test -resume-count option work (formerly -resume, but this changed a few months ago in upstream BoringSSL), I get "Assertion failure: ss->sec.peerCert, at ../../lib/ssl/tls13con.c:2082" on these tests:

FAILED (TLS13-AEAD-CHACHA20-POLY1305-client)
FAILED (TLS13-AEAD-AES128-GCM-SHA256-client)
FAILED (Resume-Client-TLS13-TLS13)
FAILED (Resume-Client-CipherMismatch-TLS13)
FAILED (TLS13-SendUnknownModeSessionTicket-Client)
FAILED (TLS13-1RTT-Client-Sync)
FAILED (TLS13-HelloRetryRequest-Client-Sync)
FAILED (TLS13-1RTT-Client-Sync-SplitHandshakeRecords)
FAILED (TLS13-HelloRetryRequest-Client-Sync-SplitHandshakeRecords)
FAILED (TLS13-1RTT-Client-Sync-PackHandshakeFlight)
FAILED (TLS13-HelloRetryRequest-Client-Sync-PackHandshakeFlight)

I don't know what the security implicatiions of this are, so I'm marking this bug security-sensitive to be safe.

Comment 1

[Daniel Veditz [:dveditz]]

looks like a null deref following that?

Comment 2

[Eric Rescorla (:ekr)]

Please leave this security sensitive until we have finished evaluating.

Comment 3

[Eric Rescorla (:ekr)]

Looks like we are losing the peerCert when we get multiple tickets. I will fix.

Comment 4

[Eric Rescorla (:ekr)]

Actually, this looks like it might be a bit complicated. I can fix peerCert specifically, but there's other stuff floating around. I'm starting to think that it was a mistake to delete and remake the sessionID rather than patching the existing one. Not sure how complicated either of these is going to be. 

A simple fix would be to just ignore repeated NST messages

ni-ing ttaubert for thoughts.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Here's another possible solution:

https://nss-dev.phacility.com/D108

We can simply update the ticket store in a cached SID, instead of freeing the SID and recreating it. That passes {A,UB,L}San tests, BoGo, and ssl_gtests.

Comment 6

[Tim Taubert [:ttaubert] (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=93af522199f249bee62fe6e7134bb65d5d9840ef

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

https://nss-review.dev.mozaws.net/D13

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

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