Closed Bug 1306985 Opened 3 years ago Closed 3 years ago

Re-disable TLS 1.3 by default

Categories

(NSS :: Build, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: ekr, Unassigned)

Details

Attachments

(1 file, 1 obsolete file)

We are seeing people who appear to be using the max version even though TLS 1.3 is still experimental. This patch just disables 1.3 and we can back out the build changes when we make TLS 1.3 non-experiment.
Attachment #8796966 - Flags: review?(martin.thomson)
Attachment #8796966 - Flags: review?(kaie)
Attachment #8796969 - Flags: review?(martin.thomson)
Attachment #8796966 - Attachment is obsolete: true
Attachment #8796966 - Flags: review?(martin.thomson)
Attachment #8796966 - Flags: review?(kaie)
Attachment #8796969 - Flags: review?(kaie)
Comment on attachment 8796969 [details] [diff] [review]
0001-Disable-TLS-1.3-by-default.patch

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

I see that you landed this already.

You (still) really need to fix that context problem.
Attachment #8796969 - Flags: review?(martin.thomson) → review+
No, I didn't land it. That was on try.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Comment on attachment 8796969 [details] [diff] [review]
0001-Disable-TLS-1.3-by-default.patch

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

Please see my comment about external_tests/ssl_gtest/ssl_auth_unittest.cc.

I find it confusing to have both NSS_ENABLE_TLS_1_3 and NSS_DISABLE_TLS_1_3
as makefile variables. I probably would prefer having only NSS_ENABLE_TLS_1_3,
but I can live with having both.

::: external_tests/ssl_gtest/Makefile
@@ +40,3 @@
>  ifdef NSS_DISABLE_TLS_1_3
>  # Run parameterized tests only, for which we can easily exclude TLS 1.3
>  CPPSRCS := $(filter-out $(shell grep -l '^TEST_F' $(CPPSRCS)), $(CPPSRCS))

IMPORTANT: external_tests/ssl_gtest/ssl_auth_unittest.cc still
has code that tests the NSS_ENABLE_TLS_1_3 macro:

#ifdef NSS_ENABLE_TLS_1_3
TEST_P(TlsConnectTls13, SignatureAlgorithmServerUnsupported) {
  ...
}

TEST_P(TlsConnectTls13, SignatureAlgorithmClientUnsupported) {
  ...
}
#endif

Ideally we should change #ifdef NSS_ENABLE_TLS_1_3 to
#ifndef NSS_DISABLE_TLS_1_3.
Attachment #8796969 - Flags: review+
Attachment #8796969 - Flags: review?(kaie)
You need to log in before you can comment on or make changes to this bug.