zookeeper-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jürgen Wagner (JIRA) <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-3504) An information leakage from FileTxnSnapLog to log:
Date Tue, 13 Aug 2019 03:23:00 GMT

    [ https://issues.apache.org/jira/browse/ZOOKEEPER-3504?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16905774#comment-16905774
] 

Jürgen Wagner commented on ZOOKEEPER-3504:
------------------------------------------

As I have seen this message on the Zookeeper lists now at least twice, I feel compelled to
reply.

The Logger object will only log if debug mode is enabled, anyway. Therefore, wrapping an SLF4J
logger call to LOG.XXX in a LOG.isXXXEnabled() is functionally redundant, and may only check
for the debug mode a little bit earlier, saving a bit of CPU time to get into the Logger and
check, and to compute TxnHeader.getType().

The only criticism one could apply from my point of view is the use of string concatenation
instead of string pattern arguments, because this way we would create string garbage even
if debug mode is not enabled. However, you can see this being used at the end of processTransaction(),
so you seem to be referring to an older version of the code.

https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnSnapLog.java

Sensitive information will not be leaked as debug mode is not supposed to be enabled in a
production system (apart from the fact that even with the addition of the proposed check,
the same amount of information would be logged).

 

> An information leakage from FileTxnSnapLog to log:
> --------------------------------------------------
>
>                 Key: ZOOKEEPER-3504
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3504
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: security, server
>    Affects Versions: 3.4.11, 3.4.12, 3.4.13, 3.5.5, 3.4.14
>            Reporter: xiaoqin.fu
>            Priority: Major
>
> In org.apache.zookeeper.server.persistence.FileTxnSnapLog, the statement LOG.debug don't
have LOG controls:
>     public void processTransaction(TxnHeader hdr,DataTree dt,
>             Map<Long, Integer> sessions, Record txn)
>         throws KeeperException.NoNodeException {  
> 		......
>         if (rc.err != Code.OK.intValue()) {
>             LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
>                     + ", error: " + rc.err + ", path: " + rc.path);
>         }  
> 		......
>     } 
> Sensitive information about hdr type or rc path was leaked. The conditional statement
LOG.isDebugEnabled() should be added:
>     public void processTransaction(TxnHeader hdr,DataTree dt,
>             Map<Long, Integer> sessions, Record txn)
>         throws KeeperException.NoNodeException {  
> 		......
>         if (rc.err != Code.OK.intValue()) {
>         	if (LOG.isDebugEnabled())
> 				LOG.debug("Ignoring processTxn failure hdr:" + hdr.getType()
>                     + ", error: " + rc.err + ", path: " + rc.path);
>         }  
> 		......
>     } 



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Mime
View raw message