[Proposal] SSL - hostname verification

[Sai Boorlagadda]

Geode currently does not implement hostname verification and is probably
not required per TLS spec. But it can be supported on TLS as an additional
check. The new proposed[1] implementation to use the default SSL context
could cause certain man-in-the-middle attacks possible if there is no
hostname verification. So in order to harden against such attacks, it puts
additional responsibilities on users to provide a custom TrustManager to
perform additional checks on server identity.

This is a proposal to add a new boolean SSL property
`ssl-enable-endpoint-identification` which enables hostname verification
for secure connections. For services exposed on HTTPS like pulse, dev-rest,
and admin-rest API, hostname validation is enabled by default and can be
skipped using `--skip-ssl-validation'. This new parameter especially
enables client applications to verify server's hostname using server
certificate during SSL handshake. The same parameter can also provide a
means to enable verification within distributed systems when a peer (acting
as an HTTPS client) accepting a connection from another peer (acting as
HTTPS server).

The proposed parameter can also be enabled when not using the default
context, in any case, we should update the documentation to clarify the
current behavior and a recommendation.

[1]
https://lists.apache.org/thread.html/b89beb6e57eb043786791c9309f13feca225a1c5119632f3fa7b7cb9@%3Cdev.geode.apache.org%3E

Sai

Re: [DISCUSS] Streamline return value from RemoteQuery

[David Kimura]

I have a couple questions:

Do you have an idea or theories of what was the original intent behind
separating ResultSet and StructSet?

Is execute a blocking or non-blocking call and does the interface have any
guarantee of that?

Thanks,
David

On Mon, Aug 13, 2018 at 4:06 PM, Ernest Burghardt 
wrote:

> Currently, geode-native's query::execute returns a
> shared_ptr and
> that pointer can be either ResultSet or StructSet.
>
>
> RemoteQuery::execute contains logical code to determine with QueryResults
> are greater than 0... We should look at removing this logic and only
> returning StructSets
> This allows removal of ResultSet which will streamline the API and
> associated code...
>
> This duality is unnecessary and should be removed.
> I am proposing that the results only return  StructSet
>
> Best,
> EB
>

Re: [Proposal] SSL - hostname verification

[Jacob Barrett]

On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda 
wrote:

> Geode currently does not implement hostname verification and is probably
> not required per TLS spec. But it can be supported on TLS as an additional
> check. The new proposed[1] implementation to use the default SSL context
> could cause certain man-in-the-middle attacks possible if there is no
> hostname verification.


The current SSL implementation is also susceptible to man-in-the-middle as
well. This proposal is really independent of those proposed changes.


> This is a proposal to add a new boolean SSL property
> `ssl-enable-endpoint-identification` which enables hostname verification
> for secure connections.


Are you proposing that we ship a custom trust manager that verifies hosts
on all TLS connections? I would rather shy away from yet another confusing
SSL property. Is there a proposed why for consumers to provide their own
trust manager and host verification process? If so, I assume that is yet
another property, can we merge those properties somehow?

At this point with all theses system properties can we come up with a
better way to configure SSL?

-Jake

Re: [Proposal] SSL - hostname verification

[Anthony Baker]

What if we require certs to have correct hostnames and automatically perform 
hostname verification.  No property required.  The only counter argument I can 
think of is that this change might require users to regenerate certificates 
when upgrading.  Perhaps we start by logging a warning for N releases, then 
make HN verification a hard requirement.

Anthony


> On Aug 14, 2018, at 8:59 AM, Jacob Barrett  wrote:
> 
> On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda 
> wrote:
> 
>> Geode currently does not implement hostname verification and is probably
>> not required per TLS spec. But it can be supported on TLS as an additional
>> check. The new proposed[1] implementation to use the default SSL context
>> could cause certain man-in-the-middle attacks possible if there is no
>> hostname verification.
> 
> 
> The current SSL implementation is also susceptible to man-in-the-middle as
> well. This proposal is really independent of those proposed changes.
> 
> 
>> This is a proposal to add a new boolean SSL property
>> `ssl-enable-endpoint-identification` which enables hostname verification
>> for secure connections.
> 
> 
> Are you proposing that we ship a custom trust manager that verifies hosts
> on all TLS connections? I would rather shy away from yet another confusing
> SSL property. Is there a proposed why for consumers to provide their own
> trust manager and host verification process? If so, I assume that is yet
> another property, can we merge those properties somehow?
> 
> At this point with all theses system properties can we come up with a
> better way to configure SSL?
> 
> -Jake

Re: [Proposal] SSL - hostname verification

[Michael Stolz]

I like this suggestion. +1

--
Mike Stolz
Principal Engineer, GemFire Product Lead
Mobile: +1-631-835-4771
Download the GemFire book here.


On Tue, Aug 14, 2018 at 1:00 PM, Anthony Baker  wrote:

> What if we require certs to have correct hostnames and automatically
> perform hostname verification.  No property required.  The only counter
> argument I can think of is that this change might require users to
> regenerate certificates when upgrading.  Perhaps we start by logging a
> warning for N releases, then make HN verification a hard requirement.
>
> Anthony
>
>
> > On Aug 14, 2018, at 8:59 AM, Jacob Barrett  wrote:
> >
> > On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda <
> sai_boorlaga...@apache.org>
> > wrote:
> >
> >> Geode currently does not implement hostname verification and is probably
> >> not required per TLS spec. But it can be supported on TLS as an
> additional
> >> check. The new proposed[1] implementation to use the default SSL context
> >> could cause certain man-in-the-middle attacks possible if there is no
> >> hostname verification.
> >
> >
> > The current SSL implementation is also susceptible to man-in-the-middle
> as
> > well. This proposal is really independent of those proposed changes.
> >
> >
> >> This is a proposal to add a new boolean SSL property
> >> `ssl-enable-endpoint-identification` which enables hostname
> verification
> >> for secure connections.
> >
> >
> > Are you proposing that we ship a custom trust manager that verifies hosts
> > on all TLS connections? I would rather shy away from yet another
> confusing
> > SSL property. Is there a proposed why for consumers to provide their own
> > trust manager and host verification process? If so, I assume that is yet
> > another property, can we merge those properties somehow?
> >
> > At this point with all theses system properties can we come up with a
> > better way to configure SSL?
> >
> > -Jake
>
>

Re: [Proposal] SSL - hostname verification

[Sai Boorlagadda]

>The current SSL implementation is also susceptible to man-in-the-middle as
>well. This proposal is really independent of those proposed changes.

This proposal is independent of the proposed changes to use default
context.
In both cases, the risk is similar if keys are compromised.
But in the proposed case there is a risk if DNS is compromised.
Also in the legacy case, we can at least recommend users to bring in trust
store with only specific keys/CAs.

> Are you proposing that we ship a custom trust manager that verifies hosts
on all TLS connections?

SSLParameters has a property 'setEndpointIdentificationAlgorithm[1]'
that can be set and it enabled hostname verification during an SSLSocket
handshake.

[1]
https://docs.oracle.com/javase/8/docs/api/javax/net/ssl/SSLParameters.html#setEndpointIdentificationAlgorithm-java.lang.String-

Sai
On Tue, Aug 14, 2018 at 8:59 AM Jacob Barrett  wrote:

> On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda <
> sai_boorlaga...@apache.org>
> wrote:
>
> > Geode currently does not implement hostname verification and is probably
> > not required per TLS spec. But it can be supported on TLS as an
> additional
> > check. The new proposed[1] implementation to use the default SSL context
> > could cause certain man-in-the-middle attacks possible if there is no
> > hostname verification.
>
>
> The current SSL implementation is also susceptible to man-in-the-middle as
> well. This proposal is really independent of those proposed changes.
>
>
> > This is a proposal to add a new boolean SSL property
> > `ssl-enable-endpoint-identification` which enables hostname verification
> > for secure connections.
>
>
> Are you proposing that we ship a custom trust manager that verifies hosts
> on all TLS connections? I would rather shy away from yet another confusing
> SSL property. Is there a proposed why for consumers to provide their own
> trust manager and host verification process? If so, I assume that is yet
> another property, can we merge those properties somehow?
>
> At this point with all theses system properties can we come up with a
> better way to configure SSL?
>
> -Jake
>

Re: [Proposal] SSL - hostname verification

[Sai Boorlagadda]

+1 for making it default and to warn users before making it a hard
requirement.

On Tue, Aug 14, 2018 at 10:00 AM Anthony Baker  wrote:

> What if we require certs to have correct hostnames and automatically
> perform hostname verification.  No property required.  The only counter
> argument I can think of is that this change might require users to
> regenerate certificates when upgrading.  Perhaps we start by logging a
> warning for N releases, then make HN verification a hard requirement.
>
> Anthony
>
>
> > On Aug 14, 2018, at 8:59 AM, Jacob Barrett  wrote:
> >
> > On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda <
> sai_boorlaga...@apache.org>
> > wrote:
> >
> >> Geode currently does not implement hostname verification and is probably
> >> not required per TLS spec. But it can be supported on TLS as an
> additional
> >> check. The new proposed[1] implementation to use the default SSL context
> >> could cause certain man-in-the-middle attacks possible if there is no
> >> hostname verification.
> >
> >
> > The current SSL implementation is also susceptible to man-in-the-middle
> as
> > well. This proposal is really independent of those proposed changes.
> >
> >
> >> This is a proposal to add a new boolean SSL property
> >> `ssl-enable-endpoint-identification` which enables hostname verification
> >> for secure connections.
> >
> >
> > Are you proposing that we ship a custom trust manager that verifies hosts
> > on all TLS connections? I would rather shy away from yet another
> confusing
> > SSL property. Is there a proposed why for consumers to provide their own
> > trust manager and host verification process? If so, I assume that is yet
> > another property, can we merge those properties somehow?
> >
> > At this point with all theses system properties can we come up with a
> > better way to configure SSL?
> >
> > -Jake
>
>

Re: [Proposal] SSL - hostname verification

[Dan Smith]

>
> The current SSL implementation is also susceptible to man-in-the-middle as
> well. This proposal is really independent of those proposed changes.
>

The current SSL implementation is not susceptible to man-in-the-middle
attacks, unless someone configures their client to trust public CAs rather
than directly trusting their gemfire servers. If you are using a public CA
model of trust, then you need hostname verification.

Sai's other proposal for ssl-use-default-provider would let the user
configure the system to use the JDK's default provider ... which trusts
public CAs.

I like Anthony's suggestion of always doing hostname verification. Until we
do, it seems like we will need this ssl-enable-endpoint-identification
property, so people can verify that their certificates will work in future
versions. If we are going to always do verification in the future maybe the
new ssl-use-default-provider property can do verification from the get-go?

-Dan



On Tue, Aug 14, 2018 at 8:59 AM, Jacob Barrett  wrote:

> On Tue, Aug 14, 2018 at 7:47 AM Sai Boorlagadda <
> sai_boorlaga...@apache.org>
> wrote:
>
> > Geode currently does not implement hostname verification and is probably
> > not required per TLS spec. But it can be supported on TLS as an
> additional
> > check. The new proposed[1] implementation to use the default SSL context
> > could cause certain man-in-the-middle attacks possible if there is no
> > hostname verification.
>
>
> The current SSL implementation is also susceptible to man-in-the-middle as
> well. This proposal is really independent of those proposed changes.
>
>
> > This is a proposal to add a new boolean SSL property
> > `ssl-enable-endpoint-identification` which enables hostname verification
> > for secure connections.
>
>
> Are you proposing that we ship a custom trust manager that verifies hosts
> on all TLS connections? I would rather shy away from yet another confusing
> SSL property. Is there a proposed why for consumers to provide their own
> trust manager and host verification process? If so, I assume that is yet
> another property, can we merge those properties somehow?
>
> At this point with all theses system properties can we come up with a
> better way to configure SSL?
>
> -Jake
>

Re: [DISCUSS] Streamline return value from RemoteQuery

[Anilkumar Gingade]

In Java, they are separated so that the results can be managed effectively.
For example StructSet has its own implementation to manage the query
results that are structs (more than one projection attributes).

-Anil



On Tue, Aug 14, 2018 at 8:28 AM David Kimura  wrote:

> I have a couple questions:
>
> Do you have an idea or theories of what was the original intent behind
> separating ResultSet and StructSet?
>
> Is execute a blocking or non-blocking call and does the interface have any
> guarantee of that?
>
> Thanks,
> David
>
> On Mon, Aug 13, 2018 at 4:06 PM, Ernest Burghardt 
> wrote:
>
> > Currently, geode-native's query::execute returns a
> > shared_ptr and
> > that pointer can be either ResultSet or StructSet.
> >
> >
> > RemoteQuery::execute contains logical code to determine with QueryResults
> > are greater than 0... We should look at removing this logic and only
> > returning StructSets
> > This allows removal of ResultSet which will streamline the API and
> > associated code...
> >
> > This duality is unnecessary and should be removed.
> > I am proposing that the results only return  StructSet
> >
> > Best,
> > EB
> >
>

Re: [DISCUSS] Streamline return value from RemoteQuery

[Dan Smith]

+1 for just having StructSet.

Return types should always be strongly typed, the user shouldn't have to
introspect or guess based on their query what the return type actually will
be at runtime.

If we think there is value in having separate ResultSet and StructSet
results, we could have a separate query::executeXXX method for each type.
But I think just returning StructSet seems reasonable.

The Java API for query is even worse. I think the Java API actually returns
an Object, which the user has to cast into one of three things - an
individual value, SelectResult of values, or a SelectResults of structs. We
should fix that too!

-Dan

On Tue, Aug 14, 2018 at 10:47 AM, Anilkumar Gingade 
wrote:

> In Java, they are separated so that the results can be managed effectively.
> For example StructSet has its own implementation to manage the query
> results that are structs (more than one projection attributes).
>
> -Anil
>
>
>
> On Tue, Aug 14, 2018 at 8:28 AM David Kimura  wrote:
>
> > I have a couple questions:
> >
> > Do you have an idea or theories of what was the original intent behind
> > separating ResultSet and StructSet?
> >
> > Is execute a blocking or non-blocking call and does the interface have
> any
> > guarantee of that?
> >
> > Thanks,
> > David
> >
> > On Mon, Aug 13, 2018 at 4:06 PM, Ernest Burghardt  >
> > wrote:
> >
> > > Currently, geode-native's query::execute returns a
> > > shared_ptr and
> > > that pointer can be either ResultSet or StructSet.
> > >
> > >
> > > RemoteQuery::execute contains logical code to determine with
> QueryResults
> > > are greater than 0... We should look at removing this logic and only
> > > returning StructSets
> > > This allows removal of ResultSet which will streamline the API and
> > > associated code...
> > >
> > > This duality is unnecessary and should be removed.
> > > I am proposing that the results only return  StructSet
> > >
> > > Best,
> > > EB
> > >
> >
>

Re: [Proposal] SSL - hostname verification

[Bruce Schuchardt]

Hi Sai

To which SSL connections would this apply?  All of them?


On 8/14/18 7:46 AM, Sai Boorlagadda wrote:

Geode currently does not implement hostname verification and is probably
not required per TLS spec. But it can be supported on TLS as an additional
check. The new proposed[1] implementation to use the default SSL context
could cause certain man-in-the-middle attacks possible if there is no
hostname verification. So in order to harden against such attacks, it puts
additional responsibilities on users to provide a custom TrustManager to
perform additional checks on server identity.

This is a proposal to add a new boolean SSL property
`ssl-enable-endpoint-identification` which enables hostname verification
for secure connections. For services exposed on HTTPS like pulse, dev-rest,
and admin-rest API, hostname validation is enabled by default and can be
skipped using `--skip-ssl-validation'. This new parameter especially
enables client applications to verify server's hostname using server
certificate during SSL handshake. The same parameter can also provide a
means to enable verification within distributed systems when a peer (acting
as an HTTPS client) accepting a connection from another peer (acting as
HTTPS server).

