Re: SecurityService versus Cluster Config

[Swapnil Bawaskar]

I think we should revisit connecting to the distributed system with
properties. In addition to what Kirk mentioned below, I think one of the
big limitation of this is the inability to change properties on a running
cluster. If there is a property that a user needs to change, they will have
to bounce the cluster as opposed to performing a rolling restart of the
cluster with that property changed. I think the quickest way for us to get
there is to make the connection a two step process, connect just to get the
properties from the cluster config avoiding any validation, then reconnect
using the new set of properties.

On Fri, Jun 9, 2017 at 2:54 PM Kirk Lund  wrote:

> The usage of CacheFactory#setSecurityManager (and setPostProcessor) is
> working as expected, both before and after my changes!
>
> The problem is that Cluster Config is requested during Cache initialization
> which means that any gemfire.properties stored in Cluster Config will not
> be used by DistributedSystem -- only the properties that affect the Cache
> (such as cache-xml-file) are used as expected in Cluster Config. This
> problem exists with/without my changes but my changes exposed this fact by
> moving the use of the security-manager property to DistributedSystem. I'm
> working on a short-term fix specific to security-manager. Correcting the
> problem for all gemfire.properties will be a long-term fix.
>
> On Fri, Jun 9, 2017 at 1:48 PM, John Blum  wrote:
>
> > I am not sure I follow everything that is happening here quite yet as I
> > have not had time to go over this in more detail and digest it.
> >
> > But, I certainly hope that the security-manager property in Geode is not
> > impacted by this in anyway since *Spring Data Geode* builds on this and,
> > well, this
> >  > src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368>
> > [1],
> > as way to configure Geode (Integrated) Security.
> >
> > Thanks,
> > John
> >
> > [1]
> > https://github.com/apache/geode/blob/develop/geode-core/
> > src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368
> >
> >
> > On Fri, Jun 9, 2017 at 1:11 PM, Kirk Lund  wrote:
> >
> > > Yeah I think we need #2 in the long run. Right now nearly all of the
> > > gemfire.properties are ignored if you use Cluster Config. The few
> > > gemfire.properties that are mutable by RuntimeDistributionConfigImpl
> will
> > > work when used in Cluster Config -- I believe all other
> > gemfire.properties
> > > would be ignored until we complete #2.
> > >
> > > On Fri, Jun 9, 2017 at 1:02 PM, Udo Kohlmeyer 
> > > wrote:
> > >
> > > > +1 for #2. It does seem more correct
> > > >
> > > >
> > > >
> > > > On 6/9/17 12:45, Kirk Lund wrote:
> > > >
> > > >> The new changes for SecurityService don't work with Cluster Config.
> We
> > > >> only
> > > >> had one test that would've failed but it was annotated with @Ignore.
> > > >>
> > > >> The problem is this:
> > > >>
> > > >> The security-manager is a gemfire property and is now used by the
> > > >> InternalDistributedSystem because membership needs SecurityService
> to
> > > >> handle peer-to-peer authentication. So with my changes, the
> > > >> InternalDistributedSystem now creates SecurityService and passes it
> > into
> > > >> the constructor of GemFireCacheImpl.
> > > >>
> > > >> Next, GemFireCacheImpl#initialize requests Cluster Config. If this
> is
> > a
> > > >> new
> > > >> Server with no gemfire properties, it will have already joined the
> > > Cluster
> > > >> (before and after my changes). It then gets Cluster Config that has
> > > >> gemfire.properties including security-manager. But it's too late,
> the
> > > >> immutable SecurityService has already been created by
> > > >> InternalDistributedSystem.
> > > >>
> > > >> In theory, Cluster Config should be requested before the creation of
> > > >> InternalDistributedSystem so that other gemfire.properties in
> Cluster
> > > >> Config can be applied (such as member-timeout). In fact most of the
> > > >> gemfire.properties control aspects of InternalDistributedSystem.
> > > >> Presently,
> > > >> all gemfire.properties that would configure
> InternalDistributedSystem
> > > are
> > > >> ignored because Cluster Config is loaded after
> > InternalDistributedSystem
> > > >> was created and the member has joined the cluster.
> > > >>
> > > >> We would need to make one of these changes:
> > > >>
> > > >> 1) GemFireCacheImpl creates its own SecurityService which is
> different
> > > >> from
> > > >> the one used by membership. Locators are the only members that
> really
> > > need
> > > >> membership to have a fully configured SecurityService (and I think
> we
> > > >> already have a bug in which 2nd Locators need to be manually
> > configured
> > > >> rather than use Cluster Config).
> > > >>
> > > >> 2) Move Cluster Config request from GemFireCacheImpl to
> > > >> 

