There is the #ignite-security Slack channel; we can use it for discussion. вт, 1 окт. 2019 г. в 09:14, Anton Vinogradov <a...@apache.org>:
> 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. > > >>>>> >> > >> > > > >>>>> >> > >> > > >>>>> >> > > > > >>>>> >> > > > >>>>> >> > > >>>>> > > > >>>>> > > >>>> > > >
