Re: [Qemu-devel] [PATCH] Improve documentation for TLS

[Eric Blake]

On 04/07/2016 05:35 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 four possible modes of operation of a server.
> 
> Signed-off-by: Alex Bligh <address@hidden>
> ---
>  doc/proto.md | 267 
> +++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 234 insertions(+), 33 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index f117394..9437e9b 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -286,6 +286,226 @@ S: (*length* bytes of data if the request is of type 
> `NBD_CMD_READ`)
>  This reply type MUST NOT be used except as documented by the
>  experimental `STRUCTURED_REPLY` extension; see below.
>  
> +## TLS support
> +
> +The NBD protocol supports TLS via negotiation with the `NBD_OPT_STARTTLS`
> +option. This is performed as an in-session upgrade. Below the term
> +'negotiation' is used to refer to the sending and receiving of
> +NBD commands, and the term 'initiation' of TLS is used to refer to
> +the actual upgrade to TLS.
> +
> +### TLS versions Certificates, authentication and authorisation

s/versions/versions,/ ?

> +
> +NBD implementations supporting TLS MUST support TLS version
> +1.2, and MAY support other (earlier or later) versions of
> +TLS/SSL.

Allowing earlier protocols should be discouraged (but not forbidden),
because of all the recent security flaws found in earlier protocols.
And forcing a server to stay with a particular version is going to bite
in the future when some bug in 1.2 is found that requires 1.3 for security.

Maybe:

MUST support at least TLS version 1.2, SHOULD support any newer versions
where possible, and MAY support older versions (although older versions
SHOULD NOT be used where there is a risk of security problems with those
older versions).

> +
> +This standard does not specify what encryption, certification
> +and signature algorithms are used. This standard does not
> +specify authentication and authortisation (for instance

s/authortisation/authorisation/

(looks like we are consistently favoring UK spellings, so my US spelling
of authorization is no more appropriate than your typo :)

> +whether client and/or server certificates are required and
> +what they should contain); this is implementation dependent.
> +
> +### Server-side requirements
> +
> +There are four modes of operation for a server. The
> +server MUST support one of these modes.
> +
> +* The server operates entirely without TLS ('NOTLS'); OR
> +
> +* The server makes TLS available (for all exports) and
> +  it is available at the option of the client ('OPTIONALTLS'); OR
> +
> +* The server insists upon TLS, and forces the client to
> +  upgrade by erroring any NBD options other than `NBD_OPT_STARTTLS`
> +  with `NBD_REP_ERR_TLS_REQD` ('FORCEDTLS'); this in practice means
> +  that all option negotiation (apart from the `NBD_OPT_STARTTLS`
> +  itself) is carried out with TLS; OR
> +
> +* The server provides TLS, and it is mandatory on zero or more
> +  exports, and is available at the client's option on all
> +  other exports ('SELECTIVETLS'). The server does not force
> +  the client to upgrade to TLS during option haggling (as
> +  if the client ultimately chose a non-TLS-only export,

s/chose/chooses/

> +  stopping TLS is not possible). Instead it permits the client
> +  to upgrade as and when it chooses, but unless an upgrade to
> +  TLS has already taken place, the server errors attempts
> +  to enter transmission mode on TLS-only exports, MAY
> +  refuse to provide information about TLS-only exports
> +  via `NBD_OPT_INFO`, and MAY refuse to provide information

or NBD_OPT_LIST (hmm, we ought to expand the text there to make it
obvious that a server may omit listings if it operating as SELECTIVETLS
but has not initiated TS)

> +  about non-existent exports via `NBD_OPT_INFO`.
> +
> +The server MAY determine the mode in which it operates
> +dependent upon the connection (for instance it might be
> +more liberal with connections made over the loopback
> +interface) but it MUST be consistent in its mode
> +of operation across the lifespan of a single TCP connection
> +to the server. A client MUST NOT assume indications from
> +a prior TCP session to a given server will be relevant
> +to a subsequent session.
> +
> +These modes of operations are described in detail below.
> +
> +#### NOTLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST respond with
> +`NBD_REP_ERR_UNSUPP`. The server MUST NOT respond to any

s/UNSUPP/UNSUP/

