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

[Qemu-devel] [PATCH] 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 four possible modes of operation of a server.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
---
 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
+
+NBD implementations supporting TLS MUST support TLS version
+1.2, and MAY support other (earlier or later) versions of
+TLS/SSL.
+
+This standard does not specify what encryption, certification
+and signature algorithms are used. This standard does not
+specify authentication and authortisation (for instance
+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,
+  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
+  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
+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`,
+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.
+
+#### 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.
+
+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 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
+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.
+
+## 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
+cllient receives `NBD_REP_ACK` in response, it
+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.
+
+If the TLS handshake is unsuccessful (for instance the servers's
+certificate does not validate) the client MUST disconnect 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 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
+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
+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
+replied, it is guaranteed the server is not in this mode.
+On all other modes, and upgrade to TLS will be performed.
+
+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`. 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
 
 This section describes the value and meaning of constants (other than
@@ -366,7 +586,7 @@ of the newstyle negotiation.
     Data: String, name of the export, as free-form text.
     The length of the name is determined from the option header. If the
     chosen export does not exist or requirements for the chosen export
-    are not met (e.g., the client did not negotiate TLS for an export
+    are not met (e.g., the client did not initiate TLS for an export
     where the server requires it), the server should close the
     connection.
 
@@ -400,21 +620,13 @@ 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_UNSUP`.
+
+    See the section on TLS above for futher details.
 
 - `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
-    negotiated first. A server MUST NOT send this error if it has one or
-    more exports that do not require TLS; not even if the client indicated
-    interest (by way of `NBD_OPT_PEEK_EXPORT`) in an export which requires
-    TLS.
-
-    If this reply is used, servers SHOULD send it in reply to each and every
-    unencrypted `NBD_OPT_*` message (apart from `NBD_OPT_STARTTLS`).
-
-    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 experimental `INFO` extension makes small but compatible
-    changes to the semantics of this error message; see below.
+    initiated first. In the case of `NBD_OPT_INFO` and `NBD_OPT_GO`
+    this unwillingness MAY be limited to the export in question.
+    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
-- 
1.9.1

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

[Daniel P. Berrange]

On Thu, Apr 07, 2016 at 12:35:59PM +0100, 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 <alex@alex.org.uk>
> ---
>  doc/proto.md | 267 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 234 insertions(+), 33 deletions(-)


> +### 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,
> +  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
> +  about non-existent exports via `NBD_OPT_INFO`.

IMHO, we should not permit what you call OPTIONALTLS or SELECTIVETLS,
because these open possibilities for a MITM to perform downgrade
attacks, where the MITM runs TLS to the real server, but runs no-TLS
to the real client.

Both the QEMU NBD server and NBD clients only implement FORCEDTLS.
ie you tell the client to use TLS and it will refuse to talk to a
server which doesn't support TLS, and you tell the server to use
TLS and it will refuse to talk to a client which does not request
TLS

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Alex Bligh]

Daniel,

On 7 Apr 2016, at 12:51, Daniel P. Berrange <berrange@redhat.com> wrote:

> IMHO, we should not permit what you call OPTIONALTLS or SELECTIVETLS,
> because these open possibilities for a MITM to perform downgrade
> attacks, where the MITM runs TLS to the real server, but runs no-TLS
> to the real client.

Could you describe how a downgrade attack might occur? It's
always open to the client to refuse to access an export (or
the server as a whole) unless TLS is negotiated, as I make
clear here (see last phrase).

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

No one should be worried about downgrade attacks on
SELECTIVETLS on the exports that are not TLS only because
clearly that is a risk they have decided to take by making
the exports not TLS only! The only way to avoid a download
attack is to make the export TLS only. SELECTIVETLS
caters for the situation where the server only has some
exports it wishes to secure. I guess it's worth documenting
this, though I thought it was obvious.

OPTIONALTLS copes with the situation where all exports
fall into the above category. Here you are effectively saying
the client and server agree out of band whether TLS
should be compulsory, and the client will reject this if not.

I think you might be making the mistake of coming
at this from the perspective where there is only one export
(possibly because in qemu-nbd as far as I know there is
only ever one export). If you agree there is a use case
for non-tls exports, and a use case for tls exports, it's
difficult to argue there isn't a use case for both.

On 7 Apr 2016, at 12:51, Daniel P. Berrange <berrange@redhat.com> wrote:

> Both the QEMU NBD server and NBD clients only implement FORCEDTLS.
> ie you tell the client to use TLS and it will refuse to talk to a
> server which doesn't support TLS, and you tell the server to use
> TLS and it will refuse to talk to a client which does not request
> TLS

So using my terminology, FORCEDTLS is something that applies
only to the server, and that's fine if that's how you want
your server to operate (i.e. it is permitted for it to
operate only in FORCEDTLS or NOTLS mode which is what
Qemu does). In qemu-NBD, it uses either FORCEDTLS or
NOTLS depending on the command line - that's just fine
by my proposals.

Re qmeu client, if the (non-qemu) server runs in any of the TLS
modes (apart from 'NOTLS') that will be compatible with the Qemu
server. There will be no downgrade attack possible if the qemu
client errors if it can't negotiate TLS (which it is
intended to do). In qemu, the client's policy toward the
server is determined by the command line as well - if
TLS is specified, it insists the connection be upgraded,
but if it isn't specified, it never tries to upgrade the
connection. That behaviour is just fine by my proposal as
well.

But it doesn't mean the *server* needs to be in FORCEDTLS
mode. Indeed the qemu client operates in exactly this way with
my server, which is SELECTIVETLS - explicitly permitted by the
INFO extension currently, and interoperability is fine. And this
is perfectly compatible behaviour with what I suggest.

Incidentally, the qemu client does not appear to assume the
server is 'FORCEDTLS', and IIRC from time spend staring into
Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
upgrade. I can check that if you like.

-- 
Alex Bligh

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

[Alex Bligh]

On 7 Apr 2016, at 13:13, Alex Bligh <alex@alex.org.uk> wrote:

> I guess it's worth documenting
> this, though I thought it was obvious.

The next version will have this section:

### Downgrade attacks

A danger inherent in any scheme	relying	on the negotiation
of whether TLS should be employed is downgrade attacks.

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

* 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 oeprating in a plain-text
  manner with the MitM (with the session being possibly
  proxied to the server with the method above).

With regard to the first, any client that does not wish
to be subject to potential downgrade attack SHOULD ensure
that if	a TLS endpoint is specified by the client, it
ensures	that TLS is negotiated prior to	sending	or
requesting sensitive data. To recap, yhe client MAY send
`NBD_OPT_STARTTLS` at any point	during option haggling,
and MAY	disconnect the session if `NBD_REP_ACK`	is not
provided.

With regard to the second, any server that does	not wish
to be subject to a potential downgrade attack SHOULD either
used FORCEDTLS mode, or	should force TLS on those exports
it is concerned about using SELECTIVE mode and TLS-only
exports. It is not possible to avoid downgrade attacks
on exports which are may be served either via TLS or
in plain text.

-- 
Alex Bligh

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

[Daniel P. Berrange]

On Thu, Apr 07, 2016 at 01:13:10PM +0100, Alex Bligh wrote:
> Daniel,
> 
> On 7 Apr 2016, at 12:51, Daniel P. Berrange <berrange@redhat.com> wrote:
> 
> > IMHO, we should not permit what you call OPTIONALTLS or SELECTIVETLS,
> > because these open possibilities for a MITM to perform downgrade
> > attacks, where the MITM runs TLS to the real server, but runs no-TLS
> > to the real client.
> 
> Could you describe how a downgrade attack might occur? It's
> always open to the client to refuse to access an export (or
> the server as a whole) unless TLS is negotiated, as I make
> clear here (see last phrase).

Right, so that's OK if the client is implementing FORCEDTLS.

If the server policy is OPTIONALTLS, and the client policy
is also OPTIONALTLS, then a MITM can just downgrade to NOTLS
and both client & server will carry on happily in cleartext.

IOW, OPTIONALTLS is offering no meaningful security when
both client & server policy is set that way.  It is only
safe if either the client or server run FORCEDTLS. This
all makes OPTIONALTLS rather pointless, and gives people
a false sense of security when they use it and don't
realize it is trivially downgradable if both sides consider
it as optional.

> > ## 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.
> 
> No one should be worried about downgrade attacks on
> SELECTIVETLS on the exports that are not TLS only because
> clearly that is a risk they have decided to take by making
> the exports not TLS only! The only way to avoid a download
> attack is to make the export TLS only. SELECTIVETLS
> caters for the situation where the server only has some
> exports it wishes to secure. I guess it's worth documenting
> this, though I thought it was obvious.
> 
> OPTIONALTLS copes with the situation where all exports
> fall into the above category. Here you are effectively saying
> the client and server agree out of band whether TLS
> should be compulsory, and the client will reject this if not.
> 
> I think you might be making the mistake of coming
> at this from the perspective where there is only one export
> (possibly because in qemu-nbd as far as I know there is
> only ever one export). If you agree there is a use case
> for non-tls exports, and a use case for tls exports, it's
> difficult to argue there isn't a use case for both.

I don't really agree that there's a use case of mixing
tls & non-tls exports in the same server. Sure it is
something you can technically support, but I think it
is really a solution in search of a problem when it comes
to the real world,, and the most likely effect is that
it'll just open the door to accidental security screw
ups by people not understanding its implications.

> On 7 Apr 2016, at 12:51, Daniel P. Berrange <berrange@redhat.com> wrote:
> 
> > Both the QEMU NBD server and NBD clients only implement FORCEDTLS.
> > ie you tell the client to use TLS and it will refuse to talk to a
> > server which doesn't support TLS, and you tell the server to use
> > TLS and it will refuse to talk to a client which does not request
> > TLS
> 
> So using my terminology, FORCEDTLS is something that applies
> only to the server, and that's fine if that's how you want
> your server to operate (i.e. it is permitted for it to
> operate only in FORCEDTLS or NOTLS mode which is what
> Qemu does). In qemu-NBD, it uses either FORCEDTLS or
> NOTLS depending on the command line - that's just fine
> by my proposals.
> 
> Re qmeu client, if the (non-qemu) server runs in any of the TLS
> modes (apart from 'NOTLS') that will be compatible with the Qemu
> server. There will be no downgrade attack possible if the qemu
> client errors if it can't negotiate TLS (which it is
> intended to do). In qemu, the client's policy toward the
> server is determined by the command line as well - if
> TLS is specified, it insists the connection be upgraded,
> but if it isn't specified, it never tries to upgrade the
> connection. That behaviour is just fine by my proposal as
> well.
> 
> But it doesn't mean the *server* needs to be in FORCEDTLS
> mode. Indeed the qemu client operates in exactly this way with
> my server, which is SELECTIVETLS - explicitly permitted by the
> INFO extension currently, and interoperability is fine. And this
> is perfectly compatible behaviour with what I suggest.

