Exercise SSL_TICKET_AEAD_METHOD in runner We never actually tested it in runner, though I think we did write ssl_test.cc tests. Adding some tests now to prepare for adding and testing some new behavior shortly. Also fix the documentation for SSL_CTX_set_tlsext_ticket_key_cb returning zero in the encrypt case. Change-Id: Ic7b8d51b2433d56cc524f62565946b906d515257 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/73027 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 0a40b84..b198e0c 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -2469,9 +2469,9 @@ // 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 // match |EVP_CIPHER_CTX_iv_length| of the cipher selected. In this mode, -// |callback| returns -1 to abort the handshake, 0 if decrypting the ticket -// failed, and 1 or 2 on success. If it returns 2, the ticket will be renewed. -// This may be used to re-key the ticket. +// |callback| returns -1 to abort the handshake, 0 if the ticket key was +// unrecognized, and 1 or 2 on success. If it returns 2, the ticket will be +// renewed. This may be used to re-key the ticket. // // WARNING: |callback| wildly breaks the usual return value convention and is // called in two different modes.
diff --git a/ssl/test/handshake_util.cc b/ssl/test/handshake_util.cc index b6d8280..65219d8 100644 --- a/ssl/test/handshake_util.cc +++ b/ssl/test/handshake_util.cc
@@ -101,6 +101,9 @@ case SSL_ERROR_WANT_CERTIFICATE_VERIFY: test_state->custom_verify_ready = true; return true; + case SSL_ERROR_PENDING_TICKET: + test_state->async_ticket_decrypt_ready = true; + return true; default: return false; }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 682ca23..532fa5f 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -8574,54 +8574,65 @@ resumeSession: true, expectResumeRejected: true, }) - // Test the ticket callback, with and without renewal. - testCases = append(testCases, testCase{ - protocol: protocol, - testType: serverTest, - name: "TicketCallback-" + suffix, - config: Config{ - MaxVersion: ver.version, - }, - resumeSession: true, - flags: []string{"-use-ticket-callback"}, - }) - testCases = append(testCases, testCase{ - protocol: protocol, - testType: serverTest, - name: "TicketCallback-Renew-" + suffix, - config: Config{ - MaxVersion: ver.version, - Bugs: ProtocolBugs{ - ExpectNewTicket: true, + // Test the ticket callbacks. + for _, aeadCallback := range []bool{false, true} { + flag := "-use-ticket-callback" + callbackSuffix := suffix + if aeadCallback { + flag = "-use-ticket-aead-callback" + callbackSuffix += "-AEAD" + } + testCases = append(testCases, testCase{ + protocol: protocol, + testType: serverTest, + name: "TicketCallback-" + callbackSuffix, + config: Config{ + MaxVersion: ver.version, }, - }, - flags: []string{"-use-ticket-callback", "-renew-ticket"}, - resumeSession: true, - }) + resumeSession: true, + flags: []string{flag}, + }) + // Only the old callback supports renewal. + if !aeadCallback { + testCases = append(testCases, testCase{ + protocol: protocol, + testType: serverTest, + name: "TicketCallback-Renew-" + callbackSuffix, + config: Config{ + MaxVersion: ver.version, + Bugs: ProtocolBugs{ + ExpectNewTicket: true, + }, + }, + flags: []string{flag, "-renew-ticket"}, + resumeSession: true, + }) + } - // Test that the ticket callback is only called once when everything before - // it in the ClientHello is asynchronous. This corrupts the ticket so - // certificate selection callbacks run. - testCases = append(testCases, testCase{ - protocol: protocol, - testType: serverTest, - name: "TicketCallback-SingleCall-" + suffix, - config: Config{ - MaxVersion: ver.version, - Bugs: ProtocolBugs{ - FilterTicket: func(in []byte) ([]byte, error) { - in[len(in)-1] ^= 1 - return in, nil + // Test that the ticket callback is only called once when everything before + // it in the ClientHello is asynchronous. This corrupts the ticket so + // certificate selection callbacks run. + testCases = append(testCases, testCase{ + protocol: protocol, + testType: serverTest, + name: "TicketCallback-SingleCall-" + callbackSuffix, + config: Config{ + MaxVersion: ver.version, + Bugs: ProtocolBugs{ + FilterTicket: func(in []byte) ([]byte, error) { + in[len(in)-1] ^= 1 + return in, nil + }, }, }, - }, - resumeSession: true, - expectResumeRejected: true, - flags: []string{ - "-use-ticket-callback", - "-async", - }, - }) + resumeSession: true, + expectResumeRejected: true, + flags: []string{ + flag, + "-async", + }, + }) + } // Resume with various lengths of ticket session id. if ver.version < VersionTLS13 {
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index abd66ed..cf47abc 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -28,6 +28,7 @@ #include <memory> #include <type_traits> +#include <openssl/aead.h> #include <openssl/base64.h> #include <openssl/hmac.h> #include <openssl/hpke.h> @@ -354,7 +355,8 @@ BoolFlag("-implicit-handshake", &TestConfig::implicit_handshake), BoolFlag("-use-early-callback", &TestConfig::use_early_callback), BoolFlag("-fail-early-callback", &TestConfig::fail_early_callback), - BoolFlag("-fail-early-callback-ech-rewind", &TestConfig::fail_early_callback_ech_rewind), + BoolFlag("-fail-early-callback-ech-rewind", + &TestConfig::fail_early_callback_ech_rewind), BoolFlag("-install-ddos-callback", &TestConfig::install_ddos_callback), BoolFlag("-fail-ddos-callback", &TestConfig::fail_ddos_callback), BoolFlag("-fail-cert-callback", &TestConfig::fail_cert_callback), @@ -375,9 +377,10 @@ &TestConfig::expect_reject_early_data), BoolFlag("-expect-no-offer-early-data", &TestConfig::expect_no_offer_early_data), - BoolFlag("-expect-no-server-name", - &TestConfig::expect_no_server_name), + BoolFlag("-expect-no-server-name", &TestConfig::expect_no_server_name), BoolFlag("-use-ticket-callback", &TestConfig::use_ticket_callback), + BoolFlag("-use-ticket-aead-callback", + &TestConfig::use_ticket_aead_callback), BoolFlag("-renew-ticket", &TestConfig::renew_ticket), BoolFlag("-enable-early-data", &TestConfig::enable_early_data), Base64Flag("-expect-ocsp-response", &TestConfig::expect_ocsp_response), @@ -1214,6 +1217,88 @@ AsyncPrivateKeyComplete, }; +static size_t AsyncTicketMaxOverhead(SSL *ssl) { + const EVP_AEAD *aead = EVP_aead_aes_128_gcm_siv(); + return EVP_AEAD_max_overhead(aead) + EVP_AEAD_nonce_length(aead); +} + +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) { + 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}; + const EVP_AEAD *aead = EVP_aead_aes_128_gcm_siv(); + size_t nonce_len = EVP_AEAD_nonce_length(aead); + if (out_span.size() < nonce_len) { + return 0; + } + auto nonce = out_span.first(nonce_len); + out_span = out_span.subspan(nonce_len); + RAND_bytes(nonce.data(), nonce.size()); + bssl::ScopedEVP_AEAD_CTX ctx; + size_t len; + if (!EVP_AEAD_CTX_init(ctx.get(), EVP_aead_aes_128_gcm_siv(), kKey, + sizeof(kKey), EVP_AEAD_DEFAULT_TAG_LENGTH, nullptr) || + !EVP_AEAD_CTX_seal(ctx.get(), out_span.data(), &len, out_span.size(), + nonce.data(), nonce.size(), in, in_len, + /*ad=*/nullptr, /*ad_len=*/0)) { + return 0; + } + *out_len = nonce.size() + len; + return 1; +} + +static ssl_ticket_aead_result_t AsyncTicketOpen(SSL *ssl, uint8_t *out, + size_t *out_len, + size_t max_out_len, + const uint8_t *in, + size_t in_len) { + auto in_span = bssl::MakeSpan(in, in_len); + const TestConfig *test_config = GetTestConfig(ssl); + TestState *test_state = GetTestState(ssl); + if (test_state->ticket_decrypt_done) { + fprintf(stderr, "AsyncTicketOpen called after completion.\n"); + return ssl_ticket_aead_error; + } + if (test_config->renew_ticket) { + fprintf(stderr, "-renew-ticket not supported with async tickets.\n"); + return ssl_ticket_aead_error; + } + if (test_config->async && !test_state->async_ticket_decrypt_ready) { + return ssl_ticket_aead_retry; + } + + const EVP_AEAD *aead = EVP_aead_aes_128_gcm_siv(); + size_t nonce_len = EVP_AEAD_nonce_length(aead); + if (in_span.size() < nonce_len) { + return ssl_ticket_aead_error; + } + auto nonce = in_span.first(nonce_len); + in_span = in_span.subspan(nonce_len); + + static const uint8_t kKey[16] = {0}; + bssl::ScopedEVP_AEAD_CTX ctx; + if (!EVP_AEAD_CTX_init(ctx.get(), EVP_aead_aes_128_gcm_siv(), kKey, + sizeof(kKey), EVP_AEAD_DEFAULT_TAG_LENGTH, nullptr)) { + return ssl_ticket_aead_error; + } + if (!EVP_AEAD_CTX_open(ctx.get(), out, out_len, max_out_len, nonce.data(), + nonce.size(), in_span.data(), in_span.size(), + /*ad=*/nullptr, /*ad_len=*/0)) { + ERR_clear_error(); + return ssl_ticket_aead_ignore_ticket; + } + test_state->ticket_decrypt_done = true; + return ssl_ticket_aead_success; +} + +static const SSL_TICKET_AEAD_METHOD g_async_ticket_aead_method = { + AsyncTicketMaxOverhead, + AsyncTicketSeal, + AsyncTicketOpen, +}; + static bssl::UniquePtr<SSL_CREDENTIAL> CredentialFromConfig( const TestConfig &config, const CredentialConfig &cred_config, int number) { bssl::UniquePtr<SSL_CREDENTIAL> cred; @@ -1758,8 +1843,10 @@ SSL_CTX_set_info_callback(ssl_ctx.get(), InfoCallback); SSL_CTX_sess_set_new_cb(ssl_ctx.get(), NewSessionCallback); - if (use_ticket_callback || handshake_hints) { - // If using handshake hints, always enable the ticket callback, so we can + if (use_ticket_aead_callback) { + SSL_CTX_set_ticket_aead_method(ssl_ctx.get(), &g_async_ticket_aead_method); + } else if (use_ticket_callback || handshake_hints) { + // If using handshake hints, always enable some ticket callback, so we can // check that hints only mismatch when allowed. The ticket callback also // uses a constant key, which simplifies the test. SSL_CTX_set_tlsext_ticket_key_cb(ssl_ctx.get(), TicketKeyCallback);
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 0afeb49..36f9701 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h
@@ -137,6 +137,7 @@ bool expect_no_offer_early_data = false; bool expect_no_server_name = false; bool use_ticket_callback = false; + bool use_ticket_aead_callback = false; bool renew_ticket = false; bool enable_early_data = false; std::string ocsp_response;
diff --git a/ssl/test/test_state.h b/ssl/test/test_state.h index 14b4e05..c5a3f9c 100644 --- a/ssl/test/test_state.h +++ b/ssl/test/test_state.h
@@ -56,6 +56,7 @@ unsigned private_key_retries = 0; bool got_new_session = false; bssl::UniquePtr<SSL_SESSION> new_session; + bool async_ticket_decrypt_ready = false; bool ticket_decrypt_done = false; bool alpn_select_done = false; bool early_callback_ready = false;