Check that a supported_versions extension is present in an HRR

If an HRR is sent then it MUST contain supported_versions according to the RFC. We were sanity checking any supported_versions extension that was sent but failed to verify that it was actually present. Fixes openssl#25041
mattcaswell · Aug 1, 2024 · 1165505 · 1165505
1 parent 7408d58
commit 1165505
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 1 deletion.
diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -1465,6 +1465,8 @@ SSL_R_MISSING_SIGALGS_EXTENSION:112:missing sigalgs extension
 SSL_R_MISSING_SIGNING_CERT:221:missing signing cert
 SSL_R_MISSING_SRP_PARAM:358:can't find SRP server param
 SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION:209:missing supported groups extension
+SSL_R_MISSING_SUPPORTED_VERSIONS_EXTENSION:420:\
+	missing supported versions extension
 SSL_R_MISSING_TMP_DH_KEY:171:missing tmp dh key
 SSL_R_MISSING_TMP_ECDH_KEY:311:missing tmp ecdh key
 SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA:293:\

diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
@@ -179,6 +179,7 @@
 # define SSL_R_MISSING_SIGNING_CERT                       221
 # define SSL_R_MISSING_SRP_PARAM                          358
 # define SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION         209
+# define SSL_R_MISSING_SUPPORTED_VERSIONS_EXTENSION       420
 # define SSL_R_MISSING_TMP_DH_KEY                         171
 # define SSL_R_MISSING_TMP_ECDH_KEY                       311
 # define SSL_R_MIXED_HANDSHAKE_AND_NON_HANDSHAKE_DATA     293

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
@@ -274,6 +274,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     "can't find SRP server param"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION),
     "missing supported groups extension"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_SUPPORTED_VERSIONS_EXTENSION),
+    "missing supported versions extension"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_TMP_DH_KEY), "missing tmp dh key"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_MISSING_TMP_ECDH_KEY),
     "missing tmp ecdh key"},

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
@@ -59,6 +59,8 @@ static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent);
 static int init_srtp(SSL_CONNECTION *s, unsigned int context);
 #endif
 static int final_sig_algs(SSL_CONNECTION *s, unsigned int context, int sent);
+static int final_supported_versions(SSL_CONNECTION *s, unsigned int context,
+                                    int sent);
 static int final_early_data(SSL_CONNECTION *s, unsigned int context, int sent);
 static int final_maxfragmentlen(SSL_CONNECTION *s, unsigned int context,
                                 int sent);
@@ -344,7 +346,7 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         /* Processed inline as part of version selection */
         NULL, tls_parse_stoc_supported_versions,
         tls_construct_stoc_supported_versions,
-        tls_construct_ctos_supported_versions, NULL
+        tls_construct_ctos_supported_versions, final_supported_versions
     },
     {
         TLSEXT_TYPE_psk_kex_modes,
@@ -1346,6 +1348,18 @@ static int final_sig_algs(SSL_CONNECTION *s, unsigned int context, int sent)
     return 1;
 }
 
+static int final_supported_versions(SSL_CONNECTION *s, unsigned int context,
+                                    int sent)
+{
+    if (!sent && context == SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) {
+        SSLfatal(s, TLS13_AD_MISSING_EXTENSION,
+                 SSL_R_MISSING_SUPPORTED_VERSIONS_EXTENSION);
+        return 0;
+    }
+
+    return 1;
+}
+
 static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent)
 {
 #if !defined(OPENSSL_NO_TLS1_3)