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