Dan Burkert has posted comments on this change. Change subject: WIP: rpc: Initiate TLS connection upgrade following SASL negotiation ......................................................................
Patch Set 1: > I was under the impression that previously, it was decided that the SASL > negotiation would need to be encrypted as well. Would that be the case when > we have the server flag that forces TLS? Yes, the original design called for initiating TLS *then* SASL inside TLS. After having though more about this, I've come to a couple conclusions: 1) It's much easier to do SASL then TLS if we want to retain backwards compatibility with old servers/clients - this commit is the proof. Negotiating TLS before SASL will require much bigger changes to the RPC layer. 2) I don't think it makes a difference, security wise, whether TLS or SASL comes first. We should check with an expert on this. My reasoning is pretty simple - if wrapping TLS around the SASL negotiation makes it more secure, then the security guarantees of SASL are broken. A passive listener learning about an individual connections' SASL credentials are the least of our worries if this is the case. I'd like to get everyones thoughts on this - I've thought a lot about it, but I may have missed some salient facts. > the downgrade attack seems very easy to do. Is there any way to deal with > this when someone sets the future server side "force TLS" flag to off It would be easy for a MITM, yes. The "force TLS" flag is the necessary and sufficient protection against this - with this flag turned on non-TLS connections will be rejected by the server. There's no way to make the upgrade protocol smarter to protect against this attack - necessarily this protocol is done outside of TLS, and is therefore MITM-able. -- To view, visit http://gerrit.cloudera.org:8080/5484 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If44a71186eb2cdeebaf46cc372596f3ee6b47ac0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: No
