Update the various SSL group getting and setting functions
authorMatt Caswell <matt@openssl.org>
Thu, 21 May 2020 15:36:32 +0000 (16:36 +0100)
committerMatt Caswell <matt@openssl.org>
Fri, 19 Jun 2020 09:19:31 +0000 (10:19 +0100)
A number of these functions returned a NID or an array of NIDs for the
groups. Now that groups can come from the providers we do not necessarily
know the NID. Therefore we need to handle this in a clean way.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11914)

doc/man3/SSL_CTX_set1_curves.pod
ssl/s3_lib.c
ssl/ssl_lib.c
ssl/ssl_local.h
ssl/t1_lib.c

index b482daa..3dd0c2a 100644 (file)
@@ -34,7 +34,11 @@ SSL_set1_curves, SSL_set1_curves_list, SSL_get1_curves, SSL_get_shared_curve
 =head1 DESCRIPTION
 
 For all of the functions below that set the supported groups there must be at
-least one group in the list.
+least one group in the list. A number of these functions identify groups via a
+unique integer NID value. However support for some groups may be added by
+external providers. In this case there will be no NID assigned for the group.
+When setting such groups applications should use the "list" form of these
+functions (i.e. SSL_CTX_set1_groups_list() and SSL_set1_groups_list).
 
 SSL_CTX_set1_groups() sets the supported groups for B<ctx> to B<glistlen>
 groups in the array B<glist>. The array consist of all NIDs of groups in
@@ -49,7 +53,8 @@ SSL_CTX_set1_groups_list() sets the supported groups for B<ctx> to
 string B<list>. The string is a colon separated list of group NIDs or
 names, for example "P-521:P-384:P-256:X25519:ffdhe2048". Currently supported
 groups for B<TLSv1.3> are B<P-256>, B<P-384>, B<P-521>, B<X25519>, B<X448>,
-B<ffdhe2048>, B<ffdhe3072>, B<ffdhe4096>, B<ffdhe6144>, B<ffdhe8192>.
+B<ffdhe2048>, B<ffdhe3072>, B<ffdhe4096>, B<ffdhe6144>, B<ffdhe8192>. Support
+for other groups may be added by external providers.
 
 SSL_set1_groups() and SSL_set1_groups_list() are similar except they set
 supported groups for the SSL structure B<ssl>.
@@ -60,17 +65,22 @@ supported groups. The B<groups> parameter can be B<NULL> to simply
 return the number of groups for memory allocation purposes. The
 B<groups> array is in the form of a set of group NIDs in preference
 order. It can return zero if the client did not send a supported groups
-extension.
+extension. If a supported group NID is unknown then the value is set to the
+bitwise OR of TLSEXT_nid_unknown (0x1000000) and the id of the group.
 
-SSL_get_shared_group() returns shared group B<n> for a server-side
-SSL B<ssl>. If B<n> is -1 then the total number of shared groups is
+SSL_get_shared_group() returns the NID of the shared group B<n> for a
+server-side SSL B<ssl>. If B<n> is -1 then the total number of shared groups is
 returned, which may be zero. Other than for diagnostic purposes,
 most applications will only be interested in the first shared group
 so B<n> is normally set to zero. If the value B<n> is out of range,
-NID_undef is returned.
+NID_undef is returned. If the NID for the shared group is unknown then the value
+is set to the bitwise OR of TLSEXT_nid_unknown (0x1000000) and the id of the
+group.
 
-SSL_get_negotiated_group() returns the negotiated group on a TLSv1.3 connection
-for key exchange. This can be called by either client or server.
+SSL_get_negotiated_group() returns the NID of the negotiated group on a TLSv1.3
+connection for key exchange. This can be called by either client or server. If
+the NID for the shared group is unknown then the value is set to the bitwise OR
+of TLSEXT_nid_unknown (0x1000000) and the id of the group.
 
 All these functions are implemented as macros.
 
index 8004c64..a7f1e4d 100644 (file)
@@ -3653,13 +3653,10 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
                     const TLS_GROUP_INFO *cinf
                         = tls1_group_id_lookup(s->ctx, clist[i]);
 
-                    if (cinf != NULL)  {
-                        cptr[i] = tls1_group_id2nid(cinf->group_id);
-                        if (cptr[i] == NID_undef)
-                            cptr[i] = TLSEXT_nid_unknown | clist[i];
-                    } else {
+                    if (cinf != NULL)
+                        cptr[i] = tls1_group_id2nid(cinf->group_id, 1);
+                    else
                         cptr[i] = TLSEXT_nid_unknown | clist[i];
-                    }
                 }
             }
             return (int)clistlen;
