1102632 - Remove SSL_ERROR_UNSUPPORTED_VERSION from insecure-fallback-enabled error code list

Status: REOPENED

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

Description

[Masatoshi Kimura [:emk]]

Before bug 1084986:
We disables SSLv2, so NSS will never return this error.

After bug 1084986:
If NSS returns this error, it means that the server sent back a ServerHello message with a negotiated version.
The server didn't disconnect abruptly, it didn't send an alert. In other words, the server negotiated the version securely.
Therefore it's a waste of time to fallback with lower max versions.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Stop triggering non-secure fallback for SSL_ERROR_UNSUPPORTED_VERSION

Comment 2

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

Comment on attachment 8526740 [details] [diff] [review]
Stop triggering non-secure fallback for SSL_ERROR_UNSUPPORTED_VERSION

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

OK - seems reasonable. Let's monitor telemetry after bug 1084986 to make sure we're not encountering this error in this code (if the 0 bucket starts collecting hits, it'll probably be because of this).

Comment 3

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d4d4a91f29a

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3d4d4a91f29a

Comment 5

[Masatoshi Kimura [:emk]]

Backed out because 111 of 561 sites couldn't connect without this fallback reason.
https://hg.mozilla.org/integration/mozilla-inbound/rev/76ac4c3323a9
My assumption in comment #0 was wrong :(

Comment 6

[Masatoshi Kimura [:emk]]

> 111 of 561 sites
The list of 561 sites is bug 1084025 comment #96.

Comment 7

[Masatoshi Kimura [:emk]]

Attachment: backout patch

Approval Request Comment
[Feature/regressing bug #]: 1102632 (this bug)
[User impact if declined]: Users can no longer connect some sites without enabling SSLv3.
[Describe test coverage new/current, TreeHerder]: Manually tested
[Risks and why]: Very low. Just a backout of one line removal.
[String/UUID change made/needed]: none

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8557473 [details] [diff] [review]
backout patch

Aurora+ for the backout.

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

I take it that this needs to be backed out of 38 as well. Can you please confirm?

Comment 10

[Masatoshi Kimura [:emk]]

Already backed out from Nightly (38):
https://hg.mozilla.org/mozilla-central/rev/76ac4c3323a9

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7e4e593cd4d7

Comment 12

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

:emk - is this a wontfix? Or are we going to move forward with this in the future?

Comment 13

[Masatoshi Kimura [:emk]]

I consider to add telemetry to check what intolerance reason is used for a successful fallback connection to find unused intolerance reason.

Comment 14

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

P1 because this is assigned.