ECH client support for sending multiple key shares

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

diff --git a/include/internal/ssl.h b/include/internal/ssl.h
index d56ed6e6dddfb266d3465c3810349000b0e3b566..c9e87d417bc5e120cd0777aa0aead471a2b2ae9e 100644 (file)
--- a/include/internal/ssl.h
+++ b/include/internal/ssl.h
@@ -23,4 +23,9 @@ int ossl_ssl_get_error(const SSL *s, int i, int check_err);
 /* Set if this is our QUIC handshake layer */
 #define TLS1_FLAGS_QUIC_INTERNAL 0x4000
 
+/* We limit the number of key shares sent */
+#ifndef OPENSSL_CLIENT_MAX_KEY_SHARES
+#define OPENSSL_CLIENT_MAX_KEY_SHARES 4
+#endif
+
 #endif

diff --git a/ssl/ech/ech_internal.c b/ssl/ech/ech_internal.c
index cac609862112ba91ba65ae2d0859cc182a09f6eb..2f43877a83b1d3d968ef529e53179dbd6ad77042 100644 (file)
--- a/ssl/ech/ech_internal.c
+++ b/ssl/ech/ech_internal.c
@@ -170,6 +170,20 @@ void ossl_ech_ctx_clear(OSSL_ECH_CTX *ce)
     return;
 }
 
+static void ech_free_stashed_key_shares(OSSL_ECH_CONN *ec)
+{
+    size_t i;
+
+    if (ec == NULL)
+        return;
+    for (i = 0; i != ec->num_ks_pkey; i++) {
+        EVP_PKEY_free(ec->ks_pkey[i]);
+        ec->ks_pkey[i] = NULL;
+    }
+    ec->num_ks_pkey = 0;
+    return;
+}
+
 void ossl_ech_conn_clear(OSSL_ECH_CONN *ec)
 {
     if (ec == NULL)
@@ -185,8 +199,8 @@ void ossl_ech_conn_clear(OSSL_ECH_CONN *ec)
     OPENSSL_free(ec->returned);
     OPENSSL_free(ec->pub);
     OSSL_HPKE_CTX_free(ec->hpke_ctx);
-    EVP_PKEY_free(ec->tmp_pkey);
     OPENSSL_free(ec->encoded_inner);
+    ech_free_stashed_key_shares(ec);
     return;
 }
 
@@ -826,15 +840,11 @@ int ossl_ech_swaperoo(SSL_CONNECTION *s)
 #ifdef OSSL_ECH_SUPERVERBOSE
     ossl_ech_ptranscript(s, "ech_swaperoo, b4");
 #endif
-    /* un-stash inner key share */
-    if (s->ext.ech.tmp_pkey == NULL) {
+    /* un-stash inner key share(s) */
+    if (ossl_ech_unstash_keyshares(s) != 1) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
     }
-    EVP_PKEY_free(s->s3.tmp.pkey);
-    s->s3.tmp.pkey = s->ext.ech.tmp_pkey;
-    s->s3.group_id = s->ext.ech.group_id;
-    s->ext.ech.tmp_pkey = NULL;
     /*
      * When not doing HRR... fix up the transcript to reflect the inner CH.
      * If there's a client hello at the start of the buffer, then that's
@@ -1165,4 +1175,38 @@ int ossl_ech_intbuf_fetch(SSL_CONNECTION *s, unsigned char **buf, size_t *blen)
     *blen = s->ext.ech.transbuf_len;
     return 1;
 }
+
+int ossl_ech_stash_keyshares(SSL_CONNECTION *s)
+{
+    size_t i;
+
+    ech_free_stashed_key_shares(&s->ext.ech);
+    for (i = 0; i != s->s3.tmp.num_ks_pkey; i++) {
+        s->ext.ech.ks_pkey[i] = s->s3.tmp.ks_pkey[i];
+        if (EVP_PKEY_up_ref(s->ext.ech.ks_pkey[i]) != 1)
+            return 0;
+        s->ext.ech.ks_group_id[i] = s->s3.tmp.ks_group_id[i];
+    }
+    s->ext.ech.num_ks_pkey = s->s3.tmp.num_ks_pkey;
+    return 1;
+}
+
+int ossl_ech_unstash_keyshares(SSL_CONNECTION *s)
+{
+    size_t i;
+
+    for (i = 0; i != s->s3.tmp.num_ks_pkey; i++) {
+        EVP_PKEY_free(s->s3.tmp.ks_pkey[i]);
+        s->s3.tmp.ks_pkey[i] = NULL;
+    }
+    for (i = 0; i != s->ext.ech.num_ks_pkey; i++) {
+        s->s3.tmp.ks_pkey[i] = s->ext.ech.ks_pkey[i];
+        if (EVP_PKEY_up_ref(s->s3.tmp.ks_pkey[i]) != 1)
+            return 0;
+        s->s3.tmp.ks_group_id[i] = s->ext.ech.ks_group_id[i];
+    }
+    s->s3.tmp.num_ks_pkey = s->ext.ech.num_ks_pkey;
+    ech_free_stashed_key_shares(&s->ext.ech);
+    return 1;
+}
 #endif

diff --git a/ssl/ech/ech_local.h b/ssl/ech/ech_local.h
index 553de8c9c33afdba47f3208bc836acc087bb96c7..426d42e3601ff739e423fdc4148a08c4af1e14e9 100644 (file)
--- a/ssl/ech/ech_local.h
+++ b/ssl/ech/ech_local.h
@@ -58,7 +58,6 @@
  * defined in a header file I could find.
  */
 #define CLIENT_VERSION_LEN 2
