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. >> >> > >> > >> >> > >> >> >> > > >> >> > >> >> >> > >> >
