Fix buffer size computation.
The maximum buffer size computation wasn't quite done right in
ssl_buffer.c, so we were failing with BUFFER_TOO_SMALL for sufficiently
large records. Fix this and, as penance, add 103 tests.
(Test that we can receive maximum-size records in all cipher suites.
Also test SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER while I'm here.)
BUG=526998
Change-Id: I714c16dda2ed13f49d8e6cd1b48adc5a8491f43c
Reviewed-on: https://boringssl-review.googlesource.com/5785
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_buffer.c b/ssl/ssl_buffer.c
index 37f27be..63dcd80 100644
--- a/ssl/ssl_buffer.c
+++ b/ssl/ssl_buffer.c
@@ -84,7 +84,12 @@
}
size_t header_len = ssl_record_prefix_len(ssl);
- size_t cap = SSL3_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH;
+ size_t cap = SSL3_RT_MAX_ENCRYPTED_LENGTH;
+ if (SSL_IS_DTLS(ssl)) {
+ cap += DTLS1_RT_HEADER_LENGTH;
+ } else {
+ cap += SSL3_RT_HEADER_LENGTH;
+ }
if (ssl->options & SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER) {
cap += SSL3_RT_MAX_EXTRA;
}
@@ -234,10 +239,14 @@
/* TODO(davidben): This matches the original behavior in keeping the malloc
* size consistent. Does this matter? |cap| could just be |max_len|. */
- size_t cap = SSL3_RT_HEADER_LENGTH + SSL3_RT_MAX_PLAIN_LENGTH +
- SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
- if (!SSL_IS_DTLS(ssl) && (ssl->mode & SSL_MODE_CBC_RECORD_SPLITTING)) {
- cap += SSL3_RT_HEADER_LENGTH + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+ size_t cap = SSL3_RT_MAX_PLAIN_LENGTH + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+ if (SSL_IS_DTLS(ssl)) {
+ cap += DTLS1_RT_HEADER_LENGTH;
+ } else {
+ cap += SSL3_RT_HEADER_LENGTH;
+ if (ssl->mode & SSL_MODE_CBC_RECORD_SPLITTING) {
+ cap += SSL3_RT_HEADER_LENGTH + SSL3_RT_SEND_MAX_ENCRYPTED_OVERHEAD;
+ }
}
if (max_len > cap) {
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index dabe5ec..fa0cb75 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -965,6 +965,9 @@
if (config->tls_d5_bug) {
SSL_set_options(ssl.get(), SSL_OP_TLS_D5_BUG);
}
+ if (config->microsoft_big_sslv3_buffer) {
+ SSL_set_options(ssl.get(), SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER);
+ }
if (config->no_legacy_server_connect) {
SSL_clear_options(ssl.get(), SSL_OP_LEGACY_SERVER_CONNECT);
}
@@ -1173,8 +1176,10 @@
}
if (!config->shim_shuts_down) {
for (;;) {
- uint8_t buf[512];
- int n = DoRead(ssl.get(), buf, sizeof(buf));
+ uint8_t buf[16384];
+ // Read only 512 bytes at a time in TLS to ensure records may be
+ // returned in multiple reads.
+ int n = DoRead(ssl.get(), buf, config->is_dtls ? sizeof(buf) : 512);
int err = SSL_get_error(ssl.get(), n);
if (err == SSL_ERROR_ZERO_RETURN ||
(n == 0 && err == SSL_ERROR_SYSCALL)) {
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index 2b7e29b..f578631 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -762,6 +762,10 @@
// shutdown. Records from the peer received after close_notify is sent
// are not discard.
ExpectCloseNotify bool
+
+ // SendLargeRecords, if true, allows outgoing records to be sent
+ // arbitrarily large.
+ SendLargeRecords bool
}
func (c *Config) serverInit() {
diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go
index 42bc840..1b8700c 100644
--- a/ssl/test/runner/conn.go
+++ b/ssl/test/runner/conn.go
@@ -876,7 +876,7 @@
isClientHello := typ == recordTypeHandshake && len(data) > 0 && data[0] == typeClientHello
for len(data) > 0 || first {
m := len(data)
- if m > maxPlaintext {
+ if m > maxPlaintext && !c.config.Bugs.SendLargeRecords {
m = maxPlaintext
}
if typ == recordTypeHandshake && c.config.Bugs.MaxHandshakeRecordLength > 0 && m > c.config.Bugs.MaxHandshakeRecordLength {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 7ada5f1..d085913 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1984,6 +1984,47 @@
})
}
}
+
+ // Ensure both TLS and DTLS accept their maximum record sizes.
+ testCases = append(testCases, testCase{
+ name: suite.name + "-LargeRecord",
+ config: Config{
+ CipherSuites: []uint16{suite.id},
+ Certificates: []Certificate{cert},
+ PreSharedKey: []byte(psk),
+ PreSharedKeyIdentity: pskIdentity,
+ },
+ flags: flags,
+ messageLen: maxPlaintext,
+ })
+ testCases = append(testCases, testCase{
+ name: suite.name + "-LargeRecord-Extra",
+ config: Config{
+ CipherSuites: []uint16{suite.id},
+ Certificates: []Certificate{cert},
+ PreSharedKey: []byte(psk),
+ PreSharedKeyIdentity: pskIdentity,
+ Bugs: ProtocolBugs{
+ SendLargeRecords: true,
+ },
+ },
+ flags: append(flags, "-microsoft-big-sslv3-buffer"),
+ messageLen: maxPlaintext + 16384,
+ })
+ if isDTLSCipher(suite.name) {
+ testCases = append(testCases, testCase{
+ protocol: dtls,
+ name: suite.name + "-LargeRecord-DTLS",
+ config: Config{
+ CipherSuites: []uint16{suite.id},
+ Certificates: []Certificate{cert},
+ PreSharedKey: []byte(psk),
+ PreSharedKeyIdentity: pskIdentity,
+ },
+ flags: flags,
+ messageLen: maxPlaintext,
+ })
+ }
}
testCases = append(testCases, testCase{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 4191b2b..edf3d6e 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -92,6 +92,7 @@
{ "-custom-extension-fail-add", &TestConfig::custom_extension_fail_add },
{ "-check-close-notify", &TestConfig::check_close_notify },
{ "-shim-shuts-down", &TestConfig::shim_shuts_down },
+ { "-microsoft-big-sslv3-buffer", &TestConfig::microsoft_big_sslv3_buffer },
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index c7bdab3..d6ccda2 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -89,6 +89,7 @@
std::string ocsp_response;
bool check_close_notify = false;
bool shim_shuts_down = false;
+ bool microsoft_big_sslv3_buffer = false;
};
bool ParseConfig(int argc, char **argv, TestConfig *out_config);