* [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling
@ 2025-10-20 14:12 Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 1/7] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake
This series includes three patches that were posted a fairly
long time ago. They are borderline between a feature request
and a bug fix, but I'm classing them more bug fix, since they
addressing issues with cert acceptance that we really should
not have had.
The patches by Henry had outstanding comments from myself,
and I've chosen to simply fix them in two followup commits
of my own now to get this over the line.
The patch from "matoro" was not accepted because they were
contributed under a github alias. With our change to have
a more relaxed interpretation of the DCO allowing any
"known identity", we can now accept this patch. It had
some conflicts with Henry's patch which I've fixed up.
Then there is one other small bug fix and one improvement
to use a newer gnutls API.
Changed in v3:
- Re-ordered patch for fixing error reporting to be
near start of series, instead of end
- Add unit test for validating error reporting with
incomplete CA chains
- Unit test to validate that an Error is filled on
expected failures
Changed in v2:
- Update to latest upstream
Daniel P. Berrangé (5):
crypto: remove extraneous pointer usage in gnutls certs
crypto: validate an error is reported in test expected fails
crypto: fix error reporting in cert chain checks
crypto: stop requiring "key encipherment" usage in x509 certs
crypto: switch to newer gnutls API for distinguished name
Henry Kleynhans (1):
crypto: only verify CA certs in chain of trust
matoro (1):
crypto: allow client/server cert chains
crypto/tlscredsx509.c | 223 +++++++++++++++-----------
crypto/tlssession.c | 12 +-
docs/system/tls.rst | 13 +-
tests/unit/crypto-tls-x509-helpers.h | 6 +-
tests/unit/test-crypto-tlscredsx509.c | 155 +++++++++++++++---
tests/unit/test-crypto-tlssession.c | 14 +-
tests/unit/test-io-channel-tls.c | 4 +-
7 files changed, 280 insertions(+), 147 deletions(-)
--
2.50.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/7] crypto: only verify CA certs in chain of trust
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 2/7] crypto: remove extraneous pointer usage in gnutls certs Daniel P. Berrangé
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake, Henry Kleynhans
From: Henry Kleynhans <hkleynhans@fb.com>
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>
---
crypto/tlscredsx509.c | 57 ++++++++++++++++++++++++---
tests/unit/test-crypto-tlscredsx509.c | 25 +++++++++++-
2 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index cd1f504471..3df2a33b0b 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -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 &&
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 3c25d75ca1..a7ea5f422d 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -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);
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/7] crypto: remove extraneous pointer usage in gnutls certs
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 1/7] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 3/7] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eric Blake, Philippe Mathieu-Daudé
The 'gnutls_x509_crt_t' type is already a pointer, not a struct,
so the extra level of pointer indirection is not needed.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 3df2a33b0b..4169ad9a75 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -325,25 +325,25 @@ 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 = 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)) {
+ 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,
+ creds, cert_to_check, cacertFile,
isServer, isCA, errp);
}
for (int i = 0; i < ncacerts; i++) {
- if (gnutls_x509_crt_check_issuer(*cert_to_check,
+ if (gnutls_x509_crt_check_issuer(cert_to_check,
cacerts[i])) {
retval = qcrypto_tls_creds_check_cert(
creds, cacerts[i], cacertFile,
@@ -351,7 +351,7 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
if (retval < 0) {
return retval;
}
- cert_to_check = &cacerts[i];
+ cert_to_check = cacerts[i];
checking_issuer = true;
break;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/7] crypto: validate an error is reported in test expected fails
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 1/7] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 2/7] crypto: remove extraneous pointer usage in gnutls certs Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 19:40 ` Eric Blake
2025-10-20 20:14 ` Philippe Mathieu-Daudé
2025-10-20 14:12 ` [PATCH v3 4/7] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
` (3 subsequent siblings)
6 siblings, 2 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake
There was a bug where TLS x509 credentials validation failed
to fill out the Error object. Validate this in the failure
scenarios.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-crypto-tlscredsx509.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index a7ea5f422d..85f51aee1b 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -73,6 +73,7 @@ static void test_tls_creds(const void *opaque)
struct QCryptoTLSCredsTestData *data =
(struct QCryptoTLSCredsTestData *)opaque;
QCryptoTLSCreds *creds;
+ Error *err = NULL;
#define CERT_DIR "tests/test-crypto-tlscredsx509-certs/"
g_mkdir_with_parents(CERT_DIR, 0700);
@@ -111,10 +112,12 @@ static void test_tls_creds(const void *opaque)
QCRYPTO_TLS_CREDS_ENDPOINT_SERVER :
QCRYPTO_TLS_CREDS_ENDPOINT_CLIENT),
CERT_DIR,
- data->expectFail ? NULL : &error_abort);
+ data->expectFail ? &err : &error_abort);
if (data->expectFail) {
g_assert(creds == NULL);
+ g_assert(err != NULL);
+ error_free(err);
} else {
g_assert(creds != NULL);
}
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/7] crypto: fix error reporting in cert chain checks
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
` (2 preceding siblings ...)
2025-10-20 14:12 ` [PATCH v3 3/7] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 19:50 ` Eric Blake
2025-10-20 14:12 ` [PATCH v3 5/7] crypto: allow client/server cert chains Daniel P. Berrangé
` (2 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake
The loop that checks the CA certificate chain can fail to report
an error message if one of the certs in the chain has an issuer
that is not present in the chain. In this case, the outer loop
'while (checking_issuer)' will terminate after failing to find
the issuer, and no error message will be reported.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 33 ++++++++++++++++++---------
tests/unit/test-crypto-tlscredsx509.c | 12 ++++++++++
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 4169ad9a75..e1f82f1cdc 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -326,11 +326,11 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
Error **errp)
{
gnutls_x509_crt_t cert_to_check = cert;
- bool checking_issuer = true;
int retval = 0;
+ gnutls_datum_t dn = {};
- while (checking_issuer) {
- checking_issuer = false;
+ for (;;) {
+ gnutls_x509_crt_t cert_issuer = NULL;
if (gnutls_x509_crt_check_issuer(cert_to_check,
cert_to_check)) {
@@ -345,19 +345,30 @@ qcrypto_tls_creds_check_authority_chain(QCryptoTLSCredsX509 *creds,
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;
+ cert_issuer = cacerts[i];
break;
}
}
+ if (!cert_issuer) {
+ break;
+ }
+
+ if (qcrypto_tls_creds_check_cert(creds, cert_issuer, cacertFile,
+ isServer, isCA, errp) < 0) {
+ return -1;
+ }
+
+ cert_to_check = cert_issuer;
}
+ retval = gnutls_x509_crt_get_dn2(cert_to_check, &dn);
+ if (retval < 0) {
+ error_setg(errp, "Unable to fetch cert DN: %s",
+ gnutls_strerror(retval));
+ return -1;
+ }
+ error_setg(errp, "Cert '%s' has no issuer in CA chain", dn.data);
+ gnutls_free(dn.data);
return -1;
}
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 85f51aee1b..5f78fe7d6f 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -643,6 +643,15 @@ int main(int argc, char **argv)
certchain_with_invalid,
G_N_ELEMENTS(certchain_with_invalid));
+ gnutls_x509_crt_t certchain_incomplete[] = {
+ cacertrootreq.crt,
+ cacertlevel2areq.crt,
+ };
+
+ test_tls_write_cert_chain(WORKDIR "cacertchain-incomplete-ctx.pem",
+ certchain_incomplete,
+ G_N_ELEMENTS(certchain_incomplete));
+
TLS_TEST_REG(chain1, true,
WORKDIR "cacertchain-ctx.pem",
servercertlevel3areq.filename, false);
@@ -652,6 +661,9 @@ int main(int argc, char **argv)
TLS_TEST_REG(certchainwithexpiredcert, false,
WORKDIR "cacertchain-with-invalid-ctx.pem",
clientcertlevel2breq.filename, false);
+ TLS_TEST_REG(chainincomlpete, true,
+ WORKDIR "cacertchain-incomplete-ctx.pem",
+ servercertlevel3areq.filename, true);
/* Some missing certs - first two are fatal, the last
* is ok
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/7] crypto: allow client/server cert chains
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
` (3 preceding siblings ...)
2025-10-20 14:12 ` [PATCH v3 4/7] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 6/7] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 7/7] crypto: switch to newer gnutls API for distinguished name Daniel P. Berrangé
6 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake, matoro
From: matoro <matoro@users.noreply.github.com>
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>
---
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 e1f82f1cdc..e21d85fe16 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) {
+ gnutls_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);
+ gnutls_free(dn.data);
+ gnutls_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 5f78fe7d6f..546225a191 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -580,6 +580,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,
@@ -626,6 +632,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,
@@ -665,6 +697,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
*/
@@ -734,8 +806,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.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 6/7] crypto: stop requiring "key encipherment" usage in x509 certs
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
` (4 preceding siblings ...)
2025-10-20 14:12 ` [PATCH v3 5/7] crypto: allow client/server cert chains Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 7/7] crypto: switch to newer gnutls API for distinguished name Daniel P. Berrangé
6 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake
This usage flag was deprecated by RFC8813, such that it is
forbidden to be present for certs using ECDSA/ECDH algorithms,
and in TLS 1.3 is conceptually obsolete.
As such many valid certs will no longer have this key usage
flag set, and QEMU should not be rejecting them, as this
prevents use of otherwise valid & desirable algorithms.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlscredsx509.c | 10 +-------
docs/system/tls.rst | 13 +++-------
tests/unit/crypto-tls-x509-helpers.h | 6 ++---
tests/unit/test-crypto-tlscredsx509.c | 36 +++++++++++++--------------
tests/unit/test-crypto-tlssession.c | 14 +++++------
tests/unit/test-io-channel-tls.c | 4 +--
6 files changed, 34 insertions(+), 49 deletions(-)
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index e21d85fe16..f26429736c 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -144,7 +144,7 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 *creds,
if (status < 0) {
if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT;
+ GNUTLS_KEY_DIGITAL_SIGNATURE;
} else {
error_setg(errp,
"Unable to query certificate %s key usage: %s",
@@ -171,14 +171,6 @@ qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 *creds,
return -1;
}
}
- if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
- if (critical) {
- error_setg(errp,
- "Certificate %s usage does not permit key "
- "encipherment", certFile);
- return -1;
- }
- }
}
return 0;
diff --git a/docs/system/tls.rst b/docs/system/tls.rst
index e284c82801..a4f6781d62 100644
--- a/docs/system/tls.rst
+++ b/docs/system/tls.rst
@@ -118,7 +118,6 @@ information for each server, and use it to issue server certificates.
ip_address = 2620:0:cafe::87
ip_address = 2001:24::92
tls_www_server
- encryption_key
signing_key
EOF
# certtool --generate-privkey > server-hostNNN-key.pem
@@ -134,9 +133,8 @@ the subject alt name extension data. The ``tls_www_server`` keyword is
the key purpose extension to indicate this certificate is intended for
usage in a web server. Although QEMU network services are not in fact
HTTP servers (except for VNC websockets), setting this key purpose is
-still recommended. The ``encryption_key`` and ``signing_key`` keyword is
-the key usage extension to indicate this certificate is intended for
-usage in the data session.
+still recommended. The ``signing_key`` keyword is the key usage extension
+to indicate this certificate is intended for usage in the data session.
The ``server-hostNNN-key.pem`` and ``server-hostNNN-cert.pem`` files
should now be securely copied to the server for which they were
@@ -171,7 +169,6 @@ certificates.
organization = Name of your organization
cn = hostNNN.foo.example.com
tls_www_client
- encryption_key
signing_key
EOF
# certtool --generate-privkey > client-hostNNN-key.pem
@@ -187,9 +184,8 @@ the ``dns_name`` and ``ip_address`` fields are not included. The
``tls_www_client`` keyword is the key purpose extension to indicate this
certificate is intended for usage in a web client. Although QEMU network
clients are not in fact HTTP clients, setting this key purpose is still
-recommended. The ``encryption_key`` and ``signing_key`` keyword is the
-key usage extension to indicate this certificate is intended for usage
-in the data session.
+recommended. The ``signing_key`` keyword is the key usage extension to
+indicate this certificate is intended for usage in the data session.
The ``client-hostNNN-key.pem`` and ``client-hostNNN-cert.pem`` files
should now be securely copied to the client for which they were
@@ -222,7 +218,6 @@ client and server instructions in one.
ip_address = 2001:24::92
tls_www_server
tls_www_client
- encryption_key
signing_key
EOF
# certtool --generate-privkey > both-hostNNN-key.pem
diff --git a/tests/unit/crypto-tls-x509-helpers.h b/tests/unit/crypto-tls-x509-helpers.h
index 2a0f7c04fd..7e9a508ad6 100644
--- a/tests/unit/crypto-tls-x509-helpers.h
+++ b/tests/unit/crypto-tls-x509-helpers.h
@@ -148,8 +148,7 @@ void test_tls_cleanup(const char *keyfile);
.basicConstraintsIsCA = false, \
.keyUsageEnable = true, \
.keyUsageCritical = true, \
- .keyUsageValue = \
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyUsageValue = GNUTLS_KEY_DIGITAL_SIGNATURE, \
.keyPurposeEnable = true, \
.keyPurposeCritical = true, \
.keyPurposeOID1 = GNUTLS_KP_TLS_WWW_CLIENT, \
@@ -168,8 +167,7 @@ void test_tls_cleanup(const char *keyfile);
.basicConstraintsIsCA = false, \
.keyUsageEnable = true, \
.keyUsageCritical = true, \
- .keyUsageValue = \
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT, \
+ .keyUsageValue = GNUTLS_KEY_DIGITAL_SIGNATURE, \
.keyPurposeEnable = true, \
.keyPurposeCritical = true, \
.keyPurposeOID1 = GNUTLS_KP_TLS_WWW_SERVER, \
diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
index 546225a191..794b0e2605 100644
--- a/tests/unit/test-crypto-tlscredsx509.c
+++ b/tests/unit/test-crypto-tlscredsx509.c
@@ -169,14 +169,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -199,7 +199,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -214,7 +214,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -229,7 +229,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -253,7 +253,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* no-basic */
@@ -267,7 +267,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* Key usage:dig-sig:critical */
@@ -281,7 +281,7 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -306,7 +306,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT |
+ GNUTLS_KEY_DIGITAL_SIGNATURE |
GNUTLS_KEY_KEY_CERT_SIGN,
false, false, NULL, NULL,
0, 0);
@@ -409,7 +409,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT |
+ GNUTLS_KEY_DIGITAL_SIGNATURE |
GNUTLS_KEY_KEY_CERT_SIGN,
false, false, NULL, NULL,
0, 0);
@@ -511,21 +511,21 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(servercertexp1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, -1);
TLS_CERT_REQ(clientcertexp1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, -1);
@@ -549,21 +549,21 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(servercertnew1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
1, 2);
TLS_CERT_REQ(clientcertnew1req, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
1, 2);
@@ -614,14 +614,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertlevel2breq, cacertlevel1breq,
"UK", "qemu client level 2b", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
diff --git a/tests/unit/test-crypto-tlssession.c b/tests/unit/test-crypto-tlssession.c
index 61311cbe6e..d0baf3b304 100644
--- a/tests/unit/test-crypto-tlssession.c
+++ b/tests/unit/test-crypto-tlssession.c
@@ -472,14 +472,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -487,7 +487,7 @@ int main(int argc, char **argv)
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
@@ -506,7 +506,7 @@ int main(int argc, char **argv)
"192.168.122.1", "fec0::dead:beaf",
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
/* This intentionally doesn't replicate */
@@ -515,7 +515,7 @@ int main(int argc, char **argv)
"192.168.122.1", "fec0::dead:beaf",
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
@@ -619,14 +619,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertlevel2breq, cacertlevel1breq,
"UK", "qemu client level 2b", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
diff --git a/tests/unit/test-io-channel-tls.c b/tests/unit/test-io-channel-tls.c
index 6f282ad45d..4e4034af67 100644
--- a/tests/unit/test-io-channel-tls.c
+++ b/tests/unit/test-io-channel-tls.c
@@ -302,14 +302,14 @@ int main(int argc, char **argv)
"UK", "qemu.org", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_SERVER, NULL,
0, 0);
TLS_CERT_REQ(clientcertreq, cacertreq,
"UK", "qemu", NULL, NULL, NULL, NULL,
true, true, false,
true, true,
- GNUTLS_KEY_DIGITAL_SIGNATURE | GNUTLS_KEY_KEY_ENCIPHERMENT,
+ GNUTLS_KEY_DIGITAL_SIGNATURE,
true, true, GNUTLS_KP_TLS_WWW_CLIENT, NULL,
0, 0);
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 7/7] crypto: switch to newer gnutls API for distinguished name
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
` (5 preceding siblings ...)
2025-10-20 14:12 ` [PATCH v3 6/7] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
@ 2025-10-20 14:12 ` Daniel P. Berrangé
6 siblings, 0 replies; 11+ messages in thread
From: Daniel P. Berrangé @ 2025-10-20 14:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P. Berrangé, Eric Blake
The new API automatically allocates the right amount of memory
to hold the distinguished name, avoiding the need to loop and
realloc.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlssession.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index ac38c2121d..00c7ab567b 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -409,20 +409,14 @@ qcrypto_tls_session_check_certificate(QCryptoTLSSession *session,
}
if (i == 0) {
- size_t dnameSize = 1024;
- session->peername = g_malloc(dnameSize);
- requery:
- ret = gnutls_x509_crt_get_dn(cert, session->peername, &dnameSize);
+ gnutls_datum_t dname = {};
+ ret = gnutls_x509_crt_get_dn2(cert, &dname);
if (ret < 0) {
- if (ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
- session->peername = g_realloc(session->peername,
- dnameSize);
- goto requery;
- }
error_setg(errp, "Cannot get client distinguished name: %s",
gnutls_strerror(ret));
goto error;
}
+ session->peername = (char *)g_steal_pointer(&dname.data);
if (session->authzid) {
bool allow;
--
2.50.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/7] crypto: validate an error is reported in test expected fails
2025-10-20 14:12 ` [PATCH v3 3/7] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
@ 2025-10-20 19:40 ` Eric Blake
2025-10-20 20:14 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 11+ messages in thread
From: Eric Blake @ 2025-10-20 19:40 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel
On Mon, Oct 20, 2025 at 03:12:33PM +0100, Daniel P. Berrangé wrote:
> There was a bug where TLS x509 credentials validation failed
> to fill out the Error object. Validate this in the failure
> scenarios.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-tlscredsx509.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/7] crypto: fix error reporting in cert chain checks
2025-10-20 14:12 ` [PATCH v3 4/7] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
@ 2025-10-20 19:50 ` Eric Blake
0 siblings, 0 replies; 11+ messages in thread
From: Eric Blake @ 2025-10-20 19:50 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel
On Mon, Oct 20, 2025 at 03:12:34PM +0100, Daniel P. Berrangé wrote:
> The loop that checks the CA certificate chain can fail to report
> an error message if one of the certs in the chain has an issuer
> that is not present in the chain. In this case, the outer loop
> 'while (checking_issuer)' will terminate after failing to find
> the issuer, and no error message will be reported.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> crypto/tlscredsx509.c | 33 ++++++++++++++++++---------
> tests/unit/test-crypto-tlscredsx509.c | 12 ++++++++++
> 2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/tests/unit/test-crypto-tlscredsx509.c b/tests/unit/test-crypto-tlscredsx509.c
> index 85f51aee1b..5f78fe7d6f 100644
> --- a/tests/unit/test-crypto-tlscredsx509.c
> @@ -652,6 +661,9 @@ int main(int argc, char **argv)
> TLS_TEST_REG(certchainwithexpiredcert, false,
> WORKDIR "cacertchain-with-invalid-ctx.pem",
> clientcertlevel2breq.filename, false);
> + TLS_TEST_REG(chainincomlpete, true,
chainincomplete
> + WORKDIR "cacertchain-incomplete-ctx.pem",
> + servercertlevel3areq.filename, true);
With the typo fixed,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/7] crypto: validate an error is reported in test expected fails
2025-10-20 14:12 ` [PATCH v3 3/7] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
2025-10-20 19:40 ` Eric Blake
@ 2025-10-20 20:14 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-20 20:14 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel; +Cc: Eric Blake
On 20/10/25 16:12, Daniel P. Berrangé wrote:
> There was a bug where TLS x509 credentials validation failed
> to fill out the Error object. Validate this in the failure
> scenarios.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> tests/unit/test-crypto-tlscredsx509.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-10-20 20:15 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 14:12 [PATCH v3 0/7] crypto: misc fixes and improvements to cert handling Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 1/7] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 2/7] crypto: remove extraneous pointer usage in gnutls certs Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 3/7] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
2025-10-20 19:40 ` Eric Blake
2025-10-20 20:14 ` Philippe Mathieu-Daudé
2025-10-20 14:12 ` [PATCH v3 4/7] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
2025-10-20 19:50 ` Eric Blake
2025-10-20 14:12 ` [PATCH v3 5/7] crypto: allow client/server cert chains Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 6/7] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
2025-10-20 14:12 ` [PATCH v3 7/7] crypto: switch to newer gnutls API for distinguished name Daniel P. Berrangé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).