> +command with `NBD_REP_ERR_TLS_REQD`.
> +
> +#### OPTIONALTLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with
> +`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 connection.
> +
> +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 disconnect as
> +by this stage it is too late to continue without TLS as the
> +acknowledgement has been sent.
> +
> +The server MUST NOT respond to any command with `NBD_REP_ERR_TLS_REQD`,
> +as TLS is optional.
> +
> +#### FORCEDTLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with
> +`NBD_REP_ACK` and initiate TLS as set out under 'OPTIONALTLS'
> +above.
> +
> +If the server receives any other option, including `NBD_OPT_INFO`,

I'd go one step further:

including `NBD_OPT_INFO` or unrecognized options

That is, even if we standardize a new option, the correct course of
action for the server is to reject it with ERR_TLS_REQD, rather than
admit that the option is unknown with ERR_UNSUP.

> +it SHOULD 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
> +disconnect the connection. 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 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.

I'd go one step further:

If a server uses FORCEDTLS, it MUST implement the
NBD_FLAG_FIXED_NEWSTYLE flag, and SHOULD implement the INFO extension.

That way, a client can send ANY option to learn if TLS is required (even
an option that the server does not recognize); where NBD_OPT_INFO and
NBD_OPT_LIST are probably the two most useful options, but where ANY
option works.  A server with TLS but not FIXED_NEWSTYLE is pointless
(since TLS was introduced at the same time as fixed newstyle, we can
reasonably require, rather than just suggest, that both things be
implemented at once to be a compliant FORCEDTLS server).

> +
> +#### SELECTIVETLS mode
> +
> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with
> +`NBD_REP_ACK` and initiate TLS as set out under 'OPTIONALTLS'
> +above.
> +
> +If the server receives any other option except `NBD_OPT_INFO`,
> +it MUST NOT reply with `NBD_REP_ERR_TLS_REQD`. If the
> +server receives `NBD_OPT_INFO` and TLS has not been
> +initiated, it MAY reply with `NBD_REP_ERR_TLS_REQD` if that
> +export is non-existent, and MUST reply with `NBD_REP_ERR_TLS_REQD`
> +if that export is TLS-only.
> +
> +If the server receives a request to enter transmission mode
> +via `NBD_OPT_EXPORT_NAME` on a TLS-only export when TLS has not
> +been initiated, then as this request cannot error, it MUST
> +disconnect the connection. 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 neogitated. The server
> +MUST NOT send `NBD_REP_ERR_TLS_REQD` in response to any
> +option other than `NBD_OPT_INFO` or `NBD_OPT_GO`, and
> +only in those case in respect of a TLS-only non-existent export.

s/case/cases/
s/TLS-only/TLS-only or/

> +
> +There are two degenerate cases of SELECTIVETLS: where all
> +exports are TLS-only, and where no exports are TLS-only.
> +These are permitted to make programming of servers easier.
> +Where no exports are TLS-only, operation is very similar
> +to OPTIONALTLS. Where all exports are TLS-only, operation
> +is a little different from FORCEDTLS, as the client is not
> +forced to upgrade to TLS prior to any options being processed,
> +and the server MAY choose to give information on TLS-only
> +or non-existent exports via NBD_OPT_INFO exports prior to an
> +upgrade to TLS.

The information it would give on a TLS-only export is required to be
NBD_REP_ERR_TLS_REQD (indistinguishable from the information it would
give in FORCEDTLS).  Therefore, the difference for NBD_OPT_INFO is
limited to possibly getting NBD_REP_ERR_UNKNOWN for non-existent exports.

> +
> +The SELECTIVETLS mode of operation has an implementation problem
> +in that unless the INFO extension is supported, the client that
> +does not use TLS may have its access to exports denied without
> +it being able to ascertain the reason. For instance it may
> +go into transmission mode using `NBD_OPT_EXPORT_NAME` - which
> +does not return an error as no options will be denied with
> +`NBD_REP_ERR_TLS_REQD`. Futher there is no way to remotely

s/Futher/Further/

> +determine whether an export requires TLS, and therefore this
> +must be initiated between client and server out of band.
> +Therefore, if a server uses SELECTIVETLS, it SHOULD implement
> +the INFO extension.

Unlike FORCEDTLS (where even unknown commands make it easy to probe
whether TLS must be initiated), you are correct that SELECTIVETLS really
needs the INFO extension.  Since all existing implementations prior to
this month are FORCEDTLS, we have no reference SELECTIVETLS server yet;
and since your Go implementation is SELECTIVETLS but implements INFO,
I'd lean towards this being a MUST rather than SHOULD.

Also, as mentioned for FORCEDTLS, a server MUST support
NBD_FLAG_FIXED_NEWSTYLE for SELECTIVETLS to be useful.

> +
> +## 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 disconnect or
> +refuse to connect to a particular export if TLS is
> +not available and the user requires TLS.
> +
> +The client MAY send `NBD_OPT_STARTTLS` at any time to initiate
> +a TLS session, except that the client MUST NOT send
> +`NBD_OPT_STARTTLS` if TLS has alreay been initiated. If the

s/alreay/already/

> +cllient receives `NBD_REP_ACK` in response, it

s/cllient/client/

> +MUST immediately upgrade the connection to TLS. If it receives
> +`NBD_ERR_REP_UNSUP` in response or any other error, it indicates
> +that the server cannot or will not upgrade the connection to
> +TLS and therefore MUST either continue the connection without
> +TLS, or discconnect.

s/discconnect/disconnect/

Maybe also add:

A client that wants to use TLS SHOULD use NBD_OPT_STARTTLS as its first
option.

[you may have already done this in your followup mail where you document
MitM attacks]

> +
> +If the TLS handshake is unsuccessful (for instance the servers's

s/servers's/server's/

> +certificate does not validate) the client MUST disconnect as
> +by this      stage it is too late to continue without TLS.

drop the extra spaces

> +
> +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 disconnect the session (if
> +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 server MAY therefore choose to issue

s/server/client/

> +a `NBD_OPT_STARTTLS`, MAY disconnect the session (if
> +for instance it does not support TLS or does not have
> +appropriate credentials for this server), or MAY continue
> +in another manner without tls, for instance by querying

s/tls/TLS/

> +or using other exports.
> +
> +The client MAY discover the server operates in NOTLS mode by
> +sending `NBD_OPT_STARTTLS`. If `NBD_REP_ERR_UNSUPP` is

s/UNSUPP/UNSUP/

> +replied, it is guaranteed the server is not in this mode.

s/not //

> +On all other modes, and upgrade to TLS will be performed.

A client MAY discover the server operates in FORCEDTLS mode by sending
any option other than `NBD_OPT_STARTTLS` or `NBD_OPT_EXPORT_NAME`, even
an option that the server does not understand, since it is guaranteed
the server will reply with `NBD_REP_ERR_TLS_REQD`.

Discovery of OPTIONALTLS and SELECTIVETLS is not reliable due to MitM
attacks.

> +
> +If a client supports TLS, it SHOULD also support the INFO
> +extension, and SHOULD use `NBD_OPT_GO` if available in place
> +of `NBD_OPT_EXPORT_NAME`.

add: it MUST also support the NBD_FLAG_C_FIXED_NEWSTYLE flag

> The 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`.
> +
> +### Status
> +
> +This functionality has not yet been implemented by the reference
> +implementation, but was implemented by qemu and subsequently
> +by other users, so has been moved out of the "experimental" section.
> +
>  ## Values
>  

