Closed Bug 1247250 Opened 4 years ago Closed 4 years ago

Enable TLS 1.3 anti-downgrade on non-secure fallback

Categories

(Core :: Security: PSM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: emk, Unassigned)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
TLS 1.2 clients can also use this feature.
Attachment #8717875 - Flags: review?(dkeeler)
Comment on attachment 8717875 [details] [diff] [review]
patch

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

Ok - r=me with comments updated.

::: security/manager/ssl/nsNSSIOLayer.cpp
@@ +1077,5 @@
>      socketInfo->SetSecurityState(nsIWebProgressListener::STATE_IS_INSECURE |
>                                   nsIWebProgressListener::STATE_USES_SSL_3);
>    }
>  
> +  // NSS will return SSL_ERROR_RX_MALFORMED_SERVER_HELLO if TLS 1.3

We haven't enabled TLS 1.3 yet (unless I've missed something), so this comment is at least incomplete, if not just confusing.

@@ +2557,5 @@
>             ("[%p] nsSSLIOLayerSetOptions: enabling TLS_FALLBACK_SCSV\n", fd));
>      if (SECSuccess != SSL_OptionSet(fd, SSL_ENABLE_FALLBACK_SCSV, true)) {
>        return NS_ERROR_FAILURE;
>      }
> +    // tell NSS to enable the max enabled version to make TLS 1.3

Again, this comment seems incomplete.
Attachment #8717875 - Flags: review?(dkeeler) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/374e6d0abf0ebc19e84376181276f5cc53a6914a
Bug 1247250 - Enable TLS 1.3 draft 11 anti-downgrade on non-secure fallback. r=keeler
(In reply to David Keeler [:keeler] (use needinfo?) from comment #1)
> We haven't enabled TLS 1.3 yet (unless I've missed something), so this
> comment is at least incomplete, if not just confusing.

I removed the word "TLS 1.3" because NSS will always check the downgrade even if TLS 1.3 is disabled.
https://hg.mozilla.org/integration/mozilla-inbound/rev/8aded3a039f55ad950c995422fee5c880f626f01
Bug 1247250 - followup: fix comments to reflect the review comment. r=keeler DONTBUILD
Oops, forgot qrefresh.
Backed out for bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/c56ae44b87f6

Job with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=374e6d0abf0e
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=21576492&repo=mozilla-inbound

15:11:57     INFO -      INPUT("StaticXULComponentsEnd/StaticXULComponentsEnd.o")
15:11:57     INFO -  ../../security/manager/ssl/Unified_cpp_security_manager_ssl2.o: In function `nsSSLIOLayerSetOptions':
15:11:57     INFO -  /builds/slave/m-in-lx-0000000000000000000000/build/src/security/manager/ssl/nsNSSIOLayer.cpp:2563: undefined reference to `SSL_SetDowngradeCheckVersion'
15:11:57     INFO -  collect2: error: ld returned 1 exit status
15:11:57     INFO -  gmake[5]: *** [libxul.so] Error 1
15:11:57     INFO -  gmake[5]: Leaving directory `/builds/slave/m-in-lx-0000000000000000000000/build/src/obj-firefox/toolkit/library'
15:11:57     INFO -  gmake[4]: *** [toolkit/library/target] Error 2
15:11:57     INFO -  gmake[4]: Leaving directory `/builds/slave/m-in-lx-0000000000000000000000/build/src/obj-firefox'
15:11:57     INFO -  gmake[3]: *** [compile] Error 2
15:11:57     INFO -  gmake[3]: Leaving directory `/builds/slave/m-in-lx-0000000000000000000000/build/src/obj-firefox'
15:11:57     INFO -  gmake[2]: *** [default] Error 2
15:11:57     INFO -  gmake[2]: Leaving directory `/builds/slave/m-in-lx-0000000000000000000000/build/src/obj-firefox'
15:11:57     INFO -  gmake[1]: *** [realbuild] Error 2
15:11:57     INFO -  gmake[1]: Leaving directory `/builds/slave/m-in-lx-0000000000000000000000/build/src'
15:11:57     INFO -  gmake: *** [build] Error 2
Flags: needinfo?(VYV03354)
Linux builds do not use config/external/nss/nss.symbols and security/nss/lib/ssl/ssl.def does not contain SSL_SetDowngradeCheckVersion yet :(
Flags: needinfo?(VYV03354)
Depends on: 1247819
Depends on: 1245053
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca2b8152c721a2c2442912ad9fc4b05685a9523c
Bug 1247250 - Enable TLS 1.3 anti-downgrade on non-secure fallback. r=keeler
https://hg.mozilla.org/mozilla-central/rev/ca2b8152c721
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.