rpms/libnbd
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Rebase to new stable branch version 1.10.5

Browse Source 
resolves: rhbz#2011708
New upstream API to control initialization of pread buffer
related: rhbz#2046194
This commit is contained in:
Richard W.M. Jones 2022-02-10 16:05:22 +00:00
parent f50782c8be
commit 39fc500eeb
3 changed files with 476 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

469
0001-api-Add-new-API-nbd_set_pread_initialize.patch Normal file
View File

@ -0,0 +1,469 @@
From c79706af4e7475bf58861a143b77b77a54e7a1cd Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 9 Feb 2022 15:39:49 -0600
Subject: [PATCH] api: Add new API nbd_set_pread_initialize()
The recent patch series for CVE-2022-0485 demonstrated that when
applications using libnbd are not careful about error checking, the
difference on whether a data leak is at least sanitized (all zeroes,
partial reads, or data leftover from a prior read) vs. a dangerous
information leak (uninitialized data from the heap) was partly under
libnbd's control. The previous two patches changed libnbd to always
sanitize, as a security hardening technique that prevents heap leaks
no matter how buggy the client app is. But a blind memset() also adds
an execution delay, even if it doesn't show up as the hot spot in our
profiling when compared to the time spent with network traffic.
At any rate, if client apps choose to pre-initialize their buffers, or
otherwise audit their code to take on their own risk about not
dereferencing a buffer on failure paths, then the time spent by libnbd
doing memset() is wasted; so it is worth adding a knob to let a user
opt in to faster execution at the expense of giving up our memset()
hardening on their behalf.
In addition to adding two new APIs, this patch also causes changes to
the four existing APIs nbd_{aio_,}pread{,_structured}, with those
generated lib/api.c changes looking like:
| --- lib/api.c.bak 2022-02-10 08:17:09.973381979 -0600
| +++ lib/api.c 2022-02-10 08:22:27.503428024 -0600
| @@ -2871,7 +2914,8 @@ nbd_pread (struct nbd_handle *h, void *b
| debug (h, "enter: buf=<buf> count=%zu offset=%" PRIu64 " flags=0x%x", count, offset, flags);
| }
|
| - memset (buf, 0, count);
| + if (h->pread_initialize)
| + memset (buf, 0, count);
| if (unlikely (!pread_in_permitted_state (h))) {
| ret = -1;
| goto out;
Message-Id: <20220209220726.1902761-4-eblake@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
[eblake: enhance commit message to show generated file diff, mention CVE
in doc text]
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
(cherry picked from commit e0953cb71250947bb97b25e34ff1ea34bd504bf3)
---
generator/API.ml | 90 ++++++++++++++++---
generator/C.ml | 3 +-
.../libnbd/libnbd_110_defaults_test.go | 10 ++-
.../libnbd_120_set_non_defaults_test.go | 12 +++
lib/handle.c | 17 +++-
lib/internal.h | 5 +-
ocaml/tests/test_110_defaults.ml | 4 +-
ocaml/tests/test_120_set_non_defaults.ml | 5 +-
python/t/110-defaults.py | 3 +-
python/t/120-set-non-defaults.py | 4 +-
tests/errors.c | 25 +++++-
11 files changed, 156 insertions(+), 22 deletions(-)
diff --git a/generator/API.ml b/generator/API.ml
index d8df7c8..00ab34f 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -778,6 +778,49 @@ the time of compilation.";
Link "aio_is_created"; Link "aio_is_ready"];
};
+ "set_pread_initialize", {
+ default_call with
+ args = [Bool "request"]; ret = RErr;
+ shortdesc = "control whether libnbd pre-initializes read buffers";
+ longdesc = "\
+By default, libnbd will pre-initialize the contents of a buffer
+passed to calls such as L<nbd_pread(3)> to all zeroes prior to
+checking for any other errors, so that even if a client application
+passed in an uninitialized buffer but fails to check for errors, it
+will not result in a potential security risk caused by an accidental
+leak of prior heap contents (see CVE-2022-0485 in
+L<libnbd-security(3)> for an example of a security hole in an
+application built against an earlier version of libnbd that lacked
+consistent pre-initialization). However, for a client application
+that has audited that an uninitialized buffer is never dereferenced,
+or which performs its own pre-initialization, libnbd's sanitization
+efforts merely pessimize performance (although the time spent in
+pre-initialization may pale in comparison to time spent waiting on
+network packets).
+
+Calling this function with C<request> set to false tells libnbd to
+skip the buffer initialization step in read commands.";
+ see_also = [Link "get_pread_initialize";
+ Link "set_strict_mode";
+ Link "pread"; Link "pread_structured"; Link "aio_pread";
+ Link "aio_pread_structured"];
+ };
+
+ "get_pread_initialize", {
+ default_call with
+ args = []; ret = RBool;
+ may_set_error = false;
+ shortdesc = "see whether libnbd pre-initializes read buffers";
+ longdesc = "\
+Return whether libnbd performs a pre-initialization of a buffer passed
+to L<nbd_pread(3)> and similar to all zeroes, as set by
+L<nbd_set_pread_initialize(3)>.";
+ see_also = [Link "set_pread_initialize";
+ Link "set_strict_mode";
+ Link "pread"; Link "pread_structured"; Link "aio_pread";
+ Link "aio_pread_structured"];
+ };
+
"set_strict_mode", {
default_call with
args = [ Flags ("flags", strict_flags) ]; ret = RErr;
@@ -1825,11 +1868,16 @@ C<LIBNBD_CMD_FLAG_DF>.
The C<flags> parameter must be C<0> for now (it exists for future NBD
protocol extensions).
-Note that if this command fails, it is unspecified whether the contents
-of C<buf> will read as zero or as partial results from the server."
+Note that if this command fails, and L<nbd_get_pread_initialize(3)>
+returns true, then libnbd sanitized C<buf>, but it is unspecified
+whether the contents of C<buf> will read as zero or as partial results
+from the server. If L<nbd_get_pread_initialize(3)> returns false,
+then libnbd did not sanitize C<buf>, and the contents are undefined
+on failure."
^ strict_call_description;
see_also = [Link "aio_pread"; Link "pread_structured";
- Link "get_block_size"; Link "set_strict_mode"];
+ Link "get_block_size"; Link "set_strict_mode";
+ Link "set_pread_initialize"];
example = Some "examples/fetch-first-sector.c";
};
@@ -1907,12 +1955,16 @@ more than one fragment (if that is supported - some servers cannot do
this, see L<nbd_can_df(3)>). Libnbd does not validate that the server
actually obeys the flag.
-Note that if this command fails, it is unspecified whether the contents
-of C<buf> will read as zero or as partial results from the server."
+Note that if this command fails, and L<nbd_get_pread_initialize(3)>
+returns true, then libnbd sanitized C<buf>, but it is unspecified
+whether the contents of C<buf> will read as zero or as partial results
+from the server. If L<nbd_get_pread_initialize(3)> returns false,
+then libnbd did not sanitize C<buf>, and the contents are undefined
+on failure."
^ strict_call_description;
see_also = [Link "can_df"; Link "pread";
Link "aio_pread_structured"; Link "get_block_size";
- Link "set_strict_mode"];
+ Link "set_strict_mode"; Link "set_pread_initialize"];
};
"pwrite", {
@@ -2420,14 +2472,19 @@ as described in L<libnbd(3)/Completion callbacks>.
Note that you must ensure C<buf> is valid until the command has
completed. Furthermore, if the C<error> parameter to
C<completion_callback> is set or if L<nbd_aio_command_completed(3)>
-reports failure, it is unspecified whether the contents
-of C<buf> will read as zero or as partial results from the server.
+reports failure, and if L<nbd_get_pread_initialize(3)> returns true,
+then libnbd sanitized C<buf>, but it is unspecified whether the
+contents of C<buf> will read as zero or as partial results from the
+server. If L<nbd_get_pread_initialize(3)> returns false, then
+libnbd did not sanitize C<buf>, and the contents are undefined
+on failure.
+
Other parameters behave as documented in L<nbd_pread(3)>."
^ strict_call_description;
example = Some "examples/aio-connect-read.c";
see_also = [SectionLink "Issuing asynchronous commands";
Link "aio_pread_structured"; Link "pread";
- Link "set_strict_mode"];
+ Link "set_strict_mode"; Link "set_pread_initialize"];
};
"aio_pread_structured", {
@@ -2449,13 +2506,18 @@ as described in L<libnbd(3)/Completion callbacks>.
Note that you must ensure C<buf> is valid until the command has
completed. Furthermore, if the C<error> parameter to
C<completion_callback> is set or if L<nbd_aio_command_completed(3)>
-reports failure, it is unspecified whether the contents
-of C<buf> will read as zero or as partial results from the server.
+reports failure, and if L<nbd_get_pread_initialize(3)> returns true,
+then libnbd sanitized C<buf>, but it is unspecified whether the
+contents of C<buf> will read as zero or as partial results from the
+server. If L<nbd_get_pread_initialize(3)> returns false, then
+libnbd did not sanitize C<buf>, and the contents are undefined
+on failure.
+
Other parameters behave as documented in L<nbd_pread_structured(3)>."
^ strict_call_description;
see_also = [SectionLink "Issuing asynchronous commands";
Link "aio_pread"; Link "pread_structured";
- Link "set_strict_mode"];
+ Link "set_strict_mode"; Link "set_pread_initialize"];
};
"aio_pwrite", {
@@ -3093,6 +3155,10 @@ let first_version = [
"get_private_data", (1, 8);
"get_uri", (1, 8);
+ (* Added in 1.11.x development cycle, will be stable and supported in 1.12. *)
+ "set_pread_initialize", (1, 12);
+ "get_pread_initialize", (1, 12);
+
(* These calls are proposed for a future version of libnbd, but
* have not been added to any released version so far.
"get_tls_certificates", (1, ??);
diff --git a/generator/C.ml b/generator/C.ml
index 4a5bb58..2b6198c 100644
--- a/generator/C.ml
+++ b/generator/C.ml
@@ -496,7 +496,8 @@ let generate_lib_api_c () =
function
| BytesOut (n, count)
| BytesPersistOut (n, count) ->
- pr " memset (%s, 0, %s);\n" n count
+ pr " if (h->pread_initialize)\n";
+ pr " memset (%s, 0, %s);\n" n count
| _ -> ()
) args;
diff --git a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
index b3ceb45..ca7c1c4 100644
--- a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
+++ b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go
@@ -1,5 +1,5 @@
/* libnbd golang tests
- * Copyright (C) 2013-2021 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -59,6 +59,14 @@ func Test110Defaults(t *testing.T) {
t.Fatalf("unexpected structured replies state")
}
+ init, err := h.GetPreadInitialize()
+ if err != nil {
+ t.Fatalf("could not get pread initialize state: %s", err)
+ }
+ if init != true {
+ t.Fatalf("unexpected pread initialize state")
+ }
+
flags, err := h.GetHandshakeFlags()
if err != nil {
t.Fatalf("could not get handshake flags: %s", err)
diff --git a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
index f112456..029f0db 100644
--- a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
+++ b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go
@@ -93,6 +93,18 @@ func Test120SetNonDefaults(t *testing.T) {
t.Fatalf("unexpected structured replies state")
}
+ err = h.SetPreadInitialize(false)
+ if err != nil {
+ t.Fatalf("could not set pread initialize state: %s", err)
+ }
+ init, err := h.GetPreadInitialize()
+ if err != nil {
+ t.Fatalf("could not get pread initialize state: %s", err)
+ }
+ if init != false {
+ t.Fatalf("unexpected pread initialize state")
+ }
+
err = h.SetHandshakeFlags(HANDSHAKE_FLAG_MASK + 1)
if err == nil {
t.Fatalf("expect failure for out-of-range flags")
diff --git a/lib/handle.c b/lib/handle.c
index 67aa875..ac6c16e 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -1,5 +1,5 @@
/* NBD client library in userspace
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -64,6 +64,7 @@ nbd_create (void)
h->unique = 1;
h->tls_verify_peer = true;
h->request_sr = true;
+ h->pread_initialize = true;
h->uri_allow_transports = LIBNBD_ALLOW_TRANSPORT_MASK;
h->uri_allow_tls = LIBNBD_TLS_ALLOW;
@@ -393,6 +394,20 @@ nbd_unlocked_get_handshake_flags (struct nbd_handle *h)
return h->gflags;
}
+int
+nbd_unlocked_set_pread_initialize (struct nbd_handle *h, bool request)
+{
+ h->pread_initialize = request;
+ return 0;
+}
+
+/* NB: may_set_error = false. */
+int
+nbd_unlocked_get_pread_initialize (struct nbd_handle *h)
+{
+ return h->pread_initialize;
+}
+
int
nbd_unlocked_set_strict_mode (struct nbd_handle *h, uint32_t flags)
{
diff --git a/lib/internal.h b/lib/internal.h
index 0e205ab..525499a 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -1,5 +1,5 @@
/* nbd client library in userspace: internal definitions
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -123,6 +123,9 @@ struct nbd_handle {
/* Full info mode. */
bool full_info;
+ /* Sanitization for pread. */
+ bool pread_initialize;
+
/* Global flags from the server. */
uint16_t gflags;
diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml
index b36949f..04aa744 100644
--- a/ocaml/tests/test_110_defaults.ml
+++ b/ocaml/tests/test_110_defaults.ml
@@ -1,6 +1,6 @@
(* hey emacs, this is OCaml code: -*- tuareg -*- *)
(* libnbd OCaml test case
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -28,6 +28,8 @@ let () =
assert (tls = NBD.TLS.DISABLE);
let sr = NBD.get_request_structured_replies nbd in
assert (sr = true);
+ let init = NBD.get_pread_initialize nbd in
+ assert (init = true);
let flags = NBD.get_handshake_flags nbd in
assert (flags = NBD.HANDSHAKE_FLAG.mask);
let opt = NBD.get_opt_mode nbd in
diff --git a/ocaml/tests/test_120_set_non_defaults.ml b/ocaml/tests/test_120_set_non_defaults.ml
index 67928bb..f949807 100644
--- a/ocaml/tests/test_120_set_non_defaults.ml
+++ b/ocaml/tests/test_120_set_non_defaults.ml
@@ -1,6 +1,6 @@
(* hey emacs, this is OCaml code: -*- tuareg -*- *)
(* libnbd OCaml test case
- * Copyright (C) 2013-2020 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -42,6 +42,9 @@ let () =
NBD.set_request_structured_replies nbd false;
let sr = NBD.get_request_structured_replies nbd in
assert (sr = false);
+ NBD.set_pread_initialize nbd false;
+ let init = NBD.get_pread_initialize nbd in
+ assert (init = false);
(try
NBD.set_handshake_flags nbd [ NBD.HANDSHAKE_FLAG.UNKNOWN 2 ];
assert false
diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py
index fb961cf..a4262da 100644
--- a/python/t/110-defaults.py
+++ b/python/t/110-defaults.py
@@ -1,5 +1,5 @@
# libnbd Python bindings
-# Copyright (C) 2010-2020 Red Hat Inc.
+# Copyright (C) 2010-2022 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -22,5 +22,6 @@ assert h.get_export_name() == ""
assert h.get_full_info() is False
assert h.get_tls() == nbd.TLS_DISABLE
assert h.get_request_structured_replies() is True
+assert h.get_pread_initialize() is True
assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK
assert h.get_opt_mode() is False
diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py
index 3da0c23..e71c6ad 100644
--- a/python/t/120-set-non-defaults.py
+++ b/python/t/120-set-non-defaults.py
@@ -1,5 +1,5 @@
# libnbd Python bindings
-# Copyright (C) 2010-2020 Red Hat Inc.
+# Copyright (C) 2010-2022 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -33,6 +33,8 @@ if h.supports_tls():
assert h.get_tls() == nbd.TLS_ALLOW
h.set_request_structured_replies(False)
assert h.get_request_structured_replies() is False
+h.set_pread_initialize(False)
+assert h.get_pread_initialize() is False
try:
h.set_handshake_flags(nbd.HANDSHAKE_FLAG_MASK + 1)
assert False
diff --git a/tests/errors.c b/tests/errors.c
index f597b7e..0298da8 100644
--- a/tests/errors.c
+++ b/tests/errors.c
@@ -213,7 +213,15 @@ main (int argc, char *argv[])
}
- /* Issue a connected command when not connected. */
+ /* Issue a connected command when not connected. pread_initialize defaults
+ * to set.
+ */
+ if (nbd_get_pread_initialize (nbd) != 1) {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_get_pread_initialize gave unexpected result\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
buf[0] = '1';
if (nbd_pread (nbd, buf, 512, 0, 0) != -1) {
fprintf (stderr, "%s: test failed: "
@@ -294,7 +302,14 @@ main (int argc, char *argv[])
}
check (EINVAL, "nbd_aio_command_completed: ");
- /* Read from an invalid offset, client-side */
+ /* Read from an invalid offset, client-side. When pread_initialize is off,
+ * libnbd should not have touched our buffer.
+ */
+ if (nbd_set_pread_initialize (nbd, false) == -1) {
+ fprintf (stderr, "%s\n", nbd_get_error ());
+ exit (EXIT_FAILURE);
+ }
+ buf[0] = '1';
strict = nbd_get_strict_mode (nbd) | LIBNBD_STRICT_BOUNDS;
if (nbd_set_strict_mode (nbd, strict) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
@@ -307,6 +322,12 @@ main (int argc, char *argv[])
exit (EXIT_FAILURE);
}
check (EINVAL, "nbd_aio_pread: ");
+ if (buf[0] != '1') {
+ fprintf (stderr, "%s: test failed: "
+ "nbd_pread incorrectly sanitized buffer on client-side error\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
/* We guarantee callbacks will be freed even on all error paths. */
if (nbd_aio_pread_structured (nbd, buf, 512, -1,
--
2.31.1

9
libnbd.spec
View File

@ -8,7 +8,7 @@
%global source_directory 1.10-stable
Name: libnbd
Version: 1.10.4
Version: 1.10.5
Release: 1%{?dist}
Summary: NBD client library in userspace
@ -29,7 +29,7 @@ Source3: copy-patches.sh
# https://gitlab.com/nbdkit/libnbd/-/commits/rhel-9.0/
# Patches.
#(nothing)
Patch0001: 0001-api-Add-new-API-nbd_set_pread_initialize.patch
%if 0%{patches_touch_autotools}
BuildRequires: autoconf, automake, libtool
@ -323,12 +323,13 @@ make %{?_smp_mflags} check || {
%changelog
* Sat Feb 05 2022 Richard W.M. Jones <rjones@redhat.com> - 1.10.4-1
- Rebase to new stable branch version 1.10.4
* Thu Feb 10 2022 Richard W.M. Jones <rjones@redhat.com> - 1.10.5-1
- Rebase to new stable branch version 1.10.5
resolves: rhbz#2011708
- Map uint32_t to OCaml int64 to avoid signedness problems
resolves: rhbz#2040610
- CVE-2022-0485 nbdcopy destination image corruption
- New upstream API to control initialization of pread buffer
resolves: rhbz#2046194
* Mon Aug 09 2021 Mohan Boddu <mboddu@redhat.com> - 1.8.2-3

4
sources
View File

@ -1,2 +1,2 @@
SHA512 (libnbd-1.10.4.tar.gz) = 6b690e75a7b9c7754b461bb085fa3aa7120313362e16082bed0855e04f68c34e6ee3c65882437d554c536736857a13e746da482c972e560a6cb100ea48c6f046
SHA512 (libnbd-1.10.4.tar.gz.sig) = ef58f8eea45673a80e84a33efd1f03576b13c49e244102b2c36d07d22773d5ac5e73e3ead6a2a9f330793af10751fa8c553aebba5e56a56000be68b8820becd8
SHA512 (libnbd-1.10.5.tar.gz) = 036c13d29bc1490d44051db79dcce01c5f61d8b2108f8ffa28eb5f225c75a622f7e490e7a5a4dff4eba14b8ff2535ba1d4183cf8e4a5569d6201c72bfea4bb11
SHA512 (libnbd-1.10.5.tar.gz.sig) = 9f1b16da76038e0920f5d01e2cf6313e08d683fc368f45785e53dcd30d91538794168502900dc444a63ee020b8ffbd53b59dffc8aca5c17ae0dc6adf3640b8c8