Import from AlmaLinux stable repository

This commit is contained in:
eabdullin 2025-12-04 12:31:25 +00:00
parent e9cf2c92cf
commit 15c2473f84
30 changed files with 2004 additions and 1055 deletions

2
.gitignore vendored
View File

@ -1,2 +1,2 @@
SOURCES/libguestfs.keyring
SOURCES/libnbd-1.18.1.tar.gz
SOURCES/libnbd-1.20.3.tar.gz

View File

@ -1,2 +1,2 @@
cc1b37b9cfafa515aab3eefd345ecc59aac2ce7b SOURCES/libguestfs.keyring
4f99e6f21edffe62b394aa9c7fb68149e6d4d5e4 SOURCES/libnbd-1.18.1.tar.gz
0dd368dc40c30fda4225d90120bdf96dd1724557 SOURCES/libnbd-1.20.3.tar.gz

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

@ -0,0 +1,191 @@
From 654ab90d57d1ba54e27fd68a8550703a0f1059c4 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 23 Jul 2024 17:22:12 +0100
Subject: [PATCH] generator: Print full error in handle_reply_error
Print the full error from the server during handshaking. This
modifies the contract of handle_reply_error so it calls set_error,
which can be overridden by callers or ignored completely.
(cherry picked from commit cf49a49adc8abc8c917437db7461ed9956583877)
---
generator/states-newstyle-opt-go.c | 32 +--------
generator/states-newstyle-opt-list.c | 5 +-
generator/states-newstyle-opt-meta-context.c | 8 +--
generator/states-newstyle.c | 68 ++++++++++++++++++--
4 files changed, 69 insertions(+), 44 deletions(-)
diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index 5bc9a9ae..f6eb8afc 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -247,37 +247,9 @@ STATE_MACHINE {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- /* Decode expected known errors into a nicer string */
- switch (reply) {
- case NBD_REP_ERR_UNSUP:
+ if (reply == NBD_REP_ERR_UNSUP)
assert (h->opt_current == NBD_OPT_INFO);
- set_error (ENOTSUP, "handshake: server lacks NBD_OPT_INFO support");
- break;
- case NBD_REP_ERR_POLICY:
- case NBD_REP_ERR_PLATFORM:
- set_error (0, "handshake: server policy prevents NBD_OPT_GO");
- break;
- case NBD_REP_ERR_INVALID:
- case NBD_REP_ERR_TOO_BIG:
- set_error (EINVAL, "handshake: server rejected NBD_OPT_GO as invalid");
- break;
- case NBD_REP_ERR_TLS_REQD:
- set_error (ENOTSUP, "handshake: server requires TLS encryption first");
- break;
- case NBD_REP_ERR_UNKNOWN:
- set_error (ENOENT, "handshake: server has no export named '%s'",
- h->export_name);
- break;
- case NBD_REP_ERR_SHUTDOWN:
- set_error (ESHUTDOWN, "handshake: server is shutting down");
- break;
- case NBD_REP_ERR_BLOCK_SIZE_REQD:
- set_error (EINVAL, "handshake: server requires specific block sizes");
- break;
- default:
- set_error (0, "handshake: unknown reply from NBD_OPT_GO: 0x%" PRIx32,
- reply);
- }
+
nbd_internal_reset_size_and_flags (h);
h->meta_valid = false;
err = nbd_get_errno () ? : ENOTSUP;
diff --git a/generator/states-newstyle-opt-list.c b/generator/states-newstyle-opt-list.c
index cdd4676e..6605ee0a 100644
--- a/generator/states-newstyle-opt-list.c
+++ b/generator/states-newstyle-opt-list.c
@@ -127,9 +127,8 @@ STATE_MACHINE {
SET_NEXT_STATE (%.DEAD);
return 0;
}
- err = ENOTSUP;
- set_error (err, "unexpected response, possibly the server does not "
- "support listing exports");
+ debug (h, "unexpected response, possibly the server does not "
+ "support listing exports");
break;
}
diff --git a/generator/states-newstyle-opt-meta-context.c b/generator/states-newstyle-opt-meta-context.c
index 6f016e66..3945411e 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -270,12 +270,8 @@ STATE_MACHINE {
}
if (opt == h->opt_current) {
- /* XXX Should we decode specific expected errors, like
- * REP_ERR_UNKNOWN to ENOENT or REP_ERR_TOO_BIG to ERANGE?
- */
- err = ENOTSUP;
- set_error (err, "unexpected response, possibly the server does not "
- "support meta contexts");
+ debug (h, "unexpected response, possibly the server does not "
+ "support meta contexts");
CALL_CALLBACK (h->opt_cb.completion, &err);
nbd_internal_free_option (h);
SET_NEXT_STATE (%.NEGOTIATING);
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 45893a8b..6c7cc45c 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -79,14 +79,18 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt)
return 0;
}
-/* Check an unexpected server reply. If it is an error, log any
- * message from the server and return 0; otherwise, return -1.
+/* Check an unexpected server reply error.
+ *
+ * This calls set_error with a descriptive error message and returns
+ * 0. Unless there is a further unexpected error while processing
+ * this error, in which case it calls set_error and returns -1.
*/
static int
handle_reply_error (struct nbd_handle *h)
{
uint32_t len;
uint32_t reply;
+ char *msg = NULL;
len = be32toh (h->sbuf.or.option_reply.replylen);
reply = be32toh (h->sbuf.or.option_reply.reply);
@@ -101,9 +105,63 @@ handle_reply_error (struct nbd_handle *h)
return -1;
}
- if (len > 0)
- debug (h, "handshake: server error message: %.*s", (int)len,
- h->sbuf.or.payload.err_msg);
+ /* Decode expected errors into a nicer string.
+ *
+ * XXX Note this string comes directly from the server, and most
+ * libnbd users simply print the error using 'fprintf'. We really
+ * ought to quote this string somehow, but we don't have a useful
+ * function for that.
+ */
+ if (len > 0) {
+ if (asprintf (&msg, ": %.*s",
+ (int)len, h->sbuf.or.payload.err_msg) == -1) {
+ set_error (errno, "asprintf");
+ return -1;
+ }
+ }
+
+ switch (reply) {
+ case NBD_REP_ERR_UNSUP:
+ set_error (ENOTSUP, "the operation is not supported by the server%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_POLICY:
+ set_error (0, "server policy prevents the operation%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_PLATFORM:
+ set_error (0, "the operation is not supported by the server platform%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_INVALID:
+ set_error (EINVAL, "the server rejected this operation as invalid%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_TOO_BIG:
+ set_error (EINVAL, "the operation is too large to process%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_TLS_REQD:
+ set_error (ENOTSUP, "the server requires TLS encryption first%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_UNKNOWN:
+ set_error (ENOENT, "the server has no export named '%s'%s",
+ h->export_name, msg ? : "");
+ break;
+ case NBD_REP_ERR_SHUTDOWN:
+ set_error (ESHUTDOWN, "the server is shutting down%s",
+ msg ? : "");
+ break;
+ case NBD_REP_ERR_BLOCK_SIZE_REQD:
+ set_error (EINVAL, "the server requires specific block sizes%s",
+ msg ? : "");
+ break;
+ default:
+ set_error (0, "handshake: unknown reply from the server: 0x%" PRIx32 "%s",
+ reply, msg ? : "");
+ }
+ free (msg);
return 0;
}
--
2.47.1

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

@ -0,0 +1,38 @@
From e1b029ff41ed2f3c3d7decae7a50e70a86766350 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 23 Jul 2024 17:26:39 +0100
Subject: [PATCH] lib: Don't overwrite error in nbd_opt_{go,info}
We already set the error in handle_reply_error, so don't overwrite
that here.
(cherry picked from commit 474a4ae6c8d11212a4a8c06ea3e8b3fd97a7e97d)
---
lib/opt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/opt.c b/lib/opt.c
index 600265a0..5872dd54 100644
--- a/lib/opt.c
+++ b/lib/opt.c
@@ -99,7 +99,7 @@ nbd_unlocked_opt_go (struct nbd_handle *h)
if (r == 0 && err) {
assert (nbd_internal_is_state_negotiating (get_next_state (h)) ||
nbd_internal_is_state_dead (get_next_state (h)));
- set_error (err, "server replied with error to opt_go request");
+ /* handle_reply_error already called set_error */
return -1;
}
if (r == 0)
@@ -122,7 +122,7 @@ nbd_unlocked_opt_info (struct nbd_handle *h)
if (r == 0 && err) {
assert (nbd_internal_is_state_negotiating (get_next_state (h)) ||
nbd_internal_is_state_dead (get_next_state (h)));
- set_error (err, "server replied with error to opt_info request");
+ /* handle_reply_error already called set_error */
return -1;
}
return r;
--
2.47.1

View File

@ -0,0 +1,43 @@
From 0a21eef34ba568c662c338a345692bfd0524d726 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 25 Jul 2024 13:39:28 +0100
Subject: [PATCH] generator: Restore assignment to local 'err'
I accidentally removed the assignment of local variable 'err' along
these paths in commit cf49a49adc ("generator: Print full error in
handle_reply_error").
Fixes: commit cf49a49adc8abc8c917437db7461ed9956583877
(cherry picked from commit e75d20b9e19143b1bd0d232fc49cb2e0287f824a)
---
generator/states-newstyle-opt-list.c | 1 +
generator/states-newstyle-opt-meta-context.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/generator/states-newstyle-opt-list.c b/generator/states-newstyle-opt-list.c
index 6605ee0a..48559574 100644
--- a/generator/states-newstyle-opt-list.c
+++ b/generator/states-newstyle-opt-list.c
@@ -129,6 +129,7 @@ STATE_MACHINE {
}
debug (h, "unexpected response, possibly the server does not "
"support listing exports");
+ err = ENOTSUP;
break;
}
diff --git a/generator/states-newstyle-opt-meta-context.c b/generator/states-newstyle-opt-meta-context.c
index 3945411e..699e24aa 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -272,6 +272,7 @@ STATE_MACHINE {
if (opt == h->opt_current) {
debug (h, "unexpected response, possibly the server does not "
"support meta contexts");
+ err = ENOTSUP;
CALL_CALLBACK (h->opt_cb.completion, &err);
nbd_internal_free_option (h);
SET_NEXT_STATE (%.NEGOTIATING);
--
2.47.1

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,91 +0,0 @@
From 596626369b90016f6852610c217da22668158521 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 25 Jun 2024 10:55:54 +0100
Subject: [PATCH] build: Move to minimum gnutls >= 3.5.18
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This version matches current qemu.
RHEL 7 gnutls is too old (lacks gnutls_session_set_verify_cert), which
means TLS will be disabled on this platform. RHEL 8 has gnutls 3.6.14.
I also unconditionally enabled the gnutls/socket.h header. This
header was added in 2016 (gnutls 3.5.3), so it's not present in RHEL 7.
On RHEL 7 the configure-time test now prints:
checking for GNUTLS... no
configure: WARNING: gnutls not found or < 3.5.18, TLS support will be disabled.
...
Optional library features:
TLS support ............................ no
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 5ff09cdbbd19226dd2d5015d76134f88dee9321e)
(cherry picked from commit 177fd0847723640829eff8d1ab102f8d28a7328e)
---
configure.ac | 5 ++---
lib/crypto.c | 6 ------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/configure.ac b/configure.ac
index 91fe004b..c0d6a472 100644
--- a/configure.ac
+++ b/configure.ac
@@ -178,13 +178,13 @@ AC_ARG_WITH([gnutls],
[],
[with_gnutls=check])
AS_IF([test "$with_gnutls" != "no"],[
- PKG_CHECK_MODULES([GNUTLS], [gnutls >= 3.3.0], [
+ PKG_CHECK_MODULES([GNUTLS], [gnutls >= 3.5.18], [
printf "gnutls version is "; $PKG_CONFIG --modversion gnutls
AC_SUBST([GNUTLS_CFLAGS])
AC_SUBST([GNUTLS_LIBS])
AC_DEFINE([HAVE_GNUTLS],[1],[gnutls found at compile time.])
], [
- AC_MSG_WARN([gnutls not found or < 3.3.0, TLS support will be disabled.])
+ AC_MSG_WARN([gnutls not found or < 3.5.18, TLS support will be disabled.])
])
])
AM_CONDITIONAL([HAVE_GNUTLS], [test "x$GNUTLS_LIBS" != "x"])
@@ -210,7 +210,6 @@ AS_IF([test "$GNUTLS_LIBS" != ""],[
old_LIBS="$LIBS"
LIBS="$GNUTLS_LIBS $LIBS"
AC_CHECK_FUNCS([\
- gnutls_session_set_verify_cert \
gnutls_transport_is_ktls_enabled \
])
LIBS="$old_LIBS"
diff --git a/lib/crypto.c b/lib/crypto.c
index 22a1cfa5..d131f1d0 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -28,10 +28,8 @@
#ifdef HAVE_GNUTLS
#include <gnutls/gnutls.h>
-#ifdef HAVE_GNUTLS_SOCKET_H
#include <gnutls/socket.h>
#endif
-#endif
#include "internal.h"
#include "nbdkit-string.h"
@@ -532,12 +530,8 @@ set_up_certificate_credentials (struct nbd_handle *h,
return NULL;
found_certificates:
-#ifdef HAVE_GNUTLS_SESSION_SET_VERIFY_CERT
if (h->hostname && h->tls_verify_peer)
gnutls_session_set_verify_cert (session, h->hostname, 0);
-#else
- debug (h, "ignoring nbd_set_tls_verify_peer, this requires GnuTLS >= 3.4.6");
-#endif
err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, ret);
if (err < 0) {
--
2.43.0

View File

@ -0,0 +1,175 @@
From 144a024f99674ff7845b86b81e9e3cf6c1d88bf9 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 25 Jul 2024 13:25:34 +0100
Subject: [PATCH] generator/states-newstyle.c: Quote untrusted string from the
server
Updates: commit cf49a49adc8abc8c917437db7461ed9956583877
(cherry picked from commit 5dbfc418cb6176102634acea2256b2335520159c)
---
generator/states-newstyle.c | 124 ++++++++++++++++++++----------------
1 file changed, 68 insertions(+), 56 deletions(-)
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 6c7cc45c..8c483bd2 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -18,6 +18,7 @@
#include <assert.h>
+#include "ascii-ctype.h"
#include "internal.h"
/* Common code for parsing a reply to NBD_OPT_*. */
@@ -88,80 +89,91 @@ prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt)
static int
handle_reply_error (struct nbd_handle *h)
{
- uint32_t len;
uint32_t reply;
- char *msg = NULL;
+ uint32_t replylen;
+ FILE *fp;
+ char *s = NULL;
+ size_t len = 0;
+ int err = 0;
- len = be32toh (h->sbuf.or.option_reply.replylen);
reply = be32toh (h->sbuf.or.option_reply.reply);
if (!NBD_REP_IS_ERR (reply)) {
set_error (0, "handshake: unexpected option reply type %d", reply);
return -1;
}
+ replylen = be32toh (h->sbuf.or.option_reply.replylen);
assert (NBD_MAX_STRING < sizeof h->sbuf.or.payload);
- if (len > NBD_MAX_STRING) {
+ if (replylen > NBD_MAX_STRING) {
set_error (0, "handshake: option error string too long");
return -1;
}
- /* Decode expected errors into a nicer string.
- *
- * XXX Note this string comes directly from the server, and most
- * libnbd users simply print the error using 'fprintf'. We really
- * ought to quote this string somehow, but we don't have a useful
- * function for that.
- */
- if (len > 0) {
- if (asprintf (&msg, ": %.*s",
- (int)len, h->sbuf.or.payload.err_msg) == -1) {
- set_error (errno, "asprintf");
- return -1;
- }
+ /* Decode expected errors into a nicer string. */
+ fp = open_memstream (&s, &len);
+ if (fp == NULL) {
+ set_error (errno, "open_memstream");
+ return -1;
}
switch (reply) {
case NBD_REP_ERR_UNSUP:
- set_error (ENOTSUP, "the operation is not supported by the server%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_POLICY:
- set_error (0, "server policy prevents the operation%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_PLATFORM:
- set_error (0, "the operation is not supported by the server platform%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_INVALID:
- set_error (EINVAL, "the server rejected this operation as invalid%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_TOO_BIG:
- set_error (EINVAL, "the operation is too large to process%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_TLS_REQD:
- set_error (ENOTSUP, "the server requires TLS encryption first%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_UNKNOWN:
- set_error (ENOENT, "the server has no export named '%s'%s",
- h->export_name, msg ? : "");
- break;
- case NBD_REP_ERR_SHUTDOWN:
- set_error (ESHUTDOWN, "the server is shutting down%s",
- msg ? : "");
- break;
- case NBD_REP_ERR_BLOCK_SIZE_REQD:
- set_error (EINVAL, "the server requires specific block sizes%s",
- msg ? : "");
- break;
- default:
- set_error (0, "handshake: unknown reply from the server: 0x%" PRIx32 "%s",
- reply, msg ? : "");
+ err = ENOTSUP;
+ fprintf (fp, "the operation is not supported by the server");
+ break;
+ case NBD_REP_ERR_POLICY:
+ fprintf (fp, "server policy prevents the operation");
+ break;
+ case NBD_REP_ERR_PLATFORM:
+ fprintf (fp, "the operation is not supported by the server platform");
+ break;
+ case NBD_REP_ERR_INVALID:
+ err = EINVAL;
+ fprintf (fp, "the server rejected this operation as invalid");
+ break;
+ case NBD_REP_ERR_TOO_BIG:
+ err = EINVAL;
+ fprintf (fp, "the operation is too large to process");
+ break;
+ case NBD_REP_ERR_TLS_REQD:
+ err = ENOTSUP;
+ fprintf (fp, "the server requires TLS encryption first");
+ break;
+ case NBD_REP_ERR_UNKNOWN:
+ err = ENOENT;
+ fprintf (fp, "the server has no export named '%s'", h->export_name);
+ break;
+ case NBD_REP_ERR_SHUTDOWN:
+ err = ESHUTDOWN;
+ fprintf (fp, "the server is shutting down");
+ break;
+ case NBD_REP_ERR_BLOCK_SIZE_REQD:
+ err = EINVAL;
+ fprintf (fp, "the server requires specific block sizes");
+ break;
+ default:
+ fprintf (fp, "handshake: unknown reply from the server: 0x%" PRIx32,
+ reply);
+ }
+
+ if (replylen > 0) {
+ /* Since this message comes from the server, take steps to quote it. */
+ uint32_t i;
+ const char *msg = h->sbuf.or.payload.err_msg;
+
+ fprintf (fp, ": ");
+ for (i = 0; i < replylen; ++i) {
+ if (ascii_isprint (msg[i]))
+ fputc (msg[i], fp);
+ else
+ fprintf (fp, "\\x%02x", msg[i]);
}
- free (msg);
+ }
+
+ fclose (fp);
+
+ set_error (err, "%s", s);
+ free (s);
return 0;
}
--
2.47.1

View File

@ -0,0 +1,27 @@
From 73b48ea48f5681e4b25c3744db0aaf9dbb25b2f3 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 25 Jul 2024 15:48:46 +0100
Subject: [PATCH] generator/states-newstyle.c: Don't sign extend escaped chars
Fixes: commit 5dbfc418cb6176102634acea2256b2335520159c
(cherry picked from commit 0d6c6bbb3386de3b60ab6c4831045f2b1896051b)
---
generator/states-newstyle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 8c483bd2..1e026a8a 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -159,7 +159,7 @@ handle_reply_error (struct nbd_handle *h)
if (replylen > 0) {
/* Since this message comes from the server, take steps to quote it. */
uint32_t i;
- const char *msg = h->sbuf.or.payload.err_msg;
+ const unsigned char *msg = (unsigned char *) h->sbuf.or.payload.err_msg;
fprintf (fp, ": ");
for (i = 0; i < replylen; ++i) {
--
2.47.1

View File

@ -1,57 +0,0 @@
From d8ec4c8ecc5244ed192f58bc3a976c4b2f9cc6d7 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 24 Jun 2024 10:48:12 +0100
Subject: [PATCH] lib/crypto.c: Check server certificate even when using system
CA
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The previous code checked the server certificate only when a custom
certificate directory was set (ie. nbd_set_tls_certificates /
?tls-certificates=DIR). In the fallback case where we use the system
CA, we never called gnutls_session_set_verify_cert and so the server
certificate was never checked.
Move the call to gnutls_session_set_verify_cert later so it is called
on both paths.
If the server certificate does not match the hostname you will see:
nbdinfo: nbd_connect_uri: gnutls_handshake: Error in the certificate verification. (15/1)
Reported-by: Jon Szymaniak <jon.szymaniak@gmail.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 87ef41b69929d5d293390ec36b1c10aba2c9a57a)
(cherry picked from commit 7a6739aeca8250515a449bacd23d09bf40587dec)
---
lib/crypto.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/crypto.c b/lib/crypto.c
index d131f1d0..c542ce6b 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -530,9 +530,6 @@ set_up_certificate_credentials (struct nbd_handle *h,
return NULL;
found_certificates:
- if (h->hostname && h->tls_verify_peer)
- gnutls_session_set_verify_cert (session, h->hostname, 0);
-
err = gnutls_credentials_set (session, GNUTLS_CRD_CERTIFICATE, ret);
if (err < 0) {
set_error (0, "gnutls_credentials_set: %s", gnutls_strerror (err));
@@ -647,6 +644,9 @@ nbd_internal_crypto_create_session (struct nbd_handle *h,
gnutls_deinit (session);
return NULL;
}
+
+ if (h->hostname && h->tls_verify_peer)
+ gnutls_session_set_verify_cert (session, h->hostname, 0);
}
/* Wrap the underlying socket with GnuTLS. */
--
2.43.0

View File

@ -0,0 +1,49 @@
From 1c9d65c6100edfd6050b34dcf29a7a1bbdf5e89a Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Sat, 29 Mar 2025 14:00:39 +0000
Subject: [PATCH] copy: Set the total size in bytes copied
Ensure that src->size contains the total size in bytes copied. There
is (only) one place where this is not known in advance, which is when
we are reading from a pipe.
(cherry picked from commit afe4f390a65a0d1b9f3625bf90c73726866e0a64)
---
copy/main.c | 3 +++
copy/synch-copying.c | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/copy/main.c b/copy/main.c
index ae9840ae..cd7b2fd2 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -479,6 +479,9 @@ main (int argc, char *argv[])
/* Always set the progress bar to 100% at the end of the copy. */
progress_bar (1, 1);
+ /* We should always know the total size copied here. */
+ assert (src->size >= 0);
+
/* Shut down the source side. */
src->ops->close (src);
diff --git a/copy/synch-copying.c b/copy/synch-copying.c
index 2f6627bf..200c97f6 100644
--- a/copy/synch-copying.c
+++ b/copy/synch-copying.c
@@ -53,6 +53,12 @@ synch_copying (void)
offset += r;
progress_bar (offset, src->size);
}
+
+ /* Record the total amount of data that was copied. In all other
+ * cases, src->size will already be set to the true size, so here
+ * is the only place we have to set this.
+ */
+ src->size = offset;
}
/* Otherwise we know how much we're copying, so we can copy in whole
--
2.47.1

View File

@ -1,76 +0,0 @@
From af09b72a486fd870ab72170a0cba4b1d6d37894f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 24 Jun 2024 10:31:10 +0100
Subject: [PATCH] lib/crypto.c: Allow CA verification even if h->hostname is
not set
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Calling gnutls_session_set_verify_cert with the hostname parameter set
to NULL is permitted:
https://www.gnutls.org/manual/html_node/Core-TLS-API.html#gnutls_005fsession_005fset_005fverify_005fcert
It means that the server's hostname in the certificate will not be
verified but we can at least check that the certificate was signed by
the CA. This allows the CA to be checked even for connections over
Unix domain sockets.
Example:
$ rm -f /tmp/sock
$ nbdkit -U /tmp/sock -f --tls=require --tls-certificates=$HOME/d/nbdkit/tests/pki memory 1G &
Before this change:
$ nbdinfo 'nbds+unix://?socket=/tmp/sock'
protocol: newstyle-fixed with TLS, using structured packets
export="":
export-size: 1073741824 (1G)
content: data
uri: nbds+unix:///?socket=/tmp/sock
[etc]
(works because it never called gnutls_session_set_verify_cert).
After this change:
$ nbdinfo 'nbds+unix://?socket=/tmp/sock'
nbdinfo: nbd_connect_uri: gnutls_handshake: Error in the certificate verification. (15/1)
(fails because system CA does not know about nbdkit's certificate
which is signed by the CA from the nbdkit/tests/pki directory)
$ nbdinfo 'nbds+unix://?socket=/tmp/sock&tls-certificates=/home/rjones/d/nbdkit/tests/pki'
protocol: newstyle-fixed with TLS, using structured packets
export="":
export-size: 1073741824 (1G)
content: data
uri: nbds+unix:///?socket=/tmp/sock&tls-certificates=/home/rjones/d/nbdkit/tests/pki
[etc]
(works because we supplied the correct CA)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 6ed47a27d14f6f11946bb096d94e5bf21d97083d)
(cherry picked from commit 3a427e6d7a83f89299ab6fdaeeffbd9074610ecc)
---
lib/crypto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/crypto.c b/lib/crypto.c
index c542ce6b..437e24ec 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -645,7 +645,7 @@ nbd_internal_crypto_create_session (struct nbd_handle *h,
return NULL;
}
- if (h->hostname && h->tls_verify_peer)
+ if (h->tls_verify_peer)
gnutls_session_set_verify_cert (session, h->hostname, 0);
}
--
2.43.0

File diff suppressed because it is too large Load Diff

View File

@ -1,145 +0,0 @@
From 764fc45a258c08177d01b6b6b6a0e431ee29089a Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 24 Jun 2024 11:49:07 +0100
Subject: [PATCH] interop: Pass -DCERTS and -DPSK as strings
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Rather than implicitly defining the certificates dir or PSK file in
interop.c, pass the actual paths from the Makefile.
This also allows -DCERTS=NULL which is interpreted as not calling
nbd_set_tls_certificates at all. This makes the test added in a
subsequent commit possible.
No real change here, just refactoring the tests.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 69ab18442994c68f749e2b84b91d41031ebbb088)
(cherry picked from commit 33d7f3aa8e3cf8c826a534107529e1d409c0c004)
---
interop/Makefile.am | 18 +++++++++---------
interop/interop.c | 11 ++++++-----
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/interop/Makefile.am b/interop/Makefile.am
index ac12d84a..4cdc55e9 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -100,7 +100,7 @@ interop_nbd_server_tls_CPPFLAGS = \
-DSERVER=\"$(NBD_SERVER)\" \
-DSERVER_PARAMS='"-d", "-C", "nbd-server-tls.conf", "0", TMPFILE' \
-DEXPORT_NAME='""' \
- -DCERTS=1 \
+ -DCERTS='"../tests/pki"' \
-DTLS_MODE=LIBNBD_TLS_REQUIRE \
$(NULL)
interop_nbd_server_tls_LDADD = \
@@ -186,7 +186,7 @@ interop_qemu_nbd_tls_certs_CPPFLAGS = \
-DSERVER=\"$(QEMU_NBD)\" \
-DSERVER_PARAMS='"--object", "tls-creds-x509,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests/pki", "--tls-creds", "tls0", "-f", "raw", "-x", "/", TMPFILE' \
-DEXPORT_NAME='"/"' \
- -DCERTS=1 \
+ -DCERTS='"../tests/pki"' \
-DTLS_MODE=LIBNBD_TLS_REQUIRE \
$(NULL)
interop_qemu_nbd_tls_certs_LDADD = \
@@ -208,7 +208,7 @@ interop_qemu_nbd_tls_psk_CPPFLAGS = \
-DSERVER=\"$(QEMU_NBD)\" \
-DSERVER_PARAMS='"--object", "tls-creds-psk,id=tls0,endpoint=server,dir=$(abs_top_builddir)/tests", "--tls-creds", "tls0", "-f", "raw", "-x", "/", TMPFILE' \
-DEXPORT_NAME='"/"' \
- -DPSK=1 \
+ -DPSK='"../tests/keys.psk"' \
-DTLS_MODE=LIBNBD_TLS_REQUIRE \
$(NULL)
interop_qemu_nbd_tls_psk_LDADD = \
@@ -323,7 +323,7 @@ interop_nbdkit_tls_certs_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=require", "--tls-certificates=../tests/pki", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DCERTS=1 \
+ -DCERTS='"../tests/pki"' \
-DTLS_MODE=LIBNBD_TLS_REQUIRE \
$(NULL)
interop_nbdkit_tls_certs_LDADD = \
@@ -342,7 +342,7 @@ interop_nbdkit_tls_certs_allow_enabled_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=require", "--tls-certificates=../tests/pki", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DCERTS=1 \
+ -DCERTS='"../tests/pki"' \
-DTLS_MODE=LIBNBD_TLS_ALLOW \
$(NULL)
interop_nbdkit_tls_certs_allow_enabled_LDADD = \
@@ -361,7 +361,7 @@ interop_nbdkit_tls_certs_allow_fallback_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=off", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DCERTS=1 \
+ -DCERTS='"../tests/pki"' \
-DTLS_MODE=LIBNBD_TLS_ALLOW \
-DTLS_FALLBACK=1 \
$(NULL)
@@ -381,7 +381,7 @@ interop_nbdkit_tls_psk_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=require", "--tls-psk=../tests/keys.psk", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DPSK=1 \
+ -DPSK='"../tests/keys.psk"' \
-DTLS_MODE=LIBNBD_TLS_REQUIRE \
$(NULL)
interop_nbdkit_tls_psk_LDADD = \
@@ -400,7 +400,7 @@ interop_nbdkit_tls_psk_allow_enabled_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=require", "--tls-psk=../tests/keys.psk", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DPSK=1 \
+ -DPSK='"../tests/keys.psk"' \
-DTLS_MODE=LIBNBD_TLS_ALLOW \
$(NULL)
interop_nbdkit_tls_psk_allow_enabled_LDADD = \
@@ -419,7 +419,7 @@ interop_nbdkit_tls_psk_allow_fallback_CPPFLAGS = \
-DNEEDS_TMPFILE=1 \
-DSERVER=\"$(NBDKIT)\" \
-DSERVER_PARAMS='"--tls=off", "-s", "--exit-with-parent", "file", TMPFILE' \
- -DPSK=1 \
+ -DPSK='"../tests/keys.psk"' \
-DTLS_MODE=LIBNBD_TLS_ALLOW \
-DTLS_FALLBACK=1 \
$(NULL)
diff --git a/interop/interop.c b/interop/interop.c
index 20e101d4..d4d6671e 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -41,7 +41,7 @@
#define SIZE (1024*1024)
-#if CERTS || PSK
+#if defined(CERTS) || defined(PSK)
#define TLS 1
#ifndef TLS_MODE
#error "TLS_MODE must be defined when using CERTS || PSK"
@@ -149,13 +149,14 @@ main (int argc, char *argv[])
}
#endif
-#if CERTS
- if (nbd_set_tls_certificates (nbd, "../tests/pki") == -1) {
+#if defined(CERTS)
+ const char *certs = CERTS;
+ if (certs && nbd_set_tls_certificates (nbd, certs) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
-#elif PSK
- if (nbd_set_tls_psk_file (nbd, "../tests/keys.psk") == -1) {
+#elif defined(PSK)
+ if (nbd_set_tls_psk_file (nbd, PSK) == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
--
2.43.0

View File

@ -0,0 +1,33 @@
From 7c92a9c782970c168f12107eb5cf7816d4741710 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 7 Apr 2025 11:35:25 +0100
Subject: [PATCH] copy: Fix crash when blkhash size is not a power of 2
nbdcopy: blkhash.c:105: init_blkhash: Assertion `is_power_of_2 (blkhash_size)' failed.
The check for this was wrong, resulting in a later assertion failure
instead of an error message.
Reported-by: Vera Wu
Fixes: https://issues.redhat.com/browse/RHEL-85513
(cherry picked from commit 6c6e0822c854e423d79bef87caf1c20c5bdb5eb5)
---
copy/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/copy/main.c b/copy/main.c
index 76b09ded..613b9ede 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -220,7 +220,7 @@ main (int argc, char *argv[])
prog, "--blkhash", error, pstr);
exit (EXIT_FAILURE);
}
- if (! is_power_of_2 (blkhash_size)) {
+ if (! is_power_of_2 (i64)) {
fprintf (stderr, "%s: %s is not a power of two: %s\n",
prog, "--blkhash", &optarg[i+1]);
exit (EXIT_FAILURE);
--
2.47.1

View File

@ -1,53 +0,0 @@
From fcb7d28e4dd2ab438c6070e7e5b1aae54cc75f28 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 24 Jun 2024 13:54:48 +0100
Subject: [PATCH] interop: Add -DEXPECT_FAIL=1 where we expect the test to fail
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit c7a8df4f78f2c1901f5c532f262dadd6cce84750)
(cherry picked from commit 175ee89f4a64c52cdb1412a2a72fc8c52fecaf93)
---
interop/interop.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/interop/interop.c b/interop/interop.c
index d4d6671e..469327ee 100644
--- a/interop/interop.c
+++ b/interop/interop.c
@@ -78,6 +78,7 @@ main (int argc, char *argv[])
int64_t actual_size;
char buf[512];
size_t i;
+ int r;
/* Check requirements or skip the test. */
#ifdef REQUIRES
@@ -174,10 +175,21 @@ main (int argc, char *argv[])
#else
#define NBD_CONNECT nbd_connect_command
#endif
- if (NBD_CONNECT (nbd, args) == -1) {
+ r = NBD_CONNECT (nbd, args);
+#if EXPECT_FAIL
+ if (r != -1) {
+ fprintf (stderr, "%s: expected connection to fail but it did not\n",
+ argv[0]);
+ exit (EXIT_FAILURE);
+ }
+ exit (EXIT_SUCCESS);
+ /*NOTREACHED*/
+#else
+ if (r == -1) {
fprintf (stderr, "%s\n", nbd_get_error ());
exit (EXIT_FAILURE);
}
+#endif
#if TLS
if (TLS_MODE == LIBNBD_TLS_REQUIRE) {
--
2.43.0

View File

@ -0,0 +1,66 @@
From b7cd0e53f61fc72be3025f0e1969507279af8842 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Sun, 13 Apr 2025 14:51:09 +0000
Subject: [PATCH] copy: Define block_type outside of block struct
This make the code easier to follow and maintain.
(cherry picked from commit dc5f0e6c79e7aa03ba634b71d4780f6d7d039cdd)
---
copy/blkhash.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/copy/blkhash.c b/copy/blkhash.c
index 622d8a39..526db4d2 100644
--- a/copy/blkhash.c
+++ b/copy/blkhash.c
@@ -43,26 +43,28 @@
#ifdef HAVE_GNUTLS
+/* unknown => We haven't seen this block yet. 'ptr' is NULL.
+ *
+ * zero => The block is all zeroes. 'ptr' is NULL.
+ *
+ * data => The block is all data, and we have seen the whole block,
+ * and the hash has been computed. 'ptr' points to the computed
+ * hash. 'n' is unused.
+ *
+ * incomplete => Part of the block was seen. 'ptr' points to the
+ * data block, waiting to be completed. 'n' is the number of bytes
+ * seen so far. We will compute the hash and turn this into a
+ * 'data' or 'zero' block, either when we have seen all bytes of
+ * this block, or at the end.
+ *
+ * Note that this code assumes that we are called exactly once for a
+ * range in the disk image.
+ */
+enum block_type { block_unknown = 0, block_zero, block_data, block_incomplete };
+
/* We will have one of these structs per blkhash block. */
struct block {
- /* unknown => We haven't seen this block yet. 'ptr' is NULL.
- *
- * zero => The block is all zeroes. 'ptr' is NULL.
- *
- * data => The block is all data, and we have seen the whole block,
- * and the hash has been computed. 'ptr' points to the computed
- * hash. 'n' is unused.
- *
- * incomplete => Part of the block was seen. 'ptr' points to the
- * data block, waiting to be completed. 'n' is the number of bytes
- * seen so far. We will compute the hash and turn this into a
- * 'data' or 'zero' block, either when we have seen all bytes of
- * this block, or at the end.
- *
- * Note that this code assumes that we are called exactly once for a
- * range in the disk image.
- */
- enum { block_unknown = 0, block_zero, block_data, block_incomplete } type;
+ enum block_type type;
void *ptr;
size_t n;
};
--
2.47.1

View File

@ -1,84 +0,0 @@
From c20ac23a9a3673cca863974ec53f9129392fd447 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Mon, 24 Jun 2024 11:39:01 +0100
Subject: [PATCH] interop: Test interop with a bad system CA
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is expected to fail now.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 1c7db8f3337632f0395dac9b13cf03b100cf1a4a)
(cherry picked from commit cb3519eeefa788b8fef466bf9394eefa9d6a6c18)
---
.gitignore | 1 +
interop/Makefile.am | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/.gitignore b/.gitignore
index 0b1cf764..597043e1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -113,6 +113,7 @@ Makefile.in
/interop/interop-nbdkit-tls-certs
/interop/interop-nbdkit-tls-certs-allow-enabled
/interop/interop-nbdkit-tls-certs-allow-fallback
+/interop/interop-nbdkit-tls-certs-bad-CA
/interop/interop-nbdkit-tls-psk
/interop/interop-nbdkit-tls-psk-allow-enabled
/interop/interop-nbdkit-tls-psk-allow-fallback
diff --git a/interop/Makefile.am b/interop/Makefile.am
index 4cdc55e9..bc974b99 100644
--- a/interop/Makefile.am
+++ b/interop/Makefile.am
@@ -281,6 +281,7 @@ check_PROGRAMS += \
interop-nbdkit-tls-certs \
interop-nbdkit-tls-certs-allow-enabled \
interop-nbdkit-tls-certs-allow-fallback \
+ interop-nbdkit-tls-certs-bad-CA \
interop-nbdkit-tls-psk \
interop-nbdkit-tls-psk-allow-enabled \
interop-nbdkit-tls-psk-allow-fallback \
@@ -292,6 +293,7 @@ TESTS += \
interop-nbdkit-tls-certs \
interop-nbdkit-tls-certs-allow-enabled \
interop-nbdkit-tls-certs-allow-fallback \
+ interop-nbdkit-tls-certs-bad-CA \
interop-nbdkit-tls-psk \
interop-nbdkit-tls-psk-allow-enabled \
interop-nbdkit-tls-psk-allow-fallback \
@@ -370,6 +372,30 @@ interop_nbdkit_tls_certs_allow_fallback_LDADD = \
$(GNUTLS_LIBS) \
$(NULL)
+# In this test, nbdkit offers a server certificate signed by our CA in
+# the tests/pki directory, but we deliberately tell libnbd to test
+# against the system CA (-DCERTS=NULL). This is expected to fail the
+# connection with the error:
+# libnbd: debug: nbd1: nbd_connect_command: handle dead: nbd_connect_command: gnutls_handshake: Error in the certificate verification. (15/1)
+interop_nbdkit_tls_certs_bad_CA_SOURCES = \
+ interop.c \
+ requires.c \
+ ../tests/requires.h \
+ $(NULL)
+interop_nbdkit_tls_certs_bad_CA_CPPFLAGS = \
+ $(AM_CPPFLAGS) \
+ -DREQUIRES=' requires ("test -d ../tests/pki"); ' \
+ -DSERVER=\"$(NBDKIT)\" \
+ -DSERVER_PARAMS='"--tls=require", "--tls-certificates=../tests/pki", "-s", "--exit-with-parent", "null"' \
+ -DCERTS=NULL \
+ -DTLS_MODE=LIBNBD_TLS_REQUIRE \
+ -DEXPECT_FAIL=1 \
+ $(NULL)
+interop_nbdkit_tls_certs_bad_CA_LDADD = \
+ $(top_builddir)/lib/libnbd.la \
+ $(GNUTLS_LIBS) \
+ $(NULL)
+
interop_nbdkit_tls_psk_SOURCES = \
interop.c \
requires.c \
--
2.43.0

View File

@ -0,0 +1,78 @@
From 298297a2ac28dc443b64cf0610b53e3c72bf4d39 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Sun, 13 Apr 2025 14:54:31 +0000
Subject: [PATCH] copy: Shrink struct block
Change n to uint32_t since block size bigger than 4g does not make
sense. Move the type field to the end to shrink struct size from 24
bytes to 16.
This minimizes memory usage and improves locality. For example we can
have 4 blocks in a single cache line instead of 2.5.
Testing shows up to 8% improvement in time and 33% in maximum resident
set size with 1000g empty image. With images full of zeros or images
full of non-zero bytes we see lower memory usage but no difference in
time.
| size | content | tool | source | version | memory | time |
|--------|---------|------------|--------|---------|----------|----------|
| 1000g | hole | nbdcopy | file | before | 644716k | 3.33s |
| 1000g | hole | nbdcopy | file | after | 516716k | 3.10s |
| 1000g | hole | nbdcopy | nbd | before | 388844k | 1.13s |
| 1000g | hole | nbdcopy | nbd | after | 260716k | 1.04s |
| 1000g | hole | blksum | nbd | - | 10792k | 0.29s |
| 1000g | hole | sha256sum | file | - | *2796k | *445.00s |
|--------|---------|------------|--------|---------|----------|----------|
| 10g | zero | nbdcopy | file | before | 20236k | 1.33s |
| 10g | zero | nbdcopy | file | after | 18796k | 1.32s |
| 10g | zero | nbdcopy | nbd | before | 32648k | 8.21s |
| 10g | zero | nbdcopy | nbd | after | 31416k | 8.23s |
| 10g | zero | nbdcopy | pipe | before | 19052k | 4.56s |
| 10g | zero | nbdcopy | pipe | after | 17772k | 4.56s |
| 10g | zero | blksum | nbd | - | 13948k | 3.90s |
| 10g | zero | blksum | pipe | - | 10340k | 0.55s |
| 10g | zero | sha256sum | file | - | 2796k | 4.45s |
|--------|---------|------------|--------|---------|----------|----------|
| 10g | data | nbdcopy | file | before | 20224k | 1.28s |
| 10g | data | nbdcopy | file | after | 19036k | 1.26s |
| 10g | data | nbdcopy | nbd | before | 32792k | 8.02s |
| 10g | data | nbdcopy | nbd | after | 31512k | 8.02s |
| 10g | data | nbdcopy | pipe | before | 19052k | 4.56s |
| 10g | data | nbdcopy | pipe | after | 17772k | 4.57s |
| 10g | data | blksum | nbd | - | 13888k | 3.88s |
| 10g | data | blksum | pipe | - | 12512k | 1.10s |
| 10g | data | sha256sum | file | - | 2788k | 4.49s |
* estimated based on 10g image
Measured using:
/usr/bin/time -f "memory=%Mk time=%es" ./nbdcopy --blkhash ...
Tested on Fedora 41 VM on MacBook Pro M2 Max.
(cherry picked from commit f3e1b5fe8423558b49a2b829c0fe13f601b475f2)
---
copy/blkhash.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/copy/blkhash.c b/copy/blkhash.c
index 526db4d2..41253ec8 100644
--- a/copy/blkhash.c
+++ b/copy/blkhash.c
@@ -64,9 +64,9 @@ enum block_type { block_unknown = 0, block_zero, block_data, block_incomplete };
/* We will have one of these structs per blkhash block. */
struct block {
- enum block_type type;
void *ptr;
- size_t n;
+ uint32_t n;
+ enum block_type type;
};
DEFINE_VECTOR_TYPE(blocks, struct block);
--
2.47.1

View File

@ -1,89 +0,0 @@
From a2541de206b3560fdfadf5dfada2cac1b69c09a1 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 25 Jun 2024 11:12:56 +0100
Subject: [PATCH] lib/uri.c: Allow tls-verify-peer to be overridden in URIs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Older versions of libnbd didn't always check the server certificate.
Since some clients might be depending on this, allow
?tls-verify-peer=false in URIs to skip this check.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 75641c6b30155abce272f60cf3518a65654aa401)
(cherry picked from commit b12466821fc534fb68d5b8e695832ee03496e0af)
---
generator/API.ml | 5 +++++
lib/uri.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/generator/API.ml b/generator/API.ml
index c4547615..f2752f25 100644
--- a/generator/API.ml
+++ b/generator/API.ml
@@ -1994,6 +1994,11 @@ Note this is not allowed by default - see next section.
Set the PSK file. See L<nbd_set_tls_psk_file(3)>. Note
this is not allowed by default - see next section.
+=item B<tls-verify-peer=false>
+
+Do not verify the server certificate. See L<nbd_set_tls_verify_peer(3)>.
+The default is C<true>.
+
=back
=head2 Disable URI features
diff --git a/lib/uri.c b/lib/uri.c
index 0c8e87cf..969e88be 100644
--- a/lib/uri.c
+++ b/lib/uri.c
@@ -150,6 +150,31 @@ parse_uri_queries (const char *query_raw, uri_query_list *list)
return -1;
}
+/* Similar to nbdkit_parse_bool */
+int
+parse_bool (const char *param, const char *value)
+{
+ if (!strcmp (value, "1") ||
+ !strcasecmp (value, "true") ||
+ !strcasecmp (value, "t") ||
+ !strcasecmp (value, "yes") ||
+ !strcasecmp (value, "y") ||
+ !strcasecmp (value, "on"))
+ return 1;
+
+ if (!strcmp (value, "0") ||
+ !strcasecmp (value, "false") ||
+ !strcasecmp (value, "f") ||
+ !strcasecmp (value, "no") ||
+ !strcasecmp (value, "n") ||
+ !strcasecmp (value, "off"))
+ return 0;
+
+ set_error (EINVAL, "could not parse %s parameter, expecting %s=true|false",
+ param, param);
+ return -1;
+}
+
int
nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
{
@@ -298,6 +323,13 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const char *raw_uri)
if (nbd_unlocked_set_tls_psk_file (h, queries.ptr[i].value) == -1)
goto cleanup;
}
+ else if (strcasecmp (queries.ptr[i].name, "tls-verify-peer") == 0) {
+ int v = parse_bool ("tls-verify-peer", queries.ptr[i].value);
+ if (v == -1)
+ goto cleanup;
+ if (nbd_unlocked_set_tls_verify_peer (h, v) == -1)
+ goto cleanup;
+ }
}
/* Username. */
--
2.43.0

View File

@ -0,0 +1,65 @@
From fa6a07a8fd5cc3216eb53cd2ad54e9e0dea42036 Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Sun, 13 Apr 2025 23:39:15 +0000
Subject: [PATCH] copy: Enable zero optimization for allocated extents
We optimized zero extents but computed the hash for all data blocks,
including data blocks full of zeros. Detecting a zero block is 20-100
times faster than computing a hash, depending on the machine and the
hash algorithm.
When adding a completed block, detect zero blocks and mark the block as
zero block, saving the computation of the hash and the allocation of the
digest buffer.
This optimization is already implemented for incomplete blocks.
Testing shows that computing a hash for image full of zeros is up to 7.4
times faster, and memory usage is up to 40% lower.
| size | content | tool | source | version | memory | time |
|--------|---------|------------|--------|---------|----------|----------|
| 10g | zero | nbdcopy | file | before | 20236k | 1.33s |
| 10g | zero | nbdcopy | file | after | 13212k | 0.33s |
| 10g | zero | nbdcopy | nbd | before | 32648k | 8.21s |
| 10g | zero | nbdcopy | nbd | after | 24996k | 3.32s |
| 10g | zero | nbdcopy | pipe | before | 19052k | 4.56s |
| 10g | zero | nbdcopy | pipe | after | 11244k | 0.61s |
| 10g | zero | blksum | nbd | - | 13948k | 3.90s |
| 10g | zero | blksum | pipe | - | 10340k | 0.55s |
| 10g | zero | sha256sum | file | - | 2796k | 4.45s |
|--------|---------|------------|--------|---------|----------|----------|
| 10g | data | nbdcopy | file | before | 20224k | 1.28s |
| 10g | data | nbdcopy | file | after | 20400k | 1.28s |
| 10g | data | nbdcopy | nbd | before | 32792k | 8.02s |
| 10g | data | nbdcopy | nbd | after | 32536k | 8.01s |
| 10g | data | nbdcopy | pipe | before | 19052k | 4.56s |
| 10g | data | nbdcopy | pipe | after | 19048k | 4.55s |
| 10g | data | blksum | nbd | - | 13888k | 3.88s |
| 10g | data | blksum | pipe | - | 12512k | 1.10s |
| 10g | data | sha256sum | file | - | 2788k | 4.49s |
(cherry picked from commit efbe283f9fcfc8b4e57370f71356b1bfe7ffd0a4)
---
copy/blkhash.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/copy/blkhash.c b/copy/blkhash.c
index 41253ec8..92ffafbd 100644
--- a/copy/blkhash.c
+++ b/copy/blkhash.c
@@ -213,7 +213,10 @@ set_complete_block (uint64_t blknum, const char *buf)
/* Assert that we haven't seen this block before. */
assert (b.type == block_unknown);
- if (buf) {
+ /* Detecting a zero block is 20-100 times faster than computing a hash
+ * depending on the machine and the algorithm.
+ */
+ if (buf && !is_zero (buf, blkhash_size)) {
b.type = block_data;
/* Compute the hash of the whole block now. */
--
2.47.1

View File

@ -1,31 +0,0 @@
From dfa2a23c7638e325694101fe81b5330ceede68f9 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Tue, 25 Jun 2024 17:53:47 +0100
Subject: [PATCH] docs: security: Add link to TLS server certificate checking
announcement
(cherry picked from commit 9c723aa660c6ee7d224afbfc16eb7450d21fb9cf)
(cherry picked from commit 820f45a58fda50dc7d5e126c55403e33824cffe4)
---
docs/libnbd-security.pod | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/docs/libnbd-security.pod b/docs/libnbd-security.pod
index 216efa43..c9960d8c 100644
--- a/docs/libnbd-security.pod
+++ b/docs/libnbd-security.pod
@@ -45,6 +45,11 @@ negative size result from nbd_get_size(3)
See the full announcement here:
L<https://listman.redhat.com/archives/libguestfs/2023-September/032711.html>
+=head2 multiple flaws in TLS server certificate checking
+
+See the full announcement here:
+L<https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/thread/LHR3BW6RJ7K4BJBQIYV3GTZLSY27VZO2/>
+
=head1 SEE ALSO
L<libnbd(3)>.
--
2.43.0

View File

@ -0,0 +1,39 @@
From fcac97261f26ad486e45dedfdfa6da3ee04fe6ca Mon Sep 17 00:00:00 2001
From: Nir Soffer <nsoffer@redhat.com>
Date: Mon, 14 Apr 2025 21:40:16 +0000
Subject: [PATCH] copy: Fix corrupted hash on incomplete read
When using synchronous read with unknown file size, if the read was
shorter than request size, we updated the hash with the complete buffer,
inserting leftover bytes from the previous read into the hash.
I'm not sure if there is validation for source size and number of blocks
in the blocks vector, so this can generate a corrupted hash silently.
We probably need to validate later that the image size matches the size
of the hashed data.
I could not reproduce a corrupted hash, the issue discovered by reading
the code.
(cherry picked from commit 49cd9fbc0022c0ae5bc5d0b9dd48219dfb92b2f7)
---
copy/synch-copying.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/copy/synch-copying.c b/copy/synch-copying.c
index 4c65c86d..b030a85a 100644
--- a/copy/synch-copying.c
+++ b/copy/synch-copying.c
@@ -49,7 +49,7 @@ synch_copying (void)
size_t r;
while ((r = src->ops->synch_read (src, buf, request_size, offset)) > 0) {
- update_blkhash ((const char *) buf, offset, request_size);
+ update_blkhash ((const char *) buf, offset, r);
dst->ops->synch_write (dst, buf, r, offset);
offset += r;
progress_bar (offset, src->size);
--
2.47.1

View File

@ -1,32 +0,0 @@
From 8334404ee0883dcfa90697b6fdae541ed4751b79 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones@redhat.com>
Date: Thu, 1 Aug 2024 15:17:29 +0100
Subject: [PATCH] docs/libnbd-security.pod: Assign CVE-2024-7383
CVE-2024-7383 was assigned to the (already published & fixed) flaws
found in libnbd certificate checking.
Reported-by: Jon Szymaniak
Thanks: Mauro Matteo Cascella
(cherry picked from commit 81a22ac6697ccdeb13509aba3072609251d1378b)
---
docs/libnbd-security.pod | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/docs/libnbd-security.pod b/docs/libnbd-security.pod
index c9960d8c..ece0cf5a 100644
--- a/docs/libnbd-security.pod
+++ b/docs/libnbd-security.pod
@@ -45,7 +45,8 @@ negative size result from nbd_get_size(3)
See the full announcement here:
L<https://listman.redhat.com/archives/libguestfs/2023-September/032711.html>
-=head2 multiple flaws in TLS server certificate checking
+=head2 CVE-2024-7383
+multiple flaws in TLS server certificate checking
See the full announcement here:
L<https://lists.libguestfs.org/archives/list/guestfs@lists.libguestfs.org/thread/LHR3BW6RJ7K4BJBQIYV3GTZLSY27VZO2/>
--
2.43.0

View File

@ -6,7 +6,7 @@ set -e
# directory. Use it like this:
# ./copy-patches.sh
rhel_version=9.4
rhel_version=9.7
# Check we're in the right directory.
if [ ! -f libnbd.spec ]; then

View File

@ -1,17 +0,0 @@
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEE93dPsa0HSn6Mh2fqkXOPc+G3aKAFAmU2izoRHHJpY2hAYW5u
ZXhpYS5vcmcACgkQkXOPc+G3aKA+txAAkLeWdvH2ryibEyqMeyejvh9vMgQO5I46
LaygI8jDi+XG+rGy7imiwIxxWvyCZI3y2U5MFudLZoFi+gCyVAC+LeBxjF41NBGz
fbgwFaQHrCbxyLlsj9OcR6M0+EPU8NXXPPGgXZNcnf7tHNZkTO0OGS9chml0wXHA
Zx9WheHl6wbLTVIAtLWOJqzRQj80RlcPC+De1wZL+WFMPMkfF8L8K5FRNsfeTIXn
l31d1R0g5QOMTTqBiKE2iopPmVmA5uC/adWCuqF3mzzjzCkHp+Ux/Ys99tkCETrU
jUuHgJ+1pYjn4Lmt/HUwXQZD3L+RkNAWWQziY/3ejK31tGxZqR/XTwq5RPrc6Qs1
/zuoWvSWJZZo9yvX1Iq2RQAaZF5724V/svm+HgaCakaK8EJEj6sntM0OhAl2pRC4
G45Kb2o7k016WgL8plNOlrbHNxaruBcPrkFYDMoyy3KLnWaw3OYMARTD5w/Pd4dC
CJa3tIXIKedhXw9xDtEWfxiKIHfO+LBHBMjpW9KzP/oxE2akmcJZJT+JNpsrpdzV
O6mbVDPedWd5LQQ1bNmwzxkCsMEC9HFhVbCaTuYuSXe/By04Norns4xyEJyDXlG/
QqFgEUS8R+V9xwYHoAPg5RUmycXubSmm64iTAQNqc46QsxeP0Va7gpbrPLe5qVk/
irUsxdBhGIM=
=pgmp
-----END PGP SIGNATURE-----

View File

@ -0,0 +1,17 @@
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEE93dPsa0HSn6Mh2fqkXOPc+G3aKAFAmb2th8RHHJpY2hAYW5u
ZXhpYS5vcmcACgkQkXOPc+G3aKCt/w//TKKhSrKc1CHZjKQpDF7pCe9WpCIDsyAP
HaVIX4aBe+b6FoYyfnJG6D/o3qvv1yllaGRM77gttCBVmeTb/2pjrjOkxX1GcsMw
yqW31YSNo2rkKCVFOGa3RggJb5Tn++g/IuoCI0sF0OZuejqkOIPLQ/yK3HlU01FE
E4C2aZlbAn11RZ8Rz/uVfiNMmnUn9IqzyHozloIQNZjRGMFuOw4DAFPDc3wtq/yz
ckTur09yGSLLu2q3vnbqdpJBvjoAWFv5F3UlxHtSz5tBaxyPQKnmI04Q8r3IiM+8
hqZJ74dzeVtmfXf8Y8dFb40dYxn5OWrCj01qZu15RzhIhoOrhJcSBth1QPv0LXQS
6b3n5Nx+rC4QzRPvEi0g4uBMgqdfBW5WesykbdH3+lSoxAJQgGwJXc2ceMzN9zGU
+wvGvX1wuTqtszEde1ydcpC3UayDDoMQYuY2QjJW0QbzPT1P884Mydc6zC7PCu8A
QTMUbqdUOyg4i+jFjWMWrbU0Db9Kv5QaIrDGoR+cxlERYQh6rLA3kS7RDZexKlkc
N2KrHbWJ/O24/JtLMqnl3X7Xcyek1MiQwN1SaEQl0QMmHGG7pF7MnZ3aVWmkhjlO
J/FkBN+2hCQQPaANHCjPT/tDjqo7PcIAuUxP/fEp6048zk4BnAEgV4T4u10mz3mr
xy8jdSC39iI=
=pc1A
-----END PGP SIGNATURE-----

View File

@ -1,14 +1,26 @@
# 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
Version: 1.20.3
Release: 4%{?dist}
Summary: NBD client library in userspace
@ -26,30 +38,29 @@ Source2: libguestfs.keyring
Source3: copy-patches.sh
# Patches are stored in the upstream repository:
# https://gitlab.com/nbdkit/libnbd/-/commits/rhel-9.4/
# https://gitlab.com/nbdkit/libnbd/-/commits/rhel-9.6/
# 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
Patch0004: 0004-build-Move-to-minimum-gnutls-3.5.18.patch
Patch0005: 0005-lib-crypto.c-Check-server-certificate-even-when-usin.patch
Patch0006: 0006-lib-crypto.c-Allow-CA-verification-even-if-h-hostnam.patch
Patch0007: 0007-interop-Pass-DCERTS-and-DPSK-as-strings.patch
Patch0008: 0008-interop-Add-DEXPECT_FAIL-1-where-we-expect-the-test-.patch
Patch0009: 0009-interop-Test-interop-with-a-bad-system-CA.patch
Patch0010: 0010-lib-uri.c-Allow-tls-verify-peer-to-be-overridden-in-.patch
Patch0011: 0011-docs-security-Add-link-to-TLS-server-certificate-che.patch
Patch0012: 0012-docs-libnbd-security.pod-Assign-CVE-2024-7383.patch
%if 0%{patches_touch_autotools}
BuildRequires: autoconf, automake, libtool
%endif
Patch0001: 0001-generator-Print-full-error-in-handle_reply_error.patch
Patch0002: 0002-lib-Don-t-overwrite-error-in-nbd_opt_-go-info.patch
Patch0003: 0003-generator-Restore-assignment-to-local-err.patch
Patch0004: 0004-generator-states-newstyle.c-Quote-untrusted-string-f.patch
Patch0005: 0005-generator-states-newstyle.c-Don-t-sign-extend-escape.patch
Patch0006: 0006-copy-Set-the-total-size-in-bytes-copied.patch
Patch0007: 0007-copy-Add-blkhash-option.patch
Patch0008: 0008-copy-Fix-crash-when-blkhash-size-is-not-a-power-of-2.patch
Patch0009: 0009-copy-Define-block_type-outside-of-block-struct.patch
Patch0010: 0010-copy-Shrink-struct-block.patch
Patch0011: 0011-copy-Enable-zero-optimization-for-allocated-extents.patch
Patch0012: 0012-copy-Fix-corrupted-hash-on-incomplete-read.patch
%if 0%{verify_tarball_signature}
BuildRequires: gnupg2
%endif
# For rebuilding autoconf cruft.
BuildRequires: autoconf, automake, libtool
# For the core library.
BuildRequires: gcc
BuildRequires: make
@ -60,7 +71,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
@ -69,7 +80,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
@ -88,7 +99,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
@ -109,7 +120,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
@ -145,7 +156,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}
@ -191,7 +202,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}
@ -224,25 +235,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}
@ -256,16 +272,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 ()
@ -277,12 +288,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
@ -333,7 +338,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
@ -372,7 +377,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*
@ -392,9 +397,24 @@ make %{?_smp_mflags} check || {
%changelog
* Tue Aug 27 2024 Richard W.M. Jones <rjones@redhat.com> - 1.18.1-4
- Fix CVE-2024-7383 NBD server improper certificate validation
resolves: RHEL-52730
* Tue Apr 15 2025 Richard W.M. Jones <rjones@redhat.com> - 1.20.3-4
- Add nbdcopy --blkhash option
resolves: RHEL-85509
* Sat Sep 28 2024 Richard W.M. Jones <rjones@redhat.com> - 1.20.3-1
- Rebase to libnbd 1.20.3
* Fri Jul 26 2024 Richard W.M. Jones <rjones@redhat.com> - 1.20.2-2
- Rebase to libnbd 1.20.2
resolves: RHEL-31883
- Fix multiple flaws in TLS server certificate checking
resolves: RHEL-49801
- Print full NBD error from server
resolves: RHEL-50665
* 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