HelloRetryRequest getter
Adds getter indicating whether HelloRetryRequest was triggered
during TLSv1.3 handshake.
Change-Id: I84922188ded81ec89259b5f333c80494426759f8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37304
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index be83edf..b0ee69a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3946,6 +3946,11 @@
// mechanism would have aborted |ssl|'s handshake and zero otherwise.
OPENSSL_EXPORT int SSL_is_tls13_downgrade(const SSL *ssl);
+// SSL_used_hello_retry_request returns one if the TLS 1.3 HelloRetryRequest
+// message has been either sent by the server or received by the client. It
+// returns zero otherwise.
+OPENSSL_EXPORT int SSL_used_hello_retry_request(const SSL *ssl);
+
// SSL_set_jdk11_workaround configures whether to workaround various bugs in
// JDK 11's TLS 1.3 implementation by disabling TLS 1.3 for such clients.
//
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 6791d2a..707c6d2 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -128,8 +128,6 @@
: ssl(ssl_arg),
scts_requested(false),
needs_psk_binder(false),
- received_hello_retry_request(false),
- sent_hello_retry_request(false),
handshake_finalized(false),
accept_psk_mode(false),
cert_request(false),
diff --git a/ssl/internal.h b/ssl/internal.h
index 7f163a4..5f81b22 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1693,9 +1693,6 @@
// be filled in.
bool needs_psk_binder : 1;
- bool received_hello_retry_request : 1;
- bool sent_hello_retry_request : 1;
-
// handshake_finalized is true once the handshake has completed, at which
// point accessors should use the established state.
bool handshake_finalized : 1;
@@ -2387,6 +2384,10 @@
// HelloRequest.
bool renegotiate_pending : 1;
+ // used_hello_retry_request is whether the handshake used a TLS 1.3
+ // HelloRetryRequest message.
+ bool used_hello_retry_request : 1;
+
// hs_buf is the buffer of handshake data to process.
UniquePtr<BUF_MEM> hs_buf;
diff --git a/ssl/s3_lib.cc b/ssl/s3_lib.cc
index f1f0ec7..6b0635b 100644
--- a/ssl/s3_lib.cc
+++ b/ssl/s3_lib.cc
@@ -181,7 +181,8 @@
token_binding_negotiated(false),
pq_experiment_signal_seen(false),
alert_dispatch(false),
- renegotiate_pending(false) {}
+ renegotiate_pending(false),
+ used_hello_retry_request(false) {}
SSL3_STATE::~SSL3_STATE() {}
diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index 1af8506..1daf03f 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -2859,6 +2859,10 @@
int SSL_is_tls13_downgrade(const SSL *ssl) { return ssl->s3->tls13_downgrade; }
+int SSL_used_hello_retry_request(const SSL *ssl) {
+ return ssl->s3->used_hello_retry_request;
+}
+
void SSL_CTX_set_ignore_tls13_downgrade(SSL_CTX *ctx, int ignore) {
ctx->ignore_tls13_downgrade = !!ignore;
}
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index e5a33dd..298dc9b 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1845,7 +1845,7 @@
// Per RFC 8446 section 4.1.4, skip offering the session if the selected
// cipher in HelloRetryRequest does not match. This avoids performing the
// transcript hash transformation for multiple hashes.
- if (hs->received_hello_retry_request &&
+ if (ssl->s3 && ssl->s3->used_hello_retry_request &&
ssl->session->cipher->algorithm_prf != hs->new_cipher->algorithm_prf) {
return true;
}
@@ -2035,7 +2035,7 @@
SSL *const ssl = hs->ssl;
// The second ClientHello never offers early data, and we must have already
// filled in |early_data_reason| by this point.
- if (hs->received_hello_retry_request) {
+ if (ssl->s3->used_hello_retry_request) {
assert(ssl->s3->early_data_reason != ssl_early_data_unknown);
return true;
}
@@ -2089,7 +2089,7 @@
CBS *contents) {
SSL *const ssl = hs->ssl;
if (contents == NULL) {
- if (hs->early_data_offered && !hs->received_hello_retry_request) {
+ if (hs->early_data_offered && !ssl->s3->used_hello_retry_request) {
ssl->s3->early_data_reason = ssl->s3->session_reused
? ssl_early_data_peer_declined
: ssl_early_data_session_not_resumed;
@@ -2104,7 +2104,7 @@
// If we received an HRR, the second ClientHello never offers early data, so
// the extensions logic will automatically reject early data extensions as
// unsolicited. This covered by the ServerAcceptsEarlyDataOnHRR test.
- assert(!hs->received_hello_retry_request);
+ assert(!ssl->s3->used_hello_retry_request);
if (CBS_len(contents) != 0) {
*out_alert = SSL_AD_DECODE_ERROR;
@@ -2173,7 +2173,7 @@
uint16_t group_id = hs->retry_group;
uint16_t second_group_id = 0;
- if (hs->received_hello_retry_request) {
+ if (ssl->s3 && ssl->s3->used_hello_retry_request) {
// We received a HelloRetryRequest without a new curve, so there is no new
// share to append. Leave |hs->key_share| as-is.
if (group_id == 0 &&
@@ -2235,7 +2235,7 @@
// Save the contents of the extension to repeat it in the second
// ClientHello.
- if (!hs->received_hello_retry_request &&
+ if (ssl->s3 && !ssl->s3->used_hello_retry_request &&
!hs->key_share_bytes.CopyFrom(
MakeConstSpan(CBB_data(&kse_bytes), CBB_len(&kse_bytes)))) {
return false;
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 61de136..5748fb9 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -676,6 +676,12 @@
return false;
}
+ if ((config->expect_hrr && !SSL_used_hello_retry_request(ssl)) ||
+ (config->expect_no_hrr && SSL_used_hello_retry_request(ssl))) {
+ fprintf(stderr, "Got %sHRR, but wanted opposite.\n",
+ SSL_used_hello_retry_request(ssl) ? "" : "no ");
+ return false;
+ }
return true;
}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index a4f787a..e78e9a2 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -145,12 +145,12 @@
type CurveID uint16
const (
- CurveP224 CurveID = 21
- CurveP256 CurveID = 23
- CurveP384 CurveID = 24
- CurveP521 CurveID = 25
- CurveX25519 CurveID = 29
- CurveCECPQ2 CurveID = 16696
+ CurveP224 CurveID = 21
+ CurveP256 CurveID = 23
+ CurveP384 CurveID = 24
+ CurveP521 CurveID = 25
+ CurveX25519 CurveID = 29
+ CurveCECPQ2 CurveID = 16696
)
// TLS Elliptic Curve Point Formats
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 18b01aa..660be0a 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -4383,6 +4383,7 @@
resumeSession: true,
// Ensure session tickets are used, not session IDs.
noSessionCache: true,
+ flags: []string{"-expect-no-hrr"},
})
tests = append(tests, testCase{
name: "Basic-Client-RenewTicket",
@@ -4414,7 +4415,10 @@
},
},
resumeSession: true,
- flags: []string{"-expect-no-session-id"},
+ flags: []string{
+ "-expect-no-session-id",
+ "-expect-no-hrr",
+ },
})
tests = append(tests, testCase{
testType: serverTest,
@@ -4482,6 +4486,7 @@
},
// Cover HelloRetryRequest during an ECDHE-PSK resumption.
resumeSession: true,
+ flags: []string{"-expect-hrr"},
})
tests = append(tests, testCase{
@@ -4495,6 +4500,7 @@
},
// Cover HelloRetryRequest during an ECDHE-PSK resumption.
resumeSession: true,
+ flags: []string{"-expect-hrr"},
})
tests = append(tests, testCase{
@@ -12538,6 +12544,7 @@
CurvePreferences: []CurveID{CurveX25519},
},
expectedCurveID: CurveX25519,
+ flags: []string{"-expect-hrr"},
})
testCases = append(testCases, testCase{
@@ -12551,6 +12558,7 @@
// Although the ClientHello did not predict our preferred curve,
// we always select it whether it is predicted or not.
expectedCurveID: CurveX25519,
+ flags: []string{"-expect-hrr"},
})
testCases = append(testCases, testCase{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 23de5e9..a4a37e6 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -153,6 +153,8 @@
&TestConfig::expect_delegated_credential_used},
{"-enable-pq-experiment-signal", &TestConfig::enable_pq_experiment_signal},
{"-expect-pq-experiment-signal", &TestConfig::expect_pq_experiment_signal},
+ {"-expect-hrr", &TestConfig::expect_hrr},
+ {"-expect-no-hrr", &TestConfig::expect_no_hrr},
};
const Flag<std::string> kStringFlags[] = {
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 8c25ed2..b94ca10 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -178,6 +178,8 @@
std::string expect_early_data_reason;
bool enable_pq_experiment_signal = false;
bool expect_pq_experiment_signal = false;
+ bool expect_hrr = false;
+ bool expect_no_hrr = false;
int argc;
char **argv;
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index 15c18c2..fa3f3a6 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -184,7 +184,7 @@
}
ssl->method->next_message(ssl);
- hs->received_hello_retry_request = true;
+ ssl->s3->used_hello_retry_request = true;
hs->tls13_state = state_send_second_client_hello;
// 0-RTT is rejected if we receive a HelloRetryRequest.
if (hs->in_early_data) {
@@ -269,8 +269,7 @@
}
// Check that the cipher matches the one in the HelloRetryRequest.
- if (hs->received_hello_retry_request &&
- hs->new_cipher != cipher) {
+ if (ssl->s3->used_hello_retry_request && hs->new_cipher != cipher) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index abacefc..d8115f5 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -515,7 +515,7 @@
return ssl_hs_error;
}
- hs->sent_hello_retry_request = true;
+ ssl->s3->used_hello_retry_request = true;
hs->tls13_state = state_read_second_client_hello;
return ssl_hs_flush;
}
@@ -612,7 +612,7 @@
return ssl_hs_error;
}
- if (!hs->sent_hello_retry_request &&
+ if (!ssl->s3->used_hello_retry_request &&
!ssl->method->add_change_cipher_spec(ssl)) {
return ssl_hs_error;
}
