ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anton Vinogradov ...@apache.org>
Subject Re: Improvements for new security approach.
Date Tue, 01 Oct 2019 06:14:35 GMT
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 <garus.d.g@gmail.com> 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 <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