Update config API for TLS to match DIP #126
Conversation
This modifes the Config API to talk about encryption rather than TLS, as well as renaming the on/off and the tls auth config to EncryptionLevel and TrustStrategy, respectively.
| public enum EncryptionLevel | ||
| { | ||
| /** With this level, the driver will only connect to the server if it can do it without encryption. */ | ||
| REJECTED, |
There was a problem hiding this comment.
This name feels off. @nigelsmall?
There was a problem hiding this comment.
Agreed. Not even sure we need two levels of settings (security on/off + security mode). We should probably be able to make do with something like SECURITY_NONE, SECURITY_TOFU and SECURITY_FULL.
There was a problem hiding this comment.
FULL is misleading also. I presume you mean certificate authority based? So perhaps SECURITY_CERT?
Which, I think, makes it the same as the TrustStrategy.
There was a problem hiding this comment.
Why not just ENCRYPTED and PLAIN_TEXT? Isn't that what this is about? And the class could be something like "EncryptionPolicy"
There was a problem hiding this comment.
+1. BTW, libneo4j-client handles this with a single INSECURE flag, which is essentially a Boolean, since there is only two different "levels" anyway.
There was a problem hiding this comment.
The point of REQUIRED/REJECTED was that it allows adding OPTIONAL later, so if you're connecting to a server that may or may not require TLS, the driver will work either way. This is a nice option if you want to centralize control of this policy - you only have to configure the server to say REQUIRE, and as long as all the drivers are set to OPTIONAL they will follow suit.
Without that option it becomes hard, I think, to change the policy without full system downtime as well. This is how Oracle defines their encryption policy. (actually, they have a fourth, something like DESIRED which solves an ambiguity if both sides are configured as OPTIONAL, but I can't see a real use case for that, just default to using encryption if both sides are set to OPTIONAL)
Obviously, we don't have OPTIONAL on the driver side here, but the naming of the other two was intended to leave a slot for that in a later version.
There was a problem hiding this comment.
Also, note that this policy is orthogonal to how the driver determines trust. This policy is choosing if encryption is required, optional or rejected. How trust is established is a separate variable.
There was a problem hiding this comment.
Right. But it's probably a bit speculative to leave an option for OPTIONAL on the client (driver) side. Indeed, it's not even possible in the current protocol, since the client starts the conversation and has to choose one or the other with no knowledge or negotiation.
There was a problem hiding this comment.
It's certainly possible - the driver would try encrypted first, and degrade to non-encrypted. OPTIONAL was part of the design and was originally in the DIP, but we agreed to wait to include it due to time constraints. Since this is a useful operational feature, and the overhead of leaving space here (calling this EncryptionPolicy.REQUIRED/REJECTED vs EncryptionPolicy.SECURE/INSECURE as you suggest) is minimal, I think the alternative paints us into a corner for very little benefit while this leaves a nice door open to add this feature later.
There was a problem hiding this comment.
Ah, you mean try two independent connections? Yes, that'd work. I was referring to the inability to decline a TLS handshake, but of course that can be done by closing the connection and starting again.
zhenlineo
force-pushed
the
1.0-tls
branch
2 times, most recently
from
d132626 to
e30758e
Compare
Update config API for TLS to match DIP
No description provided.