Require QUIC method with Transport Parameters and vice versa
Bug: 296, 322
Change-Id: I297f53674ee7177f61d75695f47b5caeed78bd17
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40384
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 27ad65c..ab6b15f 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -133,6 +133,7 @@
SSL,196,PSK_NO_CLIENT_CB
SSL,197,PSK_NO_SERVER_CB
SSL,298,QUIC_INTERNAL_ERROR
+SSL,305,QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED
SSL,198,READ_TIMEOUT_EXPIRED
SSL,199,RECORD_LENGTH_MISMATCH
SSL,200,RECORD_TOO_LARGE
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 23489dd..66243e2 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3067,38 +3067,6 @@
OPENSSL_EXPORT const char *SSL_get_psk_identity(const SSL *ssl);
-// QUIC transport parameters.
-//
-// draft-ietf-quic-tls defines a new TLS extension quic_transport_parameters
-// used by QUIC for each endpoint to unilaterally declare its supported
-// transport parameters. draft-ietf-quic-transport (section 7.4) defines the
-// contents of that extension (a TransportParameters struct) and describes how
-// to handle it and its semantic meaning.
-//
-// BoringSSL handles this extension as an opaque byte string. The caller is
-// responsible for serializing and parsing it.
-
-// SSL_set_quic_transport_params configures |ssl| to send |params| (of length
-// |params_len|) in the quic_transport_parameters extension in either the
-// ClientHello or EncryptedExtensions handshake message. This extension will
-// only be sent if the TLS version is at least 1.3, and for a server, only if
-// the client sent the extension. The buffer pointed to by |params| only need be
-// valid for the duration of the call to this function. This function returns 1
-// on success and 0 on failure.
-OPENSSL_EXPORT int SSL_set_quic_transport_params(SSL *ssl,
- const uint8_t *params,
- size_t params_len);
-
-// SSL_get_peer_quic_transport_params provides the caller with the value of the
-// quic_transport_parameters extension sent by the peer. A pointer to the buffer
-// containing the TransportParameters will be put in |*out_params|, and its
-// length in |*params_len|. This buffer will be valid for the lifetime of the
-// |SSL|. If no params were received from the peer, |*out_params_len| will be 0.
-OPENSSL_EXPORT void SSL_get_peer_quic_transport_params(const SSL *ssl,
- const uint8_t **out_params,
- size_t *out_params_len);
-
-
// Delegated credentials.
//
// *** EXPERIMENTAL — PRONE TO CHANGE ***
@@ -3172,6 +3140,12 @@
//
// Note: 0-RTT support is incomplete and does not currently handle QUIC
// transport parameters and server SETTINGS frame.
+//
+// QUIC implementations must additionally configure transport parameters with
+// |SSL_set_quic_transport_params|. |SSL_get_peer_quic_transport_params| may be
+// used to query the value received from the peer. BoringSSL handles this
+// extension as an opaque byte string. The caller is responsible for serializing
+// and parsing them. See draft-ietf-quic-transport (section 7.3) for details.
// ssl_encryption_level_t represents a specific QUIC encryption level used to
// transmit handshake messages.
@@ -3300,6 +3274,24 @@
OPENSSL_EXPORT int SSL_set_quic_method(SSL *ssl,
const SSL_QUIC_METHOD *quic_method);
+// SSL_set_quic_transport_params configures |ssl| to send |params| (of length
+// |params_len|) in the quic_transport_parameters extension in either the
+// ClientHello or EncryptedExtensions handshake message. It is an error to set
+// transport parameters if |ssl| is not configured for QUIC. The buffer pointed
+// to by |params| only need be valid for the duration of the call to this
+// function. This function returns 1 on success and 0 on failure.
+OPENSSL_EXPORT int SSL_set_quic_transport_params(SSL *ssl,
+ const uint8_t *params,
+ size_t params_len);
+
+// SSL_get_peer_quic_transport_params provides the caller with the value of the
+// quic_transport_parameters extension sent by the peer. A pointer to the buffer
+// containing the TransportParameters will be put in |*out_params|, and its
+// length in |*params_len|. This buffer will be valid for the lifetime of the
+// |SSL|. If no params were received from the peer, |*out_params_len| will be 0.
+OPENSSL_EXPORT void SSL_get_peer_quic_transport_params(
+ const SSL *ssl, const uint8_t **out_params, size_t *out_params_len);
+
// Early data.
//
@@ -5117,6 +5109,7 @@
#define SSL_R_KEY_USAGE_BIT_INCORRECT 302
#define SSL_R_INCONSISTENT_CLIENT_HELLO 303
#define SSL_R_CIPHER_MISMATCH_ON_EARLY_DATA 304
+#define SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED 305
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 3f7bbfc..96d67bc 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -5106,13 +5106,34 @@
transport_->client()->AllowOutOfOrderWrites();
transport_->server()->AllowOutOfOrderWrites();
}
+ static const uint8_t transport_params[] = {0};
+ if (!SSL_set_quic_transport_params(client_.get(), transport_params,
+ sizeof(transport_params)) ||
+ !SSL_set_quic_transport_params(server_.get(), transport_params,
+ sizeof(transport_params))) {
+ return false;
+ }
return true;
}
+ enum class ExpectedError {
+ kNoError,
+ kClientError,
+ kServerError,
+ };
+
// CompleteHandshakesForQUIC runs |SSL_do_handshake| on |client_| and
// |server_| until each completes once. It returns true on success and false
// on failure.
bool CompleteHandshakesForQUIC() {
+ return RunQUICHandshakesAndExpectError(ExpectedError::kNoError);
+ }
+
+ // Runs |SSL_do_handshake| on |client_| and |server_| until each completes
+ // once. If |expect_client_error| is true, it will return true only if the
+ // client handshake failed. Otherwise, it returns true if both handshakes
+ // succeed and false otherwise.
+ bool RunQUICHandshakesAndExpectError(ExpectedError expected_error) {
bool client_done = false, server_done = false;
while (!client_done || !server_done) {
if (!client_done) {
@@ -5125,6 +5146,9 @@
if (client_ret == 1) {
client_done = true;
} else if (client_ret != -1 || client_err != SSL_ERROR_WANT_READ) {
+ if (expected_error == ExpectedError::kClientError) {
+ return true;
+ }
ADD_FAILURE() << "Unexpected client output: " << client_ret << " "
<< client_err;
return false;
@@ -5141,13 +5165,16 @@
if (server_ret == 1) {
server_done = true;
} else if (server_ret != -1 || server_err != SSL_ERROR_WANT_READ) {
+ if (expected_error == ExpectedError::kServerError) {
+ return true;
+ }
ADD_FAILURE() << "Unexpected server output: " << server_ret << " "
<< server_err;
return false;
}
}
}
- return true;
+ return expected_error == ExpectedError::kNoError;
}
bssl::UniquePtr<SSL_SESSION> CreateClientSessionForQUIC() {
@@ -5866,6 +5893,26 @@
EXPECT_FALSE(SSL_session_reused(server.get()));
}
+TEST_F(QUICMethodTest, ClientRejectsMissingTransportParams) {
+ 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());
+ ASSERT_TRUE(SSL_set_quic_transport_params(server_.get(), nullptr, 0));
+ ASSERT_TRUE(RunQUICHandshakesAndExpectError(ExpectedError::kServerError));
+}
+
+TEST_F(QUICMethodTest, ServerRejectsMissingTransportParams) {
+ 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());
+ ASSERT_TRUE(SSL_set_quic_transport_params(client_.get(), nullptr, 0));
+ ASSERT_TRUE(RunQUICHandshakesAndExpectError(ExpectedError::kClientError));
+}
+
extern "C" {
int BORINGSSL_enum_c_type_test(void);
}
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index d04c874..dad0fcf 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2547,10 +2547,17 @@
static bool ext_quic_transport_params_add_clienthello(SSL_HANDSHAKE *hs,
CBB *out) {
- if (hs->config->quic_transport_params.empty() ||
- hs->max_version <= TLS1_2_VERSION) {
+ if (hs->config->quic_transport_params.empty() && !hs->ssl->quic_method) {
return true;
}
+ if (hs->config->quic_transport_params.empty() || !hs->ssl->quic_method) {
+ // QUIC Transport Parameters must be sent over QUIC, and they must not be
+ // sent over non-QUIC transports. If transport params are set, then
+ // SSL(_CTX)_set_quic_method must also be called.
+ OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+ return false;
+ }
+ assert(hs->min_version > TLS1_2_VERSION);
CBB contents;
if (!CBB_add_u16(out, TLSEXT_TYPE_quic_transport_parameters) ||
@@ -2568,13 +2575,19 @@
CBS *contents) {
SSL *const ssl = hs->ssl;
if (contents == nullptr) {
- return true;
+ if (!ssl->quic_method) {
+ return true;
+ }
+ assert(ssl->quic_method);
+ *out_alert = SSL_AD_MISSING_EXTENSION;
+ return false;
}
- // QUIC requires TLS 1.3.
- if (ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+ 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);
}
@@ -2583,8 +2596,22 @@
uint8_t *out_alert,
CBS *contents) {
SSL *const ssl = hs->ssl;
- if (!contents || !ssl->quic_method) {
- return true;
+ if (!contents) {
+ if (!ssl->quic_method) {
+ if (hs->config->quic_transport_params.empty()) {
+ return true;
+ }
+ // QUIC transport parameters must not be set if |ssl| is not configured
+ // for QUIC.
+ OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ }
+ *out_alert = SSL_AD_MISSING_EXTENSION;
+ return false;
+ }
+ if (!ssl->quic_method) {
+ *out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
+ return false;
}
assert(ssl_protocol_version(ssl) == TLS1_3_VERSION);
return ssl->s3->peer_quic_transport_params.CopyFrom(*contents);
@@ -2592,8 +2619,11 @@
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()) {
- return true;
+ // Transport parameters must be set when using QUIC.
+ OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED);
+ return false;
}
CBB contents;
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 762caff..6a00a64 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -636,6 +636,9 @@
// exportTrafficSecrets, if true, configures the test to export the TLS 1.3
// traffic secrets and confirms that they match.
exportTrafficSecrets bool
+ // skipTransportParamsConfig, if true, will skip automatic configuration of
+ // sending QUIC transport parameters when protocol == quic.
+ skipTransportParamsConfig bool
}
var testCases []testCase
@@ -1244,6 +1247,20 @@
flags = append(flags, "-dtls")
} else if test.protocol == quic {
flags = append(flags, "-quic")
+ if !test.skipTransportParamsConfig {
+ test.config.QUICTransportParams = []byte{1, 2}
+ if test.resumeConfig != nil {
+ test.resumeConfig.QUICTransportParams = []byte{1, 2}
+ }
+ test.expectedQUICTransportParams = []byte{3, 4}
+ flags = append(flags,
+ []string{
+ "-quic-transport-params",
+ base64.StdEncoding.EncodeToString([]byte{3, 4}),
+ "-expect-quic-transport-params",
+ base64.StdEncoding.EncodeToString([]byte{1, 2}),
+ }...)
+ }
}
var resumeCount int
@@ -7160,6 +7177,23 @@
base64.StdEncoding.EncodeToString([]byte{1, 2}),
},
expectedQUICTransportParams: []byte{3, 4},
+ skipTransportParamsConfig: true,
+ })
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: quic,
+ name: "QUICTransportParams-Client-RejectMissing-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-quic-transport-params",
+ base64.StdEncoding.EncodeToString([]byte{3, 4}),
+ },
+ shouldFail: true,
+ expectedError: ":MISSING_EXTENSION:",
+ skipTransportParamsConfig: true,
})
// Server sends params
testCases = append(testCases, testCase{
@@ -7178,65 +7212,59 @@
base64.StdEncoding.EncodeToString([]byte{1, 2}),
},
expectedQUICTransportParams: []byte{3, 4},
+ skipTransportParamsConfig: true,
})
- } else {
testCases = append(testCases, testCase{
- testType: clientTest,
- name: "QUICTransportParams-Client-NotSent-" + ver.name,
+ testType: serverTest,
+ protocol: quic,
+ name: "QUICTransportParams-Server-RejectMissing-" + ver.name,
config: Config{
MinVersion: ver.version,
MaxVersion: ver.version,
},
flags: []string{
- "-max-version",
- strconv.Itoa(int(ver.version)),
"-quic-transport-params",
base64.StdEncoding.EncodeToString([]byte{3, 4}),
},
- })
- testCases = append(testCases, testCase{
- testType: clientTest,
- name: "QUICTransportParams-Client-Rejected-" + ver.name,
- config: Config{
- MinVersion: ver.version,
- MaxVersion: ver.version,
- QUICTransportParams: []byte{1, 2},
- },
- flags: []string{
- "-quic-transport-params",
- base64.StdEncoding.EncodeToString([]byte{3, 4}),
- },
- shouldFail: true,
- expectedError: ":ERROR_PARSING_EXTENSION:",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "QUICTransportParams-Server-Rejected-" + ver.name,
- config: Config{
- MinVersion: ver.version,
- MaxVersion: ver.version,
- QUICTransportParams: []byte{1, 2},
- },
- flags: []string{
- "-expect-quic-transport-params",
- base64.StdEncoding.EncodeToString([]byte{1, 2}),
- },
- shouldFail: true,
- expectedError: "QUIC transport params mismatch",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "QUICTransportParams-OldServerIgnores-" + ver.name,
- config: Config{
- MaxVersion: VersionTLS13,
- QUICTransportParams: []byte{1, 2},
- },
- flags: []string{
- "-min-version", ver.shimFlag(tls),
- "-max-version", ver.shimFlag(tls),
- },
+ expectedQUICTransportParams: []byte{3, 4},
+ shouldFail: true,
+ expectedError: ":MISSING_EXTENSION:",
+ skipTransportParamsConfig: true,
})
}
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ name: "QUICTransportParams-Client-NotSentInTLS-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ },
+ flags: []string{
+ "-max-version",
+ strconv.Itoa(int(ver.version)),
+ "-quic-transport-params",
+ base64.StdEncoding.EncodeToString([]byte{3, 4}),
+ },
+ shouldFail: true,
+ expectedError: ":QUIC_TRANSPORT_PARAMETERS_MISCONFIGURED:",
+ skipTransportParamsConfig: true,
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "QUICTransportParams-Server-RejectedInTLS-" + ver.name,
+ config: Config{
+ MinVersion: ver.version,
+ MaxVersion: ver.version,
+ QUICTransportParams: []byte{1, 2},
+ },
+ flags: []string{
+ "-expect-quic-transport-params",
+ base64.StdEncoding.EncodeToString([]byte{1, 2}),
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: unsupported extension",
+ skipTransportParamsConfig: true,
+ })
// Test ticket behavior.
