Thanks for the update! Karthik R
On Sun, Aug 12, 2018 at 4:27 AM Robert Dale <[email protected]> wrote: > Karthik, basic coding is done on branch TINKERPOP-2023. I will have to add > some additional tests and docs before creating the PR. > > Client connectionPool: > keyStore > keyStorePassword > trustStore > trustStorePassword > keyStoreType > sslEnabledProtocols > sslCipherSuites > sslSkipCertValidation > > Server sslSettings: > keyStore > keyStorePassword > trustStore > trustStorePassword > keyStoreType > sslEnabledProtocols > sslCipherSuites > > Robert Dale > > > On Fri, Aug 10, 2018 at 5:29 PM Robert Dale <[email protected]> wrote: > > > Actually, I was thinking of adding those to just the server. The server > > usually dictates the security requirements. But we could add it to the > > client as well. I think it makes sense to keep them symmetric. > > > > Robert Dale > > > > > > On Fri, Aug 10, 2018 at 2:43 PM kARTHIK R <[email protected]> wrote: > > > >> Both proposals look good, if we are making a backwards incompatible > >> change, > >> then it makes sense to clean it up and address other issues around that > >> space. Limiting cipherSuites and TLS versions -> Are you planning to add > >> those settings to Gremlin Server as well, or just on the client? Is this > >> something that you will be picking up soon? Just wanted to weigh the > >> option > >> of waiting for this fix versus hacking something up at my end. > >> > >> Thanks! > >> Karthik R > >> > >> > >> On Thu, Aug 9, 2018 at 1:06 PM Robert Dale <[email protected]> wrote: > >> > >> > The other thing we can do is add these configuration keys: > >> > keyStore > >> > keyStorePassword > >> > trustStore > >> > trustStorePassword > >> > keystoreType > >> > > >> > This is more natural for a java application and would allow reading > Java > >> > keystore and PKCS12 files. > >> > > >> > Then keyCertChainFile, keyFile, keyPassword, trustCertChainFile become > >> > optional. We could even go as far as deprecating these. > >> > > >> > Also, add ability to limit enabled protocols. Because no one should > be > >> > using < TLSv1.2, right? This should be the default. Will also add > >> cipher > >> > suites. > >> > sslEnabledProtocols > >> > sslCipherSuites > >> > > >> > Robert Dale > >> > > >> > > >> > On Thu, Aug 9, 2018 at 3:21 PM Robert Dale <[email protected]> wrote: > >> > > >> > > Looks about right. Yes, it would be a breaking change. And > something > >> > will > >> > > be included in the Upgrade section. > >> > > > >> > > As a workaround, you can just point to your local system ca cert > >> bundle. > >> > > e.g. on Fedora 27, you would configure like this: > >> > > > >> > > conf/remote-secure.yaml: > >> > > hosts: [my.fqdn.com] > >> > > port: 443 > >> > > connectionPool: { > >> > > enableSsl: true, > >> > > trustCertChainFile: > >> /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem} > >> > > serializer: { className: > >> > > org.apache.tinkerpop.gremlin.driver.ser.GryoMessageSerializerV1d0, > >> > config: > >> > > { serializeResultToString: true }} > >> > > > >> > > > >> > > > >> > > Robert Dale > >> > > > >> > > > >> > > On Thu, Aug 9, 2018 at 2:50 PM kARTHIK R <[email protected]> > wrote: > >> > > > >> > >> I agree to the proposal, thanks for driving it! Would like to > >> understand > >> > >> what your thoughts are on the fix/change though. From my > >> understanding, > >> > >> this is your plan: > >> > >> 1. trustCertChain file will be an optional setting, if mentioned, > we > >> > would > >> > >> create a TrustManager that would use the mentioned CA. Else it > would > >> use > >> > >> the default runtime truststore. (default JRE truststore in the case > >> of > >> > >> Java) > >> > >> 2. Add a new setting (say skipCertValidation) which will be similar > >> to > >> > -k > >> > >> option of CURL where endpoint validation would be skipped. > Defaulted > >> to > >> > >> false. Clients would need to opt in to this. > >> > >> > >> > >> One thing to note is that this would be a backwards incompatible > >> change > >> > >> for > >> > >> clients if I'm not mistaken. Clients who originally used the old > >> driver > >> > >> (and used to skip cert validation) would start to break now unless > >> they > >> > >> explicitly update the skipCertValidation setting to true. > >> > >> Let me know your thoughts. > >> > >> > >> > >> I'm pretty much blocked on this as of now, and as a workaround I'm > >> > trying > >> > >> to download the CA cert and wiring that to my client. > >> > >> > >> > >> Karthik R > >> > >> > >> > >> > >> > >> On Thu, Aug 9, 2018 at 9:06 AM Robert Dale <[email protected]> > >> wrote: > >> > >> > >> > >> > Just to clarify, the PR shall be done, not CTR'ed. > >> > >> > > >> > >> > Robert Dale > >> > >> > > >> > >> > > >> > >> > On Thu, Aug 9, 2018 at 8:42 AM Robert Dale <[email protected]> > >> wrote: > >> > >> > > >> > >> > > > >> > >> > > An issue was raised on the user list [1] that the Cluster > client > >> SSL > >> > >> > > requires the trustCertChain to be set explicitly or suffer the > >> wrath > >> > >> of > >> > >> > > WARN logging. And that got me thinking about the server too. > It > >> > >> seems > >> > >> > > like the only good reason to have the client trust everything > by > >> > >> default > >> > >> > is > >> > >> > > simply because the server's most simple ssl default is to > create > >> > >> > > self-signed certs. Since there is no access to these certs, > they > >> > can > >> > >> not > >> > >> > > be imported into the client thus the client must trust > anything. > >> > >> Well, > >> > >> > > this is not good security all the way around. > >> > >> > > > >> > >> > > If we take a 'security should be secure be default' posture, > then > >> > >> > > 1) the client would never trust all certs without verification > by > >> > >> > default. > >> > >> > > logging messages, at any level, are not sufficient. > >> > >> > > 2) the server would never create self-signed certs. It's not > >> that > >> > >> hard > >> > >> > to > >> > >> > > create self-signed certs anyway. > >> > >> > > > >> > >> > > Instead, the server should require certs to be configured > >> explicitly > >> > >> > > whether self-signed or commercial. The client should trust > >> packaged > >> > >> ca > >> > >> > > certs by default. The client can be configured to trust > >> self-signed > >> > >> > certs. > >> > >> > > An additional config option can be used to configure the client > >> to > >> > >> > > disregard hostname. (This is where the hostname doesn't match > the > >> > >> cert. > >> > >> > It > >> > >> > > is sometimes a necessary evil). In order to use self-signed > >> certs, > >> > >> the > >> > >> > > rather painless process to configure the client and server will > >> have > >> > >> to > >> > >> > be > >> > >> > > followed. This is pretty standard stuff for applications. > >> > >> > > > >> > >> > > I've create two tickets to address these issues. > >> > >> > > TINKERPOP-2022 - > >> > https://issues.apache.org/jira/browse/TINKERPOP-2022 > >> > >> > > TINKERPOP-2023 - > >> > https://issues.apache.org/jira/browse/TINKERPOP-2023 > >> > >> > > > >> > >> > > If there are no objections in 72 hours then it shall be done > >> > starting > >> > >> > with > >> > >> > > tp32. > >> > >> > > > >> > >> > > 1. > >> > >> > > >> > > https://groups.google.com/d/msg/gremlin-users/2z7Si4wM0SU/lL3OqR0LCgAJ > >> > >> > > > >> > >> > > Robert Dale > >> > >> > > > >> > >> > > >> > >> > >> > > > >> > > >> > > >
