search for: nbd_opt_starttl

Some of these affect attempts to connect to older qemu-nbd versions, some of them were triggered by manual edits to qemu-nbd source code to provoke various other compliant (if uncommon) server behaviors. Eric Blake (4): starttls: Skip error payload if falling back to unencrypted states: Reject payload to NBD_REP_ACK meta-context: Skip error payload if server lacks meta_context states: Add

...erator/generator b/generator/generator index e3dd10f..a289741 100755 --- a/generator/generator +++ b/generator/generator @@ -373,8 +373,8 @@ and newstyle_opt_starttls_state_machine = [ State { default_state with - name = "SKIP_REPLY_PAYLOAD"; - comment = "Skip newstyle NBD_OPT_STARTTLS reply payload"; + name = "RECV_REPLY_PAYLOAD"; + comment = "Receive any newstyle NBD_OPT_STARTTLS reply payload"; external_events = [ NotifyRead, "" ]; }; @@ -425,8 +425,8 @@ and newstyle_opt_structured_reply_state_machine = [ State { defa...

...ting. + =item Structured Replies I<Not supported>. diff --git a/src/protocol.h b/src/protocol.h index 792a905..088dcab 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -98,6 +98,7 @@ struct fixed_new_option_reply { #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 #define NBD_OPT_STARTTLS 5 +#define NBD_OPT_INFO 6 #define NBD_OPT_GO 7 #define NBD_REP_ACK 1 diff --git a/src/connections.c b/src/connections.c index 1b40e46..410a893 100644 --- a/src/connections.c +++ b/src/connections.c @@ -636,6 +636,7 @@ _negotiate_handshake_newstyle_options (struct...

I'm still playing with ideas on how to split rstate from wstate (so that we can send a request without waiting for POLLIN to complete a pending reply), but this is some preliminary refactoring I found useful. I also fixed a couple of bugs while in the area (already pushed). There's a question of whether we want nbd_handle to be nearly 5k, or if we should instead keep it small and add one

On Fri, Aug 14, 2020 at 05:00:25PM -0500, Eric Blake wrote: > diff --git a/lib/internal.h b/lib/internal.h > index 5f495fb..03baacd 100644 > --- a/lib/internal.h > +++ b/lib/internal.h > @@ -101,6 +101,7 @@ struct nbd_handle { > > /* Option negotiation mode. */ > bool opt_mode; > + uint8_t current_opt; Be nice to add a comment here about what current_opt can

...name list. */ + debug ("newstyle negotiation: %s: advertising exports", + name_of_nbd_opt (option)); + if (send_newstyle_option_reply_exportnames (option, &nr_options) == -1) + return -1; + list_seen = true; + } break; case NBD_OPT_STARTTLS: @@ -826,8 +847,8 @@ negotiate_handshake_newstyle_options (void) break; } - if (nr_options >= MAX_NR_OPTIONS) { - nbdkit_error ("client exceeded maximum number of options (%d)", - MAX_NR_OPTIONS); + if (nr_options == 0) { + nbdkit_error ("clien...

...ator/states-newstyle-opt-starttls.c index e994ffd..61f254f 100644 --- a/generator/states-newstyle-opt-starttls.c +++ b/generator/states-newstyle-opt-starttls.c @@ -83,9 +83,10 @@ return 0; default: - if (!NBD_REP_IS_ERR (reply)) - debug (h, - "server is confused by NBD_OPT_STARTTLS, continuing anyway"); + if (handle_reply_error (h) == -1) { + SET_NEXT_STATE (%.DEAD); + return -1; + } /* Server refused to upgrade to TLS. If h->tls is not require (2) * then we can continue unencrypted. @@ -100,7 +101,6 @@ debug (h, "se...

...ource and binary forms, with or without @@ -98,12 +98,13 @@ extern const char *name_of_nbd_flag (int); /* NBD options (new style handshake only). */ extern const char *name_of_nbd_opt (int); -#define NBD_OPT_EXPORT_NAME 1 -#define NBD_OPT_ABORT 2 -#define NBD_OPT_LIST 3 -#define NBD_OPT_STARTTLS 5 -#define NBD_OPT_INFO 6 -#define NBD_OPT_GO 7 +#define NBD_OPT_EXPORT_NAME 1 +#define NBD_OPT_ABORT 2 +#define NBD_OPT_LIST 3 +#define NBD_OPT_STARTTLS 5 +#define NBD_OPT_INFO 6 +#define NBD_OPT_GO 7...

...ON_LENGTH) { - nbdkit_error ("client option data too long (%" PRIu32 ")", optlen); - return -1; - } - - option = be32toh (new_option.option); - - /* In --tls=require / FORCEDTLS mode the only options allowed - * before TLS negotiation are NBD_OPT_ABORT and NBD_OPT_STARTTLS. - */ - if (tls == 2 && !conn->using_tls && - !(option == NBD_OPT_ABORT || option == NBD_OPT_STARTTLS)) { - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_TLS_REQD)) - return -1; - continue; - } - - switch (option) { - case NBD_...

...rsing has successfully finished negotiation + * it will jump to this state for final steps before moving to + * the %READY state. + *) + State { + default_state with + name = "FINISHED"; + comment = "Finish off newstyle negotiation"; + }; ] (* Fixed newstyle NBD_OPT_STARTTLS option. *) @@ -1574,6 +1584,20 @@ are free to pass in other contexts." "L<nbd_block_status(3)>"; "L<nbd_aio_block_status(3)>"]; }; + "get_protocol", { + default_call with + args = []; ret = RStaticString; + permitted_st...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

...ewstyle_opt_structured_reply_state_machine); Group ("OPT_SET_META_CONTEXT", newstyle_opt_set_meta_context_state_machine); Group ("OPT_GO", newstyle_opt_go_state_machine); + Group ("OPT_EXPORT_NAME", newstyle_opt_export_name_state_machine); ] (* Fixed newstyle NBD_OPT_STARTTLS option. *) @@ -565,6 +566,44 @@ and newstyle_opt_go_state_machine = [ }; ] +(* Newstyle NBD_OPT_EXPORT_NAME option. *) +and newstyle_opt_export_name_state_machine = [ + State { + default_state with + name = "START"; + comment = "Try to send newstyle NBD_OPT_EXPORT_NAM...

...eroes[124]; /* must be sent as zero bytes */ -} __attribute__((packed)); - /* Global flags. */ #define NBD_FLAG_FIXED_NEWSTYLE 1 #define NBD_FLAG_NO_ZEROES 2 @@ -105,15 +98,35 @@ struct new_handshake_finish { #define NBD_OPT_ABORT 2 #define NBD_OPT_LIST 3 #define NBD_OPT_STARTTLS 5 +#define NBD_OPT_GO 7 #define NBD_REP_ACK 1 #define NBD_REP_SERVER 2 +#define NBD_REP_INFO 3 #define NBD_REP_ERR_UNSUP 0x80000001 #define NBD_REP_ERR_POLICY 0x80000002 #define NBD_REP_ERR_INVALID 0x80000003 #define NBD_REP_ERR_PLATFORM 0x8000000...

This is similar to the recent --no-sr option - it's a change that is unlikely to ever be used except by someone testing whether a client is compliant to the protocol, but in that niche case, it can be quite handy (it's a lot nicer to be able to purposefully cripple a server from the command line than from a one-off compile, when testing if a client's fallback for a spec-compliant but

...that error for a later `NBD_OPT_GO` without a client request for > > + `NBD_INFO_BLOCK_SIZE`, since the use of extended headers provides > > + more incentive for a client to promise to obey block size > > + constraints. > > + > > + If the client requests `NBD_OPT_STARTTLS` after this option, it > > + MUST renegotiate extended headers. > > + > > Does it make sense here to also forbid use of NBD_OPT_EXPORT_NAME? I > think the sooner we get rid of that, the better ;-) I hadn't thought of that, but it does indeed sound desirable. I can to...

