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

Print full NBD error from server

Browse Source 
resolves: RHEL-50667
This commit is contained in:
Richard W.M. Jones 2024-07-26 15:10:52 +01:00
parent a21be9a253
commit c0cdb87a80
7 changed files with 488 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

191
0001-generator-Print-full-error-in-handle_reply_error.patch Normal file
View File

@ -0,0 +1,191 @@
From 9e51ca3dc11b4abe2e5145837ae80863fc300646 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.43.0

38
0002-lib-Don-t-overwrite-error-in-nbd_opt_-go-info.patch Normal file
View File

@ -0,0 +1,38 @@
From 6ef03cf2a1fe9f88c07c6f2d97afe9f82bfe03a8 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.43.0

43
0003-generator-Restore-assignment-to-local-err.patch Normal file
View File

@ -0,0 +1,43 @@
From 1357302046b5eaae09a8817ff050790b0285183d 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.43.0

175
0004-generator-states-newstyle.c-Quote-untrusted-string-f.patch Normal file
View File

@ -0,0 +1,175 @@
From 1de017428047f1a8991285766b69b767ab895c24 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.43.0

27
0005-generator-states-newstyle.c-Don-t-sign-extend-escape.patch Normal file
View File

@ -0,0 +1,27 @@
From f24c7801aef0e2f8936d74ac5237c3391fb39d26 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.43.0

2
copy-patches.sh
View File

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

15
libnbd.spec
View File

@ -21,7 +21,7 @@
Name: libnbd
Version: 1.20.2
Release: 1%{?dist}
Release: 2%{?dist}
Summary: NBD client library in userspace
License: LGPL-2.0-or-later AND BSD-3-Clause
@ -37,6 +37,15 @@ Source2: libguestfs.keyring
# Maintainer script which helps with handling patches.
Source3: copy-patches.sh
# Patches are stored in the upstream repository:
# https://gitlab.com/nbdkit/libnbd/-/commits/rhel-10.0/
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
%if 0%{verify_tarball_signature}
BuildRequires: gnupg2
%endif
@ -376,10 +385,12 @@ make %{?_smp_mflags} check || {
%changelog
* Fri Jul 19 2024 Richard W.M. Jones <rjones@redhat.com> - 1.20.2-1
* Fri Jul 26 2024 Richard W.M. Jones <rjones@redhat.com> - 1.20.2-2
- Rebase to libnbd 1.20.2
- Fix multiple flaws in TLS server certificate checking
resolves: RHEL-49802
- Print full NBD error from server
resolves: RHEL-50667
* Tue Jun 25 2024 Troy Dawson <tdawson@redhat.com> - 1.20.1-5
- Bump release for June 2024 mass rebuild