You say you're only describing these options from the server's POV, but
I think it is clear that they need to be considered from both the client
and server POV in order to evaluate the security characteristics.

> Incidentally, the qemu client does not appear to assume the
> server is 'FORCEDTLS', and IIRC from time spend staring into
> Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
> upgrade. I can check that if you like.

If the qemu NBD client has TLS credentials set then it will
refuse to connect to a server without TLS. The OPT_TLS is
definitely the first thing it sends, becasue the QEMU NBD
server will reject all options until OPT_TLS has been sent.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Alex Bligh]

Daniel,

>> Could you describe how a downgrade attack might occur? It's
>> always open to the client to refuse to access an export (or
>> the server as a whole) unless TLS is negotiated, as I make
>> clear here (see last phrase).
> 
> Right, so that's OK if the client is implementing FORCEDTLS.

Clients do not have named policies (at least not per
the nomenclature of the patch I sent in).

I'm taking it you mean 'if the client considers it requires
TLS'

> If the server policy is OPTIONALTLS, and the client policy
> is also OPTIONALTLS,

I'm taking it you mean 'if the client considers TLS to be optional'.

> then a MITM can just downgrade to NOTLS
> and both client & server will carry on happily in cleartext.

Yes indeed. This is an obvious result of the client
considering TLS to be optional and the server being
happy to provide plaintext.

