qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alex Bligh
Subject: Re: [Qemu-devel] [PATCH] Improve documentation for TLS
Date: Thu, 7 Apr 2016 15:57:33 +0100

On 7 Apr 2016, at 15:31, Eric Blake <address@hidden> 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: signature.asc
Description: Message signed with OpenPGP using GPGMail


reply via email to

[Prev in Thread] Current Thread [Next in Thread]