>  
>  - `NBD_OPT_STARTTLS` (5)
>  

> +    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_UNSUP`.
> +
> +    See the section on TLS above for futher details.

s/futher/further/

>  
>  - `NBD_OPT_INFO` (6)
>  
> @@ -489,20 +701,9 @@ case that data is an error message string suitable for 
> display to the user.
>  * `NBD_REP_ERR_TLS_REQD` (2^31 + 5)
>  
>      The server is unwilling to continue negotiation unless TLS is

> +    initiated first. In the case of `NBD_OPT_INFO` and `NBD_OPT_GO`
> +    this unwillingness MAY be limited to the export in question.

Maybe add: "but only for a server in SELECTIVETLS mode"

> +    See the section on TLS above for further details.
>  
>  * `NBD_REP_ERR_UNKNOWN` (2^31 + 6)
>  
> @@ -735,13 +936,13 @@ Therefore these commands share common documentation.
>      - `NBD_REP_ERR_UNKNOWN`: The chosen export does not exist on this
>        server.
>      - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this
> -      block device unless the client negotiates TLS first.
> +      block device unless the client initiates TLS first.
>      - `NBD_REP_SERVER`: The server accepts the chosen export.
>  
> -    Additionally, if TLS has not been negotiated, the server MAY reply
> +    Additionally, if TLS has not been initiated, the server MAY reply
>      with `NBD_REP_ERR_TLS_REQD` (instead of `NBD_REP_ERR_UNKNOWN`)
>      to requests for exports that are unknown. This is so that clients
> -    that have not negotiated TLS cannot enumerate exports.
> +    that have not initiated TLS cannot enumerate exports.
>  
>      In the case of `NBD_REP_SERVER`, the message's data takes on a different
>      interpretation than the default (so as to provide additional
> 

and don't forget to tweak NBD_OPT_LIST to cater to skip listings of
TLS-required exports during SELECTIVETLS mode.

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

signature.asc
Description: OpenPGP digital signature