...t - enabling the `NBD_OPT_STRUCTURED_REPLY` extension. + extensions until the client has negotiated either structured + replies or extended headers; and a client MUST NOT make use of + those extensions without first enabling support for reply + payloads. If the client requests `NBD_OPT_STARTTLS` after this option, it MUST renegotiate structured replies and any other dependent @@ -1370,9 +1475,10 @@ of the newstyle negotiation. Return a list of `NBD_REP_META_CONTEXT` replies, one per context, followed by an `NBD_REP_ACK` or an error. - This option SHOULD NOT be requeste...

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.

When LIBNBD_TLS_ALLOW is used we don't have a way to find out if TLS was really negotiated. This adds a flag and a way to read it back. Unfortunately there is no test yet, because LIBNBD_TLS_ALLOW is not tested -- it really should be but requires quite a complicated set of tests because ideally we'd like to find out whether it falls back correctly for all supported servers. --- TODO

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

...e { debug (h, "continuing with unencrypted connection"); - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); + SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START); } return 0; } @@ -185,7 +185,7 @@ NEWSTYLE.OPT_STARTTLS.TLS_HANDSHAKE_DONE: if (h->opt_current == NBD_OPT_STARTTLS) SET_NEXT_STATE (%.NEGOTIATING); else - SET_NEXT_STATE (%^OPT_STRUCTURED_REPLY.START); + SET_NEXT_STATE (%^OPT_EXTENDED_HEADERS.START); return 0; } /* END STATE MACHINE */ diff --git a/generator/Makefile.am b/generator/Makefile.am index c3d53b26..c8477842 100644 --- a/generator/...