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....@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.stepac...@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....@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.stepac...@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....@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 <a...@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.stepac...@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 <a...@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....@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.stepac...@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.glu...@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. > >>>>> >> > >> > > >>>>> >> > >> > >>>>> >> > > > >>>>> >> > > >>>>> >> > >>>>> > > >>>>> > >>>> >
