Fix bogus check for EVP_PKEY_supports_digest_nid() in check_cert_usable()

In commit 2d263a4a73 ("Honour mandatory digest on private key in
has_usable_cert()" I added two checks for the capabilities of the
EVP_PKEY being used. One of them was wrong, as it should only be
checking the signature of the X.509 cert (by its issuer) against the
sigalgs given in a TLS v1.3 signature_algorithms_cert extension.

Remove it and provide the code comments which, if they'd been present
in the first place, would hopefully have prevented the mistake.

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

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index ab0a0e01436b74c0452c4f6fa03fceb39d85f65b..2f7b570a0603cfd9b1fe9b0a48b7d379776edae9 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2644,46 +2644,44 @@ static int check_cert_usable(SSL *s, const SIGALG_LOOKUP *sig, X509 *x,
     int mdnid, pknid, supported;
     size_t i;
 
+    /*
+     * If the given EVP_PKEY cannot supporting signing with this sigalg,
+     * the answer is simply 'no'.
+     */
+    ERR_set_mark();
+    supported = EVP_PKEY_supports_digest_nid(pkey, sig->hash);
+    ERR_pop_to_mark();
+    if (supported == 0)
+        return 0;
+
+    /*
+     * The TLS 1.3 signature_algorithms_cert extension places restrictions
+     * on the sigalg with which the certificate was signed (by its issuer).
+     */
     if (s->s3.tmp.peer_cert_sigalgs != NULL) {
+        if (!X509_get_signature_info(x, &mdnid, &pknid, NULL, NULL))
+            return 0;
         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(x, &mdnid, &pknid, NULL, NULL)
-                /*
-                 * TODO this does not differentiate between the
-                 * rsa_pss_pss_* and rsa_pss_rsae_* schemes since we do not
-                 * have a chain here that lets us look at the key OID in the
-                 * signing certificate.
-                 */
-                || mdnid != lu->hash
-                || pknid != lu->sig)
+            if (lu == NULL)
                 continue;
 
-            ERR_set_mark();
-            supported = EVP_PKEY_supports_digest_nid(pkey, mdnid);
-            ERR_pop_to_mark();
-            if (supported == 0)
-                continue;
             /*
-             * 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.
+             * TODO this does not differentiate between the
+             * rsa_pss_pss_* and rsa_pss_rsae_* schemes since we do not
+             * have a chain here that lets us look at the key OID in the
+             * signing certificate.
              */
-            return 1;
+            if (mdnid == lu->hash && pknid == lu->sig)
+                return 1;
         }
         return 0;
     }
-    ERR_set_mark();
-    supported = EVP_PKEY_supports_digest_nid(pkey, sig->hash);
-    ERR_pop_to_mark();
-    if (supported == 0)
-        return 0;
+
     /*
-     * 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.
+     * Without signat_algorithms_cert, any certificate for which we have
+     * a viable public key is permitted.
      */
-
     return 1;
 }