Remove custom extensions support. Update-Note: Custom extensions APIs are removed. Change-Id: Ic5e0fb3c018bf15d35d9149623f6b29940041b59 Reviewed-on: https://boringssl-review.googlesource.com/29685 Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Reviewed-by: Adam Langley <agl@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index f693030..a16a367 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h
@@ -1533,92 +1533,6 @@ const uint8_t *context, size_t context_len, int use_context); -// Custom extensions. -// -// The custom extension functions allow TLS extensions to be added to -// ClientHello and ServerHello messages. - -// SSL_custom_ext_add_cb is a callback function that is called when the -// ClientHello (for clients) or ServerHello (for servers) is constructed. In -// the case of a server, this callback will only be called for a given -// extension if the ClientHello contained that extension – it's not possible to -// inject extensions into a ServerHello that the client didn't request. -// -// When called, |extension_value| will contain the extension number that is -// being considered for addition (so that a single callback can handle multiple -// extensions). If the callback wishes to include the extension, it must set -// |*out| to point to |*out_len| bytes of extension contents and return one. In -// this case, the corresponding |SSL_custom_ext_free_cb| callback will later be -// called with the value of |*out| once that data has been copied. -// -// If the callback does not wish to add an extension it must return zero. -// -// Alternatively, the callback can abort the connection by setting -// |*out_alert_value| to a TLS alert number and returning -1. -typedef int (*SSL_custom_ext_add_cb)(SSL *ssl, unsigned extension_value, - const uint8_t **out, size_t *out_len, - int *out_alert_value, void *add_arg); - -// SSL_custom_ext_free_cb is a callback function that is called by OpenSSL iff -// an |SSL_custom_ext_add_cb| callback previously returned one. In that case, -// this callback is called and passed the |out| pointer that was returned by -// the add callback. This is to free any dynamically allocated data created by -// the add callback. -typedef void (*SSL_custom_ext_free_cb)(SSL *ssl, unsigned extension_value, - const uint8_t *out, void *add_arg); - -// SSL_custom_ext_parse_cb is a callback function that is called by OpenSSL to -// parse an extension from the peer: that is from the ServerHello for a client -// and from the ClientHello for a server. -// -// When called, |extension_value| will contain the extension number and the -// contents of the extension are |contents_len| bytes at |contents|. -// -// The callback must return one to continue the handshake. Otherwise, if it -// returns zero, a fatal alert with value |*out_alert_value| is sent and the -// handshake is aborted. -typedef int (*SSL_custom_ext_parse_cb)(SSL *ssl, unsigned extension_value, - const uint8_t *contents, - size_t contents_len, - int *out_alert_value, void *parse_arg); - -// SSL_extension_supported returns one iff OpenSSL internally handles -// extensions of type |extension_value|. This can be used to avoid registering -// custom extension handlers for extensions that a future version of OpenSSL -// may handle internally. -OPENSSL_EXPORT int SSL_extension_supported(unsigned extension_value); - -// SSL_CTX_add_client_custom_ext registers callback functions for handling -// custom TLS extensions for client connections. -// -// If |add_cb| is NULL then an empty extension will be added in each -// ClientHello. Otherwise, see the comment for |SSL_custom_ext_add_cb| about -// this callback. -// -// The |free_cb| may be NULL if |add_cb| doesn't dynamically allocate data that -// needs to be freed. -// -// It returns one on success or zero on error. It's always an error to register -// callbacks for the same extension twice, or to register callbacks for an -// extension that OpenSSL handles internally. See |SSL_extension_supported| to -// discover, at runtime, which extensions OpenSSL handles internally. -OPENSSL_EXPORT int SSL_CTX_add_client_custom_ext( - SSL_CTX *ctx, unsigned extension_value, SSL_custom_ext_add_cb add_cb, - SSL_custom_ext_free_cb free_cb, void *add_arg, - SSL_custom_ext_parse_cb parse_cb, void *parse_arg); - -// SSL_CTX_add_server_custom_ext is the same as -// |SSL_CTX_add_client_custom_ext|, but for server connections. -// -// Unlike on the client side, if |add_cb| is NULL no extension will be added. -// The |add_cb|, if any, will only be called if the ClientHello contained a -// matching extension. -OPENSSL_EXPORT int SSL_CTX_add_server_custom_ext( - SSL_CTX *ctx, unsigned extension_value, SSL_custom_ext_add_cb add_cb, - SSL_custom_ext_free_cb free_cb, void *add_arg, - SSL_custom_ext_parse_cb parse_cb, void *parse_arg); - - // Sessions. // // An |SSL_SESSION| represents an SSL session that may be resumed in an
diff --git a/ssl/CMakeLists.txt b/ssl/CMakeLists.txt index 3eab0b5..f2f60ca 100644 --- a/ssl/CMakeLists.txt +++ b/ssl/CMakeLists.txt
@@ -4,7 +4,6 @@ ssl bio_ssl.cc - custom_extensions.cc d1_both.cc d1_lib.cc d1_pkt.cc
diff --git a/ssl/custom_extensions.cc b/ssl/custom_extensions.cc deleted file mode 100644 index 85b8a33..0000000 --- a/ssl/custom_extensions.cc +++ /dev/null
@@ -1,265 +0,0 @@ -/* Copyright (c) 2014, Google Inc. - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY - * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION - * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN - * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ - -#include <openssl/ssl.h> - -#include <assert.h> -#include <string.h> - -#include <openssl/bytestring.h> -#include <openssl/err.h> -#include <openssl/mem.h> -#include <openssl/stack.h> - -#include "internal.h" - - -namespace bssl { - -void SSL_CUSTOM_EXTENSION_free(SSL_CUSTOM_EXTENSION *custom_extension) { - OPENSSL_free(custom_extension); -} - -static const SSL_CUSTOM_EXTENSION *custom_ext_find( - STACK_OF(SSL_CUSTOM_EXTENSION) *stack, - unsigned *out_index, uint16_t value) { - for (size_t i = 0; i < sk_SSL_CUSTOM_EXTENSION_num(stack); i++) { - const SSL_CUSTOM_EXTENSION *ext = sk_SSL_CUSTOM_EXTENSION_value(stack, i); - if (ext->value == value) { - if (out_index != NULL) { - *out_index = i; - } - return ext; - } - } - - return NULL; -} - -// default_add_callback is used as the |add_callback| when the user doesn't -// provide one. For servers, it does nothing while, for clients, it causes an -// empty extension to be included. -static int default_add_callback(SSL *ssl, unsigned extension_value, - const uint8_t **out, size_t *out_len, - int *out_alert_value, void *add_arg) { - if (ssl->server) { - return 0; - } - *out_len = 0; - return 1; -} - -static int custom_ext_add_hello(SSL_HANDSHAKE *hs, CBB *extensions) { - SSL *const ssl = hs->ssl; - STACK_OF(SSL_CUSTOM_EXTENSION) *stack = ssl->ctx->client_custom_extensions; - if (ssl->server) { - stack = ssl->ctx->server_custom_extensions; - } - - if (stack == NULL) { - return 1; - } - - for (size_t i = 0; i < sk_SSL_CUSTOM_EXTENSION_num(stack); i++) { - const SSL_CUSTOM_EXTENSION *ext = sk_SSL_CUSTOM_EXTENSION_value(stack, i); - - if (ssl->server && - !(hs->custom_extensions.received & (1u << i))) { - // Servers cannot echo extensions that the client didn't send. - continue; - } - - const uint8_t *contents; - size_t contents_len; - int alert = SSL_AD_DECODE_ERROR; - CBB contents_cbb; - - switch (ext->add_callback(ssl, ext->value, &contents, &contents_len, &alert, - ext->add_arg)) { - case 1: - if (!CBB_add_u16(extensions, ext->value) || - !CBB_add_u16_length_prefixed(extensions, &contents_cbb) || - !CBB_add_bytes(&contents_cbb, contents, contents_len) || - !CBB_flush(extensions)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - ERR_add_error_dataf("extension %u", (unsigned) ext->value); - if (ext->free_callback && 0 < contents_len) { - ext->free_callback(ssl, ext->value, contents, ext->add_arg); - } - return 0; - } - - if (ext->free_callback && 0 < contents_len) { - ext->free_callback(ssl, ext->value, contents, ext->add_arg); - } - - if (!ssl->server) { - assert((hs->custom_extensions.sent & (1u << i)) == 0); - hs->custom_extensions.sent |= (1u << i); - } - break; - - case 0: - break; - - default: - ssl_send_alert(ssl, SSL3_AL_FATAL, alert); - OPENSSL_PUT_ERROR(SSL, SSL_R_CUSTOM_EXTENSION_ERROR); - ERR_add_error_dataf("extension %u", (unsigned) ext->value); - return 0; - } - } - - return 1; -} - -int custom_ext_add_clienthello(SSL_HANDSHAKE *hs, CBB *extensions) { - return custom_ext_add_hello(hs, extensions); -} - -int custom_ext_parse_serverhello(SSL_HANDSHAKE *hs, int *out_alert, - uint16_t value, const CBS *extension) { - SSL *const ssl = hs->ssl; - unsigned index; - const SSL_CUSTOM_EXTENSION *ext = - custom_ext_find(ssl->ctx->client_custom_extensions, &index, value); - - if (// Unknown extensions are not allowed in a ServerHello. - ext == NULL || - // Also, if we didn't send the extension, that's also unacceptable. - !(hs->custom_extensions.sent & (1u << index))) { - OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); - ERR_add_error_dataf("extension %u", (unsigned)value); - *out_alert = SSL_AD_UNSUPPORTED_EXTENSION; - return 0; - } - - if (ext->parse_callback != NULL && - !ext->parse_callback(ssl, value, CBS_data(extension), CBS_len(extension), - out_alert, ext->parse_arg)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_CUSTOM_EXTENSION_ERROR); - ERR_add_error_dataf("extension %u", (unsigned)ext->value); - return 0; - } - - return 1; -} - -int custom_ext_parse_clienthello(SSL_HANDSHAKE *hs, int *out_alert, - uint16_t value, const CBS *extension) { - SSL *const ssl = hs->ssl; - unsigned index; - const SSL_CUSTOM_EXTENSION *ext = - custom_ext_find(ssl->ctx->server_custom_extensions, &index, value); - - if (ext == NULL) { - return 1; - } - - assert((hs->custom_extensions.received & (1u << index)) == 0); - hs->custom_extensions.received |= (1u << index); - - if (ext->parse_callback && - !ext->parse_callback(ssl, value, CBS_data(extension), CBS_len(extension), - out_alert, ext->parse_arg)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_CUSTOM_EXTENSION_ERROR); - ERR_add_error_dataf("extension %u", (unsigned)ext->value); - return 0; - } - - return 1; -} - -int custom_ext_add_serverhello(SSL_HANDSHAKE *hs, CBB *extensions) { - return custom_ext_add_hello(hs, extensions); -} - -// MAX_NUM_CUSTOM_EXTENSIONS is the maximum number of custom extensions that -// can be set on an |SSL_CTX|. It's determined by the size of the bitset used -// to track when an extension has been sent. -#define MAX_NUM_CUSTOM_EXTENSIONS \ - (sizeof(((SSL_HANDSHAKE *)NULL)->custom_extensions.sent) * 8) - -static int custom_ext_append(STACK_OF(SSL_CUSTOM_EXTENSION) **stack, - unsigned extension_value, - SSL_custom_ext_add_cb add_cb, - SSL_custom_ext_free_cb free_cb, void *add_arg, - SSL_custom_ext_parse_cb parse_cb, - void *parse_arg) { - if (add_cb == NULL || - 0xffff < extension_value || - SSL_extension_supported(extension_value) || - // Specifying a free callback without an add callback is nonsensical - // and an error. - (*stack != NULL && - (MAX_NUM_CUSTOM_EXTENSIONS <= sk_SSL_CUSTOM_EXTENSION_num(*stack) || - custom_ext_find(*stack, NULL, extension_value) != NULL))) { - return 0; - } - - SSL_CUSTOM_EXTENSION *ext = - (SSL_CUSTOM_EXTENSION *)OPENSSL_malloc(sizeof(SSL_CUSTOM_EXTENSION)); - if (ext == NULL) { - return 0; - } - ext->add_callback = add_cb; - ext->add_arg = add_arg; - ext->free_callback = free_cb; - ext->parse_callback = parse_cb; - ext->parse_arg = parse_arg; - ext->value = extension_value; - - if (*stack == NULL) { - *stack = sk_SSL_CUSTOM_EXTENSION_new_null(); - if (*stack == NULL) { - SSL_CUSTOM_EXTENSION_free(ext); - return 0; - } - } - - if (!sk_SSL_CUSTOM_EXTENSION_push(*stack, ext)) { - SSL_CUSTOM_EXTENSION_free(ext); - if (sk_SSL_CUSTOM_EXTENSION_num(*stack) == 0) { - sk_SSL_CUSTOM_EXTENSION_free(*stack); - *stack = NULL; - } - return 0; - } - - return 1; -} - -} // namespace bssl - -using namespace bssl; - -int SSL_CTX_add_client_custom_ext(SSL_CTX *ctx, unsigned extension_value, - SSL_custom_ext_add_cb add_cb, - SSL_custom_ext_free_cb free_cb, void *add_arg, - SSL_custom_ext_parse_cb parse_cb, - void *parse_arg) { - return custom_ext_append(&ctx->client_custom_extensions, extension_value, - add_cb ? add_cb : default_add_callback, free_cb, - add_arg, parse_cb, parse_arg); -} - -int SSL_CTX_add_server_custom_ext(SSL_CTX *ctx, unsigned extension_value, - SSL_custom_ext_add_cb add_cb, - SSL_custom_ext_free_cb free_cb, void *add_arg, - SSL_custom_ext_parse_cb parse_cb, - void *parse_arg) { - return custom_ext_append(&ctx->server_custom_extensions, extension_value, - add_cb ? add_cb : default_add_callback, free_cb, - add_arg, parse_cb, parse_arg); -}
diff --git a/ssl/handshake.cc b/ssl/handshake.cc index 9549f7c..4683ac5 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc
@@ -130,7 +130,6 @@ needs_psk_binder(false), received_hello_retry_request(false), sent_hello_retry_request(false), - received_custom_extension(false), handshake_finalized(false), accept_psk_mode(false), cert_request(false),
diff --git a/ssl/internal.h b/ssl/internal.h index 20efc9d..14daeff 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -929,36 +929,6 @@ Span<const uint8_t> in); -// Custom extensions - -} // namespace bssl - -// |SSL_CUSTOM_EXTENSION| is a structure that contains information about -// custom-extension callbacks. It is defined unnamespaced for compatibility with -// |STACK_OF(SSL_CUSTOM_EXTENSION)|. -typedef struct ssl_custom_extension { - SSL_custom_ext_add_cb add_callback; - void *add_arg; - SSL_custom_ext_free_cb free_callback; - SSL_custom_ext_parse_cb parse_callback; - void *parse_arg; - uint16_t value; -} SSL_CUSTOM_EXTENSION; - -DEFINE_STACK_OF(SSL_CUSTOM_EXTENSION) - -namespace bssl { - -void SSL_CUSTOM_EXTENSION_free(SSL_CUSTOM_EXTENSION *custom_extension); - -int custom_ext_add_clienthello(SSL_HANDSHAKE *hs, CBB *extensions); -int custom_ext_parse_serverhello(SSL_HANDSHAKE *hs, int *out_alert, - uint16_t value, const CBS *extension); -int custom_ext_parse_clienthello(SSL_HANDSHAKE *hs, int *out_alert, - uint16_t value, const CBS *extension); -int custom_ext_add_serverhello(SSL_HANDSHAKE *hs, CBB *extensions); - - // Key shares. // SSLKeyShare abstracts over Diffie-Hellman-like key exchanges. @@ -1421,17 +1391,6 @@ uint32_t received; } extensions; - union { - // sent is a bitset where the bits correspond to elements of - // |client_custom_extensions| in the |SSL_CTX|. Each bit is set if that - // extension was sent in a ClientHello. It's not used by servers. - uint16_t sent = 0; - // received is a bitset, like |sent|, but is used by servers to record - // which custom extensions were received from a client. The bits here - // correspond to |server_custom_extensions|. - uint16_t received; - } custom_extensions; - // retry_group is the group ID selected by the server in HelloRetryRequest in // TLS 1.3. uint16_t retry_group = 0; @@ -1532,8 +1491,6 @@ bool received_hello_retry_request:1; bool sent_hello_retry_request:1; - bool received_custom_extension:1; - // handshake_finalized is true once the handshake has completed, at which // point accessors should use the established state. bool handshake_finalized:1; @@ -2889,11 +2846,6 @@ CRYPTO_EX_DATA ex_data; - // custom_*_extensions stores any callback sets for custom extensions. Note - // that these pointers will be NULL if the stack would otherwise be empty. - STACK_OF(SSL_CUSTOM_EXTENSION) *client_custom_extensions = nullptr; - STACK_OF(SSL_CUSTOM_EXTENSION) *server_custom_extensions = nullptr; - // Default values used when no per-SSL value is defined follow void (*info_callback)(const SSL *ssl, int type, int value) = nullptr;
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc index 7e26fa0..572e79d 100644 --- a/ssl/ssl_lib.cc +++ b/ssl/ssl_lib.cc
@@ -562,10 +562,6 @@ CRYPTO_MUTEX_cleanup(&lock); lh_SSL_SESSION_free(sessions); - sk_SSL_CUSTOM_EXTENSION_pop_free(client_custom_extensions, - SSL_CUSTOM_EXTENSION_free); - sk_SSL_CUSTOM_EXTENSION_pop_free(server_custom_extensions, - SSL_CUSTOM_EXTENSION_free); x509_method->ssl_ctx_free(this); sk_CertCompressionAlg_pop_free(cert_compression_algs, Delete<CertCompressionAlg>);
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 3cb1063..cfb4cef 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc
@@ -3068,7 +3068,6 @@ } hs->extensions.sent = 0; - hs->custom_extensions.sent = 0; for (size_t i = 0; i < kNumExtensions; i++) { if (kExtensions[i].init != NULL) { @@ -3100,11 +3099,6 @@ } } - if (!custom_ext_add_clienthello(hs, &extensions)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return 0; - } - if (ssl->ctx->grease_enabled) { // Add a fake non-empty extension. See draft-davidben-tls-grease-01. uint16_t grease_ext2 = ssl_get_grease_value(hs, ssl_grease_extension2); @@ -3189,10 +3183,6 @@ } } - if (!custom_ext_add_serverhello(hs, &extensions)) { - goto err; - } - // Discard empty extensions blocks before TLS 1.3. if (ssl_protocol_version(ssl) < TLS1_3_VERSION && CBB_len(&extensions) == 0) { @@ -3216,7 +3206,6 @@ } hs->extensions.received = 0; - hs->custom_extensions.received = 0; CBS extensions; CBS_init(&extensions, client_hello->extensions, client_hello->extensions_len); while (CBS_len(&extensions) != 0) { @@ -3234,10 +3223,6 @@ const struct tls_extension *const ext = tls_extension_find(&ext_index, type); if (ext == NULL) { - if (!custom_ext_parse_clienthello(hs, out_alert, type, &extension)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_PARSING_EXTENSION); - return 0; - } continue; } @@ -3333,11 +3318,10 @@ tls_extension_find(&ext_index, type); if (ext == NULL) { - hs->received_custom_extension = true; - if (!custom_ext_parse_serverhello(hs, out_alert, type, &extension)) { - return 0; - } - continue; + OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION); + ERR_add_error_dataf("extension %u", (unsigned)type); + *out_alert = SSL_AD_UNSUPPORTED_EXTENSION; + return 0; } static_assert(kNumExtensions <= sizeof(hs->extensions.sent) * 8, @@ -3941,9 +3925,3 @@ void SSL_CTX_set_rsa_pss_rsae_certs_enabled(SSL_CTX *ctx, int enabled) { ctx->rsa_pss_rsae_certs_enabled = !!enabled; } - -int SSL_extension_supported(unsigned extension_value) { - uint32_t index; - return extension_value == TLSEXT_TYPE_padding || - tls_extension_find(&index, extension_value) != NULL; -}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index b296260..d88e389 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -10259,278 +10259,6 @@ } func addCustomExtensionTests() { - expectedContents := "custom extension" - emptyString := "" - - for _, isClient := range []bool{false, true} { - suffix := "Server" - flag := "-enable-server-custom-extension" - testType := serverTest - if isClient { - suffix = "Client" - flag = "-enable-client-custom-extension" - testType = clientTest - } - - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-" + suffix, - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag}, - }) - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-" + suffix + "-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag}, - }) - - // If the parse callback fails, the handshake should also fail. - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-ParseError-" + suffix, - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents + "foo", - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag}, - shouldFail: true, - expectedError: ":CUSTOM_EXTENSION_ERROR:", - }) - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-ParseError-" + suffix + "-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents + "foo", - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag}, - shouldFail: true, - expectedError: ":CUSTOM_EXTENSION_ERROR:", - }) - - // If the add callback fails, the handshake should also fail. - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-FailAdd-" + suffix, - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag, "-custom-extension-fail-add"}, - shouldFail: true, - expectedError: ":CUSTOM_EXTENSION_ERROR:", - }) - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-FailAdd-" + suffix + "-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - flags: []string{flag, "-custom-extension-fail-add"}, - shouldFail: true, - expectedError: ":CUSTOM_EXTENSION_ERROR:", - }) - - // If the add callback returns zero, no extension should be - // added. - skipCustomExtension := expectedContents - if isClient { - // For the case where the client skips sending the - // custom extension, the server must not “echo” it. - skipCustomExtension = "" - } - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-Skip-" + suffix, - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - CustomExtension: skipCustomExtension, - ExpectedCustomExtension: &emptyString, - }, - }, - flags: []string{flag, "-custom-extension-skip"}, - }) - testCases = append(testCases, testCase{ - testType: testType, - name: "CustomExtensions-Skip-" + suffix + "-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - CustomExtension: skipCustomExtension, - ExpectedCustomExtension: &emptyString, - }, - }, - flags: []string{flag, "-custom-extension-skip"}, - }) - } - - // If the client sends both early data and custom extension, the handshake - // should succeed as long as both the extensions aren't returned by the - // server. - testCases = append(testCases, testCase{ - testType: clientTest, - name: "CustomExtensions-Client-EarlyData-None", - config: Config{ - MaxVersion: VersionTLS13, - MaxEarlyDataSize: 16384, - Bugs: ProtocolBugs{ - ExpectedCustomExtension: &expectedContents, - AlwaysRejectEarlyData: true, - }, - }, - resumeSession: true, - flags: []string{ - "-enable-client-custom-extension", - "-enable-early-data", - "-expect-ticket-supports-early-data", - "-expect-reject-early-data", - }, - }) - - testCases = append(testCases, testCase{ - testType: clientTest, - name: "CustomExtensions-Client-EarlyData-EarlyDataAccepted", - config: Config{ - MaxVersion: VersionTLS13, - MaxEarlyDataSize: 16384, - Bugs: ProtocolBugs{ - ExpectedCustomExtension: &expectedContents, - }, - }, - resumeSession: true, - flags: []string{ - "-enable-client-custom-extension", - "-enable-early-data", - "-expect-ticket-supports-early-data", - "-expect-accept-early-data", - }, - }) - - testCases = append(testCases, testCase{ - testType: clientTest, - name: "CustomExtensions-Client-EarlyData-CustomExtensionAccepted", - config: Config{ - MaxVersion: VersionTLS13, - MaxEarlyDataSize: 16384, - Bugs: ProtocolBugs{ - AlwaysRejectEarlyData: true, - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - resumeSession: true, - flags: []string{ - "-enable-client-custom-extension", - "-enable-early-data", - "-expect-ticket-supports-early-data", - "-expect-reject-early-data", - }, - }) - - testCases = append(testCases, testCase{ - testType: clientTest, - name: "CustomExtensions-Client-EarlyDataAndCustomExtensions", - config: Config{ - MaxVersion: VersionTLS13, - MaxEarlyDataSize: 16384, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - }, - }, - resumeConfig: &Config{ - MaxVersion: VersionTLS13, - MaxEarlyDataSize: 16384, - Bugs: ProtocolBugs{ - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - SendEarlyDataExtension: true, - }, - }, - resumeSession: true, - shouldFail: true, - expectedError: ":UNEXPECTED_EXTENSION_ON_EARLY_DATA:", - flags: []string{ - "-enable-client-custom-extension", - "-enable-early-data", - "-expect-ticket-supports-early-data", - }, - }) - - // If the server receives both early data and custom extension, only the - // custom extension should be accepted. - testCases = append(testCases, testCase{ - testType: serverTest, - name: "CustomExtensions-Server-EarlyDataOffered", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - SendEarlyData: [][]byte{{1, 2, 3, 4}}, - CustomExtension: expectedContents, - ExpectedCustomExtension: &expectedContents, - ExpectEarlyDataAccepted: false, - }, - }, - resumeSession: true, - flags: []string{ - "-enable-server-custom-extension", - "-enable-early-data", - }, - }) - - // The custom extension add callback should not be called if the client - // doesn't send the extension. - testCases = append(testCases, testCase{ - testType: serverTest, - name: "CustomExtensions-NotCalled-Server", - config: Config{ - MaxVersion: VersionTLS12, - Bugs: ProtocolBugs{ - ExpectedCustomExtension: &emptyString, - }, - }, - flags: []string{"-enable-server-custom-extension", "-custom-extension-fail-add"}, - }) - - testCases = append(testCases, testCase{ - testType: serverTest, - name: "CustomExtensions-NotCalled-Server-TLS13", - config: Config{ - MaxVersion: VersionTLS13, - Bugs: ProtocolBugs{ - ExpectedCustomExtension: &emptyString, - }, - }, - flags: []string{"-enable-server-custom-extension", "-custom-extension-fail-add"}, - }) - // Test an unknown extension from the server. testCases = append(testCases, testCase{ testType: clientTest, @@ -10538,7 +10266,7 @@ config: Config{ MaxVersion: VersionTLS12, Bugs: ProtocolBugs{ - CustomExtension: expectedContents, + CustomExtension: "custom extension", }, }, shouldFail: true, @@ -10551,7 +10279,7 @@ config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - CustomExtension: expectedContents, + CustomExtension: "custom extension", }, }, shouldFail: true, @@ -10564,7 +10292,7 @@ config: Config{ MaxVersion: VersionTLS13, Bugs: ProtocolBugs{ - CustomUnencryptedExtension: expectedContents, + CustomUnencryptedExtension: "custom extension", }, }, shouldFail: true,
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index 3c5046e..3b9bcd9 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc
@@ -92,12 +92,6 @@ { "-use-ticket-callback", &TestConfig::use_ticket_callback }, { "-renew-ticket", &TestConfig::renew_ticket }, { "-enable-early-data", &TestConfig::enable_early_data }, - { "-enable-client-custom-extension", - &TestConfig::enable_client_custom_extension }, - { "-enable-server-custom-extension", - &TestConfig::enable_server_custom_extension }, - { "-custom-extension-skip", &TestConfig::custom_extension_skip }, - { "-custom-extension-fail-add", &TestConfig::custom_extension_fail_add }, { "-check-close-notify", &TestConfig::check_close_notify }, { "-shim-shuts-down", &TestConfig::shim_shuts_down }, { "-verify-fail", &TestConfig::verify_fail }, @@ -392,63 +386,6 @@ return SSL_TLSEXT_ERR_OK; } -// kCustomExtensionValue is the extension value that the custom extension -// callbacks will add. -static const uint16_t kCustomExtensionValue = 1234; -static void *const kCustomExtensionAddArg = - reinterpret_cast<void *>(kCustomExtensionValue); -static void *const kCustomExtensionParseArg = - reinterpret_cast<void *>(kCustomExtensionValue + 1); -static const char kCustomExtensionContents[] = "custom extension"; - -static int CustomExtensionAddCallback(SSL *ssl, unsigned extension_value, - const uint8_t **out, size_t *out_len, - int *out_alert_value, void *add_arg) { - if (extension_value != kCustomExtensionValue || - add_arg != kCustomExtensionAddArg) { - abort(); - } - - if (GetTestConfig(ssl)->custom_extension_skip) { - return 0; - } - if (GetTestConfig(ssl)->custom_extension_fail_add) { - return -1; - } - - *out = reinterpret_cast<const uint8_t *>(kCustomExtensionContents); - *out_len = sizeof(kCustomExtensionContents) - 1; - - return 1; -} - -static void CustomExtensionFreeCallback(SSL *ssl, unsigned extension_value, - const uint8_t *out, void *add_arg) { - if (extension_value != kCustomExtensionValue || - add_arg != kCustomExtensionAddArg || - out != reinterpret_cast<const uint8_t *>(kCustomExtensionContents)) { - abort(); - } -} - -static int CustomExtensionParseCallback(SSL *ssl, unsigned extension_value, - const uint8_t *contents, - size_t contents_len, - int *out_alert_value, void *parse_arg) { - if (extension_value != kCustomExtensionValue || - parse_arg != kCustomExtensionParseArg) { - abort(); - } - - if (contents_len != sizeof(kCustomExtensionContents) - 1 || - OPENSSL_memcmp(contents, kCustomExtensionContents, contents_len) != 0) { - *out_alert_value = SSL_AD_DECODE_ERROR; - return 0; - } - - return 1; -} - static int ServerNameCallback(SSL *ssl, int *out_alert, void *arg) { // SNI must be accessible from the SNI callback. const TestConfig *config = GetTestConfig(ssl); @@ -1234,22 +1171,6 @@ SSL_CTX_set_tlsext_ticket_key_cb(ssl_ctx.get(), TicketKeyCallback); } - if (enable_client_custom_extension && - !SSL_CTX_add_client_custom_ext( - ssl_ctx.get(), kCustomExtensionValue, CustomExtensionAddCallback, - CustomExtensionFreeCallback, kCustomExtensionAddArg, - CustomExtensionParseCallback, kCustomExtensionParseArg)) { - return nullptr; - } - - if (enable_server_custom_extension && - !SSL_CTX_add_server_custom_ext( - ssl_ctx.get(), kCustomExtensionValue, CustomExtensionAddCallback, - CustomExtensionFreeCallback, kCustomExtensionAddArg, - CustomExtensionParseCallback, kCustomExtensionParseArg)) { - return nullptr; - } - if (!use_custom_verify_callback) { SSL_CTX_set_cert_verify_callback(ssl_ctx.get(), CertVerifyCallback, NULL); }
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc index 9fdd313..40281a0 100644 --- a/ssl/tls13_client.cc +++ b/ssl/tls13_client.cc
@@ -442,8 +442,7 @@ OPENSSL_PUT_ERROR(SSL, SSL_R_ALPN_MISMATCH_ON_EARLY_DATA); return ssl_hs_error; } - if (ssl->s3->channel_id_valid || hs->received_custom_extension || - ssl->s3->token_binding_negotiated) { + if (ssl->s3->channel_id_valid || ssl->s3->token_binding_negotiated) { OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION_ON_EARLY_DATA); return ssl_hs_error; }
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 67aa5d0..3c2c774 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc
@@ -392,8 +392,6 @@ !ssl->s3->channel_id_valid && // If Token Binding is negotiated, reject 0-RTT. !ssl->s3->token_binding_negotiated && - // Custom extensions is incompatible with 0-RTT. - hs->custom_extensions.received == 0 && // The negotiated ALPN must match the one in the ticket. MakeConstSpan(ssl->s3->alpn_selected) == session->early_alpn) { ssl->s3->early_data_accepted = true;