diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata index a279ca6b1..661e52b03 100644 --- a/crypto/err/ssl.errordata +++ b/crypto/err/ssl.errordata @@ -190,6 +190,7 @@ SSL,1111,TLSV1_ALERT_CERTIFICATE_UNOBTAINABLE SSL,1050,TLSV1_ALERT_DECODE_ERROR SSL,1021,TLSV1_ALERT_DECRYPTION_FAILED SSL,1051,TLSV1_ALERT_DECRYPT_ERROR +SSL,1121,TLSV1_ALERT_ECH_REQUIRED SSL,1060,TLSV1_ALERT_EXPORT_RESTRICTION SSL,1086,TLSV1_ALERT_INAPPROPRIATE_FALLBACK SSL,1071,TLSV1_ALERT_INSUFFICIENT_SECURITY diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index 828938dc2..036db8be5 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -3631,6 +3631,9 @@ OPENSSL_EXPORT int SSL_ECH_SERVER_CONFIG_LIST_add( OPENSSL_EXPORT int SSL_CTX_set1_ech_server_config_list( SSL_CTX *ctx, SSL_ECH_SERVER_CONFIG_LIST *list); +// SSL_ech_accepted returns one if |ssl| negotiated ECH and zero otherwise. +OPENSSL_EXPORT int SSL_ech_accepted(const SSL *ssl); + // Alerts. // @@ -3685,6 +3688,7 @@ OPENSSL_EXPORT int SSL_CTX_set1_ech_server_config_list( #define SSL_AD_UNKNOWN_PSK_IDENTITY TLS1_AD_UNKNOWN_PSK_IDENTITY #define SSL_AD_CERTIFICATE_REQUIRED TLS1_AD_CERTIFICATE_REQUIRED #define SSL_AD_NO_APPLICATION_PROTOCOL TLS1_AD_NO_APPLICATION_PROTOCOL +#define SSL_AD_ECH_REQUIRED TLS1_AD_ECH_REQUIRED // SSL_alert_type_string_long returns a string description of |value| as an // alert type (warning or fatal). @@ -5475,5 +5479,6 @@ BSSL_NAMESPACE_END #define SSL_R_TLSV1_ALERT_UNKNOWN_PSK_IDENTITY 1115 #define SSL_R_TLSV1_ALERT_CERTIFICATE_REQUIRED 1116 #define SSL_R_TLSV1_ALERT_NO_APPLICATION_PROTOCOL 1120 +#define SSL_R_TLSV1_ALERT_ECH_REQUIRED 1121 #endif // OPENSSL_HEADER_SSL_H diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h index c41eba238..2886e2cf0 100644 --- a/include/openssl/tls1.h +++ b/include/openssl/tls1.h @@ -171,7 +171,6 @@ extern "C" { #define TLS1_AD_USER_CANCELLED 90 #define TLS1_AD_NO_RENEGOTIATION 100 #define TLS1_AD_MISSING_EXTENSION 109 -// codes 110-114 are from RFC3546 #define TLS1_AD_UNSUPPORTED_EXTENSION 110 #define TLS1_AD_CERTIFICATE_UNOBTAINABLE 111 #define TLS1_AD_UNRECOGNIZED_NAME 112 @@ -180,6 +179,7 @@ extern "C" { #define TLS1_AD_UNKNOWN_PSK_IDENTITY 115 #define TLS1_AD_CERTIFICATE_REQUIRED 116 #define TLS1_AD_NO_APPLICATION_PROTOCOL 120 +#define TLS1_AD_ECH_REQUIRED 121 // draft-ietf-tls-esni-10 // ExtensionType values from RFC6066 #define TLSEXT_TYPE_server_name 0 diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc index 336b9d989..48eb70743 100644 --- a/ssl/encrypted_client_hello.cc +++ b/ssl/encrypted_client_hello.cc @@ -558,3 +558,7 @@ int SSL_CTX_set1_ech_server_config_list(SSL_CTX *ctx, ctx->ech_server_config_list.swap(owned_list); return 1; } + +int SSL_ech_accepted(const SSL *ssl) { + return ssl->s3->ech_accept; +} diff --git a/ssl/handoff.cc b/ssl/handoff.cc index 5f836fadb..7186b00d9 100644 --- a/ssl/handoff.cc +++ b/ssl/handoff.cc @@ -232,7 +232,8 @@ static bool apply_remote_features(SSL *ssl, CBS *in) { // disqualifies it for split handshakes. static bool uses_disallowed_feature(const SSL *ssl) { return ssl->method->is_dtls || (ssl->config->cert && ssl->config->cert->dc) || - ssl->config->quic_transport_params.size() > 0; + ssl->config->quic_transport_params.size() > 0 || + ssl->ctx->ech_server_config_list; } bool SSL_apply_handoff(SSL *ssl, Span handoff) { diff --git a/ssl/handshake.cc b/ssl/handshake.cc index ebd5df005..f33547e7a 100644 --- a/ssl/handshake.cc +++ b/ssl/handshake.cc @@ -126,7 +126,6 @@ BSSL_NAMESPACE_BEGIN SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg) : ssl(ssl_arg), - ech_accept(false), ech_present(false), ech_is_inner_present(false), scts_requested(false), diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index 2d1cc38cf..c9a2048c3 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc @@ -661,14 +661,14 @@ static enum ssl_hs_wait_t do_read_client_hello(SSL_HANDSHAKE *hs) { } hs->ech_config_id = config_id; - hs->ech_accept = true; + ssl->s3->ech_accept = true; break; } } - // If we did not set |hs->ech_accept| to true, we will send the current - // ECHConfigs as retry_configs in the ServerHello's encrypted extensions. - // Proceed with the ClientHelloOuter. + // If we did not accept ECH, we will send the current ECHConfigs as + // retry_configs in the ServerHello's encrypted extensions. Proceed with the + // ClientHelloOuter. } uint8_t alert = SSL_AD_DECODE_ERROR; @@ -803,7 +803,7 @@ static enum ssl_hs_wait_t do_select_certificate(SSL_HANDSHAKE *hs) { // It should not be possible to negotiate TLS 1.2 with ECH. The // ClientHelloInner decoding function rejects ClientHellos which offer TLS 1.2 // or below. - assert(!hs->ech_accept); + assert(!ssl->s3->ech_accept); // TODO(davidben): Also compute hints for TLS 1.2. When doing so, update the // check in bssl_shim.cc to test this. diff --git a/ssl/internal.h b/ssl/internal.h index 1cb608356..9c904b7c5 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -1852,10 +1852,6 @@ struct SSL_HANDSHAKE { // influence the handshake on match. UniquePtr hints; - // ech_accept, on the server, indicates whether the server negotiated ECH and - // is handshaking with the inner ClientHello. - bool ech_accept : 1; - // ech_present, on the server, indicates whether the ClientHello contained an // encrypted_client_hello extension. bool ech_present : 1; @@ -2533,6 +2529,9 @@ struct SSL3_STATE { // key_update_count is the number of consecutive KeyUpdates received. uint8_t key_update_count = 0; + // ech_accept indicates whether ECH was accepted by the server. + bool ech_accept : 1; + // skip_early_data instructs the record layer to skip unexpected early data // messages when 0RTT is rejected. bool skip_early_data : 1; diff --git a/ssl/s3_lib.cc b/ssl/s3_lib.cc index fa73d34a7..454daf853 100644 --- a/ssl/s3_lib.cc +++ b/ssl/s3_lib.cc @@ -164,7 +164,8 @@ BSSL_NAMESPACE_BEGIN SSL3_STATE::SSL3_STATE() - : skip_early_data(false), + : ech_accept(false), + skip_early_data(false), have_version(false), v2_hello_done(false), is_v2_hello(false), diff --git a/ssl/ssl_stat.cc b/ssl/ssl_stat.cc index 5770aac7b..f7e1675f4 100644 --- a/ssl/ssl_stat.cc +++ b/ssl/ssl_stat.cc @@ -224,6 +224,9 @@ const char *SSL_alert_desc_string_long(int value) { case TLS1_AD_NO_APPLICATION_PROTOCOL: return "no application protocol"; + case TLS1_AD_ECH_REQUIRED: + return "ECH required"; + default: return "unknown"; } diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc index 3553276a5..7155f1251 100644 --- a/ssl/t1_lib.cc +++ b/ssl/t1_lib.cc @@ -734,7 +734,7 @@ static bool ext_ech_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert, static bool ext_ech_add_serverhello(SSL_HANDSHAKE *hs, CBB *out) { SSL *const ssl = hs->ssl; if (ssl_protocol_version(ssl) < TLS1_3_VERSION || // - hs->ech_accept || // + ssl->s3->ech_accept || // hs->ech_server_config_list == nullptr) { return true; } diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc index d4b562646..ef46540d1 100644 --- a/ssl/test/bssl_shim.cc +++ b/ssl/test/bssl_shim.cc @@ -660,6 +660,12 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume, return false; } + if (config->expect_ech_accept != !!SSL_ech_accepted(ssl)) { + fprintf(stderr, "ECH was %saccepted, but wanted opposite.\n", + SSL_ech_accepted(ssl) ? "" : "not "); + return false; + } + // Test that handshake hints correctly skipped the expected operations. // // TODO(davidben): Add support for TLS 1.2 hints and remove the version check. diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 2a861f36d..cc07d02f7 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go @@ -1853,6 +1853,7 @@ NextTest: if test.protocol != tls || test.testType != serverTest || strings.Contains(test.name, "DelegatedCredentials") || + strings.Contains(test.name, "ECH-Server") || test.skipSplitHandshake { continue } @@ -16281,6 +16282,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16308,6 +16310,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16426,6 +16429,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16456,7 +16460,6 @@ func addEncryptedClientHelloTests() { "-ech-server-config", base64.StdEncoding.EncodeToString(MarshalECHConfig(echConfig)), "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", - "-expect-server-name", "secret.example", }, shouldFail: true, expectedLocalError: "remote error: illegal parameter", @@ -16485,6 +16488,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, shouldFail: true, expectedLocalError: "remote error: illegal parameter", @@ -16513,7 +16517,6 @@ func addEncryptedClientHelloTests() { "-ech-server-config", base64.StdEncoding.EncodeToString(MarshalECHConfig(echConfig)), "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", - "-expect-server-name", "secret.example", }, shouldFail: true, expectedLocalError: "remote error: illegal parameter", @@ -16538,6 +16541,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16562,6 +16566,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key1), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16586,6 +16591,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(keyRepeatID), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16614,6 +16620,7 @@ func addEncryptedClientHelloTests() { "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", "-expect-server-name", "secret.example", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16778,6 +16785,7 @@ func addEncryptedClientHelloTests() { "-ech-server-config", base64.StdEncoding.EncodeToString(MarshalECHConfig(echConfig)), "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, @@ -16802,6 +16810,7 @@ func addEncryptedClientHelloTests() { "-ech-server-config", base64.StdEncoding.EncodeToString(MarshalECHConfig(echConfig)), "-ech-server-key", base64.StdEncoding.EncodeToString(key), "-ech-is-retry-config", "1", + "-expect-ech-accept", }, expectations: connectionExpectations{ echAccepted: true, diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc index c68df9f38..9fa8c0fdc 100644 --- a/ssl/test/test_config.cc +++ b/ssl/test/test_config.cc @@ -58,6 +58,7 @@ const Flag kBoolFlags[] = { {"-quic", &TestConfig::is_quic}, {"-fallback-scsv", &TestConfig::fallback_scsv}, {"-enable-ech-grease", &TestConfig::enable_ech_grease}, + {"-expect-ech-accept", &TestConfig::expect_ech_accept}, {"-require-any-client-certificate", &TestConfig::require_any_client_certificate}, {"-false-start", &TestConfig::false_start}, diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h index 4a65abf81..2478d492c 100644 --- a/ssl/test/test_config.h +++ b/ssl/test/test_config.h @@ -43,6 +43,7 @@ struct TestConfig { std::vector ech_server_configs; std::vector ech_server_keys; std::vector ech_is_retry_config; + bool expect_ech_accept = false; std::string expect_certificate_types; bool require_any_client_certificate = false; std::string advertise_npn; diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc index 2dc285674..c772010a8 100644 --- a/ssl/tls13_server.cc +++ b/ssl/tls13_server.cc @@ -606,7 +606,7 @@ static enum ssl_hs_wait_t do_read_second_client_hello(SSL_HANDSHAKE *hs) { return ssl_hs_error; } - if (hs->ech_accept) { + if (ssl->s3->ech_accept) { // If we previously accepted the ClientHelloInner, check that the second // ClientHello contains an encrypted_client_hello extension. CBS ech_body; @@ -769,8 +769,7 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) { } } - assert(!hs->ech_accept || hs->ech_is_inner_present); - + assert(!ssl->s3->ech_accept || hs->ech_is_inner_present); if (hs->ech_is_inner_present) { // Construct the ServerHelloECHConf message, which is the same as // ServerHello, except the last 8 bytes of its random field are zeroed out. diff --git a/tool/transport_common.cc b/tool/transport_common.cc index b985221de..cba3c7b9c 100644 --- a/tool/transport_common.cc +++ b/tool/transport_common.cc @@ -335,6 +335,9 @@ void PrintConnectionInfo(BIO *bio, const SSL *ssl) { bio, " Early data: %s\n", (SSL_early_data_accepted(ssl) || SSL_in_early_data(ssl)) ? "yes" : "no"); + BIO_printf(bio, " Encrypted ClientHello: %s\n", + SSL_ech_accepted(ssl) ? "yes" : "no"); + // Print the server cert subject and issuer names. bssl::UniquePtr peer(SSL_get_peer_certificate(ssl)); if (peer != nullptr) {