search for: nbd_rep_info

...reply magic or option"); - return -1; - } + switch (reply) { case NBD_REP_ACK: - if (len != 0) { - SET_NEXT_STATE (%.DEAD); - set_error (0, "handshake: invalid option reply length"); - return -1; - } SET_NEXT_STATE (%.READY); return 0; case NBD_REP_INFO: - if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) { + if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) debug (h, "skipping large NBD_REP_INFO"); - } - else if (len < sizeof h->sbuf.or.payload.export.info) { - SET_NEXT_STATE (%.DEAD...

...I'm not sure that it matters much (outside of libnbd, nbd_handle is an opaque type always referenced as a pointer). Eric Blake (7): states: Log structured error messages, if any states: Refactor SET_META_CONTEXT reply parsing states: Allow large SET_CONTEXT_NAME replies states: Rewrite NBD_REP_INFO parsing states: Factor out NBD_REP payload prep states: Give up on oversized reply length states: Capture NBD_REP_ERR message generator/generator | 19 ++-- generator/states-newstyle-opt-go.c | 71 ++++++------- .../states-newstyle-opt-set-meta-context.c...

...3fb4e 100644 --- a/lib/nbd-protocol.h +++ b/lib/nbd-protocol.h @@ -123,15 +123,19 @@ struct nbd_fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_CONTEXT 4 -#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) -#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) -#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) +#define NBD_REP_ACK...

