Igor, I see nothing criminal in attribute visibility to internal Ignite components. It could be a hole if attributes were available through public API.
вт, 28 янв. 2020 г., 13:07 Igor Sapego <isap...@apache.org>: > Ilya, > > The fact that we have some security holes does not sound to me > like a good reason to create a new one. > > Best Regards, > Igor > > > On Tue, Jan 28, 2020 at 12:22 PM Ilya Kasnacheev < > ilya.kasnach...@gmail.com> > wrote: > > > Hello! > > > > We already have several "security contexts" and we even have some code > > which tries to clear sensitive data from it. > > > > However, it is no good for authenticating nodes anyway, since discovery > > message have to travel via ring, so all server nodes will have access to > > sensitive information. > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > вт, 28 янв. 2020 г. в 11:41, Igor Sapego <isap...@gridgain.com>: > > > > > Dmitrii, > > > > > > I'm not a security expert, but I have a concern about the usage > > > of common purpose mechanism for authentication purposes. > > > > > > I mean any other component seem to be able to get this data which > > > should be private. Looks like a dangerous hack to me, to be honest. > > > > > > Maybe we should create and use some kind of "security context" for > > > this purpose with limited access. > > > > > > Best Regards, > > > Igor > > > > > > > > > On Mon, Jan 27, 2020 at 3:29 PM Dmitrii Ryabov <somefire...@gmail.com> > > > wrote: > > > > > > > Ok, if no one have objections, I will restrict a map by strings only. > > > > > > > > чт, 23 янв. 2020 г., 17:20 Pavel Tupitsyn <ptupit...@apache.org>: > > > > > > > > > > Well, my understanding was a binary object with compact footer = > > > false > > > > > is totally standalone entity and can be read without any external > > > > metadata > > > > > Not exactly: type name and field names are unknown, you only have > > > typeId, > > > > > field ids and positions. > > > > > There is one more Marshaller "mode": UNREGISTERED_TYPE_ID. In this > > > mode, > > > > > full type name is included in the binary object. > > > > > However, field names are still missing and the class needs to be > > > present > > > > to > > > > > deserialize. > > > > > That's why arbitrary objects should not be allowed in the > handshake: > > in > > > > > most cases they can't be decoded. > > > > > > > > > > On Thu, Jan 23, 2020 at 2:27 PM Dmitrii Ryabov < > > somefire...@gmail.com> > > > > > wrote: > > > > > > > > > > > Alexei, yes, compactFooter is used only in 1 place. > > > > > > > > > > > > ``` > > > > > > > > > > > > BinaryWriterExImpl.marshal0() { > > > > > > > > > > > > BinaryClassDescriptor desc = ctx.descriptorForClass(cls); > > > > > > > > > > > > // descriptor transportation fails here > > > > > > > > > > > > ... > > > > > > > > > > > > desc.write(obj, this); // compactFooter here > > > > > > > > > > > > } > > > > > > > > > > > > ``` > > > > > > > > > > > > чт, 23 янв. 2020 г., 14:15 Pavel Tupitsyn <ptupit...@apache.org > >: > > > > > > > > > > > > > > If we support only strings it will be necessary to encode > > binary > > > > > values > > > > > > > to > > > > > > > > something like BASE64 which is not sounds good from usability > > > side > > > > > > > > > > > > > > There should be no need to put binary values to attributes. > > What's > > > > the > > > > > > use > > > > > > > case? > > > > > > > > > > > > > > On Thu, Jan 23, 2020 at 2:08 PM Alexei Scherbakov < > > > > > > > alexey.scherbak...@gmail.com> wrote: > > > > > > > > > > > > > > > > Footer is checked in postWrite - much later class > descriptor > > > > check. > > > > > > > > > > > > > > > > Well, my understanding was a binary object with compact > footer > > = > > > > > false > > > > > > is > > > > > > > > totally standalone entity and can be read without any > external > > > > > > metadata. > > > > > > > > Dmitrii Ryabov can you double check ? > > > > > > > > > > > > > > > > If we support only strings it will be necessary to encode > > binary > > > > > values > > > > > > > to > > > > > > > > something like BASE64 which is not sounds good from usability > > > side. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > чт, 23 янв. 2020 г. в 13:26, Nikita Amelchev < > > > nsamelc...@gmail.com > > > > >: > > > > > > > > > > > > > > > > > +1 for the hardcoded String type only > > > > > > > > > > > > > > > > > > чт, 23 янв. 2020 г. в 13:15, Pavel Tupitsyn < > > > > ptupit...@apache.org > > > > > >: > > > > > > > > > > > > > > > > > > > > - Cross-platform binary objects are totally possible, all > > > those > > > > > > thin > > > > > > > > > > clients support them. > > > > > > > > > > - User attributes can be useful, no objections here > > > > > > > > > > > > > > > > > > > > However, I don't think we should allow arbitrary objects > in > > > > user > > > > > > > > > attributes. > > > > > > > > > > Let's make them string only, much less to worry about. > > > > > > > > > > > > > > > > > > > > And using attributes for authentication still seems weird > > and > > > > > > dirty. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Jan 23, 2020 at 12:40 PM Dmitrii Ryabov < > > > > > > > somefire...@gmail.com > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > Even if compact footer is disabled ? > > > > > > > > > > > Footer is checked in postWrite - much later class > > > descriptor > > > > > > check. > > > > > > > > > > > > > > > > > > > > > > чт, 23 янв. 2020 г., 12:23 Alexei Scherbakov < > > > > > > > > > alexey.scherbak...@gmail.com > > > > > > > > > > > >: > > > > > > > > > > > > > > > > > > > > > > > чт, 23 янв. 2020 г. в 12:17, Dmitrii Ryabov < > > > > > > > somefire...@gmail.com > > > > > > > > >: > > > > > > > > > > > > > > > > > > > > > > > > > > The protocol must be language-agnostic. If we add > > > some > > > > > > > features > > > > > > > > > > > there, > > > > > > > > > > > > > let's make sure they are usable from anywhere. > > > > > > > > > > > > > > > > > > > > > > > > > > That's why I want to allow primitives only. Any > > > language > > > > > can > > > > > > > send > > > > > > > > > > > numbers > > > > > > > > > > > > > and strings. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > In general it's possible to have cross-platform > complex > > > > data > > > > > > > > > structures, > > > > > > > > > > > > for example see protobuf. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Binary marshaller, before packing object to byte[], > > > will > > > > > try > > > > > > to > > > > > > > > use > > > > > > > > > > > > > discovery processor and send message containing > class > > > > > > > descriptor. > > > > > > > > > But > > > > > > > > > > > > thin > > > > > > > > > > > > > clients don't have discovery. Furthermore, if we > > write > > > > > binary > > > > > > > > > > > marshaller > > > > > > > > > > > > > without class descriptor synchronization, we can > get > > > > > objects > > > > > > > with > > > > > > > > > > > > different > > > > > > > > > > > > > class versions and corresponding exceptions. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Even if compact footer is disabled ? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > But we can say users to declare their classes in > > > > > > > > > > > > > META-INF/classnames.properties and current binary > > > > > marshaller > > > > > > > will > > > > > > > > > works > > > > > > > > > > > > > good. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This approach doesn't looks like cross-platform. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > чт, 23 янв. 2020 г., 12:13 Alex Plehanov < > > > > > > > > plehanov.a...@gmail.com > > > > > > > > > >: > > > > > > > > > > > > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > > > > > > > > > > > > > User attributes also (besides authentication) can > > be > > > > used > > > > > > to > > > > > > > > pass > > > > > > > > > > > some > > > > > > > > > > > > > info > > > > > > > > > > > > > > about an application that uses a client and then > > > > display > > > > > > this > > > > > > > > > > > > information > > > > > > > > > > > > > > in monitoring tools. Other vendors use such > > approach > > > > > > (Oracle > > > > > > > > DB, > > > > > > > > > for > > > > > > > > > > > > > > example, have DBMS_APPLICATION_INFO package, > > > > PostgreeSQL > > > > > > have > > > > > > > > > > > > > > application_name connection property and > > application > > > > > > > > information > > > > > > > > > > > > > available > > > > > > > > > > > > > > later in system views). > > > > > > > > > > > > > > > > > > > > > > > > > > > > About allowed data types: we should definitely > > limit > > > > > > > attribute > > > > > > > > > types > > > > > > > > > > > to > > > > > > > > > > > > > > only primitive types. Thin client binary > marshaller > > > > can't > > > > > > > send > > > > > > > > > > > > > information > > > > > > > > > > > > > > about custom types before the handshake. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ср, 22 янв. 2020 г. в 21:39, Pavel Tupitsyn < > > > > > > > > > ptupit...@apache.org>: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I've looked through the PR more closely, trying > > to > > > > > > > understand > > > > > > > > > the > > > > > > > > > > > use > > > > > > > > > > > > > > case, > > > > > > > > > > > > > > > and there are some Java-specific things going > on > > > > (left > > > > > a > > > > > > > > > comment). > > > > > > > > > > > > > > > Please keep in mind that we have thin clients > in > > > > > Python, > > > > > > > > > Node.js, > > > > > > > > > > > > C++, > > > > > > > > > > > > > > C#. > > > > > > > > > > > > > > > The protocol must be language-agnostic. If we > add > > > > some > > > > > > > > features > > > > > > > > > > > > there, > > > > > > > > > > > > > > > let's make sure they are usable from anywhere. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Jan 22, 2020 at 9:21 PM Pavel Tupitsyn > < > > > > > > > > > > > ptupit...@apache.org > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The approach with UserAttributes map looks > > dirty > > > to > > > > > me > > > > > > > and > > > > > > > > > raises > > > > > > > > > > > > > > > > questions: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - Why is UserAttributes property related to > > > > > > > authentication? > > > > > > > > > > > > > > > > - UserAttributes name implies that users can > > put > > > > > there > > > > > > > > > anything > > > > > > > > > > > > they > > > > > > > > > > > > > > > want, > > > > > > > > > > > > > > > > but what for? What are those additional use > > > cases? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I think we should focus on a specific problem > > at > > > > hand > > > > > > and > > > > > > > > > avoid > > > > > > > > > > > > > > > > unnecessary future-proofing. > > > > > > > > > > > > > > > > What are current and potential future custom > > > > > > > authenticators > > > > > > > > > and > > > > > > > > > > > > what > > > > > > > > > > > > > > kind > > > > > > > > > > > > > > > > of data do they need? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Jan 22, 2020 at 6:28 PM Nikita > > Amelchev < > > > > > > > > > > > > > nsamelc...@gmail.com> > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >> I think we should add this. It will provide > an > > > > extra > > > > > > > level > > > > > > > > > of > > > > > > > > > > > > > > security. > > > > > > > > > > > > > > > >> This approach is used in many products, for > > > > example > > > > > in > > > > > > > AWS > > > > > > > > > > > (MFA). > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> [1] > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://docs.aws.amazon.com/general/latest/gr/aws-sec-cred-types.html#multi-factor-authentication > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> ср, 22 янв. 2020 г. в 18:13, Andrey > Kuznetsov > > < > > > > > > > > > > > stku...@gmail.com > > > > > > > > > > > > >: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > Hi, Pavel! > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > Sometimes single authentication factor is > > not > > > > > > enough. > > > > > > > > > > > Attributes > > > > > > > > > > > > > > > >> proposed > > > > > > > > > > > > > > > >> > allow to add extra factors flexibly. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > ср, 22 янв. 2020 г., 17:39 Pavel Tupitsyn > < > > > > > > > > > > > ptupit...@apache.org > > > > > > > > > > > > >: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > Token can be sent instead of a password > > > (like > > > > > git > > > > > > > > works > > > > > > > > > with > > > > > > > > > > > > > > GitHub > > > > > > > > > > > > > > > >> > > tokens). > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > For now I don't see a reason to include > > > > > attributes > > > > > > > > into > > > > > > > > > the > > > > > > > > > > > > > > > handshake > > > > > > > > > > > > > > > >> > > message. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > On Wed, Jan 22, 2020 at 5:32 PM Ilya > > > > Kasnacheev > > > > > < > > > > > > > > > > > > > > > >> ilya.kasnach...@gmail.com > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > wrote: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > Hello! > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > One does not send security certificate > > as > > > > > > > attribute. > > > > > > > > > The > > > > > > > > > > > > only > > > > > > > > > > > > > > way > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > >> > > obtain > > > > > > > > > > > > > > > >> > > > peer security certificate is to ask > SSL > > > > engine > > > > > > to > > > > > > > > > provide > > > > > > > > > > > > it. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > Nevertheless, I can see how it can be > > > useful > > > > > > with > > > > > > > > e.g. > > > > > > > > > > > > > Kerberos, > > > > > > > > > > > > > > > >> which is > > > > > > > > > > > > > > > >> > > > token-based IIRC. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > Regards, > > > > > > > > > > > > > > > >> > > > -- > > > > > > > > > > > > > > > >> > > > Ilya Kasnacheev > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > ср, 22 янв. 2020 г. в 17:20, Dmitrii > > > Ryabov > > > > < > > > > > > > > > > > > > > > somefire...@gmail.com > > > > > > > > > > > > > > > >> >: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > This map is something like user > object > > > > from > > > > > > > > > > > > > > > `SecurityCredentials`. > > > > > > > > > > > > > > > >> > > > > Sometimes login and password are not > > > > enough > > > > > > for > > > > > > > > > security > > > > > > > > > > > > > > checks. > > > > > > > > > > > > > > > >> For > > > > > > > > > > > > > > > >> > > > > example, we can send security > > > certificate > > > > > and > > > > > > > > > validate > > > > > > > > > > > it > > > > > > > > > > > > > > inside > > > > > > > > > > > > > > > >> > > > > authenticator. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > ср, 22 янв. 2020 г., 17:16 Igor > > Sapego < > > > > > > > > > > > > isap...@apache.org > > > > > > > > > > > > > >: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > Hi Dmitrii, > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > Can you please explain your use > > case? > > > > > > > > > > > > > > > >> > > > > > I'm not sure I'm getting what is > the > > > > > > > motivation > > > > > > > > of > > > > > > > > > > > this > > > > > > > > > > > > > > > change. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > Best Regards, > > > > > > > > > > > > > > > >> > > > > > Igor > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > On Wed, Jan 22, 2020 at 5:11 PM > > Pavel > > > > > > > Tupitsyn < > > > > > > > > > > > > > > > >> ptupit...@apache.org > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > wrote: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > Hi Dmitrii, > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > Honestly, I could not grasp the > > > > problem, > > > > > > can > > > > > > > > you > > > > > > > > > > > > explain > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > >> in more > > > > > > > > > > > > > > > >> > > > > > detail? > > > > > > > > > > > > > > > >> > > > > > > What do we solve by adding a map > > > with > > > > > > > > arbitrary > > > > > > > > > > > stuff > > > > > > > > > > > > to > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > >> client > > > > > > > > > > > > > > > >> > > > > > > protocol handshake? > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > On Wed, Jan 22, 2020 at 5:02 PM > > > > Dmitrii > > > > > > > > Ryabov < > > > > > > > > > > > > > > > >> > > > somefire...@gmail.com> > > > > > > > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > Hello, Igniters! > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > I want to add the possibility > of > > > > > sending > > > > > > > > user > > > > > > > > > > > > defined > > > > > > > > > > > > > > > >> attributes > > > > > > > > > > > > > > > >> > > > from > > > > > > > > > > > > > > > >> > > > > > > thin > > > > > > > > > > > > > > > >> > > > > > > > clients. And check them inside > > > > custom > > > > > > > > > > > authenticator > > > > > > > > > > > > > > during > > > > > > > > > > > > > > > >> > > > handshake > > > > > > > > > > > > > > > >> > > > > > [1]. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > There is an issue in hardcoded > > > > binary > > > > > > > writer > > > > > > > > > for > > > > > > > > > > > > JDBC > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > >> > > > > > `IgniteClient`. > > > > > > > > > > > > > > > >> > > > > > > > This writer searches for a > > classes > > > > in > > > > > > the > > > > > > > > JDK > > > > > > > > > and > > > > > > > > > > > > > > > >> > > > > > > > > META-INF/classnames.properties, > > > and > > > > > > tries > > > > > > > to > > > > > > > > > sync > > > > > > > > > > > > > > > >> notdeclared > > > > > > > > > > > > > > > >> > > > classes > > > > > > > > > > > > > > > >> > > > > > > with > > > > > > > > > > > > > > > >> > > > > > > > cluster. But fails because > > current > > > > > > > > > classloading > > > > > > > > > > > uses > > > > > > > > > > > > > > > >> discovery. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > I'd like to keep this writer > and > > > > allow > > > > > > > only > > > > > > > > > > > > primitive > > > > > > > > > > > > > > > types > > > > > > > > > > > > > > > >> and > > > > > > > > > > > > > > > >> > > > > > `String` > > > > > > > > > > > > > > > >> > > > > > > > for user attributes to prevent > > > > > > unexpected > > > > > > > > > fails. I > > > > > > > > > > > > > think > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > >> is > > > > > > > > > > > > > > > >> > > > better > > > > > > > > > > > > > > > >> > > > > > > than > > > > > > > > > > > > > > > >> > > > > > > > changing writer to one with > > heavy > > > > > > > > > classloading. > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > Is it ok to restrict thin > > > attributes > > > > > to > > > > > > > > > primitives > > > > > > > > > > > > and > > > > > > > > > > > > > > > >> 'String'? > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > [1] > > > > > > > > > > > > > https://issues.apache.org/jira/browse/IGNITE-12049 > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> -- > > > > > > > > > > > > > > > >> Best wishes, > > > > > > > > > > > > > > > >> Amelchev Nikita > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > Best regards, > > > > > > > > > > > > Alexei Scherbakov > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > Best wishes, > > > > > > > > > Amelchev Nikita > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > Best regards, > > > > > > > > Alexei Scherbakov > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >