Re: SSLParameters for SslContextFactory
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 : > > 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 > > 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 : > > > > > > > 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. >
Re: SSLParameters for SslContextFactory
> 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 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 >: > > > 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 : > > > > > 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.
Re: SSLParameters for SslContextFactory
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 : > 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 : > > > 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. > > > > > > > > > >
Re: SSLParameters for SslContextFactory
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 : > 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 > > 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. > > > > > >
Re: SSLParameters for SslContextFactory
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 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 >: > > > 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. > > >
Re: SSLParameters for SslContextFactory
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 : > 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. >
SSLParameters for SslContextFactory
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.