Re: SecurityService versus Cluster Config

[Kirk Lund]

The usage of CacheFactory#setSecurityManager (and setPostProcessor) is
working as expected, both before and after my changes!

The problem is that Cluster Config is requested during Cache initialization
which means that any gemfire.properties stored in Cluster Config will not
be used by DistributedSystem -- only the properties that affect the Cache
(such as cache-xml-file) are used as expected in Cluster Config. This
problem exists with/without my changes but my changes exposed this fact by
moving the use of the security-manager property to DistributedSystem. I'm
working on a short-term fix specific to security-manager. Correcting the
problem for all gemfire.properties will be a long-term fix.

On Fri, Jun 9, 2017 at 1:48 PM, John Blum  wrote:

> I am not sure I follow everything that is happening here quite yet as I
> have not had time to go over this in more detail and digest it.
>
> But, I certainly hope that the security-manager property in Geode is not
> impacted by this in anyway since *Spring Data Geode* builds on this and,
> well, this
>  src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368>
> [1],
> as way to configure Geode (Integrated) Security.
>
> Thanks,
> John
>
> [1]
> https://github.com/apache/geode/blob/develop/geode-core/
> src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368
>
>
> On Fri, Jun 9, 2017 at 1:11 PM, Kirk Lund  wrote:
>
> > Yeah I think we need #2 in the long run. Right now nearly all of the
> > gemfire.properties are ignored if you use Cluster Config. The few
> > gemfire.properties that are mutable by RuntimeDistributionConfigImpl will
> > work when used in Cluster Config -- I believe all other
> gemfire.properties
> > would be ignored until we complete #2.
> >
> > On Fri, Jun 9, 2017 at 1:02 PM, Udo Kohlmeyer 
> > wrote:
> >
> > > +1 for #2. It does seem more correct
> > >
> > >
> > >
> > > On 6/9/17 12:45, Kirk Lund wrote:
> > >
> > >> The new changes for SecurityService don't work with Cluster Config. We
> > >> only
> > >> had one test that would've failed but it was annotated with @Ignore.
> > >>
> > >> The problem is this:
> > >>
> > >> The security-manager is a gemfire property and is now used by the
> > >> InternalDistributedSystem because membership needs SecurityService to
> > >> handle peer-to-peer authentication. So with my changes, the
> > >> InternalDistributedSystem now creates SecurityService and passes it
> into
> > >> the constructor of GemFireCacheImpl.
> > >>
> > >> Next, GemFireCacheImpl#initialize requests Cluster Config. If this is
> a
> > >> new
> > >> Server with no gemfire properties, it will have already joined the
> > Cluster
> > >> (before and after my changes). It then gets Cluster Config that has
> > >> gemfire.properties including security-manager. But it's too late, the
> > >> immutable SecurityService has already been created by
> > >> InternalDistributedSystem.
> > >>
> > >> In theory, Cluster Config should be requested before the creation of
> > >> InternalDistributedSystem so that other gemfire.properties in Cluster
> > >> Config can be applied (such as member-timeout). In fact most of the
> > >> gemfire.properties control aspects of InternalDistributedSystem.
> > >> Presently,
> > >> all gemfire.properties that would configure InternalDistributedSystem
> > are
> > >> ignored because Cluster Config is loaded after
> InternalDistributedSystem
> > >> was created and the member has joined the cluster.
> > >>
> > >> We would need to make one of these changes:
> > >>
> > >> 1) GemFireCacheImpl creates its own SecurityService which is different
> > >> from
> > >> the one used by membership. Locators are the only members that really
> > need
> > >> membership to have a fully configured SecurityService (and I think we
> > >> already have a bug in which 2nd Locators need to be manually
> configured
> > >> rather than use Cluster Config).
> > >>
> > >> 2) Move Cluster Config request from GemFireCacheImpl to
> > >> InternalDistributedSystem. This is actually more correct, since
> Cluster
> > >> Config is potentially going to contain a lot of
> > InternalDistributedSystem
> > >> configuration options that are currently ignored (in both Geode 1.x
> and
> > >> current develop).
> > >>
> > >> Unfortunately #2 is a fairly big refactoring change.
> > >>
> > >> We should probably revert my changes from develop and 1.2 until #1 or
> #2
> > >> can be completed. Or we have to file a bug that says Cluster Config
> and
> > >> Security don't play nice together until we can complete one of these
> > >> options. I think the latter is probably not acceptable.
> > >>
> > >>
> > >
> >
>
>
>
> --
> -John
> john.blum10101 (skype)
>

