Fix SSL_check_chain()
authorMatt Caswell <matt@openssl.org>
Tue, 23 Jul 2019 14:14:29 +0000 (15:14 +0100)
committerMatt Caswell <matt@openssl.org>
Fri, 9 Aug 2019 16:29:39 +0000 (17:29 +0100)
The function SSL_check_chain() can be used by applications to check that
a cert and chain is compatible with the negotiated parameters. This could
be useful (for example) from the certificate callback. Unfortunately this
function was applying TLSv1.2 sig algs rules and did not work correctly if
TLSv1.3 was negotiated.

We refactor tls_choose_sigalg to split it up and create a new function
find_sig_alg which can (optionally) take a certificate and key as
parameters and find an appropriate sig alg if one exists. If the cert and
key are not supplied then we try to find a cert and key from the ones we
have available that matches the shared sig algs.

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9442)

ssl/t1_lib.c

index 2470270..ab0a0e0 100644 (file)
@@ -21,6 +21,8 @@
 #include "ssl_locl.h"
 #include <openssl/ct.h>
 
+static const SIGALG_LOOKUP *find_sig_alg(SSL *s, X509 *x, EVP_PKEY *pkey);
+
 SSL3_ENC_METHOD const TLSv1_enc_data = {
     tls1_enc,
     tls1_mac,
@@ -2151,16 +2153,34 @@ int tls1_set_sigalgs(CERT *c, const int *psig_nids, size_t salglen, int client)
 
 static int tls1_check_sig_alg(SSL *s, X509 *x, int default_nid)
 {
-    int sig_nid;
+    int sig_nid, use_pc_sigalgs = 0;
     size_t i;
+    const SIGALG_LOOKUP *sigalg;
+    size_t sigalgslen;
     if (default_nid == -1)
         return 1;
     sig_nid = X509_get_signature_nid(x);
     if (default_nid)
         return sig_nid == default_nid ? 1 : 0;
-    for (i = 0; i < s->shared_sigalgslen; i++)
-        if (sig_nid == s->shared_sigalgs[i]->sigandhash)
+
+    if (SSL_IS_TLS13(s) && s->s3.tmp.peer_cert_sigalgs != NULL) {
+        /*
+         * If we're in TLSv1.3 then we only get here if we're checking the
+         * chain. If the peer has specified peer_cert_sigalgs then we use them
+         * otherwise we default to normal sigalgs.
+         */
+        sigalgslen = s->s3.tmp.peer_cert_sigalgslen;
+        use_pc_sigalgs = 1;
+    } else {
+        sigalgslen = s->shared_sigalgslen;
+    }
+    for (i = 0; i < sigalgslen; i++) {
+        sigalg = use_pc_sigalgs
+                 ? tls1_lookup_sigalg(s->s3.tmp.peer_cert_sigalgs[i])
+                 : s->shared_sigalgs[i];
+        if (sig_nid == sigalg->sigandhash)
             return 1;
+    }
     return 0;
 }
 
@@ -2317,7 +2337,14 @@ int tls1_check_chain(SSL *s, X509 *x, EVP_PKEY *pk, STACK_OF(X509) *chain,
             }
         }
         /* Check signature algorithm of each cert in chain */
-        if (!tls1_check_sig_alg(s, x, default_nid)) {
+        if (SSL_IS_TLS13(s)) {
+            /*
+             * We only get here if the application has called SSL_check_chain(),
+             * so check_flags is always set.
+             */
+            if (find_sig_alg(s, x, pk) != NULL)
+                rv |= CERT_PKEY_EE_SIGNATURE;
+        } else if (!tls1_check_sig_alg(s, x, default_nid)) {
             if (!check_flags)
                 goto end;
         } else
@@ -2605,29 +2632,23 @@ static int tls12_get_cert_sigalg_idx(const SSL *s, const SIGALG_LOOKUP *lu)
 }
 
 /*
- * Returns true if |s| has a usable certificate configured for use
- * with signature scheme |sig|.
- * "Usable" includes a check for presence as well as applying
- * the signature_algorithm_cert restrictions sent by the peer (if any).
- * Returns false if no usable certificate is found.
+ * Checks the given cert against signature_algorithm_cert restrictions sent by
+ * the peer (if any) as well as whether the hash from the sigalg is usable with
+ * the key.
+ * Returns true if the cert is usable and false otherwise.
  */
