Re: Re[2]: Setting IGNITE_TO_STRING_INCLUDE_SENSITIVE=false prevents VM Arguments output

2021-07-02 Thread Shishkov Ilya
Ok, and what's about other VM arguments, not included to
IgniteSystemProperties?

пт, 2 июл. 2021 г. в 09:27, Zhenya Stanilovsky :

>
> -1 for extra arg, +1 for Ivan`s upper proposal : @IgniteSystemProperty
> annotation.
> Look, someone will set some of IGNITE_* option and after possibly cluster
> problems will give this logs into analysis and engineer can`t reproduce
> such a case, cause no param is applied.
>
> >An extra argument for IgniteSystemProperty sounds reasonable.
> >
> >-Val
> >
> >On Thu, Jul 1, 2021 at 10:04 AM Ivan Daschinsky < ivanda...@gmail.com >
> wrote:
> >
> >> Ok, this can be excluded using blocklist-jvm-params.properties or just
> by
> >> providing and extra arg to annotation, as I have just suggested
> >>
> >> чт, 1 июл. 2021 г., 19:51 Valentin Kulichenko <
> >>  valentin.kuliche...@gmail.com
> >> >:
> >>
> >> > Ivan,
> >> >
> >> > IP addresses (e.g. IGNITE_TCP_DISCOVERY_ADDRESSES) and file paths
> >> > (e.g. IGNITE_CONFIG_URL) are often considered sensitive information.
> Data
> >> > related to authentication (e.g. IGNITE_SSH_USER_NAME) is very likely
> to
> >> be
> >> > sensitive.
> >> >
> >> > Once again - I would exclude any property that can contain
> user-specific
> >> > information. Only our internal settings (stuff
> >> > like IGNITE_SQL_MERGE_TABLE_MAX_SIZE) are OK to appear in the logs.
> >> >
> >> > -Val
> >> >
> >> > On Thu, Jul 1, 2021 at 9:47 AM Ivan Daschinsky < ivanda...@gmail.com
> >
> >> > wrote:
> >> >
> >> > > We can add add an extra param in annotation, that blocks param to be
> >> > > printed, just set it to false by default and block it wheb set to
> true
> >> > >
> >> > > чт, 1 июл. 2021 г., 19:45 Atri Sharma < a...@apache.org >:
> >> > >
> >> > > > What if we allowed a blocklist of parameters that are never
> printed?
> >> > > >
> >> > > > On Thu, 1 Jul 2021, 22:06 Valentin Kulichenko, <
> >> > > >  valentin.kuliche...@gmail.com > wrote:
> >> > > >
> >> > > > > Not all of them are OK to be printed out. At the very least, we
> >> > should
> >> > > > have
> >> > > > > a mechanism to exclude some of them. I would still go with
> opt-in
> >> > > rather
> >> > > > > than opt-out though, but I guess that is up to a discussion.
> >> > > > >
> >> > > > > -Val
> >> > > > >
> >> > > > > On Thu, Jul 1, 2021 at 9:29 AM Ivan Daschinsky <
> >>  ivanda...@gmail.com >
> >> > > > > wrote:
> >> > > > >
> >> > > > > > This is security through obscurity, an obvious and a
> well-known
> >> > anti
> >> > > > > > pattern. I suppose that printing jvm options, that is
> registered
> >> by
> >> > > > > > @IgniteSystemProperty annotation is an ideal variant
> >> > > > > >
> >> > > > > > чт, 1 июл. 2021 г., 19:25 Valentin Kulichenko <
> >> > > > > >  valentin.kuliche...@gmail.com
> >> > > > > > >:
> >> > > > > >
> >> > > > > > > Folks,
> >> > > > > > >
> >> > > > > > > *Anything* that a user provides to the system can
> potentially
> >> be
> >> > > > > > considered
> >> > > > > > > sensitive information. This includes the VM arguments. We
> can't
> >> > > > predict
> >> > > > > > > what exactly one can put there, so let's not make
> assumptions.
> >> > > > > > >
> >> > > > > > > When dealing with security, we should be as conservative as
> >> > > possible.
> >> > > > > > That
> >> > > > > > > said, I do not even agree with the pattern approach - there
> >> might
> >> > > be
> >> > > > a
> >> > > > > > > user's system property named IGNITE_xxx. It is also possible
> >> for
> >> > > our
> >> > > > > > > internal properties to contain sensitive information (not
> all
> >> of
> >> > > them
> >> > > > > are
> >> > > > > > > boolean flags).
> >> > > > > > >
> >> > > > > > > The only option I see is to print out specific properties
> for
> >> > which
> >> > > > we
> >> > > > > > > agree that they are safe. For example, we can introduce an
> >> > > annotation
> >> > > > > > that
> >> > > > > > > would mark "safe" properties in IgniteSystemProperties; we
> will
> >> > > then
> >> > > > > > print
> >> > > > > > > out only those that are marked with the annotation.
> >> > > > > > >
> >> > > > > > > -Val
> >> > > > > > >
> >> > > > > > >
> >> > > > > > > On Thu, Jul 1, 2021 at 7:07 AM Вячеслав Коптилин <
> >> > > > > >  slava.kopti...@gmail.com
> >> > > > > > > >
> >> > > > > > > wrote:
> >> > > > > > >
> >> > > > > > > > Hello Ivan,
> >> > > > > > > >
> >> > > > > > > > > At least, we could just hide params that match a
> specific
> >> > > pattern
> >> > > > > > > > Yes, we can filter out all vm options that do not relate
> to
> >> > > Ignite,
> >> > > > > for
> >> > > > > > > > instance.
> >> > > > > > > >
> >> > > > > > > > > Ilya, go ahead, file ticket and prepare a PR.
> >> > > > > > > > Please do not rush. Let's listen to other community
> members.
> >> > This
> >> > > > > > > question
> >> > > > > > > > is about security and it should not be discussed in a
> hurry
> >> > (even
> >> > > > > > though
> >> > > > > > > it
> >> > > > > > > > looks like an obvious thing).
> >> > > > > > > >
> >> 

