wbrawner/openssl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix bogus check for EVP_PKEY_supports_digest_nid() in check_cert_usable()

Browse source 
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)
This commit is contained in:
David Woodhouse 2019-08-22 18:09:11 +01:00 committed by Matt Caswell
parent 8b138d3fe4
commit b5a276884b
1 changed files with 32 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

66
ssl/t1_lib.c
View file

@ -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 (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(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)
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.
*/
return 1;
}
return 0;
}
/*
* 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;
/*
* 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.
*/
/*
* 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)
continue;
/*
* 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.
*/
if (mdnid == lu->hash && pknid == lu->sig)
return 1;
}
return 0;
}
/*
* Without signat_algorithms_cert, any certificate for which we have
* a viable public key is permitted.
*/
return 1;
}