TLS 1.3 resumption fails to carry over peer cert

RESOLVED FIXED in 3.24

Status

NSS
Libraries
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mt, Assigned: mt)

Tracking

Firefox Tracking Flags

(firefox48 affected)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
This was an issue during integration with Cloudflare.
(Assignee)

Comment 1

2 years ago
Created attachment 8737604 [details] [diff] [review]
bug1261677.patch
Attachment #8737604 - Flags: review?(ekr)
(Assignee)

Comment 2

2 years ago
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.
Flags: needinfo?(ekr)

Comment 3

2 years ago
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.
Attachment #8737604 - Flags: review?(ekr) → review+
(Assignee)

Comment 4

2 years ago
https://hg.mozilla.org/projects/nss/rev/53601845beb3
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.24

Comment 5

2 years ago
This broke the build
https://circleci.com/gh/nss-dev/nss/446?utm_campaign=build-failed&utm_medium=email&utm_source=notification
Status: RESOLVED → REOPENED
Flags: needinfo?(ekr) → needinfo?(martin.thomson)
Resolution: FIXED → ---
(Assignee)

Comment 6

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

https://hg.mozilla.org/projects/nss/rev/973932c87cc9
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Flags: needinfo?(martin.thomson)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.