> IOW, OPTIONALTLS is offering no meaningful security when
> both client & server policy is set that way.

It offers protection where no MTiM is possible but wiretapping
can be done.

> It is only
> safe if either the client or server run FORCEDTLS.

I believe SELECTIVETLS offers the same security on tls-only
mounts.

> This
> all makes OPTIONALTLS rather pointless, and gives people
> a false sense of security when they use it and don't
> realize it is trivially downgradable if both sides consider
> it as optional.

Well, I replied with a new section that documents this issue.
However, it's an inevitable result of a situation where TLS
is in any sense optional. That's already the case with the
INFO extension and (by my reading) without it. What I'm doing
is documenting this fact. It now (with the addition I
sent through) explicitly calls out what the risks are which
better than not calling them out (current situation).

I believe having a choice of TLS-or-not is useful provided
you know what you are doing. Note that a certain well-known
search engine seems to take the same view with its home
page.

> I don't really agree that there's a use case of mixing
> tls & non-tls exports in the same server. Sure it is
> something you can technically support, but I think it
> is really a solution in search of a problem when it comes
> to the real world,, and the most likely effect is that
> it'll just open the door to accidental security screw
> ups by people not understanding its implications.

This is *already there* in the INFO extension. I'm am
documenting it.

>> But it doesn't mean the *server* needs to be in FORCEDTLS
>> mode. Indeed the qemu client operates in exactly this way with
>> my server, which is SELECTIVETLS - explicitly permitted by the
>> INFO extension currently, and interoperability is fine. And this
>> is perfectly compatible behaviour with what I suggest.
> 
> You say you're only describing these options from the server's POV, but
> I think it is clear that they need to be considered from both the client
> and server POV in order to evaluate the security characteristics.