-static int has_usable_cert(SSL *s, const SIGALG_LOOKUP *sig, int idx)
+static int check_cert_usable(SSL *s, const SIGALG_LOOKUP *sig, X509 *x,
+                             EVP_PKEY *pkey)
 {
     const SIGALG_LOOKUP *lu;
     int mdnid, pknid, supported;
     size_t i;
 
-    /* TLS 1.2 callers can override lu->sig_idx, but not TLS 1.3 callers. */
-    if (idx == -1)
-        idx = sig->sig_idx;
-    if (!ssl_has_cert(s, idx))
-        return 0;
     if (s->s3.tmp.peer_cert_sigalgs != NULL) {
         for (i = 0; i < s->s3.tmp.peer_cert_sigalgslen; i++) {
             lu = tls1_lookup_sigalg(s->s3.tmp.peer_cert_sigalgs[i]);
             if (lu == NULL
-                || !X509_get_signature_info(s->cert->pkeys[idx].x509, &mdnid,
-                                            &pknid, NULL, NULL)
+                || !X509_get_signature_info(x, &mdnid, &pknid, NULL, NULL)
                 /*
                  * TODO this does not differentiate between the
                  * rsa_pss_pss_* and rsa_pss_rsae_* schemes since we do not
@@ -2639,30 +2660,130 @@ static int has_usable_cert(SSL *s, const SIGALG_LOOKUP *sig, int idx)
                 continue;
 
             ERR_set_mark();
-            supported = EVP_PKEY_supports_digest_nid(s->cert->pkeys[idx].privatekey,
-                                                     mdnid);
+            supported = EVP_PKEY_supports_digest_nid(pkey, mdnid);
+            ERR_pop_to_mark();
             if (supported == 0)
                 continue;
-            else if (supported < 0)
-            {
-                /* If it didn't report a mandatory NID, for whatever reasons,
-                 * just clear the error and allow all hashes to be used. */
-                ERR_pop_to_mark();
-            }
+            /*
+             * If it didn't report a mandatory NID (supported < 0), for
+             * whatever reasons, we just ignore the error and allow all
+             * hashes to be used.
+             */
             return 1;
         }
         return 0;
     }
-    supported = EVP_PKEY_supports_digest_nid(s->cert->pkeys[idx].privatekey,
-                                             sig->hash);
+    ERR_set_mark();
+    supported = EVP_PKEY_supports_digest_nid(pkey, sig->hash);
+    ERR_pop_to_mark();
     if (supported == 0)
         return 0;
-    else if (supported < 0)
-        ERR_clear_error();
+    /*
+     * If it didn't report a mandatory NID (supported < 0), for
+     * whatever reasons, we just ignore the error and allow all
+     * hashes to be used.
+     */
 
     return 1;
 }
 
