ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Denis Garus <garus....@gmail.com>
Subject Re: Improvements for new security approach.
Date Mon, 30 Sep 2019 13:16:47 GMT
>>>> 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 <maksim.stepachev@gmail.com>:

> 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 <garus.d.g@gmail.com>:
>
>> 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 <av@apache.org>:
>>
>>> 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 <
>>> maksim.stepachev@gmail.com>
>>> 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 <av@apache.org>:
>>> >
>>> >> 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 <garus.d.g@gmail.com>
>>> 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 <
>>> >> maksim.stepachev@gmail.com
>>> >> > >:
>>> >> >
>>> >> > > 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 <ivan.glukos@gmail.com>:
>>> >> > >
>>> >> > >> 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.
>>> >> > >> >
>>> >> > >>
>>> >> > >
>>> >> >
>>> >>
>>> >
>>>
>>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message