Re: SSLParameters for SslContextFactory

[Michael Cherkasov <mi...@gmail.com>]

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 <mc...@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.kasnacheev@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.cherkasov@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 <vo...@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.kasnacheev@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.cherkasov@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.
>