diff options
author | David Woodhouse <dwmw2@infradead.org> | 2019-08-22 18:09:11 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2019-08-27 15:32:59 +0100 |
commit | b5a276884b8e945815732845540af3c8143e8457 (patch) | |
tree | 7585880453531c2dfaf353885e211c6b18ec6a69 /ssl | |
parent | 8b138d3fe4d9dbbc632511d4ac8b7099b793630b (diff) |
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)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/t1_lib.c | 54 |
1 files changed, 26 insertions, 28 deletions
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index ab0a0e0143..2f7b570a06 100644 --- 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; } |