Re: SecurityService versus Cluster Config

[John Blum]

I am not sure I follow everything that is happening here quite yet as I
have not had time to go over this in more detail and digest it.

But, I certainly hope that the security-manager property in Geode is not
impacted by this in anyway since *Spring Data Geode* builds on this and,
well, this

[1],
as way to configure Geode (Integrated) Security.

Thanks,
John

[1]
https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/cache/CacheFactory.java#L355-L368


On Fri, Jun 9, 2017 at 1:11 PM, Kirk Lund  wrote:

> Yeah I think we need #2 in the long run. Right now nearly all of the
> gemfire.properties are ignored if you use Cluster Config. The few
> gemfire.properties that are mutable by RuntimeDistributionConfigImpl will
> work when used in Cluster Config -- I believe all other gemfire.properties
> would be ignored until we complete #2.
>
> On Fri, Jun 9, 2017 at 1:02 PM, Udo Kohlmeyer 
> wrote:
>
> > +1 for #2. It does seem more correct
> >
> >
> >
> > On 6/9/17 12:45, Kirk Lund wrote:
> >
> >> The new changes for SecurityService don't work with Cluster Config. We
> >> only
> >> had one test that would've failed but it was annotated with @Ignore.
> >>
> >> The problem is this:
> >>
> >> The security-manager is a gemfire property and is now used by the
> >> InternalDistributedSystem because membership needs SecurityService to
> >> handle peer-to-peer authentication. So with my changes, the
> >> InternalDistributedSystem now creates SecurityService and passes it into
> >> the constructor of GemFireCacheImpl.
> >>
> >> Next, GemFireCacheImpl#initialize requests Cluster Config. If this is a
> >> new
> >> Server with no gemfire properties, it will have already joined the
> Cluster
> >> (before and after my changes). It then gets Cluster Config that has
> >> gemfire.properties including security-manager. But it's too late, the
> >> immutable SecurityService has already been created by
> >> InternalDistributedSystem.
> >>
> >> In theory, Cluster Config should be requested before the creation of
> >> InternalDistributedSystem so that other gemfire.properties in Cluster
> >> Config can be applied (such as member-timeout). In fact most of the
> >> gemfire.properties control aspects of InternalDistributedSystem.
> >> Presently,
> >> all gemfire.properties that would configure InternalDistributedSystem
> are
> >> ignored because Cluster Config is loaded after InternalDistributedSystem
> >> was created and the member has joined the cluster.
> >>
> >> We would need to make one of these changes:
> >>
> >> 1) GemFireCacheImpl creates its own SecurityService which is different
> >> from
> >> the one used by membership. Locators are the only members that really
> need
> >> membership to have a fully configured SecurityService (and I think we
> >> already have a bug in which 2nd Locators need to be manually configured
> >> rather than use Cluster Config).
> >>
> >> 2) Move Cluster Config request from GemFireCacheImpl to
> >> InternalDistributedSystem. This is actually more correct, since Cluster
> >> Config is potentially going to contain a lot of
> InternalDistributedSystem
> >> configuration options that are currently ignored (in both Geode 1.x and
> >> current develop).
> >>
> >> Unfortunately #2 is a fairly big refactoring change.
> >>
> >> We should probably revert my changes from develop and 1.2 until #1 or #2
> >> can be completed. Or we have to file a bug that says Cluster Config and
> >> Security don't play nice together until we can complete one of these
> >> options. I think the latter is probably not acceptable.
> >>
> >>
> >
>



