Re: [Proposal] SSL - hostname verification

2018-08-20 Thread Sai Boorlagadda
We are using the default algorithm provided by JDK and the proposed
parameter is made boolean as there is no foresee for using different
algorithms. I have the PR#2346[1] for review.

[1] https://github.com/apache/geode/pull/2346

On Fri, Aug 17, 2018, 8:50 AM Jacob Barrett  wrote:

> Are we implementing the identification algorithm ourself or using one
> provided by setting that algorithm property? If we are implementing our own
> or just setting the algorithm to HTTPS and don’t foresee us using the LDAPS
> then a Boolean is sufficient. There isn’t really a whole lot different
> between the two algorithms by scanning the RFCs. The HTTPS algorithm is a
> bit more defined and covers IP addresses. The LDAPS has language explicitly
> denying DNS resolution in the process. I can’t imagine why we would use the
> LDAPS algorithm.
>
>
> > On Aug 17, 2018, at 8:11 AM, Sai Boorlagadda 
> wrote:
> >
> > Thanks, Jake for the feedback.
> >
> > The reason for endpoint-identification in the parameter name is to be
> > aligned with JDK. So maybe making it as
> > 'ssl-endpoint-identification-enabled' would have been less confusing.
> > I also want to bring to notice that in JDK its a string property in
> > SSLParameters.setEndpointIdentificationAlgorithm(String algorithm) -
> which
> > I believe takes HTTPS or LDAPS.
> >
> > Not sure if Geode has plans to support LDAPS, in which case I would
> rather
> > change the proposal to have a string parameter.
> >
> > But for now I like your suggestion about making it read like "is
> > ssl endpoint identification enabled?"
> >
> > Sai
> >
> >> On Fri, Aug 17, 2018 at 6:38 AM Jacob Barrett 
> wrote:
> >>
> >> My biggest concern now is the name of the property to enable this.
> >> 'ssl-enabled-endpoint-identification' in no way suggests any kind of
> >> validation. The word identification implies to me that it will identify
> and
> >> maybe log endpoints. I would change that to validation or some other
> term
> >> that implies the actual action the system intends to make when enabled.
> >>
> >> Secondly I find the order of the words confusing. Is it ssl that is
> being
> >> enabled or endpoint identification?
> >>
> >> I would suggest ‘ssl-hostname-validation-enabled’ or some variant of
> that.
> >> First off this reads like a natural English sentence, “is ssl host name
> >> validation enabled?” Secondly the action “host validation” implies an
> >> assertion is being made about the validity of the hosts name.
> >>
> >> -Jake
> >>
> >>
> >>> On 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
> >>>
> >>
>


[DISCUSS] Which assert is the right one to use moving forward?

2018-08-20 Thread Mark Hanson
Hi All,

In the course fo fixing some tests, I have found that the existing Geode 
asserts are deprecated. In wanting to leave the code as clean of deprecations 
as possible, I have come to the inevitable quandary.  Which Assert should we be 
using? JUnit or Assertj? I am happy with either though I will note that JUnit 
Assert does not seem to have a fail where you can pass in the exception, which 
is used a lot in Geode. I look forward to an answer.

Thanks,
Mark




Re: [DISCUSS] Which assert is the right one to use moving forward?

2018-08-20 Thread Dale Emery
Between JUnit and AssertJ, my preference is AssertJ, for two reasons. First is 
AssertJ’s pervasiveness in Geode. Second, it decouples assertions from the 
testing framework (in support of my secret desire to move to JUnit 5).

Cheers,
Dale

> On Aug 20, 2018, at 11:49 AM, Mark Hanson  wrote:
> 
> Hi All,
> 
> In the course fo fixing some tests, I have found that the existing Geode 
> asserts are deprecated. In wanting to leave the code as clean of deprecations 
> as possible, I have come to the inevitable quandary.  Which Assert should we 
> be using? JUnit or Assertj? I am happy with either though I will note that 
> JUnit Assert does not seem to have a fail where you can pass in the 
> exception, which is used a lot in Geode. I look forward to an answer.
> 
> Thanks,
> Mark
> 
> 

—
Dale Emery
dem...@pivotal.io



Re: [DISCUSS] Which assert is the right one to use moving forward?