The proposed parameter can also be enabled when not using the default
context, in any case, we should update the documentation to clarify the
current behavior and a recommendation.

[1]
https://lists.apache.org/thread.html/b89beb6e57eb043786791c9309f13feca225a1c5119632f3fa7b7cb9@%3Cdev.geode.apache.org%3E

Sai

Re: [DISCUSS] Streamline return value from RemoteQuery

[Jacob Barrett]

If we are going to change it I wonder if we can’t do better or look for some 
other standard to adopt. 

For the C++ side could we adopt something that is maybe std::tuple or 
std::vector based? Tuple probably doesn’t work because it’s fixed at compile 
time but std::vector and StructSet are very similar in behavior.

-Jake


> On Aug 14, 2018, at 11:01 AM, Dan Smith  wrote:
> 
> +1 for just having StructSet.
> 
> Return types should always be strongly typed, the user shouldn't have to
> introspect or guess based on their query what the return type actually will
> be at runtime.
> 
> If we think there is value in having separate ResultSet and StructSet
> results, we could have a separate query::executeXXX method for each type.
> But I think just returning StructSet seems reasonable.
> 
> The Java API for query is even worse. I think the Java API actually returns
> an Object, which the user has to cast into one of three things - an
> individual value, SelectResult of values, or a SelectResults of structs. We
> should fix that too!
> 
> -Dan
> 
> On Tue, Aug 14, 2018 at 10:47 AM, Anilkumar Gingade 
> wrote:
> 
>> In Java, they are separated so that the results can be managed effectively.
>> For example StructSet has its own implementation to manage the query
>> results that are structs (more than one projection attributes).
>> 
>> -Anil
>> 
>> 
>> 
>>> On Tue, Aug 14, 2018 at 8:28 AM David Kimura  wrote:
>>> 
>>> I have a couple questions:
>>> 
>>> Do you have an idea or theories of what was the original intent behind
>>> separating ResultSet and StructSet?
>>> 
>>> Is execute a blocking or non-blocking call and does the interface have
>> any
>>> guarantee of that?
>>> 
>>> Thanks,
>>> David
>>> 
>>> On Mon, Aug 13, 2018 at 4:06 PM, Ernest Burghardt >> 
>>> wrote:
>>> 
 Currently, geode-native's query::execute returns a
 shared_ptr and
 that pointer can be either ResultSet or StructSet.
 
 
 RemoteQuery::execute contains logical code to determine with
