Re: [Libguestfs] [PATCH nbdkit v2 2/7] tls-fallback: Fix filter for new .block_size callback
On Thu, Feb 17, 2022 at 02:36:43PM +0000, Richard W.M. Jones wrote:
This filter doesn't call the next_open function in the non-TLS
case,
and therefore it never opens the plugin. This leaves the internal
state of nbdkit a bit strange. There is no plugin context allocated,
and the last filter in the chain has a context c_next pointer of NULL.
This works, provided we intercept every possible callback, check the
non-TLS case, and prevent it from calling the next function (because
it would dereference the NULL c_next).
To avoid a crash in backend_block_size we must therefore provide a
.block_size callback in this filter.
---
filters/tls-fallback/tls-fallback.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
ACK.
Would you like to squash this in, and/or have me commit this separately?
commit 8c00ca2fe418aeecf0818feed227a72e76d87f18
Author: Eric Blake <eblake(a)redhat.com>
Date: Thu Feb 17 10:24:50 2022 -0600
tls-fallback: Enhance comments about required callbacks
Rich ran into a crash while trying to add a new .block_size callback
reachable during the client connection handshake (via .prepare), and
nearly worked around the crash by reintroducing CVE-2019-14850 instead
of the proper fix of implementing the new callback. Add some comments
to help us avoid a similar regression the next time we add a callback.
diff --git a/filters/tls-fallback/tls-fallback.c b/filters/tls-fallback/tls-fallback.c
index fab9e58b..64e84926 100644
--- a/filters/tls-fallback/tls-fallback.c
+++ b/filters/tls-fallback/tls-fallback.c
@@ -107,7 +107,11 @@ tls_fallback_open (nbdkit_next_open *next, nbdkit_context *nxdata,
const char *exportname, int is_tls)
{
/* We do NOT want to call next() when insecure, because we don't
- * know how long it will take.
+ * know how long it will take. See also CVE-2019-14850 in
+ * nbdkit-security.pod. But that means that this filter must
+ * override every possible callback that can be reached during
+ * handshake, to avoid passing through a non-TLS call to a missing
+ * backend.
*/
if (!is_tls)
return &message; /* See NOT_TLS for this choice of handle */
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org