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

[Alex Bligh]

Eric,

Thanks. In v7 I've taken all of these, save where indicated
(or as described) below.

>> +If the server receives `NBD_OPT_STARTTLS` it MUST reply with
> 
> Worth mentioning 'prior to initiating TLS', since...
> ...the behavior after initiating TLS is required to be different?

As you point out later (and as I found independently) this
difference was not explicitly set out. It now is.

> 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.

I think not worth mentioning. There may be (and probably are)
buggy clients of all sorts. In this case Qemu's behaviour isn't
technically a bug anyway as it can drop the session at any
time. It's merely being someone ungracious.

> 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.

Reinstated explicitly in the TLS section, and mentioned in the
NBD_OPT_ section too. Thanks for catching that one.

--
Alex Bligh






signature.asc
Description: Message signed with OpenPGP using GPGMail

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

[Eric Blake]

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 
> Signed-off-by: Alex Bligh 
> ---
>  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 

[Qemu-devel] [PATCHv6] Improve documentation for TLS

[Alex Bligh]

* 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 
Signed-off-by: Alex Bligh 
---
 doc/proto.md | 338 +--
 1 file changed, 304 insertions(+), 34 deletions(-)

Changes from v5:

* Delete OPTIONALTLS (RIP)

* Add NBD_REP_ERR_POLICY

* s/NBD_ERR_REP/NBD_REP_ERR/ in one place

* Consistently use the phrase 'terminate the session' to mean dropping
  the connection, as per Wouter. Note there are other inconsistent
  uses of 'dropping the connection', 'disconnecting' etc. elsewhere
  which I haven't touched.

* Similarly refer to the connection as a 'session' when it doesn't
  explicitly mean the L3 TCP connection (TLS section only).

* Introduce a paragraph under newstyle negotiation emphasising that
  terminating the session is legal and sometimes required, and defining
  it.

Changes from v4

* Minor grammar nit

Changes from v3:

* Delete confusing text about server omitting entries from NBD_OPT_LIST
if TLS is not negotiated and FORCETLS is used, as that (of course)
requires NBD_REP_ERR_TLS_REQD elsewhere in the text.

* Further nits from Eric Blake

Changes from v2:

* The response to a command is a response, not a NBD_REP_ACK

* Make it clear that the response can be errored

* Nits from Eric Blake

Changes from v1:

* Make a NBD_CMD_CLOSE imply a flush

* Nits from Eric Blake

diff --git a/doc/proto.md b/doc/proto.md
index f117394..b8a9b5d 100644
--- a/doc/proto.md
+++ 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
+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'.
+
 ### Transmission
 
 There are three message types in the transmission phase: the request,
@@ -286,6 +293,284 @@ 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 Transport Layer Security (TLS) (see
+[RFC5246](https://tools.ietf.org/html/rfc5246)
+as updated by
+[RFC6176](https://tools.ietf.org/html/rfc6176)
+).
+
+TLS is negotiated 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.
+
+### Certificates, authentication and authorisation
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authorisation (for instance
+whether client and/or server certificates are required and
+what they should contain); this is implementation dependent.
+
+TLS requires fixed newstyle negotiation to have completed.
+
+### 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 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 were to choose a non-TLS-only export,
+  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`, MAY refuse to provide