Re: [PATCH] spec: Clarify NBD_FLAG_ vs. MitM downgrade attack
On Wed, Aug 25, 2021 at 09:15:41AM -0500, Eric Blake wrote:
> Codify the fact that downgrade attacks are possible not only by
> manipulation of NBD_OPT_STARTTLS, but also by manipulation of the
> NBD_FLAG[_C]_* handshake flags. To ensure we don't accidentally
> introduce a new MitM attack vector, we want the specification to
> clearly document that controlling any new protocol changes prior to
> TLS is unwise, and therefore we are unlikely to add any new handshake
> flags.
>
> Viewed from another perspective, the 16 bits for handshake flags
> control the protocol used during NBD_OPT_*, but what we have with
> NBD_FLAG_FIXED_NEWSTYLE is already fairly robust for future extension
> (since all but NBD_OPT_EXPORT_NAME encode a length, and we've gone to
> great lengths to document what servers and clients should do with
> unknown requests). Meanwhile, any extension that wants to affect the
> protocol used by transmission phase, such as structured replies, is
> fine waiting until after TLS is started.
>
> The expense of an extra round trip or two during NBD_OPT_ haggling
> pales in comparison to the amount of data that will go over the wire
> during transmission phase; and if startup efficiency really matters,
> we could add a new NBD_OPT_ that does more things in one round trip
> (where the fallback is still the older one-at-a-time approach).
> ---
> doc/proto.md | 42 +++++++++++++++++++++++++++---------------
> 1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/doc/proto.md b/doc/proto.md
> index 9dd59da..8044886 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -663,13 +663,16 @@ or using other exports.
>
> If a client supports TLS, it SHOULD use `NBD_OPT_GO`
> (if the server supports it) in place
> -of `NBD_OPT_EXPORT_NAME`. The reason for this is set out in
> +of `NBD_OPT_EXPORT_NAME`. One reason for this is set out in
> the final paragraphs of the sections under 'FORCEDTLS'
> and 'SELECTIVETLS': this gives an opportunity for the
> server to transmit that an error going into transmission
> mode is due to the client's failure to initiate TLS,
> and the fact that the client may obtain information about
> -which exports are TLS-only through `NBD_OPT_INFO`.
> +which exports are TLS-only through `NBD_OPT_INFO`. Another reason is
> +that the handshake flag `NBD_FLAG_C_NO_ZEROES` can be altered by a
> +MitM downgrade attack, which can cause a protocol mismatch with
> +`NBD_OPT_EXPORT_NAME` but not with `NBD_OPT_GO`.
>
> ### Security considerations
>
> @@ -691,18 +694,20 @@ the NBD protocol.
>
> There are two main dangers:
>
> -* A Man-in-the-Middle (MitM) hijacks a session and impersonates
> - the server (possibly by proxying it) claiming not to support
> - TLS. In this manner, the client is confused into operating
> - in a plain-text manner with the MitM (with the session possibly
> - being proxied in plain-text to the server using the method
> - below).
> +* A Man-in-the-Middle (MitM) hijacks a session and impersonates the
> + server (possibly by proxying it) claiming not to support TLS (for
> + example, by omitting `NBD_FLAG_FIXED_NEWSTYLE` or changing a
> + response to `NBD_OPT_STARTTLS`). In this manner, the client is
> + confused into operating in a plain-text manner with the MitM (with
> + the session possibly being proxied in plain-text to the server using
> + the method below).
>
> -* The MitM hijacks a session and impersonates the client
> - (possibly by proxying it) claiming not to support TLS. In
> - this manner the server is confused into operating in a plain-text
> - manner with the MitM (with the session being possibly
> - proxied to the client with the method above).
> +* The MitM hijacks a session and impersonates the client (possibly by
> + proxying it) claiming not to support TLS (for example, by omitting
> + `NBD_FLAG_C_FIXED_NEWSTYLE` or eliding a request for
> + `NBD_OPT_STARTTLS`). In this manner the server is confused into
> + operating in a plain-text manner with the MitM (with the session
> + being possibly proxied to the client with the method above).
>
> With regard to the first, any client that does not wish
> to be subject to potential downgrade attack SHOULD ensure
> @@ -1000,12 +1005,18 @@ the first magic number.
> support the fixed newstyle protocol
> - bit 1, `NBD_FLAG_NO_ZEROES`; if set, and if the client replies with
> `NBD_FLAG_C_NO_ZEROES` in the client flags field, the server MUST NOT
> - send the 124 bytes of zero at the end of the negotiation.
> + send the 124 bytes of zero when the client ends negotiation with
> + `NBD_OPT_EXPORT_NAME`.
>
> The server MUST NOT set any other flags, and SHOULD NOT change behaviour
> unless the client responds with a corresponding flag. The server MUST
> NOT set any of these flags during oldstyle negotiation.
>
> +It is unlikely that additional capability flags will be defined in the
> +NBD protocol since this phase is susceptible to MitM downgrade attacks
> +when using TLS. Rather, additional features are best negotiated using
> +protocol options.
> +
> ##### Client flags
>
> This field of 32 bits is sent after initial connection and after
> @@ -1017,7 +1028,8 @@ receiving the handshake flags from the server.
> this isn't recommended.
> - bit 1, `NBD_FLAG_C_NO_ZEROES`; MUST NOT be set if the server did not
> set `NBD_FLAG_NO_ZEROES`. If set, the server MUST NOT send the 124
> - bytes of zeroes at the end of the negotiation.
> + bytes of zeroes when the client ends negotiation with
> + `NBD_OPT_EXPORT_NAME`.
>
> Clients MUST NOT set any other flags; the server MUST drop the TCP
> connection if the client sets an unknown flag, or a flag that does
LGTM
--
w@uter.{be,co.za}
wouter@{grep.be,fosdem.org,debian.org}
Reply to: