Compare commits

..

2 Commits

Author SHA1 Message Date
Miroslav Rezanina
6f5af8f8c7 * Tue Apr 09 2024 Miroslav Rezanina <mrezanin@redhat.com> - 1.20.0-1 2024-04-11 02:47:56 +00:00
Eric Blake
552cf308b2 Backport unit test of recent libnbd API addition
resolves: RHEL-16292
2023-11-13 17:18:38 -06:00
6 changed files with 47 additions and 371 deletions

View File

@ -1,2 +1,2 @@
4f99e6f21edffe62b394aa9c7fb68149e6d4d5e4 libnbd-1.18.1.tar.gz
f9a431cb1f235dabb4482f961da8d19a9e3719c8 libnbd-1.18.1.tar.gz.sig
175ea6036c2c7a451a53b81a2ecba5a029582ddc libnbd-1.20.0.tar.gz
7846f8e741a4dad4b6f44670a29bdd0384d434c1 libnbd-1.20.0.tar.gz.sig

View File

@ -1,88 +0,0 @@
From 4451e5b61ca07771ceef3e012223779e7a0c7701 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Mon, 30 Oct 2023 12:50:53 -0500
Subject: [PATCH] generator: Fix assertion in ext-mode BLOCK_STATUS,
CVE-2023-5871
Another round of fuzz testing revealed that when a server negotiates
extended headers and replies with a 64-bit flag value where the client
used the 32-bit API command, we were correctly flagging the server's
response as being an EOVERFLOW condition, but then immediately failing
in an assertion failure instead of reporting it to the application.
The following one-byte change to qemu.git at commit fd9a38fd43 allows
the creation of an intentionally malicious server:
| diff --git i/nbd/server.c w/nbd/server.c
| index 859c163d19f..32e1e771a95 100644
| --- i/nbd/server.c
| +++ w/nbd/server.c
| @@ -2178,7 +2178,7 @@ static void nbd_extent_array_convert_to_be(NBDExtentArray *ea)
|
| for (i = 0; i < ea->count; i++) {
| ea->extents[i].length = cpu_to_be64(ea->extents[i].length);
| - ea->extents[i].flags = cpu_to_be64(ea->extents[i].flags);
| + ea->extents[i].flags = ~cpu_to_be64(ea->extents[i].flags);
| }
| }
and can then be detected with the following command line:
$ nbdsh -c - <<\EOF
> def f(a,b,c,d):
> pass
>
> h.connect_systemd_socket_activation(["/path/to/bad/qemu-nbd",
> "-r", "-f", "raw", "TODO"])
> h.block_staus(h.get_size(), 0, f)
> EOF
nbdsh: generator/states-reply-chunk.c:626: enter_STATE_REPLY_CHUNK_REPLY_RECV_BS_ENTRIES: Assertion `(len | flags) <= UINT32_MAX' failed.
Aborted (core dumped)
whereas a fixed libnbd will give:
nbdsh: command line script failed: nbd_block_status: block-status: command failed: Value too large for defined data type
We can either relax the assertion (by changing to 'assert ((len |
flags) <= UINT32_MAX || cmd->error)'), or intentionally truncate flags
to make the existing assertion reliable. This patch goes with the
latter approach.
Sadly, this crash is possible in all existing 1.18.x stable releases,
if they were built with assertions enabled (most distros do this by
default), meaning a malicious server has an easy way to cause a Denial
of Service attack by triggering the assertion failure in vulnerable
clients, so we have assigned this CVE-2023-5871. Mitigating factors:
the crash only happens for a server that sends a 64-bit status block
reply (no known production servers do so; qemu 8.2 will be the first
known server to support extended headers, but it is not yet released);
and as usual, a client can use TLS to guarantee it is connecting only
to a known-safe server. If libnbd is compiled without assertions,
there is no crash or other mistaken behavior; and when assertions are
enabled, the attacker cannot accomplish anything more than a denial of
service.
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Fixes: 20dadb0e10 ("generator: Prepare for extent64 callback", v1.17.4)
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 177308adb17e81fce7c0f2b2fcf655c5c0b6a4d6)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
generator/states-reply-chunk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 5a31c19..8ab7e8b 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -600,6 +600,7 @@ STATE_MACHINE {
break; /* Skip this and later extents; we already made progress */
/* Expose this extent as an error; we made no progress */
cmd->error = cmd->error ? : EOVERFLOW;
+ flags = (uint32_t)flags;
}
}
--
2.39.3

View File

@ -1,34 +0,0 @@
From c39e31b7a20c7dc8aa12c5fa3f1742824e1e0c76 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 9 Nov 2023 09:40:30 +0000
Subject: [libnbd PATCH] docs: Fix incorrect xref in libnbd-release-notes for
1.18
Content-type: text/plain
LIBNBD_STRICT_AUTO_FLAG was added to nbd_set_strict_mode(3).
Reported-by: Vera Wu
(cherry picked from commit 4fef3dbc07e631fce58487d25d991e83bbb424b1)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
docs/libnbd-release-notes-1.18.pod | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/libnbd-release-notes-1.18.pod b/docs/libnbd-release-notes-1.18.pod
index 935fab11..836ebe19 100644
--- a/docs/libnbd-release-notes-1.18.pod
+++ b/docs/libnbd-release-notes-1.18.pod
@@ -84,8 +84,8 @@ Golang, OCaml and Python language bindings (Eric Blake).
L<nbd_shutdown(3)> now works correctly when in opt mode (Eric Blake).
-L<nbd_set_string(3)> adds C<LIBNBD_STRICT_AUTO_FLAG> which allows the
-client to test how servers behave when the payload length flag is
+L<nbd_set_strict_mode(3)> adds C<LIBNBD_STRICT_AUTO_FLAG> which allows
+the client to test how servers behave when the payload length flag is
adjusted (Eric Blake).
=head2 Protocol
--
2.41.0

View File

@ -1,206 +0,0 @@
From 32cb9ab9f1701b1a1a826b48f2083cb75adf1e87 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 9 Nov 2023 20:11:08 -0600
Subject: [libnbd PATCH] tests: Check behavior of
nbd_set_strict_mode(STRICT_AUTO_FLAG)
Content-type: text/plain
While developing extended header support for qemu 8.2, I needed a way
to make libnbd quickly behave as a non-compliant client to test corner
cases in qemu's server code; so I wrote commit 5c1dae9236 ("api: Add
LIBNBD_STRICT_AUTO_FLAG to nbd_set_strict", v1.18.0) to meet my needs.
However, I failed to codify my manual tests of that bit into a unit
test for libnbd, until now. Most sane clients will never call
nbd_set_strict_mode() in the first place (after all, it is explicitly
documented as an integration tool, which is how I used it with my qemu
code development), but it never hurts to make sure we don't break it
even for the relatively small set of users that would ever use it.
The test added here runs in two parts; if you get a SKIP despite
having qemu-nbd, then the first part ran successfully before the
second half gave up due to lack of extended headers in qemu
(presumably qemu 8.1 or older); if you get a PASS, then both parts
were run. However, both parts are inherently fragile, depending on
behavior known to be in qemu 8.2 - while it is unlikely to change in
future qemu releases (at least as long as I continue to maintain NBD
code there), the fact that we are intentionally violating the NBD
protocol means a different server is within its rights to behave
differently than qemu 8.2 did. Hence this test lives in interop/
rather than tests/ because of its strong ties to a particular qemu.
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 54d4426394c372413f55f648d4ad1d21b3395e07)
Signed-off-by: Eric Blake <eblake@redhat.com>
---
interop/Makefile.am | 2 +
interop/strict-mode-auto-flag.sh | 138 +++++++++++++++++++++++++++++++
2 files changed, 140 insertions(+)
create mode 100755 interop/strict-mode-auto-flag.sh
diff --git a/interop/Makefile.am b/interop/Makefile.am
index d6485adf..ac12d84a 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -28,6 +28,7 @@ EXTRA_DIST = \
structured-read.sh \
opt-extended-headers.sh \
block-status-payload.sh \
+ strict-mode-auto-flag.sh \
$(NULL)
TESTS_ENVIRONMENT = \
@@ -153,6 +154,7 @@ TESTS += \
interop-qemu-block-size.sh \
opt-extended-headers.sh \
block-status-payload.sh \
+ strict-mode-auto-flag.sh \
$(NULL)
interop_qemu_nbd_SOURCES = \
diff --git a/interop/strict-mode-auto-flag.sh b/interop/strict-mode-auto-flag.sh
new file mode 100755
index 00000000..8f73ea73
--- /dev/null
+++ b/interop/strict-mode-auto-flag.sh
@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright Red Hat
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+# Test effect of AUTO_FLAG bit in set_strict_mode()
+
+source ../tests/functions.sh
+set -e
+set -x
+
+requires truncate --version
+requires qemu-nbd --version
+requires nbdsh --version
+
+file="strict-mode-auto-flag.file"
+rm -f $file
+cleanup_fn rm -f $file
+
+truncate -s 1M $file
+
+# Unconditional part of test: behavior when extended headers are not in use
+$VG nbdsh -c '
+import errno
+
+h.set_request_extended_headers(False)
+args = ["qemu-nbd", "-f", "raw", "'"$file"'"]
+h.connect_systemd_socket_activation(args)
+assert h.get_extended_headers_negotiated() is False
+
+# STRICT_AUTO_FLAG and STRICT_COMMANDS are on by default
+flags = h.get_strict_mode()
+assert flags & nbd.STRICT_AUTO_FLAG
+assert flags & nbd.STRICT_COMMANDS
+
+# Under STRICT_AUTO_FLAG, using or omitting flag does not matter; client
+# side auto-corrects the flag before passing to server
+h.pwrite(b"1"*512, 0, 0)
+h.pwrite(b"2"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+
+# Without STRICT_AUTO_FLAG but still STRICT_COMMANDS, client side now sees
+# attempts to use the flag as invalid
+flags = flags & ~nbd.STRICT_AUTO_FLAG
+h.set_strict_mode(flags)
+h.pwrite(b"3"*512, 0, 0)
+stats = h.stats_bytes_sent()
+try:
+ h.pwrite(b"4"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+ assert False
+except nbd.Error as e:
+ assert e.errnum == errno.EINVAL
+assert stats == h.stats_bytes_sent()
+
+# Warning: fragile test ahead. Without STRICT_COMMANDS, we send unexpected
+# flag to qemu, and expect failure. For qemu <= 8.1, this is safe (those
+# versions did not know the flag, and correctly reject unknown flags with
+# NBD_EINVAL). For qemu 8.2, this also works (qemu knows the flag, but warns
+# that we were not supposed to send it without extended headers). But if
+# future qemu versions change to start silently ignoring the flag (after all,
+# a write command obviously has a payload even without extended headers, so
+# the flag is redundant for NBD_CMD_WRITE), then we may need to tweak this.
+flags = flags & ~nbd.STRICT_COMMANDS
+h.set_strict_mode(flags)
+h.pwrite(b"5"*512, 0, 0)
+stats = h.stats_bytes_sent()
+try:
+ h.pwrite(b"6"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+ print("Did newer qemu change behavior?")
+ assert False
+except nbd.Error as e:
+ assert e.errnum == errno.EINVAL
+assert stats < h.stats_bytes_sent()
+
+h.shutdown()
+'
+
+# Conditional part of test: only run if qemu supports extended headers
+requires nbdinfo --has extended-headers -- [ qemu-nbd -r -f raw "$file" ]
+$VG nbdsh -c '
+import errno
+
+args = ["qemu-nbd", "-f", "raw", "'"$file"'"]
+h.connect_systemd_socket_activation(args)
+assert h.get_extended_headers_negotiated() is True
+
+# STRICT_AUTO_FLAG and STRICT_COMMANDS are on by default
+flags = h.get_strict_mode()
+assert flags & nbd.STRICT_AUTO_FLAG
+assert flags & nbd.STRICT_COMMANDS
+
+# Under STRICT_AUTO_FLAG, using or omitting flag does not matter; client
+# side auto-corrects the flag before passing to server
+h.pwrite(b"1"*512, 0, 0)
+h.pwrite(b"2"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+
+# Without STRICT_AUTO_FLAG but still STRICT_COMMANDS, client side now sees
+# attempts to omit the flag as invalid
+flags = flags & ~nbd.STRICT_AUTO_FLAG
+h.set_strict_mode(flags)
+h.pwrite(b"3"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+stats = h.stats_bytes_sent()
+try:
+ h.pwrite(b"4"*512, 0, 0)
+ assert False
+except nbd.Error as e:
+ assert e.errnum == errno.EINVAL
+assert stats == h.stats_bytes_sent()
+
+# Warning: fragile test ahead. Without STRICT_COMMANDS, omitting the flag
+# is a protocol violation. qemu 8.2 silently ignores the violation; but a
+# future qemu might start failing the command, at which point we would need
+# to tweak this part of the test.
+flags = flags & ~nbd.STRICT_COMMANDS
+h.set_strict_mode(flags)
+h.pwrite(b"5"*512, 0, nbd.CMD_FLAG_PAYLOAD_LEN)
+stats = h.stats_bytes_sent()
+try:
+ h.pwrite(b"6"*512, 0, 0)
+except nbd.Error:
+ print("Did newer qemu change behavior?")
+ assert False
+assert stats < h.stats_bytes_sent()
+
+h.shutdown()
+'
--
2.41.0

View File

@ -1,15 +1,27 @@
# i686 no longer has any kind of OCaml compiler, not even ocamlc.
%ifnarch %{ix86}
%global have_ocaml 1
%endif
# No ublk in RHEL 9.
%if !0%{?rhel}
%global have_ublk 1
%endif
# No nbd.ko in RHEL 9.
%if !0%{?rhel}
%global have_nbd_ko 1
%endif
# If we should verify tarball signature with GPGv2.
%global verify_tarball_signature 1
# If there are patches which touch autotools files, set this to 1.
%global patches_touch_autotools 1
# The source directory.
%global source_directory 1.18-stable
%global source_directory 1.20-stable
Name: libnbd
Version: 1.18.1
Release: 3%{?dist}
Version: 1.20.0
Release: 1%{?dist}
Summary: NBD client library in userspace
License: LGPL-2.0-or-later AND BSD-3-Clause
@ -28,19 +40,13 @@ Source3: copy-patches.sh
# Patches are stored in the upstream repository:
# https://gitlab.com/nbdkit/libnbd/-/commits/rhel-9.4/
# Patches.
Patch0001: 0001-generator-Fix-assertion-in-ext-mode-BLOCK_STATUS-CVE.patch
Patch0002: 0002-docs-Fix-incorrect-xref-in-libnbd-release-notes-for-.patch
Patch0003: 0003-tests-Check-behavior-of-nbd_set_strict_mode-STRICT_A.patch
%if 0%{patches_touch_autotools}
BuildRequires: autoconf, automake, libtool
%endif
%if 0%{verify_tarball_signature}
BuildRequires: gnupg2
%endif
# For rebuilding autoconf cruft.
BuildRequires: autoconf, automake, libtool
# For the core library.
BuildRequires: gcc
BuildRequires: make
@ -51,7 +57,7 @@ BuildRequires: libxml2-devel
# For nbdfuse.
BuildRequires: fuse3, fuse3-devel
%if !0%{?rhel}
%if 0%{?have_ublk}
# For nbdublk
BuildRequires: liburing-devel >= 2.2
BuildRequires: ubdsrv-devel >= 1.0-3.rc6
@ -60,7 +66,7 @@ BuildRequires: ubdsrv-devel >= 1.0-3.rc6
# For the Python 3 bindings.
BuildRequires: python3-devel
%ifnarch %{ix86}
%if 0%{?have_ocaml}
# For the OCaml bindings.
BuildRequires: ocaml
BuildRequires: ocaml-findlib-devel
@ -79,7 +85,7 @@ BuildRequires: gcc-c++
BuildRequires: gnutls-utils
BuildRequires: iproute
BuildRequires: jq
%if !0%{?rhel}
%if 0%{?have_nbd_ko}
BuildRequires: nbd
%endif
BuildRequires: util-linux
@ -100,7 +106,7 @@ BuildRequires: nbdkit-sh-plugin
BuildRequires: nbdkit-sparse-random-plugin
%endif
%ifnarch %{ix86}
%if 0%{?have_ocaml}
# The OCaml runtime system does not provide this symbol
%global __ocaml_requires_opts -x Stdlib__Callback
%endif
@ -136,7 +142,7 @@ Requires: %{name}%{?_isa} = %{version}-%{release}
This package contains development headers for %{name}.
%ifnarch %{ix86}
%if 0%{?have_ocaml}
%package -n ocaml-%{name}
Summary: OCaml language bindings for %{name}
Requires: %{name}%{?_isa} = %{version}-%{release}
@ -182,7 +188,7 @@ Recommends: fuse3
This package contains FUSE support for %{name}.
%if !0%{?rhel}
%if 0%{?have_ublk}
%package -n nbdublk
Summary: Userspace NBD block device
Requires: %{name}%{?_isa} = %{version}-%{release}
@ -215,25 +221,30 @@ for %{name}.
%{gpgverify} --keyring='%{SOURCE2}' --signature='%{SOURCE1}' --data='%{SOURCE0}'
%endif
%autosetup -p1
%if 0%{patches_touch_autotools}
autoreconf -i
%endif
%build
%configure \
--disable-static \
--with-tls-priority=@LIBNBD,SYSTEM \
--with-bash-completions \
PYTHON=%{__python3} \
--enable-python \
%ifnarch %{ix86}
%if 0%{?have_ocaml}
--enable-ocaml \
%else
--disable-ocaml \
%endif
--enable-fuse \
--disable-golang \
--disable-rust
--disable-rust \
%if 0%{?have_ublk}
--enable-ublk \
%else
--disable-ublk \
%endif
%{nil}
make %{?_smp_mflags}
@ -247,16 +258,11 @@ find $RPM_BUILD_ROOT -name '*.la' -delete
# Delete the golang man page since we're not distributing the bindings.
rm $RPM_BUILD_ROOT%{_mandir}/man3/libnbd-golang.3*
%ifarch %{ix86}
%if !0%{?have_ocaml}
# Delete the OCaml man page on i686.
rm $RPM_BUILD_ROOT%{_mandir}/man3/libnbd-ocaml.3*
%endif
%if 0%{?rhel}
# Delete nbdublk on RHEL.
rm $RPM_BUILD_ROOT%{_datadir}/bash-completion/completions/nbdublk
%endif
%check
function skip_test ()
@ -268,12 +274,6 @@ function skip_test ()
done
}
# interop/structured-read.sh fails with the old qemu-nbd in Fedora 29,
# so disable it there.
%if 0%{?fedora} <= 29
skip_test interop/structured-read.sh
%endif
# interop/interop-qemu-storage-daemon.sh fails in RHEL 9 because of
# this bug in qemu:
# https://lists.nongnu.org/archive/html/qemu-devel/2021-03/threads.html#03544
@ -324,7 +324,7 @@ make %{?_smp_mflags} check || {
%{_mandir}/man3/nbd_*.3*
%ifnarch %{ix86}
%if 0%{?have_ocaml}
%files -n ocaml-%{name}
%dir %{_libdir}/ocaml/nbd
%{_libdir}/ocaml/nbd/META
@ -363,7 +363,7 @@ make %{?_smp_mflags} check || {
%{_mandir}/man1/nbdfuse.1*
%if !0%{?rhel}
%if 0%{?have_ublk}
%files -n nbdublk
%{_bindir}/nbdublk
%{_mandir}/man1/nbdublk.1*
@ -383,6 +383,10 @@ make %{?_smp_mflags} check || {
%changelog
* Tue Apr 09 2024 Miroslav Rezanina <mrezanin@redhat.com> - 1.20.0-1
- Rebase to 1.20.0
resolves: RHEL-31883
* Mon Nov 13 2023 Eric Blake <eblake@redhat.com> - 1.18.1-3
- Backport unit test of recent libnbd API addition
resolves: RHEL-16292

View File

@ -1,2 +1,2 @@
SHA512 (libnbd-1.18.1.tar.gz) = f4262666be55d580550e053355f14f80d352bf869ae7241e9fa032a9b5cd9e027eb89a536871c1206422413fc7ed745da7d612b3e1413f76ec17168705fbf12c
SHA512 (libnbd-1.18.1.tar.gz.sig) = 57798aa8b8c0973c0e13f431a6735e13a5aa546190e5de9cb43f78d54c5438df70bdf6e875282a3c4221c222a1517c64bb311e769f7c1a3e61d5b1a4e7f75e2d
SHA512 (libnbd-1.20.0.tar.gz) = 28b72c8252cc7f497fc87c2a885256bdaeeb5fcf60f8df882e603b94e6a753191a9f081e65f8afc3d70cf1156b78c49ec53b89188bb82f6d2eeb172402ad7bd8
SHA512 (libnbd-1.20.0.tar.gz.sig) = 214233d7d0f06bd1774d4edba99c3d4bc37715023ca798cc0982820ceaf9cad4926989078a1544897e2fb4bf9b450a8e2d9b9113d4ed8b6eb08d9c5e4618f255