Hi all, Looks like no one has objections about the API I want to introduce. Could someone please review and push my patch?
Thanks, Mike. вт, 31 июл. 2018 г. в 13:02, Mikhail Cherkasov <mcherka...@gridgain.com>: > > 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. >
