Test resuming renewed sessions.
In TLS 1.3 draft 14, due to resumption using a different cipher, this
is actually not too hard to mess up. (In fact BoGo didn't quite get it
right.)
Fortunately, the new cipher suite negotiation in draft 15 should make
this reasonable again once we implement it. In the meantime, test it.
Change-Id: I2eb948eeaaa051ecacaa9095b66ff149582ea11d
Reviewed-on: https://boringssl-review.googlesource.com/10442
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 13229e9..2a9ba88 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1699,17 +1699,20 @@
}
ScopedSSL_SESSION session;
- if (!DoExchange(&session, ssl_ctx.get(), &config, false /* is_resume */,
- NULL /* session */)) {
- ERR_print_errors_fp(stderr);
- return 1;
- }
+ for (int i = 0; i < config.resume_count + 1; i++) {
+ bool is_resume = i > 0;
+ if (is_resume && !config.is_server && !session) {
+ fprintf(stderr, "No session to offer.\n");
+ return 1;
+ }
- if (config.resume &&
- !DoExchange(NULL, ssl_ctx.get(), &config, true /* is_resume */,
- session.get())) {
- ERR_print_errors_fp(stderr);
- return 1;
+ ScopedSSL_SESSION offer_session = std::move(session);
+ if (!DoExchange(&session, ssl_ctx.get(), &config, is_resume,
+ offer_session.get())) {
+ fprintf(stderr, "Connection %d failed.\n", i + 1);
+ ERR_print_errors_fp(stderr);
+ return 1;
+ }
}
return 0;
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index e8e6490..e8e56d7 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -203,10 +203,24 @@
// Check that the ciphersuite/version used for the
// previous session are still valid.
cipherSuiteOk := false
- for _, id := range hello.cipherSuites {
- if id == candidateSession.cipherSuite {
- cipherSuiteOk = true
- break
+ if candidateSession.vers >= VersionTLS13 {
+ // Account for ciphers changing on resumption.
+ //
+ // TODO(davidben): This will be gone with the
+ // new cipher negotiation scheme.
+ resumeCipher := ecdhePSKSuite(candidateSession.cipherSuite)
+ for _, id := range hello.cipherSuites {
+ if ecdhePSKSuite(id) == resumeCipher {
+ cipherSuiteOk = true
+ break
+ }
+ }
+ } else {
+ for _, id := range hello.cipherSuites {
+ if id == candidateSession.cipherSuite {
+ cipherSuiteOk = true
+ break
+ }
}
}
@@ -234,17 +248,8 @@
// TODO(nharper): Support sending more
// than one PSK identity.
if session.ticketFlags&ticketAllowDHEResumption != 0 || c.config.Bugs.SendBothTickets {
- var found bool
- for _, id := range hello.cipherSuites {
- if id == session.cipherSuite {
- found = true
- break
- }
- }
- if found {
- hello.pskIdentities = [][]uint8{ticket}
- hello.cipherSuites = append(hello.cipherSuites, ecdhePSKSuite(session.cipherSuite))
- }
+ hello.pskIdentities = [][]uint8{ticket}
+ hello.cipherSuites = append(hello.cipherSuites, ecdhePSKSuite(session.cipherSuite))
}
}
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 6d4d70a..29f36bb 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -350,10 +350,15 @@
}
suite := mutualCipherSuite(clientCipherSuites, suiteId)
- // Check the cipher is enabled by the server.
+ // Check the cipher is enabled by the server or is a resumption
+ // suite of one enabled by the server. Account for the cipher
+ // change on resume.
+ //
+ // TODO(davidben): The ecdhePSKSuite mess will be gone with the
+ // new cipher negotiation scheme.
var found bool
for _, id := range config.cipherSuites() {
- if id == sessionState.cipherSuite {
+ if ecdhePSKSuite(id) == suiteId {
found = true
break
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 19b8ee7..78e4191 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -294,6 +294,9 @@
// resumeSession controls whether a second connection should be tested
// which attempts to resume the first session.
resumeSession bool
+ // resumeRenewedSession controls whether a third connection should be
+ // tested which attempts to resume the second connection's session.
+ resumeRenewedSession bool
// expectResumeRejected, if true, specifies that the attempted
// resumption must be rejected by the client. This is only valid for a
// serverTest.
@@ -831,8 +834,16 @@
flags = append(flags, "-dtls")
}
+ var resumeCount int
if test.resumeSession {
- flags = append(flags, "-resume")
+ resumeCount++
+ if test.resumeRenewedSession {
+ resumeCount++
+ }
+ }
+
+ if resumeCount > 0 {
+ flags = append(flags, "-resume-count", strconv.Itoa(resumeCount))
}
if test.shimWritesFirst {
@@ -898,7 +909,7 @@
conn.Close()
}
- if err == nil && test.resumeSession {
+ for i := 0; err == nil && i < resumeCount; i++ {
var resumeConfig Config
if test.resumeConfig != nil {
resumeConfig = *test.resumeConfig
@@ -2115,9 +2126,7 @@
FailIfSessionOffered: true,
},
},
- flags: []string{"-expect-no-session"},
- resumeSession: true,
- expectResumeRejected: true,
+ flags: []string{"-expect-no-session"},
},
{
name: "BadHelloRequest-1",
@@ -3072,8 +3081,9 @@
RenewTicketOnResume: true,
},
},
- flags: []string{"-expect-ticket-renewal"},
- resumeSession: true,
+ flags: []string{"-expect-ticket-renewal"},
+ resumeSession: true,
+ resumeRenewedSession: true,
})
tests = append(tests, testCase{
name: "Basic-Client-NoTicket",
@@ -3138,7 +3148,8 @@
MaxVersion: VersionTLS13,
MinVersion: VersionTLS13,
},
- resumeSession: true,
+ resumeSession: true,
+ resumeRenewedSession: true,
})
tests = append(tests, testCase{
@@ -3148,7 +3159,8 @@
MaxVersion: VersionTLS13,
MinVersion: VersionTLS13,
},
- resumeSession: true,
+ resumeSession: true,
+ resumeRenewedSession: true,
})
tests = append(tests, testCase{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 2fa1f17..677aa54 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -46,7 +46,6 @@
const Flag<bool> kBoolFlags[] = {
{ "-server", &TestConfig::is_server },
{ "-dtls", &TestConfig::is_dtls },
- { "-resume", &TestConfig::resume },
{ "-fallback-scsv", &TestConfig::fallback_scsv },
{ "-require-any-client-certificate",
&TestConfig::require_any_client_certificate },
@@ -143,6 +142,7 @@
const Flag<int> kIntFlags[] = {
{ "-port", &TestConfig::port },
+ { "-resume-count", &TestConfig::resume_count },
{ "-min-version", &TestConfig::min_version },
{ "-max-version", &TestConfig::max_version },
{ "-fallback-version", &TestConfig::fallback_version },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index f6a1f12..8ed74ac 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -23,7 +23,7 @@
int port = 0;
bool is_server = false;
bool is_dtls = false;
- bool resume = false;
+ int resume_count = 0;
bool fallback_scsv = false;
std::string digest_prefs;
std::vector<int> signing_prefs;