@@ -3670,7 +3667,7 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
                                &s->ext.supportedgroups_len, parg, larg);
 
     case SSL_CTRL_SET_GROUPS_LIST:
-        return tls1_set_groups_list(&s->ext.supportedgroups,
+        return tls1_set_groups_list(s->ctx, &s->ext.supportedgroups,
                                     &s->ext.supportedgroups_len, parg);
 
     case SSL_CTRL_GET_SHARED_GROUP:
@@ -3678,11 +3675,11 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
             uint16_t id = tls1_shared_group(s, larg);
 
             if (larg != -1)
-                return tls1_group_id2nid(id);
+                return tls1_group_id2nid(id, 1);
             return id;
         }
     case SSL_CTRL_GET_NEGOTIATED_GROUP:
-        ret = tls1_group_id2nid(s->s3.group_id);
+        ret = tls1_group_id2nid(s->s3.group_id, 1);
         break;
 #endif /* !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) */
 
@@ -3967,7 +3964,7 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
                                parg, larg);
 
     case SSL_CTRL_SET_GROUPS_LIST:
-        return tls1_set_groups_list(&ctx->ext.supportedgroups,
+        return tls1_set_groups_list(ctx, &ctx->ext.supportedgroups,
                                     &ctx->ext.supportedgroups_len,
                                     parg);
 #endif /* !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) */
index 0473433..cee8889 100644 (file)
@@ -2429,10 +2429,8 @@ long SSL_CTX_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
     /* For some cases with ctx == NULL perform syntax checks */
     if (ctx == NULL) {
         switch (cmd) {
-#ifndef OPENSSL_NO_EC
         case SSL_CTRL_SET_GROUPS_LIST:
-            return tls1_set_groups_list(NULL, NULL, parg);
-#endif
+            return tls1_set_groups_list(ctx, NULL, NULL, parg);
         case SSL_CTRL_SET_SIGALGS_LIST:
         case SSL_CTRL_SET_CLIENT_SIGALGS_LIST:
             return tls1_set_sigalgs_list(NULL, parg, 0);
index 017e8f9..58bc1f9 100644 (file)
@@ -2636,12 +2636,12 @@ __owur int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s);
 SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n);
 
 __owur const TLS_GROUP_INFO *tls1_group_id_lookup(SSL_CTX *ctx, uint16_t curve_id);
-__owur int tls1_group_id2nid(uint16_t group_id);
+__owur int tls1_group_id2nid(uint16_t group_id, int include_unknown);
 __owur int tls1_check_group_id(SSL *s, uint16_t group_id, int check_own_curves);
 __owur uint16_t tls1_shared_group(SSL *s, int nmatch);
 __owur int tls1_set_groups(uint16_t **pext, size_t *pextlen,
                            int *curves, size_t ncurves);
-__owur int tls1_set_groups_list(uint16_t **pext, size_t *pextlen,
+__owur int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
                                 const char *str);
 __owur EVP_PKEY *ssl_generate_pkey_group(SSL *s, uint16_t id);
 __owur int tls_valid_group(SSL *s, uint16_t group_id, int minversion,
index 7f8fc5e..d99384e 100644 (file)
@@ -400,6 +400,31 @@ int ssl_load_groups(SSL_CTX *ctx)
     return OSSL_PROVIDER_do_all(ctx->libctx, discover_provider_groups, ctx);
 }
 
+static uint16_t tls1_group_name2id(SSL_CTX *ctx, const char *name)
+{
+    size_t i;
+    int nid = NID_undef;
+
+    /* See if we can identify a nid for this name */
+#ifndef OPENSSL_NO_EC
+    nid = EC_curve_nist2nid(name);
+#endif
+    if (nid == NID_undef)
+        nid = OBJ_sn2nid(name);
+    if (nid == NID_undef)
+        nid = OBJ_ln2nid(name);
+
+    for (i = 0; i < ctx->group_list_len; i++) {
+        if (strcmp(ctx->group_list[i].tlsname, name) == 0
+                || (nid != NID_undef
+                    && nid == tls1_group_id2nid(ctx->group_list[i].group_id,
+                                                0)))
+            return ctx->group_list[i].group_id;
+    }
+
+    return 0;
+}
+
 const TLS_GROUP_INFO *tls1_group_id_lookup(SSL_CTX *ctx, uint16_t group_id)
 {
     size_t i;
@@ -413,10 +438,13 @@ const TLS_GROUP_INFO *tls1_group_id_lookup(SSL_CTX *ctx, uint16_t group_id)
 }
 
 #if !defined(OPENSSL_NO_DH) || !defined(OPENSSL_NO_EC)
-int tls1_group_id2nid(uint16_t group_id)
+int tls1_group_id2nid(uint16_t group_id, int include_unknown)
 {
     size_t i;
 
+    if (group_id == 0)
+        return NID_undef;
+
     /*
      * Return well known Group NIDs - for backwards compatibility. This won't
      * work for groups we don't know about.
@@ -426,7 +454,9 @@ int tls1_group_id2nid(uint16_t group_id)
         if (nid_to_group[i].group_id == group_id)
             return nid_to_group[i].nid;
     }
-    return NID_undef;
+    if (!include_unknown)
+        return NID_undef;
+    return TLSEXT_nid_unknown | (int)group_id;
 }
 
 static uint16_t tls1_nid2group_id(int nid)
@@ -533,7 +563,7 @@ int tls_group_allowed(SSL *s, uint16_t group, int op)
     gtmp[0] = group >> 8;
     gtmp[1] = group & 0xff;
     return ssl_security(s, op, ginfo->secbits,
-                        tls1_group_id2nid(ginfo->group_id), (void *)gtmp);
+                        tls1_group_id2nid(ginfo->group_id, 0), (void *)gtmp);
 }
 
 /* Return 1 if "id" is in "list" */
@@ -655,60 +685,65 @@ err:
 #endif /* !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) */
 }
 
-#if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH)
 /* TODO(3.0): An arbitrary amount for now. Take another look at this */
 # define MAX_GROUPLIST   40
 
 typedef struct {
-    size_t nidcnt;
-    int nid_arr[MAX_GROUPLIST];
-} nid_cb_st;
+    SSL_CTX *ctx;
+    size_t gidcnt;
+    uint16_t gid_arr[MAX_GROUPLIST];
+} gid_cb_st;
 
