New issue
Advanced search Search tips
Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 446505: Fix fallback tests with BoringSSL pre-ServerHello alert handling

Reported by davidben@chromium.org, Jan 6 2015 Project Member

Issue description

So, after the various version negotiation rewrites, rolling BoringSSL into Chromium is failing on HTTPSFallbackTest.FallbackSCSV and HTTPSFallbackTest.SSLv3Fallback.

So, first off, the FallbackSCSV test is actually bust. It's currently identical to SSLv3Fallback. We've since disabled SSLv3 and the server is intolerant to TLS 1.0+, so it's never testing the inappropriate_fallback codepath. We need to change that test to TLS 1.1+ intolerance. This is almost orthogonal, except it then hits this BoringSSL quirk below:

The immediate behavior change is that BoringSSL, if receiving an alert before ServerHello, used to first check the record-layer version number. If that version was outside the configured acceptable versions, it would ignore the record and return SSL_R_UNSUPPORTED_PROTOCOL, which we map to ERR_SSL_VERSION_OR_CIPHER_MISMATCH.

After the rewrite, the version field is ignored (matching the ServerHello codepath) and we return the error code. This is probably better since it means we're no longer sensitive to the record-level version in an inappropriate_fallback alert. It's also consistent with NSS.

SSLv3Fallback configures the server to reply to all TLS ClientHellos with a handshake_failure alert. In both NSS and BoringSSL, this maps to ERR_SSL_PROTOCOL_ERROR, but we expect ERR_SSL_VERSION_OR_CIPHER_MISMATCH. The reason this passes in NSS is pretty tame: it maps handshake_failure prior to SSL_ERROR_NO_CYPHER_OVERLAP (which then goes to ERR_SSL_VERSION_OR_CIPHER_MISMATCH) if received prior to ServerHello:
https://code.google.com/p/chromium/codesearch#chromium/src/net/third_party/nss/ssl/ssl3con.c&q=SSL_ERROR_NO_CYPHER_OVERLAP&sq=package:chromium&type=cs&l=3517

This is consistent with OpenSSL's server behavior.
https://git.openssl.org/gitweb/?p=openssl.git;a=blob;f=ssl/s3_srvr.c;h=90e95d6bc11fb4908156437673aaec9f4e8c6927;hb=HEAD#l1385

Old BoringSSL only passed that check on accident. tlslite puts its maxVersion (TLS 1.1 because it lacks 1.2) in the handshake_failure alert. So we get:
ClientHello 1.2 -> handshake_failure 1.1 -> ERR_SSL_PROTOCOL_ERROR
ClientHello 1.1 -> handshake_failure 1.1 -> ERR_SSL_PROTOCOL_ERROR
ClientHello 1.0 -> handshake_failure 1.1 -> ERR_SSL_VERSION_OR_CIPHER_MISMATCH

New BoringSSL switches that last error to ERR_SSL_PROTOCOL_ERROR so we fail the test expectation.


So... we should fix the FallbackSCSV tests. We should also either match NSS in BoringSSL and tag a handshake_failure on ClientHello separately or adjust the test expectations to be backend-specific.
 

Comment 1 by davidben@chromium.org, Jan 6 2015

Status: Started
https://boringssl-review.googlesource.com/#/c/2751/
https://codereview.chromium.org/834313002

Not sure if this is worth bothering with. The alternative is that we just adjust the test expectations for those two tests (and still drop that old TODO).

The upside is we get a slightly better error message if there's no cipher suite overlap and we match our NSS behavior. But it's a marginal amount of special-casing for marginal gain.

Comment 2 by bugdroid1@chromium.org, Jan 6 2015

Project Member

Comment 3 by davidben@chromium.org, Jan 6 2015

Status: Fixed

Comment 4 by dhara.vo...@gmail.com, Jul 11 2015

I am facing same error Error code: ssl_error_no_cypher_overlap.

pls let me know how to fix the issue...

Comment 5 by davidben@chromium.org, Jul 11 2015

dhara.vora.19: This bug is about a failure in one of our unit tests, not anything to do with a site failing.

Chrome also never returns the string "ssl_error_no_cypher_overlap". That looks like a Firefox SSL error code. If so, their bug tracker is https://bugzilla.mozilla.org/

Sign in to add a comment