+/*
+ * Returns true if |s| has a usable certificate configured for use
+ * with signature scheme |sig|.
+ * "Usable" includes a check for presence as well as applying
+ * the signature_algorithm_cert restrictions sent by the peer (if any).
+ * Returns false if no usable certificate is found.
+ */
+static int has_usable_cert(SSL *s, const SIGALG_LOOKUP *sig, int idx)
+{
+    /* TLS 1.2 callers can override sig->sig_idx, but not TLS 1.3 callers. */
+    if (idx == -1)
+        idx = sig->sig_idx;
+    if (!ssl_has_cert(s, idx))
+        return 0;
+
+    return check_cert_usable(s, sig, s->cert->pkeys[idx].x509,
+                             s->cert->pkeys[idx].privatekey);
+}
+
+/*
+ * Returns true if the supplied cert |x| and key |pkey| is usable with the
+ * specified signature scheme |sig|, or false otherwise.
+ */
+static int is_cert_usable(SSL *s, const SIGALG_LOOKUP *sig, X509 *x,
+                          EVP_PKEY *pkey)
+{
+    size_t idx;
+
+    if (ssl_cert_lookup_by_pkey(pkey, &idx) == NULL)
+        return 0;
+
+    /* Check the key is consistent with the sig alg */
+    if ((int)idx != sig->sig_idx)
+        return 0;
+
+    return check_cert_usable(s, sig, x, pkey);
+}
+
+/*
+ * Find a signature scheme that works with the supplied certificate |x| and key
+ * |pkey|. |x| and |pkey| may be NULL in which case we additionally look at our
+ * available certs/keys to find one that works.
+ */
+static const SIGALG_LOOKUP *find_sig_alg(SSL *s, X509 *x, EVP_PKEY *pkey)
+{
+    const SIGALG_LOOKUP *lu = NULL;
+    size_t i;
+#ifndef OPENSSL_NO_EC
+    int curve = -1;
+#endif
+    EVP_PKEY *tmppkey;
+
+    /* Look for a shared sigalgs matching possible certificates */
+    for (i = 0; i < s->shared_sigalgslen; i++) {
+        lu = s->shared_sigalgs[i];
+
+        /* Skip SHA1, SHA224, DSA and RSA if not PSS */
+        if (lu->hash == NID_sha1
+            || lu->hash == NID_sha224
+            || lu->sig == EVP_PKEY_DSA
+            || lu->sig == EVP_PKEY_RSA)
+            continue;
+        /* Check that we have a cert, and signature_algorithms_cert */
+        if (!tls1_lookup_md(lu, NULL))
+            continue;
+        if ((pkey == NULL && !has_usable_cert(s, lu, -1))
+                || (pkey != NULL && !is_cert_usable(s, lu, x, pkey)))
+            continue;
+
+        tmppkey = (pkey != NULL) ? pkey
+                                 : s->cert->pkeys[lu->sig_idx].privatekey;
+
+        if (lu->sig == EVP_PKEY_EC) {
+#ifndef OPENSSL_NO_EC
+            if (curve == -1) {
+                EC_KEY *ec = EVP_PKEY_get0_EC_KEY(tmppkey);
+                curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
+            }
+            if (lu->curve != NID_undef && curve != lu->curve)
+                continue;
+#else
+            continue;
+#endif
+        } else if (lu->sig == EVP_PKEY_RSA_PSS) {
+            /* validate that key is large enough for the signature algorithm */
+            if (!rsa_pss_check_min_key_size(EVP_PKEY_get0(tmppkey), lu))
+                continue;
+        }
+        break;
+    }
+
+    if (i == s->shared_sigalgslen)
+        return NULL;
+
+    return lu;
+}
+
 /*
  * Choose an appropriate signature algorithm based on available certificates
  * Sets chosen certificate and signature algorithm.
@@ -2683,48 +2804,8 @@ int tls_choose_sigalg(SSL *s, int fatalerrs)
     s->s3.tmp.sigalg = NULL;
 
     if (SSL_IS_TLS13(s)) {
-        size_t i;
-#ifndef OPENSSL_NO_EC
-        int curve = -1;
-#endif
-
-        /* Look for a certificate matching shared sigalgs */
-        for (i = 0; i < s->shared_sigalgslen; i++) {
-            lu = s->shared_sigalgs[i];
-            sig_idx = -1;
-
-            /* Skip SHA1, SHA224, DSA and RSA if not PSS */
-            if (lu->hash == NID_sha1
-                || lu->hash == NID_sha224
-                || lu->sig == EVP_PKEY_DSA
-                || lu->sig == EVP_PKEY_RSA)
-                continue;
-            /* Check that we have a cert, and signature_algorithms_cert */
-            if (!tls1_lookup_md(lu, NULL) || !has_usable_cert(s, lu, -1))
-                continue;
-            if (lu->sig == EVP_PKEY_EC) {
-#ifndef OPENSSL_NO_EC
-                if (curve == -1) {
-                    EC_KEY *ec = EVP_PKEY_get0_EC_KEY(s->cert->pkeys[SSL_PKEY_ECC].privatekey);
-
-                    curve = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
-                }
-                if (lu->curve != NID_undef && curve != lu->curve)
-                    continue;
-#else
-                continue;
-#endif
-            } else if (lu->sig == EVP_PKEY_RSA_PSS) {
-                /* validate that key is large enough for the signature algorithm */
-                EVP_PKEY *pkey;
-
-                pkey = s->cert->pkeys[lu->sig_idx].privatekey;
-                if (!rsa_pss_check_min_key_size(EVP_PKEY_get0(pkey), lu))
-                    continue;
-            }
-            break;
-        }
-        if (i == s->shared_sigalgslen) {
+        lu = find_sig_alg(s, NULL, NULL);
+        if (lu == NULL) {
             if (!fatalerrs)
                 return 1;
             SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_F_TLS_CHOOSE_SIGALG,