1124157 - Detect servers that require RC4

Status: RESOLVED WONTFIX

(Core :: Security: PSM, defect)

Description

[Masatoshi Kimura [:emk]]

We need changes to NSS to get information early enough.

Comment 1

[Masatoshi Kimura [:emk]]

Actually, NSS change will not help much. Usually the server will not select an RC4 cipher suite unless we offer the cipher suite. For the SSLv3 case, ClientHello doesn't include information about the minimum supported version, so the server can select SSLv3.

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Detect servers that require RC4

- SSL_GetChannelInfo() is guaranteed to be available in CanFalseStartCallback().
- No application data will be sent because CanFalseStartCallback() is the callback to determine if it is safe.
- If the auth is not complete when sending the client second round, CanFalseStartCallback() will not be called. But in this case, SSL_GetChannelInfo() will be available in SetCertVerificationResult(). And no application data will be sent because the auth is not complete.
- If a sync auth is used, the auth will never be pending when sending the client second round.

So I believe this patch will catch all cases. See bug 999544 comment #34 and onwards about the motivation of this change.
Ideally, this bug should be based on NSS changes. But it would be too late to wait until NSS is fixed and merged, especially for branches.

Comment 3

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

Comment on attachment 8567475 [details] [diff] [review]
Detect servers that require RC4

(In reply to Masatoshi Kimura [:emk] from comment #2)
> - SSL_GetChannelInfo() is guaranteed to be available in
> CanFalseStartCallback().
> - No application data will be sent because CanFalseStartCallback() is the
> callback to determine if it is safe.

CanFalseStartCallback isn't called for every full handshake. CanFalseStartCallback is only called if cert verification finishes fast enough and other conditions are met. Even if it were the case that CanFalseStartCallback is called in all the scenerios that matter *today*, that may change in the future, with terrible consequences.

> So I believe this patch will catch all cases. See bug 999544 comment #34 and
> onwards about the motivation of this change.

> Ideally, this bug should be based on NSS changes. But it would be too late
> to wait until NSS is fixed and merged, especially for branches.

The PSM/libssl state machine is already quite difficult to understand due to *necessary* complexity. This patch makes it *unnecessarily* complicated and convoluted.

Mozilla needs to address the social problems that make it seem hard to change NSS. It is not that hard to change libssl. Last year we made changes to NSS that required similar changes on similar schedules. It just requires people from the Mozilla side to be more insistent and to do both the patch writing and the code review.

Wouldn't it be much simpler and safer to amend the SSL_NO_CYPHER_OVERLAP error page to add the statement "Note that RC4 cipher suites are no longer supported; see https://support.mozilla.org/whatever"?

Note if you're planning to show a new error message for RC4 errors, it won't get approved for -beta or (usually) -aurora because it would require localization, regardless of the approach you take.

Comment 4

[Masatoshi Kimura [:emk]]

This is impossible without adding complexity to the PSM.