From e78bfded9f24725c47e0f5d0a64465566c3bdcf2 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sat, 6 Sep 2014 12:45:15 -0400 Subject: [PATCH] Improve test coverage for server_name extension. Notably, this would have caught ed8270a55c3845abbc85dfeed358597fef059ea9 (although, apart from staring at code coverage, knowing to set resumeSession on the server test isn't exactly obvious). Perhaps we should systematically set it on all extension server tests; ClientHello extension parsing happens after resumption has been determined and is often sensitive to it. Change-Id: Ie83f294a26881a6a41969e9dbd102d0a93cb68b5 Reviewed-on: https://boringssl-review.googlesource.com/1750 Reviewed-by: Adam Langley --- ssl/test/bssl_shim.cc | 3 + ssl/test/runner/common.go | 4 ++ ssl/test/runner/handshake_server.go | 3 + ssl/test/runner/runner.go | 98 ++++++++++++++++++++--------- ssl/test/test_config.cc | 1 + ssl/test/test_config.h | 1 + 6 files changed, 80 insertions(+), 30 deletions(-) diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index 0459afc7..4ca7c58e 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -336,6 +336,9 @@ static int do_exchange(SSL_SESSION **out_session, } EVP_PKEY_free(pkey); } + if (!config->host_name.empty()) { + SSL_set_tlsext_host_name(ssl, config->host_name.c_str()); + } BIO *bio = BIO_new_fd(fd, 1 /* take ownership */); if (bio == NULL) { diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index f22f95a0..9af30637 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -449,6 +449,10 @@ type ProtocolBugs struct { // SkipCipherVersionCheck causes the server to negotiate // TLS 1.2 ciphers in earlier versions of TLS. SkipCipherVersionCheck bool + + // ExpectServerName, if not empty, is the hostname the client + // must specify in the server_name extension. + ExpectServerName string } func (c *Config) serverInit() { diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 6d61fd55..e456891d 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go @@ -237,6 +237,9 @@ Curves: if len(hs.clientHello.serverName) > 0 { hs.cert = config.getCertificateForName(hs.clientHello.serverName) } + if expected := c.config.Bugs.ExpectServerName; expected != "" && expected != hs.clientHello.serverName { + return false, errors.New("tls: unexpected server name") + } if hs.clientHello.channelIDSupported && config.RequestChannelID { hs.hello.channelIDRequested = true diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 64df21df..ae744646 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -201,36 +201,6 @@ var testCases = []testCase{ }, flags: []string{"-fallback-scsv"}, }, - { - testType: serverTest, - name: "ServerNameExtension", - config: Config{ - ServerName: "example.com", - }, - flags: []string{"-expect-server-name", "example.com"}, - }, - { - testType: clientTest, - name: "DuplicateExtensionClient", - config: Config{ - Bugs: ProtocolBugs{ - DuplicateExtension: true, - }, - }, - shouldFail: true, - expectedLocalError: "remote error: error decoding message", - }, - { - testType: serverTest, - name: "DuplicateExtensionServer", - config: Config{ - Bugs: ProtocolBugs{ - DuplicateExtension: true, - }, - }, - shouldFail: true, - expectedLocalError: "remote error: error decoding message", - }, { name: "ClientCertificateTypes", config: Config{ @@ -1372,6 +1342,73 @@ func addD5BugTests() { }) } +func addExtensionTests() { + testCases = append(testCases, testCase{ + testType: clientTest, + name: "DuplicateExtensionClient", + config: Config{ + Bugs: ProtocolBugs{ + DuplicateExtension: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: error decoding message", + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "DuplicateExtensionServer", + config: Config{ + Bugs: ProtocolBugs{ + DuplicateExtension: true, + }, + }, + shouldFail: true, + expectedLocalError: "remote error: error decoding message", + }) + testCases = append(testCases, testCase{ + testType: clientTest, + name: "ServerNameExtensionClient", + config: Config{ + Bugs: ProtocolBugs{ + ExpectServerName: "example.com", + }, + }, + flags: []string{"-host-name", "example.com"}, + }) + testCases = append(testCases, testCase{ + testType: clientTest, + name: "ServerNameExtensionClient", + config: Config{ + Bugs: ProtocolBugs{ + ExpectServerName: "mismatch.com", + }, + }, + flags: []string{"-host-name", "example.com"}, + shouldFail: true, + expectedLocalError: "tls: unexpected server name", + }) + testCases = append(testCases, testCase{ + testType: clientTest, + name: "ServerNameExtensionClient", + config: Config{ + Bugs: ProtocolBugs{ + ExpectServerName: "missing.com", + }, + }, + shouldFail: true, + expectedLocalError: "tls: unexpected server name", + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "ServerNameExtensionServer", + config: Config{ + ServerName: "example.com", + }, + flags: []string{"-expect-server-name", "example.com"}, + resumeSession: true, + }) +} + func worker(statusChan chan statusMsg, c chan *testCase, buildDir string, wg *sync.WaitGroup) { defer wg.Done() @@ -1425,6 +1462,7 @@ func main() { addClientAuthTests() addVersionNegotiationTests() addD5BugTests() + addExtensionTests() for _, async := range []bool{false, true} { for _, splitHandshake := range []bool{false, true} { for _, protocol := range []protocol{tls, dtls} { diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 41188afa..77d51a1d 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -67,6 +67,7 @@ const StringFlag kStringFlags[] = { { "-expect-next-proto", &TestConfig::expected_next_proto }, { "-select-next-proto", &TestConfig::select_next_proto }, { "-send-channel-id", &TestConfig::send_channel_id }, + { "-host-name", &TestConfig::host_name }, }; const size_t kNumStringFlags = sizeof(kStringFlags) / sizeof(kStringFlags[0]); diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 208f19b1..c3ba63f3 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -47,6 +47,7 @@ struct TestConfig { std::string send_channel_id; bool shim_writes_first; bool tls_d5_bug; + std::string host_name; }; bool ParseConfig(int argc, char **argv, TestConfig *out_config);