Add SSL_ech_accepted API and ech_is_required alerts.
The first thing any deployment will want to monitor is whether ECH was
actually used. Also it's useful if the command-line tool can output
this. (The alert is how the client signals it discarded the connection
due to ECH reject.)
This also disables ECH with the handoff mechanism for now. (The
immediate cause being that ech_accept isn't serialized.) We'll probably
need to make some decisions around the ordering here, since ECH affects
where the true ClientHello is available.
Bug: 275
Change-Id: Ie4559733290e653a514fcd94431090bf86bc3172
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47911
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index a279ca6..661e52b 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -190,6 +190,7 @@
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 828938d..036db8b 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3631,6 +3631,9 @@
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 @@
#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 @@
#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 c41eba2..2886e2c 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -171,7 +171,6 @@
#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 @@
#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 336b9d9..48eb707 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -558,3 +558,7 @@
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 5f836fa..7186b00 100644
--- a/ssl/handoff.cc
+++ b/ssl/handoff.cc
@@ -232,7 +232,8 @@
// 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<const uint8_t> handoff) {
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index ebd5df0..f33547e 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -126,7 +126,6 @@
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 2d1cc38..c9a2048 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -661,14 +661,14 @@
}
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 @@
// 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 1cb6083..9c904b7 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1852,10 +1852,6 @@
// influence the handshake on match.
UniquePtr<SSL_HANDSHAKE_HINTS> 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 @@
// 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 fa73d34..454daf8 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 5770aac..f7e1675 100644
--- a/ssl/ssl_stat.cc
+++ b/ssl/ssl_stat.cc
@@ -224,6 +224,9 @@
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 3553276..7155f12 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -734,7 +734,7 @@
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 d4b5626..ef46540 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -660,6 +660,12 @@
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 2a861f3..cc07d02 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -1853,6 +1853,7 @@
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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 @@
"-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 c68df9f..9fa8c0f 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -58,6 +58,7 @@
{"-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 4a65abf..2478d49 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -43,6 +43,7 @@
std::vector<std::string> ech_server_configs;
std::vector<std::string> ech_server_keys;
std::vector<int> 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 2dc2856..c772010 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -606,7 +606,7 @@
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 @@
}
}
- 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 b985221..cba3c7b 100644
--- a/tool/transport_common.cc
+++ b/tool/transport_common.cc
@@ -335,6 +335,9 @@
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<X509> peer(SSL_get_peer_certificate(ssl));
if (peer != nullptr) {
