You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Denis Garus <ga...@gmail.com> on 2020/02/20 12:55:55 UTC
Security Subject of thin client on remote nodes
Hi, Igniters!
At present, a security subject id is assumed to be node id.
But when we are dealing with thin client, JDBC or REST subject id is random
UUID. In this case, we cannot get the subject information on a remote node,
and we get problems like these [1], [2].
To fix the problem, we should spread the client session to the whole
cluster.
I want to suggest a solution to the problem.
First, we should get subject information using GridSecurityProcessor.
How GridSecurityProcessor will retrieve a subject data, it is up to plugin
developers.
Second, we should get rid of the assumption that a subject id is a node id
and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
I have prepared PoC PR [3] that:
- places the existing logic of spreading security context to
GridSecurityProcessor;
- uses GridSecurityProcessor to get SecurityContext.
1.
http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
2. https://issues.apache.org/jira/browse/IGNITE-12589
3. https://github.com/apache/ignite/pull/7375
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Ivan, thank you very much for the review and proposals!
I've made more descriptive JavaDoc; take a look, please.
> Please think about driving creating IEP on security API overhaul prior to
2.9.
Yes, I'll create IEP on security API soon. I also have some ideas about how
we can improve Ignite's security.
Thank you!
пт, 27 мар. 2020 г. в 11:41, Ivan Rakov <iv...@gmail.com>:
> Denis,
>
> In general, code changes look good to me. If we decide to keep security API
> in its current state for a while, I highly recommend to extend its
> documentation. We don't have descriptive javadocs or articles about
> security API so far, so I expect that next contributors will face
> difficulties in untangling security logic. Let's help them a bit.
> See more details in my JIRA comment:
> https://issues.apache.org/jira/browse/IGNITE-12759
>
> On Thu, Mar 26, 2020 at 5:54 PM Ivan Rakov <iv...@gmail.com> wrote:
>
> > Denis,
> >
> > I'll review your PR. If this issue is a subject to be included in 2.8.1
> in
> > emergency mode, I'm ok with the current API changes.
> > Please think about driving creating IEP on security API overhaul prior to
> > 2.9. I believe that you are the most suitable Ignite community member to
> > drive this activity. I'd love to share some ideas as well.
> >
> > On Tue, Mar 24, 2020 at 2:04 PM Denis Garus <ga...@gmail.com> wrote:
> >
> >> Hi, guys!
> >>
> >>
> >> I agree that we should rework the security API, but it can take a long
> >> time.
> >>
> >> And currently, our users have certain impediments that are blockers for
> >> their job.
> >>
> >> I think we have to fix bugs that IEP-41 [1] contains as soon as possible
> >> to
> >> support our users.
> >>
> >> From my point of view, IEP-41 is the best place to track bug fixing.
> >>
> >>
> >>
> >> 1.
> >>
> >>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes
> >>
> >>
> >> вт, 24 мар. 2020 г. в 12:26, Ivan Rakov <iv...@gmail.com>:
> >>
> >> > Alexey,
> >> >
> >> > That can be another version of our plan. If everyone agrees that
> >> > SecurityContext and SecuritySubject should be merged, such fix of thin
> >> > clients' issue will bring us closer to the final solution.
> >> > Denis, what do you think?
> >> >
> >> > On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
> >> > alexey.scherbakoff@gmail.com> wrote:
> >> >
> >> > > Why can't we start gradually changing security API right now ?
> >> > > I see no point in delaying with.
> >> > > All changes will go to next 2.9 release anyway.
> >> > >
> >> > > My proposal:
> >> > > 1. Get rid of security context. Doing this will bring security API
> to
> >> > more
> >> > > or less consistent state.
> >> > > 2. Remove IEP-41 because it's no longer needed because of change [1]
> >> > > 3. Propose an IEP to make security API avoid using internals.
> >> > >
> >> > >
> >> > >
> >> > > пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
> >> > >
> >> > > > Hello, Alexei, Ivan!
> >> > > >
> >> > > > >> Seems like security API is indeed a bit over-engineered
> >> > > >
> >> > > > Nobody has doubt we should do a reworking of
> GridSecurityProcessor.
> >> > > > But this point is outside of scope thin client's problem that we
> are
> >> > > > solving.
> >> > > > I think we can create new IEP that will accumulate all ideas of
> >> > Ignite's
> >> > > > security improvements.
> >> > > >
> >> > > > >> Presence of the separate #securityContext(UUID) highlights that
> >> user
> >> > > > indeed should care
> >> > > > >> about propagation of thin clients' contexts between the cluster
> >> > nodes.
> >> > > >
> >> > > > I agree with Ivan. I've implemented both variants,
> >> > > > and I like one with #securityContext(UUID) more.
> >> > > >
> >> > > > Could you please take a look at PR [1] for the issue [2]?
> >> > > >
> >> > > > 1. https://github.com/apache/ignite/pull/7523
> >> > > > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> >> > > >
> >> > > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
> >> > > >
> >> > > > > Alex, Denis,
> >> > > > >
> >> > > > > Seems like security API is indeed a bit over-engineered.
> >> > > > >
> >> > > > > Let's get rid of SecurityContext and use SecuritySubject
> instead.
> >> > > > > > SecurityContext is just a POJO wrapper over
> >> > > > > > SecuritySubject's
> >> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> >> > > > > > It's functionality can be easily moved to SecuritySubject.
> >> > > > >
> >> > > > > I totally agree. Both subject and context are implemented by
> >> plugin
> >> > > > > provider, and I don't see any reason to keep both abstractions,
> >> > > > especially
> >> > > > > if we are going to get rid of transferring subject in node
> >> attributes
> >> > > > > (argument that subject is more lightweight won't work anymore).
> >> > > > >
> >> > > > > Also, there's kind of mess in node authentication logic. There
> >> are at
> >> > > > least
> >> > > > > two components responsible for it: DiscoverySpiNodeAuthenticator
> >> > (which
> >> > > > is
> >> > > > > forcibly set by GridDiscoveryManager, but in fact public) and
> >> > > > > GridSecurityProcessor (which performs actual node auth logic,
> but
> >> > > > private).
> >> > > > > I also don't understand why we need both
> >> > > > > #authenticate(AuthenticationContext) and
> >> > #authenticateNode(ClusterNode,
> >> > > > > SecurityCredentials) methods while it's possible to set explicit
> >> > > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this
> is
> >> > > > arguable;
> >> > > > > perhaps there are strong reasons).
> >> > > > >
> >> > > > > Finally, areas of responsibility between IgniteSecurity and
> >> > > > > GridSecurityProcessor are kind of mixed. As far as I understand,
> >> the
> >> > > > first
> >> > > > > is responsible for Ignite-internal management of security logic
> >> > > (keeping
> >> > > > > thread-local context, caching security contexts, etc; we don't
> >> expect
> >> > > > > IgniteSecurity to be replaced by plugin provider) and the latter
> >> is
> >> > > > > responsible for user-custom authentication / authorization
> logic.
> >> To
> >> > be
> >> > > > > honest, it took plenty of time to figure this out for me.
> >> > > > >
> >> > > > > From my point of view, we should make GridSecurityProcessor
> >> interface
> >> > > > > public, rename it (it requires plenty of time to find the
> >> difference
> >> > > from
> >> > > > > IgniteSecurity), make its API as simple and non-duplicating as
> >> > possible
> >> > > > and
> >> > > > > clarify its area of responsibility (e.g. should it be
> responsible
> >> for
> >> > > > > propagation of successfully authenticated subject among all
> nodes
> >> or
> >> > > > not?)
> >> > > > > to make it easy to embed custom security logic in Ignite.
> >> > > > >
> >> > > > > Regarding thin clients fix: implementation made by Denis suits
> >> better
> >> > > to
> >> > > > > the very implicit contract that it's better to change API
> >> contracts
> >> > of
> >> > > an
> >> > > > > internal IgniteSecurity than of internal GridSecurityProcessor
> >> (which
> >> > > > > actually mustn't be internal).
> >> > > > >
> >> > > > > > My approach doesn't require any IEPs, just minor change in
> code
> >> and
> >> > > to
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> >> > > > > > contract.
> >> > > > >
> >> > > > > Looks like a misuse of #authenticate method to me. It should
> >> perform
> >> > > > > initial authentication based on credentials (this may include
> >> queries
> >> > > to
> >> > > > > external authentication subsystem, e.g. LDAP). User may want to
> >> don't
> >> > > > > authenticate thin client on every node (this will increase the
> >> number
> >> > > of
> >> > > > > requests to auth subsystem unless user implicitly implements
> >> > > propagation
> >> > > > of
> >> > > > > thin clients' contexts between nodes and make #authenticate
> >> > > cluster-wide
> >> > > > > idempotent: first call should perform actual authentication,
> next
> >> > calls
> >> > > > > should retrieve context of already authenticated client).
> >> Presence of
> >> > > the
> >> > > > > separate #securityContext(UUID) highlights that user indeed
> should
> >> > care
> >> > > > > about propagation of thin clients' contexts between the cluster
> >> > nodes.
> >> > > > >
> >> > > > > --
> >> > > > > Ivan
> >> > > > >
> >> > > > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <
> >> > > V.Mithare@cmcmarkets.com
> >> > > > >
> >> > > > > wrote:
> >> > > > >
> >> > > > > > Hi Alexei, Denis,
> >> > > > > >
> >> > > > > > One of the main usecases of thin client authentication is to
> be
> >> > able
> >> > > to
> >> > > > > > audit the changes done using the thin client user.
> >> > > > > > To enable that :
> >> > > > > > We really need to resolve this concern as well :
> >> > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> >> > > > > >
> >> > > > > > ( Incorrect security subject id is associated with a
> cache_put
> >> > event
> >> > > > > > when the originator of the event is a thin client. )
> >> > > > > >
> >> > > > > > Regards,
> >> > > > > > Veena
> >> > > > > >
> >> > > > > >
> >> > > > > > -----Original Message-----
> >> > > > > > From: Alexei Scherbakov <al...@gmail.com>
> >> > > > > > Sent: 18 March 2020 08:11
> >> > > > > > To: dev <de...@ignite.apache.org>
> >> > > > > > Subject: Re: Security Subject of thin client on remote nodes
> >> > > > > >
> >> > > > > > Denis Garus,
> >> > > > > >
> >> > > > > > Both variants are capable of solving the thin client security
> >> > context
> >> > > > > > problem.
> >> > > > > >
> >> > > > > > My approach doesn't require any IEPs, just minor change in
> code
> >> and
> >> > > to
> >> > > > > >
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> >> > > > > > contract.
> >> > > > > > We can add appropriate documentation to emphasize this.
> >> > > > > > The argument "fragile" is not very convincing for me.
> >> > > > > >
> >> > > > > > I think we should collect more opinions before proceeding with
> >> IEP.
> >> > > > > >
> >> > > > > > Considering a fact we actually *may not care* about
> >> compatibility
> >> > > (I've
> >> > > > > > already explained why), I'm thinking of another approach.
> >> > > > > > Let's get rid of SecurityContext and use SecuritySubject
> >> instead.
> >> > > > > > SecurityContext is just a POJO wrapper over SecuritySubject's
> >> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> >> > > > > > It's functionality can be easily moved to SecuritySubject.
> >> > > > > >
> >> > > > > > What do you think?
> >> > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <garus.d.g@gmail.com
> >:
> >> > > > > >
> >> > > > > > > Hello, Alexei!
> >> > > > > > >
> >> > > > > > > I agree with you if we may not care about compatibility at
> >> all,
> >> > > then
> >> > > > > > > we can solve the problem much more straightforward way.
> >> > > > > > >
> >> > > > > > > In your case, the method GridSecurityProcessor#authenticate
> >> will
> >> > > have
> >> > > > > > > an implicit contract:
> >> > > > > > > [ if actx.subject() != null then
> >> > > > > > > returns SecurityContext
> >> > > > > > > else
> >> > > > > > > do authenticate ]
> >> > > > > > >
> >> > > > > > > It looks fragile.
> >> > > > > > >
> >> > > > > > > When we extend the GridSecurityProcessor, there isn't this
> >> > problem:
> >> > > > > > > we have the explicit contract and can make default
> >> implementation
> >> > > > that
> >> > > > > > > throws an unsupported operation exception to enforcing
> >> > > compatibility
> >> > > > > > > check.
> >> > > > > > >
> >> > > > > > > In any case, we need to change GridSecurityProcessor
> >> > > implementation.
> >> > > > > > >
> >> > > > > > > But I think your proposal to try to find a security context
> in
> >> > the
> >> > > > > > > node's attributes first is right for backward compatibility
> >> when
> >> > > > > > > Ignite users don't use thin clients.
> >> > > > > > >
> >> > > > > > > Summary:
> >> > > > > > > I suggest adding a new method to GridSecurityProcessor
> >> because it
> >> > > has
> >> > > > > > > a clear contract and enforces compatibility check natural
> way.
> >> > > > > > >
> >> > > > > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> >> > > > > > > alexey.scherbakoff@gmail.com
> >> > > > > > > >:
> >> > > > > > >
> >> > > > > > > > Denis Garus,
> >> > > > > > > >
> >> > > > > > > > I've looked at the IEP proposed by you and currently I'm
> >> > thinking
> >> > > > > > > > it's
> >> > > > > > > not
> >> > > > > > > > immediately required.
> >> > > > > > > >
> >> > > > > > > > The problem of missing SecurityContexts of thin clients
> can
> >> be
> >> > > > > > > > solved
> >> > > > > > > much
> >> > > > > > > > easily.
> >> > > > > > > >
> >> > > > > > > > Below is the stub of a fix, it requires correct
> >> implementation
> >> > of
> >> > > > > > > > method
> >> > > > > > > >
> >> > > > > > >
> >> > > >
> >> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> >> > > > > > > #authenticatedSubject
> >> > > > > > > > by GridSecurityProcessor:
> >> > > > > > > >
> >> > > > > > > > /** {@inheritDoc} */
> >> > > > > > > > @Override public OperationSecurityContext
> >> withContext(UUID
> >> > > > > nodeId)
> >> > > > > > {
> >> > > > > > > > try {
> >> > > > > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> >> > > > > > > >
> >> > > > > > > > if (ctx0 == null) {
> >> > > > > > > > ClusterNode node =
> >> > > > > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> >> > > > > > > > .orElseGet(() ->
> >> > > > > > > > ctx.discovery().historicalNode(nodeId));
> >> > > > > > > >
> >> > > > > > > > // This is a cluster node.
> >> > > > > > > > if (node != null)
> >> > > > > > > > ctx0 = nodeSecurityContext(marsh,
> >> > > > > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> >> > > > > > > > else {
> >> > > > > > > > // This is already authenticated thin
> >> > client.
> >> > > > > > > > SecuritySubject subj =
> >> > > > > > > > authenticatedSubject(nodeId);
> >> > > > > > > >
> >> > > > > > > > assert subj != null : "Subject is null
> >> " +
> >> > > > > > > > nodeId;
> >> > > > > > > >
> >> > > > > > > > AuthenticationContext actx = new
> >> > > > > > > > AuthenticationContext();
> >> > > > > > > > actx.subject(subj);
> >> > > > > > > >
> >> > > > > > > > ctx0 = secPrc.authenticate(actx);
> >> > > > > > > > }
> >> > > > > > > > }
> >> > > > > > > >
> >> > > > > > > > secCtxs.putIfAbsent(nodeId, ctx0);
> >> > > > > > > >
> >> > > > > > > > return withContext(ctx0);
> >> > > > > > > > } catch (IgniteCheckedException e) {
> >> > > > > > > > throw new IgniteException(e);
> >> > > > > > > > }
> >> > > > > > > >
> >> > > > > > > > The idea is to create a thin client SecurityContext on a
> >> node
> >> > not
> >> > > > > > > > having
> >> > > > > > > a
> >> > > > > > > > local context using existing SecuritySubject data.
> >> > > > > > > >
> >> > > > > > > > Method
> >> > > > > > > >
> >> > > > > > >
> >> > > >
> >> org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> >> > > > > > > uthenticate
> >> > > > > > > > should check for not null SecuritySubject field and just
> >> > recreate
> >> > > > > > > > SecurityContext using passed info (because it's already
> >> > > > > authenticated).
> >> > > > > > > >
> >> > > > > > > > We have all necessary information in SecuritySubject
> >> returned
> >> > by
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > >
> >> > > >
> >> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> >> > > > > > > #authenticatedSubject
> >> > > > > > > > by GridSecurityProcessor method.
> >> > > > > > > >
> >> > > > > > > > Because it is internal API, we may not care about
> >> > compatibility
> >> > > at
> >> > > > > > > > all, but nevertheless it is possible to add compatibility
> >> check
> >> > > in
> >> > > > > > > > the method above. If a feature is not supported the
> >> operations
> >> > > from
> >> > > > > > > > thin clients should be forbidden.
> >> > > > > > > >
> >> > > > > > > > You proposal has the similar problem: if
> >> GridSecurityProcessor
> >> > > does
> >> > > > > > > > not support retriving context for thin clients, such
> clients
> >> > will
> >> > > > > > > > not be able to proceed with operation.
> >> > > > > > > >
> >> > > > > > > > Still, the cleanup of security API is necessary and should
> >> be
> >> > > done
> >> > > > > > > > in 3.0
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
> >> > > > v.mithare@cmcmarkets.com
> >> > > > > >:
> >> > > > > > > >
> >> > > > > > > > > HI ,
> >> > > > > > > > >
> >> > > > > > > > > Created this jira :
> >> > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> >> > > > > > > > >
> >> > > > > > > > > regards,
> >> > > > > > > > > Veena.
> >> > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > > >
> >> > > > > > > > > --
> >> > > > > > > > > Sent from:
> >> > > > http://apache-ignite-developers.2346864.n4.nabble.com/
> >> > > > > > > > >
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > --
> >> > > > > > > >
> >> > > > > > > > Best regards,
> >> > > > > > > > Alexei Scherbakov
> >> > > > > > > >
> >> > > > > > >
> >> > > > > >
> >> > > > > >
> >> > > > > > --
> >> > > > > >
> >> > > > > > Best regards,
> >> > > > > > Alexei Scherbakov
> >> > > > > > ________________________________
> >> > > > > >
> >> > > > > >
> >> > > > > > Spread bets and CFDs are complex instruments and come with a
> >> high
> >> > > risk
> >> > > > of
> >> > > > > > losing money rapidly due to leverage. 70.5% of retail investor
> >> > > accounts
> >> > > > > > lose money when spread betting and/or trading CFDs with this
> >> > > provider.
> >> > > > > You
> >> > > > > > should consider whether you understand how spread bets and
> CFDs
> >> > work
> >> > > > and
> >> > > > > > whether you can afford to take the high risk of losing your
> >> money.
> >> > > > > >
> >> > > > > > Professional clients: Losses can exceed deposits when spread
> >> > betting
> >> > > > and
> >> > > > > > trading CFDs. Countdowns carry a level of risk to your capital
> >> as
> >> > you
> >> > > > > could
> >> > > > > > lose all of your investment. These products may not be
> suitable
> >> for
> >> > > all
> >> > > > > > clients therefore ensure you understand the risks and seek
> >> > > independent
> >> > > > > > advice. Invest only what you can afford to lose.
> >> > > > > >
> >> > > > > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
> >> > > > authorised
> >> > > > > > and regulated by the Financial Conduct Authority in the United
> >> > > Kingdom.
> >> > > > > CMC
> >> > > > > > Markets UK plc and CMC Spreadbet plc are registered in England
> >> and
> >> > > > Wales
> >> > > > > > with Company Numbers 02448409 and 02589529 and with their
> >> > registered
> >> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> >> > > > > >
> >> > > > > > The content of this e-mail (including any attachments) is
> >> strictly
> >> > > > > > confidential and is for the sole use of the intended
> >> addressee(s).
> >> > If
> >> > > > you
> >> > > > > > are not the intended recipient of this e-mail please notify
> the
> >> > > sender
> >> > > > > > immediately and delete this e-mail from your system. Any
> >> > disclosure,
> >> > > > > > copying, dissemination or use of its content (including any
> >> > > > attachments)
> >> > > > > is
> >> > > > > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc
> >> > reserve
> >> > > > the
> >> > > > > > right to intercept and monitor the content of the e-mail
> >> messages
> >> > to
> >> > > > and
> >> > > > > > from its systems.
> >> > > > > >
> >> > > > > > E-mails may be interfered with or may contain viruses or other
> >> > > defects
> >> > > > > for
> >> > > > > > which CMC Markets UK plc and CMC Spreadbet plc accept no
> >> > > > responsibility.
> >> > > > > It
> >> > > > > > is the responsibility of the recipient to carry out a virus
> >> check
> >> > on
> >> > > > the
> >> > > > > > e-mail and any attachment(s).
> >> > > > > >
> >> > > > > > This communication is not intended as an offer or solicitation
> >> for
> >> > > the
> >> > > > > > purchase or sale of a financial instrument or as an official
> >> > > > confirmation
> >> > > > > > of any transaction unless specifically presented as such.
> >> > > > > >
> >> > > > > > CMC Markets UK plc and CMC Spreadbet plc are registered in
> >> England
> >> > > and
> >> > > > > > Wales with Company Numbers 02448409 and 02589529 and with
> their
> >> > > > > registered
> >> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> >> > > > > >
> >> > > > >
> >> > > >
> >> > >
> >> > >
> >> > > --
> >> > >
> >> > > Best regards,
> >> > > Alexei Scherbakov
> >> > >
> >> >
> >>
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Ivan Rakov <iv...@gmail.com>.
Denis,
In general, code changes look good to me. If we decide to keep security API
in its current state for a while, I highly recommend to extend its
documentation. We don't have descriptive javadocs or articles about
security API so far, so I expect that next contributors will face
difficulties in untangling security logic. Let's help them a bit.
See more details in my JIRA comment:
https://issues.apache.org/jira/browse/IGNITE-12759
On Thu, Mar 26, 2020 at 5:54 PM Ivan Rakov <iv...@gmail.com> wrote:
> Denis,
>
> I'll review your PR. If this issue is a subject to be included in 2.8.1 in
> emergency mode, I'm ok with the current API changes.
> Please think about driving creating IEP on security API overhaul prior to
> 2.9. I believe that you are the most suitable Ignite community member to
> drive this activity. I'd love to share some ideas as well.
>
> On Tue, Mar 24, 2020 at 2:04 PM Denis Garus <ga...@gmail.com> wrote:
>
>> Hi, guys!
>>
>>
>> I agree that we should rework the security API, but it can take a long
>> time.
>>
>> And currently, our users have certain impediments that are blockers for
>> their job.
>>
>> I think we have to fix bugs that IEP-41 [1] contains as soon as possible
>> to
>> support our users.
>>
>> From my point of view, IEP-41 is the best place to track bug fixing.
>>
>>
>>
>> 1.
>>
>> https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes
>>
>>
>> вт, 24 мар. 2020 г. в 12:26, Ivan Rakov <iv...@gmail.com>:
>>
>> > Alexey,
>> >
>> > That can be another version of our plan. If everyone agrees that
>> > SecurityContext and SecuritySubject should be merged, such fix of thin
>> > clients' issue will bring us closer to the final solution.
>> > Denis, what do you think?
>> >
>> > On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
>> > alexey.scherbakoff@gmail.com> wrote:
>> >
>> > > Why can't we start gradually changing security API right now ?
>> > > I see no point in delaying with.
>> > > All changes will go to next 2.9 release anyway.
>> > >
>> > > My proposal:
>> > > 1. Get rid of security context. Doing this will bring security API to
>> > more
>> > > or less consistent state.
>> > > 2. Remove IEP-41 because it's no longer needed because of change [1]
>> > > 3. Propose an IEP to make security API avoid using internals.
>> > >
>> > >
>> > >
>> > > пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
>> > >
>> > > > Hello, Alexei, Ivan!
>> > > >
>> > > > >> Seems like security API is indeed a bit over-engineered
>> > > >
>> > > > Nobody has doubt we should do a reworking of GridSecurityProcessor.
>> > > > But this point is outside of scope thin client's problem that we are
>> > > > solving.
>> > > > I think we can create new IEP that will accumulate all ideas of
>> > Ignite's
>> > > > security improvements.
>> > > >
>> > > > >> Presence of the separate #securityContext(UUID) highlights that
>> user
>> > > > indeed should care
>> > > > >> about propagation of thin clients' contexts between the cluster
>> > nodes.
>> > > >
>> > > > I agree with Ivan. I've implemented both variants,
>> > > > and I like one with #securityContext(UUID) more.
>> > > >
>> > > > Could you please take a look at PR [1] for the issue [2]?
>> > > >
>> > > > 1. https://github.com/apache/ignite/pull/7523
>> > > > 2. https://issues.apache.org/jira/browse/IGNITE-12759
>> > > >
>> > > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
>> > > >
>> > > > > Alex, Denis,
>> > > > >
>> > > > > Seems like security API is indeed a bit over-engineered.
>> > > > >
>> > > > > Let's get rid of SecurityContext and use SecuritySubject instead.
>> > > > > > SecurityContext is just a POJO wrapper over
>> > > > > > SecuritySubject's
>> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
>> > > > > > It's functionality can be easily moved to SecuritySubject.
>> > > > >
>> > > > > I totally agree. Both subject and context are implemented by
>> plugin
>> > > > > provider, and I don't see any reason to keep both abstractions,
>> > > > especially
>> > > > > if we are going to get rid of transferring subject in node
>> attributes
>> > > > > (argument that subject is more lightweight won't work anymore).
>> > > > >
>> > > > > Also, there's kind of mess in node authentication logic. There
>> are at
>> > > > least
>> > > > > two components responsible for it: DiscoverySpiNodeAuthenticator
>> > (which
>> > > > is
>> > > > > forcibly set by GridDiscoveryManager, but in fact public) and
>> > > > > GridSecurityProcessor (which performs actual node auth logic, but
>> > > > private).
>> > > > > I also don't understand why we need both
>> > > > > #authenticate(AuthenticationContext) and
>> > #authenticateNode(ClusterNode,
>> > > > > SecurityCredentials) methods while it's possible to set explicit
>> > > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
>> > > > arguable;
>> > > > > perhaps there are strong reasons).
>> > > > >
>> > > > > Finally, areas of responsibility between IgniteSecurity and
>> > > > > GridSecurityProcessor are kind of mixed. As far as I understand,
>> the
>> > > > first
>> > > > > is responsible for Ignite-internal management of security logic
>> > > (keeping
>> > > > > thread-local context, caching security contexts, etc; we don't
>> expect
>> > > > > IgniteSecurity to be replaced by plugin provider) and the latter
>> is
>> > > > > responsible for user-custom authentication / authorization logic.
>> To
>> > be
>> > > > > honest, it took plenty of time to figure this out for me.
>> > > > >
>> > > > > From my point of view, we should make GridSecurityProcessor
>> interface
>> > > > > public, rename it (it requires plenty of time to find the
>> difference
>> > > from
>> > > > > IgniteSecurity), make its API as simple and non-duplicating as
>> > possible
>> > > > and
>> > > > > clarify its area of responsibility (e.g. should it be responsible
>> for
>> > > > > propagation of successfully authenticated subject among all nodes
>> or
>> > > > not?)
>> > > > > to make it easy to embed custom security logic in Ignite.
>> > > > >
>> > > > > Regarding thin clients fix: implementation made by Denis suits
>> better
>> > > to
>> > > > > the very implicit contract that it's better to change API
>> contracts
>> > of
>> > > an
>> > > > > internal IgniteSecurity than of internal GridSecurityProcessor
>> (which
>> > > > > actually mustn't be internal).
>> > > > >
>> > > > > > My approach doesn't require any IEPs, just minor change in code
>> and
>> > > to
>> > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
>> > > > > > contract.
>> > > > >
>> > > > > Looks like a misuse of #authenticate method to me. It should
>> perform
>> > > > > initial authentication based on credentials (this may include
>> queries
>> > > to
>> > > > > external authentication subsystem, e.g. LDAP). User may want to
>> don't
>> > > > > authenticate thin client on every node (this will increase the
>> number
>> > > of
>> > > > > requests to auth subsystem unless user implicitly implements
>> > > propagation
>> > > > of
>> > > > > thin clients' contexts between nodes and make #authenticate
>> > > cluster-wide
>> > > > > idempotent: first call should perform actual authentication, next
>> > calls
>> > > > > should retrieve context of already authenticated client).
>> Presence of
>> > > the
>> > > > > separate #securityContext(UUID) highlights that user indeed should
>> > care
>> > > > > about propagation of thin clients' contexts between the cluster
>> > nodes.
>> > > > >
>> > > > > --
>> > > > > Ivan
>> > > > >
>> > > > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <
>> > > V.Mithare@cmcmarkets.com
>> > > > >
>> > > > > wrote:
>> > > > >
>> > > > > > Hi Alexei, Denis,
>> > > > > >
>> > > > > > One of the main usecases of thin client authentication is to be
>> > able
>> > > to
>> > > > > > audit the changes done using the thin client user.
>> > > > > > To enable that :
>> > > > > > We really need to resolve this concern as well :
>> > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
>> > > > > >
>> > > > > > ( Incorrect security subject id is associated with a cache_put
>> > event
>> > > > > > when the originator of the event is a thin client. )
>> > > > > >
>> > > > > > Regards,
>> > > > > > Veena
>> > > > > >
>> > > > > >
>> > > > > > -----Original Message-----
>> > > > > > From: Alexei Scherbakov <al...@gmail.com>
>> > > > > > Sent: 18 March 2020 08:11
>> > > > > > To: dev <de...@ignite.apache.org>
>> > > > > > Subject: Re: Security Subject of thin client on remote nodes
>> > > > > >
>> > > > > > Denis Garus,
>> > > > > >
>> > > > > > Both variants are capable of solving the thin client security
>> > context
>> > > > > > problem.
>> > > > > >
>> > > > > > My approach doesn't require any IEPs, just minor change in code
>> and
>> > > to
>> > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
>> > > > > > contract.
>> > > > > > We can add appropriate documentation to emphasize this.
>> > > > > > The argument "fragile" is not very convincing for me.
>> > > > > >
>> > > > > > I think we should collect more opinions before proceeding with
>> IEP.
>> > > > > >
>> > > > > > Considering a fact we actually *may not care* about
>> compatibility
>> > > (I've
>> > > > > > already explained why), I'm thinking of another approach.
>> > > > > > Let's get rid of SecurityContext and use SecuritySubject
>> instead.
>> > > > > > SecurityContext is just a POJO wrapper over SecuritySubject's
>> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
>> > > > > > It's functionality can be easily moved to SecuritySubject.
>> > > > > >
>> > > > > > What do you think?
>> > > > > >
>> > > > > >
>> > > > > >
>> > > > > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
>> > > > > >
>> > > > > > > Hello, Alexei!
>> > > > > > >
>> > > > > > > I agree with you if we may not care about compatibility at
>> all,
>> > > then
>> > > > > > > we can solve the problem much more straightforward way.
>> > > > > > >
>> > > > > > > In your case, the method GridSecurityProcessor#authenticate
>> will
>> > > have
>> > > > > > > an implicit contract:
>> > > > > > > [ if actx.subject() != null then
>> > > > > > > returns SecurityContext
>> > > > > > > else
>> > > > > > > do authenticate ]
>> > > > > > >
>> > > > > > > It looks fragile.
>> > > > > > >
>> > > > > > > When we extend the GridSecurityProcessor, there isn't this
>> > problem:
>> > > > > > > we have the explicit contract and can make default
>> implementation
>> > > > that
>> > > > > > > throws an unsupported operation exception to enforcing
>> > > compatibility
>> > > > > > > check.
>> > > > > > >
>> > > > > > > In any case, we need to change GridSecurityProcessor
>> > > implementation.
>> > > > > > >
>> > > > > > > But I think your proposal to try to find a security context in
>> > the
>> > > > > > > node's attributes first is right for backward compatibility
>> when
>> > > > > > > Ignite users don't use thin clients.
>> > > > > > >
>> > > > > > > Summary:
>> > > > > > > I suggest adding a new method to GridSecurityProcessor
>> because it
>> > > has
>> > > > > > > a clear contract and enforces compatibility check natural way.
>> > > > > > >
>> > > > > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
>> > > > > > > alexey.scherbakoff@gmail.com
>> > > > > > > >:
>> > > > > > >
>> > > > > > > > Denis Garus,
>> > > > > > > >
>> > > > > > > > I've looked at the IEP proposed by you and currently I'm
>> > thinking
>> > > > > > > > it's
>> > > > > > > not
>> > > > > > > > immediately required.
>> > > > > > > >
>> > > > > > > > The problem of missing SecurityContexts of thin clients can
>> be
>> > > > > > > > solved
>> > > > > > > much
>> > > > > > > > easily.
>> > > > > > > >
>> > > > > > > > Below is the stub of a fix, it requires correct
>> implementation
>> > of
>> > > > > > > > method
>> > > > > > > >
>> > > > > > >
>> > > >
>> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
>> > > > > > > #authenticatedSubject
>> > > > > > > > by GridSecurityProcessor:
>> > > > > > > >
>> > > > > > > > /** {@inheritDoc} */
>> > > > > > > > @Override public OperationSecurityContext
>> withContext(UUID
>> > > > > nodeId)
>> > > > > > {
>> > > > > > > > try {
>> > > > > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
>> > > > > > > >
>> > > > > > > > if (ctx0 == null) {
>> > > > > > > > ClusterNode node =
>> > > > > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
>> > > > > > > > .orElseGet(() ->
>> > > > > > > > ctx.discovery().historicalNode(nodeId));
>> > > > > > > >
>> > > > > > > > // This is a cluster node.
>> > > > > > > > if (node != null)
>> > > > > > > > ctx0 = nodeSecurityContext(marsh,
>> > > > > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
>> > > > > > > > else {
>> > > > > > > > // This is already authenticated thin
>> > client.
>> > > > > > > > SecuritySubject subj =
>> > > > > > > > authenticatedSubject(nodeId);
>> > > > > > > >
>> > > > > > > > assert subj != null : "Subject is null
>> " +
>> > > > > > > > nodeId;
>> > > > > > > >
>> > > > > > > > AuthenticationContext actx = new
>> > > > > > > > AuthenticationContext();
>> > > > > > > > actx.subject(subj);
>> > > > > > > >
>> > > > > > > > ctx0 = secPrc.authenticate(actx);
>> > > > > > > > }
>> > > > > > > > }
>> > > > > > > >
>> > > > > > > > secCtxs.putIfAbsent(nodeId, ctx0);
>> > > > > > > >
>> > > > > > > > return withContext(ctx0);
>> > > > > > > > } catch (IgniteCheckedException e) {
>> > > > > > > > throw new IgniteException(e);
>> > > > > > > > }
>> > > > > > > >
>> > > > > > > > The idea is to create a thin client SecurityContext on a
>> node
>> > not
>> > > > > > > > having
>> > > > > > > a
>> > > > > > > > local context using existing SecuritySubject data.
>> > > > > > > >
>> > > > > > > > Method
>> > > > > > > >
>> > > > > > >
>> > > >
>> org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
>> > > > > > > uthenticate
>> > > > > > > > should check for not null SecuritySubject field and just
>> > recreate
>> > > > > > > > SecurityContext using passed info (because it's already
>> > > > > authenticated).
>> > > > > > > >
>> > > > > > > > We have all necessary information in SecuritySubject
>> returned
>> > by
>> > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > >
>> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
>> > > > > > > #authenticatedSubject
>> > > > > > > > by GridSecurityProcessor method.
>> > > > > > > >
>> > > > > > > > Because it is internal API, we may not care about
>> > compatibility
>> > > at
>> > > > > > > > all, but nevertheless it is possible to add compatibility
>> check
>> > > in
>> > > > > > > > the method above. If a feature is not supported the
>> operations
>> > > from
>> > > > > > > > thin clients should be forbidden.
>> > > > > > > >
>> > > > > > > > You proposal has the similar problem: if
>> GridSecurityProcessor
>> > > does
>> > > > > > > > not support retriving context for thin clients, such clients
>> > will
>> > > > > > > > not be able to proceed with operation.
>> > > > > > > >
>> > > > > > > > Still, the cleanup of security API is necessary and should
>> be
>> > > done
>> > > > > > > > in 3.0
>> > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
>> > > > v.mithare@cmcmarkets.com
>> > > > > >:
>> > > > > > > >
>> > > > > > > > > HI ,
>> > > > > > > > >
>> > > > > > > > > Created this jira :
>> > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
>> > > > > > > > >
>> > > > > > > > > regards,
>> > > > > > > > > Veena.
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > > --
>> > > > > > > > > Sent from:
>> > > > http://apache-ignite-developers.2346864.n4.nabble.com/
>> > > > > > > > >
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > --
>> > > > > > > >
>> > > > > > > > Best regards,
>> > > > > > > > Alexei Scherbakov
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > > >
>> > > > > > --
>> > > > > >
>> > > > > > Best regards,
>> > > > > > Alexei Scherbakov
>> > > > > > ________________________________
>> > > > > >
>> > > > > >
>> > > > > > Spread bets and CFDs are complex instruments and come with a
>> high
>> > > risk
>> > > > of
>> > > > > > losing money rapidly due to leverage. 70.5% of retail investor
>> > > accounts
>> > > > > > lose money when spread betting and/or trading CFDs with this
>> > > provider.
>> > > > > You
>> > > > > > should consider whether you understand how spread bets and CFDs
>> > work
>> > > > and
>> > > > > > whether you can afford to take the high risk of losing your
>> money.
>> > > > > >
>> > > > > > Professional clients: Losses can exceed deposits when spread
>> > betting
>> > > > and
>> > > > > > trading CFDs. Countdowns carry a level of risk to your capital
>> as
>> > you
>> > > > > could
>> > > > > > lose all of your investment. These products may not be suitable
>> for
>> > > all
>> > > > > > clients therefore ensure you understand the risks and seek
>> > > independent
>> > > > > > advice. Invest only what you can afford to lose.
>> > > > > >
>> > > > > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
>> > > > authorised
>> > > > > > and regulated by the Financial Conduct Authority in the United
>> > > Kingdom.
>> > > > > CMC
>> > > > > > Markets UK plc and CMC Spreadbet plc are registered in England
>> and
>> > > > Wales
>> > > > > > with Company Numbers 02448409 and 02589529 and with their
>> > registered
>> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
>> > > > > >
>> > > > > > The content of this e-mail (including any attachments) is
>> strictly
>> > > > > > confidential and is for the sole use of the intended
>> addressee(s).
>> > If
>> > > > you
>> > > > > > are not the intended recipient of this e-mail please notify the
>> > > sender
>> > > > > > immediately and delete this e-mail from your system. Any
>> > disclosure,
>> > > > > > copying, dissemination or use of its content (including any
>> > > > attachments)
>> > > > > is
>> > > > > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc
>> > reserve
>> > > > the
>> > > > > > right to intercept and monitor the content of the e-mail
>> messages
>> > to
>> > > > and
>> > > > > > from its systems.
>> > > > > >
>> > > > > > E-mails may be interfered with or may contain viruses or other
>> > > defects
>> > > > > for
>> > > > > > which CMC Markets UK plc and CMC Spreadbet plc accept no
>> > > > responsibility.
>> > > > > It
>> > > > > > is the responsibility of the recipient to carry out a virus
>> check
>> > on
>> > > > the
>> > > > > > e-mail and any attachment(s).
>> > > > > >
>> > > > > > This communication is not intended as an offer or solicitation
>> for
>> > > the
>> > > > > > purchase or sale of a financial instrument or as an official
>> > > > confirmation
>> > > > > > of any transaction unless specifically presented as such.
>> > > > > >
>> > > > > > CMC Markets UK plc and CMC Spreadbet plc are registered in
>> England
>> > > and
>> > > > > > Wales with Company Numbers 02448409 and 02589529 and with their
>> > > > > registered
>> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> > > --
>> > >
>> > > Best regards,
>> > > Alexei Scherbakov
>> > >
>> >
>>
>
Re: Security Subject of thin client on remote nodes
Posted by Ivan Rakov <iv...@gmail.com>.
Denis,
I'll review your PR. If this issue is a subject to be included in 2.8.1 in
emergency mode, I'm ok with the current API changes.
Please think about driving creating IEP on security API overhaul prior to
2.9. I believe that you are the most suitable Ignite community member to
drive this activity. I'd love to share some ideas as well.
On Tue, Mar 24, 2020 at 2:04 PM Denis Garus <ga...@gmail.com> wrote:
> Hi, guys!
>
>
> I agree that we should rework the security API, but it can take a long
> time.
>
> And currently, our users have certain impediments that are blockers for
> their job.
>
> I think we have to fix bugs that IEP-41 [1] contains as soon as possible to
> support our users.
>
> From my point of view, IEP-41 is the best place to track bug fixing.
>
>
>
> 1.
>
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes
>
>
> вт, 24 мар. 2020 г. в 12:26, Ivan Rakov <iv...@gmail.com>:
>
> > Alexey,
> >
> > That can be another version of our plan. If everyone agrees that
> > SecurityContext and SecuritySubject should be merged, such fix of thin
> > clients' issue will bring us closer to the final solution.
> > Denis, what do you think?
> >
> > On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
> > alexey.scherbakoff@gmail.com> wrote:
> >
> > > Why can't we start gradually changing security API right now ?
> > > I see no point in delaying with.
> > > All changes will go to next 2.9 release anyway.
> > >
> > > My proposal:
> > > 1. Get rid of security context. Doing this will bring security API to
> > more
> > > or less consistent state.
> > > 2. Remove IEP-41 because it's no longer needed because of change [1]
> > > 3. Propose an IEP to make security API avoid using internals.
> > >
> > >
> > >
> > > пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
> > >
> > > > Hello, Alexei, Ivan!
> > > >
> > > > >> Seems like security API is indeed a bit over-engineered
> > > >
> > > > Nobody has doubt we should do a reworking of GridSecurityProcessor.
> > > > But this point is outside of scope thin client's problem that we are
> > > > solving.
> > > > I think we can create new IEP that will accumulate all ideas of
> > Ignite's
> > > > security improvements.
> > > >
> > > > >> Presence of the separate #securityContext(UUID) highlights that
> user
> > > > indeed should care
> > > > >> about propagation of thin clients' contexts between the cluster
> > nodes.
> > > >
> > > > I agree with Ivan. I've implemented both variants,
> > > > and I like one with #securityContext(UUID) more.
> > > >
> > > > Could you please take a look at PR [1] for the issue [2]?
> > > >
> > > > 1. https://github.com/apache/ignite/pull/7523
> > > > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> > > >
> > > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
> > > >
> > > > > Alex, Denis,
> > > > >
> > > > > Seems like security API is indeed a bit over-engineered.
> > > > >
> > > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > > > SecurityContext is just a POJO wrapper over
> > > > > > SecuritySubject's
> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > > > It's functionality can be easily moved to SecuritySubject.
> > > > >
> > > > > I totally agree. Both subject and context are implemented by plugin
> > > > > provider, and I don't see any reason to keep both abstractions,
> > > > especially
> > > > > if we are going to get rid of transferring subject in node
> attributes
> > > > > (argument that subject is more lightweight won't work anymore).
> > > > >
> > > > > Also, there's kind of mess in node authentication logic. There are
> at
> > > > least
> > > > > two components responsible for it: DiscoverySpiNodeAuthenticator
> > (which
> > > > is
> > > > > forcibly set by GridDiscoveryManager, but in fact public) and
> > > > > GridSecurityProcessor (which performs actual node auth logic, but
> > > > private).
> > > > > I also don't understand why we need both
> > > > > #authenticate(AuthenticationContext) and
> > #authenticateNode(ClusterNode,
> > > > > SecurityCredentials) methods while it's possible to set explicit
> > > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> > > > arguable;
> > > > > perhaps there are strong reasons).
> > > > >
> > > > > Finally, areas of responsibility between IgniteSecurity and
> > > > > GridSecurityProcessor are kind of mixed. As far as I understand,
> the
> > > > first
> > > > > is responsible for Ignite-internal management of security logic
> > > (keeping
> > > > > thread-local context, caching security contexts, etc; we don't
> expect
> > > > > IgniteSecurity to be replaced by plugin provider) and the latter is
> > > > > responsible for user-custom authentication / authorization logic.
> To
> > be
> > > > > honest, it took plenty of time to figure this out for me.
> > > > >
> > > > > From my point of view, we should make GridSecurityProcessor
> interface
> > > > > public, rename it (it requires plenty of time to find the
> difference
> > > from
> > > > > IgniteSecurity), make its API as simple and non-duplicating as
> > possible
> > > > and
> > > > > clarify its area of responsibility (e.g. should it be responsible
> for
> > > > > propagation of successfully authenticated subject among all nodes
> or
> > > > not?)
> > > > > to make it easy to embed custom security logic in Ignite.
> > > > >
> > > > > Regarding thin clients fix: implementation made by Denis suits
> better
> > > to
> > > > > the very implicit contract that it's better to change API contracts
> > of
> > > an
> > > > > internal IgniteSecurity than of internal GridSecurityProcessor
> (which
> > > > > actually mustn't be internal).
> > > > >
> > > > > > My approach doesn't require any IEPs, just minor change in code
> and
> > > to
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > > > contract.
> > > > >
> > > > > Looks like a misuse of #authenticate method to me. It should
> perform
> > > > > initial authentication based on credentials (this may include
> queries
> > > to
> > > > > external authentication subsystem, e.g. LDAP). User may want to
> don't
> > > > > authenticate thin client on every node (this will increase the
> number
> > > of
> > > > > requests to auth subsystem unless user implicitly implements
> > > propagation
> > > > of
> > > > > thin clients' contexts between nodes and make #authenticate
> > > cluster-wide
> > > > > idempotent: first call should perform actual authentication, next
> > calls
> > > > > should retrieve context of already authenticated client). Presence
> of
> > > the
> > > > > separate #securityContext(UUID) highlights that user indeed should
> > care
> > > > > about propagation of thin clients' contexts between the cluster
> > nodes.
> > > > >
> > > > > --
> > > > > Ivan
> > > > >
> > > > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <
> > > V.Mithare@cmcmarkets.com
> > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Alexei, Denis,
> > > > > >
> > > > > > One of the main usecases of thin client authentication is to be
> > able
> > > to
> > > > > > audit the changes done using the thin client user.
> > > > > > To enable that :
> > > > > > We really need to resolve this concern as well :
> > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > > >
> > > > > > ( Incorrect security subject id is associated with a cache_put
> > event
> > > > > > when the originator of the event is a thin client. )
> > > > > >
> > > > > > Regards,
> > > > > > Veena
> > > > > >
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: Alexei Scherbakov <al...@gmail.com>
> > > > > > Sent: 18 March 2020 08:11
> > > > > > To: dev <de...@ignite.apache.org>
> > > > > > Subject: Re: Security Subject of thin client on remote nodes
> > > > > >
> > > > > > Denis Garus,
> > > > > >
> > > > > > Both variants are capable of solving the thin client security
> > context
> > > > > > problem.
> > > > > >
> > > > > > My approach doesn't require any IEPs, just minor change in code
> and
> > > to
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > > > contract.
> > > > > > We can add appropriate documentation to emphasize this.
> > > > > > The argument "fragile" is not very convincing for me.
> > > > > >
> > > > > > I think we should collect more opinions before proceeding with
> IEP.
> > > > > >
> > > > > > Considering a fact we actually *may not care* about compatibility
> > > (I've
> > > > > > already explained why), I'm thinking of another approach.
> > > > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > > > SecurityContext is just a POJO wrapper over SecuritySubject's
> > > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > > > It's functionality can be easily moved to SecuritySubject.
> > > > > >
> > > > > > What do you think?
> > > > > >
> > > > > >
> > > > > >
> > > > > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> > > > > >
> > > > > > > Hello, Alexei!
> > > > > > >
> > > > > > > I agree with you if we may not care about compatibility at all,
> > > then
> > > > > > > we can solve the problem much more straightforward way.
> > > > > > >
> > > > > > > In your case, the method GridSecurityProcessor#authenticate
> will
> > > have
> > > > > > > an implicit contract:
> > > > > > > [ if actx.subject() != null then
> > > > > > > returns SecurityContext
> > > > > > > else
> > > > > > > do authenticate ]
> > > > > > >
> > > > > > > It looks fragile.
> > > > > > >
> > > > > > > When we extend the GridSecurityProcessor, there isn't this
> > problem:
> > > > > > > we have the explicit contract and can make default
> implementation
> > > > that
> > > > > > > throws an unsupported operation exception to enforcing
> > > compatibility
> > > > > > > check.
> > > > > > >
> > > > > > > In any case, we need to change GridSecurityProcessor
> > > implementation.
> > > > > > >
> > > > > > > But I think your proposal to try to find a security context in
> > the
> > > > > > > node's attributes first is right for backward compatibility
> when
> > > > > > > Ignite users don't use thin clients.
> > > > > > >
> > > > > > > Summary:
> > > > > > > I suggest adding a new method to GridSecurityProcessor because
> it
> > > has
> > > > > > > a clear contract and enforces compatibility check natural way.
> > > > > > >
> > > > > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > > > > > > alexey.scherbakoff@gmail.com
> > > > > > > >:
> > > > > > >
> > > > > > > > Denis Garus,
> > > > > > > >
> > > > > > > > I've looked at the IEP proposed by you and currently I'm
> > thinking
> > > > > > > > it's
> > > > > > > not
> > > > > > > > immediately required.
> > > > > > > >
> > > > > > > > The problem of missing SecurityContexts of thin clients can
> be
> > > > > > > > solved
> > > > > > > much
> > > > > > > > easily.
> > > > > > > >
> > > > > > > > Below is the stub of a fix, it requires correct
> implementation
> > of
> > > > > > > > method
> > > > > > > >
> > > > > > >
> > > >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > > > #authenticatedSubject
> > > > > > > > by GridSecurityProcessor:
> > > > > > > >
> > > > > > > > /** {@inheritDoc} */
> > > > > > > > @Override public OperationSecurityContext
> withContext(UUID
> > > > > nodeId)
> > > > > > {
> > > > > > > > try {
> > > > > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > > > > > > >
> > > > > > > > if (ctx0 == null) {
> > > > > > > > ClusterNode node =
> > > > > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > > > > > > .orElseGet(() ->
> > > > > > > > ctx.discovery().historicalNode(nodeId));
> > > > > > > >
> > > > > > > > // This is a cluster node.
> > > > > > > > if (node != null)
> > > > > > > > ctx0 = nodeSecurityContext(marsh,
> > > > > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > > > > > > else {
> > > > > > > > // This is already authenticated thin
> > client.
> > > > > > > > SecuritySubject subj =
> > > > > > > > authenticatedSubject(nodeId);
> > > > > > > >
> > > > > > > > assert subj != null : "Subject is null "
> +
> > > > > > > > nodeId;
> > > > > > > >
> > > > > > > > AuthenticationContext actx = new
> > > > > > > > AuthenticationContext();
> > > > > > > > actx.subject(subj);
> > > > > > > >
> > > > > > > > ctx0 = secPrc.authenticate(actx);
> > > > > > > > }
> > > > > > > > }
> > > > > > > >
> > > > > > > > secCtxs.putIfAbsent(nodeId, ctx0);
> > > > > > > >
> > > > > > > > return withContext(ctx0);
> > > > > > > > } catch (IgniteCheckedException e) {
> > > > > > > > throw new IgniteException(e);
> > > > > > > > }
> > > > > > > >
> > > > > > > > The idea is to create a thin client SecurityContext on a node
> > not
> > > > > > > > having
> > > > > > > a
> > > > > > > > local context using existing SecuritySubject data.
> > > > > > > >
> > > > > > > > Method
> > > > > > > >
> > > > > > >
> > > >
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > > > > > > uthenticate
> > > > > > > > should check for not null SecuritySubject field and just
> > recreate
> > > > > > > > SecurityContext using passed info (because it's already
> > > > > authenticated).
> > > > > > > >
> > > > > > > > We have all necessary information in SecuritySubject returned
> > by
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > > > #authenticatedSubject
> > > > > > > > by GridSecurityProcessor method.
> > > > > > > >
> > > > > > > > Because it is internal API, we may not care about
> > compatibility
> > > at
> > > > > > > > all, but nevertheless it is possible to add compatibility
> check
> > > in
> > > > > > > > the method above. If a feature is not supported the
> operations
> > > from
> > > > > > > > thin clients should be forbidden.
> > > > > > > >
> > > > > > > > You proposal has the similar problem: if
> GridSecurityProcessor
> > > does
> > > > > > > > not support retriving context for thin clients, such clients
> > will
> > > > > > > > not be able to proceed with operation.
> > > > > > > >
> > > > > > > > Still, the cleanup of security API is necessary and should be
> > > done
> > > > > > > > in 3.0
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
> > > > v.mithare@cmcmarkets.com
> > > > > >:
> > > > > > > >
> > > > > > > > > HI ,
> > > > > > > > >
> > > > > > > > > Created this jira :
> > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > > > > > >
> > > > > > > > > regards,
> > > > > > > > > Veena.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > Sent from:
> > > > http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > >
> > > > > > > > Best regards,
> > > > > > > > Alexei Scherbakov
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > >
> > > > > > Best regards,
> > > > > > Alexei Scherbakov
> > > > > > ________________________________
> > > > > >
> > > > > >
> > > > > > Spread bets and CFDs are complex instruments and come with a high
> > > risk
> > > > of
> > > > > > losing money rapidly due to leverage. 70.5% of retail investor
> > > accounts
> > > > > > lose money when spread betting and/or trading CFDs with this
> > > provider.
> > > > > You
> > > > > > should consider whether you understand how spread bets and CFDs
> > work
> > > > and
> > > > > > whether you can afford to take the high risk of losing your
> money.
> > > > > >
> > > > > > Professional clients: Losses can exceed deposits when spread
> > betting
> > > > and
> > > > > > trading CFDs. Countdowns carry a level of risk to your capital as
> > you
> > > > > could
> > > > > > lose all of your investment. These products may not be suitable
> for
> > > all
> > > > > > clients therefore ensure you understand the risks and seek
> > > independent
> > > > > > advice. Invest only what you can afford to lose.
> > > > > >
> > > > > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
> > > > authorised
> > > > > > and regulated by the Financial Conduct Authority in the United
> > > Kingdom.
> > > > > CMC
> > > > > > Markets UK plc and CMC Spreadbet plc are registered in England
> and
> > > > Wales
> > > > > > with Company Numbers 02448409 and 02589529 and with their
> > registered
> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > > > >
> > > > > > The content of this e-mail (including any attachments) is
> strictly
> > > > > > confidential and is for the sole use of the intended
> addressee(s).
> > If
> > > > you
> > > > > > are not the intended recipient of this e-mail please notify the
> > > sender
> > > > > > immediately and delete this e-mail from your system. Any
> > disclosure,
> > > > > > copying, dissemination or use of its content (including any
> > > > attachments)
> > > > > is
> > > > > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc
> > reserve
> > > > the
> > > > > > right to intercept and monitor the content of the e-mail messages
> > to
> > > > and
> > > > > > from its systems.
> > > > > >
> > > > > > E-mails may be interfered with or may contain viruses or other
> > > defects
> > > > > for
> > > > > > which CMC Markets UK plc and CMC Spreadbet plc accept no
> > > > responsibility.
> > > > > It
> > > > > > is the responsibility of the recipient to carry out a virus check
> > on
> > > > the
> > > > > > e-mail and any attachment(s).
> > > > > >
> > > > > > This communication is not intended as an offer or solicitation
> for
> > > the
> > > > > > purchase or sale of a financial instrument or as an official
> > > > confirmation
> > > > > > of any transaction unless specifically presented as such.
> > > > > >
> > > > > > CMC Markets UK plc and CMC Spreadbet plc are registered in
> England
> > > and
> > > > > > Wales with Company Numbers 02448409 and 02589529 and with their
> > > > > registered
> > > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Alexei Scherbakov
> > >
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Hi, guys!
I agree that we should rework the security API, but it can take a long
time.
And currently, our users have certain impediments that are blockers for
their job.
I think we have to fix bugs that IEP-41 [1] contains as soon as possible to
support our users.
From my point of view, IEP-41 is the best place to track bug fixing.
1.
https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes
вт, 24 мар. 2020 г. в 12:26, Ivan Rakov <iv...@gmail.com>:
> Alexey,
>
> That can be another version of our plan. If everyone agrees that
> SecurityContext and SecuritySubject should be merged, such fix of thin
> clients' issue will bring us closer to the final solution.
> Denis, what do you think?
>
> On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
> alexey.scherbakoff@gmail.com> wrote:
>
> > Why can't we start gradually changing security API right now ?
> > I see no point in delaying with.
> > All changes will go to next 2.9 release anyway.
> >
> > My proposal:
> > 1. Get rid of security context. Doing this will bring security API to
> more
> > or less consistent state.
> > 2. Remove IEP-41 because it's no longer needed because of change [1]
> > 3. Propose an IEP to make security API avoid using internals.
> >
> >
> >
> > пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
> >
> > > Hello, Alexei, Ivan!
> > >
> > > >> Seems like security API is indeed a bit over-engineered
> > >
> > > Nobody has doubt we should do a reworking of GridSecurityProcessor.
> > > But this point is outside of scope thin client's problem that we are
> > > solving.
> > > I think we can create new IEP that will accumulate all ideas of
> Ignite's
> > > security improvements.
> > >
> > > >> Presence of the separate #securityContext(UUID) highlights that user
> > > indeed should care
> > > >> about propagation of thin clients' contexts between the cluster
> nodes.
> > >
> > > I agree with Ivan. I've implemented both variants,
> > > and I like one with #securityContext(UUID) more.
> > >
> > > Could you please take a look at PR [1] for the issue [2]?
> > >
> > > 1. https://github.com/apache/ignite/pull/7523
> > > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> > >
> > > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
> > >
> > > > Alex, Denis,
> > > >
> > > > Seems like security API is indeed a bit over-engineered.
> > > >
> > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > > SecurityContext is just a POJO wrapper over
> > > > > SecuritySubject's
> > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > > It's functionality can be easily moved to SecuritySubject.
> > > >
> > > > I totally agree. Both subject and context are implemented by plugin
> > > > provider, and I don't see any reason to keep both abstractions,
> > > especially
> > > > if we are going to get rid of transferring subject in node attributes
> > > > (argument that subject is more lightweight won't work anymore).
> > > >
> > > > Also, there's kind of mess in node authentication logic. There are at
> > > least
> > > > two components responsible for it: DiscoverySpiNodeAuthenticator
> (which
> > > is
> > > > forcibly set by GridDiscoveryManager, but in fact public) and
> > > > GridSecurityProcessor (which performs actual node auth logic, but
> > > private).
> > > > I also don't understand why we need both
> > > > #authenticate(AuthenticationContext) and
> #authenticateNode(ClusterNode,
> > > > SecurityCredentials) methods while it's possible to set explicit
> > > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> > > arguable;
> > > > perhaps there are strong reasons).
> > > >
> > > > Finally, areas of responsibility between IgniteSecurity and
> > > > GridSecurityProcessor are kind of mixed. As far as I understand, the
> > > first
> > > > is responsible for Ignite-internal management of security logic
> > (keeping
> > > > thread-local context, caching security contexts, etc; we don't expect
> > > > IgniteSecurity to be replaced by plugin provider) and the latter is
> > > > responsible for user-custom authentication / authorization logic. To
> be
> > > > honest, it took plenty of time to figure this out for me.
> > > >
> > > > From my point of view, we should make GridSecurityProcessor interface
> > > > public, rename it (it requires plenty of time to find the difference
> > from
> > > > IgniteSecurity), make its API as simple and non-duplicating as
> possible
> > > and
> > > > clarify its area of responsibility (e.g. should it be responsible for
> > > > propagation of successfully authenticated subject among all nodes or
> > > not?)
> > > > to make it easy to embed custom security logic in Ignite.
> > > >
> > > > Regarding thin clients fix: implementation made by Denis suits better
> > to
> > > > the very implicit contract that it's better to change API contracts
> of
> > an
> > > > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > > > actually mustn't be internal).
> > > >
> > > > > My approach doesn't require any IEPs, just minor change in code and
> > to
> > > > >
> > > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > > contract.
> > > >
> > > > Looks like a misuse of #authenticate method to me. It should perform
> > > > initial authentication based on credentials (this may include queries
> > to
> > > > external authentication subsystem, e.g. LDAP). User may want to don't
> > > > authenticate thin client on every node (this will increase the number
> > of
> > > > requests to auth subsystem unless user implicitly implements
> > propagation
> > > of
> > > > thin clients' contexts between nodes and make #authenticate
> > cluster-wide
> > > > idempotent: first call should perform actual authentication, next
> calls
> > > > should retrieve context of already authenticated client). Presence of
> > the
> > > > separate #securityContext(UUID) highlights that user indeed should
> care
> > > > about propagation of thin clients' contexts between the cluster
> nodes.
> > > >
> > > > --
> > > > Ivan
> > > >
> > > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <
> > V.Mithare@cmcmarkets.com
> > > >
> > > > wrote:
> > > >
> > > > > Hi Alexei, Denis,
> > > > >
> > > > > One of the main usecases of thin client authentication is to be
> able
> > to
> > > > > audit the changes done using the thin client user.
> > > > > To enable that :
> > > > > We really need to resolve this concern as well :
> > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > >
> > > > > ( Incorrect security subject id is associated with a cache_put
> event
> > > > > when the originator of the event is a thin client. )
> > > > >
> > > > > Regards,
> > > > > Veena
> > > > >
> > > > >
> > > > > -----Original Message-----
> > > > > From: Alexei Scherbakov <al...@gmail.com>
> > > > > Sent: 18 March 2020 08:11
> > > > > To: dev <de...@ignite.apache.org>
> > > > > Subject: Re: Security Subject of thin client on remote nodes
> > > > >
> > > > > Denis Garus,
> > > > >
> > > > > Both variants are capable of solving the thin client security
> context
> > > > > problem.
> > > > >
> > > > > My approach doesn't require any IEPs, just minor change in code and
> > to
> > > > >
> > > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > > contract.
> > > > > We can add appropriate documentation to emphasize this.
> > > > > The argument "fragile" is not very convincing for me.
> > > > >
> > > > > I think we should collect more opinions before proceeding with IEP.
> > > > >
> > > > > Considering a fact we actually *may not care* about compatibility
> > (I've
> > > > > already explained why), I'm thinking of another approach.
> > > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > > SecurityContext is just a POJO wrapper over SecuritySubject's
> > > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > > It's functionality can be easily moved to SecuritySubject.
> > > > >
> > > > > What do you think?
> > > > >
> > > > >
> > > > >
> > > > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> > > > >
> > > > > > Hello, Alexei!
> > > > > >
> > > > > > I agree with you if we may not care about compatibility at all,
> > then
> > > > > > we can solve the problem much more straightforward way.
> > > > > >
> > > > > > In your case, the method GridSecurityProcessor#authenticate will
> > have
> > > > > > an implicit contract:
> > > > > > [ if actx.subject() != null then
> > > > > > returns SecurityContext
> > > > > > else
> > > > > > do authenticate ]
> > > > > >
> > > > > > It looks fragile.
> > > > > >
> > > > > > When we extend the GridSecurityProcessor, there isn't this
> problem:
> > > > > > we have the explicit contract and can make default implementation
> > > that
> > > > > > throws an unsupported operation exception to enforcing
> > compatibility
> > > > > > check.
> > > > > >
> > > > > > In any case, we need to change GridSecurityProcessor
> > implementation.
> > > > > >
> > > > > > But I think your proposal to try to find a security context in
> the
> > > > > > node's attributes first is right for backward compatibility when
> > > > > > Ignite users don't use thin clients.
> > > > > >
> > > > > > Summary:
> > > > > > I suggest adding a new method to GridSecurityProcessor because it
> > has
> > > > > > a clear contract and enforces compatibility check natural way.
> > > > > >
> > > > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > > > > > alexey.scherbakoff@gmail.com
> > > > > > >:
> > > > > >
> > > > > > > Denis Garus,
> > > > > > >
> > > > > > > I've looked at the IEP proposed by you and currently I'm
> thinking
> > > > > > > it's
> > > > > > not
> > > > > > > immediately required.
> > > > > > >
> > > > > > > The problem of missing SecurityContexts of thin clients can be
> > > > > > > solved
> > > > > > much
> > > > > > > easily.
> > > > > > >
> > > > > > > Below is the stub of a fix, it requires correct implementation
> of
> > > > > > > method
> > > > > > >
> > > > > >
> > > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > > #authenticatedSubject
> > > > > > > by GridSecurityProcessor:
> > > > > > >
> > > > > > > /** {@inheritDoc} */
> > > > > > > @Override public OperationSecurityContext withContext(UUID
> > > > nodeId)
> > > > > {
> > > > > > > try {
> > > > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > > > > > >
> > > > > > > if (ctx0 == null) {
> > > > > > > ClusterNode node =
> > > > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > > > > > .orElseGet(() ->
> > > > > > > ctx.discovery().historicalNode(nodeId));
> > > > > > >
> > > > > > > // This is a cluster node.
> > > > > > > if (node != null)
> > > > > > > ctx0 = nodeSecurityContext(marsh,
> > > > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > > > > > else {
> > > > > > > // This is already authenticated thin
> client.
> > > > > > > SecuritySubject subj =
> > > > > > > authenticatedSubject(nodeId);
> > > > > > >
> > > > > > > assert subj != null : "Subject is null " +
> > > > > > > nodeId;
> > > > > > >
> > > > > > > AuthenticationContext actx = new
> > > > > > > AuthenticationContext();
> > > > > > > actx.subject(subj);
> > > > > > >
> > > > > > > ctx0 = secPrc.authenticate(actx);
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > secCtxs.putIfAbsent(nodeId, ctx0);
> > > > > > >
> > > > > > > return withContext(ctx0);
> > > > > > > } catch (IgniteCheckedException e) {
> > > > > > > throw new IgniteException(e);
> > > > > > > }
> > > > > > >
> > > > > > > The idea is to create a thin client SecurityContext on a node
> not
> > > > > > > having
> > > > > > a
> > > > > > > local context using existing SecuritySubject data.
> > > > > > >
> > > > > > > Method
> > > > > > >
> > > > > >
> > > org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > > > > > uthenticate
> > > > > > > should check for not null SecuritySubject field and just
> recreate
> > > > > > > SecurityContext using passed info (because it's already
> > > > authenticated).
> > > > > > >
> > > > > > > We have all necessary information in SecuritySubject returned
> by
> > > > > > >
> > > > > > >
> > > > > >
> > > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > > #authenticatedSubject
> > > > > > > by GridSecurityProcessor method.
> > > > > > >
> > > > > > > Because it is internal API, we may not care about
> compatibility
> > at
> > > > > > > all, but nevertheless it is possible to add compatibility check
> > in
> > > > > > > the method above. If a feature is not supported the operations
> > from
> > > > > > > thin clients should be forbidden.
> > > > > > >
> > > > > > > You proposal has the similar problem: if GridSecurityProcessor
> > does
> > > > > > > not support retriving context for thin clients, such clients
> will
> > > > > > > not be able to proceed with operation.
> > > > > > >
> > > > > > > Still, the cleanup of security API is necessary and should be
> > done
> > > > > > > in 3.0
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
> > > v.mithare@cmcmarkets.com
> > > > >:
> > > > > > >
> > > > > > > > HI ,
> > > > > > > >
> > > > > > > > Created this jira :
> > > > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > > > > >
> > > > > > > > regards,
> > > > > > > > Veena.
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > Sent from:
> > > http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > >
> > > > > > > Best regards,
> > > > > > > Alexei Scherbakov
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > Best regards,
> > > > > Alexei Scherbakov
> > > > > ________________________________
> > > > >
> > > > >
> > > > > Spread bets and CFDs are complex instruments and come with a high
> > risk
> > > of
> > > > > losing money rapidly due to leverage. 70.5% of retail investor
> > accounts
> > > > > lose money when spread betting and/or trading CFDs with this
> > provider.
> > > > You
> > > > > should consider whether you understand how spread bets and CFDs
> work
> > > and
> > > > > whether you can afford to take the high risk of losing your money.
> > > > >
> > > > > Professional clients: Losses can exceed deposits when spread
> betting
> > > and
> > > > > trading CFDs. Countdowns carry a level of risk to your capital as
> you
> > > > could
> > > > > lose all of your investment. These products may not be suitable for
> > all
> > > > > clients therefore ensure you understand the risks and seek
> > independent
> > > > > advice. Invest only what you can afford to lose.
> > > > >
> > > > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
> > > authorised
> > > > > and regulated by the Financial Conduct Authority in the United
> > Kingdom.
> > > > CMC
> > > > > Markets UK plc and CMC Spreadbet plc are registered in England and
> > > Wales
> > > > > with Company Numbers 02448409 and 02589529 and with their
> registered
> > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > > >
> > > > > The content of this e-mail (including any attachments) is strictly
> > > > > confidential and is for the sole use of the intended addressee(s).
> If
> > > you
> > > > > are not the intended recipient of this e-mail please notify the
> > sender
> > > > > immediately and delete this e-mail from your system. Any
> disclosure,
> > > > > copying, dissemination or use of its content (including any
> > > attachments)
> > > > is
> > > > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc
> reserve
> > > the
> > > > > right to intercept and monitor the content of the e-mail messages
> to
> > > and
> > > > > from its systems.
> > > > >
> > > > > E-mails may be interfered with or may contain viruses or other
> > defects
> > > > for
> > > > > which CMC Markets UK plc and CMC Spreadbet plc accept no
> > > responsibility.
> > > > It
> > > > > is the responsibility of the recipient to carry out a virus check
> on
> > > the
> > > > > e-mail and any attachment(s).
> > > > >
> > > > > This communication is not intended as an offer or solicitation for
> > the
> > > > > purchase or sale of a financial instrument or as an official
> > > confirmation
> > > > > of any transaction unless specifically presented as such.
> > > > >
> > > > > CMC Markets UK plc and CMC Spreadbet plc are registered in England
> > and
> > > > > Wales with Company Numbers 02448409 and 02589529 and with their
> > > > registered
> > > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > > >
> > > >
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Ivan Rakov <iv...@gmail.com>.
Alexey,
That can be another version of our plan. If everyone agrees that
SecurityContext and SecuritySubject should be merged, such fix of thin
clients' issue will bring us closer to the final solution.
Denis, what do you think?
On Tue, Mar 24, 2020 at 10:38 AM Alexei Scherbakov <
alexey.scherbakoff@gmail.com> wrote:
> Why can't we start gradually changing security API right now ?
> I see no point in delaying with.
> All changes will go to next 2.9 release anyway.
>
> My proposal:
> 1. Get rid of security context. Doing this will bring security API to more
> or less consistent state.
> 2. Remove IEP-41 because it's no longer needed because of change [1]
> 3. Propose an IEP to make security API avoid using internals.
>
>
>
> пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
>
> > Hello, Alexei, Ivan!
> >
> > >> Seems like security API is indeed a bit over-engineered
> >
> > Nobody has doubt we should do a reworking of GridSecurityProcessor.
> > But this point is outside of scope thin client's problem that we are
> > solving.
> > I think we can create new IEP that will accumulate all ideas of Ignite's
> > security improvements.
> >
> > >> Presence of the separate #securityContext(UUID) highlights that user
> > indeed should care
> > >> about propagation of thin clients' contexts between the cluster nodes.
> >
> > I agree with Ivan. I've implemented both variants,
> > and I like one with #securityContext(UUID) more.
> >
> > Could you please take a look at PR [1] for the issue [2]?
> >
> > 1. https://github.com/apache/ignite/pull/7523
> > 2. https://issues.apache.org/jira/browse/IGNITE-12759
> >
> > пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
> >
> > > Alex, Denis,
> > >
> > > Seems like security API is indeed a bit over-engineered.
> > >
> > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > SecurityContext is just a POJO wrapper over
> > > > SecuritySubject's
> > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > It's functionality can be easily moved to SecuritySubject.
> > >
> > > I totally agree. Both subject and context are implemented by plugin
> > > provider, and I don't see any reason to keep both abstractions,
> > especially
> > > if we are going to get rid of transferring subject in node attributes
> > > (argument that subject is more lightweight won't work anymore).
> > >
> > > Also, there's kind of mess in node authentication logic. There are at
> > least
> > > two components responsible for it: DiscoverySpiNodeAuthenticator (which
> > is
> > > forcibly set by GridDiscoveryManager, but in fact public) and
> > > GridSecurityProcessor (which performs actual node auth logic, but
> > private).
> > > I also don't understand why we need both
> > > #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
> > > SecurityCredentials) methods while it's possible to set explicit
> > > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> > arguable;
> > > perhaps there are strong reasons).
> > >
> > > Finally, areas of responsibility between IgniteSecurity and
> > > GridSecurityProcessor are kind of mixed. As far as I understand, the
> > first
> > > is responsible for Ignite-internal management of security logic
> (keeping
> > > thread-local context, caching security contexts, etc; we don't expect
> > > IgniteSecurity to be replaced by plugin provider) and the latter is
> > > responsible for user-custom authentication / authorization logic. To be
> > > honest, it took plenty of time to figure this out for me.
> > >
> > > From my point of view, we should make GridSecurityProcessor interface
> > > public, rename it (it requires plenty of time to find the difference
> from
> > > IgniteSecurity), make its API as simple and non-duplicating as possible
> > and
> > > clarify its area of responsibility (e.g. should it be responsible for
> > > propagation of successfully authenticated subject among all nodes or
> > not?)
> > > to make it easy to embed custom security logic in Ignite.
> > >
> > > Regarding thin clients fix: implementation made by Denis suits better
> to
> > > the very implicit contract that it's better to change API contracts of
> an
> > > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > > actually mustn't be internal).
> > >
> > > > My approach doesn't require any IEPs, just minor change in code and
> to
> > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > contract.
> > >
> > > Looks like a misuse of #authenticate method to me. It should perform
> > > initial authentication based on credentials (this may include queries
> to
> > > external authentication subsystem, e.g. LDAP). User may want to don't
> > > authenticate thin client on every node (this will increase the number
> of
> > > requests to auth subsystem unless user implicitly implements
> propagation
> > of
> > > thin clients' contexts between nodes and make #authenticate
> cluster-wide
> > > idempotent: first call should perform actual authentication, next calls
> > > should retrieve context of already authenticated client). Presence of
> the
> > > separate #securityContext(UUID) highlights that user indeed should care
> > > about propagation of thin clients' contexts between the cluster nodes.
> > >
> > > --
> > > Ivan
> > >
> > > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <
> V.Mithare@cmcmarkets.com
> > >
> > > wrote:
> > >
> > > > Hi Alexei, Denis,
> > > >
> > > > One of the main usecases of thin client authentication is to be able
> to
> > > > audit the changes done using the thin client user.
> > > > To enable that :
> > > > We really need to resolve this concern as well :
> > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > >
> > > > ( Incorrect security subject id is associated with a cache_put event
> > > > when the originator of the event is a thin client. )
> > > >
> > > > Regards,
> > > > Veena
> > > >
> > > >
> > > > -----Original Message-----
> > > > From: Alexei Scherbakov <al...@gmail.com>
> > > > Sent: 18 March 2020 08:11
> > > > To: dev <de...@ignite.apache.org>
> > > > Subject: Re: Security Subject of thin client on remote nodes
> > > >
> > > > Denis Garus,
> > > >
> > > > Both variants are capable of solving the thin client security context
> > > > problem.
> > > >
> > > > My approach doesn't require any IEPs, just minor change in code and
> to
> > > >
> > > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > > contract.
> > > > We can add appropriate documentation to emphasize this.
> > > > The argument "fragile" is not very convincing for me.
> > > >
> > > > I think we should collect more opinions before proceeding with IEP.
> > > >
> > > > Considering a fact we actually *may not care* about compatibility
> (I've
> > > > already explained why), I'm thinking of another approach.
> > > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > > SecurityContext is just a POJO wrapper over SecuritySubject's
> > > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > > It's functionality can be easily moved to SecuritySubject.
> > > >
> > > > What do you think?
> > > >
> > > >
> > > >
> > > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> > > >
> > > > > Hello, Alexei!
> > > > >
> > > > > I agree with you if we may not care about compatibility at all,
> then
> > > > > we can solve the problem much more straightforward way.
> > > > >
> > > > > In your case, the method GridSecurityProcessor#authenticate will
> have
> > > > > an implicit contract:
> > > > > [ if actx.subject() != null then
> > > > > returns SecurityContext
> > > > > else
> > > > > do authenticate ]
> > > > >
> > > > > It looks fragile.
> > > > >
> > > > > When we extend the GridSecurityProcessor, there isn't this problem:
> > > > > we have the explicit contract and can make default implementation
> > that
> > > > > throws an unsupported operation exception to enforcing
> compatibility
> > > > > check.
> > > > >
> > > > > In any case, we need to change GridSecurityProcessor
> implementation.
> > > > >
> > > > > But I think your proposal to try to find a security context in the
> > > > > node's attributes first is right for backward compatibility when
> > > > > Ignite users don't use thin clients.
> > > > >
> > > > > Summary:
> > > > > I suggest adding a new method to GridSecurityProcessor because it
> has
> > > > > a clear contract and enforces compatibility check natural way.
> > > > >
> > > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > > > > alexey.scherbakoff@gmail.com
> > > > > >:
> > > > >
> > > > > > Denis Garus,
> > > > > >
> > > > > > I've looked at the IEP proposed by you and currently I'm thinking
> > > > > > it's
> > > > > not
> > > > > > immediately required.
> > > > > >
> > > > > > The problem of missing SecurityContexts of thin clients can be
> > > > > > solved
> > > > > much
> > > > > > easily.
> > > > > >
> > > > > > Below is the stub of a fix, it requires correct implementation of
> > > > > > method
> > > > > >
> > > > >
> > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > #authenticatedSubject
> > > > > > by GridSecurityProcessor:
> > > > > >
> > > > > > /** {@inheritDoc} */
> > > > > > @Override public OperationSecurityContext withContext(UUID
> > > nodeId)
> > > > {
> > > > > > try {
> > > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > > > > >
> > > > > > if (ctx0 == null) {
> > > > > > ClusterNode node =
> > > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > > > > .orElseGet(() ->
> > > > > > ctx.discovery().historicalNode(nodeId));
> > > > > >
> > > > > > // This is a cluster node.
> > > > > > if (node != null)
> > > > > > ctx0 = nodeSecurityContext(marsh,
> > > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > > > > else {
> > > > > > // This is already authenticated thin client.
> > > > > > SecuritySubject subj =
> > > > > > authenticatedSubject(nodeId);
> > > > > >
> > > > > > assert subj != null : "Subject is null " +
> > > > > > nodeId;
> > > > > >
> > > > > > AuthenticationContext actx = new
> > > > > > AuthenticationContext();
> > > > > > actx.subject(subj);
> > > > > >
> > > > > > ctx0 = secPrc.authenticate(actx);
> > > > > > }
> > > > > > }
> > > > > >
> > > > > > secCtxs.putIfAbsent(nodeId, ctx0);
> > > > > >
> > > > > > return withContext(ctx0);
> > > > > > } catch (IgniteCheckedException e) {
> > > > > > throw new IgniteException(e);
> > > > > > }
> > > > > >
> > > > > > The idea is to create a thin client SecurityContext on a node not
> > > > > > having
> > > > > a
> > > > > > local context using existing SecuritySubject data.
> > > > > >
> > > > > > Method
> > > > > >
> > > > >
> > org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > > > > uthenticate
> > > > > > should check for not null SecuritySubject field and just recreate
> > > > > > SecurityContext using passed info (because it's already
> > > authenticated).
> > > > > >
> > > > > > We have all necessary information in SecuritySubject returned by
> > > > > >
> > > > > >
> > > > >
> > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > > #authenticatedSubject
> > > > > > by GridSecurityProcessor method.
> > > > > >
> > > > > > Because it is internal API, we may not care about compatibility
> at
> > > > > > all, but nevertheless it is possible to add compatibility check
> in
> > > > > > the method above. If a feature is not supported the operations
> from
> > > > > > thin clients should be forbidden.
> > > > > >
> > > > > > You proposal has the similar problem: if GridSecurityProcessor
> does
> > > > > > not support retriving context for thin clients, such clients will
> > > > > > not be able to proceed with operation.
> > > > > >
> > > > > > Still, the cleanup of security API is necessary and should be
> done
> > > > > > in 3.0
> > > > > >
> > > > > >
> > > > > >
> > > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
> > v.mithare@cmcmarkets.com
> > > >:
> > > > > >
> > > > > > > HI ,
> > > > > > >
> > > > > > > Created this jira :
> > > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > > > >
> > > > > > > regards,
> > > > > > > Veena.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Sent from:
> > http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > >
> > > > > > Best regards,
> > > > > > Alexei Scherbakov
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Best regards,
> > > > Alexei Scherbakov
> > > > ________________________________
> > > >
> > > >
> > > > Spread bets and CFDs are complex instruments and come with a high
> risk
> > of
> > > > losing money rapidly due to leverage. 70.5% of retail investor
> accounts
> > > > lose money when spread betting and/or trading CFDs with this
> provider.
> > > You
> > > > should consider whether you understand how spread bets and CFDs work
> > and
> > > > whether you can afford to take the high risk of losing your money.
> > > >
> > > > Professional clients: Losses can exceed deposits when spread betting
> > and
> > > > trading CFDs. Countdowns carry a level of risk to your capital as you
> > > could
> > > > lose all of your investment. These products may not be suitable for
> all
> > > > clients therefore ensure you understand the risks and seek
> independent
> > > > advice. Invest only what you can afford to lose.
> > > >
> > > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
> > authorised
> > > > and regulated by the Financial Conduct Authority in the United
> Kingdom.
> > > CMC
> > > > Markets UK plc and CMC Spreadbet plc are registered in England and
> > Wales
> > > > with Company Numbers 02448409 and 02589529 and with their registered
> > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > >
> > > > The content of this e-mail (including any attachments) is strictly
> > > > confidential and is for the sole use of the intended addressee(s). If
> > you
> > > > are not the intended recipient of this e-mail please notify the
> sender
> > > > immediately and delete this e-mail from your system. Any disclosure,
> > > > copying, dissemination or use of its content (including any
> > attachments)
> > > is
> > > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc reserve
> > the
> > > > right to intercept and monitor the content of the e-mail messages to
> > and
> > > > from its systems.
> > > >
> > > > E-mails may be interfered with or may contain viruses or other
> defects
> > > for
> > > > which CMC Markets UK plc and CMC Spreadbet plc accept no
> > responsibility.
> > > It
> > > > is the responsibility of the recipient to carry out a virus check on
> > the
> > > > e-mail and any attachment(s).
> > > >
> > > > This communication is not intended as an offer or solicitation for
> the
> > > > purchase or sale of a financial instrument or as an official
> > confirmation
> > > > of any transaction unless specifically presented as such.
> > > >
> > > > CMC Markets UK plc and CMC Spreadbet plc are registered in England
> and
> > > > Wales with Company Numbers 02448409 and 02589529 and with their
> > > registered
> > > > offices at 133 Houndsditch, London, EC3A 7BX.
> > > >
> > >
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>
Re: Security Subject of thin client on remote nodes
Posted by Alexei Scherbakov <al...@gmail.com>.
Why can't we start gradually changing security API right now ?
I see no point in delaying with.
All changes will go to next 2.9 release anyway.
My proposal:
1. Get rid of security context. Doing this will bring security API to more
or less consistent state.
2. Remove IEP-41 because it's no longer needed because of change [1]
3. Propose an IEP to make security API avoid using internals.
пн, 23 мар. 2020 г. в 19:53, Denis Garus <ga...@gmail.com>:
> Hello, Alexei, Ivan!
>
> >> Seems like security API is indeed a bit over-engineered
>
> Nobody has doubt we should do a reworking of GridSecurityProcessor.
> But this point is outside of scope thin client's problem that we are
> solving.
> I think we can create new IEP that will accumulate all ideas of Ignite's
> security improvements.
>
> >> Presence of the separate #securityContext(UUID) highlights that user
> indeed should care
> >> about propagation of thin clients' contexts between the cluster nodes.
>
> I agree with Ivan. I've implemented both variants,
> and I like one with #securityContext(UUID) more.
>
> Could you please take a look at PR [1] for the issue [2]?
>
> 1. https://github.com/apache/ignite/pull/7523
> 2. https://issues.apache.org/jira/browse/IGNITE-12759
>
> пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
>
> > Alex, Denis,
> >
> > Seems like security API is indeed a bit over-engineered.
> >
> > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > SecurityContext is just a POJO wrapper over
> > > SecuritySubject's
> > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > It's functionality can be easily moved to SecuritySubject.
> >
> > I totally agree. Both subject and context are implemented by plugin
> > provider, and I don't see any reason to keep both abstractions,
> especially
> > if we are going to get rid of transferring subject in node attributes
> > (argument that subject is more lightweight won't work anymore).
> >
> > Also, there's kind of mess in node authentication logic. There are at
> least
> > two components responsible for it: DiscoverySpiNodeAuthenticator (which
> is
> > forcibly set by GridDiscoveryManager, but in fact public) and
> > GridSecurityProcessor (which performs actual node auth logic, but
> private).
> > I also don't understand why we need both
> > #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
> > SecurityCredentials) methods while it's possible to set explicit
> > SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is
> arguable;
> > perhaps there are strong reasons).
> >
> > Finally, areas of responsibility between IgniteSecurity and
> > GridSecurityProcessor are kind of mixed. As far as I understand, the
> first
> > is responsible for Ignite-internal management of security logic (keeping
> > thread-local context, caching security contexts, etc; we don't expect
> > IgniteSecurity to be replaced by plugin provider) and the latter is
> > responsible for user-custom authentication / authorization logic. To be
> > honest, it took plenty of time to figure this out for me.
> >
> > From my point of view, we should make GridSecurityProcessor interface
> > public, rename it (it requires plenty of time to find the difference from
> > IgniteSecurity), make its API as simple and non-duplicating as possible
> and
> > clarify its area of responsibility (e.g. should it be responsible for
> > propagation of successfully authenticated subject among all nodes or
> not?)
> > to make it easy to embed custom security logic in Ignite.
> >
> > Regarding thin clients fix: implementation made by Denis suits better to
> > the very implicit contract that it's better to change API contracts of an
> > internal IgniteSecurity than of internal GridSecurityProcessor (which
> > actually mustn't be internal).
> >
> > > My approach doesn't require any IEPs, just minor change in code and to
> > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > contract.
> >
> > Looks like a misuse of #authenticate method to me. It should perform
> > initial authentication based on credentials (this may include queries to
> > external authentication subsystem, e.g. LDAP). User may want to don't
> > authenticate thin client on every node (this will increase the number of
> > requests to auth subsystem unless user implicitly implements propagation
> of
> > thin clients' contexts between nodes and make #authenticate cluster-wide
> > idempotent: first call should perform actual authentication, next calls
> > should retrieve context of already authenticated client). Presence of the
> > separate #securityContext(UUID) highlights that user indeed should care
> > about propagation of thin clients' contexts between the cluster nodes.
> >
> > --
> > Ivan
> >
> > On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <V.Mithare@cmcmarkets.com
> >
> > wrote:
> >
> > > Hi Alexei, Denis,
> > >
> > > One of the main usecases of thin client authentication is to be able to
> > > audit the changes done using the thin client user.
> > > To enable that :
> > > We really need to resolve this concern as well :
> > > https://issues.apache.org/jira/browse/IGNITE-12781
> > >
> > > ( Incorrect security subject id is associated with a cache_put event
> > > when the originator of the event is a thin client. )
> > >
> > > Regards,
> > > Veena
> > >
> > >
> > > -----Original Message-----
> > > From: Alexei Scherbakov <al...@gmail.com>
> > > Sent: 18 March 2020 08:11
> > > To: dev <de...@ignite.apache.org>
> > > Subject: Re: Security Subject of thin client on remote nodes
> > >
> > > Denis Garus,
> > >
> > > Both variants are capable of solving the thin client security context
> > > problem.
> > >
> > > My approach doesn't require any IEPs, just minor change in code and to
> > >
> > >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > > contract.
> > > We can add appropriate documentation to emphasize this.
> > > The argument "fragile" is not very convincing for me.
> > >
> > > I think we should collect more opinions before proceeding with IEP.
> > >
> > > Considering a fact we actually *may not care* about compatibility (I've
> > > already explained why), I'm thinking of another approach.
> > > Let's get rid of SecurityContext and use SecuritySubject instead.
> > > SecurityContext is just a POJO wrapper over SecuritySubject's
> > > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > > It's functionality can be easily moved to SecuritySubject.
> > >
> > > What do you think?
> > >
> > >
> > >
> > > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> > >
> > > > Hello, Alexei!
> > > >
> > > > I agree with you if we may not care about compatibility at all, then
> > > > we can solve the problem much more straightforward way.
> > > >
> > > > In your case, the method GridSecurityProcessor#authenticate will have
> > > > an implicit contract:
> > > > [ if actx.subject() != null then
> > > > returns SecurityContext
> > > > else
> > > > do authenticate ]
> > > >
> > > > It looks fragile.
> > > >
> > > > When we extend the GridSecurityProcessor, there isn't this problem:
> > > > we have the explicit contract and can make default implementation
> that
> > > > throws an unsupported operation exception to enforcing compatibility
> > > > check.
> > > >
> > > > In any case, we need to change GridSecurityProcessor implementation.
> > > >
> > > > But I think your proposal to try to find a security context in the
> > > > node's attributes first is right for backward compatibility when
> > > > Ignite users don't use thin clients.
> > > >
> > > > Summary:
> > > > I suggest adding a new method to GridSecurityProcessor because it has
> > > > a clear contract and enforces compatibility check natural way.
> > > >
> > > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > > > alexey.scherbakoff@gmail.com
> > > > >:
> > > >
> > > > > Denis Garus,
> > > > >
> > > > > I've looked at the IEP proposed by you and currently I'm thinking
> > > > > it's
> > > > not
> > > > > immediately required.
> > > > >
> > > > > The problem of missing SecurityContexts of thin clients can be
> > > > > solved
> > > > much
> > > > > easily.
> > > > >
> > > > > Below is the stub of a fix, it requires correct implementation of
> > > > > method
> > > > >
> > > >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > #authenticatedSubject
> > > > > by GridSecurityProcessor:
> > > > >
> > > > > /** {@inheritDoc} */
> > > > > @Override public OperationSecurityContext withContext(UUID
> > nodeId)
> > > {
> > > > > try {
> > > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > > > >
> > > > > if (ctx0 == null) {
> > > > > ClusterNode node =
> > > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > > > .orElseGet(() ->
> > > > > ctx.discovery().historicalNode(nodeId));
> > > > >
> > > > > // This is a cluster node.
> > > > > if (node != null)
> > > > > ctx0 = nodeSecurityContext(marsh,
> > > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > > > else {
> > > > > // This is already authenticated thin client.
> > > > > SecuritySubject subj =
> > > > > authenticatedSubject(nodeId);
> > > > >
> > > > > assert subj != null : "Subject is null " +
> > > > > nodeId;
> > > > >
> > > > > AuthenticationContext actx = new
> > > > > AuthenticationContext();
> > > > > actx.subject(subj);
> > > > >
> > > > > ctx0 = secPrc.authenticate(actx);
> > > > > }
> > > > > }
> > > > >
> > > > > secCtxs.putIfAbsent(nodeId, ctx0);
> > > > >
> > > > > return withContext(ctx0);
> > > > > } catch (IgniteCheckedException e) {
> > > > > throw new IgniteException(e);
> > > > > }
> > > > >
> > > > > The idea is to create a thin client SecurityContext on a node not
> > > > > having
> > > > a
> > > > > local context using existing SecuritySubject data.
> > > > >
> > > > > Method
> > > > >
> > > >
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > > > uthenticate
> > > > > should check for not null SecuritySubject field and just recreate
> > > > > SecurityContext using passed info (because it's already
> > authenticated).
> > > > >
> > > > > We have all necessary information in SecuritySubject returned by
> > > > >
> > > > >
> > > >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > > #authenticatedSubject
> > > > > by GridSecurityProcessor method.
> > > > >
> > > > > Because it is internal API, we may not care about compatibility at
> > > > > all, but nevertheless it is possible to add compatibility check in
> > > > > the method above. If a feature is not supported the operations from
> > > > > thin clients should be forbidden.
> > > > >
> > > > > You proposal has the similar problem: if GridSecurityProcessor does
> > > > > not support retriving context for thin clients, such clients will
> > > > > not be able to proceed with operation.
> > > > >
> > > > > Still, the cleanup of security API is necessary and should be done
> > > > > in 3.0
> > > > >
> > > > >
> > > > >
> > > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <
> v.mithare@cmcmarkets.com
> > >:
> > > > >
> > > > > > HI ,
> > > > > >
> > > > > > Created this jira :
> > > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > > >
> > > > > > regards,
> > > > > > Veena.
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Sent from:
> http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > >
> > > > > Best regards,
> > > > > Alexei Scherbakov
> > > > >
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Alexei Scherbakov
> > > ________________________________
> > >
> > >
> > > Spread bets and CFDs are complex instruments and come with a high risk
> of
> > > losing money rapidly due to leverage. 70.5% of retail investor accounts
> > > lose money when spread betting and/or trading CFDs with this provider.
> > You
> > > should consider whether you understand how spread bets and CFDs work
> and
> > > whether you can afford to take the high risk of losing your money.
> > >
> > > Professional clients: Losses can exceed deposits when spread betting
> and
> > > trading CFDs. Countdowns carry a level of risk to your capital as you
> > could
> > > lose all of your investment. These products may not be suitable for all
> > > clients therefore ensure you understand the risks and seek independent
> > > advice. Invest only what you can afford to lose.
> > >
> > > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are
> authorised
> > > and regulated by the Financial Conduct Authority in the United Kingdom.
> > CMC
> > > Markets UK plc and CMC Spreadbet plc are registered in England and
> Wales
> > > with Company Numbers 02448409 and 02589529 and with their registered
> > > offices at 133 Houndsditch, London, EC3A 7BX.
> > >
> > > The content of this e-mail (including any attachments) is strictly
> > > confidential and is for the sole use of the intended addressee(s). If
> you
> > > are not the intended recipient of this e-mail please notify the sender
> > > immediately and delete this e-mail from your system. Any disclosure,
> > > copying, dissemination or use of its content (including any
> attachments)
> > is
> > > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc reserve
> the
> > > right to intercept and monitor the content of the e-mail messages to
> and
> > > from its systems.
> > >
> > > E-mails may be interfered with or may contain viruses or other defects
> > for
> > > which CMC Markets UK plc and CMC Spreadbet plc accept no
> responsibility.
> > It
> > > is the responsibility of the recipient to carry out a virus check on
> the
> > > e-mail and any attachment(s).
> > >
> > > This communication is not intended as an offer or solicitation for the
> > > purchase or sale of a financial instrument or as an official
> confirmation
> > > of any transaction unless specifically presented as such.
> > >
> > > CMC Markets UK plc and CMC Spreadbet plc are registered in England and
> > > Wales with Company Numbers 02448409 and 02589529 and with their
> > registered
> > > offices at 133 Houndsditch, London, EC3A 7BX.
> > >
> >
>
--
Best regards,
Alexei Scherbakov
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Hello, Alexei, Ivan!
>> Seems like security API is indeed a bit over-engineered
Nobody has doubt we should do a reworking of GridSecurityProcessor.
But this point is outside of scope thin client's problem that we are
solving.
I think we can create new IEP that will accumulate all ideas of Ignite's
security improvements.
>> Presence of the separate #securityContext(UUID) highlights that user
indeed should care
>> about propagation of thin clients' contexts between the cluster nodes.
I agree with Ivan. I've implemented both variants,
and I like one with #securityContext(UUID) more.
Could you please take a look at PR [1] for the issue [2]?
1. https://github.com/apache/ignite/pull/7523
2. https://issues.apache.org/jira/browse/IGNITE-12759
пн, 23 мар. 2020 г. в 11:45, Ivan Rakov <iv...@gmail.com>:
> Alex, Denis,
>
> Seems like security API is indeed a bit over-engineered.
>
> Let's get rid of SecurityContext and use SecuritySubject instead.
> > SecurityContext is just a POJO wrapper over
> > SecuritySubject's
> > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > It's functionality can be easily moved to SecuritySubject.
>
> I totally agree. Both subject and context are implemented by plugin
> provider, and I don't see any reason to keep both abstractions, especially
> if we are going to get rid of transferring subject in node attributes
> (argument that subject is more lightweight won't work anymore).
>
> Also, there's kind of mess in node authentication logic. There are at least
> two components responsible for it: DiscoverySpiNodeAuthenticator (which is
> forcibly set by GridDiscoveryManager, but in fact public) and
> GridSecurityProcessor (which performs actual node auth logic, but private).
> I also don't understand why we need both
> #authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
> SecurityCredentials) methods while it's possible to set explicit
> SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is arguable;
> perhaps there are strong reasons).
>
> Finally, areas of responsibility between IgniteSecurity and
> GridSecurityProcessor are kind of mixed. As far as I understand, the first
> is responsible for Ignite-internal management of security logic (keeping
> thread-local context, caching security contexts, etc; we don't expect
> IgniteSecurity to be replaced by plugin provider) and the latter is
> responsible for user-custom authentication / authorization logic. To be
> honest, it took plenty of time to figure this out for me.
>
> From my point of view, we should make GridSecurityProcessor interface
> public, rename it (it requires plenty of time to find the difference from
> IgniteSecurity), make its API as simple and non-duplicating as possible and
> clarify its area of responsibility (e.g. should it be responsible for
> propagation of successfully authenticated subject among all nodes or not?)
> to make it easy to embed custom security logic in Ignite.
>
> Regarding thin clients fix: implementation made by Denis suits better to
> the very implicit contract that it's better to change API contracts of an
> internal IgniteSecurity than of internal GridSecurityProcessor (which
> actually mustn't be internal).
>
> > My approach doesn't require any IEPs, just minor change in code and to
> >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > contract.
>
> Looks like a misuse of #authenticate method to me. It should perform
> initial authentication based on credentials (this may include queries to
> external authentication subsystem, e.g. LDAP). User may want to don't
> authenticate thin client on every node (this will increase the number of
> requests to auth subsystem unless user implicitly implements propagation of
> thin clients' contexts between nodes and make #authenticate cluster-wide
> idempotent: first call should perform actual authentication, next calls
> should retrieve context of already authenticated client). Presence of the
> separate #securityContext(UUID) highlights that user indeed should care
> about propagation of thin clients' contexts between the cluster nodes.
>
> --
> Ivan
>
> On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <V....@cmcmarkets.com>
> wrote:
>
> > Hi Alexei, Denis,
> >
> > One of the main usecases of thin client authentication is to be able to
> > audit the changes done using the thin client user.
> > To enable that :
> > We really need to resolve this concern as well :
> > https://issues.apache.org/jira/browse/IGNITE-12781
> >
> > ( Incorrect security subject id is associated with a cache_put event
> > when the originator of the event is a thin client. )
> >
> > Regards,
> > Veena
> >
> >
> > -----Original Message-----
> > From: Alexei Scherbakov <al...@gmail.com>
> > Sent: 18 March 2020 08:11
> > To: dev <de...@ignite.apache.org>
> > Subject: Re: Security Subject of thin client on remote nodes
> >
> > Denis Garus,
> >
> > Both variants are capable of solving the thin client security context
> > problem.
> >
> > My approach doesn't require any IEPs, just minor change in code and to
> >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> > contract.
> > We can add appropriate documentation to emphasize this.
> > The argument "fragile" is not very convincing for me.
> >
> > I think we should collect more opinions before proceeding with IEP.
> >
> > Considering a fact we actually *may not care* about compatibility (I've
> > already explained why), I'm thinking of another approach.
> > Let's get rid of SecurityContext and use SecuritySubject instead.
> > SecurityContext is just a POJO wrapper over SecuritySubject's
> > org.apache.ignite.plugin.security.SecuritySubject#permissions.
> > It's functionality can be easily moved to SecuritySubject.
> >
> > What do you think?
> >
> >
> >
> > пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> >
> > > Hello, Alexei!
> > >
> > > I agree with you if we may not care about compatibility at all, then
> > > we can solve the problem much more straightforward way.
> > >
> > > In your case, the method GridSecurityProcessor#authenticate will have
> > > an implicit contract:
> > > [ if actx.subject() != null then
> > > returns SecurityContext
> > > else
> > > do authenticate ]
> > >
> > > It looks fragile.
> > >
> > > When we extend the GridSecurityProcessor, there isn't this problem:
> > > we have the explicit contract and can make default implementation that
> > > throws an unsupported operation exception to enforcing compatibility
> > > check.
> > >
> > > In any case, we need to change GridSecurityProcessor implementation.
> > >
> > > But I think your proposal to try to find a security context in the
> > > node's attributes first is right for backward compatibility when
> > > Ignite users don't use thin clients.
> > >
> > > Summary:
> > > I suggest adding a new method to GridSecurityProcessor because it has
> > > a clear contract and enforces compatibility check natural way.
> > >
> > > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > > alexey.scherbakoff@gmail.com
> > > >:
> > >
> > > > Denis Garus,
> > > >
> > > > I've looked at the IEP proposed by you and currently I'm thinking
> > > > it's
> > > not
> > > > immediately required.
> > > >
> > > > The problem of missing SecurityContexts of thin clients can be
> > > > solved
> > > much
> > > > easily.
> > > >
> > > > Below is the stub of a fix, it requires correct implementation of
> > > > method
> > > >
> > > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > #authenticatedSubject
> > > > by GridSecurityProcessor:
> > > >
> > > > /** {@inheritDoc} */
> > > > @Override public OperationSecurityContext withContext(UUID
> nodeId)
> > {
> > > > try {
> > > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > > >
> > > > if (ctx0 == null) {
> > > > ClusterNode node =
> > > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > > .orElseGet(() ->
> > > > ctx.discovery().historicalNode(nodeId));
> > > >
> > > > // This is a cluster node.
> > > > if (node != null)
> > > > ctx0 = nodeSecurityContext(marsh,
> > > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > > else {
> > > > // This is already authenticated thin client.
> > > > SecuritySubject subj =
> > > > authenticatedSubject(nodeId);
> > > >
> > > > assert subj != null : "Subject is null " +
> > > > nodeId;
> > > >
> > > > AuthenticationContext actx = new
> > > > AuthenticationContext();
> > > > actx.subject(subj);
> > > >
> > > > ctx0 = secPrc.authenticate(actx);
> > > > }
> > > > }
> > > >
> > > > secCtxs.putIfAbsent(nodeId, ctx0);
> > > >
> > > > return withContext(ctx0);
> > > > } catch (IgniteCheckedException e) {
> > > > throw new IgniteException(e);
> > > > }
> > > >
> > > > The idea is to create a thin client SecurityContext on a node not
> > > > having
> > > a
> > > > local context using existing SecuritySubject data.
> > > >
> > > > Method
> > > >
> > > org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > > uthenticate
> > > > should check for not null SecuritySubject field and just recreate
> > > > SecurityContext using passed info (because it's already
> authenticated).
> > > >
> > > > We have all necessary information in SecuritySubject returned by
> > > >
> > > >
> > > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > > #authenticatedSubject
> > > > by GridSecurityProcessor method.
> > > >
> > > > Because it is internal API, we may not care about compatibility at
> > > > all, but nevertheless it is possible to add compatibility check in
> > > > the method above. If a feature is not supported the operations from
> > > > thin clients should be forbidden.
> > > >
> > > > You proposal has the similar problem: if GridSecurityProcessor does
> > > > not support retriving context for thin clients, such clients will
> > > > not be able to proceed with operation.
> > > >
> > > > Still, the cleanup of security API is necessary and should be done
> > > > in 3.0
> > > >
> > > >
> > > >
> > > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v.mithare@cmcmarkets.com
> >:
> > > >
> > > > > HI ,
> > > > >
> > > > > Created this jira :
> > > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > > >
> > > > > regards,
> > > > > Veena.
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > > >
> > > >
> > > >
> > > > --
> > > >
> > > > Best regards,
> > > > Alexei Scherbakov
> > > >
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> > ________________________________
> >
> >
> > Spread bets and CFDs are complex instruments and come with a high risk of
> > losing money rapidly due to leverage. 70.5% of retail investor accounts
> > lose money when spread betting and/or trading CFDs with this provider.
> You
> > should consider whether you understand how spread bets and CFDs work and
> > whether you can afford to take the high risk of losing your money.
> >
> > Professional clients: Losses can exceed deposits when spread betting and
> > trading CFDs. Countdowns carry a level of risk to your capital as you
> could
> > lose all of your investment. These products may not be suitable for all
> > clients therefore ensure you understand the risks and seek independent
> > advice. Invest only what you can afford to lose.
> >
> > CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are authorised
> > and regulated by the Financial Conduct Authority in the United Kingdom.
> CMC
> > Markets UK plc and CMC Spreadbet plc are registered in England and Wales
> > with Company Numbers 02448409 and 02589529 and with their registered
> > offices at 133 Houndsditch, London, EC3A 7BX.
> >
> > The content of this e-mail (including any attachments) is strictly
> > confidential and is for the sole use of the intended addressee(s). If you
> > are not the intended recipient of this e-mail please notify the sender
> > immediately and delete this e-mail from your system. Any disclosure,
> > copying, dissemination or use of its content (including any attachments)
> is
> > strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc reserve the
> > right to intercept and monitor the content of the e-mail messages to and
> > from its systems.
> >
> > E-mails may be interfered with or may contain viruses or other defects
> for
> > which CMC Markets UK plc and CMC Spreadbet plc accept no responsibility.
> It
> > is the responsibility of the recipient to carry out a virus check on the
> > e-mail and any attachment(s).
> >
> > This communication is not intended as an offer or solicitation for the
> > purchase or sale of a financial instrument or as an official confirmation
> > of any transaction unless specifically presented as such.
> >
> > CMC Markets UK plc and CMC Spreadbet plc are registered in England and
> > Wales with Company Numbers 02448409 and 02589529 and with their
> registered
> > offices at 133 Houndsditch, London, EC3A 7BX.
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Ivan Rakov <iv...@gmail.com>.
Alex, Denis,
Seems like security API is indeed a bit over-engineered.
Let's get rid of SecurityContext and use SecuritySubject instead.
> SecurityContext is just a POJO wrapper over
> SecuritySubject's
> org.apache.ignite.plugin.security.SecuritySubject#permissions.
> It's functionality can be easily moved to SecuritySubject.
I totally agree. Both subject and context are implemented by plugin
provider, and I don't see any reason to keep both abstractions, especially
if we are going to get rid of transferring subject in node attributes
(argument that subject is more lightweight won't work anymore).
Also, there's kind of mess in node authentication logic. There are at least
two components responsible for it: DiscoverySpiNodeAuthenticator (which is
forcibly set by GridDiscoveryManager, but in fact public) and
GridSecurityProcessor (which performs actual node auth logic, but private).
I also don't understand why we need both
#authenticate(AuthenticationContext) and #authenticateNode(ClusterNode,
SecurityCredentials) methods while it's possible to set explicit
SecuritySubjectType.REMOTE_NODE in AuthenticationContext (this is arguable;
perhaps there are strong reasons).
Finally, areas of responsibility between IgniteSecurity and
GridSecurityProcessor are kind of mixed. As far as I understand, the first
is responsible for Ignite-internal management of security logic (keeping
thread-local context, caching security contexts, etc; we don't expect
IgniteSecurity to be replaced by plugin provider) and the latter is
responsible for user-custom authentication / authorization logic. To be
honest, it took plenty of time to figure this out for me.
From my point of view, we should make GridSecurityProcessor interface
public, rename it (it requires plenty of time to find the difference from
IgniteSecurity), make its API as simple and non-duplicating as possible and
clarify its area of responsibility (e.g. should it be responsible for
propagation of successfully authenticated subject among all nodes or not?)
to make it easy to embed custom security logic in Ignite.
Regarding thin clients fix: implementation made by Denis suits better to
the very implicit contract that it's better to change API contracts of an
internal IgniteSecurity than of internal GridSecurityProcessor (which
actually mustn't be internal).
> My approach doesn't require any IEPs, just minor change in code and to
>
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> contract.
Looks like a misuse of #authenticate method to me. It should perform
initial authentication based on credentials (this may include queries to
external authentication subsystem, e.g. LDAP). User may want to don't
authenticate thin client on every node (this will increase the number of
requests to auth subsystem unless user implicitly implements propagation of
thin clients' contexts between nodes and make #authenticate cluster-wide
idempotent: first call should perform actual authentication, next calls
should retrieve context of already authenticated client). Presence of the
separate #securityContext(UUID) highlights that user indeed should care
about propagation of thin clients' contexts between the cluster nodes.
--
Ivan
On Fri, Mar 20, 2020 at 12:22 PM Veena Mithare <V....@cmcmarkets.com>
wrote:
> Hi Alexei, Denis,
>
> One of the main usecases of thin client authentication is to be able to
> audit the changes done using the thin client user.
> To enable that :
> We really need to resolve this concern as well :
> https://issues.apache.org/jira/browse/IGNITE-12781
>
> ( Incorrect security subject id is associated with a cache_put event
> when the originator of the event is a thin client. )
>
> Regards,
> Veena
>
>
> -----Original Message-----
> From: Alexei Scherbakov <al...@gmail.com>
> Sent: 18 March 2020 08:11
> To: dev <de...@ignite.apache.org>
> Subject: Re: Security Subject of thin client on remote nodes
>
> Denis Garus,
>
> Both variants are capable of solving the thin client security context
> problem.
>
> My approach doesn't require any IEPs, just minor change in code and to
>
> org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
> contract.
> We can add appropriate documentation to emphasize this.
> The argument "fragile" is not very convincing for me.
>
> I think we should collect more opinions before proceeding with IEP.
>
> Considering a fact we actually *may not care* about compatibility (I've
> already explained why), I'm thinking of another approach.
> Let's get rid of SecurityContext and use SecuritySubject instead.
> SecurityContext is just a POJO wrapper over SecuritySubject's
> org.apache.ignite.plugin.security.SecuritySubject#permissions.
> It's functionality can be easily moved to SecuritySubject.
>
> What do you think?
>
>
>
> пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
>
> > Hello, Alexei!
> >
> > I agree with you if we may not care about compatibility at all, then
> > we can solve the problem much more straightforward way.
> >
> > In your case, the method GridSecurityProcessor#authenticate will have
> > an implicit contract:
> > [ if actx.subject() != null then
> > returns SecurityContext
> > else
> > do authenticate ]
> >
> > It looks fragile.
> >
> > When we extend the GridSecurityProcessor, there isn't this problem:
> > we have the explicit contract and can make default implementation that
> > throws an unsupported operation exception to enforcing compatibility
> > check.
> >
> > In any case, we need to change GridSecurityProcessor implementation.
> >
> > But I think your proposal to try to find a security context in the
> > node's attributes first is right for backward compatibility when
> > Ignite users don't use thin clients.
> >
> > Summary:
> > I suggest adding a new method to GridSecurityProcessor because it has
> > a clear contract and enforces compatibility check natural way.
> >
> > вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> > alexey.scherbakoff@gmail.com
> > >:
> >
> > > Denis Garus,
> > >
> > > I've looked at the IEP proposed by you and currently I'm thinking
> > > it's
> > not
> > > immediately required.
> > >
> > > The problem of missing SecurityContexts of thin clients can be
> > > solved
> > much
> > > easily.
> > >
> > > Below is the stub of a fix, it requires correct implementation of
> > > method
> > >
> > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > #authenticatedSubject
> > > by GridSecurityProcessor:
> > >
> > > /** {@inheritDoc} */
> > > @Override public OperationSecurityContext withContext(UUID nodeId)
> {
> > > try {
> > > SecurityContext ctx0 = secCtxs.get(nodeId);
> > >
> > > if (ctx0 == null) {
> > > ClusterNode node =
> > > Optional.ofNullable(ctx.discovery().node(nodeId))
> > > .orElseGet(() ->
> > > ctx.discovery().historicalNode(nodeId));
> > >
> > > // This is a cluster node.
> > > if (node != null)
> > > ctx0 = nodeSecurityContext(marsh,
> > > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > > else {
> > > // This is already authenticated thin client.
> > > SecuritySubject subj =
> > > authenticatedSubject(nodeId);
> > >
> > > assert subj != null : "Subject is null " +
> > > nodeId;
> > >
> > > AuthenticationContext actx = new
> > > AuthenticationContext();
> > > actx.subject(subj);
> > >
> > > ctx0 = secPrc.authenticate(actx);
> > > }
> > > }
> > >
> > > secCtxs.putIfAbsent(nodeId, ctx0);
> > >
> > > return withContext(ctx0);
> > > } catch (IgniteCheckedException e) {
> > > throw new IgniteException(e);
> > > }
> > >
> > > The idea is to create a thin client SecurityContext on a node not
> > > having
> > a
> > > local context using existing SecuritySubject data.
> > >
> > > Method
> > >
> > org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> > uthenticate
> > > should check for not null SecuritySubject field and just recreate
> > > SecurityContext using passed info (because it's already authenticated).
> > >
> > > We have all necessary information in SecuritySubject returned by
> > >
> > >
> > org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> > #authenticatedSubject
> > > by GridSecurityProcessor method.
> > >
> > > Because it is internal API, we may not care about compatibility at
> > > all, but nevertheless it is possible to add compatibility check in
> > > the method above. If a feature is not supported the operations from
> > > thin clients should be forbidden.
> > >
> > > You proposal has the similar problem: if GridSecurityProcessor does
> > > not support retriving context for thin clients, such clients will
> > > not be able to proceed with operation.
> > >
> > > Still, the cleanup of security API is necessary and should be done
> > > in 3.0
> > >
> > >
> > >
> > > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v....@cmcmarkets.com>:
> > >
> > > > HI ,
> > > >
> > > > Created this jira :
> > > > https://issues.apache.org/jira/browse/IGNITE-12781
> > > >
> > > > regards,
> > > > Veena.
> > > >
> > > >
> > > >
> > > > --
> > > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > > >
> > >
> > >
> > > --
> > >
> > > Best regards,
> > > Alexei Scherbakov
> > >
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
> ________________________________
>
>
> Spread bets and CFDs are complex instruments and come with a high risk of
> losing money rapidly due to leverage. 70.5% of retail investor accounts
> lose money when spread betting and/or trading CFDs with this provider. You
> should consider whether you understand how spread bets and CFDs work and
> whether you can afford to take the high risk of losing your money.
>
> Professional clients: Losses can exceed deposits when spread betting and
> trading CFDs. Countdowns carry a level of risk to your capital as you could
> lose all of your investment. These products may not be suitable for all
> clients therefore ensure you understand the risks and seek independent
> advice. Invest only what you can afford to lose.
>
> CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are authorised
> and regulated by the Financial Conduct Authority in the United Kingdom. CMC
> Markets UK plc and CMC Spreadbet plc are registered in England and Wales
> with Company Numbers 02448409 and 02589529 and with their registered
> offices at 133 Houndsditch, London, EC3A 7BX.
>
> The content of this e-mail (including any attachments) is strictly
> confidential and is for the sole use of the intended addressee(s). If you
> are not the intended recipient of this e-mail please notify the sender
> immediately and delete this e-mail from your system. Any disclosure,
> copying, dissemination or use of its content (including any attachments) is
> strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc reserve the
> right to intercept and monitor the content of the e-mail messages to and
> from its systems.
>
> E-mails may be interfered with or may contain viruses or other defects for
> which CMC Markets UK plc and CMC Spreadbet plc accept no responsibility. It
> is the responsibility of the recipient to carry out a virus check on the
> e-mail and any attachment(s).
>
> This communication is not intended as an offer or solicitation for the
> purchase or sale of a financial instrument or as an official confirmation
> of any transaction unless specifically presented as such.
>
> CMC Markets UK plc and CMC Spreadbet plc are registered in England and
> Wales with Company Numbers 02448409 and 02589529 and with their registered
> offices at 133 Houndsditch, London, EC3A 7BX.
>
RE: Security Subject of thin client on remote nodes
Posted by Veena Mithare <V....@cmcmarkets.com>.
Hi Alexei, Denis,
One of the main usecases of thin client authentication is to be able to audit the changes done using the thin client user.
To enable that :
We really need to resolve this concern as well :
https://issues.apache.org/jira/browse/IGNITE-12781
( Incorrect security subject id is associated with a cache_put event when the originator of the event is a thin client. )
Regards,
Veena
-----Original Message-----
From: Alexei Scherbakov <al...@gmail.com>
Sent: 18 March 2020 08:11
To: dev <de...@ignite.apache.org>
Subject: Re: Security Subject of thin client on remote nodes
Denis Garus,
Both variants are capable of solving the thin client security context problem.
My approach doesn't require any IEPs, just minor change in code and to
org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
contract.
We can add appropriate documentation to emphasize this.
The argument "fragile" is not very convincing for me.
I think we should collect more opinions before proceeding with IEP.
Considering a fact we actually *may not care* about compatibility (I've already explained why), I'm thinking of another approach.
Let's get rid of SecurityContext and use SecuritySubject instead.
SecurityContext is just a POJO wrapper over SecuritySubject's org.apache.ignite.plugin.security.SecuritySubject#permissions.
It's functionality can be easily moved to SecuritySubject.
What do you think?
пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> Hello, Alexei!
>
> I agree with you if we may not care about compatibility at all, then
> we can solve the problem much more straightforward way.
>
> In your case, the method GridSecurityProcessor#authenticate will have
> an implicit contract:
> [ if actx.subject() != null then
> returns SecurityContext
> else
> do authenticate ]
>
> It looks fragile.
>
> When we extend the GridSecurityProcessor, there isn't this problem:
> we have the explicit contract and can make default implementation that
> throws an unsupported operation exception to enforcing compatibility
> check.
>
> In any case, we need to change GridSecurityProcessor implementation.
>
> But I think your proposal to try to find a security context in the
> node's attributes first is right for backward compatibility when
> Ignite users don't use thin clients.
>
> Summary:
> I suggest adding a new method to GridSecurityProcessor because it has
> a clear contract and enforces compatibility check natural way.
>
> вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> alexey.scherbakoff@gmail.com
> >:
>
> > Denis Garus,
> >
> > I've looked at the IEP proposed by you and currently I'm thinking
> > it's
> not
> > immediately required.
> >
> > The problem of missing SecurityContexts of thin clients can be
> > solved
> much
> > easily.
> >
> > Below is the stub of a fix, it requires correct implementation of
> > method
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> #authenticatedSubject
> > by GridSecurityProcessor:
> >
> > /** {@inheritDoc} */
> > @Override public OperationSecurityContext withContext(UUID nodeId) {
> > try {
> > SecurityContext ctx0 = secCtxs.get(nodeId);
> >
> > if (ctx0 == null) {
> > ClusterNode node =
> > Optional.ofNullable(ctx.discovery().node(nodeId))
> > .orElseGet(() ->
> > ctx.discovery().historicalNode(nodeId));
> >
> > // This is a cluster node.
> > if (node != null)
> > ctx0 = nodeSecurityContext(marsh,
> > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > else {
> > // This is already authenticated thin client.
> > SecuritySubject subj =
> > authenticatedSubject(nodeId);
> >
> > assert subj != null : "Subject is null " +
> > nodeId;
> >
> > AuthenticationContext actx = new
> > AuthenticationContext();
> > actx.subject(subj);
> >
> > ctx0 = secPrc.authenticate(actx);
> > }
> > }
> >
> > secCtxs.putIfAbsent(nodeId, ctx0);
> >
> > return withContext(ctx0);
> > } catch (IgniteCheckedException e) {
> > throw new IgniteException(e);
> > }
> >
> > The idea is to create a thin client SecurityContext on a node not
> > having
> a
> > local context using existing SecuritySubject data.
> >
> > Method
> >
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#a
> uthenticate
> > should check for not null SecuritySubject field and just recreate
> > SecurityContext using passed info (because it's already authenticated).
> >
> > We have all necessary information in SecuritySubject returned by
> >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor
> #authenticatedSubject
> > by GridSecurityProcessor method.
> >
> > Because it is internal API, we may not care about compatibility at
> > all, but nevertheless it is possible to add compatibility check in
> > the method above. If a feature is not supported the operations from
> > thin clients should be forbidden.
> >
> > You proposal has the similar problem: if GridSecurityProcessor does
> > not support retriving context for thin clients, such clients will
> > not be able to proceed with operation.
> >
> > Still, the cleanup of security API is necessary and should be done
> > in 3.0
> >
> >
> >
> > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v....@cmcmarkets.com>:
> >
> > > HI ,
> > >
> > > Created this jira :
> > > https://issues.apache.org/jira/browse/IGNITE-12781
> > >
> > > regards,
> > > Veena.
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>
--
Best regards,
Alexei Scherbakov
________________________________
Spread bets and CFDs are complex instruments and come with a high risk of losing money rapidly due to leverage. 70.5% of retail investor accounts lose money when spread betting and/or trading CFDs with this provider. You should consider whether you understand how spread bets and CFDs work and whether you can afford to take the high risk of losing your money.
Professional clients: Losses can exceed deposits when spread betting and trading CFDs. Countdowns carry a level of risk to your capital as you could lose all of your investment. These products may not be suitable for all clients therefore ensure you understand the risks and seek independent advice. Invest only what you can afford to lose.
CMC Markets UK plc (173730) and CMC Spreadbet plc (170627) are authorised and regulated by the Financial Conduct Authority in the United Kingdom. CMC Markets UK plc and CMC Spreadbet plc are registered in England and Wales with Company Numbers 02448409 and 02589529 and with their registered offices at 133 Houndsditch, London, EC3A 7BX.
The content of this e-mail (including any attachments) is strictly confidential and is for the sole use of the intended addressee(s). If you are not the intended recipient of this e-mail please notify the sender immediately and delete this e-mail from your system. Any disclosure, copying, dissemination or use of its content (including any attachments) is strictly prohibited. CMC Markets UK plc and CMC Spreadbet plc reserve the right to intercept and monitor the content of the e-mail messages to and from its systems.
E-mails may be interfered with or may contain viruses or other defects for which CMC Markets UK plc and CMC Spreadbet plc accept no responsibility. It is the responsibility of the recipient to carry out a virus check on the e-mail and any attachment(s).
This communication is not intended as an offer or solicitation for the purchase or sale of a financial instrument or as an official confirmation of any transaction unless specifically presented as such.
CMC Markets UK plc and CMC Spreadbet plc are registered in England and Wales with Company Numbers 02448409 and 02589529 and with their registered offices at 133 Houndsditch, London, EC3A 7BX.
Re: Security Subject of thin client on remote nodes
Posted by Alexei Scherbakov <al...@gmail.com>.
Denis Garus,
Both variants are capable of solving the thin client security context
problem.
My approach doesn't require any IEPs, just minor change in code and to
org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
contract.
We can add appropriate documentation to emphasize this.
The argument "fragile" is not very convincing for me.
I think we should collect more opinions before proceeding with IEP.
Considering a fact we actually *may not care* about compatibility (I've
already explained why), I'm thinking of another approach.
Let's get rid of SecurityContext and use SecuritySubject instead.
SecurityContext is just a POJO wrapper over
SecuritySubject's org.apache.ignite.plugin.security.SecuritySubject#permissions.
It's functionality can be easily moved to SecuritySubject.
What do you think?
пн, 16 мар. 2020 г. в 15:47, Denis Garus <ga...@gmail.com>:
> Hello, Alexei!
>
> I agree with you if we may not care about compatibility at all,
> then we can solve the problem much more straightforward way.
>
> In your case, the method GridSecurityProcessor#authenticate will have an
> implicit contract:
> [ if actx.subject() != null then
> returns SecurityContext
> else
> do authenticate ]
>
> It looks fragile.
>
> When we extend the GridSecurityProcessor, there isn't this problem:
> we have the explicit contract and can make default implementation
> that throws an unsupported operation exception to enforcing compatibility
> check.
>
> In any case, we need to change GridSecurityProcessor implementation.
>
> But I think your proposal to try to find a security context in the node's
> attributes first is right
> for backward compatibility when Ignite users don't use thin clients.
>
> Summary:
> I suggest adding a new method to GridSecurityProcessor because
> it has a clear contract and enforces compatibility check natural way.
>
> вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> alexey.scherbakoff@gmail.com
> >:
>
> > Denis Garus,
> >
> > I've looked at the IEP proposed by you and currently I'm thinking it's
> not
> > immediately required.
> >
> > The problem of missing SecurityContexts of thin clients can be solved
> much
> > easily.
> >
> > Below is the stub of a fix, it requires correct implementation of
> > method
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> > by GridSecurityProcessor:
> >
> > /** {@inheritDoc} */
> > @Override public OperationSecurityContext withContext(UUID nodeId) {
> > try {
> > SecurityContext ctx0 = secCtxs.get(nodeId);
> >
> > if (ctx0 == null) {
> > ClusterNode node =
> > Optional.ofNullable(ctx.discovery().node(nodeId))
> > .orElseGet(() ->
> > ctx.discovery().historicalNode(nodeId));
> >
> > // This is a cluster node.
> > if (node != null)
> > ctx0 = nodeSecurityContext(marsh,
> > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > else {
> > // This is already authenticated thin client.
> > SecuritySubject subj = authenticatedSubject(nodeId);
> >
> > assert subj != null : "Subject is null " + nodeId;
> >
> > AuthenticationContext actx = new
> > AuthenticationContext();
> > actx.subject(subj);
> >
> > ctx0 = secPrc.authenticate(actx);
> > }
> > }
> >
> > secCtxs.putIfAbsent(nodeId, ctx0);
> >
> > return withContext(ctx0);
> > } catch (IgniteCheckedException e) {
> > throw new IgniteException(e);
> > }
> >
> > The idea is to create a thin client SecurityContext on a node not having
> a
> > local context using existing SecuritySubject data.
> >
> > Method
> >
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#authenticate
> > should check for not null SecuritySubject field and just recreate
> > SecurityContext using passed info (because it's already authenticated).
> >
> > We have all necessary information in SecuritySubject returned by
> >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> > by GridSecurityProcessor method.
> >
> > Because it is internal API, we may not care about compatibility at all,
> > but nevertheless it is possible to add compatibility check in the method
> > above. If a feature is not supported the operations from thin clients
> > should be forbidden.
> >
> > You proposal has the similar problem: if GridSecurityProcessor does not
> > support retriving context for thin clients, such clients will not be able
> > to proceed with operation.
> >
> > Still, the cleanup of security API is necessary and should be done in 3.0
> >
> >
> >
> > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v....@cmcmarkets.com>:
> >
> > > HI ,
> > >
> > > Created this jira :
> > > https://issues.apache.org/jira/browse/IGNITE-12781
> > >
> > > regards,
> > > Veena.
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>
--
Best regards,
Alexei Scherbakov
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Hello, Alexei!
I agree with you if we may not care about compatibility at all,
then we can solve the problem much more straightforward way.
In your case, the method GridSecurityProcessor#authenticate will have an
implicit contract:
[ if actx.subject() != null then
returns SecurityContext
else
do authenticate ]
It looks fragile.
When we extend the GridSecurityProcessor, there isn't this problem:
we have the explicit contract and can make default implementation
that throws an unsupported operation exception to enforcing compatibility
check.
In any case, we need to change GridSecurityProcessor implementation.
But I think your proposal to try to find a security context in the node's
attributes first is right
for backward compatibility when Ignite users don't use thin clients.
Summary:
I suggest adding a new method to GridSecurityProcessor because
it has a clear contract and enforces compatibility check natural way.
вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <alexey.scherbakoff@gmail.com
>:
> Denis Garus,
>
> I've looked at the IEP proposed by you and currently I'm thinking it's not
> immediately required.
>
> The problem of missing SecurityContexts of thin clients can be solved much
> easily.
>
> Below is the stub of a fix, it requires correct implementation of
> method
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> by GridSecurityProcessor:
>
> /** {@inheritDoc} */
> @Override public OperationSecurityContext withContext(UUID nodeId) {
> try {
> SecurityContext ctx0 = secCtxs.get(nodeId);
>
> if (ctx0 == null) {
> ClusterNode node =
> Optional.ofNullable(ctx.discovery().node(nodeId))
> .orElseGet(() ->
> ctx.discovery().historicalNode(nodeId));
>
> // This is a cluster node.
> if (node != null)
> ctx0 = nodeSecurityContext(marsh,
> U.resolveClassLoader(ctx.config()), findNode(nodeId));
> else {
> // This is already authenticated thin client.
> SecuritySubject subj = authenticatedSubject(nodeId);
>
> assert subj != null : "Subject is null " + nodeId;
>
> AuthenticationContext actx = new
> AuthenticationContext();
> actx.subject(subj);
>
> ctx0 = secPrc.authenticate(actx);
> }
> }
>
> secCtxs.putIfAbsent(nodeId, ctx0);
>
> return withContext(ctx0);
> } catch (IgniteCheckedException e) {
> throw new IgniteException(e);
> }
>
> The idea is to create a thin client SecurityContext on a node not having a
> local context using existing SecuritySubject data.
>
> Method
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#authenticate
> should check for not null SecuritySubject field and just recreate
> SecurityContext using passed info (because it's already authenticated).
>
> We have all necessary information in SecuritySubject returned by
>
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> by GridSecurityProcessor method.
>
> Because it is internal API, we may not care about compatibility at all,
> but nevertheless it is possible to add compatibility check in the method
> above. If a feature is not supported the operations from thin clients
> should be forbidden.
>
> You proposal has the similar problem: if GridSecurityProcessor does not
> support retriving context for thin clients, such clients will not be able
> to proceed with operation.
>
> Still, the cleanup of security API is necessary and should be done in 3.0
>
>
>
> чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v....@cmcmarkets.com>:
>
> > HI ,
> >
> > Created this jira :
> > https://issues.apache.org/jira/browse/IGNITE-12781
> >
> > regards,
> > Veena.
> >
> >
> >
> > --
> > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> >
>
>
> --
>
> Best regards,
> Alexei Scherbakov
>
Re: Security Subject of thin client on remote nodes
Posted by Alexei Scherbakov <al...@gmail.com>.
Denis Garus,
I've looked at the IEP proposed by you and currently I'm thinking it's not
immediately required.
The problem of missing SecurityContexts of thin clients can be solved much
easily.
Below is the stub of a fix, it requires correct implementation of
method org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
by GridSecurityProcessor:
/** {@inheritDoc} */
@Override public OperationSecurityContext withContext(UUID nodeId) {
try {
SecurityContext ctx0 = secCtxs.get(nodeId);
if (ctx0 == null) {
ClusterNode node =
Optional.ofNullable(ctx.discovery().node(nodeId))
.orElseGet(() ->
ctx.discovery().historicalNode(nodeId));
// This is a cluster node.
if (node != null)
ctx0 = nodeSecurityContext(marsh,
U.resolveClassLoader(ctx.config()), findNode(nodeId));
else {
// This is already authenticated thin client.
SecuritySubject subj = authenticatedSubject(nodeId);
assert subj != null : "Subject is null " + nodeId;
AuthenticationContext actx = new
AuthenticationContext();
actx.subject(subj);
ctx0 = secPrc.authenticate(actx);
}
}
secCtxs.putIfAbsent(nodeId, ctx0);
return withContext(ctx0);
} catch (IgniteCheckedException e) {
throw new IgniteException(e);
}
The idea is to create a thin client SecurityContext on a node not having a
local context using existing SecuritySubject data.
Method org.apache.ignite.internal.processors.security.GridSecurityProcessor#authenticate
should check for not null SecuritySubject field and just recreate
SecurityContext using passed info (because it's already authenticated).
We have all necessary information in SecuritySubject returned by
org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
by GridSecurityProcessor method.
Because it is internal API, we may not care about compatibility at all,
but nevertheless it is possible to add compatibility check in the method
above. If a feature is not supported the operations from thin clients
should be forbidden.
You proposal has the similar problem: if GridSecurityProcessor does not
support retriving context for thin clients, such clients will not be able
to proceed with operation.
Still, the cleanup of security API is necessary and should be done in 3.0
чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v....@cmcmarkets.com>:
> HI ,
>
> Created this jira :
> https://issues.apache.org/jira/browse/IGNITE-12781
>
> regards,
> Veena.
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
--
Best regards,
Alexei Scherbakov
Re: Security Subject of thin client on remote nodes
Posted by VeenaMithare <v....@cmcmarkets.com>.
HI ,
Created this jira :
https://issues.apache.org/jira/browse/IGNITE-12781
regards,
Veena.
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
> I dont think that is true.
/**
* *Gets authenticated node subject.*
*
* @param subjId Subject ID.
* @return Security subject.
* @throws IgniteCheckedException If error occurred.
*/
public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException;
> Nothing stops an implementer to return a Remote Client Security Subject.
We must provide backward compatibility inside minor versions
that is why we cannot expect inside the Ignite code base
that an implementer returns a Remote Client Security Subject.
> The primary issue is how to link the event which contains the remote node
uuid ( but was originated by a remote client ) , to the originator remote
client.
Could you please create the issue with label iep-41?
We'll fix it soon.
ср, 11 мар. 2020 г. в 16:39, VeenaMithare <v....@cmcmarkets.com>:
> >>2. Method GridSecurityProcessor#authenticatedSubject returns
> authenticated *node
> subject* (from JavaDoc).
>
> I dont think that is true. Nothing stops an implementer to return a Remote
> Client Security Subject.
>
> The primary issue is how to link the event which contains the remote node
> uuid ( but was originated by a remote client ) , to the originator remote
> client.
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
Re: Security Subject of thin client on remote nodes
Posted by VeenaMithare <v....@cmcmarkets.com>.
>>2. Method GridSecurityProcessor#authenticatedSubject returns
authenticated *node
subject* (from JavaDoc).
I dont think that is true. Nothing stops an implementer to return a Remote
Client Security Subject.
The primary issue is how to link the event which contains the remote node
uuid ( but was originated by a remote client ) , to the originator remote
client.
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
1.Return type. SecurityContext is implemented by security plugin
developers,
and we cannot make SecurityContext from SecuritySubject.
But GridSecurityProcessor#authorize method requires SecurityContext as
parameter.
2. Method GridSecurityProcessor#authenticatedSubject returns
authenticated *node
subject* (from JavaDoc).
It means we don't get a subject as a remote client using this method.
GridSecurityProcessor#securityContext will not have this restriction.
But I agree with you interface GridSecurityProcessor is too tricky.
We could do a reworking of this interface for the 3.0 version of Ignite.
ср, 11 мар. 2020 г. в 14:06, VeenaMithare <v....@cmcmarkets.com>:
> Hi Denis,
>
> Thank you for working on the security issues w.r.to thin client logins .
> We
> plan to use the thin clients extensively in our project , it will really
> help if the blockers around it are resolved,
>
> regards,
> Veena.
>
>
>
> --
> Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
>
Re: Security Subject of thin client on remote nodes
Posted by VeenaMithare <v....@cmcmarkets.com>.
The biggest blocker is the one I mention here :
http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: Security Subject of thin client on remote nodes
Posted by VeenaMithare <v....@cmcmarkets.com>.
Hi Denis,
Thank you for working on the security issues w.r.to thin client logins . We
plan to use the thin clients extensively in our project , it will really
help if the blockers around it are resolved,
regards,
Veena.
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: Security Subject of thin client on remote nodes
Posted by Ilya Kasnacheev <il...@gmail.com>.
Hello!
Please avoid targeting feature tickets on 2.8.1 by default.
As far as my understanding goes, plan for 2.8.1 is to be a bugfix release
with specific changes cherry picked, so new features are unlikely to be
included, and should be negotiated with person responsible for this release
before being included in scope.
Regards,
--
Ilya Kasnacheev
вт, 10 мар. 2020 г. в 12:00, Denis Garus <ga...@gmail.com>:
> Hi guys!
> I created the iep ticket [1] and started work.
>
> 1. https://issues.apache.org/jira/browse/IGNITE-12759
>
> чт, 5 мар. 2020 г. в 12:00, Denis Garus <ga...@gmail.com>:
>
> > Hi, guys!
> >
> >
> > I've prepared the IEP-41: Security Context of a thin client on remote
> > nodes [1]; take a look, please.
> >
> > If there aren't any questions, I could create an issue and start work.
> >
> >
> > Ivan, could you be an IEP sponsor?
> >
> >
> > Thx!
> >
> >
> >
> > 1.
> >
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+a+thin+client+on+remote+nodes
> >
> >
> > ср, 26 февр. 2020 г. в 12:42, Mikhail Petrov <pm...@gmail.com>:
> >
> >> Hi, Alexei.
> >>
> >> The ticket [1] describes the general problem for all thin client
> >> authorizations. Its particular case is described in the mentioned in [1]
> >> ticket [2] on the example of a JDBC client with the reproducer
> attached.
> >>
> >> [1] https://issues.apache.org/jira/browse/IGNITE-12589
> >> [2] https://issues.apache.org/jira/browse/IGNITE-12579
> >>
> >> On 26.02.2020 11:47, Alexei Scherbakov wrote:
> >> > Denis Garus,
> >> >
> >> > It is forbidden to remove any public IGNITE attribute without proper
> >> > deprecation steps.
> >> >
> >> > I have read the thread and still do not clearly see the problem. The
> >> subject id
> >> > is not required to be a node id.
> >> >
> >> > The referenced in the thread ticket [1] do not provide any
> reproducers.
> >> >
> >> > Can you properly demonstrate a broken scenario ?
> >> >
> >> > [1] https://issues.apache.org/jira/browse/IGNITE-12589
> >> >
> >> > пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov <st...@gmail.com>:
> >> >
> >> >> Hi, guys!
> >> >>
> >> >> The change suggested by Denis looks robust to me: it covers security
> >> >> subject handling by all kinds of clients/nodes at once. As for
> >> >> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
> >> >> plugin implementations to support backward compatibility with peer
> >> nodes of
> >> >> older versions. Obviously, cluster with security disabled will not
> >> suffer
> >> >> from attribute removal. Ignite core should know nothing about the
> >> specific
> >> >> way of security context propagation.
> >> >>
> >> >> Denis, could you please create Jira issue for your change?
> >> >>
> >> >> чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
> >> >>
> >> >>>> I just transmitted security subjects for rest requests.
> >> >>> SecurityContext has an unlimited size so we can get significant
> >> overhead.
> >> >>> And we do not solve problems with other thin clients.
> >> >>>
> >> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
> >> between
> >> >>> old
> >> >>> versions and new.
> >> >>>
> >> >>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase,
> >> but
> >> >> for
> >> >>> compatibility, it can be used by a security plugin like in PoC.
> >> >>>
> >> >>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <
> >> >> maksim.stepachev@gmail.com
> >> >>>> :
> >> >>>> Yes, I said about it at 07.19.
> >> >>>>
> >> >>>>
> >> >>
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
> >> >>>> And in my solution, I just transmitted security subjects for rest
> >> >>> requests.
> >> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
> >> between
> >> >>> old
> >> >>>> versions and new.
> >> >>>>
> >> >>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
> >> >>>>
> >> >>>>> Hi, Igniters!
> >> >>>>>
> >> >>>>>
> >> >>>>> At present, a security subject id is assumed to be node id.
> >> >>>>>
> >> >>>>> But when we are dealing with thin client, JDBC or REST subject id
> is
> >> >>>> random
> >> >>>>> UUID. In this case, we cannot get the subject information on a
> >> remote
> >> >>>> node,
> >> >>>>> and we get problems like these [1], [2].
> >> >>>>>
> >> >>>>> To fix the problem, we should spread the client session to the
> whole
> >> >>>>> cluster.
> >> >>>>>
> >> >>>>>
> >> >>>>> I want to suggest a solution to the problem.
> >> >>>>>
> >> >>>>>
> >> >>>>> First, we should get subject information using
> >> GridSecurityProcessor.
> >> >>>>>
> >> >>>>> How GridSecurityProcessor will retrieve a subject data, it is up
> to
> >> >>>> plugin
> >> >>>>> developers.
> >> >>>>>
> >> >>>>>
> >> >>>>> Second, we should get rid of the assumption that a subject id is a
> >> >> node
> >> >>>> id
> >> >>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
> >> >>>>>
> >> >>>>>
> >> >>>>> I have prepared PoC PR [3] that:
> >> >>>>>
> >> >>>>> - places the existing logic of spreading security context to
> >> >>>>> GridSecurityProcessor;
> >> >>>>>
> >> >>>>> - uses GridSecurityProcessor to get SecurityContext.
> >> >>>>>
> >> >>>>>
> >> >>>>>
> >> >>>>> 1.
> >> >>>>>
> >> >>>>>
> >> >>
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> >> >>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589
> >> >>>>> 3. https://github.com/apache/ignite/pull/7375
> >> >>>>>
> >> >>
> >> >> --
> >> >> Best regards,
> >> >> Andrey Kuznetsov.
> >> >>
> >> >
> >>
> >
>
Re: Security Subject of thin client on remote nodes
Posted by VeenaMithare <v....@cmcmarkets.com>.
HI Denis,
Ref :
https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+thin+client+on+remote+nodes
How is this method
public interface GridSecurityProcessor extends GridProcessor {
// Other methods.
/**
* Gets security context.
*
* @param subjId Security subject id.
* @return Security context or null if not found.
*/
public default SecurityContext securityContext(UUID subjId){
return null; //for backward compatibility
}
}
Different from :
/**
* Gets authenticated node subject.
*
* @param subjId Subject ID.
* @return Security subject.
* @throws IgniteCheckedException If error occurred.
*/
public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException;
securityContext(UUID subjId) method will give us SecurityContext,
authenticatedSubject will give us SecuritySubject. We do the
securitysubject's permission check on authorize method. What is the usecase
for which we would want to use securityContext(UUID subjId) instead of
authenticatedSubject or authorize?
--
Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Hi guys!
I created the iep ticket [1] and started work.
1. https://issues.apache.org/jira/browse/IGNITE-12759
чт, 5 мар. 2020 г. в 12:00, Denis Garus <ga...@gmail.com>:
> Hi, guys!
>
>
> I've prepared the IEP-41: Security Context of a thin client on remote
> nodes [1]; take a look, please.
>
> If there aren't any questions, I could create an issue and start work.
>
>
> Ivan, could you be an IEP sponsor?
>
>
> Thx!
>
>
>
> 1.
> https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+a+thin+client+on+remote+nodes
>
>
> ср, 26 февр. 2020 г. в 12:42, Mikhail Petrov <pm...@gmail.com>:
>
>> Hi, Alexei.
>>
>> The ticket [1] describes the general problem for all thin client
>> authorizations. Its particular case is described in the mentioned in [1]
>> ticket [2] on the example of a JDBC client with the reproducer attached.
>>
>> [1] https://issues.apache.org/jira/browse/IGNITE-12589
>> [2] https://issues.apache.org/jira/browse/IGNITE-12579
>>
>> On 26.02.2020 11:47, Alexei Scherbakov wrote:
>> > Denis Garus,
>> >
>> > It is forbidden to remove any public IGNITE attribute without proper
>> > deprecation steps.
>> >
>> > I have read the thread and still do not clearly see the problem. The
>> subject id
>> > is not required to be a node id.
>> >
>> > The referenced in the thread ticket [1] do not provide any reproducers.
>> >
>> > Can you properly demonstrate a broken scenario ?
>> >
>> > [1] https://issues.apache.org/jira/browse/IGNITE-12589
>> >
>> > пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov <st...@gmail.com>:
>> >
>> >> Hi, guys!
>> >>
>> >> The change suggested by Denis looks robust to me: it covers security
>> >> subject handling by all kinds of clients/nodes at once. As for
>> >> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
>> >> plugin implementations to support backward compatibility with peer
>> nodes of
>> >> older versions. Obviously, cluster with security disabled will not
>> suffer
>> >> from attribute removal. Ignite core should know nothing about the
>> specific
>> >> way of security context propagation.
>> >>
>> >> Denis, could you please create Jira issue for your change?
>> >>
>> >> чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
>> >>
>> >>>> I just transmitted security subjects for rest requests.
>> >>> SecurityContext has an unlimited size so we can get significant
>> overhead.
>> >>> And we do not solve problems with other thin clients.
>> >>>
>> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
>> between
>> >>> old
>> >>> versions and new.
>> >>>
>> >>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase,
>> but
>> >> for
>> >>> compatibility, it can be used by a security plugin like in PoC.
>> >>>
>> >>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <
>> >> maksim.stepachev@gmail.com
>> >>>> :
>> >>>> Yes, I said about it at 07.19.
>> >>>>
>> >>>>
>> >>
>> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
>> >>>> And in my solution, I just transmitted security subjects for rest
>> >>> requests.
>> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
>> between
>> >>> old
>> >>>> versions and new.
>> >>>>
>> >>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
>> >>>>
>> >>>>> Hi, Igniters!
>> >>>>>
>> >>>>>
>> >>>>> At present, a security subject id is assumed to be node id.
>> >>>>>
>> >>>>> But when we are dealing with thin client, JDBC or REST subject id is
>> >>>> random
>> >>>>> UUID. In this case, we cannot get the subject information on a
>> remote
>> >>>> node,
>> >>>>> and we get problems like these [1], [2].
>> >>>>>
>> >>>>> To fix the problem, we should spread the client session to the whole
>> >>>>> cluster.
>> >>>>>
>> >>>>>
>> >>>>> I want to suggest a solution to the problem.
>> >>>>>
>> >>>>>
>> >>>>> First, we should get subject information using
>> GridSecurityProcessor.
>> >>>>>
>> >>>>> How GridSecurityProcessor will retrieve a subject data, it is up to
>> >>>> plugin
>> >>>>> developers.
>> >>>>>
>> >>>>>
>> >>>>> Second, we should get rid of the assumption that a subject id is a
>> >> node
>> >>>> id
>> >>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
>> >>>>>
>> >>>>>
>> >>>>> I have prepared PoC PR [3] that:
>> >>>>>
>> >>>>> - places the existing logic of spreading security context to
>> >>>>> GridSecurityProcessor;
>> >>>>>
>> >>>>> - uses GridSecurityProcessor to get SecurityContext.
>> >>>>>
>> >>>>>
>> >>>>>
>> >>>>> 1.
>> >>>>>
>> >>>>>
>> >>
>> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
>> >>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589
>> >>>>> 3. https://github.com/apache/ignite/pull/7375
>> >>>>>
>> >>
>> >> --
>> >> Best regards,
>> >> Andrey Kuznetsov.
>> >>
>> >
>>
>
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
Hi, guys!
I've prepared the IEP-41: Security Context of a thin client on remote nodes
[1]; take a look, please.
If there aren't any questions, I could create an issue and start work.
Ivan, could you be an IEP sponsor?
Thx!
1.
https://cwiki.apache.org/confluence/display/IGNITE/IEP-41%3A+Security+Context+of+a+thin+client+on+remote+nodes
ср, 26 февр. 2020 г. в 12:42, Mikhail Petrov <pm...@gmail.com>:
> Hi, Alexei.
>
> The ticket [1] describes the general problem for all thin client
> authorizations. Its particular case is described in the mentioned in [1]
> ticket [2] on the example of a JDBC client with the reproducer attached.
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12589
> [2] https://issues.apache.org/jira/browse/IGNITE-12579
>
> On 26.02.2020 11:47, Alexei Scherbakov wrote:
> > Denis Garus,
> >
> > It is forbidden to remove any public IGNITE attribute without proper
> > deprecation steps.
> >
> > I have read the thread and still do not clearly see the problem. The
> subject id
> > is not required to be a node id.
> >
> > The referenced in the thread ticket [1] do not provide any reproducers.
> >
> > Can you properly demonstrate a broken scenario ?
> >
> > [1] https://issues.apache.org/jira/browse/IGNITE-12589
> >
> > пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov <st...@gmail.com>:
> >
> >> Hi, guys!
> >>
> >> The change suggested by Denis looks robust to me: it covers security
> >> subject handling by all kinds of clients/nodes at once. As for
> >> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
> >> plugin implementations to support backward compatibility with peer
> nodes of
> >> older versions. Obviously, cluster with security disabled will not
> suffer
> >> from attribute removal. Ignite core should know nothing about the
> specific
> >> way of security context propagation.
> >>
> >> Denis, could you please create Jira issue for your change?
> >>
> >> чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
> >>
> >>>> I just transmitted security subjects for rest requests.
> >>> SecurityContext has an unlimited size so we can get significant
> overhead.
> >>> And we do not solve problems with other thin clients.
> >>>
> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
> between
> >>> old
> >>> versions and new.
> >>>
> >>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but
> >> for
> >>> compatibility, it can be used by a security plugin like in PoC.
> >>>
> >>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <
> >> maksim.stepachev@gmail.com
> >>>> :
> >>>> Yes, I said about it at 07.19.
> >>>>
> >>>>
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
> >>>> And in my solution, I just transmitted security subjects for rest
> >>> requests.
> >>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility
> between
> >>> old
> >>>> versions and new.
> >>>>
> >>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
> >>>>
> >>>>> Hi, Igniters!
> >>>>>
> >>>>>
> >>>>> At present, a security subject id is assumed to be node id.
> >>>>>
> >>>>> But when we are dealing with thin client, JDBC or REST subject id is
> >>>> random
> >>>>> UUID. In this case, we cannot get the subject information on a remote
> >>>> node,
> >>>>> and we get problems like these [1], [2].
> >>>>>
> >>>>> To fix the problem, we should spread the client session to the whole
> >>>>> cluster.
> >>>>>
> >>>>>
> >>>>> I want to suggest a solution to the problem.
> >>>>>
> >>>>>
> >>>>> First, we should get subject information using GridSecurityProcessor.
> >>>>>
> >>>>> How GridSecurityProcessor will retrieve a subject data, it is up to
> >>>> plugin
> >>>>> developers.
> >>>>>
> >>>>>
> >>>>> Second, we should get rid of the assumption that a subject id is a
> >> node
> >>>> id
> >>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
> >>>>>
> >>>>>
> >>>>> I have prepared PoC PR [3] that:
> >>>>>
> >>>>> - places the existing logic of spreading security context to
> >>>>> GridSecurityProcessor;
> >>>>>
> >>>>> - uses GridSecurityProcessor to get SecurityContext.
> >>>>>
> >>>>>
> >>>>>
> >>>>> 1.
> >>>>>
> >>>>>
> >>
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> >>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589
> >>>>> 3. https://github.com/apache/ignite/pull/7375
> >>>>>
> >>
> >> --
> >> Best regards,
> >> Andrey Kuznetsov.
> >>
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Mikhail Petrov <pm...@gmail.com>.
Hi, Alexei.
The ticket [1] describes the general problem for all thin client
authorizations. Its particular case is described in the mentioned in [1]
ticket [2] on the example of a JDBC client with the reproducer attached.
[1] https://issues.apache.org/jira/browse/IGNITE-12589
[2] https://issues.apache.org/jira/browse/IGNITE-12579
On 26.02.2020 11:47, Alexei Scherbakov wrote:
> Denis Garus,
>
> It is forbidden to remove any public IGNITE attribute without proper
> deprecation steps.
>
> I have read the thread and still do not clearly see the problem. The subject id
> is not required to be a node id.
>
> The referenced in the thread ticket [1] do not provide any reproducers.
>
> Can you properly demonstrate a broken scenario ?
>
> [1] https://issues.apache.org/jira/browse/IGNITE-12589
>
> пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov <st...@gmail.com>:
>
>> Hi, guys!
>>
>> The change suggested by Denis looks robust to me: it covers security
>> subject handling by all kinds of clients/nodes at once. As for
>> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
>> plugin implementations to support backward compatibility with peer nodes of
>> older versions. Obviously, cluster with security disabled will not suffer
>> from attribute removal. Ignite core should know nothing about the specific
>> way of security context propagation.
>>
>> Denis, could you please create Jira issue for your change?
>>
>> чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
>>
>>>> I just transmitted security subjects for rest requests.
>>> SecurityContext has an unlimited size so we can get significant overhead.
>>> And we do not solve problems with other thin clients.
>>>
>>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
>>> old
>>> versions and new.
>>>
>>> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but
>> for
>>> compatibility, it can be used by a security plugin like in PoC.
>>>
>>> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <
>> maksim.stepachev@gmail.com
>>>> :
>>>> Yes, I said about it at 07.19.
>>>>
>>>>
>> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
>>>> And in my solution, I just transmitted security subjects for rest
>>> requests.
>>>> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
>>> old
>>>> versions and new.
>>>>
>>>> чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
>>>>
>>>>> Hi, Igniters!
>>>>>
>>>>>
>>>>> At present, a security subject id is assumed to be node id.
>>>>>
>>>>> But when we are dealing with thin client, JDBC or REST subject id is
>>>> random
>>>>> UUID. In this case, we cannot get the subject information on a remote
>>>> node,
>>>>> and we get problems like these [1], [2].
>>>>>
>>>>> To fix the problem, we should spread the client session to the whole
>>>>> cluster.
>>>>>
>>>>>
>>>>> I want to suggest a solution to the problem.
>>>>>
>>>>>
>>>>> First, we should get subject information using GridSecurityProcessor.
>>>>>
>>>>> How GridSecurityProcessor will retrieve a subject data, it is up to
>>>> plugin
>>>>> developers.
>>>>>
>>>>>
>>>>> Second, we should get rid of the assumption that a subject id is a
>> node
>>>> id
>>>>> and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
>>>>>
>>>>>
>>>>> I have prepared PoC PR [3] that:
>>>>>
>>>>> - places the existing logic of spreading security context to
>>>>> GridSecurityProcessor;
>>>>>
>>>>> - uses GridSecurityProcessor to get SecurityContext.
>>>>>
>>>>>
>>>>>
>>>>> 1.
>>>>>
>>>>>
>> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
>>>>> 2. https://issues.apache.org/jira/browse/IGNITE-12589
>>>>> 3. https://github.com/apache/ignite/pull/7375
>>>>>
>>
>> --
>> Best regards,
>> Andrey Kuznetsov.
>>
>
Re: Security Subject of thin client on remote nodes
Posted by Alexei Scherbakov <al...@gmail.com>.
Denis Garus,
It is forbidden to remove any public IGNITE attribute without proper
deprecation steps.
I have read the thread and still do not clearly see the problem. The subject id
is not required to be a node id.
The referenced in the thread ticket [1] do not provide any reproducers.
Can you properly demonstrate a broken scenario ?
[1] https://issues.apache.org/jira/browse/IGNITE-12589
пт, 21 февр. 2020 г. в 13:35, Andrey Kuznetsov <st...@gmail.com>:
> Hi, guys!
>
> The change suggested by Denis looks robust to me: it covers security
> subject handling by all kinds of clients/nodes at once. As for
> ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
> plugin implementations to support backward compatibility with peer nodes of
> older versions. Obviously, cluster with security disabled will not suffer
> from attribute removal. Ignite core should know nothing about the specific
> way of security context propagation.
>
> Denis, could you please create Jira issue for your change?
>
> чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
>
> > > I just transmitted security subjects for rest requests.
> >
> > SecurityContext has an unlimited size so we can get significant overhead.
> > And we do not solve problems with other thin clients.
> >
> > >If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
> > old
> > versions and new.
> >
> > I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but
> for
> > compatibility, it can be used by a security plugin like in PoC.
> >
> > чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <
> maksim.stepachev@gmail.com
> > >:
> >
> > > Yes, I said about it at 07.19.
> > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
> > > And in my solution, I just transmitted security subjects for rest
> > requests.
> > >
> > > If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
> > old
> > > versions and new.
> > >
> > > чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
> > >
> > > > Hi, Igniters!
> > > >
> > > >
> > > > At present, a security subject id is assumed to be node id.
> > > >
> > > > But when we are dealing with thin client, JDBC or REST subject id is
> > > random
> > > > UUID. In this case, we cannot get the subject information on a remote
> > > node,
> > > > and we get problems like these [1], [2].
> > > >
> > > > To fix the problem, we should spread the client session to the whole
> > > > cluster.
> > > >
> > > >
> > > > I want to suggest a solution to the problem.
> > > >
> > > >
> > > > First, we should get subject information using GridSecurityProcessor.
> > > >
> > > > How GridSecurityProcessor will retrieve a subject data, it is up to
> > > plugin
> > > > developers.
> > > >
> > > >
> > > > Second, we should get rid of the assumption that a subject id is a
> node
> > > id
> > > > and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
> > > >
> > > >
> > > > I have prepared PoC PR [3] that:
> > > >
> > > > - places the existing logic of spreading security context to
> > > > GridSecurityProcessor;
> > > >
> > > > - uses GridSecurityProcessor to get SecurityContext.
> > > >
> > > >
> > > >
> > > > 1.
> > > >
> > > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> > > > 2. https://issues.apache.org/jira/browse/IGNITE-12589
> > > > 3. https://github.com/apache/ignite/pull/7375
> > > >
> > >
> >
>
>
> --
> Best regards,
> Andrey Kuznetsov.
>
--
Best regards,
Alexei Scherbakov
Re: Security Subject of thin client on remote nodes
Posted by Andrey Kuznetsov <st...@gmail.com>.
Hi, guys!
The change suggested by Denis looks robust to me: it covers security
subject handling by all kinds of clients/nodes at once. As for
ATTR_SECURITY_SUBJECT_V2 attribute, it is really better to move it to
plugin implementations to support backward compatibility with peer nodes of
older versions. Obviously, cluster with security disabled will not suffer
from attribute removal. Ignite core should know nothing about the specific
way of security context propagation.
Denis, could you please create Jira issue for your change?
чт, 20 февр. 2020 г. в 17:01, Denis Garus <ga...@gmail.com>:
> > I just transmitted security subjects for rest requests.
>
> SecurityContext has an unlimited size so we can get significant overhead.
> And we do not solve problems with other thin clients.
>
> >If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
> old
> versions and new.
>
> I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but for
> compatibility, it can be used by a security plugin like in PoC.
>
> чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <maksim.stepachev@gmail.com
> >:
>
> > Yes, I said about it at 07.19.
> >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
> > And in my solution, I just transmitted security subjects for rest
> requests.
> >
> > If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between
> old
> > versions and new.
> >
> > чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
> >
> > > Hi, Igniters!
> > >
> > >
> > > At present, a security subject id is assumed to be node id.
> > >
> > > But when we are dealing with thin client, JDBC or REST subject id is
> > random
> > > UUID. In this case, we cannot get the subject information on a remote
> > node,
> > > and we get problems like these [1], [2].
> > >
> > > To fix the problem, we should spread the client session to the whole
> > > cluster.
> > >
> > >
> > > I want to suggest a solution to the problem.
> > >
> > >
> > > First, we should get subject information using GridSecurityProcessor.
> > >
> > > How GridSecurityProcessor will retrieve a subject data, it is up to
> > plugin
> > > developers.
> > >
> > >
> > > Second, we should get rid of the assumption that a subject id is a node
> > id
> > > and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
> > >
> > >
> > > I have prepared PoC PR [3] that:
> > >
> > > - places the existing logic of spreading security context to
> > > GridSecurityProcessor;
> > >
> > > - uses GridSecurityProcessor to get SecurityContext.
> > >
> > >
> > >
> > > 1.
> > >
> > >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> > > 2. https://issues.apache.org/jira/browse/IGNITE-12589
> > > 3. https://github.com/apache/ignite/pull/7375
> > >
> >
>
--
Best regards,
Andrey Kuznetsov.
Re: Security Subject of thin client on remote nodes
Posted by Denis Garus <ga...@gmail.com>.
> I just transmitted security subjects for rest requests.
SecurityContext has an unlimited size so we can get significant overhead.
And we do not solve problems with other thin clients.
>If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between old
versions and new.
I suggest removing ATTR_SECURITY_SUBJECT_V2 from Ignite's codebase, but for
compatibility, it can be used by a security plugin like in PoC.
чт, 20 февр. 2020 г. в 16:47, Maksim Stepachev <ma...@gmail.com>:
> Yes, I said about it at 07.19.
>
> http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
> And in my solution, I just transmitted security subjects for rest requests.
>
> If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between old
> versions and new.
>
> чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
>
> > Hi, Igniters!
> >
> >
> > At present, a security subject id is assumed to be node id.
> >
> > But when we are dealing with thin client, JDBC or REST subject id is
> random
> > UUID. In this case, we cannot get the subject information on a remote
> node,
> > and we get problems like these [1], [2].
> >
> > To fix the problem, we should spread the client session to the whole
> > cluster.
> >
> >
> > I want to suggest a solution to the problem.
> >
> >
> > First, we should get subject information using GridSecurityProcessor.
> >
> > How GridSecurityProcessor will retrieve a subject data, it is up to
> plugin
> > developers.
> >
> >
> > Second, we should get rid of the assumption that a subject id is a node
> id
> > and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
> >
> >
> > I have prepared PoC PR [3] that:
> >
> > - places the existing logic of spreading security context to
> > GridSecurityProcessor;
> >
> > - uses GridSecurityProcessor to get SecurityContext.
> >
> >
> >
> > 1.
> >
> >
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> > 2. https://issues.apache.org/jira/browse/IGNITE-12589
> > 3. https://github.com/apache/ignite/pull/7375
> >
>
Re: Security Subject of thin client on remote nodes
Posted by Maksim Stepachev <ma...@gmail.com>.
Yes, I said about it at 07.19.
http://apache-ignite-developers.2346864.n4.nabble.com/Improvements-for-new-security-approach-td42698.html#a42708
And in my solution, I just transmitted security subjects for rest requests.
If you remove ATTR_SECURITY_SUBJECT_V2, it breaks compatibility between old
versions and new.
чт, 20 февр. 2020 г. в 15:56, Denis Garus <ga...@gmail.com>:
> Hi, Igniters!
>
>
> At present, a security subject id is assumed to be node id.
>
> But when we are dealing with thin client, JDBC or REST subject id is random
> UUID. In this case, we cannot get the subject information on a remote node,
> and we get problems like these [1], [2].
>
> To fix the problem, we should spread the client session to the whole
> cluster.
>
>
> I want to suggest a solution to the problem.
>
>
> First, we should get subject information using GridSecurityProcessor.
>
> How GridSecurityProcessor will retrieve a subject data, it is up to plugin
> developers.
>
>
> Second, we should get rid of the assumption that a subject id is a node id
> and remove the ATTR_SECURITY_SUBJECT_V2 attribute.
>
>
> I have prepared PoC PR [3] that:
>
> - places the existing logic of spreading security context to
> GridSecurityProcessor;
>
> - uses GridSecurityProcessor to get SecurityContext.
>
>
>
> 1.
>
> http://apache-ignite-developers.2346864.n4.nabble.com/JDBC-thin-client-incorrect-security-context-td45929.html
> 2. https://issues.apache.org/jira/browse/IGNITE-12589
> 3. https://github.com/apache/ignite/pull/7375
>