No, I am saying the MODES I describe only apply to servers, as that's
how they are written, and they are under the section called
"Sever-side requirements"

The section "Client-side requirements" describes things from the
client's point of view. The new section I have added on "Downgrade
Attacks" (which should probably be called "Security considerations"
thinking about it) sets out how these things interoperate. Incidentally
there are two different downgrade attacks possible (on the client
and on the server).
>> 
>> Incidentally, the qemu client does not appear to assume the
>> server is 'FORCEDTLS', and IIRC from time spend staring into
>> Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
>> upgrade. I can check that if you like.
> 
> If the qemu NBD client has TLS credentials set then it will
> refuse to connect to a server without TLS. The OPT_TLS is
> definitely the first thing it sends, becasue the QEMU NBD
> server will reject all options until OPT_TLS has been sent.

Yep you are right - just checked wireshark.

-- 
Alex Bligh

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 17506 bytes --]

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 <alex@alex.org.uk>
> ---
>  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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2517 bytes --]

On 04/07/2016 05:51 AM, Daniel P. Berrange wrote:

>> +
>> +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,
>> +  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
>> +  about non-existent exports via `NBD_OPT_INFO`.
> 
> IMHO, we should not permit what you call OPTIONALTLS or SELECTIVETLS,
> because these open possibilities for a MITM to perform downgrade
> attacks, where the MITM runs TLS to the real server, but runs no-TLS
> to the real client.

On a safe interface (like loopback) where there cannot be a MitM attack,
they still make sense.  I think the protocol should discourage, but not
forbid, their use (and I think the followup mail does this, by
documenting pitfalls of downgrade attacks).

> 
> Both the QEMU NBD server and NBD clients only implement FORCEDTLS.

which is fine. You don't have to implement all four server modes to be
compliant to the protocol, implementing just one is okay.

> ie you tell the client to use TLS and it will refuse to talk to a
> server which doesn't support TLS, and you tell the server to use
> TLS and it will refuse to talk to a client which does not request
> TLS
> 
> Regards,
> Daniel
> 

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Alex Bligh]

[-- Attachment #1: Type: text/plain, Size: 13068 bytes --]


On 7 Apr 2016, at 15:31, Eric Blake <eblake@redhat.com> wrote:

>> +### TLS versions Certificates, authentication and authorisation
> 
> s/versions/versions,/ ?

ok

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

ok

>> +
>> +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/

ok

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

wondered when that would come up!

>> +* 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/

I sort of meant "chose" in the sense of "if the client ultimately
were to choose" changing it one way or another is clearer.

>> +  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)

+1. Will do.

>> +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/

thanks

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

Yeah I think that's right.

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