Re[2]: Setting IGNITE_TO_STRING_INCLUDE_SENSITIVE=false prevents VM Arguments output

2021-07-02 Thread Zhenya Stanilovsky

-1 for extra arg, +1 for Ivan`s upper proposal : @IgniteSystemProperty 
annotation.
Look, someone will set some of IGNITE_* option and after possibly cluster 
problems will give this logs into analysis and engineer can`t reproduce such a 
case, cause no param is applied.
 
>An extra argument for IgniteSystemProperty sounds reasonable.
>
>-Val
>
>On Thu, Jul 1, 2021 at 10:04 AM Ivan Daschinsky < ivanda...@gmail.com > wrote:
> 
>> Ok, this can be excluded using blocklist-jvm-params.properties or just by
>> providing and extra arg to annotation, as I have just suggested
>>
>> чт, 1 июл. 2021 г., 19:51 Valentin Kulichenko <
>>  valentin.kuliche...@gmail.com
>> >:
>>
>> > Ivan,
>> >
>> > IP addresses (e.g. IGNITE_TCP_DISCOVERY_ADDRESSES) and file paths
>> > (e.g. IGNITE_CONFIG_URL) are often considered sensitive information. Data
>> > related to authentication (e.g. IGNITE_SSH_USER_NAME) is very likely to
>> be
>> > sensitive.
>> >
>> > Once again - I would exclude any property that can contain user-specific
>> > information. Only our internal settings (stuff
>> > like IGNITE_SQL_MERGE_TABLE_MAX_SIZE) are OK to appear in the logs.
>> >
>> > -Val
>> >
>> > On Thu, Jul 1, 2021 at 9:47 AM Ivan Daschinsky < ivanda...@gmail.com >
>> > wrote:
>> >
>> > > We can add add an extra param in annotation, that blocks param to be
>> > > printed, just set it to false by default and block it wheb set to true
>> > >
>> > > чт, 1 июл. 2021 г., 19:45 Atri Sharma < a...@apache.org >:
>> > >
>> > > > What if we allowed a blocklist of parameters that are never printed?
>> > > >
>> > > > On Thu, 1 Jul 2021, 22:06 Valentin Kulichenko, <
>> > > >  valentin.kuliche...@gmail.com > wrote:
>> > > >
>> > > > > Not all of them are OK to be printed out. At the very least, we
>> > should
>> > > > have
>> > > > > a mechanism to exclude some of them. I would still go with opt-in
>> > > rather
>> > > > > than opt-out though, but I guess that is up to a discussion.
>> > > > >
>> > > > > -Val
>> > > > >
>> > > > > On Thu, Jul 1, 2021 at 9:29 AM Ivan Daschinsky <
>>  ivanda...@gmail.com >
>> > > > > wrote:
>> > > > >
>> > > > > > This is security through obscurity, an obvious and a well-known
>> > anti
>> > > > > > pattern. I suppose that printing jvm options, that is registered
>> by
>> > > > > > @IgniteSystemProperty annotation is an ideal variant
>> > > > > >
>> > > > > > чт, 1 июл. 2021 г., 19:25 Valentin Kulichenko <
>> > > > > >  valentin.kuliche...@gmail.com
>> > > > > > >:
>> > > > > >
>> > > > > > > Folks,
>> > > > > > >
>> > > > > > > *Anything* that a user provides to the system can potentially
>> be
>> > > > > > considered
>> > > > > > > sensitive information. This includes the VM arguments. We can't
>> > > > predict
>> > > > > > > what exactly one can put there, so let's not make assumptions.
>> > > > > > >
>> > > > > > > When dealing with security, we should be as conservative as
>> > > possible.
>> > > > > > That
>> > > > > > > said, I do not even agree with the pattern approach - there
>> might
>> > > be
>> > > > a
>> > > > > > > user's system property named IGNITE_xxx. It is also possible
>> for
>> > > our
>> > > > > > > internal properties to contain sensitive information (not all
>> of
>> > > them
>> > > > > are
>> > > > > > > boolean flags).
>> > > > > > >
>> > > > > > > The only option I see is to print out specific properties for
>> > which
>> > > > we
>> > > > > > > agree that they are safe. For example, we can introduce an
>> > > annotation
>> > > > > > that
>> > > > > > > would mark "safe" properties in IgniteSystemProperties; we will
>> > > then
>> > > > > > print
>> > > > > > > out only those that are marked with the annotation.
>> > > > > > >
>> > > > > > > -Val
>> > > > > > >
>> > > > > > >
>> > > > > > > On Thu, Jul 1, 2021 at 7:07 AM Вячеслав Коптилин <
>> > > > > >  slava.kopti...@gmail.com
>> > > > > > > >
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > Hello Ivan,
>> > > > > > > >
>> > > > > > > > > At least, we could just hide params that match a specific
>> > > pattern
>> > > > > > > > Yes, we can filter out all vm options that do not relate to
>> > > Ignite,
>> > > > > for
>> > > > > > > > instance.
>> > > > > > > >
>> > > > > > > > > Ilya, go ahead, file ticket and prepare a PR.
>> > > > > > > > Please do not rush. Let's listen to other community members.
>> > This
>> > > > > > > question
>> > > > > > > > is about security and it should not be discussed in a hurry
>> > (even
>> > > > > > though
>> > > > > > > it
>> > > > > > > > looks like an obvious thing).
>> > > > > > > >
>> > > > > > > > Thanks,
>> > > > > > > > S.
>> > > > > > > >
>> > > > > > > > чт, 1 июл. 2021 г. в 16:55, Ivan Daschinsky <
>> >  ivanda...@gmail.com
>> > > >:
>> > > > > > > >
>> > > > > > > > > I suppose, that all normal users should not suffer from
>> this
>> > > > > > > > restrictions.
>> > > > > > > > > Nobody will pass password using jvm options. It is
>> absolutely
>> > > > > insane,
>> > > > > > > > > normal users pass 

