On 04/09/2016 06:06 AM, Alex Bligh wrote:
> * Call out TLS into a separate section
> 
> * Add details of the TLS protocol itself
> 
> * Emphasise that actual TLS session initiation (i.e. the TLS handshake) can
>   be initiated from either side (as required by the TLS standard I believe
>   and as actually works in practice)
> 
> * Clarify what is a requirement on servers, and what is a requirement on
>   clients, separately, specifying their behaviour in a single place
>   in the document.
> 
> * Document the three possible modes of operation of a server.
> 
> * Add text defining what 'terminate the session' means during
>   negotiation, and when it is available.
> 
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> Signed-off-by: Alex Bligh <a...@alex.org.uk>
> ---
>  doc/proto.md | 338 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 304 insertions(+), 34 deletions(-)

> +++ b/doc/proto.md
> @@ -195,6 +195,13 @@ request before sending the next one of the same type. 
> The server MAY
>  send replies in the order that the requests were received, but is not
>  required to.
>  
> +There is no requirement for the client or server to complete a negotiation
> +if it does not wish to do so. If ithe client does not find an export it

s/ithe/the/

> +is looking for (for instance) it may simply close the TCP connection.
> +Under certain circumstances either the client or the server may be required
> +by this document to close the TCP connection. In each case, this is
> +referred to as to 'terminate the session'.

s/to as to/to as/


> +
> +### Server-side requirements
> +
> +There are four modes of operation for a server. The

s/four/three/

> +#### NOTLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST respond with
> +`NBD_REP_ERR_POLICY` (if it does not support TLS for
> +policy reasons) or `NBD_REP_ERR_UNSUP` (if it does not
> +support the `NBD_OPT_STARTTLS` option at all. The server MUST NOT

s/all./all)./

> +respond to any option request with `NBD_REP_ERR_TLS_REQD`.
> +
> +#### FORCEDTLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with

Worth mentioning 'prior to initiating TLS', since...

> +`NBD_REP_ACK`. After this reply has been sent, the server MUST
> +be prepared for a TLS handshake, and all further data MUST
> +be sent and received over TLS. There is no downgrade to a
> +non-TLS session.
> +
> +As per the TLS standard, the handshake MAY be initiated either
> +by the server (having sent the `NBD_REP_ACK`) or by the client.
> +If the handshake is unsuccessful (for instance the client's
> +certificate does not match) the server MUST terminate the
> +session as by this stage it is too late to continue without TLS
> +as the acknowledgement has been sent.
> +
> +If the server receives any other option, including `NBD_OPT_INFO`
> +and unsupported options, it MUST reply with `NBD_REP_ERR_TLS_REQD`
> +if TLS has not been initiated; `NBD_OPT_INFO` is included as in this
> +mode, all exports are TLS-only. If the server receives a request to
> +enter transmission mode via `NBD_OPT_EXPORT_NAME` when TLS has not
> +been initiated, then as this request cannot error, it MUST
> +terminate the session. If the server receives a request to
> +enter transmission mode via `NBD_OPT_GO` when TLS has not been
> +initiated, it MUST error with `NBD_REP_ERR_TLS_REQD`.
> +
> +The server MUST NOT send `NBD_REP_ERR_TLS_REQD` in reply to
> +any command if TLS has already been initiated.

...the behavior after initiating TLS is required to be different?

> +
> +The FORCEDTLS mode of operation has an implementation problem in
> +that the client MAY legally simply send a `NBD_OPT_EXPORT_NAME`
> +to enter transmission mode without previously sending any options.
> +Therefore, if a server uses FORCEDTLS, it SHOULD implement the
> +INFO extension.
> +
> +#### SELECTIVETLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with

Same phrasing of 'prior to initiating TLS'

> +`NBD_REP_ACK` and initiate TLS as set out under 'FORCEDTLS'
> +above.
> +

> +
> +## Client-side requirements
> +
> +If the client supports TLS at all, it MUST be prepared
> +to deal with servers operating in any of the above modes.
> +Notwithstanding, a client MAY always terminate the session or
> +refuse to connect to a particular export if TLS is
> +not available and the user requires TLS.
> +
> +The client MUST NOT issue `NBD_OPT_STARTTLS` unless the server
> +set flag NBD_FLAG_FIXED_NEWSTYLE and the client replied
> +with NBD_FLAG_C_FIXED_NEWSTYLE in the fixed newstyle
> +negotiation.
> +
> +The client MUST NOT issue `NBD_OPT_STARTTLS` if TLS has already
> +been initiated.

Worth mentioning that clients should be prepared to encounter buggy
servers (qemu 2.5) that disconnect rather than properly send an error
message?  Maybe not, since the buggy versions will eventually be ancient
history, and since you already stated above that either party can
disconnect at any time for any reason during handshake phase.

