1286694 - add tls 1.3 xpcshell test and add tls version configuration function to nsITLSServerSocket in preparation of enabling tls 1.3

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Masatoshi Kimura [:emk]]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a391db733c8

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

Review commit: https://reviewboard.mozilla.org/r/64114/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64114/

Comment 2

[Masatoshi Kimura [:emk]]

I also changed the fallback order to reduce fallback count.
Currently the fallback order is TLS 1.2-DHE -> TLS 1.2+DHE. If TLS 1.3 is enabled, it would be TLS 1.3 -> TLS 1.2+DHE. We enabled only CBC cipher suites for DHE that are not usable in TLS 1.3 anyway.

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

https://reviewboard.mozilla.org/r/64114/#review61372

Looks good in general, but needs a bit of cleanup/clarification.

::: netwerk/base/TLSServerSocket.cpp:249
(Diff revision 1)
>  
>    return NS_OK;
>  }
>  
> +NS_IMETHODIMP
> +TLSServerSocket::SetVersionRange(uint16_t aMinVersion, uint16_t aMaxVersion)

Maybe we should filter out invalid versions here?

::: netwerk/base/nsITLSServerSocket.idl:74
(Diff revision 1)
>     */
>    void setCipherSuites([array, size_is(aLength)] in unsigned short aCipherSuites,
>                         in unsigned long aLength);
> +
> +  /**
> +   * setVersionRange

A necko peer should sign off on this new API and implementation.

::: netwerk/base/nsITLSServerSocket.idl:78
(Diff revision 1)
> +  /**
> +   * setVersionRange
> +   *
> +   * The server's TLS versions that is used by the TLS handshake.
> +   * This is required to be set before calling |asyncListen|.
> +   */

We should document what values are valid to pass to this function.

