Tidy up transitions out of 0-RTT keys on the client.
This change does two things. First, it funnels the transition out of
0-RTT into one function so that, later, when QUIC releases keys in
set_(read|write)_state, we can handle the QUIC quirks better.
Second, it switches to handshake (or initial) keys as soon as 0-RTT is
closed. In particular, if EncryptedExtensions reports a 0-RTT reject, we
switch keys before processing Certificate. This way, if we then reject
the server certificate, we send the alert with keys the server can read.
If there is an error in EncryptedExtensions or earlier, we do not know
whether the server is expecting 0-RTT-encrypted alerts or
handshake-encrypted alerts, so we cannot reliably send an alert. This is
fine because all such error cases are server implementation bugs and
alerts are purely a debugging courtesy. However, after a 0-RTT reject,
we may reject the Certificate message due to local policy, in which case
the certificate error alerts make more sense.
Bug: 303
Change-Id: I4c4bc9c8ab2c2ecb89e20141518e1b7ea7b39af3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/40125
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 4981fc4..27fc3af 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -670,9 +670,8 @@
case ssl_hs_early_data_rejected:
assert(ssl->s3->early_data_reason != ssl_early_data_unknown);
+ assert(!hs->can_early_write);
ssl->s3->rwstate = SSL_ERROR_EARLY_DATA_REJECTED;
- // Cause |SSL_write| to start failing immediately.
- hs->can_early_write = false;
return -1;
case ssl_hs_early_return:
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 255cd9e..6026c48 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5084,6 +5084,19 @@
suffix += "-CustomCallback"
}
+ // The custom callback and legacy callback have different default
+ // alerts.
+ verifyFailLocalError := "remote error: handshake failure"
+ if useCustomCallback {
+ verifyFailLocalError = "remote error: unknown certificate"
+ }
+
+ // We do not reliably send asynchronous fatal alerts. See
+ // https://crbug.com/boringssl/130.
+ if config.async {
+ verifyFailLocalError = ""
+ }
+
flags := []string{"-verify-peer"}
if testType == serverTest {
flags = append(flags, "-require-any-client-certificate")
@@ -5109,9 +5122,10 @@
MaxVersion: vers.version,
Certificates: []Certificate{rsaCertificate},
},
- flags: append([]string{"-verify-fail"}, flags...),
- shouldFail: true,
- expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ flags: append([]string{"-verify-fail"}, flags...),
+ shouldFail: true,
+ expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ expectedLocalError: verifyFailLocalError,
})
// Tests that although the verify callback fails on resumption, by default we don't call it.
tests = append(tests, testCase{
@@ -5136,9 +5150,10 @@
"-on-resume-verify-fail",
"-reverify-on-resume",
}, flags...),
- resumeSession: true,
- shouldFail: true,
- expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ resumeSession: true,
+ shouldFail: true,
+ expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ expectedLocalError: verifyFailLocalError,
})
tests = append(tests, testCase{
testType: testType,
@@ -5246,6 +5261,7 @@
expectResumeRejected: false,
shouldFail: true,
expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ expectedLocalError: verifyFailLocalError,
flags: append([]string{
"-enable-early-data",
"-expect-reject-early-data",
@@ -5296,6 +5312,8 @@
resumeSession: true,
shouldFail: true,
expectedError: ":CERTIFICATE_VERIFY_FAILED:",
+ // We do not set expectedLocalError here because the shim rejects
+ // the connection without an alert.
flags: append([]string{
"-enable-early-data",
"-expect-ticket-supports-early-data",
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 716c7b4..9418185 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -52,6 +52,50 @@
static const uint8_t kZeroes[EVP_MAX_MD_SIZE] = {0};
+// end_of_early_data closes the early data stream for |hs| and switches the
+// encryption level to |level|. It returns true on success and false on error.
+static bool close_early_data(SSL_HANDSHAKE *hs, ssl_encryption_level_t level) {
+ SSL *const ssl = hs->ssl;
+ assert(hs->in_early_data);
+
+ // Note |can_early_write| may already be false if |SSL_write| exceeded the
+ // early data write limit.
+ hs->can_early_write = false;
+
+ // 0-RTT write states on the client differ between TLS 1.3, DTLS 1.3, and
+ // QUIC. TLS 1.3 has one write encryption level at a time. 0-RTT write keys
+ // overwrite the null cipher and defer handshake write keys. While a
+ // HelloRetryRequest can cause us to rewind back to the null cipher, sequence
+ // numbers have no effect, so we can install a "new" null cipher.
+ //
+ // In QUIC and DTLS 1.3, 0-RTT write state cannot override or defer the normal
+ // write state. The two ClientHello sequence numbers must align, and handshake
+ // write keys must be installed early to ACK the EncryptedExtensions.
+ //
+ // We do not currently implement DTLS 1.3 and, in QUIC, the caller handles
+ // 0-RTT data, so we can skip installing 0-RTT keys and act as if there is one
+ // write level. If we implement DTLS 1.3, we'll need to model this better.
+ if (level == ssl_encryption_initial) {
+ bssl::UniquePtr<SSLAEADContext> null_ctx =
+ SSLAEADContext::CreateNullCipher(SSL_is_dtls(ssl));
+ if (!null_ctx || !ssl->method->set_write_state(ssl, ssl_encryption_initial,
+ std::move(null_ctx))) {
+ return false;
+ }
+ ssl->s3->aead_write_ctx->SetVersionIfNullCipher(ssl->version);
+ } else {
+ assert(level == ssl_encryption_handshake);
+ if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
+ hs->new_session.get(),
+ hs->client_handshake_secret())) {
+ return false;
+ }
+ }
+
+ assert(ssl->s3->write_level == level);
+ return true;
+}
+
static enum ssl_hs_wait_t do_read_hello_retry_request(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
assert(ssl->s3->have_version);
@@ -196,23 +240,17 @@
// 0-RTT is rejected if we receive a HelloRetryRequest.
if (hs->in_early_data) {
ssl->s3->early_data_reason = ssl_early_data_hello_retry_request;
+ if (!close_early_data(hs, ssl_encryption_initial)) {
+ return ssl_hs_error;
+ }
return ssl_hs_early_data_rejected;
}
return ssl_hs_ok;
}
static enum ssl_hs_wait_t do_send_second_client_hello(SSL_HANDSHAKE *hs) {
- SSL *const ssl = hs->ssl;
- // Restore the null cipher. We may have switched due to 0-RTT.
- bssl::UniquePtr<SSLAEADContext> null_ctx =
- SSLAEADContext::CreateNullCipher(SSL_is_dtls(ssl));
- if (!null_ctx ||
- !ssl->method->set_write_state(ssl, ssl_encryption_initial,
- std::move(null_ctx))) {
- return ssl_hs_error;
- }
-
- ssl->s3->aead_write_ctx->SetVersionIfNullCipher(ssl->version);
+ // Any 0-RTT keys must have been discarded.
+ assert(hs->ssl->s3->write_level == ssl_encryption_initial);
if (!ssl_write_client_hello(hs)) {
return ssl_hs_error;
@@ -406,9 +444,11 @@
return ssl_hs_error;
}
- if (!hs->early_data_offered) {
- // If not sending early data, set client traffic keys now so that alerts are
- // encrypted.
+ // If currently sending early data, we defer installing client traffic keys to
+ // when the early data stream is closed. See |close_early_data|. Note if the
+ // server has already rejected 0-RTT via HelloRetryRequest, |in_early_data| is
+ // already false.
+ if (!hs->in_early_data) {
if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
hs->new_session.get(),
hs->client_handshake_secret())) {
@@ -474,6 +514,9 @@
ssl->method->next_message(ssl);
hs->tls13_state = state_read_certificate_request;
if (hs->in_early_data && !ssl->s3->early_data_accepted) {
+ if (!close_early_data(hs, ssl_encryption_handshake)) {
+ return ssl_hs_error;
+ }
return ssl_hs_early_data_rejected;
}
return ssl_hs_ok;
@@ -654,7 +697,6 @@
SSL *const ssl = hs->ssl;
if (ssl->s3->early_data_accepted) {
- hs->can_early_write = false;
// QUIC omits the EndOfEarlyData message. See draft-ietf-quic-tls-22,
// section 8.3.
if (ssl->quic_method == nullptr) {
@@ -666,12 +708,8 @@
return ssl_hs_error;
}
}
- }
- if (hs->early_data_offered) {
- if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_seal,
- hs->new_session.get(),
- hs->client_handshake_secret())) {
+ if (!close_early_data(hs, ssl_encryption_handshake)) {
return ssl_hs_error;
}
}