>> QueryResults
 are greater than 0... We should look at removing this logic and only
 returning StructSets
 This allows removal of ResultSet which will streamline the API and
 associated code...
 
 This duality is unnecessary and should be removed.
 I am proposing that the results only return  StructSet
 
 Best,
 EB
 
>>> 
>> 

Re: [Proposal] SSL - hostname verification

[Sai Boorlagadda]

Bruce,

> To which SSL connections would this apply?  All of them?
When enabled, these validations affect any client connecting to ssl enabled
component depending on `ssl-enabled-components`.

Sai
On Tue, Aug 14, 2018 at 11:26 AM Bruce Schuchardt 
wrote:

> Hi Sai
>
> To which SSL connections would this apply?  All of them?
>
>
> On 8/14/18 7:46 AM, Sai Boorlagadda wrote:
> > Geode currently does not implement hostname verification and is probably
> > not required per TLS spec. But it can be supported on TLS as an
> additional
> > check. The new proposed[1] implementation to use the default SSL context
> > could cause certain man-in-the-middle attacks possible if there is no
> > hostname verification. So in order to harden against such attacks, it
> puts
> > additional responsibilities on users to provide a custom TrustManager to
> > perform additional checks on server identity.
> >
> > This is a proposal to add a new boolean SSL property
> > `ssl-enable-endpoint-identification` which enables hostname verification
> > for secure connections. For services exposed on HTTPS like pulse,
> dev-rest,
> > and admin-rest API, hostname validation is enabled by default and can be
> > skipped using `--skip-ssl-validation'. This new parameter especially
> > enables client applications to verify server's hostname using server
> > certificate during SSL handshake. The same parameter can also provide a
> > means to enable verification within distributed systems when a peer
> (acting
> > as an HTTPS client) accepting a connection from another peer (acting as
> > HTTPS server).
> >
> > The proposed parameter can also be enabled when not using the default
> > context, in any case, we should update the documentation to clarify the
> > current behavior and a recommendation.
> >
> > [1]
> >
> https://lists.apache.org/thread.html/b89beb6e57eb043786791c9309f13feca225a1c5119632f3fa7b7cb9@%3Cdev.geode.apache.org%3E
> >
> > Sai
> >
>
>

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1009 was SUCCESSFUL (with 2423 tests). Change made by John Blum.

[Spring CI]

---
Spring Data GemFire > Nightly-ApacheGeode > #1009 was successful.
---
Scheduled with changes by John Blum.
2425 tests in total.

https://build.spring.io/browse/SGF-NAG-1009/




--
Code Changes
--
John Blum (519afee64ad18317d4ad1f10fb0121a089280845):

>DATAGEODE-138 - Adapt to API changes in Spring Data Commons CDI implementation 
>detector.



--
This message is automatically generated by Atlassian Bamboo

Re: [DISCUSS] Streamline return value from RemoteQuery

[David Kimura]

If this is a non-blocking function (and maybe even if it isn't), then it
might be worth considering a future/promise contract.   Perhaps something
like folly which provides a very rich interface.

Thanks,
David

On Tue, Aug 14, 2018 at 12:57 PM, Jacob Barrett  wrote:

> If we are going to change it I wonder if we can’t do better or look for
> some other standard to adopt.
>
> For the C++ side could we adopt something that is maybe std::tuple or
> std::vector based? Tuple probably doesn’t work because it’s fixed at
> compile time but std::vector and StructSet are very similar in behavior.
>
> -Jake
>
>
> > On Aug 14, 2018, at 11:01 AM, Dan Smith  wrote:
> >
> > +1 for just having StructSet.
> >
> > Return types should always be strongly typed, the user shouldn't have to
> > introspect or guess based on their query what the return type actually
> will
> > be at runtime.
> >
> > If we think there is value in having separate ResultSet and StructSet
> > results, we could have a separate query::executeXXX method for each type.
> > But I think just returning StructSet seems reasonable.
> >
> > The Java API for query is even worse. I think the Java API actually
> returns
> > an Object, which the user has to cast into one of three things - an
> > individual value, SelectResult of values, or a SelectResults of structs.
> We
> > should fix that too!
> >
> > -Dan
> >
> > On Tue, Aug 14, 2018 at 10:47 AM, Anilkumar Gingade  >
> > wrote:
> >
> >> In Java, they are separated so that the results can be managed
> effectively.
> >> For example StructSet has its own implementation to manage the query
> >> results that are structs (more than one projection attributes).
> >>
> >> -Anil
> >>
> >>
> >>
> >>> On Tue, Aug 14, 2018 at 8:28 AM David Kimura 
> wrote:
> >>>
> >>> I have a couple questions:
> >>>
> >>> Do you have an idea or theories of what was the original intent behind
> >>> separating ResultSet and StructSet?
> >>>
> >>> Is execute a blocking or non-blocking call and does the interface have
> >> any
> >>> guarantee of that?
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>> On Mon, Aug 13, 2018 at 4:06 PM, Ernest Burghardt <
> eburgha...@pivotal.io
> >>>
> >>> wrote:
> >>>
>  Currently, geode-native's query::execute returns a
>  shared_ptr and
>  that pointer can be either ResultSet or StructSet.
> 
> 
>  RemoteQuery::execute contains logical code to determine with
> >> QueryResults
>  are greater than 0... We should look at removing this logic and only
>  returning StructSets
>  This allows removal of ResultSet which will streamline the API and
>  associated code...
> 
>  This duality is unnecessary and should be removed.
>  I am proposing that the results only return  StructSet
> 
>  Best,
>  EB
> 
> >>>
> >>
>