Don't mint TLS 1.3 tickets if SSL_OP_NO_TICKETS is set.
Change-Id: I03e05acb024e34beaeaf2f02860da1763e08a093
Reviewed-on: https://boringssl-review.googlesource.com/29844
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index f77a4cc..aa17350 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -137,8 +137,8 @@
extensionNextProtoNeg uint16 = 13172 // not IANA assigned
extensionRenegotiationInfo uint16 = 0xff01
extensionQUICTransportParams uint16 = 0xffa5 // draft-ietf-quic-tls-13
- extensionChannelID uint16 = 30032 // not IANA assigned
- extensionDummyPQPadding uint16 = 54537 // not IANA assigned
+ extensionChannelID uint16 = 30032 // not IANA assigned
+ extensionDummyPQPadding uint16 = 54537 // not IANA assigned
)
// TLS signaling cipher suite values
@@ -1168,6 +1168,10 @@
// sessions use session tickets instead of session IDs.
RequireSessionTickets bool
+ // RequireSessionIDs, if true, causes the client to require new sessions use
+ // session IDs instead of session tickets.
+ RequireSessionIDs bool
+
// NullAllCiphers, if true, causes every cipher to behave like the null
// cipher.
NullAllCiphers bool
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index f367dc4..3e20d87 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -729,6 +729,9 @@
if c.config.Bugs.RequireSessionTickets && len(hs.session.sessionTicket) == 0 {
return errors.New("tls: new session used session IDs instead of tickets")
}
+ if c.config.Bugs.RequireSessionIDs && len(hs.session.sessionId) == 0 {
+ return errors.New("tls: new session used session tickets instead of IDs")
+ }
sessionCache.Put(cacheKey, hs.session)
}
@@ -1672,6 +1675,9 @@
if c.vers == VersionSSL30 {
return errors.New("tls: negotiated session tickets in SSL 3.0")
}
+ if c.config.Bugs.ExpectNoNewSessionTicket {
+ return errors.New("tls: received unexpected NewSessionTicket")
+ }
msg, err := c.readHandshake()
if err != nil {
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index f5fac2b..7597764 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -11163,6 +11163,44 @@
resumeSession: true,
expectResumeRejected: true,
})
+
+ for _, ver := range tlsVersions {
+ // Prior to TLS 1.3, disabling session tickets enables session IDs.
+ useStatefulResumption := ver.version < VersionTLS13
+
+ // SSL_OP_NO_TICKET implies the server must not mint any tickets.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: ver.name + "-NoTicket-NoMint",
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ Bugs: ProtocolBugs{
+ ExpectNoNewSessionTicket: true,
+ RequireSessionIDs: useStatefulResumption,
+ },
+ },
+ resumeSession: useStatefulResumption,
+ tls13Variant: ver.tls13Variant,
+ flags: []string{"-no-ticket"},
+ })
+
+ // SSL_OP_NO_TICKET implies the server must not accept any tickets.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: ver.name + "-NoTicket-NoAccept",
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ tls13Variant: ver.tls13Variant,
+ resumeSession: true,
+ expectResumeRejected: true,
+ // Set SSL_OP_NO_TICKET on the second connection, after the first
+ // has established tickets.
+ flags: []string{"-on-resume-no-ticket"},
+ })
+ }
}
func addChangeCipherSpecTests() {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 1731431..c973fb5 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -66,6 +66,7 @@
{ "-no-tls12", &TestConfig::no_tls12 },
{ "-no-tls11", &TestConfig::no_tls11 },
{ "-no-tls1", &TestConfig::no_tls1 },
+ { "-no-ticket", &TestConfig::no_ticket },
{ "-enable-channel-id", &TestConfig::enable_channel_id },
{ "-shim-writes-first", &TestConfig::shim_writes_first },
{ "-expect-session-miss", &TestConfig::expect_session_miss },
@@ -1498,6 +1499,9 @@
if (no_tls1) {
SSL_set_options(ssl.get(), SSL_OP_NO_TLSv1);
}
+ if (no_ticket) {
+ SSL_set_options(ssl.get(), SSL_OP_NO_TICKET);
+ }
if (!expected_channel_id.empty() || enable_channel_id) {
SSL_set_tls_channel_id_enabled(ssl.get(), 1);
}
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index fef029f..f6c66e6 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -50,6 +50,7 @@
bool no_tls12 = false;
bool no_tls11 = false;
bool no_tls1 = false;
+ bool no_ticket = false;
std::string expected_channel_id;
bool enable_channel_id = false;
std::string send_channel_id;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 3c2c774..203e704 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -148,8 +148,18 @@
return best;
}
-static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
+static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) {
SSL *const ssl = hs->ssl;
+ if (// If the client doesn't accept resumption with PSK_DHE_KE, don't send a
+ // session ticket.
+ !hs->accept_psk_mode ||
+ // We only implement stateless resumption in TLS 1.3, so skip sending
+ // tickets if disabled.
+ (SSL_get_options(ssl) & SSL_OP_NO_TICKET)) {
+ *out_sent_tickets = false;
+ return true;
+ }
+
// TLS 1.3 recommends single-use tickets, so issue multiple tickets in case
// the client makes several connections before getting a renewal.
static const int kNumTickets = 2;
@@ -162,11 +172,11 @@
UniquePtr<SSL_SESSION> session(
SSL_SESSION_dup(hs->new_session.get(), SSL_SESSION_INCLUDE_NONAUTH));
if (!session) {
- return 0;
+ return false;
}
if (!RAND_bytes((uint8_t *)&session->ticket_age_add, 4)) {
- return 0;
+ return false;
}
session->ticket_age_add_valid = true;
if (ssl->enable_early_data) {
@@ -188,7 +198,7 @@
!tls13_derive_session_psk(session.get(), nonce) ||
!ssl_encrypt_ticket(hs, &ticket, session.get()) ||
!CBB_add_u16_length_prefixed(&body, &extensions)) {
- return 0;
+ return false;
}
if (ssl->enable_early_data) {
@@ -197,7 +207,7 @@
!CBB_add_u16_length_prefixed(&extensions, &early_data_info) ||
!CBB_add_u32(&early_data_info, session->ticket_max_early_data) ||
!CBB_flush(&extensions)) {
- return 0;
+ return false;
}
}
@@ -205,15 +215,16 @@
if (!CBB_add_u16(&extensions,
ssl_get_grease_value(hs, ssl_grease_ticket_extension)) ||
!CBB_add_u16(&extensions, 0 /* empty */)) {
- return 0;
+ return false;
}
if (!ssl_add_message_cbb(ssl, cbb.get())) {
- return 0;
+ return false;
}
}
- return 1;
+ *out_sent_tickets = true;
+ return true;
}
static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) {
@@ -723,11 +734,12 @@
assert(hs->hash_len <= 0xff);
uint8_t header[4] = {SSL3_MT_FINISHED, 0, 0,
static_cast<uint8_t>(hs->hash_len)};
+ bool unused_sent_tickets;
if (!hs->transcript.Update(header) ||
!hs->transcript.Update(
MakeConstSpan(hs->expected_client_finished, hs->hash_len)) ||
!tls13_derive_resumption_secret(hs) ||
- !add_new_session_tickets(hs)) {
+ !add_new_session_tickets(hs, &unused_sent_tickets)) {
return ssl_hs_error;
}
}
@@ -904,19 +916,13 @@
}
static enum ssl_hs_wait_t do_send_new_session_ticket(SSL_HANDSHAKE *hs) {
- // If the client doesn't accept resumption with PSK_DHE_KE, don't send a
- // session ticket.
- if (!hs->accept_psk_mode) {
- hs->tls13_state = state_done;
- return ssl_hs_ok;
- }
-
- if (!add_new_session_tickets(hs)) {
+ bool sent_tickets;
+ if (!add_new_session_tickets(hs, &sent_tickets)) {
return ssl_hs_error;
}
hs->tls13_state = state_done;
- return ssl_hs_flush;
+ return sent_tickets ? ssl_hs_flush : ssl_hs_ok;
}
enum ssl_hs_wait_t tls13_server_handshake(SSL_HANDSHAKE *hs) {