-- 
-John
john.blum10101 (skype)

Re: SecurityService versus Cluster Config

[Kirk Lund]

Yeah I think we need #2 in the long run. Right now nearly all of the
gemfire.properties are ignored if you use Cluster Config. The few
gemfire.properties that are mutable by RuntimeDistributionConfigImpl will
work when used in Cluster Config -- I believe all other gemfire.properties
would be ignored until we complete #2.

On Fri, Jun 9, 2017 at 1:02 PM, Udo Kohlmeyer  wrote:

> +1 for #2. It does seem more correct
>
>
>
> On 6/9/17 12:45, Kirk Lund wrote:
>
>> The new changes for SecurityService don't work with Cluster Config. We
>> only
>> had one test that would've failed but it was annotated with @Ignore.
>>
>> The problem is this:
>>
>> The security-manager is a gemfire property and is now used by the
>> InternalDistributedSystem because membership needs SecurityService to
>> handle peer-to-peer authentication. So with my changes, the
>> InternalDistributedSystem now creates SecurityService and passes it into
>> the constructor of GemFireCacheImpl.
>>
>> Next, GemFireCacheImpl#initialize requests Cluster Config. If this is a
>> new
>> Server with no gemfire properties, it will have already joined the Cluster
>> (before and after my changes). It then gets Cluster Config that has
>> gemfire.properties including security-manager. But it's too late, the
>> immutable SecurityService has already been created by
>> InternalDistributedSystem.
>>
>> In theory, Cluster Config should be requested before the creation of
>> InternalDistributedSystem so that other gemfire.properties in Cluster
>> Config can be applied (such as member-timeout). In fact most of the
>> gemfire.properties control aspects of InternalDistributedSystem.
>> Presently,
>> all gemfire.properties that would configure InternalDistributedSystem are
>> ignored because Cluster Config is loaded after InternalDistributedSystem
>> was created and the member has joined the cluster.
>>
>> We would need to make one of these changes:
>>
>> 1) GemFireCacheImpl creates its own SecurityService which is different
>> from
>> the one used by membership. Locators are the only members that really need
>> membership to have a fully configured SecurityService (and I think we
>> already have a bug in which 2nd Locators need to be manually configured
>> rather than use Cluster Config).
>>
>> 2) Move Cluster Config request from GemFireCacheImpl to
>> InternalDistributedSystem. This is actually more correct, since Cluster
>> Config is potentially going to contain a lot of InternalDistributedSystem
>> configuration options that are currently ignored (in both Geode 1.x and
>> current develop).
>>
>> Unfortunately #2 is a fairly big refactoring change.
>>
>> We should probably revert my changes from develop and 1.2 until #1 or #2
>> can be completed. Or we have to file a bug that says Cluster Config and
>> Security don't play nice together until we can complete one of these
>> options. I think the latter is probably not acceptable.
>>
>>
>

Re: SecurityService versus Cluster Config

[Kirk Lund]

I've reverted my change on release/1.2.0.

I'm currently planning to work on #1 on develop in the short term.

On Fri, Jun 9, 2017 at 12:45 PM, Kirk Lund  wrote:

