From: Frederik Wedel-Heinen <frederik.wedel-heinen@dencrypt.dk>
Date: Thu, 12 Oct 2023 12:35:37 +0000 (+0200)
Subject: Support TLS 1.3 kexs and groups with DTLS 1.3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=33c96b2dc632b51026a970337c4737d7611fecc1;p=thirdparty%2Fopenssl.git

Support TLS 1.3 kexs and groups with DTLS 1.3

SSL_CONNECTION_IS_VERSION13 macro is used where appropriate.

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/22364)
---

diff --git a/providers/common/capabilities.c b/providers/common/capabilities.c
index 550eca1af74..2a8fca4693f 100644
--- a/providers/common/capabilities.c
+++ b/providers/common/capabilities.c
@@ -86,15 +86,15 @@ static const TLS_GROUP_CONSTANTS group_list[] = {
       DTLS1_VERSION, DTLS1_2_VERSION },
     { OSSL_TLS_GROUP_ID_x25519, 128, TLS1_VERSION, 0, DTLS1_VERSION, 0 },
     { OSSL_TLS_GROUP_ID_x448, 224, TLS1_VERSION, 0, DTLS1_VERSION, 0 },
-    { OSSL_TLS_GROUP_ID_brainpoolP256r1_tls13, 128, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_brainpoolP384r1_tls13, 192, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_brainpoolP512r1_tls13, 256, TLS1_3_VERSION, 0, -1, -1 },
+    { OSSL_TLS_GROUP_ID_brainpoolP256r1_tls13, 128, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_brainpoolP384r1_tls13, 192, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_brainpoolP512r1_tls13, 256, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
     /* Security bit values as given by BN_security_bits() */
-    { OSSL_TLS_GROUP_ID_ffdhe2048, 112, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_ffdhe3072, 128, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_ffdhe4096, 128, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_ffdhe6144, 128, TLS1_3_VERSION, 0, -1, -1 },
-    { OSSL_TLS_GROUP_ID_ffdhe8192, 192, TLS1_3_VERSION, 0, -1, -1 },
+    { OSSL_TLS_GROUP_ID_ffdhe2048, 112, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_ffdhe3072, 128, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_ffdhe4096, 128, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_ffdhe6144, 128, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
+    { OSSL_TLS_GROUP_ID_ffdhe8192, 192, TLS1_3_VERSION, 0, DTLS1_3_VERSION, 0 },
 };
 
 #define TLS_GROUP_ENTRY(tlsname, realname, algorithm, idx) \
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index b98464256e6..a10d350f922 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -46,7 +46,7 @@ static SSL_CIPHER tls13_ciphers[] = {
         SSL_AES128GCM,
         SSL_AEAD,
         TLS1_3_VERSION, TLS1_3_VERSION,
-        0, 0,
+        DTLS1_3_VERSION, DTLS1_3_VERSION,
         SSL_HIGH,
         SSL_HANDSHAKE_MAC_SHA256 | SSL_QUIC,
         128,
@@ -61,7 +61,7 @@ static SSL_CIPHER tls13_ciphers[] = {
         SSL_AES256GCM,
         SSL_AEAD,
         TLS1_3_VERSION, TLS1_3_VERSION,
-        0, 0,
+        DTLS1_3_VERSION, DTLS1_3_VERSION,
         SSL_HIGH,
         SSL_HANDSHAKE_MAC_SHA384 | SSL_QUIC,
         256,
@@ -77,7 +77,7 @@ static SSL_CIPHER tls13_ciphers[] = {
         SSL_CHACHA20POLY1305,
         SSL_AEAD,
         TLS1_3_VERSION, TLS1_3_VERSION,
-        0, 0,
+        DTLS1_3_VERSION, DTLS1_3_VERSION,
         SSL_HIGH,
         SSL_HANDSHAKE_MAC_SHA256 | SSL_QUIC,
         256,
@@ -93,7 +93,7 @@ static SSL_CIPHER tls13_ciphers[] = {
         SSL_AES128CCM,
         SSL_AEAD,
         TLS1_3_VERSION, TLS1_3_VERSION,
-        0, 0,
+        DTLS1_3_VERSION, DTLS1_3_VERSION,
         SSL_NOT_DEFAULT | SSL_HIGH,
         SSL_HANDSHAKE_MAC_SHA256,
         128,
@@ -108,7 +108,7 @@ static SSL_CIPHER tls13_ciphers[] = {
         SSL_AES128CCM8,
         SSL_AEAD,
         TLS1_3_VERSION, TLS1_3_VERSION,
-        0, 0,
+        DTLS1_3_VERSION, DTLS1_3_VERSION,
         SSL_NOT_DEFAULT | SSL_MEDIUM,
         SSL_HANDSHAKE_MAC_SHA256,
         64, /* CCM8 uses a short tag, so we have a low security strength */
@@ -3731,7 +3731,7 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
         {
             unsigned int id;
 
-            if (SSL_CONNECTION_IS_TLS13(sc) && sc->s3.did_kex)
+            if (SSL_CONNECTION_IS_VERSION13(sc) && sc->s3.did_kex)
                 id = sc->s3.group_id;
             else
                 id = sc->session->kex_group;
@@ -4319,7 +4319,7 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL_CONNECTION *s, STACK_OF(SSL_CIPHER) *cl
         allow = srvr;
     }
 
-    if (SSL_CONNECTION_IS_TLS13(s)) {
+    if (SSL_CONNECTION_IS_VERSION13(s)) {
 #ifndef OPENSSL_NO_PSK
         size_t j;
 
@@ -4359,7 +4359,7 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL_CONNECTION *s, STACK_OF(SSL_CIPHER) *cl
          * Since TLS 1.3 ciphersuites can be used with any auth or
          * key exchange scheme skip tests.
          */
-        if (!SSL_CONNECTION_IS_TLS13(s)) {
+        if (!SSL_CONNECTION_IS_VERSION13(s)) {
             mask_k = s->s3.tmp.mask_k;
             mask_a = s->s3.tmp.mask_a;
 #ifndef OPENSSL_NO_SRP
@@ -4902,7 +4902,7 @@ int ssl_gensecret(SSL_CONNECTION *s, unsigned char *pms, size_t pmslen)
     int rv = 0;
 
     /* SSLfatal() called as appropriate in the below functions */
-    if (SSL_CONNECTION_IS_TLS13(s)) {
+    if (SSL_CONNECTION_IS_VERSION13(s)) {
         /*
          * If we are resuming then we already generated the early secret
          * when we created the ClientHello, so don't recreate it.
@@ -4945,7 +4945,7 @@ int ssl_derive(SSL_CONNECTION *s, EVP_PKEY *privkey, EVP_PKEY *pubkey, int gense
         goto err;
     }
 
-    if (SSL_CONNECTION_IS_TLS13(s) &&  EVP_PKEY_is_a(privkey, "DH"))
+    if (SSL_CONNECTION_IS_VERSION13(s) &&  EVP_PKEY_is_a(privkey, "DH"))
         EVP_PKEY_CTX_set_dh_pad(pctx, 1);
 
     pms = OPENSSL_malloc(pmslen);
@@ -5097,7 +5097,7 @@ const char *SSL_get0_group_name(SSL *s)
     if (sc == NULL)
         return NULL;
 
-    if (SSL_CONNECTION_IS_TLS13(sc) && sc->s3.did_kex)
+    if (SSL_CONNECTION_IS_VERSION13(sc) && sc->s3.did_kex)
         id = sc->s3.group_id;
     else
         id = sc->session->kex_group;
diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 375308c5f77..4d5ea66974b 100644
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -564,7 +564,7 @@ int extension_is_relevant(SSL_CONNECTION *s, unsigned int extctx,
     if ((thisctx & SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST) != 0)
         is_version13 = 1;
     else
-        is_version13 = SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s);
+        is_version13 = SSL_CONNECTION_IS_VERSION13(s);
 
     if ((SSL_CONNECTION_IS_DTLS(s)
                 && (extctx & SSL_EXT_TLS_IMPLEMENTATION_ONLY) != 0)
@@ -1073,7 +1073,7 @@ static int final_server_name(SSL_CONNECTION *s, unsigned int context, int sent)
 
     case SSL_TLSEXT_ERR_ALERT_WARNING:
         /* (D)TLSv1.3 doesn't have warning alerts so we suppress this */
-        if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
+        if (!SSL_CONNECTION_IS_VERSION13(s))
             ssl3_send_alert(s, SSL3_AL_WARNING, altmp);
         s->servername_done = 0;
         return 1;
@@ -1180,7 +1180,7 @@ static int final_alpn(SSL_CONNECTION *s, unsigned int context, int sent)
     if (!s->server && !sent && s->session->ext.alpn_selected != NULL)
             s->ext.early_data_ok = 0;
 
-    if (!s->server || !(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
+    if (!s->server || !SSL_CONNECTION_IS_VERSION13(s))
         return 1;
 
     /*
@@ -1340,7 +1340,7 @@ static int init_srtp(SSL_CONNECTION *s, unsigned int context)
 
 static int final_sig_algs(SSL_CONNECTION *s, unsigned int context, int sent)
 {
-    if (!sent && (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && !s->hit) {
+    if (!sent && SSL_CONNECTION_IS_VERSION13(s) && !s->hit) {
         SSLfatal(s, TLS13_AD_MISSING_EXTENSION,
                  SSL_R_MISSING_SIGALGS_EXTENSION);
         return 0;
@@ -1364,7 +1364,7 @@ static int final_supported_versions(SSL_CONNECTION *s, unsigned int context,
 static int final_key_share(SSL_CONNECTION *s, unsigned int context, int sent)
 {
 #if !defined(OPENSSL_NO_TLS1_3)
-    if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
+    if (!SSL_CONNECTION_IS_VERSION13(s))
         return 1;
 
     /* Nothing to do for key_share in an HRR */
diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 2d3486ad34f..5b0144187d9 100644
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -1487,12 +1487,12 @@ int tls_parse_stoc_status_request(SSL_CONNECTION *s, PACKET *pkt,
         SSLfatal(s, SSL_AD_UNSUPPORTED_EXTENSION, SSL_R_BAD_EXTENSION);
         return 0;
     }
-    if (!(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && PACKET_remaining(pkt) > 0) {
+    if (!SSL_CONNECTION_IS_VERSION13(s) && PACKET_remaining(pkt) > 0) {
         SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_R_BAD_EXTENSION);
         return 0;
     }
 
-    if (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) {
+    if (SSL_CONNECTION_IS_VERSION13(s)) {
         /* We only know how to handle this if it's for the first Certificate in
          * the chain. We ignore any other responses.
          */
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index fa3b8fdfdf2..f90e5843645 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -136,7 +136,7 @@ int tls_parse_ctos_server_name(SSL_CONNECTION *s, PACKET *pkt,
      * In (D)TLSv1.2 and below the SNI is associated with the session. In (D)TLSv1.3
      * we always use the SNI value from the handshake.
      */
-    if (!s->hit || (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))) {
+    if (!s->hit || SSL_CONNECTION_IS_VERSION13(s)) {
         if (PACKET_remaining(&hostname) > TLSEXT_MAXLEN_host_name) {
             SSLfatal(s, SSL_AD_UNRECOGNIZED_NAME, SSL_R_BAD_EXTENSION);
             return 0;
@@ -947,7 +947,7 @@ int tls_parse_ctos_supported_groups(SSL_CONNECTION *s, PACKET *pkt,
         return 0;
     }
 
-    if (!s->hit || (SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))) {
+    if (!s->hit || SSL_CONNECTION_IS_VERSION13(s)) {
         OPENSSL_free(s->ext.peer_supportedgroups);
         s->ext.peer_supportedgroups = NULL;
         s->ext.peer_supportedgroups_len = 0;
@@ -1324,7 +1324,7 @@ EXT_RETURN tls_construct_stoc_server_name(SSL_CONNECTION *s, WPACKET *pkt,
      * Prior to (D)TLSv1.3 we ignore any SNI in the current handshake if resuming.
      * We just use the servername from the initial handshake.
      */
-    if (s->hit && !(SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))
+    if (s->hit && !SSL_CONNECTION_IS_VERSION13(s))
         return EXT_RETURN_NOT_SENT;
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_server_name)
@@ -1475,7 +1475,7 @@ EXT_RETURN tls_construct_stoc_status_request(SSL_CONNECTION *s, WPACKET *pkt,
     if (!s->ext.status_expected)
         return EXT_RETURN_NOT_SENT;
 
-    if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)) && chainidx != 0)
+    if (SSL_CONNECTION_IS_VERSION13(s) && chainidx != 0)
         return EXT_RETURN_NOT_SENT;
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_status_request)
@@ -1489,7 +1489,7 @@ EXT_RETURN tls_construct_stoc_status_request(SSL_CONNECTION *s, WPACKET *pkt,
      * send back an empty extension, with the certificate status appearing as a
      * separate message
      */
-    if ((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s))
+    if (SSL_CONNECTION_IS_VERSION13(s)
             && !tls_construct_cert_status_body(s, pkt)) {
        /* SSLfatal() already called */
        return EXT_RETURN_FAIL;
@@ -1627,7 +1627,7 @@ EXT_RETURN tls_construct_stoc_supported_versions(SSL_CONNECTION *s, WPACKET *pkt
                                                  unsigned int context, X509 *x,
                                                  size_t chainidx)
 {
-    if (!ossl_assert((SSL_CONNECTION_IS_TLS13(s) || SSL_CONNECTION_IS_DTLS13(s)))) {
+    if (!ossl_assert(SSL_CONNECTION_IS_VERSION13(s))) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 982f3275085..ff45863a587 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -870,11 +870,10 @@ int tls_valid_group(SSL_CONNECTION *s, uint16_t group_id,
     if (group_minversion > 0)
         ret &= (ssl_version_cmp(s, maxversion, group_minversion) >= 0);
 
-    if (!SSL_CONNECTION_IS_DTLS(s)) {
-        if (ret && okfortls13 != NULL && maxversion == TLS1_3_VERSION)
-            *okfortls13 = (group_maxversion == 0)
-                          || (group_maxversion >= TLS1_3_VERSION);
-    }
+    if (ret && okfortls13 != NULL && (maxversion == DTLS1_3_VERSION
+                                      || maxversion == TLS1_3_VERSION))
+        *okfortls13 = (group_maxversion == 0)
+                      || (ssl_version_cmp(s, group_maxversion, maxversion) >= 0);
     ret &= !isec
            || strcmp(ginfo->algorithm, "EC") == 0
            || strcmp(ginfo->algorithm, "X25519") == 0
@@ -1276,7 +1275,7 @@ static int tls1_check_pkey_comp(SSL_CONNECTION *s, EVP_PKEY *pkey)
         return 0;
     if (point_conv == POINT_CONVERSION_UNCOMPRESSED) {
             comp_id = TLSEXT_ECPOINTFORMAT_uncompressed;
-    } else if (SSL_CONNECTION_IS_TLS13(s)) {
+    } else if (SSL_CONNECTION_IS_VERSION13(s)) {
         /*
          * ec_point_formats extension is not used in TLSv1.3 so we ignore
          * this check.