> I have commented your change: setProtocols is still redundant since we have protocol in sslContextFactory.
it's not the same, the protocol field allows to set only one particular protocol, while protocols allow you to set several different versions, for example with protocols you can set: TLSv1.1 and TLSv1.2, but exclude TLSv1.0. > there is a caveat with ciphers list: by default SSL will fail if you ever list a cipher there that is not present in current JVM these are options for subtle SSL configuration, so it requires some understanding what you do. On Mon, Jul 30, 2018 at 6:54 PM Ilya Kasnacheev <ilya.kasnach...@gmail.com> wrote: > Hello! > > Thank you! I have commented your change: setProtocols is still redundant > since we have protocol in sslContextFactory. > > BTW, there is a caveat with ciphers list: by default SSL will fail if you > ever list a cipher there that is not present in current JVM, even if the > rest of them are present and can be used. Thus the configuration becomes > fragile. However I don't think it's our job to take care of that. > > Regards, > > -- > Ilya Kasnacheev > > 2018-07-30 18:12 GMT+03:00 Michael Cherkasov <michael.cherka...@gmail.com > >: > > > Hi all, > > > > as was suggested, I removed all mention about SSLParameters and replaced > it > > with a simple String[]. > > I added configurations for protocols and cipher suites. > > > > Please, review <https://github.com/apache/ignite/pull/4440/files>. > > > > Thanks, > > Mike. > > > > > > > > 2018-07-30 13:58 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > > > > > Agree. It is much easier for user to set a collection of strings > > > (especially from Spring), rather than construct some complex Java 8 > > object. > > > Also we need to remember that this feature should be propagated to all > > thin > > > clients and .NET integration. String getter/setter is only way to > > maintain > > > API consistency across various platforms. > > > > > > On Thu, Jul 26, 2018 at 9:16 PM Ilya Kasnacheev < > > ilya.kasnach...@gmail.com > > > > > > > wrote: > > > > > > > Hello! > > > > > > > > I really dislike the fact that SSLParameters has 6 setter methods, > and > > we > > > > only support one of them, when two more clash with SSL settings which > > are > > > > set elsewhere. > > > > > > > > I.e. what happens if I pass SSLParameters with > > setAlgorithmConstraints() > > > or > > > > setProtocols() called on them? > > > > > > > > Is it possible that we will just have an array of allowed ciphers in > > > > configuration? > > > > > > > > Regards, > > > > > > > > -- > > > > Ilya Kasnacheev > > > > > > > > 2018-07-26 20:16 GMT+03:00 Michael Cherkasov < > > > michael.cherka...@gmail.com > > > > >: > > > > > > > > > Hi all, > > > > > > > > > > I want to add SSLParameters for SslContextFactory. > > > > > > > > > > Right now there's no way to specify a particular set of cipher > suites > > > > that > > > > > you want to use. > > > > > there's even old request to add this functionality: > > > > > https://issues.apache.org/jira/browse/IGNITE-6167 > > > > > even with current API you can achieve this, but this requires a lot > > of > > > > > boilerplate code, to avoid this I added SSLParameters, that would > be > > > > > applied to all SSL connections, please review my pull request: > > > > > https://github.com/apache/ignite/pull/4440 > > > > > > > > > > I think this patch covers 6167, so I want to push it in context of > > this > > > > > ticket. > > > > > > > > > > Thanks, > > > > > Mike. > > > > > > > > > > > > > > > -- Thanks, Mikhail.
