yggdrasil/ring
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Stop messing with ssl->version before sending protocol_version.

Browse Source 
This is the only codepath where ssl->version can get a garbage value, which is
a little concerning. Since, in all these cases, the peer is failing to connect
and speaks so low a version we don't even accept it anymore, there is probably
not much value in letting them distinguish protocol_version from a record-layer
version number mismatch, where enforced (which will give a version-related
error anyway).

Should we get a decode_error or so just before version negotiation, we'd have
this behavior already.

Change-Id: I9b3e5685ab9c9ad32a7b7e3129363cd1d4cdaaf4
Reviewed-on: https://boringssl-review.googlesource.com/8420
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin 2016-06-21 12:19:28 -04:00
parent 1fd39d84cf
commit bde00394f0
4 changed files with 29 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
ssl/handshake_client.c
View File

@ -808,10 +808,6 @@ static int ssl3_get_server_hello(SSL *ssl) {
if (!ssl->s3->have_version) {
if (!ssl3_is_version_enabled(ssl, server_version)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
ssl->version = server_version;
/* Mark the version as fixed so the record-layer version is not clamped
* to TLS 1.0. */
ssl->s3->have_version = 1;
al = SSL_AD_PROTOCOL_VERSION;
goto f_err;
}

1
ssl/handshake_server.c
View File

@ -832,7 +832,6 @@ static int ssl3_get_client_hello(SSL *ssl) {
uint16_t version = ssl3_get_mutual_version(ssl, client_version);
if (version == 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_PROTOCOL);
ssl->version = ssl->client_version;
al = SSL_AD_PROTOCOL_VERSION;
goto f_err;
}

23
ssl/test/runner/conn.go
View File

@ -714,15 +714,20 @@ func (c *Conn) doReadRecord(want recordType) (recordType, *block, error) {
vers := uint16(b.data[1])<<8 | uint16(b.data[2])
n := int(b.data[3])<<8 | int(b.data[4])
if c.haveVers {
if vers != c.vers && c.vers < VersionTLS13 {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("tls: received record with version %x when expecting version %x", vers, c.vers))
}
} else {
if expect := c.config.Bugs.ExpectInitialRecordVersion; expect != 0 && vers != expect {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("tls: received record with version %x when expecting version %x", vers, expect))
// Alerts sent near version negotiation do not have a well-defined
// record-layer version prior to TLS 1.3. (In TLS 1.3, the record-layer
// version is irrelevant.)
if typ != recordTypeAlert {
if c.haveVers {
if vers != c.vers && c.vers < VersionTLS13 {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("tls: received record with version %x when expecting version %x", vers, c.vers))
}
} else {
if expect := c.config.Bugs.ExpectInitialRecordVersion; expect != 0 && vers != expect {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("tls: received record with version %x when expecting version %x", vers, expect))
}
}
}
if n > maxCiphertext {

24
ssl/test/runner/dtls.go
View File

@ -71,15 +71,21 @@ func (c *Conn) dtlsDoReadRecord(want recordType) (recordType, *block, error) {
}
typ := recordType(b.data[0])
vers := wireToVersion(uint16(b.data[1])<<8|uint16(b.data[2]), c.isDTLS)
if c.haveVers {
if vers != c.vers {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: received record with version %x when expecting version %x", vers, c.vers))
}
} else {
if expect := c.config.Bugs.ExpectInitialRecordVersion; expect != 0 && vers != expect {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: received record with version %x when expecting version %x", vers, expect))
// Alerts sent near version negotiation do not have a well-defined
// record-layer version prior to TLS 1.3. (In TLS 1.3, the record-layer
// version is irrelevant.)
if typ != recordTypeAlert {
if c.haveVers {
if vers != c.vers {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: received record with version %x when expecting version %x", vers, c.vers))
}
} else {
// Pre-version-negotiation alerts may be sent with any version.
if expect := c.config.Bugs.ExpectInitialRecordVersion; expect != 0 && vers != expect {
c.sendAlert(alertProtocolVersion)
return 0, nil, c.in.setErrorLocked(fmt.Errorf("dtls: received record with version %x when expecting version %x", vers, expect))
}
}
}
epoch := b.data[3:5]