-static int nid_cb(const char *elem, int len, void *arg)
+static int gid_cb(const char *elem, int len, void *arg)
 {
-    nid_cb_st *narg = arg;
+    gid_cb_st *garg = arg;
     size_t i;
-    int nid = NID_undef;
+    uint16_t gid = 0;
     char etmp[20];
+
     if (elem == NULL)
         return 0;
-    if (narg->nidcnt == MAX_GROUPLIST)
+    if (garg->gidcnt == MAX_GROUPLIST)
         return 0;
     if (len > (int)(sizeof(etmp) - 1))
         return 0;
     memcpy(etmp, elem, len);
     etmp[len] = 0;
-# ifndef OPENSSL_NO_EC
-    nid = EC_curve_nist2nid(etmp);
-# endif
-    if (nid == NID_undef)
-        nid = OBJ_sn2nid(etmp);
-    if (nid == NID_undef)
-        nid = OBJ_ln2nid(etmp);
-    if (nid == NID_undef)
+
+    gid = tls1_group_name2id(garg->ctx, etmp);
+    if (gid == 0)
         return 0;
-    for (i = 0; i < narg->nidcnt; i++)
-        if (narg->nid_arr[i] == nid)
+    for (i = 0; i < garg->gidcnt; i++)
+        if (garg->gid_arr[i] == gid)
             return 0;
-    narg->nid_arr[narg->nidcnt++] = nid;
+    garg->gid_arr[garg->gidcnt++] = gid;
     return 1;
 }
-#endif /* !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH) */
 
-/* Set groups based on a colon separate list */
-int tls1_set_groups_list(uint16_t **pext, size_t *pextlen, const char *str)
+/* Set groups based on a colon separated list */
+int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
+                         const char *str)
 {
-#if !defined(OPENSSL_NO_EC) || !defined(OPENSSL_NO_DH)
-    nid_cb_st ncb;
-    ncb.nidcnt = 0;
-    if (!CONF_parse_list(str, ':', 1, nid_cb, &ncb))
+    gid_cb_st gcb;
+    uint16_t *tmparr;
+
+    gcb.gidcnt = 0;
+    gcb.ctx = ctx;
+    if (!CONF_parse_list(str, ':', 1, gid_cb, &gcb))
         return 0;
     if (pext == NULL)
         return 1;
-    return tls1_set_groups(pext, pextlen, ncb.nid_arr, ncb.nidcnt);
-#else
-    return 0;
-#endif
+
+    /*
+     * gid_cb ensurse there are no duplicates so we can just go ahead and set
+     * the result
+     */
+    tmparr = OPENSSL_memdup(gcb.gid_arr, gcb.gidcnt * sizeof(*tmparr));
+    if (tmparr == NULL)
+        return 0;
+    *pext = tmparr;
+    *pextlen = gcb.gidcnt;
+    return 1;
 }
 
 /* Check a group id matches preferences */