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 14:35:45 GMT
>>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 <maksim.stepachev@gmail.com>:

> 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 <garus.d.g@gmail.com>:
>
>> >>>> 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