I think that's implied, because if a server doesn't implement
NBD_FLAG_FIXED_NEWSTYLE, surely the client can only safely
send one option (NBD_OPT_EXPORT_NAME) which means the
server can't receive NBD_OPT_STARTTLS at all!

I think there's no harm in saying at the top of the section
on servers that the server MUST set NBD_FLAG_FIXED_NEWSTYLE
if it's going to work in a TLS mode other than 'NOTLS'.

>> +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/

thx

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

Quite correct, thanks.

>> +
>> +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/

thx

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

Hmmm. I sort of agree, and thought about this, but I decided not to as:

* the TLS specification is not in 'experimental' whereas the INFO extension is.

* I don't agree that the existing documentation necessarily limits you
  to FORCETLS - it's ambiguous.

I think I'll make this change as I was 50/50 on it.

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

See above comment.

>> +
>> +## 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/

thx

>> +cllient receives `NBD_REP_ACK` in response, it
> 
> s/cllient/client/

thx

>> +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/

(sigh) thx

> Maybe also add:
> 
> A client that wants to use TLS SHOULD use NBD_OPT_STARTTLS as its first
> option.

A client that **only** wants to use TLS SHOULD use it as its first option.
It may not care, but will use it if the server requires (in which
case it doesn't care about the downgrade attack either). But yes
something like this should go in.

> [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/

thx

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

thx

>> +
>> +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/

thx

>> +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/

thx

>> +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/

thx

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

thx (sigh)!

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

... yes, but

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

... though using SELECTIVETLS exports marked as TLS-only is safe.

Thinking about it, I think I'm just going to delete the whole para.
The client should not care what mode the server is in. It should
just care whether it gets TLS or not. If it's going to insist
on TLS come what may, the strategy is the same whether the
client is in FORCEDTLS, OPTIONALTLS or SELECTIVETLS. It just
sends NBD_OPT_STARTTLS as the first command, and if it doesn't
get a response it likes then goodbye.

The para only encourages the interpretation that it's somehow
important for the client to know what mode the server is in,
which isn't true.

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

See above. I'll put it up top.

>> +    See the section on TLS above for futher details.
> 
> s/futher/further/

thx.

>> 
>> - `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"

Well the following line ...

>> +    See the section on TLS above for further details.

... was meant to imply that! I'll make it clearer.

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

OK. v2 coming up.

--
Alex Bligh





[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

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

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

On 04/07/2016 06:36 AM, Alex Bligh wrote:
> 
> On 7 Apr 2016, at 13:13, Alex Bligh <alex@alex.org.uk> wrote:
> 
>> I guess it's worth documenting
>> this, though I thought it was obvious.
> 
> The next version will have this section:
> 
> ### Downgrade attacks
> 
> A danger inherent in any scheme	relying	on the negotiation

too much space

> of whether TLS should be employed is downgrade attacks.
> 
> 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).

looks like too much space is a problem in general in this rough draft;
I'll quit pointing it out and assume you will reflow before final
submission.

> 
> * 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 oeprating in a plain-text

s/oeprating/operating/

>   manner with the MitM (with the session being possibly
>   proxied to the server with the method above).

s/server/client/

> 
> With regard to the first, any client that does not wish
> to be subject to potential downgrade attack SHOULD ensure
> that if	a TLS endpoint is specified by the client, it
> ensures	that TLS is negotiated prior to	sending	or
> requesting sensitive data. To recap, yhe client MAY send

s/yhe/the/

> `NBD_OPT_STARTTLS` at any point	during option haggling,
> and MAY	disconnect the session if `NBD_REP_ACK`	is not
> provided.

Probably want to add: "but the client SHOULD strongly consider sending
`NBD_OPT_STARTTLS` as its first option"

> 
> With regard to the second, any server that does	not wish
> to be subject to a potential downgrade attack SHOULD either
> used FORCEDTLS mode, or	should force TLS on those exports
> it is concerned about using SELECTIVE mode and TLS-only
> exports. It is not possible to avoid downgrade attacks
> on exports which are may be served either via TLS or
> in plain text.

Probably want to add: "OPTIONALTLS mode SHOULD NOT be used if there is a
potential for man-in-the-middle attacks"


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

[Alex Bligh]

[-- Attachment #1: Type: text/plain, Size: 2166 bytes --]

Eric,

(this crossed with v2)

On 7 Apr 2016, at 16:35, Eric Blake <eblake@redhat.com> wrote:

> On 04/07/2016 06:36 AM, Alex Bligh wrote:
>> 
>> On 7 Apr 2016, at 13:13, Alex Bligh <alex@alex.org.uk> wrote:
>> 
>>> I guess it's worth documenting
>>> this, though I thought it was obvious.
>> 
>> The next version will have this section:
>> 
>> ### Downgrade attacks
>> 
>> A danger inherent in any scheme	relying	on the negotiation
> 
> too much space

Yeah the paste between emacs and OS-X Mail probably has
tabs in. I checked version 2 with hexdump -C and
that line is OK.

>> * 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 oeprating in a plain-text
> 
> s/oeprating/operating/

thx

>>  manner with the MitM (with the session being possibly
>>  proxied to the server with the method above).
> 
> s/server/client/

thx

>> 
>> With regard to the first, any client that does not wish
>> to be subject to potential downgrade attack SHOULD ensure
>> that if	a TLS endpoint is specified by the client, it
>> ensures	that TLS is negotiated prior to	sending	or
>> requesting sensitive data. To recap, yhe client MAY send
> 
> s/yhe/the/

thx

>> `NBD_OPT_STARTTLS` at any point	during option haggling,
>> and MAY	disconnect the session if `NBD_REP_ACK`	is not
>> provided.
> 
> Probably want to add: "but the client SHOULD strongly consider sending
> `NBD_OPT_STARTTLS` as its first option"

That's now elsewhere, but I've expanded that anyway in v2.

>> With regard to the second, any server that does	not wish
>> to be subject to a potential downgrade attack SHOULD either
>> used FORCEDTLS mode, or	should force TLS on those exports
>> it is concerned about using SELECTIVE mode and TLS-only
>> exports. It is not possible to avoid downgrade attacks
>> on exports which are may be served either via TLS or
>> in plain text.
> 
> Probably want to add: "OPTIONALTLS mode SHOULD NOT be used if there is a
> potential for man-in-the-middle attacks"

I've said "where man-in-the-middle attacks are a concern".

These will all be in v3.

--
Alex Bligh





[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 842 bytes --]

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

[Wouter Verhelst]

On Thu, Apr 07, 2016 at 12:35:59PM +0100, 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 <alex@alex.org.uk>
> ---
>  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
> +
> +NBD implementations supporting TLS MUST support TLS version
> +1.2, and MAY support other (earlier or later) versions of
> +TLS/SSL.
> +
> +This standard does not specify what encryption, certification
> +and signature algorithms are used. This standard does not
> +specify authentication and authortisation (for instance
> +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,
> +  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
> +  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

No. UNSUP (one P) is reserved for "this server doesn't support that"
(i.e., backwards compatibility). Not configured for supporting TLS would
be NBD_REP_ERR_POLICY.

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

I think this mode is effectively the same as what you call selective,
modulo that no exports have any TLS requirements, so I wouldn't specify
it as a separate mode of operation (save perhaps that you may want to
discourage it)

> +#### 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`,
> +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.

Right. Clearly this can't be a must, because qemu already implements
this and doesn't do INFO :-)

[...]
> +The server MUST NOT send `NBD_REP_ERR_TLS_REQD` in reply to
> +any command if TLS has already been neogitated. The server

negotiated

[...]
> +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
> +cllient receives `NBD_REP_ACK` in response, it
> +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.

That, or NBD_REP_ERR_POLICY.

[...]
> +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

client, not server

> +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
> +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
> +replied, it is guaranteed the server is not in this mode.

UNSUP or POLICY

(actually, "any error". If STARTTLS errors, the server effectively does
not support TLS)

[...]
>  - `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_UNSUP`.

(or POLICY)

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

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

[Wouter Verhelst]

On Thu, Apr 07, 2016 at 02:56:48PM +0100, Daniel P. Berrange wrote:
> I don't really agree that there's a use case of mixing
> tls & non-tls exports in the same server.

There is: swap-on-NBD and TLS do not mix.

Without special handling, swapping to the network is prone to deadlocks,
because the machine doesn't know when a block of swapped data has been
written to disk until it receives and processes the ACK from the server
that tells it the data is safe. This means it must keep reading from the
server until that ACK is received. Since the reason you're paging memory
to your swap space is, most likely, the fact that you're low on memory,
swapping to the network while simultaneously trying to view a 4K video
from youtube will get you deadlocked in seconds.

To fix that, a PF_MEMALLOC kernel-level socket option was created a few
years ago. If the kernel is low on memory, it will drop all network
packets save those destined for a socket with that option set. In doing
so, the 4K youtube video data will be dropped until the low memory
situation is resolved, but the reply packet from the NBD server will
arrive.

However, only the kernel can set the PF_MEMALLOC socket option; and due
to the way in which we pass the nbd socket to the kernel, having it
decode the TLS conversation seems complicated, if it even is possible.
Therefore, decoding the TLS conversation when the client is going to be
the kernel, will need to be done in userspace. The nbd-client would need
to call socketpair(), fork(), pass one end of the pair in the child
process to the kernel, and be a decoding proxy in the parent.

Since you *need* to have non-TLS NBD if you want to support
non-deadlocking swapspace over the network (one of the major use cases
for NBD), you *need* to have a server which supports non-TLS NBD.

Requiring that a user uses two different NBD servers, one with TLS
enabled and one with TLS disabled, just so swapping to NBD is possible,
seems silly.

Therefore, the reference implementation is not going to *require* that
all exports do TLS if TLS is enabled; however, it *is* going to default
to that.

> > Incidentally, the qemu client does not appear to assume the
> > server is 'FORCEDTLS', and IIRC from time spend staring into
> > Wireshark yesterday sends its NBD_OPT_LIST prior to the TLS
> > upgrade. I can check that if you like.
> 
> If the qemu NBD client has TLS credentials set then it will
> refuse to connect to a server without TLS.

That seems like a valid and safe mode of operation.

> The OPT_TLS is definitely the first thing it sends, becasue the QEMU
> NBD server will reject all options until OPT_TLS has been sent.

So if I want to swap to qemu-nbd, I cannot also have encrypted
connections to the same server. Got it.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

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

[Wouter Verhelst]

On Thu, Apr 07, 2016 at 08:31:23AM -0600, Eric Blake wrote:
> > +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.

Yes.

> 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

Actually, such a server is technically impossible ;-)

> (since TLS was introduced at the same time as fixed newstyle,

Eh. I don't know where you got that idea, but that's absolutely not
true. Fixed newstyle was introduced five years ago, TLS was introduced
last year or so.

> we can reasonably require, rather than just suggest, that both things
> be implemented at once to be a compliant FORCEDTLS server).

We have to make fixed newstyle a dependency of any form of tls, but
nothing more seems appropriate.

("fixed newstyle" is necessary for *anything* that is not
NBD_OPT_EXPORT_NAME)

[...]

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

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

[Alex Bligh]

On 9 Apr 2016, at 10:36, Wouter Verhelst <w@uter.be> wrote:

>> +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
> 
> No. UNSUP (one P) is reserved for "this server doesn't support that"
> (i.e., backwards compatibility). Not configured for supporting TLS would
> be NBD_REP_ERR_POLICY.

'P' fixed in later draft.

Right, but a server which doesn't actually have TLS at all will
by definition be running in NOTLS mode, and thus will be replying
NBD_REP_ERR_UNSUP (because that's what you reply with if you
receive an option you don't understand). Allowing NBD_REP_ERR_POLICY
to be sent as as a reply too would be fine.

> 
>> +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.
> 
> I think this mode is effectively the same as what you call selective,
> modulo that no exports have any TLS requirements, so I wouldn't specify
> it as a separate mode of operation (save perhaps that you may want to
> discourage it)

See the section on 'degenerate cases' in the last version. I agree
OPTIONALTLS is not to be encouraged, however it's there so it's possible
for a server to provide optionality when it doesn't support INFO.
As INFO is merely experimental, I didn't want to make a 'MUST' dependency
on it and thus lose all ability to do optional TLS without INFO.

>> +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.
> 
> Right. Clearly this can't be a must, because qemu already implements
> this and doesn't do INFO :-)

Yeah though Eric has (I think) submitted a patch to qemu to support
that.

> 
> [...]
>> +The server MUST NOT send `NBD_REP_ERR_TLS_REQD` in reply to
>> +any command if TLS has already been neogitated. The server
> 
> negotiated

I'd make sure you're looking at the latest version as Eagle Eyed Eric
pointed out a whole pile of these.

> 
> [...]
>> +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
>> +cllient receives `NBD_REP_ACK` in response, it
>> +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.
> 
> That, or NBD_REP_ERR_POLICY.

Yeah I can make that an alternative.

> 
> [...]
>> +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
> 
> client, not server

Again, EEE fixed this.
> 
>> +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
>> +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
>> +replied, it is guaranteed the server is not in this mode.
> 
> UNSUP or POLICY

OK

> (actually, "any error". If STARTTLS errors, the server effectively does
> not support TLS)

Well NBD_REP_ERR_INVALID means "The option sent by the client is known by
this server, but was determined by the server to be syntactically invalid."
which means the client has done something wrong. Given we've defined the
legal responses to NBD_OPT_STARTTLS I'd rather keep that one.

> 
> [...]
>> - `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_UNSUP`.
> 
> (or POLICY)

OK


> -- 
> < ron> I mean, the main *practical* problem with C++, is there's like a dozen
>       people in the world who think they really understand all of its rules,
>       and pretty much all of them are just lying to themselves too.
> -- #debian-devel, OFTC, 2016-02-12
> 

-- 
Alex Bligh

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

[Alex Bligh]

On 9 Apr 2016, at 10:50, Wouter Verhelst <w@uter.be> wrote:

> So if I want to swap to qemu-nbd, I cannot also have encrypted
> connections to the same server. Got it.

AFAIK qemu-nbd only supports a single export so this isn't
really an issue.

-- 
Alex Bligh

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

[Alex Bligh]

On 9 Apr 2016, at 10:55, Wouter Verhelst <w@uter.be> wrote:

> 
> Yes.
> 
>> 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
> 
> Actually, such a server is technically impossible ;-)

Yeah, you really want to be reading v5 of this patch set by which
time we cleared all this up :-)

