Wouter, On 6 Apr 2016, at 19:32, Wouter Verhelst <w@...112...> wrote: > On Wed, Apr 06, 2016 at 02:37:07PM +0100, Alex Bligh wrote: >>> From the proto.md: >> >>> 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). >> >> I think the last SHOULD is wrong and should be deleted. > > If the INFO extension is no longer experimental, indeed. Not now, > though. > > The original (and until INFO is implemented, current) semantics of > TLS_REQD are that it is *only* legal to be used if the server does not > want to speak to the client (at all, for any export) unless TLS has been > negotiated first. In such a case, negotiating over an unencrypted > channel seems wrong, and the server should reject that. Well surely the server doesn't need to force TLS on the client unless the client rejects it? IE even without NBD_OPT_INFO and friends you could have a server which could server both TLS and non-TLS clients, simply leaving it to the client to choose whether it wants to negotiate TLS or not. >> Firstly, this implies a server should reply with NBD_REP_ERR_TLS_REQD even >> before it knows the client even supports TLS. That's wrong. > > If the client doesn't support TLS, it cannot speak to a TLS-only server, > regardless of TLS_REQD. Sure. But it can speak to a TLS-optional server. > >> It even implies the server should sent it even if *it* doesn't support >> TLS. > > No, and I'm not sure how you come to that conclusion. Because the spec says "If this reply is used, servers SHOULD send it in reply to each and every unencrypted NBD_OPT_* message (apart from NBD_OPT_STARTTLS)."; the first "if" is somewhat circular, but a legal server could send this reply to every message (therefore the reply is "used") and not support NBD_OPT_STARTTLS. I think I may have exaggerated by using "implied" but the wording is infelicitous. >> Secondly, even if by magic the server somehow knows that the client supports >> TLS, and it supports TLS too, it makes it impossible for a server to serve >> both TLS and non-TLS exports as it would force the client to negotiate TLS to >> process (say) NBD_OPT_LIST, and there's then no way of un-negotiating TLS. > > The spec clearly says "MUST NOT send this error if it has one or more > exports that do not require TLS", so that's a non-issue. > >> I think this should thus be deleted. > > No, it must stay. > > There's currently no way to detect whether a particular export supports > TLS. If the client wants to connect to an export that the server only > exports through TLS, then the server must drop the connection upon > NBD_OPT_EXPORT_NAME. This is part of why we need INFO/GO. > > INFO/GO modifies TLS_REQD in that it makes it legal for TLS_REQD to be > sent as an error reply *for those two requests* if a particular export > requires TLS but another one does not. Once the INFO extension is no > longer experimental, the above-quoted language will indeed need to be > changed, but for now a server can only send it in the "I don't do no > steenking cleartext" case, and then that language is correct. Mmmm... I think it could be 'improved' then :-) -- Alex Bligh
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail