1261677 - TLS 1.3 resumption fails to carry over peer cert

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Martin Thomson [:mt:]]

This was an issue during integration with Cloudflare.

Comment 1

[Martin Thomson [:mt:]]

Attachment: bug1261677.patch

Comment 2

[Martin Thomson [:mt:]]

We need this if we are going to turn TLS 1.3 on in Firefox.  I haven't completely satisfied myself that there isn't something else that we need to carry over, but this is the obvious piece.

Comment 3

[Eric Rescorla (:ekr)]

Comment on attachment 8737604 [details] [diff] [review]
bug1261677.patch

Review of attachment 8737604 [details] [diff] [review]:
-----------------------------------------------------------------

Forgot to hit LGTM.

::: external_tests/ssl_gtest/ssl_loopback_unittest.cc
@@ +981,5 @@
>    SendReceive();
>    CheckKeys(ssl_kea_ecdh, ssl_auth_rsa);
>    DataBuffer psk2(c2->extension());
>    ASSERT_GE(psk2.len(), 0UL);
> +  ASSERT_TRUE(!!client_->peer_cert());

Can you add a test that the cipher suite is right on double-resume?

::: lib/ssl/tls13con.c
@@ +1011,5 @@
>          }
>  
>          tls13_RestoreCipherInfo(ss, sid);
>  
> +        ss->ssl3.hs.isResuming = PR_TRUE;

I don't think this does anything in TLS 1.3. I would suggest that we just remove all references in this code.

Comment 4

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/53601845beb3

Comment 5

[Eric Rescorla (:ekr)]

This broke the build
https://circleci.com/gh/nss-dev/nss/446?utm_campaign=build-failed&utm_medium=email&utm_source=notification

Comment 6

[Martin Thomson [:mt:]]

I had this fix on another branch, might as well bring it forward.

https://hg.mozilla.org/projects/nss/rev/973932c87cc9