> +
> +Subject to the above two limitations, the client MAY send
> +`NBD_OPT_STARTTLS` at any time to initiate a TLS session. If the
> +client receives `NBD_REP_ACK` in response, it MUST immediately
> +upgrade the session to TLS. If it receives `NBD_REP_ERR_UNSUP`,
> +`NBD_REP_ERR_POLICY` in response or any other error, it indicates

s/in response or any other error/or any other error in response/

> +that the server cannot or will not upgrade the session to TLS

s/TLS/TLS,/

> +and therefore MUST either continue the session without TLS,

s/therefore/therefore the client/

> +or terminate the session.
> +
> +A client that prefers to use TLS irrespective of whether
> +the server makes TLS mandatory SHOULD send `NBD_OPT_STARTTLS`
> +as the first option. This will ensure option haggling is subject
> +to TLS, and will thus prevent the possibility of options being
> +compromised by a Man-in-the-Middle attack. Note that the
> +`NBD_OPT_STARTTLS` itself may be compromised - see 'downgrade
> +attacks' for more details. For this reason, a client which only
> +wishes to use TLS SHOULD terminate the session if the
> +`NBD_OPT_STARTTLS` replies with an error.
> +
> +If the TLS handshake is unsuccessful (for instance the server's
> +certificate does not validate) the client MUST terminate the
> +session as by this stage it is too late to continue without TLS.
> +
> +If the client receives an `NBD_REP_ERR_TLS_REQD` in response
> +to any option, it implies that this option cannot be executed
> +unless a TLS upgrade is performed. If the option is any
> +option other than `NBD_OPT_INFO` or `NBD_OPT_GO`, this
> +indicates that no option will succeed unless a TLS upgrade
> +is performed; the client MAY therefore choose to issue
> +a `NBD_OPT_STARTTLS`, or MAY terminate the session (if

We're inconsistent on "an `NBD_..." vs. "a `NBD_...", I favor "an" (as I
pronounce it "en-bee-dee").

> +for instance it does not support TLS or does not have
> +appropriate credentials for this server). If the client
> +receives `NBD_REP_ERR_TLS_REQD` in response to
> +`NBD_OPT_INFO` or `NBD_OPT_GO` this indicates that the
> +export referred to within the option is either non-existent
> +or requires TLS; the client MAY therefore choose to issue
> +a `NBD_OPT_STARTTLS`, MAY terminate the session (if

and again

> +#### TLS versions
> +
> +NBD implementations supporting TLS MUST support TLS version 1.2,
> +SHOULD support any later versions. NBD implementations
> +MAY support older versions but SHOULD NOT do so by default
> +(i.e. they SHOULD only be available by a configuration change).
> +Older versions SHOULD NOT be used where there is a risk of security
> +problems with those older versions or of a downgrade attack
> +against TLS versions.

I like that wording.

> @@ -400,21 +687,14 @@ of the newstyle negotiation.
>  
>  - `NBD_OPT_STARTTLS` (5)
>  
> -    The client wishes to initiate TLS. If the server replies with
> -    `NBD_REP_ACK`, then the client should immediately initiate a TLS
> -    handshake and continue the negotiation in the encrypted channel. If
> -    the server is unwilling to perform TLS, it should reply with
> -    `NBD_REP_ERR_POLICY`. For backwards compatibility, a client should
> -    also be prepared to handle `NBD_REP_ERR_UNSUP`. If the client sent
> -    along any data with the request, the server should send back
> -    `NBD_REP_ERR_INVALID`. The client MUST NOT send this option if
> -    it has already negotiated TLS; if the server receives
> -    `NBD_OPT_STARTTLS` when TLS has already been negotiated, the server
> -    MUST send back `NBD_REP_ERR_INVALID`.
> -
> -    This functionality has not yet been implemented by the reference
> -    implementation, but was implemented by qemu so has been moved out of
> -    the "experimental" section.
> +    The client wishes to initiate TLS.
> +
> +    The server MUST either reply with `NBD_REP_ACK` after which
> +    point the connection is upgraded to TLS, or reply with
> +    `NBD_REP_ERR_POLICY` (or if it does not support the option
> +    at all, `NBD_REP_ERR_UNSUP`).
> +
> +    See the section on TLS above for further details.

You deleted the requirement about MUST reply with NBD_REP_ERR_INVALID
when the client sends NBD_CMD_STARTTLS when already in TLS, but I don't
see it above.  Arguably, since we required above that the client MUST
NOT send NBD_CMD_STARTTLS twice, it's already undefined behavior, but
requiring that the server MUST reject with NBD_REP_ERR_INVALID would
make it harder for implementations to get it wrong when dealing with
buggy clients.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial! http://pubads.g.doubleclick.net/
gampad/clk?id=1444514301&iu=/ca-pub-7940484522588532
_______________________________________________
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to