I've reviewed the PR and I believe I understand the use case, but I feel a
bit uncomfortable with the misuse of SNI. As I understand it, and as it has
been already mentioned, SNI is used to determine which SSL certificate
should be presented to a client.
I think that CLIENT_HELLO_EXTENSION should *not* be associated with SSL,
and that a new client property should be used instead. That is, a different
property, unrelated to SSL, should be used to set what will be verified
against CLIENT_HELLO_EXTENSION.
On Tue, Nov 19, 2019 at 4:43 PM Jens Deppe <jde...@pivotal.io> wrote:
> I'd like to add my comment from the original PR here again:
>
>
> Although I support the particular use case, I would prefer the
> implementation being a bit more abstracted. Specifically, if we provided an
> extension point which would allow modification of SSLParameters then we
> wouldn't be coupling to a particular use case. So I'm thinking that the
> user would define (via say a ssl-parameter-extension parameter) a class
> that takes in a SSLParameter and perhaps SSLConfig and whatever else for
> this use-case and does what it needs. The user class would need to
> implement an interface something like this:
>
> public interface SSLParameterExtension {
> SSLParameter modify(SSLParameter, SSLConfig);
> }
>
> I would imagine seeing the user implementation of this being attached to
> SSLConfig.
>
>
> (https://github.com/apache/geode/pull/4310)
>
> I don't mind (mis)using the Server Name field to convey some other
> information, but since it's possible to abstract the specific nature and
> application of that information, I think we should do so. Anyone else who
> looks at the code is going to wonder what the use is especially if the
> consumer of that particular piece of info is going to be provided via an
> external SSLEngine implementation.
>
> --Jens
>
> On Tue, Nov 19, 2019 at 1:18 PM Dan Smith <dsm...@pivotal.io> wrote:
>
> > Can you clarify which connections will use this ssl-server-name-extension
> > as part of the Client Hello? client to locator, client to server, server
> to
> > server, WAN site to WAN site, ... all of the above?
> >
> > I'm fine with adding the new property.
> >
> > At some point, I think we need to think about making it easier to plug in
> > custom logic to control the entire socket creation and TLS handshake. I
> > think technically you can take over the whole process if you set the
> > ssl-use-default-context property and then configure the default
> SSLContext
> > for your entire process, but that is not ideal.
> >
> > -Dan
> >
> > On Tue, Nov 19, 2019 at 8:24 AM Charlie Black <cbl...@pivotal.io> wrote:
> >
> > > I have read the e-mail and the ticket I am not sure how this field is
> > going
> > > to be used. Maybe you can expand on the intent of this field.
> > >
> > > From the property "ssl-server-name-extension" it feels like we are
> > > intending to correlate with something presented in the SSL certificate.
> > > It would be great if that was explained plainly for the reader in more
> > > detail.
> > >
> > > For now I can only -1.
> > >
> > > Charlie
> > >
> > > On Tue, Nov 19, 2019 at 3:27 AM Mario Ivanac <mario.iva...@est.tech>
> > > wrote:
> > >
> > > > Hi geode dev,
> > > >
> > > > as a part of solution for
> > > https://issues.apache.org/jira/browse/GEODE-7414
> > > > we would like to introduce new config property
> > > "ssl-server-name-extension".
> > > >
> > > > This property will contain generic string, which will be added as
> > Server
> > > > Name Indication (SNI) parameter to Client Hello message.
> > > >
> > > > Do you agree with this proposal?
> > > >
> > > > Thanks,
> > > > Mario
> > > >
> > >
> > >
> > > --
> > > Charlie Black | cbl...@pivotal.io
> > >
> >
>
