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>
This commit is contained in:
David Benjamin
2018-07-18 23:47:36 +00:00
committed by Adam Langley
parent 5869eb3951
commit 0cbb1af41f
6 changed files with 79 additions and 20 deletions
+6 -2
View File
@@ -137,8 +137,8 @@ const (
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 @@ type ProtocolBugs struct {
// 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
+6
View File
@@ -729,6 +729,9 @@ NextCipherSuite:
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 @@ func (hs *clientHandshakeState) readSessionTicket() error {
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 {
+38
View File
@@ -11163,6 +11163,44 @@ func addSessionTicketTests() {
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() {
+4
View File
@@ -66,6 +66,7 @@ const Flag<bool> kBoolFlags[] = {
{ "-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 @@ bssl::UniquePtr<SSL> TestConfig::NewSSL(
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);
}
+1
View File
@@ -50,6 +50,7 @@ struct TestConfig {
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;
+24 -18
View File
@@ -148,8 +148,18 @@ static const SSL_CIPHER *choose_tls13_cipher(
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 @@ static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
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 @@ static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
!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 @@ static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
!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 @@ static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
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 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) {
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_read_client_finished(SSL_HANDSHAKE *hs) {
}
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) {