Add support for draft-ietf-tls-downgrade-scsv to NSS

RESOLVED FIXED in 3.17.1

Status

NSS
Libraries
P1
enhancement
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: briansmith, Assigned: Adam Langley)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

In the Google Chrome tree, there is already a patch that implements support for the fallback SCSV:
http://src.chromium.org/viewvc/chrome/trunk/src/net/third_party/nss/patches/fallbackscsv.patch?view=log

I am not sure what the state of that implementation is, with respect to the specification. Wan-Teh and Adam, are there changes to that patch that would be needed to bring it in sync with the current draft? Is there any significant reason why we should wait to add support for this, off by default, to NSS?
Blocks: 1036737

Comment 1

3 years ago
Created attachment 8470326 [details] [diff] [review]
Patch by Adam Langley
Attachment #8470326 - Flags: review?(martin.thomson)

Updated

3 years ago
Assignee: nobody → wtc
Severity: normal → enhancement
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → 3.17
Comment on attachment 8470326 [details] [diff] [review]
Patch by Adam Langley

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

Thanks Wan-Teh.

::: lib/ssl/SSLerrs.h
@@ +421,5 @@
> +
> +ER3(SSL_ERROR_INAPPROPRIATE_FALLBACK_ALERT, (SSL_ERROR_BASE + 131),
> +"The connection was using a lesser TLS version as a result of a previous"
> +" handshake failure, but the server indicated that it should not have been"
> +" needed.")

I had different text here:

The server rejected the handshake because the client downgraded to a lower TLS version than the server supports.

::: lib/ssl/sslproto.h
@@ +209,5 @@
>  #define TLS_EMPTY_RENEGOTIATION_INFO_SCSV       0x00FF
>  
> +/* TLS_FALLBACK_SCSV is a signaling cipher suite value that indicates that a
> + * handshake is the result of TLS version fallback. This value is not IANA
> + * assigned. */

This is now IANA assigned.
Attachment #8470326 - Flags: review?(martin.thomson) → review+

Comment 3

3 years ago
Created attachment 8470345 [details] [diff] [review]
Patch by Adam Langley, v2

Martin: I made the changes you suggested. I won't be able to
check this in until next Monday because the NSS tree is closed
in preparation for the NSS 3.17 release.
Attachment #8470326 - Attachment is obsolete: true
Attachment #8470345 - Flags: review?(martin.thomson)

Updated

3 years ago
Attachment #8470345 - Flags: review?(martin.thomson) → review+
I take it then that this is going to miss 3.17.  When is this likely to land Firefox side?
Flags: needinfo?(dkeeler)
I imagine this will land in NSS 3.17.1. mozilla-central can take betas of 3.17.1 as soon as they're ready, really (see for example bug 1020695).
Flags: needinfo?(dkeeler)

Comment 6

3 years ago
Comment on attachment 8470345 [details] [diff] [review]
Patch by Adam Langley, v2

Patch checked in: https://hg.mozilla.org/projects/nss/rev/45cb71fd7bca
Attachment #8470345 - Flags: checked-in+

Updated

3 years ago
Assignee: wtc → agl
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [3.17.1]

Updated

3 years ago
Whiteboard: [3.17.1]
Target Milestone: 3.17 → 3.17.1

Comment 7

3 years ago
Comment on attachment 8470345 [details] [diff] [review]
Patch by Adam Langley, v2

Patch pushed to mozilla-inbound as part of NSS_3_17_1_BETA1:
https://hg.mozilla.org/integration/mozilla-inbound/rev/102eaae71580
https://hg.mozilla.org/mozilla-central/rev/102eaae71580

Comment 9

3 years ago
Created attachment 8506359 [details] [diff] [review]
patch v2 backported to the NSS_3_16_2_BRANCH

Wan-Teh, does this look right? I have added the dummy bit as discussed, for full binary compatibility (even if it were unnecessary, it shouldn't hurt).
Attachment #8506359 - Flags: review?(wtc)

Updated

3 years ago
Blocks: 1084005

Comment 10

3 years ago
Comment on attachment 8506359 [details] [diff] [review]
patch v2 backported to the NSS_3_16_2_BRANCH

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

r=wtc. Thanks!
Attachment #8506359 - Flags: review?(wtc) → review+

Comment 11

3 years ago
backported patch checked in to NSS_3_16_2_BRANCH
https://hg.mozilla.org/projects/nss/rev/53506619e81a
to be released as part of 3.16.2.3
You need to log in before you can comment on or make changes to this bug.