From a1eaba1dc62ae4babc3ea631cd96f5cf34cf52ce Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 1 Jan 2017 23:19:22 -0500 Subject: [PATCH] Add a test for renegotiation on busy write buffer. The write path for TLS is going to need some work. There are some fiddly cases when there is a write in progress. Start adding tests to cover this logic. Later I'm hoping we can extend this flag so it drains the unfinished write and thus test the interaction of read/write paths in 0-RTT. (We may discover 1-RTT keys while we're in the middle of writing data.) Change-Id: Iac2c417e4b5e84794fb699dd7cbba26a883b64ef Reviewed-on: https://boringssl-review.googlesource.com/13049 Reviewed-by: Adam Langley --- ssl/test/bssl_shim.cc | 13 +++++++++++++ ssl/test/runner/runner.go | 18 ++++++++++++++++++ ssl/test/test_config.cc | 1 + ssl/test/test_config.h | 1 + 4 files changed, 33 insertions(+) diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 418b9f080..d46a02722 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -1760,6 +1760,19 @@ static bool DoExchange(bssl::UniquePtr *out_session, } } } else { + if (config->read_with_unfinished_write) { + if (!config->async) { + fprintf(stderr, "-read-with-unfinished-write requires -async.\n"); + return false; + } + + int write_ret = SSL_write(ssl.get(), + reinterpret_cast("unfinished"), 10); + if (SSL_get_error(ssl.get(), write_ret) != SSL_ERROR_WANT_WRITE) { + fprintf(stderr, "Failed to leave unfinished write.\n"); + return false; + } + } if (config->shim_writes_first) { if (WriteAll(ssl.get(), reinterpret_cast("hello"), 5) < 0) { diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index a71a9ccd6..56026b235 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -6262,6 +6262,24 @@ func addRenegotiationTests() { expectedLocalError: "remote error: no renegotiation", }) + // Renegotiation is not allowed when there is an unfinished write. + testCases = append(testCases, testCase{ + name: "Renegotiate-Client-UnfinishedWrite", + config: Config{ + MaxVersion: VersionTLS12, + }, + renegotiate: 1, + flags: []string{ + "-async", + "-renegotiate-freely", + "-read-with-unfinished-write", + }, + shouldFail: true, + expectedError: ":NO_RENEGOTIATION:", + // We do not successfully send the no_renegotiation alert in + // this case. https://crbug.com/boringssl/130 + }) + // Stray HelloRequests during the handshake are ignored in TLS 1.2. testCases = append(testCases, testCase{ name: "StrayHelloRequest", diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 0b1116986..22e4c9ce2 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -116,6 +116,7 @@ const Flag kBoolFlags[] = { { "-expect-sha256-client-cert-resume", &TestConfig::expect_sha256_client_cert_resume }, { "-enable-short-header", &TestConfig::enable_short_header }, + { "-read-with-unfinished-write", &TestConfig::read_with_unfinished_write }, }; const Flag kStringFlags[] = { diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 9f3fbecb3..882cddcb4 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -124,6 +124,7 @@ struct TestConfig { bool expect_sha256_client_cert_initial = false; bool expect_sha256_client_cert_resume = false; bool enable_short_header = false; + bool read_with_unfinished_write = false; }; bool ParseConfig(int argc, char **argv, TestConfig *out_config);