-
 #endif
 
 /*
@@ -219,8 +218,10 @@ typedef struct ossl_ech_conn_st {
      * keep and swap over IFF ECH has succeeded. Same names chosen as are
      * used in SSL_CONNECTION
      */
-    EVP_PKEY *tmp_pkey; /* client's key share for inner */
-    int group_id; /*  key share group */
+    EVP_PKEY *ks_pkey[OPENSSL_CLIENT_MAX_KEY_SHARES];
+    /* The IDs of the keyshare keys */
+    uint16_t ks_group_id[OPENSSL_CLIENT_MAX_KEY_SHARES];
+    size_t num_ks_pkey; /* how many keyshares are there */
     unsigned char client_random[SSL3_RANDOM_SIZE]; /* CH random */
 } OSSL_ECH_CONN;
 
@@ -306,6 +307,8 @@ int ossl_ech_intbuf_add(SSL_CONNECTION *s, const unsigned char *buf,
 int ossl_ech_intbuf_fetch(SSL_CONNECTION *s, unsigned char **buf, size_t *blen);
 size_t ossl_ech_calc_padding(SSL_CONNECTION *s, OSSL_ECHSTORE_ENTRY *ee,
     size_t encoded_len);
+int ossl_ech_stash_keyshares(SSL_CONNECTION *s);
+int ossl_ech_unstash_keyshares(SSL_CONNECTION *s);
 
 #endif
 #endif

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 00ceab915d07aba934d209ec509ac3de359290e6..daf195fd03d787f40a5fc6a506ef21df5a22c90e 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -795,11 +795,6 @@ typedef struct {
 
 #define TLS_GROUP_FFDHE_FOR_TLS1_3 (TLS_GROUP_FFDHE | TLS_GROUP_ONLY_FOR_TLS1_3)
 
-/* We limit the number of key shares sent */
-#ifndef OPENSSL_CLIENT_MAX_KEY_SHARES
-#define OPENSSL_CLIENT_MAX_KEY_SHARES 4
-#endif
-
 struct ssl_ctx_st {
     OSSL_LIB_CTX *libctx;
 

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 52901c3d8a0dff228568c7dfd7c434afa99a1ad0..3b03a7194898165439ec6ed0a1792772eb242c35 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -832,18 +832,6 @@ static int add_key_share(SSL_CONNECTION *s, WPACKET *pkt, unsigned int group_id,
         goto err;
     }
 
-#ifndef OPENSSL_NO_ECH
-    if (s->ext.ech.ch_depth == 1) { /* stash inner */
-        if (EVP_PKEY_up_ref(key_share_key) != 1) {
-            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
-            goto err;
-        }
-        EVP_PKEY_free(s->ext.ech.tmp_pkey);
-        s->ext.ech.tmp_pkey = key_share_key;
-        s->ext.ech.group_id = group_id;
-    }
-#endif
-
     /* For backward compatibility, we use the first valid group to add a key share */
     if (loop_num == 0) {
         s->s3.tmp.pkey = key_share_key;
@@ -856,7 +844,6 @@ static int add_key_share(SSL_CONNECTION *s, WPACKET *pkt, unsigned int group_id,
         s->s3.tmp.num_ks_pkey++;
 
     OPENSSL_free(encoded_pubkey);
-
     return 1;
 err:
     if (key_share_key != s->s3.tmp.ks_pkey[loop_num])
@@ -947,6 +934,14 @@ EXT_RETURN tls_construct_ctos_key_share(SSL_CONNECTION *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
+#ifndef OPENSSL_NO_ECH
+    /* stash inner key shares */
+    if (s->ext.ech.ch_depth == 1 && ossl_ech_stash_keyshares(s) != 1) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        return EXT_RETURN_FAIL;
+    }
+#endif
+
     if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;