2018-08-20 Thread Patrick Rhomberg
I greatly favor AssertJ-core's `assertThat` and `assertThatThrownBy`.
Decoupling is nice, but the reported failure information that AssertJ
includes makes investigating failures initially much easier.  That, and
method-chaining assertions and (rarely) soft-assertions are useful features.

On Mon, Aug 20, 2018 at 12:32 PM, Dale Emery  wrote:

> Between JUnit and AssertJ, my preference is AssertJ, for two reasons.
> First is AssertJ’s pervasiveness in Geode. Second, it decouples assertions
> from the testing framework (in support of my secret desire to move to JUnit
> 5).
>
> Cheers,
> Dale
>
> > On Aug 20, 2018, at 11:49 AM, Mark Hanson  wrote:
> >
> > Hi All,
> >
> > In the course fo fixing some tests, I have found that the existing Geode
> asserts are deprecated. In wanting to leave the code as clean of
> deprecations as possible, I have come to the inevitable quandary.  Which
> Assert should we be using? JUnit or Assertj? I am happy with either though
> I will note that JUnit Assert does not seem to have a fail where you can
> pass in the exception, which is used a lot in Geode. I look forward to an
> answer.
> >
> > Thanks,
> > Mark
> >
> >
>
> —
> Dale Emery
> dem...@pivotal.io
>
>


[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #1015 was SUCCESSFUL (with 2423 tests). Change made by Oliver Gierke.

2018-08-20 Thread Spring CI

---
Spring Data GemFire > Nightly-ApacheGeode > #1015 was successful.
---
Scheduled with changes by Oliver Gierke.
2425 tests in total.

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




--
Code Changes
--
Oliver Gierke (0bd472a968853fe15e5e7760193fb8b6979b46f5):

>DATAGEODE-133 - Prepare next development iteration.

Oliver Gierke (2742d806f24fc8415fc23d1e54d450a9123560c3):

>DATAGEODE-133 - After release cleanups.

Oliver Gierke (7a9503670cb1203f1f5269fb444d89e8b08f9a5c):

>DATAGEODE-133 - Release version 2.1 RC2 (Lovelace).



--
This message is automatically generated by Atlassian Bamboo

Re: [DISCUSS] Which assert is the right one to use moving forward?

2018-08-20 Thread Kirk Lund
Use AssertJ.

If you find test code that's catching Exception and invoking fail with or
without the caught Exception, convert that to something better. Using fail
is itself an epic fail.

1) Tests should never catch unexpected Exceptions

Remove "catch (Exception" and let the test methods throw the exception.
JUnit's failure messages for uncaught Exceptions is way better than using
fail.

2) Use ErrorCollector rules in callback code such as CacheListeners

@Rule
public ErrorCollector errorCollector = new ErrorCollector();

...
} catch (Exception e) {
  errorCollector.addError(e);
}

On Mon, Aug 20, 2018 at 12:47 PM, Patrick Rhomberg 
wrote:

> I greatly favor AssertJ-core's `assertThat` and `assertThatThrownBy`.
> Decoupling is nice, but the reported failure information that AssertJ
> includes makes investigating failures initially much easier.  That, and
> method-chaining assertions and (rarely) soft-assertions are useful
> features.
>
> On Mon, Aug 20, 2018 at 12:32 PM, Dale Emery  wrote:
>
> > Between JUnit and AssertJ, my preference is AssertJ, for two reasons.
> > First is AssertJ’s pervasiveness in Geode. Second, it decouples
> assertions
> > from the testing framework (in support of my secret desire to move to
> JUnit
> > 5).
> >
> > Cheers,
> > Dale
> >
> > > On Aug 20, 2018, at 11:49 AM, Mark Hanson  wrote:
> > >
> > > Hi All,
> > >
> > > In the course fo fixing some tests, I have found that the existing
> Geode
> > asserts are deprecated. In wanting to leave the code as clean of
> > deprecations as possible, I have come to the inevitable quandary.  Which
> > Assert should we be using? JUnit or Assertj? I am happy with either
> though
> > I will note that JUnit Assert does not seem to have a fail where you can
> > pass in the exception, which is used a lot in Geode. I look forward to an
> > answer.
> > >
> > > Thanks,
> > > Mark
> > >
> > >
> >
> > —
> > Dale Emery
> > dem...@pivotal.io
> >
> >
>