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

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement, P1)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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?

Comment 1

[Wan-Teh Chang]

Attachment: Patch by Adam Langley

Comment 2

[Martin Thomson [:mt:]]

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.

Comment 3

[Wan-Teh Chang]

Attachment: 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.

Comment 4

[Martin Thomson [:mt:]]

I take it then that this is going to miss 3.17.  When is this likely to land Firefox side?

Comment 5

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

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).

Comment 6

[Wan-Teh Chang]

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

Patch checked in: https://hg.mozilla.org/projects/nss/rev/45cb71fd7bca

Comment 7

[Wan-Teh Chang]

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

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/102eaae71580

Comment 9

[Kai Engert (:KaiE:)]

Attachment: 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).

Comment 10

[Wan-Teh Chang]

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!

Comment 11

[Kai Engert (:KaiE:)]

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