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