On 08/06/2018 09:31 AM, Nir Soffer wrote: > Eric, can you point us to the part of the spec allowing ignoring the export > name sent by the client? Nothing in the NBD spec requires the server to reject unknown export names. So nbdkit never rejects export names (which means it behaves as if all names work, regardless of whether or not it was the name it was configured with, since it

...ite_full (h->fd, &nrinfos, sizeof nrinfos)) { + nbdkit_error ("unable to request NBD_OPT_GO: %m"); + return -1; + } + while (1) { + if (nbd_newstyle_recv_option_reply (h, NBD_OPT_GO, &reply, &buffer) < 0) + return -1; + switch (reply.reply) { + case NBD_REP_INFO: + /* Parse payload, but ignore all except NBD_INFO_EXPORT */ + if (reply.replylen < 2) { + nbdkit_error ("NBD_REP_INFO reply too short"); + free (buffer); + return -1; + } + memcpy (&info, buffer, sizeof info); + info = be16toh (info);...

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. Rich.

...mu client in particular does not request this, but will + * fail if we don't send it. + */ + if (get_size_and_eflags (conn) == -1) + return -1; + if (send_newstyle_option_reply_info_export (conn, option, + NBD_REP_INFO, + NBD_INFO_EXPORT) == -1) + return -1; + + /* For now we ignore all other info requests (but we must + * ignore NBD_INFO_EXPORT if it was requested, because we + * replied already above). Therefore this loop doesn&...

...are replying to */ uint32_t reply; /* NBD_REP_* */ @@ -110,33 +119,37 @@ struct fixed_new_option_reply { #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -#define NBD_REP_ACK 1 -#define NBD_REP_SERVER 2 -#define NBD_REP_INFO 3 -#define NBD_REP_META_CONTEXT 4 -#define NBD_REP_ERR_UNSUP NBD_REP_ERR (1) -#define NBD_REP_ERR_POLICY NBD_REP_ERR (2) -#define NBD_REP_ERR_INVALID NBD_REP_ERR (3) -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) +#define NBD_REP_ACK...

...export are not met (e.g., the client did not initiate TLS for an export where the server requires it), the server MUST terminate the session. and for NBD_OPT_GO, we have NBD_REP_ERR_UNKNOWN: The chosen export does not exist on this server. In this case, the server SHOULD NOT send NBD_REP_INFO replies. And the spec also says: If the server is unwilling to allow the export, it MUST terminate the session Why would a server want to allow an export using different name? What is the point of the export name in this case? Again, if export name is not useful when a server export only on...

This is only lightly tested (against just qemu NBD client), and the code might be structured a little better as the _negotiate_handshake_newstyle_options function has now grown to be huge. Anyway works for me. Rich.

...if (send_newstyle_option_reply (conn, option, + NBD_REP_ERR_UNKNOWN) == -1) + return -1; + continue; + } if (send_newstyle_option_reply_info_export (conn, option, NBD_REP_INFO, @@ -497,7 +513,7 @@ negotiate_handshake_newstyle_options (struct connection *conn) } /* Unlike NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK - * or ERROR packet. + * or ERROR packet. If this was NBD_OPT_LIST, call .close. */ if (send_newstyle_option_r...

...1 #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 @@ -101,6 +104,7 @@ struct fixed_new_option_reply { #define NBD_OPT_INFO 6 #define NBD_OPT_GO 7 +extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 @@ -110,6 +114,7 @@ struct fixed_new_option_reply { #define NBD_REP_ERR_PLATFORM 0x80000004 #define NBD_REP_ERR_TLS_REQD 0x80000005 +extern const char *name_of_nbd_info (int); #define NBD_INFO_EXPORT 0 /* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */ @@ -148,6 +1...

...b/generator/states-newstyle-opt-go.c index 49875a5..d0d2123 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -108,7 +108,7 @@ switch (reply) { case NBD_REP_ACK: - SET_NEXT_STATE (%.READY); + SET_NEXT_STATE (%^FINISHED); return 0; case NBD_REP_INFO: if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c index c8f817e..7742ea3 100644 --- a/generator/states-newstyle.c +++ b/generator/states-newstyle.c @@ -155,4 +155,13 @@ handle_reply_error (struct nbd_handle *h)...

...if we don't send it. */ - if (finish_newstyle_options (conn) == -1) + if (finish_newstyle_options (conn, &exportsize) == -1) return -1; if (send_newstyle_option_reply_info_export (conn, option, NBD_REP_INFO, - NBD_INFO_EXPORT) == -1) + NBD_INFO_EXPORT, + exportsize) == -1) return -1; /* For now we ignore all other info requests (b...

..." + "NBD_INFO_NAME: no name to send", optname); + break; + } + namelen = -1; + } + if (send_newstyle_option_reply_info_str (option, + NBD_REP_INFO, + NBD_INFO_NAME, + name, namelen) == -1) + return -1; + } + break; default: debug ("newstyle negotiation: %s: "...

...3 @@ -113,7 +110,6 @@ extern const char *name_of_nbd_opt (int); #define NBD_REP_ERR(val) (0x80000000 | (val)) #define NBD_REP_IS_ERR(val) (!!((val) & 0x80000000)) -extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 @@ -124,7 +120,6 @@ extern const char *name_of_nbd_rep (int); #define NBD_REP_ERR_PLATFORM NBD_REP_ERR (4) #define NBD_REP_ERR_TLS_REQD NBD_REP_ERR (5) -extern const char *name_of_nbd_info (int); #define NBD_INFO_EXPORT 0 /* NBD_INFO_EXPORT reply (follows fixed_new_option_rep...

We should have only one NBD protocol file. Let's make nbdkit's version the canonical one, and use it in libnbd. Rich.

I'm hoping to implement .extents for the nbd plugin; this is a prerequisite. I'm not sure about patch 3 - if we like it, I'll squash it to 2, if we don't, I think we are okay just dropping it. I'm also wondering if we have to worry about malicious plugins that don't populate the entire .pread buffer in an effort to get nbdkit to expose portions of the heap; my patch 7 loses

...crypto_init (bool tls_set_on_cli); diff --git a/server/protocol.h b/server/protocol.h index 06b917e..a7de2f0 100644 --- a/server/protocol.h +++ b/server/protocol.h @@ -112,6 +112,7 @@ extern const char *name_of_nbd_rep (int); #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 #define NBD_REP_INFO 3 +#define NBD_REP_META_CONTEXT 4 #define NBD_REP_ERR_UNSUP 0x80000001 #define NBD_REP_ERR_POLICY 0x80000002 #define NBD_REP_ERR_INVALID 0x80000003 @@ -128,6 +129,18 @@ struct fixed_new_option_reply_info_export { uint16_t eflags; /* per-export flags */ } __attribut...

...rrect NBD_INFO_INIT_STATE option reply length"); + return 0; + } + h->initflags = be16toh (h->sbuf.or.payload.init.flags); + break; default: /* XXX Handle other info types, like NBD_INFO_BLOCK_SIZE */ debug (h, "skipping unknown NBD_REP_INFO type %d", diff --git a/interop/Makefile.am b/interop/Makefile.am index 345be7c..ed5b553 100644 --- a/interop/Makefile.am +++ b/interop/Makefile.am @@ -1,5 +1,5 @@ # nbd client library in userspace -# Copyright (C) 2013-2019 Red Hat Inc. +# Copyright (C) 2013-2020 Red Hat Inc. # # This libra...