Remove SSL_OP_TLS_ROLLBACK_BUG.
It's not part of SSL_OP_ALL and is unused, so remove it. Add a test that
asserts the version check works.
Change-Id: I917516594ec5a4998a8316782f035697c33d99b0
Reviewed-on: https://boringssl-review.googlesource.com/1418
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 7221fb0..d5f34af 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -2111,36 +2111,6 @@
version_good = premaster_secret[0] ^ (s->client_version>>8);
version_good |= premaster_secret[1] ^ (s->client_version&0xff);
- /* The premaster secret must contain the same version number as
- * the ClientHello to detect version rollback attacks
- * (strangely, the protocol does not offer such protection for
- * DH ciphersuites). However, buggy clients exist that send the
- * negotiated protocol version instead if the server does not
- * support the requested protocol version. If
- * SSL_OP_TLS_ROLLBACK_BUG is set, tolerate such clients. */
- if (s->options & SSL_OP_TLS_ROLLBACK_BUG)
- {
- unsigned char workaround_mask = version_good;
- unsigned char workaround;
-
- /* workaround_mask will be 0xff if version_good is
- * non-zero (i.e. the version match failed). Otherwise
- * it'll be 0x00. */
- workaround_mask |= workaround_mask >> 4;
- workaround_mask |= workaround_mask >> 2;
- workaround_mask |= workaround_mask >> 1;
- workaround_mask = ~((workaround_mask & 1) - 1);
-
- workaround = premaster_secret[0] ^ (s->version>>8);
- workaround |= premaster_secret[1] ^ (s->version&0xff);
-
- /* If workaround_mask is 0xff (i.e. there was a version
- * mismatch) then we copy the value of workaround over
- * version_good. */
- version_good = (workaround & workaround_mask) |
- (version_good & ~workaround_mask);
- }
-
/* If any bits in version_good are set then they'll poision
* decrypt_good_mask and cause rand_premaster_secret to be
* used. */
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index eb1d57c..d69f09f 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -396,6 +396,11 @@
// handshake record. Handshake messages will be split at the record
// layer.
MaxHandshakeRecordLength int
+
+ // RsaClientKeyExchangeVersion, if non-zero, causes the client to send a
+ // ClientKeyExchange with the specified version rather than the
+ // client_version when performing the RSA key exchange.
+ RsaClientKeyExchangeVersion uint16
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/key_agreement.go b/ssl/test/runner/key_agreement.go
index 0e29d63..929eb06 100644
--- a/ssl/test/runner/key_agreement.go
+++ b/ssl/test/runner/key_agreement.go
@@ -70,8 +70,12 @@
func (ka rsaKeyAgreement) generateClientKeyExchange(config *Config, clientHello *clientHelloMsg, cert *x509.Certificate) ([]byte, *clientKeyExchangeMsg, error) {
preMasterSecret := make([]byte, 48)
- preMasterSecret[0] = byte(clientHello.vers >> 8)
- preMasterSecret[1] = byte(clientHello.vers)
+ vers := clientHello.vers
+ if config.Bugs.RsaClientKeyExchangeVersion != 0 {
+ vers = config.Bugs.RsaClientKeyExchangeVersion
+ }
+ preMasterSecret[0] = byte(vers >> 8)
+ preMasterSecret[1] = byte(vers)
_, err := io.ReadFull(config.rand(), preMasterSecret[2:])
if err != nil {
return nil, nil, err
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 3edbd8b..f2e268e 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -124,6 +124,18 @@
expectedError: ":WRONG_CURVE:",
},
{
+ testType: serverTest,
+ name: "BadRSAVersion",
+ config: Config{
+ CipherSuites: []uint16{TLS_RSA_WITH_RC4_128_SHA},
+ Bugs: ProtocolBugs{
+ RsaClientKeyExchangeVersion: VersionTLS11,
+ },
+ },
+ shouldFail: true,
+ expectedError: ":DECRYPTION_FAILED_OR_BAD_RECORD_MAC:",
+ },
+ {
name: "NoFallbackSCSV",
config: Config{
Bugs: ProtocolBugs{
@@ -736,8 +748,8 @@
"-write-different-record-sizes",
"-cbc-record-splitting",
},
- },
- testCase{
+ })
+ testCases = append(testCases, testCase{
name: "CBCRecordSplittingPartialWrite",
config: Config{
MaxVersion: VersionTLS10,
