549042 - Patch to send empty renegotiation info extension instead of SCSV in initial handshakes

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Wan-Teh Chang]

Attachment: Patch (DO NOT CHECK IN)

The attached patch causes NSS SSL clients to send empty renegotiation info
extensions instead of SCSV in initial handshakes unless TLS is disabled.

This allows implementers of server-side secure renegotiation to use the
patched NSS as a test client that sends empty RI extensions (as opposed to
SCSV, or both SCSV and empty RI) in initial handshakes.

Comment 1

[Wan-Teh Chang]

Attachment: Patch v2 (DO NOT CHECK IN)

The previous patch has a minor bug: it uses the wrong SSL version in the
session resumption case.

Comment 2

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

We (NSS team) decided to make this change, targeted for 3.13, to be consistent with Windows SChannel and Chrome.

Comment 3

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

Wan-Teh, do you know the current state of this bug? The patch says "DO NOT CHECK IN" and comment 2 says we agreed to make this change to NSS to be consistent with other implementations. Do we still need to actually make that change?

Comment 4

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

Comment on attachment 429282 [details] [diff] [review]
Patch v2 (DO NOT CHECK IN)

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

I see that this seems to have been used in Chromium for quite a while without problems.

The SCSV is very slightly smaller (two bytes) than the empty RI extension (4 bytes), so in that sense the SCSV is slightly better.

However, when only the SCSV is sent, servers that conform to RFC 5746 break the more general rule that says that a server must not send an extension unless the client sent that extension. This indicates to me that there could be interoperability issues. Did Chromium experience any interoperability issues before you started using this patch?

In general, it is good for Firefox to be consistent with IE and Chromium. That is another argument in favor of taking this patch in upstream NSS. Since we already agreed to do so, I think we should check in the patch now.

Comment 5

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

Wan-Teh, do you agree with checking in this patch for the NSS 3.15.3 release?

Are either of you interested in the potential space savings of doing the opposite, and reverting this patch in Chromium?

I think ultimately we're better off being consistent, though I think it is silly to waste two bytes.

Comment 6

[Wan-Teh Chang]

Attachment: Patch v3

Brian: thank you for the review. I am definitely for checking in
this patch. For some reason I didn't get a "review granted" email
notification (perhaps because I didn't request a review).

I updated the patch to the current NSS trunk and checked it in:
https://hg.mozilla.org/projects/nss/rev/34d14a78ed17

Comment 7

[Kai Engert (:KaiE:)]

changing target milestone to 3.15.4