394 lines
15 KiB
Diff
394 lines
15 KiB
Diff
From be0e49b1012e5e30fbdabd4a6cee3e9ed4e94ef9 Mon Sep 17 00:00:00 2001
|
|
From: matoro <matoro@users.noreply.github.com>
|
|
Date: Mon, 13 Feb 2023 13:00:49 -0500
|
|
Subject: [PATCH 05/30] crypto: allow client/server cert chains
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
RH-Author: Daniel P. Berrangé <berrange@redhat.com>
|
|
RH-MergeRequest: 460: crypto: changes to support multiple certificates for PQC
|
|
RH-Jira: RHEL-111934
|
|
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
RH-Commit: [4/27] 1ac4f7afc0600ca8c9d629477557de13ab1db7de (berrange/centos-src-qemu)
|
|
|
|
The existing implementation assumes that client/server certificates are
|
|
single individual certificates. If using publicly-issued certificates,
|
|
or internal CAs that use an intermediate issuer, this is unlikely to be
|
|
the case, and they will instead be certificate chains. While this can
|
|
be worked around by moving the intermediate certificates to the CA
|
|
certificate, which DOES currently support multiple certificates, this
|
|
instead allows the issued certificate chains to be used as-is, without
|
|
requiring the overhead of shuffling certificates around.
|
|
|
|
Corresponding libvirt change is available here:
|
|
https://gitlab.com/libvirt/libvirt/-/merge_requests/222
|
|
|
|
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
Reviewed-by: Eric Blake <eblake@redhat.com>
|
|
Signed-off-by: matoro <matoro_mailinglist_qemu@matoro.tk>
|
|
[DB: adapted for code conflicts with multi-CA patch]
|
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
(cherry picked from commit 87d6282e53a2db1915ea715cf9ee0634f6fb1c70)
|
|
---
|
|
crypto/tlscredsx509.c | 157 ++++++++++++--------------
|
|
tests/unit/test-crypto-tlscredsx509.c | 77 +++++++++++++
|
|
2 files changed, 147 insertions(+), 87 deletions(-)
|
|
|
|
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
|
|
index 2eccd71b3c..86fdfce886 100644
|
|
--- a/crypto/tlscredsx509.c
|
|
+++ b/crypto/tlscredsx509.c
|
|
@@ -317,7 +317,8 @@ qcrypto_tls_creds_check_cert(QCryptoTLSCredsX509 *creds,
|
|
|
|
static int
|
|
qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
|
|
- gnutls_x509_crt_t cert,
|
|
+ gnutls_x509_crt_t *certs,
|
|
+ unsigned int ncerts,
|
|
gnutls_x509_crt_t *cacerts,
|
|
unsigned int ncacerts,
|
|
const char *cacertFile,
|
|
@@ -325,9 +326,32 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
|
|
bool isCA,
|
|
Error **errp)
|
|
{
|
|
- gnutls_x509_crt_t cert_to_check = cert;
|
|
+ gnutls_x509_crt_t cert_to_check = certs[ncerts - 1];
|
|
int retval = 0;
|
|
- gnutls_datum_t dn = {};
|
|
+ gnutls_datum_t dn = {}, dnissuer = {};
|
|
+
|
|
+ for (int i = 0; i < (ncerts - 1); i++) {
|
|
+ if (!gnutls_x509_crt_check_issuer(certs[i], certs[i + 1])) {
|
|
+ retval = gnutls_x509_crt_get_dn2(certs[i], &dn);
|
|
+ if (retval < 0) {
|
|
+ error_setg(errp, "Unable to fetch cert DN: %s",
|
|
+ gnutls_strerror(retval));
|
|
+ return -1;
|
|
+ }
|
|
+ retval = gnutls_x509_crt_get_dn2(certs[i + 1], &dnissuer);
|
|
+ if (retval < 0) {
|
|
+ g_free(dn.data);
|
|
+ error_setg(errp, "Unable to fetch cert DN: %s",
|
|
+ gnutls_strerror(retval));
|
|
+ return -1;
|
|
+ }
|
|
+ error_setg(errp, "Cert '%s' does not match issuer of cert '%s'",
|
|
+ dnissuer.data, dn.data);
|
|
+ g_free(dn.data);
|
|
+ g_free(dnissuer.data);
|
|
+ return -1;
|
|
+ }
|
|
+ }
|
|
|
|
for (;;) {
|
|
gnutls_x509_crt_t cert_issuer = NULL;
|
|
@@ -373,7 +397,8 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
|
|
}
|
|
|
|
static int
|
|
-qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t cert,
|
|
+qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t *certs,
|
|
+ size_t ncerts,
|
|
const char *certFile,
|
|
gnutls_x509_crt_t *cacerts,
|
|
size_t ncacerts,
|
|
@@ -383,7 +408,7 @@ qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t cert,
|
|
{
|
|
unsigned int status;
|
|
|
|
- if (gnutls_x509_crt_list_verify(&cert, 1,
|
|
+ if (gnutls_x509_crt_list_verify(certs, ncerts,
|
|
cacerts, ncacerts,
|
|
NULL, 0,
|
|
0, &status) < 0) {
|
|
@@ -425,66 +450,14 @@ qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t cert,
|
|
}
|
|
|
|
|
|
-static gnutls_x509_crt_t
|
|
-qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
|
|
- const char *certFile,
|
|
- bool isServer,
|
|
- Error **errp)
|
|
-{
|
|
- gnutls_datum_t data;
|
|
- gnutls_x509_crt_t cert = NULL;
|
|
- g_autofree char *buf = NULL;
|
|
- gsize buflen;
|
|
- GError *gerr = NULL;
|
|
- int ret = -1;
|
|
- int err;
|
|
-
|
|
- trace_qcrypto_tls_creds_x509_load_cert(creds, isServer, certFile);
|
|
-
|
|
- err = gnutls_x509_crt_init(&cert);
|
|
- if (err < 0) {
|
|
- error_setg(errp, "Unable to initialize certificate: %s",
|
|
- gnutls_strerror(err));
|
|
- goto cleanup;
|
|
- }
|
|
-
|
|
- if (!g_file_get_contents(certFile, &buf, &buflen, &gerr)) {
|
|
- error_setg(errp, "Cannot load CA cert list %s: %s",
|
|
- certFile, gerr->message);
|
|
- g_error_free(gerr);
|
|
- goto cleanup;
|
|
- }
|
|
-
|
|
- data.data = (unsigned char *)buf;
|
|
- data.size = strlen(buf);
|
|
-
|
|
- err = gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM);
|
|
- if (err < 0) {
|
|
- error_setg(errp, isServer ?
|
|
- "Unable to import server certificate %s: %s" :
|
|
- "Unable to import client certificate %s: %s",
|
|
- certFile,
|
|
- gnutls_strerror(err));
|
|
- goto cleanup;
|
|
- }
|
|
-
|
|
- ret = 0;
|
|
-
|
|
- cleanup:
|
|
- if (ret != 0) {
|
|
- gnutls_x509_crt_deinit(cert);
|
|
- cert = NULL;
|
|
- }
|
|
- return cert;
|
|
-}
|
|
-
|
|
-
|
|
static int
|
|
-qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509 *creds,
|
|
- const char *certFile,
|
|
- gnutls_x509_crt_t **certs,
|
|
- unsigned int *ncerts,
|
|
- Error **errp)
|
|
+qcrypto_tls_creds_load_cert_list(QCryptoTLSCredsX509 *creds,
|
|
+ const char *certFile,
|
|
+ gnutls_x509_crt_t **certs,
|
|
+ unsigned int *ncerts,
|
|
+ bool isServer,
|
|
+ bool isCA,
|
|
+ Error **errp)
|
|
{
|
|
gnutls_datum_t data;
|
|
g_autofree char *buf = NULL;
|
|
@@ -507,7 +480,9 @@ qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509 *creds,
|
|
if (gnutls_x509_crt_list_import2(certs, ncerts, &data,
|
|
GNUTLS_X509_FMT_PEM, 0) < 0) {
|
|
error_setg(errp,
|
|
- "Unable to import CA certificate list %s",
|
|
+ isCA ? "Unable to import CA certificate list %s" :
|
|
+ (isServer ? "Unable to import server certificate %s" :
|
|
+ "Unable to import client certificate %s"),
|
|
certFile);
|
|
return -1;
|
|
}
|
|
@@ -523,7 +498,8 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
|
|
const char *certFile,
|
|
Error **errp)
|
|
{
|
|
- gnutls_x509_crt_t cert = NULL;
|
|
+ gnutls_x509_crt_t *certs = NULL;
|
|
+ unsigned int ncerts = 0;
|
|
gnutls_x509_crt_t *cacerts = NULL;
|
|
unsigned int ncacerts = 0;
|
|
size_t i;
|
|
@@ -531,41 +507,48 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
|
|
|
|
if (certFile &&
|
|
access(certFile, R_OK) == 0) {
|
|
- cert = qcrypto_tls_creds_load_cert(creds,
|
|
- certFile, isServer,
|
|
- errp);
|
|
- if (!cert) {
|
|
+ if (qcrypto_tls_creds_load_cert_list(creds,
|
|
+ certFile,
|
|
+ &certs,
|
|
+ &ncerts,
|
|
+ isServer,
|
|
+ false,
|
|
+ errp) < 0) {
|
|
goto cleanup;
|
|
}
|
|
}
|
|
if (access(cacertFile, R_OK) == 0) {
|
|
- if (qcrypto_tls_creds_load_ca_cert_list(creds,
|
|
- cacertFile,
|
|
- &cacerts,
|
|
- &ncacerts,
|
|
- errp) < 0) {
|
|
+ if (qcrypto_tls_creds_load_cert_list(creds,
|
|
+ cacertFile,
|
|
+ &cacerts,
|
|
+ &ncacerts,
|
|
+ isServer,
|
|
+ true,
|
|
+ errp) < 0) {
|
|
goto cleanup;
|
|
}
|
|
}
|
|
|
|
- if (cert &&
|
|
- qcrypto_tls_creds_check_cert(creds,
|
|
- cert, certFile, isServer,
|
|
- false, errp) < 0) {
|
|
- goto cleanup;
|
|
+ for (i = 0; i < ncerts; i++) {
|
|
+ if (qcrypto_tls_creds_check_cert(creds,
|
|
+ certs[i], certFile,
|
|
+ isServer, i != 0, errp) < 0) {
|
|
+ goto cleanup;
|
|
+ }
|
|
}
|
|
|
|
- if (cert &&
|
|
- qcrypto_tls_creds_check_authority_chain(creds, cert,
|
|
+ if (ncerts &&
|
|
+ qcrypto_tls_creds_check_authority_chain(creds,
|
|
+ certs, ncerts,
|
|
cacerts, ncacerts,
|
|
cacertFile, isServer,
|
|
true, errp) < 0) {
|
|
goto cleanup;
|
|
}
|
|
|
|
- if (cert && ncacerts &&
|
|
- qcrypto_tls_creds_check_cert_pair(cert, certFile, cacerts,
|
|
- ncacerts, cacertFile,
|
|
+ if (ncerts && ncacerts &&
|
|
+ qcrypto_tls_creds_check_cert_pair(certs, ncerts, certFile,
|
|
+ cacerts, ncacerts, cacertFile,
|
|
isServer, errp) < 0) {
|
|
goto cleanup;
|
|
}
|
|
@@ -573,8 +556,8 @@ qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
|
|
ret = 0;
|
|
|
|
cleanup:
|
|
- if (cert) {
|
|
- gnutls_x509_crt_deinit(cert);
|
|
+ for (i = 0; i < ncerts; i++) {
|
|
+ gnutls_x509_crt_deinit(certs[i]);
|
|
}
|
|
for (i = 0; i < ncacerts; i++) {
|
|
gnutls_x509_crt_deinit(cacerts[i]);
|
|
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
|
|
index d8bc3be491..a84df01e51 100644
|
|
--- a/tests/unit/test-crypto-tlscredsx509.c
|
|
+++ b/tests/unit/test-crypto-tlscredsx509.c
|
|
@@ -577,6 +577,12 @@ int main(int argc, char **argv)
|
|
true, true, GNUTLS_KEY_KEY_CERT_SIGN,
|
|
false, false, NULL, NULL,
|
|
0, 0);
|
|
+ TLS_ROOT_REQ(someotherrootreq,
|
|
+ "UK", "some other random CA", NULL, NULL, NULL, NULL,
|
|
+ true, true, true,
|
|
+ true, true, GNUTLS_KEY_KEY_CERT_SIGN,
|
|
+ false, false, NULL, NULL,
|
|
+ 0, 0);
|
|
TLS_CERT_REQ(cacertlevel1areq, cacertrootreq,
|
|
"UK", "qemu level 1a", NULL, NULL, NULL, NULL,
|
|
true, true, true,
|
|
@@ -623,6 +629,32 @@ int main(int argc, char **argv)
|
|
cacertlevel2areq.crt,
|
|
};
|
|
|
|
+ gnutls_x509_crt_t cabundle[] = {
|
|
+ someotherrootreq.crt,
|
|
+ cacertrootreq.crt,
|
|
+ };
|
|
+
|
|
+ gnutls_x509_crt_t servercertchain[] = {
|
|
+ servercertlevel3areq.crt,
|
|
+ cacertlevel2areq.crt,
|
|
+ cacertlevel1areq.crt,
|
|
+ };
|
|
+
|
|
+ gnutls_x509_crt_t servercertchain_incomplete[] = {
|
|
+ servercertlevel3areq.crt,
|
|
+ cacertlevel2areq.crt,
|
|
+ };
|
|
+
|
|
+ gnutls_x509_crt_t servercertchain_unsorted[] = {
|
|
+ servercertlevel3areq.crt,
|
|
+ cacertlevel1areq.crt,
|
|
+ cacertlevel2areq.crt,
|
|
+ };
|
|
+
|
|
+ gnutls_x509_crt_t clientcertchain[] = {
|
|
+ clientcertlevel2breq.crt,
|
|
+ cacertlevel1breq.crt,
|
|
+ };
|
|
|
|
test_tls_write_cert_chain(WORKDIR "cacertchain-ctx.pem",
|
|
certchain,
|
|
@@ -662,6 +694,46 @@ int main(int argc, char **argv)
|
|
WORKDIR "cacertchain-incomplete-ctx.pem",
|
|
servercertlevel3areq.filename, true);
|
|
|
|
+ test_tls_write_cert_chain(WORKDIR "servercertchain-ctx.pem",
|
|
+ servercertchain,
|
|
+ G_N_ELEMENTS(servercertchain));
|
|
+
|
|
+ TLS_TEST_REG(serverchain, true,
|
|
+ cacertrootreq.filename,
|
|
+ WORKDIR "servercertchain-ctx.pem", false);
|
|
+
|
|
+ test_tls_write_cert_chain(WORKDIR "cabundle-ctx.pem",
|
|
+ cabundle,
|
|
+ G_N_ELEMENTS(cabundle));
|
|
+
|
|
+ TLS_TEST_REG(multiplecaswithchain, true,
|
|
+ WORKDIR "cabundle-ctx.pem",
|
|
+ WORKDIR "servercertchain-ctx.pem", false);
|
|
+
|
|
+ test_tls_write_cert_chain(WORKDIR "servercertchain_incomplete-ctx.pem",
|
|
+ servercertchain_incomplete,
|
|
+ G_N_ELEMENTS(servercertchain_incomplete));
|
|
+
|
|
+ TLS_TEST_REG(incompleteserverchain, true,
|
|
+ cacertrootreq.filename,
|
|
+ WORKDIR "servercertchain_incomplete-ctx.pem", true);
|
|
+
|
|
+ test_tls_write_cert_chain(WORKDIR "servercertchain_unsorted-ctx.pem",
|
|
+ servercertchain_unsorted,
|
|
+ G_N_ELEMENTS(servercertchain_unsorted));
|
|
+
|
|
+ TLS_TEST_REG(unsortedserverchain, true,
|
|
+ cacertrootreq.filename,
|
|
+ WORKDIR "servercertchain_unsorted-ctx.pem", true);
|
|
+
|
|
+ test_tls_write_cert_chain(WORKDIR "clientcertchain-ctx.pem",
|
|
+ clientcertchain,
|
|
+ G_N_ELEMENTS(clientcertchain));
|
|
+
|
|
+ TLS_TEST_REG(clientchain, false,
|
|
+ cacertrootreq.filename,
|
|
+ WORKDIR "clientcertchain-ctx.pem", false);
|
|
+
|
|
/* Some missing certs - first two are fatal, the last
|
|
* is ok
|
|
*/
|
|
@@ -731,8 +803,13 @@ int main(int argc, char **argv)
|
|
test_tls_discard_cert(&cacertlevel2areq);
|
|
test_tls_discard_cert(&servercertlevel3areq);
|
|
test_tls_discard_cert(&clientcertlevel2breq);
|
|
+ test_tls_discard_cert(&someotherrootreq);
|
|
unlink(WORKDIR "cacertchain-ctx.pem");
|
|
unlink(WORKDIR "cacertchain-with-invalid-ctx.pem");
|
|
+ unlink(WORKDIR "servercertchain-ctx.pem");
|
|
+ unlink(WORKDIR "servercertchain_incomplete-ctx.pem");
|
|
+ unlink(WORKDIR "servercertchain_unsorted-ctx.pem");
|
|
+ unlink(WORKDIR "clientcertchain-ctx.pem");
|
|
|
|
test_tls_cleanup(KEYFILE);
|
|
rmdir(WORKDIR);
|
|
--
|
|
2.52.0
|
|
|