> The new changes for SecurityService don't work with Cluster Config. We
> only had one test that would've failed but it was annotated with @Ignore.
>
> The problem is this:
>
> The security-manager is a gemfire property and is now used by the
> InternalDistributedSystem because membership needs SecurityService to
> handle peer-to-peer authentication. So with my changes, the
> InternalDistributedSystem now creates SecurityService and passes it into
> the constructor of GemFireCacheImpl.
>
> Next, GemFireCacheImpl#initialize requests Cluster Config. If this is a
> new Server with no gemfire properties, it will have already joined the
> Cluster (before and after my changes). It then gets Cluster Config that has
> gemfire.properties including security-manager. But it's too late, the
> immutable SecurityService has already been created by
> InternalDistributedSystem.
>
> In theory, Cluster Config should be requested before the creation of
> InternalDistributedSystem so that other gemfire.properties in Cluster
> Config can be applied (such as member-timeout). In fact most of the
> gemfire.properties control aspects of InternalDistributedSystem. Presently,
> all gemfire.properties that would configure InternalDistributedSystem are
> ignored because Cluster Config is loaded after InternalDistributedSystem
> was created and the member has joined the cluster.
>
> We would need to make one of these changes:
>
> 1) GemFireCacheImpl creates its own SecurityService which is different
> from the one used by membership. Locators are the only members that really
> need membership to have a fully configured SecurityService (and I think we
> already have a bug in which 2nd Locators need to be manually configured
> rather than use Cluster Config).
>
> 2) Move Cluster Config request from GemFireCacheImpl to
> InternalDistributedSystem. This is actually more correct, since Cluster
> Config is potentially going to contain a lot of InternalDistributedSystem
> configuration options that are currently ignored (in both Geode 1.x and
> current develop).
>
> Unfortunately #2 is a fairly big refactoring change.
>
> We should probably revert my changes from develop and 1.2 until #1 or #2
> can be completed. Or we have to file a bug that says Cluster Config and
> Security don't play nice together until we can complete one of these
> options. I think the latter is probably not acceptable.
>
>

Re: SecurityService versus Cluster Config

[Udo Kohlmeyer]

+1 for #2. It does seem more correct


On 6/9/17 12:45, Kirk Lund wrote:

The new changes for SecurityService don't work with Cluster Config. We only
had one test that would've failed but it was annotated with @Ignore.

The problem is this:

The security-manager is a gemfire property and is now used by the
InternalDistributedSystem because membership needs SecurityService to
handle peer-to-peer authentication. So with my changes, the
InternalDistributedSystem now creates SecurityService and passes it into
the constructor of GemFireCacheImpl.

Next, GemFireCacheImpl#initialize requests Cluster Config. If this is a new
Server with no gemfire properties, it will have already joined the Cluster
(before and after my changes). It then gets Cluster Config that has
gemfire.properties including security-manager. But it's too late, the
immutable SecurityService has already been created by
InternalDistributedSystem.

In theory, Cluster Config should be requested before the creation of
InternalDistributedSystem so that other gemfire.properties in Cluster
Config can be applied (such as member-timeout). In fact most of the
gemfire.properties control aspects of InternalDistributedSystem. Presently,
all gemfire.properties that would configure InternalDistributedSystem are
ignored because Cluster Config is loaded after InternalDistributedSystem
was created and the member has joined the cluster.

We would need to make one of these changes:

1) GemFireCacheImpl creates its own SecurityService which is different from
the one used by membership. Locators are the only members that really need
membership to have a fully configured SecurityService (and I think we
already have a bug in which 2nd Locators need to be manually configured
rather than use Cluster Config).

2) Move Cluster Config request from GemFireCacheImpl to
InternalDistributedSystem. This is actually more correct, since Cluster
Config is potentially going to contain a lot of InternalDistributedSystem
configuration options that are currently ignored (in both Geode 1.x and
current develop).

Unfortunately #2 is a fairly big refactoring change.

We should probably revert my changes from develop and 1.2 until #1 or #2
can be completed. Or we have to file a bug that says Cluster Config and
Security don't play nice together until we can complete one of these
options. I think the latter is probably not acceptable.