Improvements for new security approach.

[Maksim Stepachev]

Hello, Igniters.

    The main idea of the new security is propagation security context to
other nodes and does action with initial permission. The solution looks
fine but has imperfections.

1. ZookeaperDiscoveryImpl doesn't implement security into itself.
  As a result: Caused by: class org.apache.ignite.spi.IgniteSpiException:
Security context isn't certain.
2. The visor tasks lost permission.
The method VisorQueryUtils#scheduleQueryStart makes a new thread and loses
context.
3. The GridRestProcessor does tasks outside "withContext" section.  As
result context loses.
4. The GridRestProcessor isn't client, we can't read security subject from
node attribute.
We should transmit secCtx for fake nodes and secSubjId for real.
5. NoOpIgniteSecurityProcessor should include a disabled processor and
validate it too if it is not null. It is important for a client node.
For example:
Into IgniteKernal#securityProcessor method createComponent return a
GridSecurityProcessor. For server nodes are enabled, but for clients
aren't.  The clients aren't able to pass validation for this reason.

6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.

I going to fix it.

[Ivan Rakov]

Hello Max,

Thanks for your analysis!

Have you created a JIRA issue for discovered defects?

Best Regards,
Ivan Rakov

On 17.07.2019 17:08, Maksim Stepachev wrote:

> Hello, Igniters.
>
>      The main idea of the new security is propagation security context to
> other nodes and does action with initial permission. The solution looks
> fine but has imperfections.
>
> 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
>    As a result: Caused by: class org.apache.ignite.spi.IgniteSpiException:
> Security context isn't certain.
> 2. The visor tasks lost permission.
> The method VisorQueryUtils#scheduleQueryStart makes a new thread and loses
> context.
> 3. The GridRestProcessor does tasks outside "withContext" section.  As
> result context loses.
> 4. The GridRestProcessor isn't client, we can't read security subject from
> node attribute.
> We should transmit secCtx for fake nodes and secSubjId for real.
> 5. NoOpIgniteSecurityProcessor should include a disabled processor and
> validate it too if it is not null. It is important for a client node.
> For example:
> Into IgniteKernal#securityProcessor method createComponent return a
> GridSecurityProcessor. For server nodes are enabled, but for clients
> aren't.  The clients aren't able to pass validation for this reason.
>
> 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>
> I going to fix it.
>

[Maksim Stepachev]

Hi, Ivan.

Yes, I have.
https://issues.apache.org/jira/browse/IGNITE-11992

I'm waiting for a visa.


чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:

> Hello Max,
>
> Thanks for your analysis!
>
> Have you created a JIRA issue for discovered defects?
>
> Best Regards,
> Ivan Rakov
>
> On 17.07.2019 17:08, Maksim Stepachev wrote:
> > Hello, Igniters.
> >
> >      The main idea of the new security is propagation security context to
> > other nodes and does action with initial permission. The solution looks
> > fine but has imperfections.
> >
> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
> >    As a result: Caused by: class
> org.apache.ignite.spi.IgniteSpiException:
> > Security context isn't certain.
> > 2. The visor tasks lost permission.
> > The method VisorQueryUtils#scheduleQueryStart makes a new thread and
> loses
> > context.
> > 3. The GridRestProcessor does tasks outside "withContext" section.  As
> > result context loses.
> > 4. The GridRestProcessor isn't client, we can't read security subject
> from
> > node attribute.
> > We should transmit secCtx for fake nodes and secSubjId for real.
> > 5. NoOpIgniteSecurityProcessor should include a disabled processor and
> > validate it too if it is not null. It is important for a client node.
> > For example:
> > Into IgniteKernal#securityProcessor method createComponent return a
> > GridSecurityProcessor. For server nodes are enabled, but for clients
> > aren't.  The clients aren't able to pass validation for this reason.
> >
> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
> >
> > I going to fix it.
> >
>

[Denis Garus]

Hello, Maksim!
Thanks for your analysis!

I have a few questions about your proposals.

