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

[Wouter Verhelst]

On Thu, Apr 07, 2016 at 07:32:47PM +0100, Alex Bligh wrote:
[...]
> +### 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

Since you say zero here, how is it different from OPTIONALTLS?

If "not at all", just drop optional.

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

I'm not *that* well versed in the details of TLS, but isn't it better to
specify which side should go first?

[...]
> +If the server receives any other option, including `NBD_OPT_INFO`
> +and unsupported options, it MUST reply with `NBD_REP_ERR_TLS_REQD`
> +if TLS has not been initiated; `NBD_OPT_INFO` is included as in this
> +mode, all exports are TLS-only. If the server receives a request to
> +enter transmission mode via `NBD_OPT_EXPORT_NAME` when TLS has not
> +been initiated, then as this request cannot error, it MUST
> +disconnect the connection. If the server receives a request to

s/disconnect/terminate/ reads slightly better.

[...]
> +The client MUST NOT issue `NBD_OPT_STARTTLS` unless the server
> +set flag NBD_FLAG_FIXED_NEWSTYLE and the client replied
> +with NBD_FLAG_C_FIXED_NEWSTYLE in the fixed newstyle
> +negotiation.

Why not "unless fixed newstyle negotiation is in effect"? No need to
repeat that definition.

[...]
> +### Security considerations
> +
> +#### TLS versions
> +
> +NBD implementations supporting TLS MUST support TLS version 1.2,
> +SHOULD support any later versions, and MAY support older versions.

I would prefer "SHOULD NOT allow TLS versions older than 1.2" here.
There are some serious flaws in older TLS versions; currently these are
still supported by most web browsers for backwards compatibility
reasons, but that does not apply for us.

[...]

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