::: netwerk/test/unit/test_tls_server.js:44
(Diff revision 1)
>    });
>    return deferred.promise;
>  }
>  
> -function startServer(cert, expectingPeerCert, clientCertificateConfig) {
> +function startServer(cert, expectingPeerCert, clientCertificateConfig,
> +                     version, versionStr) {

Maybe s/version/expectedVersion/

::: netwerk/test/unit/test_tls_server.js:127
(Diff revision 1)
>          let data = NetUtil.readInputStreamToString(input, input.available());
>          equal(data, "HELLO", "Echoed data received");
>          input.close();
>          output.close();
>          inputDeferred.resolve();
> +        ok(!expectingBadCertAlert, "No bad cert alert expected");

I think this should happen before resolving inputDeferred, right?

::: netwerk/test/unit/test_tls_server.js:173
(Diff revision 1)
>  
>  // Replace the UI dialog that prompts the user to pick a client certificate.
>  do_load_manifest("client_cert_chooser.manifest");
>  
> -add_task(function*() {
> -  let cert = yield getCert();
> +const tests = [{
> +  expectingPeerCert: true, clientCertificateConfig: Ci.nsITLSServerSocket.REQUIRE_ALWAYS,

I find this a little hard to read. Maybe have one parameter per line:

{
  expectingPeerCert: true,
  clientCertificateConfig: Ci.nsITLSServerSocket.REQUIRE_ALWAYS,
  sendClientCert: true,
  expectingBadCertAlert: true,
},

::: netwerk/test/unit/test_tls_server.js:193
(Diff revision 1)
> -add_task(function*() {
> -  let cert = yield getCert();
> -  ok(!!cert, "Got self-signed cert");
> -  let port = startServer(cert, true, Ci.nsITLSServerSocket.REQUEST_ALWAYS);
> -  storeCertOverride(port, cert);
> -  yield startClient(port, cert, false);
> +  expectingPeerCert: false, clientCertificateConfig: Ci.nsITLSServerSocket.REQUEST_NEVER,
> +  sendClientCert: false, expectingBadCertAlert: false
> +}];
> +
> +const versions = [{
> +  prefValue: 3, version: Ci.nsITLSClientStatus.TLS_VERSION_1_2, versionStr: "TLS 1.2"

Similar situation here.

::: security/manager/ssl/nsNSSIOLayer.cpp:1107
(Diff revision 1)
>        nsNSSComponent::AreAnyFallbackCiphersEnabled()) {
> +    // There is no point in trying fallback cipher suites with TLS 1.3
> +    // because TLS 1.3 only supports AEAD cipher suites.
>      if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
> -                                            socketInfo->GetPort(), err)) {
> +                                            socketInfo->GetPort(), err) &&
> +        range.max < SSL_LIBRARY_VERSION_TLS_1_3) {

Let's hoist this up to line 1102 (in particular, we don't want to run rememberStrongCiphersFailed if we're using TLS 1.3, right?

::: security/manager/ssl/tests/unit/test_fallback_cipher.js:209
(Diff revision 1)
> +    Services.prefs.setIntPref("security.tls.version.max", versionPref);
> +
> -  // disable fallback cipher suites
> +    // disable fallback cipher suites
> -  for (let pref of fallback_cipher_prefs) {
> +    for (let pref of fallback_cipher_prefs) {
> -    Services.prefs.setBoolPref(pref, false);
> +      Services.prefs.setBoolPref(pref, false);
> +      Services.prefs.setIntPref("security.tls.version.fallback-limit", 4);

Why is it necessary to set this when the client is using TLS 1.2 and TLS 1.3? I think this would be a more effective test if we left it at the default value (i.e. how it ships).

Comment 4

[Masatoshi Kimura [:emk]]

https://reviewboard.mozilla.org/r/64114/#review61372

> Maybe we should filter out invalid versions here?

SSL_VersionRangeSet will check the parameter and return the error appropreately. I don't think we need to add a redundant and maybe inconsistent check.

> A necko peer should sign off on this new API and implementation.

:mcmanus, please review the API change.

> Let's hoist this up to line 1102 (in particular, we don't want to run rememberStrongCiphersFailed if we're using TLS 1.3, right?

We need to call rememberStrongCiphersFailed to offer DHE cipher suites in the second (TLS 1.2) handshake.

> Why is it necessary to set this when the client is using TLS 1.2 and TLS 1.3? I think this would be a more effective test if we left it at the default value (i.e. how it ships).

It is necessary to prevent the 1.3-to-1.2 fallback when the client uses TLS 1.3. I slightly modified the logic. Now we will change the pref to a non-default value only when the client uses TLS 1.3.

Comment 5

[Masatoshi Kimura [:emk]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64114/diff/1-2/

Comment 6

[Richard Barnes [:rbarnes]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

This needs review from the team that have been working on TLS 1.3.  Adding a n-i to :mt to figure out who should review.  Putting an r- on this just to hold it while we sort things out.

Comment 7

[Martin Thomson [:mt:]]

https://reviewboard.mozilla.org/r/64114/#review61604

::: security/manager/ssl/nsNSSIOLayer.cpp:1103
(Diff revision 2)
> +    // There is no point in trying fallback cipher suites with TLS 1.3
> +    // because TLS 1.3 only supports AEAD cipher suites.
>      if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
> -                                            socketInfo->GetPort(), err)) {
> +                                            socketInfo->GetPort(), err) &&
> +        range.max < SSL_LIBRARY_VERSION_TLS_1_3) {

We can still send out a 1.3 ClientHello with bad cipher suites in it.  So this just has the strange effect of disabling cipher suite fallback if 1.3 is enabled.

Comment 8

[Martin Thomson [:mt:]]

I think that keeler, dragana/mcpersonus, and (as Richard says) myself/ekr should review this too.

In particular, we need more telemetry before turning on 1.3.  That includes fixes to the fallback telemetry, which is busted for 1.3, as well as performance-related telemetry (some thinking is required there).

Comment 9

[Masatoshi Kimura [:emk]]

(In reply to Martin Thomson [:mt:] from comment #7)
> https://reviewboard.mozilla.org/r/64114/#review61604
> 
> ::: security/manager/ssl/nsNSSIOLayer.cpp:1103
> (Diff revision 2)
> > +    // There is no point in trying fallback cipher suites with TLS 1.3
> > +    // because TLS 1.3 only supports AEAD cipher suites.
> >      if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),
> > -                                            socketInfo->GetPort(), err)) {
> > +                                            socketInfo->GetPort(), err) &&
> > +        range.max < SSL_LIBRARY_VERSION_TLS_1_3) {
> 
> We can still send out a 1.3 ClientHello with bad cipher suites in it.  So
> this just has the strange effect of disabling cipher suite fallback if 1.3
> is enabled.

We can, but we decided not to send it in the first handshake (bug 1279479). Although retryDueToTLSIntolerance doesn't return here, it will call rememberIntolerantAtVersion and return false eventually. So it will fallback with TLS 1.2 with fallback cipher suites. I modified test_fallback_cipher.js to verify this behavior.

Comment 10

[Martin Thomson [:mt:]]

This is exactly the kind of mess that really makes this stuff impossible to reason about (though that could be lack of sleep).  What is the status of our weak cipher suites?  Are we in a position to remove "weak" ciphers entirely?

Comment 11

[Masatoshi Kimura [:emk]]

https://reviewboard.mozilla.org/r/64114/#review61604

> We can still send out a 1.3 ClientHello with bad cipher suites in it.  So this just has the strange effect of disabling cipher suite fallback if 1.3 is enabled.

We can, but we decided not to send it in the first handshake (bug 1279479). Although retryDueToTLSIntolerance doesn't return here, it will call rememberIntolerantAtVersion and return false eventually. So it will fallback with TLS 1.2 with fallback cipher suites. I modified test_fallback_cipher.js to verify this behavior.

Comment 12

[Masatoshi Kimura [:emk]]

(In reply to Martin Thomson [:mt:] from comment #10)
> This is exactly the kind of mess that really makes this stuff impossible to
> reason about (though that could be lack of sleep).  What is the status of
> our weak cipher suites?  Are we in a position to remove "weak" ciphers
> entirely?

Currently DHE-based cipher suites are hidden from the first handshake. They will be completely removed in the future (bug 1227519).

Other "weak" cipher suites (e.g. static RSA, CBC, and SHA-1) will be removed in the remote future, but no ETA at the moment.

Comment 13

[Masatoshi Kimura [:emk]]

Attachment: Bug 1286694 - Part 1: Modify test_tls_server.js to test TLS 1.3.

Review commit: https://reviewboard.mozilla.org/r/64754/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64754/

Comment 14

[Masatoshi Kimura [:emk]]

Attachment: Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled.

Review commit: https://reviewboard.mozilla.org/r/64756/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64756/

Comment 15

[Masatoshi Kimura [:emk]]

Attachment: Bug 1286694 - Part 4: Enable TLS 1.3 in Nightly

Review commit: https://reviewboard.mozilla.org/r/64758/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/64758/

Comment 16

[Masatoshi Kimura [:emk]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64114/diff/2-3/

Comment 17

[Masatoshi Kimura [:emk]]

I divided the patch to make review easier.

Comment 18

[Martin Thomson [:mt:]]

Comment on attachment 8771687 [details]
Bug 1286694 - Part 4: Enable TLS 1.3 in Nightly

https://reviewboard.mozilla.org/r/64758/#review61788

This is fine, but I can't approve this without having a way to measure the impact of this change.

Comment 19

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8771685 [details]
Bug 1286694 - Part 1: Modify test_tls_server.js to test TLS 1.3.

https://reviewboard.mozilla.org/r/64754/#review62292

This looks good.

::: netwerk/test/unit/test_tls_server.js:23
(Diff revision 1)
>                              .getService(Ci.nsICertOverrideService);
>  const socketTransportService =
>    Cc["@mozilla.org/network/socket-transport-service;1"]
>    .getService(Ci.nsISocketTransportService);
>  
> +const prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);

nit: watch out for lines >80 characters

::: netwerk/test/unit/test_tls_server.js:215
(Diff revision 1)
> +
> +add_task(function*() {
> +  for (let v of versions) {
> +    prefs.setIntPref("security.tls.version.max", v.prefValue);
> +    for (let t of tests) {
> +      let cert = yield getCert();

Might as well hoist this outside of the t of tests loop (we don't need to get a new copy each time, right?)

Comment 20

[Masatoshi Kimura [:emk]]

Comment on attachment 8771685 [details]
Bug 1286694 - Part 1: Modify test_tls_server.js to test TLS 1.3.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64754/diff/1-2/

Comment 21

[Masatoshi Kimura [:emk]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64114/diff/3-4/

Comment 22

[Masatoshi Kimura [:emk]]

Comment on attachment 8771686 [details]
Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64756/diff/1-2/

Comment 23

[Masatoshi Kimura [:emk]]

Comment on attachment 8771687 [details]
Bug 1286694 - Part 4: Enable TLS 1.3 in Nightly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64758/diff/1-2/

Comment 24

[Masatoshi Kimura [:emk]]

Comment on attachment 8771687 [details]
Bug 1286694 - Part 4: Enable TLS 1.3 in Nightly

Forgot to remove the review request from the changeset summary

Comment 25

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8771686 [details]
Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled.

https://reviewboard.mozilla.org/r/64756/#review62298

Thanks for breaking this up into separate components. r- for now, though - see comments.

::: security/manager/ssl/nsNSSIOLayer.cpp:1105
(Diff revision 1)
>    if ((err == SSL_ERROR_NO_CYPHER_OVERLAP || err == PR_END_OF_FILE_ERROR ||
>         err == PR_CONNECT_RESET_ERROR) &&
>        nsNSSComponent::AreAnyFallbackCiphersEnabled()) {
> +    // There is no point in trying fallback cipher suites with TLS 1.3
> +    // because TLS 1.3 only supports AEAD cipher suites.
>      if (helpers.rememberStrongCiphersFailed(socketInfo->GetHostName(),

Ok - now I get what's going on here. By calling rememberStrongCiphersFailed but not returning true until after we've also called rememberIntolerantAtVersion, this will go from TLS 1.3 without weak ciphers straight to TLS 1.2 with weak ciphers. I think we should instead go from TLS 1.3 without weak ciphers to TLS 1.2 without weak ciphers and then to TLS 1.2 with weak ciphers, because if TLS 1.3 doesn't even support non-AEAD cipher suites, the problem is likely to be TLS 1.3-intolerance, not a cipher suite mismatch.

::: security/manager/ssl/tests/unit/test_fallback_cipher.js:210
(Diff revision 1)
> +
> -  // disable fallback cipher suites
> +    // disable fallback cipher suites
> -  for (let pref of fallback_cipher_prefs) {
> +    for (let pref of fallback_cipher_prefs) {
> -    Services.prefs.setBoolPref(pref, false);
> +      Services.prefs.setBoolPref(pref, false);
> +      if (versionPref >= 4) {
> +        Services.prefs.setIntPref("security.tls.version.fallback-limit", 4);

This shouldn't be in the for loop, right?

::: security/manager/ssl/tests/unit/test_fallback_cipher.js:215
(Diff revision 1)
> +        Services.prefs.setIntPref("security.tls.version.fallback-limit", 4);
> +      }
> -  }
> +    }
>  
> -  yield startClient("server: fallback only, client: fallback disabled, public",
> +    yield startClient("server: fallback only, client: fallback disabled, public",
> -                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
> +                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));

It occurs to me that this doesn't really test what we think it does, since even in the fallback cases we get the same error once and then it succeeds. We should probably duplicate each of these startClient lines (this can be in a follow-up bug, though).

::: security/manager/ssl/tests/unit/test_fallback_cipher.js:223
(Diff revision 1)
> -                    {isPrivate: true});
> +                      {isPrivate: true});
>  
> -  // enable fallback cipher suites
> +    // enable fallback cipher suites
> -  for (let pref of fallback_cipher_prefs) {
> +    for (let pref of fallback_cipher_prefs) {
> -    Services.prefs.setBoolPref(pref, true);
> +      Services.prefs.setBoolPref(pref, true);
> +      Services.prefs.clearUserPref("security.tls.version.fallback-limit");

Similarly, this should be outside the for loop.

Comment 26

[Masatoshi Kimura [:emk]]

https://reviewboard.mozilla.org/r/64756/#review62298

> Ok - now I get what's going on here. By calling rememberStrongCiphersFailed but not returning true until after we've also called rememberIntolerantAtVersion, this will go from TLS 1.3 without weak ciphers straight to TLS 1.2 with weak ciphers. I think we should instead go from TLS 1.3 without weak ciphers to TLS 1.2 without weak ciphers and then to TLS 1.2 with weak ciphers, because if TLS 1.3 doesn't even support non-AEAD cipher suites, the problem is likely to be TLS 1.3-intolerance, not a cipher suite mismatch.

Server can choose TLS 1.2 and CBC ciphers even if ClientHello.client_version indicates TLS 1.3. So IMO the "TLS 1.2 with weak ciphers" step is redundant. Although TLS 1.3 intolerant servers will see DHE ciphers, I would not like to add an extra fallback dance only for such an edge case. This omission will not cause connection failures.

Comment 27

[Masatoshi Kimura [:emk]]

Comment on attachment 8771686 [details]
Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64756/diff/2-3/

Comment 28

[Masatoshi Kimura [:emk]]

Comment on attachment 8771687 [details]
Bug 1286694 - Part 4: Enable TLS 1.3 in Nightly

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/64758/diff/2-3/

Comment 29

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b1a5c29d781b6ba6222f4be26094e4c8ae8d8943
Bug 1286694 - Part 1: Modify test_tls_server.js to test TLS 1.3. r=keeler

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1a5c29d781b
Part 1: Modify test_tls_server.js to test TLS 1.3. r=keeler

Comment 31

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b1a5c29d781b

Comment 32

[Dragana Damjanovic [:dragana]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

https://reviewboard.mozilla.org/r/64114/#review62550

Comment 33

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8771686 [details]
Bug 1286694 - Part 3: Change the fallback sequence to reduce a redundant fallback when TLS 1.3 is enabled.

https://reviewboard.mozilla.org/r/64756/#review62972

I had to think about this for a while, and I've come to the conclusion that this is not sufficiently necessary to change how fallback works right now. See below.

::: security/manager/ssl/nsNSSIOLayer.cpp:1058
(Diff revision 3)
>  
>    if ((err == SSL_ERROR_NO_CYPHER_OVERLAP || err == PR_END_OF_FILE_ERROR ||
>         err == PR_CONNECT_RESET_ERROR) &&
>        nsNSSComponent::AreAnyFallbackCiphersEnabled()) {
> +    // There is no point in trying fallback cipher suites with TLS 1.3
> +    // because TLS 1.3 only supports AEAD cipher suites.

Given comment 26, this comment is a bit misleading, since as you said, a server could choose TLS 1.2 and a CBC cipher suite even if the client advertised TLS 1.3. Echoing comment 10, I think this is an unnecessary optimization that makes this (already difficult) code harder to reason about. Let's just leave the cipher suite fallback as it is for now. Maybe after some additional telemetry we could see if this would actually have a significant performance impact.

::: security/manager/ssl/tests/unit/test_fallback_cipher.js:216
(Diff revision 3)
> +      Services.prefs.setIntPref("security.tls.version.fallback-limit", 4);
> +    }
>  
> -  yield startClient("server: fallback only, client: fallback disabled, public",
> +    yield startClient("server: fallback only, client: fallback disabled, public",
> -                    port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
> +                      port, getXPCOMStatusFromNSS(SSL_ERROR_NO_CYPHER_OVERLAP));
> +    // make sure we do not remember the intolerance

I would say "Retry manually to simulate the HTTP layer. This should still fail."

Comment 35

[Martin Thomson [:mt:]]

:emk, is there any reason you couldn't land those changes that you have positive reviews on?  They look pretty reasonable to me.

Comment 36

[Masatoshi Kimura [:emk]]

Again, probably because of bug 1096768. I'll try to land r+'ed reviews manually.

Comment 37

[Masatoshi Kimura [:emk]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e2b6dc7bebc18c1faeb7b4a6c27606fe329433ec

Comment 38

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c326d0e55f74
Part 2: Add TLS version configuration function to nsITLSServerSocket. r=dragana

Comment 39

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c326d0e55f74

Comment 40

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Updating bug summary to describe the work that was checked in to the tree as a result of it. From what I can tell, there is still some follow-up work (e.g. in fallback, maybe more tests). That can be done in follow-up bugs so as to not complicate this bug further.

Comment 41

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

Approval Request Comment
[Feature/Bug causing the regression]: required for the test in bug 1323843, which is required for the feature added in bug 1321783 (the "use conservative TLS settings" flag)
[User impact if declined]: there won't be a test for the new feature
[Is this code covered by automated tests?]: well, it basically adds a feature that is only used in tests
[Has the fix been verified in Nightly?]: not verified by QE, no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none (I'm pretty sure this time - https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a30548d275189095e8a62cef20ccd09bfbc3124 looks good)
[Is the change risky?]: no
[Why is the change risky/not risky?]: this is borderline test-only, but since it does add a feature to a component that is used in things other than tests (i.e. the devtools), I thought it best to get explicit approval. That said, I think this is a very low risk change since the code being added will only be called from the test in bug 1323843.
[String changes made/needed]: none

(Note to uplifter: only this patch needs uplift, as the other on actually landed in 50)

Comment 42

[Gerry Chang [:gchang]]

Comment on attachment 8770739 [details]
Bug 1286694 - Part 2: Add TLS version configuration function to nsITLSServerSocket.

This is required by bug 1323843 and we also need bug 1323843 in Beta51 as well because bug 1321783 got uplifted to beta. Beta51+. Should be in 51 beta 14.

Comment 43

[Dana Keeler (she/her) (use needinfo) [:keeler]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bdc4df57a2234d32aefe8470593dee3f2e682ce1