GridRestProcessor.
AFAIK, when GridRestProcessor handle client request
(GridRestProcessor#handleRequest)
it process authentication (GridRestProcessor#authenticate)
and then authorization of request (GridRestProcessor#authorize) inside
client context.
Can you give additional info about issues with GridRestProcessor from 3 and
4? Maybe you have a reproducer for the problem?

NoOpIgniteSecurityProcessor.
I think the case that you describe in 5 is not a bug.
All nodes (client and server) must have security enabled or disabled.
I can't imagine the case when it is not.

ATTR_SECURITY_SUBJECT.
I don't think that compatibility is needed here. If you will use node with
propagation security context to remote node and older nodes
you can get subtle errors.

чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <[hidden email]>:

> Hi, Ivan.
>
> Yes, I have.
> https://issues.apache.org/jira/browse/IGNITE-11992
>
> I'm waiting for a visa.
>
>
> чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
>
>> Hello Max,
>>
>> Thanks for your analysis!
>>
>> Have you created a JIRA issue for discovered defects?
>>
>> Best Regards,
>> Ivan Rakov
>>
>> On 17.07.2019 17:08, Maksim Stepachev wrote:
>> > Hello, Igniters.
>> >
>> >      The main idea of the new security is propagation security context
>> to
>> > other nodes and does action with initial permission. The solution looks
>> > fine but has imperfections.
>> >
>> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
>> >    As a result: Caused by: class
>> org.apache.ignite.spi.IgniteSpiException:
>> > Security context isn't certain.
>> > 2. The visor tasks lost permission.
>> > The method VisorQueryUtils#scheduleQueryStart makes a new thread and
>> loses
>> > context.
>> > 3. The GridRestProcessor does tasks outside "withContext" section.  As
>> > result context loses.
>> > 4. The GridRestProcessor isn't client, we can't read security subject
>> from
>> > node attribute.
>> > We should transmit secCtx for fake nodes and secSubjId for real.
>> > 5. NoOpIgniteSecurityProcessor should include a disabled processor and
>> > validate it too if it is not null. It is important for a client node.
>> > For example:
>> > Into IgniteKernal#securityProcessor method createComponent return a
>> > GridSecurityProcessor. For server nodes are enabled, but for clients
>> > aren't.  The clients aren't able to pass validation for this reason.
>> >
>> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>> >
>> > I going to fix it.
>> >
>>
>

[Anton Vinogradov-2]

Maksim,

Could you please split IGNITE-11992 to subtasks with proper descriptions?
This will allow us to relocate discussion to the issues to solve each
problem properly.

On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]> wrote:

> Hello, Maksim!
> Thanks for your analysis!
>
> I have a few questions about your proposals.
>
> GridRestProcessor.
> AFAIK, when GridRestProcessor handle client request
> (GridRestProcessor#handleRequest)
> it process authentication (GridRestProcessor#authenticate)
> and then authorization of request (GridRestProcessor#authorize) inside
> client context.
> Can you give additional info about issues with GridRestProcessor from 3 and
> 4? Maybe you have a reproducer for the problem?
>
> NoOpIgniteSecurityProcessor.
> I think the case that you describe in 5 is not a bug.
> All nodes (client and server) must have security enabled or disabled.
> I can't imagine the case when it is not.
>
> ATTR_SECURITY_SUBJECT.
> I don't think that compatibility is needed here. If you will use node with
> propagation security context to remote node and older nodes
> you can get subtle errors.
>
> чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <[hidden email]
> >:
>
> > Hi, Ivan.
> >
> > Yes, I have.
> > https://issues.apache.org/jira/browse/IGNITE-11992
> >
> > I'm waiting for a visa.
> >
> >
> > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
> >
> >> Hello Max,
> >>
> >> Thanks for your analysis!
> >>
> >> Have you created a JIRA issue for discovered defects?
> >>
> >> Best Regards,
> >> Ivan Rakov
> >>
> >> On 17.07.2019 17:08, Maksim Stepachev wrote:
> >> > Hello, Igniters.
> >> >
> >> >      The main idea of the new security is propagation security context
> >> to
> >> > other nodes and does action with initial permission. The solution
> looks
> >> > fine but has imperfections.
> >> >
> >> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
> >> >    As a result: Caused by: class
> >> org.apache.ignite.spi.IgniteSpiException:
> >> > Security context isn't certain.
> >> > 2. The visor tasks lost permission.
> >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread and
> >> loses
> >> > context.
> >> > 3. The GridRestProcessor does tasks outside "withContext" section.  As
> >> > result context loses.
> >> > 4. The GridRestProcessor isn't client, we can't read security subject
> >> from
> >> > node attribute.
> >> > We should transmit secCtx for fake nodes and secSubjId for real.
> >> > 5. NoOpIgniteSecurityProcessor should include a disabled processor and
> >> > validate it too if it is not null. It is important for a client node.
> >> > For example:
> >> > Into IgniteKernal#securityProcessor method createComponent return a
> >> > GridSecurityProcessor. For server nodes are enabled, but for clients
> >> > aren't.  The clients aren't able to pass validation for this reason.
> >> >
> >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
> >> >
> >> > I going to fix it.
> >> >
> >>
> >
>

[Maksim Stepachev]

Hi.

Anton Vinogradov,

I want to fix 2-3-4 points under one ticket.

The first was fixed in the ticket:
https://issues.apache.org/jira/browse/IGNITE-11094
Also, I aggry with you that 5-6 isn't required to ignite.

Denis Garus,
I made reproducer for point 3. Looks at the test from my pull-request:
JettyRestPropagationSecurityContextTest

https://github.com/apache/ignite/pull/6918

For point 2 you should apply GridRestProcessor from pr and set debug into
VisorQueryUtils#scheduleQueryStart between
ignite.context().closure().runLocalSafe  and call:
ignite.context().security().securityContext()


For point 3, do action above and call:
ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())

It returns null because this subject was created from the rest. It's the
reason why subject id isn't enough and we should transmit subject inside
message for this case.

чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:

> Maksim,
>
> Could you please split IGNITE-11992 to subtasks with proper descriptions?
> This will allow us to relocate discussion to the issues to solve each
> problem properly.
>
> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]> wrote:
>
> > Hello, Maksim!
> > Thanks for your analysis!
> >
> > I have a few questions about your proposals.
> >
> > GridRestProcessor.
> > AFAIK, when GridRestProcessor handle client request
> > (GridRestProcessor#handleRequest)
> > it process authentication (GridRestProcessor#authenticate)
> > and then authorization of request (GridRestProcessor#authorize) inside
> > client context.
> > Can you give additional info about issues with GridRestProcessor from 3
> and
> > 4? Maybe you have a reproducer for the problem?
> >
> > NoOpIgniteSecurityProcessor.
> > I think the case that you describe in 5 is not a bug.
> > All nodes (client and server) must have security enabled or disabled.
> > I can't imagine the case when it is not.
> >
> > ATTR_SECURITY_SUBJECT.
> > I don't think that compatibility is needed here. If you will use node
> with
> > propagation security context to remote node and older nodes
> > you can get subtle errors.
> >
> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
> [hidden email]
> > >:
> >
> > > Hi, Ivan.
> > >
> > > Yes, I have.
> > > https://issues.apache.org/jira/browse/IGNITE-11992
> > >
> > > I'm waiting for a visa.
> > >
> > >
> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
> > >
> > >> Hello Max,
> > >>
> > >> Thanks for your analysis!
> > >>
> > >> Have you created a JIRA issue for discovered defects?
> > >>
> > >> Best Regards,
> > >> Ivan Rakov
> > >>
> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
> > >> > Hello, Igniters.
> > >> >
> > >> >      The main idea of the new security is propagation security
> context
> > >> to
> > >> > other nodes and does action with initial permission. The solution
> > looks
> > >> > fine but has imperfections.
> > >> >
> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
> > >> >    As a result: Caused by: class
> > >> org.apache.ignite.spi.IgniteSpiException:
> > >> > Security context isn't certain.
> > >> > 2. The visor tasks lost permission.
> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread and
> > >> loses
> > >> > context.
> > >> > 3. The GridRestProcessor does tasks outside "withContext" section.
> As
> > >> > result context loses.
> > >> > 4. The GridRestProcessor isn't client, we can't read security
> subject
> > >> from
> > >> > node attribute.
> > >> > We should transmit secCtx for fake nodes and secSubjId for real.
> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled processor
> and
> > >> > validate it too if it is not null. It is important for a client
> node.
> > >> > For example:
> > >> > Into IgniteKernal#securityProcessor method createComponent return a
> > >> > GridSecurityProcessor. For server nodes are enabled, but for clients
> > >> > aren't.  The clients aren't able to pass validation for this reason.
> > >> >
> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
> > >> >
> > >> > I going to fix it.
> > >> >
> > >>
> > >
> >
>

[Anton Vinogradov-2]

Maksim

>> I want to fix 2-3-4 points under one ticket.
Please let me know once it's become ready to be reviewed.

On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <[hidden email]>
wrote:

> Hi.
>
> Anton Vinogradov,
>
> I want to fix 2-3-4 points under one ticket.
>
> The first was fixed in the ticket:
> https://issues.apache.org/jira/browse/IGNITE-11094
> Also, I aggry with you that 5-6 isn't required to ignite.
>
> Denis Garus,
> I made reproducer for point 3. Looks at the test from my pull-request:
> JettyRestPropagationSecurityContextTest
>
> https://github.com/apache/ignite/pull/6918
>
> For point 2 you should apply GridRestProcessor from pr and set debug into
> VisorQueryUtils#scheduleQueryStart between
> ignite.context().closure().runLocalSafe  and call:
> ignite.context().security().securityContext()
>
>
> For point 3, do action above and call:
> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
>
> It returns null because this subject was created from the rest. It's the
> reason why subject id isn't enough and we should transmit subject inside
> message for this case.
>
> чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
>
>> Maksim,
>>
>> Could you please split IGNITE-11992 to subtasks with proper descriptions?
>> This will allow us to relocate discussion to the issues to solve each
>> problem properly.
>>
>> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]> wrote:
>>
>> > Hello, Maksim!
>> > Thanks for your analysis!
>> >
>> > I have a few questions about your proposals.
>> >
>> > GridRestProcessor.
>> > AFAIK, when GridRestProcessor handle client request
>> > (GridRestProcessor#handleRequest)
>> > it process authentication (GridRestProcessor#authenticate)
>> > and then authorization of request (GridRestProcessor#authorize) inside
>> > client context.
>> > Can you give additional info about issues with GridRestProcessor from 3
>> and
>> > 4? Maybe you have a reproducer for the problem?
>> >
>> > NoOpIgniteSecurityProcessor.
>> > I think the case that you describe in 5 is not a bug.
>> > All nodes (client and server) must have security enabled or disabled.
>> > I can't imagine the case when it is not.
>> >
>> > ATTR_SECURITY_SUBJECT.
>> > I don't think that compatibility is needed here. If you will use node
>> with
>> > propagation security context to remote node and older nodes
>> > you can get subtle errors.
>> >
>> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
>> [hidden email]
>> > >:
>> >
>> > > Hi, Ivan.
>> > >
>> > > Yes, I have.
>> > > https://issues.apache.org/jira/browse/IGNITE-11992
>> > >
>> > > I'm waiting for a visa.
>> > >
>> > >
>> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
>> > >
>> > >> Hello Max,
>> > >>
>> > >> Thanks for your analysis!
>> > >>
>> > >> Have you created a JIRA issue for discovered defects?
>> > >>
>> > >> Best Regards,
>> > >> Ivan Rakov
>> > >>
>> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
>> > >> > Hello, Igniters.
>> > >> >
>> > >> >      The main idea of the new security is propagation security
>> context
>> > >> to
>> > >> > other nodes and does action with initial permission. The solution
>> > looks
>> > >> > fine but has imperfections.
>> > >> >
>> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
>> > >> >    As a result: Caused by: class
>> > >> org.apache.ignite.spi.IgniteSpiException:
>> > >> > Security context isn't certain.
>> > >> > 2. The visor tasks lost permission.
>> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread
>> and
>> > >> loses
>> > >> > context.
>> > >> > 3. The GridRestProcessor does tasks outside "withContext"
>> section.  As
>> > >> > result context loses.
>> > >> > 4. The GridRestProcessor isn't client, we can't read security
>> subject
>> > >> from
>> > >> > node attribute.
>> > >> > We should transmit secCtx for fake nodes and secSubjId for real.
>> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled processor
>> and
>> > >> > validate it too if it is not null. It is important for a client
>> node.
>> > >> > For example:
>> > >> > Into IgniteKernal#securityProcessor method createComponent return a
>> > >> > GridSecurityProcessor. For server nodes are enabled, but for
>> clients
>> > >> > aren't.  The clients aren't able to pass validation for this
>> reason.
>> > >> >
>> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>> > >> >
>> > >> > I going to fix it.
>> > >> >
>> > >>
>> > >
>> >
>>
>

[Denis Garus]

Hello, Maksim!

Thank you for your effort and interest in the security of Ignite.

I would like you to pay attention to the discussion [1] and issue [2].
It looks like not only task should execute in the current security context
but all operations too, that is essential to determine a security id for
events.
Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
duplication of IgnitSecurity responsibility.
I think your task is the right place to do that.
What is your opinion?

>>It's the reason why subject id isn't enough and we should transmit
subject inside message for this case.
There is a problem with this approach.
Subject's size is unlimited, that can lead to a dramatic increase in
traffic between nodes.

1.
http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
2. https://issues.apache.org/jira/browse/IGNITE-9914

пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:

> Maksim
>
> >> I want to fix 2-3-4 points under one ticket.
> Please let me know once it's become ready to be reviewed.
>
> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
> [hidden email]>
> wrote:
>
> > Hi.
> >
> > Anton Vinogradov,
> >
> > I want to fix 2-3-4 points under one ticket.
> >
> > The first was fixed in the ticket:
> > https://issues.apache.org/jira/browse/IGNITE-11094
> > Also, I aggry with you that 5-6 isn't required to ignite.
> >
> > Denis Garus,
> > I made reproducer for point 3. Looks at the test from my pull-request:
> > JettyRestPropagationSecurityContextTest
> >
> > https://github.com/apache/ignite/pull/6918
> >
> > For point 2 you should apply GridRestProcessor from pr and set debug into
> > VisorQueryUtils#scheduleQueryStart between
> > ignite.context().closure().runLocalSafe  and call:
> > ignite.context().security().securityContext()
> >
> >
> > For point 3, do action above and call:
> >
> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
> >
> > It returns null because this subject was created from the rest. It's the
> > reason why subject id isn't enough and we should transmit subject inside
> > message for this case.
> >
> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
> >
> >> Maksim,
> >>
> >> Could you please split IGNITE-11992 to subtasks with proper
> descriptions?
> >> This will allow us to relocate discussion to the issues to solve each
> >> problem properly.
> >>
> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]>
> wrote:
> >>
> >> > Hello, Maksim!
> >> > Thanks for your analysis!
> >> >
> >> > I have a few questions about your proposals.
> >> >
> >> > GridRestProcessor.
> >> > AFAIK, when GridRestProcessor handle client request
> >> > (GridRestProcessor#handleRequest)
> >> > it process authentication (GridRestProcessor#authenticate)
> >> > and then authorization of request (GridRestProcessor#authorize) inside
> >> > client context.
> >> > Can you give additional info about issues with GridRestProcessor from
> 3
> >> and
> >> > 4? Maybe you have a reproducer for the problem?
> >> >
> >> > NoOpIgniteSecurityProcessor.
> >> > I think the case that you describe in 5 is not a bug.
> >> > All nodes (client and server) must have security enabled or disabled.
> >> > I can't imagine the case when it is not.
> >> >
> >> > ATTR_SECURITY_SUBJECT.
> >> > I don't think that compatibility is needed here. If you will use node
> >> with
> >> > propagation security context to remote node and older nodes
> >> > you can get subtle errors.
> >> >
> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
> >> [hidden email]
> >> > >:
> >> >
> >> > > Hi, Ivan.
> >> > >
> >> > > Yes, I have.
> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
> >> > >
> >> > > I'm waiting for a visa.
> >> > >
> >> > >
> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
> >> > >
> >> > >> Hello Max,
> >> > >>
> >> > >> Thanks for your analysis!
> >> > >>
> >> > >> Have you created a JIRA issue for discovered defects?
> >> > >>
> >> > >> Best Regards,
> >> > >> Ivan Rakov
> >> > >>
> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
> >> > >> > Hello, Igniters.
> >> > >> >
> >> > >> >      The main idea of the new security is propagation security
> >> context
> >> > >> to
> >> > >> > other nodes and does action with initial permission. The solution
> >> > looks
> >> > >> > fine but has imperfections.
> >> > >> >
> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into itself.
> >> > >> >    As a result: Caused by: class
> >> > >> org.apache.ignite.spi.IgniteSpiException:
> >> > >> > Security context isn't certain.
> >> > >> > 2. The visor tasks lost permission.
> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread
> >> and
> >> > >> loses
> >> > >> > context.
> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
> >> section.  As
> >> > >> > result context loses.
> >> > >> > 4. The GridRestProcessor isn't client, we can't read security
> >> subject
> >> > >> from
> >> > >> > node attribute.
> >> > >> > We should transmit secCtx for fake nodes and secSubjId for real.
> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
> processor
> >> and
> >> > >> > validate it too if it is not null. It is important for a client
> >> node.
> >> > >> > For example:
> >> > >> > Into IgniteKernal#securityProcessor method createComponent
> return a
> >> > >> > GridSecurityProcessor. For server nodes are enabled, but for
> >> clients
> >> > >> > aren't.  The clients aren't able to pass validation for this
> >> reason.
> >> > >> >
> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
> >> > >> >
> >> > >> > I going to fix it.
> >> > >> >
> >> > >>
> >> > >
> >> >
> >>
> >
>

[Maksim Stepachev]

I finished with fixes: https://issues.apache.org/jira/browse/IGNITE-11992

>> Subject's size is unlimited, that can lead to a dramatic increase in
traffic between nodes.
I added network optimization for this case. I add a subject in the case
when ctx.discovery().node(secSubjId) == null.

>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
duplication of IgnitSecurity responsibility.
[2]Yes, we should rid of this. But in the next task, because I can't merge
it since 18 Jul 19:)

[1] I aggry with you.


пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:

> Hello, Maksim!
>
> Thank you for your effort and interest in the security of Ignite.
>
> I would like you to pay attention to the discussion [1] and issue [2].
> It looks like not only task should execute in the current security context
> but all operations too, that is essential to determine a security id for
> events.
> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> duplication of IgnitSecurity responsibility.
> I think your task is the right place to do that.
> What is your opinion?
>
> >>It's the reason why subject id isn't enough and we should transmit
> subject inside message for this case.
> There is a problem with this approach.
> Subject's size is unlimited, that can lead to a dramatic increase in
> traffic between nodes.
>
> 1.
> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
> 2. https://issues.apache.org/jira/browse/IGNITE-9914
>
> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
>
>> Maksim
>>
>> >> I want to fix 2-3-4 points under one ticket.
>> Please let me know once it's become ready to be reviewed.
>>
>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
>> [hidden email]>
>> wrote:
>>
>> > Hi.
>> >
>> > Anton Vinogradov,
>> >
>> > I want to fix 2-3-4 points under one ticket.
>> >
>> > The first was fixed in the ticket:
>> > https://issues.apache.org/jira/browse/IGNITE-11094
>> > Also, I aggry with you that 5-6 isn't required to ignite.
>> >
>> > Denis Garus,
>> > I made reproducer for point 3. Looks at the test from my pull-request:
>> > JettyRestPropagationSecurityContextTest
>> >
>> > https://github.com/apache/ignite/pull/6918
>> >
>> > For point 2 you should apply GridRestProcessor from pr and set debug
>> into
>> > VisorQueryUtils#scheduleQueryStart between
>> > ignite.context().closure().runLocalSafe  and call:
>> > ignite.context().security().securityContext()
>> >
>> >
>> > For point 3, do action above and call:
>> >
>> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
>> >
>> > It returns null because this subject was created from the rest. It's the
>> > reason why subject id isn't enough and we should transmit subject inside
>> > message for this case.
>> >
>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
>> >
>> >> Maksim,
>> >>
>> >> Could you please split IGNITE-11992 to subtasks with proper
>> descriptions?
>> >> This will allow us to relocate discussion to the issues to solve each
>> >> problem properly.
>> >>
>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]>
>> wrote:
>> >>
>> >> > Hello, Maksim!
>> >> > Thanks for your analysis!
>> >> >
>> >> > I have a few questions about your proposals.
>> >> >
>> >> > GridRestProcessor.
>> >> > AFAIK, when GridRestProcessor handle client request
>> >> > (GridRestProcessor#handleRequest)
>> >> > it process authentication (GridRestProcessor#authenticate)
>> >> > and then authorization of request (GridRestProcessor#authorize)
>> inside
>> >> > client context.
>> >> > Can you give additional info about issues with GridRestProcessor
>> from 3
>> >> and
>> >> > 4? Maybe you have a reproducer for the problem?
>> >> >
>> >> > NoOpIgniteSecurityProcessor.
>> >> > I think the case that you describe in 5 is not a bug.
>> >> > All nodes (client and server) must have security enabled or disabled.
>> >> > I can't imagine the case when it is not.
>> >> >
>> >> > ATTR_SECURITY_SUBJECT.
>> >> > I don't think that compatibility is needed here. If you will use node
>> >> with
>> >> > propagation security context to remote node and older nodes
>> >> > you can get subtle errors.
>> >> >
>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
>> >> [hidden email]
>> >> > >:
>> >> >
>> >> > > Hi, Ivan.
>> >> > >
>> >> > > Yes, I have.
>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
>> >> > >
>> >> > > I'm waiting for a visa.
>> >> > >
>> >> > >
>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
>> >> > >
>> >> > >> Hello Max,
>> >> > >>
>> >> > >> Thanks for your analysis!
>> >> > >>
>> >> > >> Have you created a JIRA issue for discovered defects?
>> >> > >>
>> >> > >> Best Regards,
>> >> > >> Ivan Rakov
>> >> > >>
>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
>> >> > >> > Hello, Igniters.
>> >> > >> >
>> >> > >> >      The main idea of the new security is propagation security
>> >> context
>> >> > >> to
>> >> > >> > other nodes and does action with initial permission. The
>> solution
>> >> > looks
>> >> > >> > fine but has imperfections.
>> >> > >> >
>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
>> itself.
>> >> > >> >    As a result: Caused by: class
>> >> > >> org.apache.ignite.spi.IgniteSpiException:
>> >> > >> > Security context isn't certain.
>> >> > >> > 2. The visor tasks lost permission.
>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new thread
>> >> and
>> >> > >> loses
>> >> > >> > context.
>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
>> >> section.  As
>> >> > >> > result context loses.
>> >> > >> > 4. The GridRestProcessor isn't client, we can't read security
>> >> subject
>> >> > >> from
>> >> > >> > node attribute.
>> >> > >> > We should transmit secCtx for fake nodes and secSubjId for real.
>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
>> processor
>> >> and
>> >> > >> > validate it too if it is not null. It is important for a client
>> >> node.
>> >> > >> > For example:
>> >> > >> > Into IgniteKernal#securityProcessor method createComponent
>> return a
>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but for
>> >> clients
>> >> > >> > aren't.  The clients aren't able to pass validation for this
>> >> reason.
>> >> > >> >
>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>> >> > >> >
>> >> > >> > I going to fix it.
>> >> > >> >
>> >> > >>
>> >> > >
>> >> >
>> >>
>> >
>>
>

[Denis Garus]

>>>> Subject's size is unlimited, that can lead to a dramatic increase in
traffic between nodes.
>>I added network optimization for this case. I add a subject in the case
when ctx.discovery().node(secSubjId) == null.

Yes, this optimization is good, but we have to send SecurityContext
whenever a client starts a job.
Why?

A better solution would be to send SecurityContext on demand.

Imagine the following scenario.
A client connects to node A and starts a job that runs on remote node B.
IgniteSecurityProcessor on node B tries to find SecurityContext by
subjectId.
If IgniteSecurityProcessor fails, then it sends SecurityContextRequest to
node A and gets required SecurityContext
that afterward puts to the IgniteSecurityProcessor's cache on node B.
The next request to run a job on node B with this subjectId doesn't require
SecurityContext transmission.

SecurityContextResponse can contain some additional information, for
example,
time-to-live of SecurityContext before eviction SecurityContext from the
IgniteSecurityProcessor's cache.

пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev <[hidden email]>:

> I finished with fixes: https://issues.apache.org/jira/browse/IGNITE-11992
>
> >> Subject's size is unlimited, that can lead to a dramatic increase in
> traffic between nodes.
> I added network optimization for this case. I add a subject in the case
> when ctx.discovery().node(secSubjId) == null.
>
> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> duplication of IgnitSecurity responsibility.
> [2]Yes, we should rid of this. But in the next task, because I can't merge
> it since 18 Jul 19:)
>
> [1] I aggry with you.
>
>
> пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:
>
>> Hello, Maksim!
>>
>> Thank you for your effort and interest in the security of Ignite.
>>
>> I would like you to pay attention to the discussion [1] and issue [2].
>> It looks like not only task should execute in the current security
>> context but all operations too, that is essential to determine a security
>> id for events.
>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
>> duplication of IgnitSecurity responsibility.
>> I think your task is the right place to do that.
>> What is your opinion?
>>
>> >>It's the reason why subject id isn't enough and we should transmit
>> subject inside message for this case.
>> There is a problem with this approach.
>> Subject's size is unlimited, that can lead to a dramatic increase in
>> traffic between nodes.
>>
>> 1.
>> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
>> 2. https://issues.apache.org/jira/browse/IGNITE-9914
>>
>> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
>>
>>> Maksim
>>>
>>> >> I want to fix 2-3-4 points under one ticket.
>>> Please let me know once it's become ready to be reviewed.
>>>
>>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
>>> [hidden email]>
>>> wrote:
>>>
>>> > Hi.
>>> >
>>> > Anton Vinogradov,
>>> >
>>> > I want to fix 2-3-4 points under one ticket.
>>> >
>>> > The first was fixed in the ticket:
>>> > https://issues.apache.org/jira/browse/IGNITE-11094
>>> > Also, I aggry with you that 5-6 isn't required to ignite.
>>> >
>>> > Denis Garus,
>>> > I made reproducer for point 3. Looks at the test from my pull-request:
>>> > JettyRestPropagationSecurityContextTest
>>> >
>>> > https://github.com/apache/ignite/pull/6918
>>> >
>>> > For point 2 you should apply GridRestProcessor from pr and set debug
>>> into
>>> > VisorQueryUtils#scheduleQueryStart between
>>> > ignite.context().closure().runLocalSafe  and call:
>>> > ignite.context().security().securityContext()
>>> >
>>> >
>>> > For point 3, do action above and call:
>>> >
>>> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
>>> >
>>> > It returns null because this subject was created from the rest. It's
>>> the
>>> > reason why subject id isn't enough and we should transmit subject
>>> inside
>>> > message for this case.
>>> >
>>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
>>> >
>>> >> Maksim,
>>> >>
>>> >> Could you please split IGNITE-11992 to subtasks with proper
>>> descriptions?
>>> >> This will allow us to relocate discussion to the issues to solve each
>>> >> problem properly.
>>> >>
>>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]>
>>> wrote:
>>> >>
>>> >> > Hello, Maksim!
>>> >> > Thanks for your analysis!
>>> >> >
>>> >> > I have a few questions about your proposals.
>>> >> >
>>> >> > GridRestProcessor.
>>> >> > AFAIK, when GridRestProcessor handle client request
>>> >> > (GridRestProcessor#handleRequest)
>>> >> > it process authentication (GridRestProcessor#authenticate)
>>> >> > and then authorization of request (GridRestProcessor#authorize)
>>> inside
>>> >> > client context.
>>> >> > Can you give additional info about issues with GridRestProcessor
>>> from 3
>>> >> and
>>> >> > 4? Maybe you have a reproducer for the problem?
>>> >> >
>>> >> > NoOpIgniteSecurityProcessor.
>>> >> > I think the case that you describe in 5 is not a bug.
>>> >> > All nodes (client and server) must have security enabled or
>>> disabled.
>>> >> > I can't imagine the case when it is not.
>>> >> >
>>> >> > ATTR_SECURITY_SUBJECT.
>>> >> > I don't think that compatibility is needed here. If you will use
>>> node
>>> >> with
>>> >> > propagation security context to remote node and older nodes
>>> >> > you can get subtle errors.
>>> >> >
>>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
>>> >> [hidden email]
>>> >> > >:
>>> >> >
>>> >> > > Hi, Ivan.
>>> >> > >
>>> >> > > Yes, I have.
>>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
>>> >> > >
>>> >> > > I'm waiting for a visa.
>>> >> > >
>>> >> > >
>>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
>>> >> > >
>>> >> > >> Hello Max,
>>> >> > >>
>>> >> > >> Thanks for your analysis!
>>> >> > >>
>>> >> > >> Have you created a JIRA issue for discovered defects?
>>> >> > >>
>>> >> > >> Best Regards,
>>> >> > >> Ivan Rakov
>>> >> > >>
>>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
>>> >> > >> > Hello, Igniters.
>>> >> > >> >
>>> >> > >> >      The main idea of the new security is propagation security
>>> >> context
>>> >> > >> to
>>> >> > >> > other nodes and does action with initial permission. The
>>> solution
>>> >> > looks
>>> >> > >> > fine but has imperfections.
>>> >> > >> >
>>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
>>> itself.
>>> >> > >> >    As a result: Caused by: class
>>> >> > >> org.apache.ignite.spi.IgniteSpiException:
>>> >> > >> > Security context isn't certain.
>>> >> > >> > 2. The visor tasks lost permission.
>>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new
>>> thread
>>> >> and
>>> >> > >> loses
>>> >> > >> > context.
>>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
>>> >> section.  As
>>> >> > >> > result context loses.
>>> >> > >> > 4. The GridRestProcessor isn't client, we can't read security
>>> >> subject
>>> >> > >> from
>>> >> > >> > node attribute.
>>> >> > >> > We should transmit secCtx for fake nodes and secSubjId for
>>> real.
>>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
>>> processor
>>> >> and
>>> >> > >> > validate it too if it is not null. It is important for a client
>>> >> node.
>>> >> > >> > For example:
>>> >> > >> > Into IgniteKernal#securityProcessor method createComponent
>>> return a
>>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but for
>>> >> clients
>>> >> > >> > aren't.  The clients aren't able to pass validation for this
>>> >> reason.
>>> >> > >> >
>>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>>> >> > >> >
>>> >> > >> > I going to fix it.
>>> >> > >> >
>>> >> > >>
>>> >> > >
>>> >> >
>>> >>
>>> >
>>>
>>

[Maksim Stepachev]

I suppose that code works only with requests are made from
GridRestProcessor (It isn't a client, I call it like a fake client). As a
result, you can't load security on demand. If you want to do it, you
should transmit HTTP session and backward address of a node which
received REST request.

пн, 30 сент. 2019 г. в 16:16, Denis Garus <[hidden email]>:

> >>>> Subject's size is unlimited, that can lead to a dramatic increase in
> traffic between nodes.
> >>I added network optimization for this case. I add a subject in the case
> when ctx.discovery().node(secSubjId) == null.
>
> Yes, this optimization is good, but we have to send SecurityContext
> whenever a client starts a job.
> Why?
>
> A better solution would be to send SecurityContext on demand.
>
> Imagine the following scenario.
> A client connects to node A and starts a job that runs on remote node B.
> IgniteSecurityProcessor on node B tries to find SecurityContext by
> subjectId.
> If IgniteSecurityProcessor fails, then it sends SecurityContextRequest to
> node A and gets required SecurityContext
> that afterward puts to the IgniteSecurityProcessor's cache on node B.
> The next request to run a job on node B with this subjectId doesn't
> require SecurityContext transmission.
>
> SecurityContextResponse can contain some additional information, for
> example,
> time-to-live of SecurityContext before eviction SecurityContext from the
> IgniteSecurityProcessor's cache.
>
> пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev <[hidden email]
> >:
>
>> I finished with fixes: https://issues.apache.org/jira/browse/IGNITE-11992
>>
>> >> Subject's size is unlimited, that can lead to a dramatic increase in
>> traffic between nodes.
>> I added network optimization for this case. I add a subject in the case
>> when ctx.discovery().node(secSubjId) == null.
>>
>> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
>> duplication of IgnitSecurity responsibility.
>> [2]Yes, we should rid of this. But in the next task, because I can't
>> merge it since 18 Jul 19:)
>>
>> [1] I aggry with you.
>>
>>
>> пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:
>>
>>> Hello, Maksim!
>>>
>>> Thank you for your effort and interest in the security of Ignite.
>>>
>>> I would like you to pay attention to the discussion [1] and issue [2].
>>> It looks like not only task should execute in the current security
>>> context but all operations too, that is essential to determine a security
>>> id for events.
>>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
>>> duplication of IgnitSecurity responsibility.
>>> I think your task is the right place to do that.
>>> What is your opinion?
>>>
>>> >>It's the reason why subject id isn't enough and we should transmit
>>> subject inside message for this case.
>>> There is a problem with this approach.
>>> Subject's size is unlimited, that can lead to a dramatic increase in
>>> traffic between nodes.
>>>
>>> 1.
>>> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
>>> 2. https://issues.apache.org/jira/browse/IGNITE-9914
>>>
>>> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
>>>
>>>> Maksim
>>>>
>>>> >> I want to fix 2-3-4 points under one ticket.
>>>> Please let me know once it's become ready to be reviewed.
>>>>
>>>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
>>>> [hidden email]>
>>>> wrote:
>>>>
>>>> > Hi.
>>>> >
>>>> > Anton Vinogradov,
>>>> >
>>>> > I want to fix 2-3-4 points under one ticket.
>>>> >
>>>> > The first was fixed in the ticket:
>>>> > https://issues.apache.org/jira/browse/IGNITE-11094
>>>> > Also, I aggry with you that 5-6 isn't required to ignite.
>>>> >
>>>> > Denis Garus,
>>>> > I made reproducer for point 3. Looks at the test from my pull-request:
>>>> > JettyRestPropagationSecurityContextTest
>>>> >
>>>> > https://github.com/apache/ignite/pull/6918
>>>> >
>>>> > For point 2 you should apply GridRestProcessor from pr and set debug
>>>> into
>>>> > VisorQueryUtils#scheduleQueryStart between
>>>> > ignite.context().closure().runLocalSafe  and call:
>>>> > ignite.context().security().securityContext()
>>>> >
>>>> >
>>>> > For point 3, do action above and call:
>>>> >
>>>> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
>>>> >
>>>> > It returns null because this subject was created from the rest. It's
>>>> the
>>>> > reason why subject id isn't enough and we should transmit subject
>>>> inside
>>>> > message for this case.
>>>> >
>>>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
>>>> >
>>>> >> Maksim,
>>>> >>
>>>> >> Could you please split IGNITE-11992 to subtasks with proper
>>>> descriptions?
>>>> >> This will allow us to relocate discussion to the issues to solve each
>>>> >> problem properly.
>>>> >>
>>>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]>
>>>> wrote:
>>>> >>
>>>> >> > Hello, Maksim!
>>>> >> > Thanks for your analysis!
>>>> >> >
>>>> >> > I have a few questions about your proposals.
>>>> >> >
>>>> >> > GridRestProcessor.
>>>> >> > AFAIK, when GridRestProcessor handle client request
>>>> >> > (GridRestProcessor#handleRequest)
>>>> >> > it process authentication (GridRestProcessor#authenticate)
>>>> >> > and then authorization of request (GridRestProcessor#authorize)
>>>> inside
>>>> >> > client context.
>>>> >> > Can you give additional info about issues with GridRestProcessor
>>>> from 3
>>>> >> and
>>>> >> > 4? Maybe you have a reproducer for the problem?
>>>> >> >
>>>> >> > NoOpIgniteSecurityProcessor.
>>>> >> > I think the case that you describe in 5 is not a bug.
>>>> >> > All nodes (client and server) must have security enabled or
>>>> disabled.
>>>> >> > I can't imagine the case when it is not.
>>>> >> >
>>>> >> > ATTR_SECURITY_SUBJECT.
>>>> >> > I don't think that compatibility is needed here. If you will use
>>>> node
>>>> >> with
>>>> >> > propagation security context to remote node and older nodes
>>>> >> > you can get subtle errors.
>>>> >> >
>>>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
>>>> >> [hidden email]
>>>> >> > >:
>>>> >> >
>>>> >> > > Hi, Ivan.
>>>> >> > >
>>>> >> > > Yes, I have.
>>>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
>>>> >> > >
>>>> >> > > I'm waiting for a visa.
>>>> >> > >
>>>> >> > >
>>>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]>:
>>>> >> > >
>>>> >> > >> Hello Max,
>>>> >> > >>
>>>> >> > >> Thanks for your analysis!
>>>> >> > >>
>>>> >> > >> Have you created a JIRA issue for discovered defects?
>>>> >> > >>
>>>> >> > >> Best Regards,
>>>> >> > >> Ivan Rakov
>>>> >> > >>
>>>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
>>>> >> > >> > Hello, Igniters.
>>>> >> > >> >
>>>> >> > >> >      The main idea of the new security is propagation security
>>>> >> context
>>>> >> > >> to
>>>> >> > >> > other nodes and does action with initial permission. The
>>>> solution
>>>> >> > looks
>>>> >> > >> > fine but has imperfections.
>>>> >> > >> >
>>>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
>>>> itself.
>>>> >> > >> >    As a result: Caused by: class
>>>> >> > >> org.apache.ignite.spi.IgniteSpiException:
>>>> >> > >> > Security context isn't certain.
>>>> >> > >> > 2. The visor tasks lost permission.
>>>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new
>>>> thread
>>>> >> and
>>>> >> > >> loses
>>>> >> > >> > context.
>>>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
>>>> >> section.  As
>>>> >> > >> > result context loses.
>>>> >> > >> > 4. The GridRestProcessor isn't client, we can't read security
>>>> >> subject
>>>> >> > >> from
>>>> >> > >> > node attribute.
>>>> >> > >> > We should transmit secCtx for fake nodes and secSubjId for
>>>> real.
>>>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
>>>> processor
>>>> >> and
>>>> >> > >> > validate it too if it is not null. It is important for a
>>>> client
>>>> >> node.
>>>> >> > >> > For example:
>>>> >> > >> > Into IgniteKernal#securityProcessor method createComponent
>>>> return a
>>>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but for
>>>> >> clients
>>>> >> > >> > aren't.  The clients aren't able to pass validation for this
>>>> >> reason.
>>>> >> > >> >
>>>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>>>> >> > >> >
>>>> >> > >> > I going to fix it.
>>>> >> > >> >
>>>> >> > >>
>>>> >> > >
>>>> >> >
>>>> >>
>>>> >
>>>>
>>>

[Denis Garus]

>>As a result, you can't load security on demand.

Why?
What is the difference between sending SecurityContext with every job's
request and sending SecurityContext once when remote node demand it?

пн, 30 сент. 2019 г. в 17:25, Maksim Stepachev <[hidden email]>:

> I suppose that code works only with requests are made from
> GridRestProcessor (It isn't a client, I call it like a fake client). As a
> result, you can't load security on demand. If you want to do it, you
> should transmit HTTP session and backward address of a node which
> received REST request.
>
> пн, 30 сент. 2019 г. в 16:16, Denis Garus <[hidden email]>:
>
>> >>>> Subject's size is unlimited, that can lead to a dramatic increase in
>> traffic between nodes.
>> >>I added network optimization for this case. I add a subject in the case
>> when ctx.discovery().node(secSubjId) == null.
>>
>> Yes, this optimization is good, but we have to send SecurityContext
>> whenever a client starts a job.
>> Why?
>>
>> A better solution would be to send SecurityContext on demand.
>>
>> Imagine the following scenario.
>> A client connects to node A and starts a job that runs on remote node B.
>> IgniteSecurityProcessor on node B tries to find SecurityContext by
>> subjectId.
>> If IgniteSecurityProcessor fails, then it sends SecurityContextRequest to
>> node A and gets required SecurityContext
>> that afterward puts to the IgniteSecurityProcessor's cache on node B.
>> The next request to run a job on node B with this subjectId doesn't
>> require SecurityContext transmission.
>>
>> SecurityContextResponse can contain some additional information, for
>> example,
>> time-to-live of SecurityContext before eviction SecurityContext from the
>> IgniteSecurityProcessor's cache.
>>
>> пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev <
>> [hidden email]>:
>>
>>> I finished with fixes:
>>> https://issues.apache.org/jira/browse/IGNITE-11992
>>>
>>> >> Subject's size is unlimited, that can lead to a dramatic increase in
>>> traffic between nodes.
>>> I added network optimization for this case. I add a subject in the case
>>> when ctx.discovery().node(secSubjId) == null.
>>>
>>> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
>>> duplication of IgnitSecurity responsibility.
>>> [2]Yes, we should rid of this. But in the next task, because I can't
>>> merge it since 18 Jul 19:)
>>>
>>> [1] I aggry with you.
>>>
>>>
>>> пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:
>>>
>>>> Hello, Maksim!
>>>>
>>>> Thank you for your effort and interest in the security of Ignite.
>>>>
>>>> I would like you to pay attention to the discussion [1] and issue [2].
>>>> It looks like not only task should execute in the current security
>>>> context but all operations too, that is essential to determine a security
>>>> id for events.
>>>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
>>>> duplication of IgnitSecurity responsibility.
>>>> I think your task is the right place to do that.
>>>> What is your opinion?
>>>>
>>>> >>It's the reason why subject id isn't enough and we should transmit
>>>> subject inside message for this case.
>>>> There is a problem with this approach.
>>>> Subject's size is unlimited, that can lead to a dramatic increase in
>>>> traffic between nodes.
>>>>
>>>> 1.
>>>> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
>>>> 2. https://issues.apache.org/jira/browse/IGNITE-9914
>>>>
>>>> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
>>>>
>>>>> Maksim
>>>>>
>>>>> >> I want to fix 2-3-4 points under one ticket.
>>>>> Please let me know once it's become ready to be reviewed.
>>>>>
>>>>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
>>>>> [hidden email]>
>>>>> wrote:
>>>>>
>>>>> > Hi.
>>>>> >
>>>>> > Anton Vinogradov,
>>>>> >
>>>>> > I want to fix 2-3-4 points under one ticket.
>>>>> >
>>>>> > The first was fixed in the ticket:
>>>>> > https://issues.apache.org/jira/browse/IGNITE-11094
>>>>> > Also, I aggry with you that 5-6 isn't required to ignite.
>>>>> >
>>>>> > Denis Garus,
>>>>> > I made reproducer for point 3. Looks at the test from my
>>>>> pull-request:
>>>>> > JettyRestPropagationSecurityContextTest
>>>>> >
>>>>> > https://github.com/apache/ignite/pull/6918
>>>>> >
>>>>> > For point 2 you should apply GridRestProcessor from pr and set debug
>>>>> into
>>>>> > VisorQueryUtils#scheduleQueryStart between
>>>>> > ignite.context().closure().runLocalSafe  and call:
>>>>> > ignite.context().security().securityContext()
>>>>> >
>>>>> >
>>>>> > For point 3, do action above and call:
>>>>> >
>>>>> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
>>>>> >
>>>>> > It returns null because this subject was created from the rest. It's
>>>>> the
>>>>> > reason why subject id isn't enough and we should transmit subject
>>>>> inside
>>>>> > message for this case.
>>>>> >
>>>>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
>>>>> >
>>>>> >> Maksim,
>>>>> >>
>>>>> >> Could you please split IGNITE-11992 to subtasks with proper
>>>>> descriptions?
>>>>> >> This will allow us to relocate discussion to the issues to solve
>>>>> each
>>>>> >> problem properly.
>>>>> >>
>>>>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]>
>>>>> wrote:
>>>>> >>
>>>>> >> > Hello, Maksim!
>>>>> >> > Thanks for your analysis!
>>>>> >> >
>>>>> >> > I have a few questions about your proposals.
>>>>> >> >
>>>>> >> > GridRestProcessor.
>>>>> >> > AFAIK, when GridRestProcessor handle client request
>>>>> >> > (GridRestProcessor#handleRequest)
>>>>> >> > it process authentication (GridRestProcessor#authenticate)
>>>>> >> > and then authorization of request (GridRestProcessor#authorize)
>>>>> inside
>>>>> >> > client context.
>>>>> >> > Can you give additional info about issues with GridRestProcessor
>>>>> from 3
>>>>> >> and
>>>>> >> > 4? Maybe you have a reproducer for the problem?
>>>>> >> >
>>>>> >> > NoOpIgniteSecurityProcessor.
>>>>> >> > I think the case that you describe in 5 is not a bug.
>>>>> >> > All nodes (client and server) must have security enabled or
>>>>> disabled.
>>>>> >> > I can't imagine the case when it is not.
>>>>> >> >
>>>>> >> > ATTR_SECURITY_SUBJECT.
>>>>> >> > I don't think that compatibility is needed here. If you will use
>>>>> node
>>>>> >> with
>>>>> >> > propagation security context to remote node and older nodes
>>>>> >> > you can get subtle errors.
>>>>> >> >
>>>>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
>>>>> >> [hidden email]
>>>>> >> > >:
>>>>> >> >
>>>>> >> > > Hi, Ivan.
>>>>> >> > >
>>>>> >> > > Yes, I have.
>>>>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
>>>>> >> > >
>>>>> >> > > I'm waiting for a visa.
>>>>> >> > >
>>>>> >> > >
>>>>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <[hidden email]
>>>>> >:
>>>>> >> > >
>>>>> >> > >> Hello Max,
>>>>> >> > >>
>>>>> >> > >> Thanks for your analysis!
>>>>> >> > >>
>>>>> >> > >> Have you created a JIRA issue for discovered defects?
>>>>> >> > >>
>>>>> >> > >> Best Regards,
>>>>> >> > >> Ivan Rakov
>>>>> >> > >>
>>>>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
>>>>> >> > >> > Hello, Igniters.
>>>>> >> > >> >
>>>>> >> > >> >      The main idea of the new security is propagation
>>>>> security
>>>>> >> context
>>>>> >> > >> to
>>>>> >> > >> > other nodes and does action with initial permission. The
>>>>> solution
>>>>> >> > looks
>>>>> >> > >> > fine but has imperfections.
>>>>> >> > >> >
>>>>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
>>>>> itself.
>>>>> >> > >> >    As a result: Caused by: class
>>>>> >> > >> org.apache.ignite.spi.IgniteSpiException:
>>>>> >> > >> > Security context isn't certain.
>>>>> >> > >> > 2. The visor tasks lost permission.
>>>>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new
>>>>> thread
>>>>> >> and
>>>>> >> > >> loses
>>>>> >> > >> > context.
>>>>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
>>>>> >> section.  As
>>>>> >> > >> > result context loses.
>>>>> >> > >> > 4. The GridRestProcessor isn't client, we can't read security
>>>>> >> subject
>>>>> >> > >> from
>>>>> >> > >> > node attribute.
>>>>> >> > >> > We should transmit secCtx for fake nodes and secSubjId for
>>>>> real.
>>>>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
>>>>> processor
>>>>> >> and
>>>>> >> > >> > validate it too if it is not null. It is important for a
>>>>> client
>>>>> >> node.
>>>>> >> > >> > For example:
>>>>> >> > >> > Into IgniteKernal#securityProcessor method createComponent
>>>>> return a
>>>>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but for
>>>>> >> clients
>>>>> >> > >> > aren't.  The clients aren't able to pass validation for this
>>>>> >> reason.
>>>>> >> > >> >
>>>>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke compatibility.
>>>>> >> > >> >
>>>>> >> > >> > I going to fix it.
>>>>> >> > >> >
>>>>> >> > >>
>>>>> >> > >
>>>>> >> >
>>>>> >>
>>>>> >
>>>>>
>>>>

[Anton Vinogradov-2]

Folks,

Could you please create a slack channel to discuss this in an effective way?

On Mon, Sep 30, 2019 at 5:36 PM Denis Garus <[hidden email]> wrote:

> >>As a result, you can't load security on demand.
>
> Why?
> What is the difference between sending SecurityContext with every job's
> request and sending SecurityContext once when remote node demand it?
>
> пн, 30 сент. 2019 г. в 17:25, Maksim Stepachev <[hidden email]
> >:
>
> > I suppose that code works only with requests are made from
> > GridRestProcessor (It isn't a client, I call it like a fake client). As a
> > result, you can't load security on demand. If you want to do it, you
> > should transmit HTTP session and backward address of a node which
> > received REST request.
> >
> > пн, 30 сент. 2019 г. в 16:16, Denis Garus <[hidden email]>:
> >
> >> >>>> Subject's size is unlimited, that can lead to a dramatic increase
> in
> >> traffic between nodes.
> >> >>I added network optimization for this case. I add a subject in the
> case
> >> when ctx.discovery().node(secSubjId) == null.
> >>
> >> Yes, this optimization is good, but we have to send SecurityContext
> >> whenever a client starts a job.
> >> Why?
> >>
> >> A better solution would be to send SecurityContext on demand.
> >>
> >> Imagine the following scenario.
> >> A client connects to node A and starts a job that runs on remote node B.
> >> IgniteSecurityProcessor on node B tries to find SecurityContext by
> >> subjectId.
> >> If IgniteSecurityProcessor fails, then it sends SecurityContextRequest
> to
> >> node A and gets required SecurityContext
> >> that afterward puts to the IgniteSecurityProcessor's cache on node B.
> >> The next request to run a job on node B with this subjectId doesn't
> >> require SecurityContext transmission.
> >>
> >> SecurityContextResponse can contain some additional information, for
> >> example,
> >> time-to-live of SecurityContext before eviction SecurityContext from the
> >> IgniteSecurityProcessor's cache.
> >>
> >> пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev <
> >> [hidden email]>:
> >>
> >>> I finished with fixes:
> >>> https://issues.apache.org/jira/browse/IGNITE-11992
> >>>
> >>> >> Subject's size is unlimited, that can lead to a dramatic increase in
> >>> traffic between nodes.
> >>> I added network optimization for this case. I add a subject in the case
> >>> when ctx.discovery().node(secSubjId) == null.
> >>>
> >>> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> >>> duplication of IgnitSecurity responsibility.
> >>> [2]Yes, we should rid of this. But in the next task, because I can't
> >>> merge it since 18 Jul 19:)
> >>>
> >>> [1] I aggry with you.
> >>>
> >>>
> >>> пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:
> >>>
> >>>> Hello, Maksim!
> >>>>
> >>>> Thank you for your effort and interest in the security of Ignite.
> >>>>
> >>>> I would like you to pay attention to the discussion [1] and issue [2].
> >>>> It looks like not only task should execute in the current security
> >>>> context but all operations too, that is essential to determine a
> security
> >>>> id for events.
> >>>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> >>>> duplication of IgnitSecurity responsibility.
> >>>> I think your task is the right place to do that.
> >>>> What is your opinion?
> >>>>
> >>>> >>It's the reason why subject id isn't enough and we should transmit
> >>>> subject inside message for this case.
> >>>> There is a problem with this approach.
> >>>> Subject's size is unlimited, that can lead to a dramatic increase in
> >>>> traffic between nodes.
> >>>>
> >>>> 1.
> >>>>
> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
> >>>> 2. https://issues.apache.org/jira/browse/IGNITE-9914
> >>>>
> >>>> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
> >>>>
> >>>>> Maksim
> >>>>>
> >>>>> >> I want to fix 2-3-4 points under one ticket.
> >>>>> Please let me know once it's become ready to be reviewed.
> >>>>>
> >>>>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
> >>>>> [hidden email]>
> >>>>> wrote:
> >>>>>
> >>>>> > Hi.
> >>>>> >
> >>>>> > Anton Vinogradov,
> >>>>> >
> >>>>> > I want to fix 2-3-4 points under one ticket.
> >>>>> >
> >>>>> > The first was fixed in the ticket:
> >>>>> > https://issues.apache.org/jira/browse/IGNITE-11094
> >>>>> > Also, I aggry with you that 5-6 isn't required to ignite.
> >>>>> >
> >>>>> > Denis Garus,
> >>>>> > I made reproducer for point 3. Looks at the test from my
> >>>>> pull-request:
> >>>>> > JettyRestPropagationSecurityContextTest
> >>>>> >
> >>>>> > https://github.com/apache/ignite/pull/6918
> >>>>> >
> >>>>> > For point 2 you should apply GridRestProcessor from pr and set
> debug
> >>>>> into
> >>>>> > VisorQueryUtils#scheduleQueryStart between
> >>>>> > ignite.context().closure().runLocalSafe  and call:
> >>>>> > ignite.context().security().securityContext()
> >>>>> >
> >>>>> >
> >>>>> > For point 3, do action above and call:
> >>>>> >
> >>>>>
> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
> >>>>> >
> >>>>> > It returns null because this subject was created from the rest.
> It's
> >>>>> the
> >>>>> > reason why subject id isn't enough and we should transmit subject
> >>>>> inside
> >>>>> > message for this case.
> >>>>> >
> >>>>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
> >>>>> >
> >>>>> >> Maksim,
> >>>>> >>
> >>>>> >> Could you please split IGNITE-11992 to subtasks with proper
> >>>>> descriptions?
> >>>>> >> This will allow us to relocate discussion to the issues to solve
> >>>>> each
> >>>>> >> problem properly.
> >>>>> >>
> >>>>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <[hidden email]
> >
> >>>>> wrote:
> >>>>> >>
> >>>>> >> > Hello, Maksim!
> >>>>> >> > Thanks for your analysis!
> >>>>> >> >
> >>>>> >> > I have a few questions about your proposals.
> >>>>> >> >
> >>>>> >> > GridRestProcessor.
> >>>>> >> > AFAIK, when GridRestProcessor handle client request
> >>>>> >> > (GridRestProcessor#handleRequest)
> >>>>> >> > it process authentication (GridRestProcessor#authenticate)
> >>>>> >> > and then authorization of request (GridRestProcessor#authorize)
> >>>>> inside
> >>>>> >> > client context.
> >>>>> >> > Can you give additional info about issues with GridRestProcessor
> >>>>> from 3
> >>>>> >> and
> >>>>> >> > 4? Maybe you have a reproducer for the problem?
> >>>>> >> >
> >>>>> >> > NoOpIgniteSecurityProcessor.
> >>>>> >> > I think the case that you describe in 5 is not a bug.
> >>>>> >> > All nodes (client and server) must have security enabled or
> >>>>> disabled.
> >>>>> >> > I can't imagine the case when it is not.
> >>>>> >> >
> >>>>> >> > ATTR_SECURITY_SUBJECT.
> >>>>> >> > I don't think that compatibility is needed here. If you will use
> >>>>> node
> >>>>> >> with
> >>>>> >> > propagation security context to remote node and older nodes
> >>>>> >> > you can get subtle errors.
> >>>>> >> >
> >>>>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
> >>>>> >> [hidden email]
> >>>>> >> > >:
> >>>>> >> >
> >>>>> >> > > Hi, Ivan.
> >>>>> >> > >
> >>>>> >> > > Yes, I have.
> >>>>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
> >>>>> >> > >
> >>>>> >> > > I'm waiting for a visa.
> >>>>> >> > >
> >>>>> >> > >
> >>>>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <
> [hidden email]
> >>>>> >:
> >>>>> >> > >
> >>>>> >> > >> Hello Max,
> >>>>> >> > >>
> >>>>> >> > >> Thanks for your analysis!
> >>>>> >> > >>
> >>>>> >> > >> Have you created a JIRA issue for discovered defects?
> >>>>> >> > >>
> >>>>> >> > >> Best Regards,
> >>>>> >> > >> Ivan Rakov
> >>>>> >> > >>
> >>>>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
> >>>>> >> > >> > Hello, Igniters.
> >>>>> >> > >> >
> >>>>> >> > >> >      The main idea of the new security is propagation
> >>>>> security
> >>>>> >> context
> >>>>> >> > >> to
> >>>>> >> > >> > other nodes and does action with initial permission. The
> >>>>> solution
> >>>>> >> > looks
> >>>>> >> > >> > fine but has imperfections.
> >>>>> >> > >> >
> >>>>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
> >>>>> itself.
> >>>>> >> > >> >    As a result: Caused by: class
> >>>>> >> > >> org.apache.ignite.spi.IgniteSpiException:
> >>>>> >> > >> > Security context isn't certain.
> >>>>> >> > >> > 2. The visor tasks lost permission.
> >>>>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new
> >>>>> thread
> >>>>> >> and
> >>>>> >> > >> loses
> >>>>> >> > >> > context.
> >>>>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
> >>>>> >> section.  As
> >>>>> >> > >> > result context loses.
> >>>>> >> > >> > 4. The GridRestProcessor isn't client, we can't read
> security
> >>>>> >> subject
> >>>>> >> > >> from
> >>>>> >> > >> > node attribute.
> >>>>> >> > >> > We should transmit secCtx for fake nodes and secSubjId for
> >>>>> real.
> >>>>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
> >>>>> processor
> >>>>> >> and
> >>>>> >> > >> > validate it too if it is not null. It is important for a
> >>>>> client
> >>>>> >> node.
> >>>>> >> > >> > For example:
> >>>>> >> > >> > Into IgniteKernal#securityProcessor method createComponent
> >>>>> return a
> >>>>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but
> for
> >>>>> >> clients
> >>>>> >> > >> > aren't.  The clients aren't able to pass validation for
> this
> >>>>> >> reason.
> >>>>> >> > >> >
> >>>>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke
> compatibility.
> >>>>> >> > >> >
> >>>>> >> > >> > I going to fix it.
> >>>>> >> > >> >
> >>>>> >> > >>
> >>>>> >> > >
> >>>>> >> >
> >>>>> >>
> >>>>> >
> >>>>>
> >>>>
>

[Denis Garus]

There is the #ignite-security Slack channel; we can use it for discussion.

вт, 1 окт. 2019 г. в 09:14, Anton Vinogradov <[hidden email]>:

> Folks,
>
> Could you please create a slack channel to discuss this in an effective
> way?
>
> On Mon, Sep 30, 2019 at 5:36 PM Denis Garus <[hidden email]> wrote:
>
> > >>As a result, you can't load security on demand.
> >
> > Why?
> > What is the difference between sending SecurityContext with every job's
> > request and sending SecurityContext once when remote node demand it?
> >
> > пн, 30 сент. 2019 г. в 17:25, Maksim Stepachev <
> [hidden email]
> > >:
> >
> > > I suppose that code works only with requests are made from
> > > GridRestProcessor (It isn't a client, I call it like a fake client).
> As a
> > > result, you can't load security on demand. If you want to do it, you
> > > should transmit HTTP session and backward address of a node which
> > > received REST request.
> > >
> > > пн, 30 сент. 2019 г. в 16:16, Denis Garus <[hidden email]>:
> > >
> > >> >>>> Subject's size is unlimited, that can lead to a dramatic increase
> > in
> > >> traffic between nodes.
> > >> >>I added network optimization for this case. I add a subject in the
> > case
> > >> when ctx.discovery().node(secSubjId) == null.
> > >>
> > >> Yes, this optimization is good, but we have to send SecurityContext
> > >> whenever a client starts a job.
> > >> Why?
> > >>
> > >> A better solution would be to send SecurityContext on demand.
> > >>
> > >> Imagine the following scenario.
> > >> A client connects to node A and starts a job that runs on remote node
> B.
> > >> IgniteSecurityProcessor on node B tries to find SecurityContext by
> > >> subjectId.
> > >> If IgniteSecurityProcessor fails, then it sends SecurityContextRequest
> > to
> > >> node A and gets required SecurityContext
> > >> that afterward puts to the IgniteSecurityProcessor's cache on node B.
> > >> The next request to run a job on node B with this subjectId doesn't
> > >> require SecurityContext transmission.
> > >>
> > >> SecurityContextResponse can contain some additional information, for
> > >> example,
> > >> time-to-live of SecurityContext before eviction SecurityContext from
> the
> > >> IgniteSecurityProcessor's cache.
> > >>
> > >> пт, 27 сент. 2019 г. в 15:20, Maksim Stepachev <
> > >> [hidden email]>:
> > >>
> > >>> I finished with fixes:
> > >>> https://issues.apache.org/jira/browse/IGNITE-11992
> > >>>
> > >>> >> Subject's size is unlimited, that can lead to a dramatic increase
> in
> > >>> traffic between nodes.
> > >>> I added network optimization for this case. I add a subject in the
> case
> > >>> when ctx.discovery().node(secSubjId) == null.
> > >>>
> > >>> >> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> > >>> duplication of IgnitSecurity responsibility.
> > >>> [2]Yes, we should rid of this. But in the next task, because I can't
> > >>> merge it since 18 Jul 19:)
> > >>>
> > >>> [1] I aggry with you.
> > >>>
> > >>>
> > >>> пт, 27 сент. 2019 г. в 11:42, Denis Garus <[hidden email]>:
> > >>>
> > >>>> Hello, Maksim!
> > >>>>
> > >>>> Thank you for your effort and interest in the security of Ignite.
> > >>>>
> > >>>> I would like you to pay attention to the discussion [1] and issue
> [2].
> > >>>> It looks like not only task should execute in the current security
> > >>>> context but all operations too, that is essential to determine a
> > security
> > >>>> id for events.
> > >>>> Also, we need to get rid of GridTaskThreadContextKey#TC_SUBJ_ID as
> > >>>> duplication of IgnitSecurity responsibility.
> > >>>> I think your task is the right place to do that.
> > >>>> What is your opinion?
> > >>>>
> > >>>> >>It's the reason why subject id isn't enough and we should transmit
> > >>>> subject inside message for this case.
> > >>>> There is a problem with this approach.
> > >>>> Subject's size is unlimited, that can lead to a dramatic increase in
> > >>>> traffic between nodes.
> > >>>>
> > >>>> 1.
> > >>>>
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/JavaDoc-for-Event-s-subjectId-methods-td43663.html
> > >>>> 2. https://issues.apache.org/jira/browse/IGNITE-9914
> > >>>>
> > >>>> пт, 27 сент. 2019 г. в 08:38, Anton Vinogradov <[hidden email]>:
> > >>>>
> > >>>>> Maksim
> > >>>>>
> > >>>>> >> I want to fix 2-3-4 points under one ticket.
> > >>>>> Please let me know once it's become ready to be reviewed.
> > >>>>>
> > >>>>> On Thu, Sep 26, 2019 at 5:18 PM Maksim Stepachev <
> > >>>>> [hidden email]>
> > >>>>> wrote:
> > >>>>>
> > >>>>> > Hi.
> > >>>>> >
> > >>>>> > Anton Vinogradov,
> > >>>>> >
> > >>>>> > I want to fix 2-3-4 points under one ticket.
> > >>>>> >
> > >>>>> > The first was fixed in the ticket:
> > >>>>> > https://issues.apache.org/jira/browse/IGNITE-11094
> > >>>>> > Also, I aggry with you that 5-6 isn't required to ignite.
> > >>>>> >
> > >>>>> > Denis Garus,
> > >>>>> > I made reproducer for point 3. Looks at the test from my
> > >>>>> pull-request:
> > >>>>> > JettyRestPropagationSecurityContextTest
> > >>>>> >
> > >>>>> > https://github.com/apache/ignite/pull/6918
> > >>>>> >
> > >>>>> > For point 2 you should apply GridRestProcessor from pr and set
> > debug
> > >>>>> into
> > >>>>> > VisorQueryUtils#scheduleQueryStart between
> > >>>>> > ignite.context().closure().runLocalSafe  and call:
> > >>>>> > ignite.context().security().securityContext()
> > >>>>> >
> > >>>>> >
> > >>>>> > For point 3, do action above and call:
> > >>>>> >
> > >>>>>
> >
> ignite.context().discovery().node(ignite.context().security().securityContext().subject().id())
> > >>>>> >
> > >>>>> > It returns null because this subject was created from the rest.
> > It's
> > >>>>> the
> > >>>>> > reason why subject id isn't enough and we should transmit subject
> > >>>>> inside
> > >>>>> > message for this case.
> > >>>>> >
> > >>>>> > чт, 18 июл. 2019 г. в 12:45, Anton Vinogradov <[hidden email]>:
> > >>>>> >
> > >>>>> >> Maksim,
> > >>>>> >>
> > >>>>> >> Could you please split IGNITE-11992 to subtasks with proper
> > >>>>> descriptions?
> > >>>>> >> This will allow us to relocate discussion to the issues to solve
> > >>>>> each
> > >>>>> >> problem properly.
> > >>>>> >>
> > >>>>> >> On Thu, Jul 18, 2019 at 11:57 AM Denis Garus <
> [hidden email]
> > >
> > >>>>> wrote:
> > >>>>> >>
> > >>>>> >> > Hello, Maksim!
> > >>>>> >> > Thanks for your analysis!
> > >>>>> >> >
> > >>>>> >> > I have a few questions about your proposals.
> > >>>>> >> >
> > >>>>> >> > GridRestProcessor.
> > >>>>> >> > AFAIK, when GridRestProcessor handle client request
> > >>>>> >> > (GridRestProcessor#handleRequest)
> > >>>>> >> > it process authentication (GridRestProcessor#authenticate)
> > >>>>> >> > and then authorization of request
> (GridRestProcessor#authorize)
> > >>>>> inside
> > >>>>> >> > client context.
> > >>>>> >> > Can you give additional info about issues with
> GridRestProcessor
> > >>>>> from 3
> > >>>>> >> and
> > >>>>> >> > 4? Maybe you have a reproducer for the problem?
> > >>>>> >> >
> > >>>>> >> > NoOpIgniteSecurityProcessor.
> > >>>>> >> > I think the case that you describe in 5 is not a bug.
> > >>>>> >> > All nodes (client and server) must have security enabled or
> > >>>>> disabled.
> > >>>>> >> > I can't imagine the case when it is not.
> > >>>>> >> >
> > >>>>> >> > ATTR_SECURITY_SUBJECT.
> > >>>>> >> > I don't think that compatibility is needed here. If you will
> use
> > >>>>> node
> > >>>>> >> with
> > >>>>> >> > propagation security context to remote node and older nodes
> > >>>>> >> > you can get subtle errors.
> > >>>>> >> >
> > >>>>> >> > чт, 18 июл. 2019 г. в 11:12, Maksim Stepachev <
> > >>>>> >> [hidden email]
> > >>>>> >> > >:
> > >>>>> >> >
> > >>>>> >> > > Hi, Ivan.
> > >>>>> >> > >
> > >>>>> >> > > Yes, I have.
> > >>>>> >> > > https://issues.apache.org/jira/browse/IGNITE-11992
> > >>>>> >> > >
> > >>>>> >> > > I'm waiting for a visa.
> > >>>>> >> > >
> > >>>>> >> > >
> > >>>>> >> > > чт, 18 июл. 2019 г. в 11:09, Ivan Rakov <
> > [hidden email]
> > >>>>> >:
> > >>>>> >> > >
> > >>>>> >> > >> Hello Max,
> > >>>>> >> > >>
> > >>>>> >> > >> Thanks for your analysis!
> > >>>>> >> > >>
> > >>>>> >> > >> Have you created a JIRA issue for discovered defects?
> > >>>>> >> > >>
> > >>>>> >> > >> Best Regards,
> > >>>>> >> > >> Ivan Rakov
> > >>>>> >> > >>
> > >>>>> >> > >> On 17.07.2019 17:08, Maksim Stepachev wrote:
> > >>>>> >> > >> > Hello, Igniters.
> > >>>>> >> > >> >
> > >>>>> >> > >> >      The main idea of the new security is propagation
> > >>>>> security
> > >>>>> >> context
> > >>>>> >> > >> to
> > >>>>> >> > >> > other nodes and does action with initial permission. The
> > >>>>> solution
> > >>>>> >> > looks
> > >>>>> >> > >> > fine but has imperfections.
> > >>>>> >> > >> >
> > >>>>> >> > >> > 1. ZookeaperDiscoveryImpl doesn't implement security into
> > >>>>> itself.
> > >>>>> >> > >> >    As a result: Caused by: class
> > >>>>> >> > >> org.apache.ignite.spi.IgniteSpiException:
> > >>>>> >> > >> > Security context isn't certain.
> > >>>>> >> > >> > 2. The visor tasks lost permission.
> > >>>>> >> > >> > The method VisorQueryUtils#scheduleQueryStart makes a new
> > >>>>> thread
> > >>>>> >> and
> > >>>>> >> > >> loses
> > >>>>> >> > >> > context.
> > >>>>> >> > >> > 3. The GridRestProcessor does tasks outside "withContext"
> > >>>>> >> section.  As
> > >>>>> >> > >> > result context loses.
> > >>>>> >> > >> > 4. The GridRestProcessor isn't client, we can't read
> > security
> > >>>>> >> subject
> > >>>>> >> > >> from
> > >>>>> >> > >> > node attribute.
> > >>>>> >> > >> > We should transmit secCtx for fake nodes and secSubjId
> for
> > >>>>> real.
> > >>>>> >> > >> > 5. NoOpIgniteSecurityProcessor should include a disabled
> > >>>>> processor
> > >>>>> >> and
> > >>>>> >> > >> > validate it too if it is not null. It is important for a
> > >>>>> client
> > >>>>> >> node.
> > >>>>> >> > >> > For example:
> > >>>>> >> > >> > Into IgniteKernal#securityProcessor method
> createComponent
> > >>>>> return a
> > >>>>> >> > >> > GridSecurityProcessor. For server nodes are enabled, but
> > for
> > >>>>> >> clients
> > >>>>> >> > >> > aren't.  The clients aren't able to pass validation for
> > this
> > >>>>> >> reason.
> > >>>>> >> > >> >
> > >>>>> >> > >> > 6. ATTR_SECURITY_SUBJECT was removed. It broke
> > compatibility.
> > >>>>> >> > >> >
> > >>>>> >> > >> > I going to fix it.
> > >>>>> >> > >> >
> > >>>>> >> > >>
> > >>>>> >> > >
> > >>>>> >> >
> > >>>>> >>
> > >>>>> >
> > >>>>>
> > >>>>
> >
>