diff --git a/include/openssl/base.h b/include/openssl/base.h index 29b598e15..e95efbaac 100644 --- a/include/openssl/base.h +++ b/include/openssl/base.h @@ -109,7 +109,7 @@ extern "C" { // A consumer may use this symbol in the preprocessor to temporarily build // against multiple revisions of BoringSSL at the same time. It is not // recommended to do so for longer than is necessary. -#define BORINGSSL_API_VERSION 32 +#define BORINGSSL_API_VERSION 33 #if defined(BORINGSSL_SHARED_LIBRARY) diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index b198e0c7c..b0abe116d 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -2464,7 +2464,8 @@ OPENSSL_EXPORT int SSL_CTX_set_tlsext_ticket_keys(SSL_CTX *ctx, const void *in, // When encrypting a new ticket, |encrypt| will be one. It writes a public // 16-byte key name to |key_name| and a fresh IV to |iv|. The output IV length // must match |EVP_CIPHER_CTX_iv_length| of the cipher selected. In this mode, -// |callback| returns 1 on success and -1 on error. +// |callback| returns 1 on success, 0 to decline sending a ticket, and -1 on +// error. // // When decrypting a ticket, |encrypt| will be zero. |key_name| will point to a // 16-byte key name and |iv| points to an IV. The length of the IV consumed must @@ -2508,7 +2509,8 @@ struct ssl_ticket_aead_method_st { // seal encrypts and authenticates |in_len| bytes from |in|, writes, at most, // |max_out_len| bytes to |out|, and puts the number of bytes written in // |*out_len|. The |in| and |out| buffers may be equal but will not otherwise - // alias. It returns one on success or zero on error. + // alias. It returns one on success or zero on error. If the function returns + // but |*out_len| is zero, BoringSSL will skip sending a ticket. int (*seal)(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out_len, const uint8_t *in, size_t in_len); diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 1b292a728..40329c65e 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc @@ -1835,6 +1835,9 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) { !CBB_add_u32(&body, session->timeout) || !CBB_add_u16_length_prefixed(&body, &ticket) || !ssl_encrypt_ticket(hs, &ticket, session) || + // |ticket| may be empty to skip sending a ticket. In TLS 1.2, servers + // skip sending tickets by sending empty NewSessionTicket, so no special + // handling is needed. !ssl_add_message_cbb(ssl, cbb.get())) { return ssl_hs_error; } diff --git a/ssl/internal.h b/ssl/internal.h index 3e68a8bf1..213704d49 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -3755,8 +3755,13 @@ bool ssl_is_key_type_supported(int key_type); bool ssl_compare_public_and_private_key(const EVP_PKEY *pubkey, const EVP_PKEY *privkey); bool ssl_get_new_session(SSL_HANDSHAKE *hs); + +// ssl_encrypt_ticket encrypt a ticket for |session| and writes the result to +// |out|. It returns true on success and false on error. If, on success, nothing +// was written to |out|, the caller should skip sending a ticket. bool ssl_encrypt_ticket(SSL_HANDSHAKE *hs, CBB *out, const SSL_SESSION *session); + bool ssl_ctx_rotate_ticket_encryption_key(SSL_CTX *ctx); // ssl_session_new returns a newly-allocated blank |SSL_SESSION| or nullptr on diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc index 9ceefbb0a..16fd4539f 100644 --- a/ssl/ssl_session.cc +++ b/ssl/ssl_session.cc @@ -443,14 +443,11 @@ static int ssl_encrypt_ticket_with_cipher_ctx(SSL_HANDSHAKE *hs, CBB *out, ScopedEVP_CIPHER_CTX ctx; ScopedHMAC_CTX hctx; - // If the session is too long, emit a dummy value rather than abort the - // connection. + // If the session is too long, decline to send a ticket. static const size_t kMaxTicketOverhead = 16 + EVP_MAX_IV_LENGTH + EVP_MAX_BLOCK_LENGTH + EVP_MAX_MD_SIZE; if (session_len > 0xffff - kMaxTicketOverhead) { - static const char kTicketPlaceholder[] = "TICKET TOO LARGE"; - return CBB_add_bytes(out, (const uint8_t *)kTicketPlaceholder, - strlen(kTicketPlaceholder)); + return 1; } // Initialize HMAC and cipher contexts. If callback present it does all the @@ -459,10 +456,15 @@ static int ssl_encrypt_ticket_with_cipher_ctx(SSL_HANDSHAKE *hs, CBB *out, uint8_t iv[EVP_MAX_IV_LENGTH]; uint8_t key_name[16]; if (tctx->ticket_key_cb != NULL) { - if (tctx->ticket_key_cb(hs->ssl, key_name, iv, ctx.get(), hctx.get(), - 1 /* encrypt */) < 0) { + int ret = tctx->ticket_key_cb(hs->ssl, key_name, iv, ctx.get(), hctx.get(), + 1 /* encrypt */); + if (ret < 0) { return 0; } + if (ret == 0) { + // The caller requested to send no ticket, so write nothing to |out|. + return 1; + } } else { // Rotate ticket key if necessary. if (!ssl_ctx_rotate_ticket_encryption_key(tctx)) { diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index 2a3b648b1..462172af9 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go @@ -1480,10 +1480,17 @@ type ProtocolBugs struct { // modes to send. If a non-nil empty slice, no extension will be sent. SendPSKKeyExchangeModes []byte - // ExpectNoNewSessionTicket, if present, means that the client will fail upon + // ExpectNoNewSessionTicket, if true, means that the client will fail upon // receipt of a NewSessionTicket message. ExpectNoNewSessionTicket bool + // ExpectNoNonEmptyNewSessionTicket, if true, means that the client will + // fail upon receipt of a NewSessionTicket message that was non-empty. In + // TLS 1.3, this is the same as ExpectNoNewSessionTicket. In TLS 1.2, this + // allows the server to commit to sending NewSessionTicket, but then decline + // to send one. + ExpectNoNonEmptyNewSessionTicket bool + // DuplicateTicketEarlyData causes an extra empty extension of early_data to // be sent in NewSessionTicket. DuplicateTicketEarlyData bool diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 7f6ea6496..c025548ba 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -1611,7 +1611,7 @@ func (c *Conn) processTLS13NewSessionTicket(newSessionTicket *newSessionTicketMs return errors.New("tls: no early_data ticket extension found") } - if c.config.Bugs.ExpectNoNewSessionTicket { + if c.config.Bugs.ExpectNoNewSessionTicket || c.config.Bugs.ExpectNoNonEmptyNewSessionTicket { return errors.New("tls: received unexpected NewSessionTicket") } diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 70f8c287d..35ba616c7 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go @@ -2237,6 +2237,10 @@ func (hs *clientHandshakeState) readSessionTicket() error { return unexpectedMessageError(sessionTicketMsg, msg) } + if c.config.Bugs.ExpectNoNonEmptyNewSessionTicket && len(sessionTicketMsg.ticket) != 0 { + return errors.New("tls: received unexpected non-empty NewSessionTicket") + } + session.sessionTicket = sessionTicketMsg.ticket hs.session = session diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 532fa5f81..fad3ac805 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -8608,6 +8608,18 @@ func addExtensionTests() { resumeSession: true, }) } + testCases = append(testCases, testCase{ + protocol: protocol, + testType: serverTest, + name: "TicketCallback-Skip-" + callbackSuffix, + config: Config{ + MaxVersion: ver.version, + Bugs: ProtocolBugs{ + ExpectNoNonEmptyNewSessionTicket: true, + }, + }, + flags: []string{flag, "-skip-ticket"}, + }) // Test that the ticket callback is only called once when everything before // it in the ClientHello is asynchronous. This corrupts the ticket so diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index cf47abc05..0485f9854 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -382,6 +382,7 @@ const Flag *FindFlag(const char *name) { BoolFlag("-use-ticket-aead-callback", &TestConfig::use_ticket_aead_callback), BoolFlag("-renew-ticket", &TestConfig::renew_ticket), + BoolFlag("-skip-ticket", &TestConfig::skip_ticket), BoolFlag("-enable-early-data", &TestConfig::enable_early_data), Base64Flag("-expect-ocsp-response", &TestConfig::expect_ocsp_response), BoolFlag("-check-close-notify", &TestConfig::check_close_notify), @@ -858,6 +859,9 @@ static int TicketKeyCallback(SSL *ssl, uint8_t *key_name, uint8_t *iv, static const uint8_t kZeros[16] = {0}; if (encrypt) { + if (GetTestConfig(ssl)->skip_ticket) { + return 0; + } OPENSSL_memcpy(key_name, kZeros, sizeof(kZeros)); RAND_bytes(iv, 16); } else if (OPENSSL_memcmp(key_name, kZeros, 16) != 0) { @@ -1225,6 +1229,11 @@ static size_t AsyncTicketMaxOverhead(SSL *ssl) { static int AsyncTicketSeal(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out_len, const uint8_t *in, size_t in_len) { + if (GetTestConfig(ssl)->skip_ticket) { + *out_len = 0; + return 1; + } + auto out_span = bssl::MakeSpan(out, max_out_len); // Encrypt the ticket with the all zero key and a random nonce. static const uint8_t kKey[16] = {0}; diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 36f970149..093d2410a 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -139,6 +139,7 @@ struct TestConfig { bool use_ticket_callback = false; bool use_ticket_aead_callback = false; bool renew_ticket = false; + bool skip_ticket = false; bool enable_early_data = false; std::string ocsp_response; bool check_close_notify = false; diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 7fda54bdb..709dfd3db 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc @@ -141,6 +141,7 @@ static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) { ssl_session_rebase_time(ssl, hs->new_session.get()); assert(ssl->session_ctx->num_tickets <= kMaxTickets); + bool sent_tickets = false; for (size_t i = 0; i < ssl->session_ctx->num_tickets; i++) { UniquePtr session( SSL_SESSION_dup(hs->new_session.get(), SSL_SESSION_INCLUDE_NONAUTH)); @@ -177,10 +178,18 @@ static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) { !CBB_add_u32(&body, session->ticket_age_add) || !CBB_add_u8_length_prefixed(&body, &nonce_cbb) || !CBB_add_bytes(&nonce_cbb, nonce, sizeof(nonce)) || - !CBB_add_u16_length_prefixed(&body, &ticket) || !tls13_derive_session_psk(session.get(), nonce, SSL_is_dtls(ssl)) || - !ssl_encrypt_ticket(hs, &ticket, session.get()) || - !CBB_add_u16_length_prefixed(&body, &extensions)) { + !CBB_add_u16_length_prefixed(&body, &ticket) || + !ssl_encrypt_ticket(hs, &ticket, session.get())) { + return false; + } + + if (CBB_len(&ticket) == 0) { + // The caller decided not to encrypt a ticket. Skip the message. + continue; + } + + if (!CBB_add_u16_length_prefixed(&body, &extensions)) { return false; } @@ -204,9 +213,10 @@ static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) { if (!ssl_add_message_cbb(ssl, cbb.get())) { return false; } + sent_tickets = true; } - *out_sent_tickets = true; + *out_sent_tickets = sent_tickets; return true; }