crypto: only verify CA certs in chain of trust
The CA file provided to qemu may contain CA certificates which do not form part of the chain of trust for the specific certificate we are sanity checking. This patch changes the sanity checking from validating every CA certificate to only checking the CA certificates which are part of the chain of trust (issuer chain). Other certificates are ignored. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Henry Kleynhans <hkleynhans@fb.com> [DB: changed 'int' to 'bool' in 'checking_issuer' variable] Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
parent
b7a1f2ca45
commit
75216f239f
2 changed files with 75 additions and 7 deletions
|
|
@ -315,6 +315,51 @@ qcrypto_tls_creds_check_cert(QCryptoTLSCredsX509 *creds,
|
|||
return 0;
|
||||
}
|
||||
|
||||
static int
|
||||
qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
|
||||
gnutls_x509_crt_t cert,
|
||||
gnutls_x509_crt_t *cacerts,
|
||||
unsigned int ncacerts,
|
||||
const char *cacertFile,
|
||||
bool isServer,
|
||||
bool isCA,
|
||||
Error **errp)
|
||||
{
|
||||
gnutls_x509_crt_t *cert_to_check = &cert;
|
||||
bool checking_issuer = true;
|
||||
int retval = 0;
|
||||
|
||||
while (checking_issuer) {
|
||||
checking_issuer = false;
|
||||
|
||||
if (gnutls_x509_crt_check_issuer(*cert_to_check,
|
||||
*cert_to_check)) {
|
||||
/*
|
||||
* The cert is self-signed indicating we have
|
||||
* reached the root of trust.
|
||||
*/
|
||||
return qcrypto_tls_creds_check_cert(
|
||||
creds, *cert_to_check, cacertFile,
|
||||
isServer, isCA, errp);
|
||||
}
|
||||
for (int i = 0; i < ncacerts; i++) {
|
||||
if (gnutls_x509_crt_check_issuer(*cert_to_check,
|
||||
cacerts[i])) {
|
||||
retval = qcrypto_tls_creds_check_cert(
|
||||
creds, cacerts[i], cacertFile,
|
||||
isServer, isCA, errp);
|
||||
if (retval < 0) {
|
||||
return retval;
|
||||
}
|
||||
cert_to_check = &cacerts[i];
|
||||
checking_issuer = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return -1;
|
||||
}
|
||||
|
||||
static int
|
||||
qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t cert,
|
||||
|
|
@ -499,12 +544,12 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
|
|||
goto cleanup;
|
||||
}
|
||||
|
||||
for (i = 0; i < ncacerts; i++) {
|
||||
if (qcrypto_tls_creds_check_cert(creds,
|
||||
cacerts[i], cacertFile,
|
||||
isServer, true, errp) < 0) {
|
||||
goto cleanup;
|
||||
}
|
||||
if (cert &&
|
||||
qcrypto_tls_creds_check_authority_chain(creds, cert,
|
||||
cacerts, ncacerts,
|
||||
cacertFile, isServer,
|
||||
true, errp) < 0) {
|
||||
goto cleanup;
|
||||
}
|
||||
|
||||
if (cert && ncacerts &&
|
||||
|
|
|
|||
|
|
@ -589,6 +589,12 @@ int main(int argc, char **argv)
|
|||
true, true, GNUTLS_KEY_KEY_CERT_SIGN,
|
||||
false, false, NULL, NULL,
|
||||
0, 0);
|
||||
TLS_CERT_REQ(cacertlevel1creq_invalid, cacertrootreq,
|
||||
"UK", "qemu level 1c invalid", NULL, NULL, NULL, NULL,
|
||||
true, true, true,
|
||||
true, true, GNUTLS_KEY_KEY_CERT_SIGN,
|
||||
false, false, NULL, NULL,
|
||||
360, 400);
|
||||
TLS_CERT_REQ(cacertlevel2areq, cacertlevel1areq,
|
||||
"UK", "qemu level 2a", NULL, NULL, NULL, NULL,
|
||||
true, true, true,
|
||||
|
|
@ -617,16 +623,32 @@ int main(int argc, char **argv)
|
|||
cacertlevel2areq.crt,
|
||||
};
|
||||
|
||||
|
||||
test_tls_write_cert_chain(WORKDIR "cacertchain-ctx.pem",
|
||||
certchain,
|
||||
G_N_ELEMENTS(certchain));
|
||||
|
||||
gnutls_x509_crt_t certchain_with_invalid[] = {
|
||||
cacertrootreq.crt,
|
||||
cacertlevel1areq.crt,
|
||||
cacertlevel1breq.crt,
|
||||
cacertlevel1creq_invalid.crt,
|
||||
cacertlevel2areq.crt,
|
||||
};
|
||||
|
||||
test_tls_write_cert_chain(WORKDIR "cacertchain-with-invalid-ctx.pem",
|
||||
certchain_with_invalid,
|
||||
G_N_ELEMENTS(certchain_with_invalid));
|
||||
|
||||
TLS_TEST_REG(chain1, true,
|
||||
WORKDIR "cacertchain-ctx.pem",
|
||||
servercertlevel3areq.filename, false);
|
||||
TLS_TEST_REG(chain2, false,
|
||||
WORKDIR "cacertchain-ctx.pem",
|
||||
clientcertlevel2breq.filename, false);
|
||||
TLS_TEST_REG(certchainwithexpiredcert, false,
|
||||
WORKDIR "cacertchain-with-invalid-ctx.pem",
|
||||
clientcertlevel2breq.filename, false);
|
||||
|
||||
/* Some missing certs - first two are fatal, the last
|
||||
* is ok
|
||||
|
|
@ -640,7 +662,6 @@ int main(int argc, char **argv)
|
|||
TLS_TEST_REG(missingclient, false,
|
||||
cacert1req.filename,
|
||||
"clientcertdoesnotexist.pem", false);
|
||||
|
||||
ret = g_test_run();
|
||||
|
||||
test_tls_discard_cert(&cacertreq);
|
||||
|
|
@ -694,10 +715,12 @@ int main(int argc, char **argv)
|
|||
test_tls_discard_cert(&cacertrootreq);
|
||||
test_tls_discard_cert(&cacertlevel1areq);
|
||||
test_tls_discard_cert(&cacertlevel1breq);
|
||||
test_tls_discard_cert(&cacertlevel1creq_invalid);
|
||||
test_tls_discard_cert(&cacertlevel2areq);
|
||||
test_tls_discard_cert(&servercertlevel3areq);
|
||||
test_tls_discard_cert(&clientcertlevel2breq);
|
||||
unlink(WORKDIR "cacertchain-ctx.pem");
|
||||
unlink(WORKDIR "cacertchain-with-invalid-ctx.pem");
|
||||
|
||||
test_tls_cleanup(KEYFILE);
|
||||
rmdir(WORKDIR);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue