[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libguestfs
Subject:    =?utf-8?q?=5BLibguestfs=5D?= [RFC PATCH] server: Advertise nicer size for NBD_OPT_INFO
From:       Eric Blake <eblake () redhat ! com>
Date:       2024-02-21 20:35:25
Message-ID: 20240221203813.1776879-1-eblake () redhat ! com
[Download RAW message or body]

NBD_OPT_GO either advertises the actual size (possibly with an
override from the command line or config file), the value OFFT_MAX (if
multitree or F_WAIT means computing a real size would take too long),
or the value UINT64_MAX (if size_autodetect fails, such as when fd is
non-seekable); the only time it ever advertises a size of 0 is when it
is serving a regular file that really is empty.

On the other hand, NBD_OPT_INFO had been blindly advertising 0 no
matter what.  While we can't always compute a real size, we can do a
lot better by advertising the same sentinels as NBD_OPT_GO.

Signed-off-by: Eric Blake <eblake@redhat.com>
---

Rich has already chimed in on my question about what size SHOULD
nbd-server report when NBD_OPT_GO wants to list an indeterminate size;
if we start changing the spec, then this may be incomplete or need
tweaking.  Here, I only tackled the simple case of a single-file (or
block device) export; there are multifile cases where NBD_OPT_GO still
reports a more accurate number than this.  There's also the question
of whether we want to address the information leak (right now, the
code can succeed on NBD_OPT_INFO even when it will later fail for
NBD_OPT_GO because the client is not authorized to used the export).

 nbd-server.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/nbd-server.c b/nbd-server.c
index 875c16f..f1a0d2f 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -2265,6 +2265,43 @@ static void setup_transactionlog(CLIENT *client) {
 	}
 }

+/**
+  * Probe details on the chosen export
+  *
+  * When a client sends NBD_OPT_INFO, we try to determine the export size.
+  * This function could also verify whether the client is allowed access,
+  * but for now, we do not deem the information leak of NBD_OPT_INFO
+  * succeeding where NBD_OPT_GO would fail to be severe.
+  *
+  * @param client the CLIENT structure
+  * @param server the SERVER structure
+  * @return true if the client is allowed access to the export, false
+  * otherwise
+  */
+static bool probe_client(CLIENT* client, SERVER* server) {
+	int fd;
+	gchar* error_string;
+
+	client->exportsize = server->expected_size ?
+		server->expected_size : OFFT_MAX;
+	if(!(server->flags & (F_MULTIFILE | F_TREEFILES | F_TEMPORARY |
+			      F_WAIT))) {
+		DEBUG( "Opening %s\n", server->exportname);
+		fd = open(server->exportname, O_RDONLY);
+		if(fd == -1) {
+			error_string=g_strdup_printf(
+				"Could not open exported file %s: %%m",
+				server->exportname);
+			err_nonfatal(error_string);
+			return server->expected_size > 0;
+		}
+		client->exportsize = size_autodetect(fd);
+		close(fd);
+	}
+	// Is it worth returning false if client is not authorized?
+	return true;
+}
+
 /**
   * Commit to exporting the chosen export
   *
@@ -2612,6 +2649,13 @@ static bool handle_info(CLIENT* client, uint32_t opt, GArray* servers, uint32_t
 			send_reply(client, opt, NBD_REP_ERR_POLICY, -1, "Access denied by server configuration");
 			return false;
 		}
+	} else {
+		if(!probe_client(client, server)) {
+			consume(client, n_requests * sizeof(request), buf,
+				sizeof(buf));
+			send_reply(client, opt, NBD_REP_ERR_POLICY, -1, "Access denied by server configuration");
+			return false;
+		}
 	}
 	for(i=0; i<n_requests; i++) {
 		socket_read(client, &request, sizeof(request));
-- 
2.43.2
_______________________________________________
Libguestfs mailing list -- guestfs@lists.libguestfs.org
To unsubscribe send an email to guestfs-leave@lists.libguestfs.org

[prev in list] [next in list] [prev in thread] [next in thread]