Commit b4bf79bd09 for openssl.org
commit b4bf79bd097ad9fb58417ca971cb32a45afbbcbe
Author: Viktor Dukhovni <viktor@openssl.org>
Date: Thu Jul 23 16:08:35 2026 +1000
Fix ECH server verify_result misuse
Reviewed-by: Mounir Idrassi <mounir.idrassi@idrix.fr>
Reviewed-by: Nikola Pajkovsky <nikolap@openssl.org>
Merge-date: Fri Oct 9 13:56:06 2026
Merged-from: https://github.com/openssl/openssl/pull/31925
diff --git a/doc/man3/SSL_set1_echstore.pod b/doc/man3/SSL_set1_echstore.pod
index a67c3e2acf..4d23435dda 100644
--- a/doc/man3/SSL_set1_echstore.pod
+++ b/doc/man3/SSL_set1_echstore.pod
@@ -375,7 +375,7 @@ SSL_ech_get1_status() returns one of the following values:
=item B<SSL_ECH_STATUS_NOT_TRIED> -101, ECH wasn't attempted
-=item B<SSL_ECH_STATUS_BAD_NAME> -102, ECH ok but server or client cert bad
+=item B<SSL_ECH_STATUS_BAD_NAME> -102, ECH ok but server cert bad
=item B<SSL_ECH_STATUS_NOT_CONFIGURED> -103, ECH wasn't configured
diff --git a/ssl/ech/ech_ssl_apis.c b/ssl/ech/ech_ssl_apis.c
index a9d3a24e73..09440df858 100644
--- a/ssl/ech/ech_ssl_apis.c
+++ b/ssl/ech/ech_ssl_apis.c
@@ -217,7 +217,14 @@ int SSL_ech_get1_status(SSL *ssl, char **inner_sni, char **outer_sni)
&& s->ext.ech.grease != OSSL_ECH_IS_GREASE) {
long vr = X509_V_OK;
- vr = SSL_get_verify_result(ssl);
+ /*
+ * The SUCCESS vs BAD_NAME distinction reflects whether the peer's
+ * certificate verified against the (inner) SNI -- a client-side verdict.
+ * On the server SSL_get_verify_result() is the client certificate's
+ * result, unrelated to ECH, so it is not consulted here.
+ */
+ if (s->server == 0)
+ vr = SSL_get_verify_result(ssl);
if (sinner != NULL
&& (*inner_sni = OPENSSL_strdup(sinner)) == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
diff --git a/test/ech_test.c b/test/ech_test.c
index d2331c8b40..a4de0c9a10 100644
--- a/test/ech_test.c
+++ b/test/ech_test.c
@@ -2341,6 +2341,75 @@ end:
return res;
}
+static int ech_admit_cb(int preverify_ok, X509_STORE_CTX *ctx)
+{
+ return 1;
+}
+
+/*
+ * Regression: on the server, SSL_ech_get1_status() must not derive its
+ * SUCCESS/BAD_NAME result from verify_result. On the server verify_result is
+ * the client certificate's verdict, which is unrelated to ECH. Here the server
+ * requests a client certificate, trusts no CA, and admits the failing chain
+ * through its verify callback (as a fingerprint- or ACL-based deployment does),
+ * so a successful handshake is left with a non-OK verify_result. ECH still
+ * succeeded, so the server status must be SSL_ECH_STATUS_SUCCESS, not
+ * SSL_ECH_STATUS_BAD_NAME.
+ */
+static int ech_unauth_client_status_test(void)
+{
+ int res = 0, serverstatus;
+ OSSL_ECHSTORE *es = NULL;
+ OSSL_HPKE_SUITE hpke_suite = OSSL_HPKE_SUITE_DEFAULT;
+ SSL_CTX *cctx = NULL, *sctx = NULL;
+ SSL *clientssl = NULL, *serverssl = NULL;
+ char *sinner = NULL, *souter = NULL;
+
+ if (!TEST_ptr(es = OSSL_ECHSTORE_new(libctx, propq))
+ || !TEST_true(OSSL_ECHSTORE_new_config(es, OSSL_ECH_CURRENT_VERSION, 0,
+ "example.com", hpke_suite))
+ || !TEST_true(create_ssl_ctx_pair(libctx, TLS_server_method(),
+ TLS_client_method(), TLS1_3_VERSION, TLS1_3_VERSION,
+ &sctx, &cctx, cert, privkey)))
+ goto end;
+ /*
+ * The server requests a client certificate but trusts no CA, admitting
+ * whatever the client sends via the callback -- so a successful handshake
+ * leaves verify_result non-OK.
+ */
+ SSL_CTX_set_verify(sctx, SSL_VERIFY_PEER, ech_admit_cb);
+ if (!TEST_true(SSL_CTX_use_certificate_file(cctx, cert, SSL_FILETYPE_PEM))
+ || !TEST_true(SSL_CTX_use_PrivateKey_file(cctx, privkey,
+ SSL_FILETYPE_PEM))
+ || !TEST_true(SSL_CTX_set1_echstore(cctx, es))
+ || !TEST_true(SSL_CTX_set1_echstore(sctx, es))
+ || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
+ NULL, NULL))
+ || !TEST_true(SSL_set_tlsext_host_name(clientssl, "server.example"))
+ || !TEST_true(create_ssl_connection(serverssl, clientssl,
+ SSL_ERROR_NONE)))
+ goto end;
+ /* The scenario is only meaningful if the client was admitted non-OK. */
+ if (!TEST_int_ne((int)SSL_get_verify_result(serverssl), X509_V_OK))
+ goto end;
+ serverstatus = SSL_ech_get1_status(serverssl, &sinner, &souter);
+ if (verbose)
+ TEST_info("server status %d (verify_result %ld)", serverstatus,
+ SSL_get_verify_result(serverssl));
+ if (!TEST_int_eq(serverstatus, SSL_ECH_STATUS_SUCCESS))
+ goto end;
+ res = 1;
+end:
+ OPENSSL_free(sinner);
+ OPENSSL_free(souter);
+ SSL_free(serverssl);
+ SSL_free(clientssl);
+ SSL_CTX_free(sctx);
+ SSL_CTX_free(cctx);
+ OSSL_ECHSTORE_free(es);
+ return res;
+}
+
#endif
int setup_tests(void)
@@ -2391,6 +2460,7 @@ int setup_tests(void)
ADD_ALL_TESTS(ech_grease_test, 4);
ADD_ALL_TESTS(test_ech_no_inner, suite_combos);
ADD_ALL_TESTS(test_ech_keylog_random, 4);
+ ADD_TEST(ech_unauth_client_status_test);
return 1;
err:
return 0;