Re: Re[2]: Setting IGNITE_TO_STRING_INCLUDE_SENSITIVE=false prevents VM Arguments output

2021-07-01 Thread Atri Sharma
AFAIK, this allows users to pass in custom VM options which may be undesirable.

On Thu, Jul 1, 2021 at 12:07 PM Zhenya Stanilovsky
 wrote:
>
>
> +1 for reverting, can anybody (possibly ticket starter?) explain how jvm 
> settings will rise some security problems ?
>
>
>
> >I suppose, that we should revert this particular line. I don't understand
> >who ever considers vm options as sensitive info.
> >
> >ср, 30 июн. 2021 г., 22:52 Shishkov Ilya < shishkovi...@gmail.com >:
> >
> >> Hi Igniters,
> >>
> >> This feature [1, 2] prevents logging of the VM arguments when
> >> IGNITE_TO_STRING_INCLUDE_SENSITIVE option is set to false. Till now, method
> >> IgniteKernal#ackVmArguments remains mostly the same [3].
> >>
> >> Is this behaviour actual now? Often, we should be able to get from logs the
> >> actual VM options used at startup even if output of sensitive data is
> >> restricted.
> >>
> >> 1.  https://issues.apache.org/jira/browse/IGNITE-4991
> >> 2.
> >>
> >>  
> >> https://github.com/apache/ignite/pull/2428/commits/4f90b6fd77bd23fa818620f0757b792ba388ef93
> >> 3.
> >>
> >>  
> >> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java#L3002
> >>
>
>
>
>

-- 
Regards,

Atri
Apache Concerted


Re[2]: Setting IGNITE_TO_STRING_INCLUDE_SENSITIVE=false prevents VM Arguments output

2021-07-01 Thread Zhenya Stanilovsky

+1 for reverting, can anybody (possibly ticket starter?) explain how jvm 
settings will rise some security problems ?


 
>I suppose, that we should revert this particular line. I don't understand
>who ever considers vm options as sensitive info.
>
>ср, 30 июн. 2021 г., 22:52 Shishkov Ilya < shishkovi...@gmail.com >:
> 
>> Hi Igniters,
>>
>> This feature [1, 2] prevents logging of the VM arguments when
>> IGNITE_TO_STRING_INCLUDE_SENSITIVE option is set to false. Till now, method
>> IgniteKernal#ackVmArguments remains mostly the same [3].
>>
>> Is this behaviour actual now? Often, we should be able to get from logs the
>> actual VM options used at startup even if output of sensitive data is
>> restricted.
>>
>> 1.  https://issues.apache.org/jira/browse/IGNITE-4991
>> 2.
>>
>>  
>> https://github.com/apache/ignite/pull/2428/commits/4f90b6fd77bd23fa818620f0757b792ba388ef93
>> 3.
>>
>>  
>> https://github.com/apache/ignite/blob/master/modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java#L3002
>>