-- 
Alex Bligh

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

[Wouter Verhelst]

On Sat, Apr 09, 2016 at 11:05:16AM +0100, Alex Bligh wrote:
> 
> On 9 Apr 2016, at 10:50, Wouter Verhelst <w@uter.be> wrote:
> 
> > So if I want to swap to qemu-nbd, I cannot also have encrypted
> > connections to the same server. Got it.
> 
> AFAIK qemu-nbd only supports a single export so this isn't
> really an issue.

qemu-nbd does, but the builtin server of qemu will export all virtual
hard disks connected to the VM, so it supports multiple exports.

(yes, that implies that swapping to a multiple-export qemu server is
probably a bad idea)

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

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

[Wouter Verhelst]

On Sat, Apr 09, 2016 at 11:04:09AM +0100, Alex Bligh wrote:
[...]
> > [...]
> >> +The server MUST NOT send `NBD_REP_ERR_TLS_REQD` in reply to
> >> +any command if TLS has already been neogitated. The server
> > 
> > negotiated
> 
> I'd make sure you're looking at the latest version as Eagle Eyed Eric
> pointed out a whole pile of these.

Yeah, I noticed :-)

> > [...]
> >> +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
> >> +cllient receives `NBD_REP_ACK` in response, it
> >> +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.
> > 
> > That, or NBD_REP_ERR_POLICY.
> 
> Yeah I can make that an alternative.

POLICY is the correct message; UNSUP is the alternative ;-)

(as in "for backwards compatibility, a client should also be prepared...")

[...]
> > (actually, "any error". If STARTTLS errors, the server effectively does
> > not support TLS)
> 
> Well NBD_REP_ERR_INVALID means "The option sent by the client is known by
> this server, but was determined by the server to be syntactically invalid."
> which means the client has done something wrong. Given we've defined the
> legal responses to NBD_OPT_STARTTLS I'd rather keep that one.

Fair enough. Also, INVALID is the correct error message when the client sent
NBD_OPT_STARTTLS while inside a TLS connection, too, so that would've been a
contradiction ;-)

> > [...]
> >> - `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_UNSUP`.
> > 
> > (or POLICY)
> 
> OK

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12