[
https://issues.apache.org/jira/browse/IGNITE-4167?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15769830#comment-15769830
]
Semen Boikov commented on IGNITE-4167:
--------------------------------------
Hi Alexandr,
I reviewed your change implemented so far, have few comments:
1. I do not like copy/paste in places like this:
{noformat}
log.debug(S.INCLUDE_SENSITIVE ?
"Record [key=" + key +
", val=" + val +
", incBackups=" + incBackups +
", priNode=" + (primaryNode != null ?
U.id8(primaryNode.id()) : null) +
", node=" + U.id8(cctx.localNode().id()) + ']' :
"Record [incBackups=" + incBackups +
", priNode=" + (primaryNode != null ?
U.id8(primaryNode.id()) : null) +
", node=" + U.id8(cctx.localNode().id()) + ']');
{noformat}
Is it possible to introuduce some utility methods to avoid copy/paste? For
above example it could be method 'static void debugSensitive(IgniteLogger log,
String msg, Object...sensitive)' so this code can be replaced with call without
copy/paste:
{noformat}
U.debugSensitive(log, "Record [key=" + key +
", val=" + val +
", incBackups=" + incBackups +
", priNode=" + (primaryNode != null ?
U.id8(primaryNode.id()) : null) +
", node=" + U.id8(cctx.localNode().id()),
"key", key, "val", val // Optionally printed
sensitive argument.
);
{noformat}
2. BinaryEnumObjectImpl.toString: I think className/typeId can be 'sensitive'
as well. The same for BinaryMetadata, BinaryObjectExImpl.
3. It seems 'CacheInvokeDirectResult.res' should be masrked as sensitive?
4. I do not like that you change toString for utility collections
(ConcurrentLinkedHashMap, GridBoundedConcurrentLinkedHashSet, GridLeanSet,
GridListSet, GridLongList, etc). They are not always used to store sensitive
data and now we can loose usefull debug information. Instead we need review
these collections usage and fix only places when these collections really
contains sensitive data.
5. Please add at least basic tests:
- unit test for GridToStringBuilder.
- test with multiple node, it should execute put/get/invoke/remove with
tx/atomic caches wit trace logging enabled and check that logger output do not
contain keys/values.
> Add an option to avoid printing out sensitive data into logs
> ------------------------------------------------------------
>
> Key: IGNITE-4167
> URL: https://issues.apache.org/jira/browse/IGNITE-4167
> Project: Ignite
> Issue Type: Improvement
> Reporter: Denis Kholodov
> Assignee: Alexandr Kuramshin
>
>
> We are seeing sensitive cache data being output in ignite debug logging. I've
> tracked it down to at least two places:
> 1. GridToStringBuilder uses reflection to print all fields in cache objects
> that are not annotated with @GridToStringExclude
> 2. GridCacheMapEntry does a direct toString() call on the value objects in a
> debug log
> As a fabric platform, we won't always have control over the object classes
> being added to/retrieved from the cache.
> We must always assume that all keys and values are sensitive and should not
> be outputted in logs except in local debugging situations. To this end, we
> need a configuration option (turned OFF by default) that allows keys/values
> to be written to log messages.
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
