From 84694d2baa964abcd4f3d57a2a85a8369743476c Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Tue, 18 Feb 2025 09:01:40 -0500 Subject: [PATCH] Relax checking of supported-groups/keyshare ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit quic interop testing showed that interop with the mvfst client was failing, due to detecting mis ordering of supported groups and keyshare extensions This is strictly a mvfst problem to fix, but RFC 8446 indicates that we MAY check the ordering but don't strictly have to. We've opened an issue with the client to fix this, but in the interests of client compatibility relax the ordering check so that, instead of issuing a fatal alert, we just log a trace message indicating the discrepancy Fixes openssl/project#1106 Reviewed-by: Saša Nedvědický Reviewed-by: Viktor Dukhovni Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/26818) --- ssl/statem/extensions_srvr.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index d77b087ebf..5f6c22f401 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -685,13 +685,20 @@ static KS_EXTRACTION_RESULT extract_keyshares(SSL_CONNECTION *s, PACKET *key_sha /* * Check if this share is in supported_groups sent from client - * and that the key shares are in the same sequence as the supported_groups + * RFC 8446 also mandates that clients send keyshares in the same + * order as listed in the supported groups extension, but its not + * required that the server check that, and some clients violate this + * so instead of failing the connection when that occurs, log a trace + * message indicating the client discrepancy. */ - if (!check_in_list(s, group_id, clntgroups, clnt_num_groups, 0, &key_share_pos) - || key_share_pos < previous_key_share_pos) { + if (!check_in_list(s, group_id, clntgroups, clnt_num_groups, 0, &key_share_pos)) { SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_R_BAD_KEY_SHARE); goto failure; } + + if (key_share_pos < previous_key_share_pos) + OSSL_TRACE1(TLS, "key share group id %d is out of RFC 8446 order\n", group_id); + previous_key_share_pos = key_share_pos; if (s->s3.group_id != 0) {