Revert "Add support for the new QUIC TLS extension codepoint"
This reverts commit 7ba96a675eec621bc897b25b126a95e98f1014bb.
BUG=oss-fuzz:28720
Change-Id: Ibfea49cc3101079573eedbb33c443c85a14e4b4c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44644
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
diff --git a/include/openssl/base.h b/include/openssl/base.h
index 6ecb3d3..60c7e87 100644
--- a/include/openssl/base.h
+++ b/include/openssl/base.h
@@ -191,7 +191,7 @@
// 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 13
+#define BORINGSSL_API_VERSION 12
#if defined(BORINGSSL_SHARED_LIBRARY)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 1d079a0..e40e2b2 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3384,12 +3384,6 @@
OPENSSL_EXPORT void SSL_get_peer_quic_transport_params(
const SSL *ssl, const uint8_t **out_params, size_t *out_params_len);
-// SSL_set_quic_use_legacy_codepoint configures whether to use the legacy QUIC
-// extension codepoint 0xffa5 as opposed to the official value 57. Call with
-// |use_legacy| set to 1 to use 0xffa5 and call with 0 to use 57. The default
-// value for this is currently 1 but it will change to 0 at a later date.
-OPENSSL_EXPORT void SSL_set_quic_use_legacy_codepoint(SSL *ssl, int use_legacy);
-
// SSL_set_quic_early_data_context configures a context string in QUIC servers
// for accepting early data. If a resumption connection offers early data, the
// server will check if the value matches that of the connection which minted
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 4037511..22689a2 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -206,13 +206,10 @@
// ExtensionType value from draft-ietf-tokbind-negotiation-10
#define TLSEXT_TYPE_token_binding 24
-// ExtensionType value from draft-ietf-quic-tls. Drafts 00 through 32 use
-// 0xffa5 which is part of the Private Use section of the registry, and it
-// collides with TLS-LTS and, based on scans, something else too (though this
-// hasn't been a problem in practice since it's QUIC-only). Drafts 33 onward
-// use the value 57 which was officially registered with IANA.
-#define TLSEXT_TYPE_quic_transport_parameters_legacy 0xffa5
-#define TLSEXT_TYPE_quic_transport_parameters 57
+// ExtensionType value from draft-ietf-quic-tls. Note that this collides with
+// TLS-LTS and, based on scans, something else too. Since it's QUIC-only, that
+// shouldn't be a problem in practice.
+#define TLSEXT_TYPE_quic_transport_parameters 0xffa5
// ExtensionType value from RFC8879
#define TLSEXT_TYPE_cert_compression 27
diff --git a/ssl/internal.h b/ssl/internal.h
index 1bae0a3..5545bec 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -2772,10 +2772,6 @@
// jdk11_workaround is whether to disable TLS 1.3 for JDK 11 clients, as a
// workaround for https://bugs.openjdk.java.net/browse/JDK-8211806.
bool jdk11_workaround : 1;
-
- // QUIC drafts up to and including 32 used a different TLS extension
- // codepoint to convey QUIC's transport parameters.
- bool quic_use_legacy_codepoint : 1;
};
// From RFC 8446, used in determining PSK modes.
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 7c7bbbf..8c31871 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -730,8 +730,7 @@
retain_only_sha256_of_client_certs(false),
handoff(false),
shed_handshake_config(false),
- jdk11_workaround(false),
- quic_use_legacy_codepoint(true) {
+ jdk11_workaround(false) {
assert(ssl);
}
@@ -2959,13 +2958,6 @@
ssl->config->jdk11_workaround = !!enable;
}
-void SSL_set_quic_use_legacy_codepoint(SSL *ssl, int use_legacy) {
- if (!ssl->config) {
- return;
- }
- ssl->config->quic_use_legacy_codepoint = !!use_legacy;
-}
-
int SSL_clear(SSL *ssl) {
if (!ssl->config) {
return 0; // SSL_clear may not be used after shedding config.
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index d1d525c..e62d6e2 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -6073,82 +6073,6 @@
ASSERT_TRUE(RunQUICHandshakesAndExpectError(ExpectedError::kClientError));
}
-TEST_F(QUICMethodTest, QuicLegacyCodepointEnabled) {
- const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
- ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
- ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
-
- ASSERT_TRUE(CreateClientAndServer());
- uint8_t kClientParams[] = {1, 2, 3, 4};
- uint8_t kServerParams[] = {5, 6, 7};
- SSL_set_quic_use_legacy_codepoint(client_.get(), 1);
- SSL_set_quic_use_legacy_codepoint(server_.get(), 1);
- ASSERT_TRUE(SSL_set_quic_transport_params(client_.get(), kClientParams,
- sizeof(kClientParams)));
- ASSERT_TRUE(SSL_set_quic_transport_params(server_.get(), kServerParams,
- sizeof(kServerParams)));
-
- ASSERT_TRUE(CompleteHandshakesForQUIC());
- ExpectReceivedTransportParamsEqual(client_.get(), kServerParams);
- ExpectReceivedTransportParamsEqual(server_.get(), kClientParams);
-}
-
-TEST_F(QUICMethodTest, QuicLegacyCodepointDisabled) {
- const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
- ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
- ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
-
- ASSERT_TRUE(CreateClientAndServer());
- uint8_t kClientParams[] = {1, 2, 3, 4};
- uint8_t kServerParams[] = {5, 6, 7};
- SSL_set_quic_use_legacy_codepoint(client_.get(), 0);
- SSL_set_quic_use_legacy_codepoint(server_.get(), 0);
- ASSERT_TRUE(SSL_set_quic_transport_params(client_.get(), kClientParams,
- sizeof(kClientParams)));
- ASSERT_TRUE(SSL_set_quic_transport_params(server_.get(), kServerParams,
- sizeof(kServerParams)));
-
- ASSERT_TRUE(CompleteHandshakesForQUIC());
- ExpectReceivedTransportParamsEqual(client_.get(), kServerParams);
- ExpectReceivedTransportParamsEqual(server_.get(), kClientParams);
-}
-
-TEST_F(QUICMethodTest, QuicLegacyCodepointClientOnly) {
- const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
- ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
- ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
-
- ASSERT_TRUE(CreateClientAndServer());
- uint8_t kClientParams[] = {1, 2, 3, 4};
- uint8_t kServerParams[] = {5, 6, 7};
- SSL_set_quic_use_legacy_codepoint(client_.get(), 1);
- SSL_set_quic_use_legacy_codepoint(server_.get(), 0);
- ASSERT_TRUE(SSL_set_quic_transport_params(client_.get(), kClientParams,
- sizeof(kClientParams)));
- ASSERT_TRUE(SSL_set_quic_transport_params(server_.get(), kServerParams,
- sizeof(kServerParams)));
-
- ASSERT_TRUE(RunQUICHandshakesAndExpectError(ExpectedError::kServerError));
-}
-
-TEST_F(QUICMethodTest, QuicLegacyCodepointServerOnly) {
- const SSL_QUIC_METHOD quic_method = DefaultQUICMethod();
- ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method));
- ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method));
-
- ASSERT_TRUE(CreateClientAndServer());
- uint8_t kClientParams[] = {1, 2, 3, 4};
- uint8_t kServerParams[] = {5, 6, 7};
- SSL_set_quic_use_legacy_codepoint(client_.get(), 0);
- SSL_set_quic_use_legacy_codepoint(server_.get(), 1);
- ASSERT_TRUE(SSL_set_quic_transport_params(client_.get(), kClientParams,
- sizeof(kClientParams)));
- ASSERT_TRUE(SSL_set_quic_transport_params(server_.get(), kServerParams,
- sizeof(kServerParams)));
-
- ASSERT_TRUE(RunQUICHandshakesAndExpectError(ExpectedError::kServerError));
-}
-
extern "C" {
int BORINGSSL_enum_c_type_test(void);
}
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 5affccc..955eae7 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2750,8 +2750,8 @@
// QUIC Transport Parameters
-static bool ext_quic_transport_params_add_clienthello_impl(
- SSL_HANDSHAKE *hs, CBB *out, bool use_legacy_codepoint) {
+static bool ext_quic_transport_params_add_clienthello(SSL_HANDSHAKE *hs,
+ CBB *out) {
if (hs->config->quic_transport_params.empty() && !hs->ssl->quic_method) {
return true;
}
@@ -2763,18 +2763,9 @@
return false;
}
assert(hs->min_version > TLS1_2_VERSION);
- if (use_legacy_codepoint != hs->config->quic_use_legacy_codepoint) {
- // Do nothing, we'll send the other codepoint.
- return true;
- }
-
- uint16_t extension_type = TLSEXT_TYPE_quic_transport_parameters;
- if (hs->config->quic_use_legacy_codepoint) {
- extension_type = TLSEXT_TYPE_quic_transport_parameters_legacy;
- }
CBB contents;
- if (!CBB_add_u16(out, extension_type) ||
+ if (!CBB_add_u16(out, TLSEXT_TYPE_quic_transport_parameters) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
!CBB_add_bytes(&contents, hs->config->quic_transport_params.data(),
hs->config->quic_transport_params.size()) ||
@@ -2784,61 +2775,31 @@
return true;
}
-static bool ext_quic_transport_params_add_clienthello(SSL_HANDSHAKE *hs,
- CBB *out) {
- return ext_quic_transport_params_add_clienthello_impl(
- hs, out, /*use_legacy_codepoint=*/false);
-}
-
-static bool ext_quic_transport_params_add_clienthello_legacy(SSL_HANDSHAKE *hs,
- CBB *out) {
- return ext_quic_transport_params_add_clienthello_impl(
- hs, out, /*use_legacy_codepoint=*/true);
-}
-
-static bool ext_quic_transport_params_parse_serverhello_impl(
- SSL_HANDSHAKE *hs, uint8_t *out_alert, CBS *contents,
- bool used_legacy_codepoint) {
- SSL *const ssl = hs->ssl;
- if (contents == nullptr) {
- if (used_legacy_codepoint != hs->config->quic_use_legacy_codepoint) {
- // Silently ignore because we expect the other QUIC codepoint.
- return true;
- }
- if (!ssl->quic_method) {
- return true;
- }
- *out_alert = SSL_AD_MISSING_EXTENSION;
- return false;
- }
- // The extensions parser will check for unsolicited extensions before
- // calling the callback.
- assert(ssl->quic_method != nullptr);
- assert(ssl_protocol_version(ssl) == TLS1_3_VERSION);
- assert(used_legacy_codepoint == hs->config->quic_use_legacy_codepoint);
- return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
-}
-
static bool ext_quic_transport_params_parse_serverhello(SSL_HANDSHAKE *hs,
uint8_t *out_alert,
CBS *contents) {
- return ext_quic_transport_params_parse_serverhello_impl(
- hs, out_alert, contents, /*used_legacy_codepoint=*/false);
-}
-
-static bool ext_quic_transport_params_parse_serverhello_legacy(
- SSL_HANDSHAKE *hs, uint8_t *out_alert, CBS *contents) {
- return ext_quic_transport_params_parse_serverhello_impl(
- hs, out_alert, contents, /*used_legacy_codepoint=*/true);
-}
-
-static bool ext_quic_transport_params_parse_clienthello_impl(
- SSL_HANDSHAKE *hs, uint8_t *out_alert, CBS *contents,
- bool used_legacy_codepoint) {
- if (used_legacy_codepoint != hs->config->quic_use_legacy_codepoint) {
- // Silently ignore because we expect the other QUIC codepoint.
- return true;
+ SSL *const ssl = hs->ssl;
+ if (contents == nullptr) {
+ if (!ssl->quic_method) {
+ return true;
+ }
+ assert(ssl->quic_method);
+ *out_alert = SSL_AD_MISSING_EXTENSION;
+ return false;
}
+ if (!ssl->quic_method) {
+ *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+ return false;
+ }
+ // QUIC requires TLS 1.3.
+ assert(ssl_protocol_version(ssl) == TLS1_3_VERSION);
+
+ return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
+}
+
+static bool ext_quic_transport_params_parse_clienthello(SSL_HANDSHAKE *hs,
+ uint8_t *out_alert,
+ CBS *contents) {
SSL *const ssl = hs->ssl;
if (!contents) {
if (!ssl->quic_method) {
@@ -2861,39 +2822,17 @@
return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
}
-static bool ext_quic_transport_params_parse_clienthello(SSL_HANDSHAKE *hs,
- uint8_t *out_alert,
- CBS *contents) {
- return ext_quic_transport_params_parse_clienthello_impl(
- hs, out_alert, contents, /*used_legacy_codepoint=*/false);
-}
-
-static bool ext_quic_transport_params_parse_clienthello_legacy(
- SSL_HANDSHAKE *hs, uint8_t *out_alert, CBS *contents) {
- return ext_quic_transport_params_parse_clienthello_impl(
- hs, out_alert, contents, /*used_legacy_codepoint=*/true);
-}
-
-static bool ext_quic_transport_params_add_serverhello_impl(
- SSL_HANDSHAKE *hs, CBB *out, bool use_legacy_codepoint) {
+static bool ext_quic_transport_params_add_serverhello(SSL_HANDSHAKE *hs,
+ CBB *out) {
assert(hs->ssl->quic_method != nullptr);
if (hs->config->quic_transport_params.empty()) {
// Transport parameters must be set when using QUIC.
OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
return false;
}
- if (use_legacy_codepoint != hs->config->quic_use_legacy_codepoint) {
- // Do nothing, we'll send the other codepoint.
- return true;
- }
-
- uint16_t extension_type = TLSEXT_TYPE_quic_transport_parameters;
- if (hs->config->quic_use_legacy_codepoint) {
- extension_type = TLSEXT_TYPE_quic_transport_parameters_legacy;
- }
CBB contents;
- if (!CBB_add_u16(out, extension_type) ||
+ if (!CBB_add_u16(out, TLSEXT_TYPE_quic_transport_parameters) ||
!CBB_add_u16_length_prefixed(out, &contents) ||
!CBB_add_bytes(&contents, hs->config->quic_transport_params.data(),
hs->config->quic_transport_params.size()) ||
@@ -2904,18 +2843,6 @@
return true;
}
-static bool ext_quic_transport_params_add_serverhello(SSL_HANDSHAKE *hs,
- CBB *out) {
- return ext_quic_transport_params_add_serverhello_impl(
- hs, out, /*use_legacy_codepoint=*/false);
-}
-
-static bool ext_quic_transport_params_add_serverhello_legacy(SSL_HANDSHAKE *hs,
- CBB *out) {
- return ext_quic_transport_params_add_serverhello_impl(
- hs, out, /*use_legacy_codepoint=*/true);
-}
-
// Delegated credentials.
//
// https://tools.ietf.org/html/draft-ietf-tls-subcerts
@@ -3356,14 +3283,6 @@
ext_quic_transport_params_add_serverhello,
},
{
- TLSEXT_TYPE_quic_transport_parameters_legacy,
- NULL,
- ext_quic_transport_params_add_clienthello_legacy,
- ext_quic_transport_params_parse_serverhello_legacy,
- ext_quic_transport_params_parse_clienthello_legacy,
- ext_quic_transport_params_add_serverhello_legacy,
- },
- {
TLSEXT_TYPE_token_binding,
NULL,
ext_token_binding_add_clienthello,
