* [PULL 01/13] crypto: propagate Error object on premature termination
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 02/13] qom: use ERRP_GUARD in user_creatable_complete Daniel P. Berrangé
` (12 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini, Peter Xu
The way that premature termination was handled in TLS connections was
changed to handle an ordering problem during graceful shutdown in the
migration code.
Unfortunately one of the codepaths returned -1 to indicate an error
condition, but failed to set the 'errp' parameter.
This broke error handling in the qio_channel_tls_handshake function,
as the QTask callback would no longer see that an error was raised.
As a result, the client will go on to try to use the already closed
TLS connection, resulting in misleading errors.
This was evidenced in the I/O test 233 which showed changes such as
-qemu-nbd: Certificate does not match the hostname localhost
+qemu-nbd: Failed to read initial magic: Unable to read from socket: Connection reset by peer
Fixes: 7e0c22d585581b8083ffdeb332ea497218665daf
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
crypto/tlssession.c | 8 +++++---
io/channel-tls.c | 13 +++++++------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/crypto/tlssession.c b/crypto/tlssession.c
index ac38c2121d..8c0bf457ad 100644
--- a/crypto/tlssession.c
+++ b/crypto/tlssession.c
@@ -569,8 +569,6 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
if (ret < 0) {
if (ret == GNUTLS_E_AGAIN) {
return QCRYPTO_TLS_SESSION_ERR_BLOCK;
- } else if (ret == GNUTLS_E_PREMATURE_TERMINATION) {
- return QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION;
} else {
if (session->rerr) {
error_propagate(errp, session->rerr);
@@ -580,7 +578,11 @@ qcrypto_tls_session_read(QCryptoTLSSession *session,
"Cannot read from TLS channel: %s",
gnutls_strerror(ret));
}
- return -1;
+ if (ret == GNUTLS_E_PREMATURE_TERMINATION) {
+ return QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION;
+ } else {
+ return -1;
+ }
}
}
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 1fbed4be0c..70fad38d18 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -368,6 +368,7 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
int flags,
Error **errp)
{
+ ERRP_GUARD();
QIOChannelTLS *tioc = QIO_CHANNEL_TLS(ioc);
size_t i;
ssize_t got = 0;
@@ -384,13 +385,13 @@ static ssize_t qio_channel_tls_readv(QIOChannel *ioc,
} else {
return QIO_CHANNEL_ERR_BLOCK;
}
- } else if (ret == QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION) {
- if (qio_channel_tls_allow_premature_termination(tioc, flags)) {
- ret = 0;
- } else {
- return -1;
- }
} else if (ret < 0) {
+ if (ret == QCRYPTO_TLS_SESSION_PREMATURE_TERMINATION &&
+ qio_channel_tls_allow_premature_termination(tioc, flags)) {
+ error_free(*errp);
+ *errp = NULL;
+ return got;
+ }
return -1;
}
got += ret;
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 02/13] qom: use ERRP_GUARD in user_creatable_complete
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 01/13] crypto: propagate Error object on premature termination Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 03/13] tests: use macros for registering char tests for sockets Daniel P. Berrangé
` (11 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Markus Armbruster
With error_propagate, the stack trace from any error_abort/fatal
usage will start from the error_propagate() call, which is largely
useless. Using ERRP_GUARD ensures the stack trace starts from
the origin that reported the error.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
qom/object_interfaces.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 1ffea1a728..415cbee8c5 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -24,13 +24,12 @@
bool user_creatable_complete(UserCreatable *uc, Error **errp)
{
UserCreatableClass *ucc = USER_CREATABLE_GET_CLASS(uc);
- Error *err = NULL;
+ ERRP_GUARD();
if (ucc->complete) {
- ucc->complete(uc, &err);
- error_propagate(errp, err);
+ ucc->complete(uc, errp);
}
- return !err;
+ return !*errp;
}
bool user_creatable_can_be_deleted(UserCreatable *uc)
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 03/13] tests: use macros for registering char tests for sockets
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 01/13] crypto: propagate Error object on premature termination Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 02/13] qom: use ERRP_GUARD in user_creatable_complete Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 04/13] io: release active GSource in TLS channel finalizer Daniel P. Berrangé
` (10 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Peter Maydell
The test-char.c has a couple of helper macros for registering tests that
need to be repeated for both IP and UNIX sockets. One test case was not
using the macro though.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
tests/unit/test-char.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tests/unit/test-char.c b/tests/unit/test-char.c
index f30a39f61f..e156b17329 100644
--- a/tests/unit/test-char.c
+++ b/tests/unit/test-char.c
@@ -1934,7 +1934,9 @@ int main(int argc, char **argv)
g_test_add_data_func("/char/socket/server/mainloop-fdpass/" # name, \
&server3 ##name, char_socket_server_test); \
g_test_add_data_func("/char/socket/server/wait-conn-fdpass/" # name, \
- &server4 ##name, char_socket_server_test)
+ &server4 ##name, char_socket_server_test); \
+ g_test_add_data_func("/char/socket/server/two-clients/" # name, \
+ addr, char_socket_server_two_clients_test)
#define SOCKET_CLIENT_TEST(name, addr) \
static CharSocketClientTestConfig client1 ## name = \
@@ -1974,14 +1976,10 @@ int main(int argc, char **argv)
if (has_ipv4) {
SOCKET_SERVER_TEST(tcp, &tcpaddr);
SOCKET_CLIENT_TEST(tcp, &tcpaddr);
- g_test_add_data_func("/char/socket/server/two-clients/tcp", &tcpaddr,
- char_socket_server_two_clients_test);
}
#ifndef WIN32
SOCKET_SERVER_TEST(unix, &unixaddr);
SOCKET_CLIENT_TEST(unix, &unixaddr);
- g_test_add_data_func("/char/socket/server/two-clients/unix", &unixaddr,
- char_socket_server_two_clients_test);
#endif
g_test_add_func("/char/udp", char_udp_test);
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 04/13] io: release active GSource in TLS channel finalizer
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (2 preceding siblings ...)
2025-10-24 13:19 ` [PULL 03/13] tests: use macros for registering char tests for sockets Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 05/13] io: move websock resource release to close method Daniel P. Berrangé
` (9 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Eric Blake
While code is supposed to call qio_channel_close() before releasing the
last reference on an QIOChannel, this is not guaranteed. QIOChannelFile
and QIOChannelSocket both cleanup resources in their finalizer if the
close operation was missed.
This ensures the TLS channel will do the same failsafe cleanup.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
io/channel-tls.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/io/channel-tls.c b/io/channel-tls.c
index 70fad38d18..ce041795c1 100644
--- a/io/channel-tls.c
+++ b/io/channel-tls.c
@@ -342,6 +342,16 @@ static void qio_channel_tls_finalize(Object *obj)
{
QIOChannelTLS *ioc = QIO_CHANNEL_TLS(obj);
+ if (ioc->hs_ioc_tag) {
+ trace_qio_channel_tls_handshake_cancel(ioc);
+ g_clear_handle_id(&ioc->hs_ioc_tag, g_source_remove);
+ }
+
+ if (ioc->bye_ioc_tag) {
+ trace_qio_channel_tls_bye_cancel(ioc);
+ g_clear_handle_id(&ioc->bye_ioc_tag, g_source_remove);
+ }
+
object_unref(OBJECT(ioc->master));
qcrypto_tls_session_free(ioc->session);
}
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 05/13] io: move websock resource release to close method
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (3 preceding siblings ...)
2025-10-24 13:19 ` [PULL 04/13] io: release active GSource in TLS channel finalizer Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 06/13] io: fix use after free in websocket handshake code Daniel P. Berrangé
` (8 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Eric Blake
The QIOChannelWebsock object releases all its resources in the
finalize callback. This is later than desired, as callers expect
to be able to call qio_channel_close() to fully close a channel
and release resources related to I/O.
The logic in the finalize method is at most a failsafe to handle
cases where a consumer forgets to call qio_channel_close.
This adds equivalent logic to the close method to release the
resources, using g_clear_handle_id/g_clear_pointer to be robust
against repeated invocations. The finalize method is tweaked
so that the GSource is removed before releasing the underlying
channel.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
io/channel-websock.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index 0a8c5c4712..a50a160e18 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -922,13 +922,13 @@ static void qio_channel_websock_finalize(Object *obj)
buffer_free(&ioc->encinput);
buffer_free(&ioc->encoutput);
buffer_free(&ioc->rawinput);
- object_unref(OBJECT(ioc->master));
if (ioc->io_tag) {
g_source_remove(ioc->io_tag);
}
if (ioc->io_err) {
error_free(ioc->io_err);
}
+ object_unref(OBJECT(ioc->master));
}
@@ -1218,6 +1218,15 @@ static int qio_channel_websock_close(QIOChannel *ioc,
QIOChannelWebsock *wioc = QIO_CHANNEL_WEBSOCK(ioc);
trace_qio_channel_websock_close(ioc);
+ buffer_free(&wioc->encinput);
+ buffer_free(&wioc->encoutput);
+ buffer_free(&wioc->rawinput);
+ if (wioc->io_tag) {
+ g_clear_handle_id(&wioc->io_tag, g_source_remove);
+ }
+ if (wioc->io_err) {
+ g_clear_pointer(&wioc->io_err, error_free);
+ }
return qio_channel_close(wioc->master, errp);
}
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 06/13] io: fix use after free in websocket handshake code
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (4 preceding siblings ...)
2025-10-24 13:19 ` [PULL 05/13] io: move websock resource release to close method Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 07/13] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
` (7 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Grant Millar | Cylo, Eric Blake
If the QIOChannelWebsock object is freed while it is waiting to
complete a handshake, a GSource is leaked. This can lead to the
callback firing later on and triggering a use-after-free in the
use of the channel. This was observed in the VNC server with the
following trace from valgrind:
==2523108== Invalid read of size 4
==2523108== at 0x4054A24: vnc_disconnect_start (vnc.c:1296)
==2523108== by 0x4054A24: vnc_client_error (vnc.c:1392)
==2523108== by 0x4068A09: vncws_handshake_done (vnc-ws.c:105)
==2523108== by 0x44863B4: qio_task_complete (task.c:197)
==2523108== by 0x448343D: qio_channel_websock_handshake_io (channel-websock.c:588)
==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398)
==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 (gmain.c:4249)
==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237)
==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287)
==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310)
==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108== Address 0x57a6e0dc is 28 bytes inside a block of size 103,608 free'd
==2523108== at 0x5F2FE43: free (vg_replace_malloc.c:989)
==2523108== by 0x6EDC444: g_free (gmem.c:208)
==2523108== by 0x4053F23: vnc_update_client (vnc.c:1153)
==2523108== by 0x4053F23: vnc_refresh (vnc.c:3225)
==2523108== by 0x4042881: dpy_refresh (console.c:880)
==2523108== by 0x4042881: gui_update (console.c:90)
==2523108== by 0x45EFA1B: timerlist_run_timers.part.0 (qemu-timer.c:562)
==2523108== by 0x45EFC8F: timerlist_run_timers (qemu-timer.c:495)
==2523108== by 0x45EFC8F: qemu_clock_run_timers (qemu-timer.c:576)
==2523108== by 0x45EFC8F: qemu_clock_run_all_timers (qemu-timer.c:663)
==2523108== by 0x45EC765: main_loop_wait (main-loop.c:600)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108== Block was alloc'd at
==2523108== at 0x5F343F3: calloc (vg_replace_malloc.c:1675)
==2523108== by 0x6EE2F81: g_malloc0 (gmem.c:133)
==2523108== by 0x4057DA3: vnc_connect (vnc.c:3245)
==2523108== by 0x448591B: qio_net_listener_channel_func (net-listener.c:54)
==2523108== by 0x6EDB862: UnknownInlinedFun (gmain.c:3398)
==2523108== by 0x6EDB862: g_main_context_dispatch_unlocked.lto_priv.0 (gmain.c:4249)
==2523108== by 0x6EDBAE4: g_main_context_dispatch (gmain.c:4237)
==2523108== by 0x45EC79F: glib_pollfds_poll (main-loop.c:287)
==2523108== by 0x45EC79F: os_host_main_loop_wait (main-loop.c:310)
==2523108== by 0x45EC79F: main_loop_wait (main-loop.c:589)
==2523108== by 0x423A56D: qemu_main_loop (runstate.c:835)
==2523108== by 0x454F300: qemu_default_main (main.c:37)
==2523108== by 0x73D6574: (below main) (libc_start_call_main.h:58)
==2523108==
The above can be reproduced by launching QEMU with
$ qemu-system-x86_64 -vnc localhost:0,websocket=5700
and then repeatedly running:
for i in {1..100}; do
(echo -n "GET / HTTP/1.1" && sleep 0.05) | nc -w 1 localhost 5700 &
done
CVE-2025-11234
Reported-by: Grant Millar | Cylo <rid@cylo.io>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/io/channel-websock.h | 3 ++-
io/channel-websock.c | 22 ++++++++++++++++------
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/include/io/channel-websock.h b/include/io/channel-websock.h
index e180827c57..6700cf8946 100644
--- a/include/io/channel-websock.h
+++ b/include/io/channel-websock.h
@@ -61,7 +61,8 @@ struct QIOChannelWebsock {
size_t payload_remain;
size_t pong_remain;
QIOChannelWebsockMask mask;
- guint io_tag;
+ guint hs_io_tag; /* tracking handshake task */
+ guint io_tag; /* tracking watch task */
Error *io_err;
gboolean io_eof;
uint8_t opcode;
diff --git a/io/channel-websock.c b/io/channel-websock.c
index a50a160e18..cb4dafdebb 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -545,6 +545,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
+ wioc->hs_io_tag = 0;
return FALSE;
}
@@ -560,6 +561,7 @@ static gboolean qio_channel_websock_handshake_send(QIOChannel *ioc,
trace_qio_channel_websock_handshake_complete(ioc);
qio_task_complete(task);
}
+ wioc->hs_io_tag = 0;
return FALSE;
}
trace_qio_channel_websock_handshake_pending(ioc, G_IO_OUT);
@@ -586,6 +588,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
trace_qio_channel_websock_handshake_fail(ioc, error_get_pretty(err));
qio_task_set_error(task, err);
qio_task_complete(task);
+ wioc->hs_io_tag = 0;
return FALSE;
}
if (ret == 0) {
@@ -597,7 +600,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
error_propagate(&wioc->io_err, err);
trace_qio_channel_websock_handshake_reply(ioc);
- qio_channel_add_watch(
+ wioc->hs_io_tag = qio_channel_add_watch(
wioc->master,
G_IO_OUT,
qio_channel_websock_handshake_send,
@@ -907,11 +910,12 @@ void qio_channel_websock_handshake(QIOChannelWebsock *ioc,
trace_qio_channel_websock_handshake_start(ioc);
trace_qio_channel_websock_handshake_pending(ioc, G_IO_IN);
- qio_channel_add_watch(ioc->master,
- G_IO_IN,
- qio_channel_websock_handshake_io,
- task,
- NULL);
+ ioc->hs_io_tag = qio_channel_add_watch(
+ ioc->master,
+ G_IO_IN,
+ qio_channel_websock_handshake_io,
+ task,
+ NULL);
}
@@ -922,6 +926,9 @@ static void qio_channel_websock_finalize(Object *obj)
buffer_free(&ioc->encinput);
buffer_free(&ioc->encoutput);
buffer_free(&ioc->rawinput);
+ if (ioc->hs_io_tag) {
+ g_source_remove(ioc->hs_io_tag);
+ }
if (ioc->io_tag) {
g_source_remove(ioc->io_tag);
}
@@ -1221,6 +1228,9 @@ static int qio_channel_websock_close(QIOChannel *ioc,
buffer_free(&wioc->encinput);
buffer_free(&wioc->encoutput);
buffer_free(&wioc->rawinput);
+ if (wioc->hs_io_tag) {
+ g_clear_handle_id(&wioc->hs_io_tag, g_source_remove);
+ }
if (wioc->io_tag) {
g_clear_handle_id(&wioc->io_tag, g_source_remove);
}
--
2.50.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PULL 07/13] crypto: only verify CA certs in chain of trust
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (5 preceding siblings ...)
2025-10-24 13:19 ` [PULL 06/13] io: fix use after free in websocket handshake code Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 08/13] crypto: remove extraneous pointer usage in gnutls certs Daniel P. Berrangé
` (6 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
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] 17+ messages in thread* [PULL 08/13] crypto: remove extraneous pointer usage in gnutls certs
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (6 preceding siblings ...)
2025-10-24 13:19 ` [PULL 07/13] crypto: only verify CA certs in chain of trust Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 09/13] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
` (5 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Philippe Mathieu-Daudé, Eric Blake
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] 17+ messages in thread* [PULL 09/13] crypto: validate an error is reported in test expected fails
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (7 preceding siblings ...)
2025-10-24 13:19 ` [PULL 08/13] crypto: remove extraneous pointer usage in gnutls certs Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 10/13] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
` (4 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
Eric Blake, Philippe Mathieu-Daudé
There was a bug where TLS x509 credentials validation failed
to fill out the Error object. Validate this in the failure
scenarios.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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] 17+ messages in thread* [PULL 10/13] crypto: fix error reporting in cert chain checks
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (8 preceding siblings ...)
2025-10-24 13:19 ` [PULL 09/13] crypto: validate an error is reported in test expected fails Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 11/13] crypto: allow client/server cert chains Daniel P. Berrangé
` (3 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
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.
Reviewed-by: Eric Blake <eblake@redhat.com>
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..7c5df32bcc 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(chainincomplete, 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] 17+ messages in thread* [PULL 11/13] crypto: allow client/server cert chains
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (9 preceding siblings ...)
2025-10-24 13:19 ` [PULL 10/13] crypto: fix error reporting in cert chain checks Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 12/13] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
` (2 subsequent siblings)
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
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 7c5df32bcc..96ad4e741b 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] 17+ messages in thread* [PULL 12/13] crypto: stop requiring "key encipherment" usage in x509 certs
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (10 preceding siblings ...)
2025-10-24 13:19 ` [PULL 11/13] crypto: allow client/server cert chains Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-27 10:20 ` Daniel P. Berrangé
2025-10-24 13:19 ` [PULL 13/13] crypto: switch to newer gnutls API for distinguished name Daniel P. Berrangé
2025-10-24 15:38 ` [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
13 siblings, 1 reply; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
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 96ad4e741b..a5f21728d4 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] 17+ messages in thread* Re: [PULL 12/13] crypto: stop requiring "key encipherment" usage in x509 certs
2025-10-24 13:19 ` [PULL 12/13] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
@ 2025-10-27 10:20 ` Daniel P. Berrangé
0 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-27 10:20 UTC (permalink / raw)
To: qemu-devel, qemu-stable; +Cc: Eduardo Habkost, Paolo Bonzini, Eric Blake
This patch is relevant to pick for all stable branches.
Not critical, but a "nice to have" fix if it cherry picks
cleanly.
On Fri, Oct 24, 2025 at 02:19:36PM +0100, Daniel P. Berrangé wrote:
> 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 96ad4e741b..a5f21728d4 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
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PULL 13/13] crypto: switch to newer gnutls API for distinguished name
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (11 preceding siblings ...)
2025-10-24 13:19 ` [PULL 12/13] crypto: stop requiring "key encipherment" usage in x509 certs Daniel P. Berrangé
@ 2025-10-24 13:19 ` Daniel P. Berrangé
2025-10-24 15:38 ` [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 13:19 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Eduardo Habkost, Paolo Bonzini,
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 8c0bf457ad..92fe4f0380 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] 17+ messages in thread* Re: [PULL 00/13] Next crypto & I/O patches
2025-10-24 13:19 [PULL 00/13] Next crypto & I/O patches Daniel P. Berrangé
` (12 preceding siblings ...)
2025-10-24 13:19 ` [PULL 13/13] crypto: switch to newer gnutls API for distinguished name Daniel P. Berrangé
@ 2025-10-24 15:38 ` Daniel P. Berrangé
13 siblings, 0 replies; 17+ messages in thread
From: Daniel P. Berrangé @ 2025-10-24 15:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Eduardo Habkost, Paolo Bonzini
Self-NACK
I've discovered this fails on CFI enabled builds, because
gnutls_malloc/gnutls_free are incompatible with CFI due
to requiring bad function pointer casts. Fortunately
gnutls 3.3.0 dropped support for replacing the system
allocators, so we can assume C library malloc/free are
acceptable, just as with glib.
I'll post a v2 PULL shortly.
On Fri, Oct 24, 2025 at 02:19:24PM +0100, Daniel P. Berrangé wrote:
> The following changes since commit e8779f3d1509cd07620c6166a9a280376e01ff2f:
>
> Merge tag 'pull-riscv-to-apply-20251024' of https://github.com/alistair23/qemu into staging (2025-10-24 10:53:02 +0200)
>
> are available in the Git repository at:
>
> https://gitlab.com/berrange/qemu tags/next-pr-pull-request
>
> for you to fetch changes up to 608be310a9c0a8501480a8ba76de3df8feaaf0d8:
>
> crypto: switch to newer gnutls API for distinguished name (2025-10-24 13:06:05 +0100)
>
> ----------------------------------------------------------------
> Merge misc, crypto and I/O subsystems changes
>
> * Fix use after free in websocket handshake (CVE-2025-11234)
> * Improved stack traces fatal errors/aborts raised for
> user creatable objects
> * Stop requiring 'key encipherment' usage in x509 certs
> * Only sanity check CA certs needed in the chain of trust
> * Allow intermediate CA certs to be present in client/server
> cert file
> * Fix regression propagating errors in premature shutdown
> of TLS connections
>
> ----------------------------------------------------------------
>
> Daniel P. Berrangé (11):
> crypto: propagate Error object on premature termination
> qom: use ERRP_GUARD in user_creatable_complete
> tests: use macros for registering char tests for sockets
> io: release active GSource in TLS channel finalizer
> io: move websock resource release to close method
> io: fix use after free in websocket handshake code
> 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 | 20 +--
> docs/system/tls.rst | 13 +-
> include/io/channel-websock.h | 3 +-
> io/channel-tls.c | 23 ++-
> io/channel-websock.c | 33 +++-
> qom/object_interfaces.c | 7 +-
> tests/unit/crypto-tls-x509-helpers.h | 6 +-
> tests/unit/test-char.c | 8 +-
> tests/unit/test-crypto-tlscredsx509.c | 155 +++++++++++++++---
> tests/unit/test-crypto-tlssession.c | 14 +-
> tests/unit/test-io-channel-tls.c | 4 +-
> 12 files changed, 336 insertions(+), 173 deletions(-)
>
> --
> 2.50.1
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 17+ messages in thread