[
https://issues.apache.org/jira/browse/ZOOKEEPER-3488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16900568#comment-16900568
]
Patrick Hunt commented on ZOOKEEPER-3488:
-----------------------------------------
> Sensitive information about DataDir, SnapDir, SessionId and
> RemoteSocketAddress may be leaked
I don't consider this sensitive information. Also it's being logged on the
server machines which only the admin should have access to (otw you have bigger
problems). Sessionid and remote socket address is also critical for debugging
in certain situations.
Note that LOG.isInfoEnabled is an optimization. (see
https://stackoverflow.com/questions/963492/in-log4j-does-checking-isdebugenabled-before-logging-improve-performance
- it's in the context of debug but essentially the same thing)
> Possible information leakage to log without LOG configuration control
> LOG.isInfoEnabled()
> -----------------------------------------------------------------------------------------
>
> Key: ZOOKEEPER-3488
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3488
> 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
> Environment: Ubuntu 16.04.3 LTS
> Open JDK version "1.8.0_191" build 25.191-b12
> Reporter: xiaoqin.fu
> Priority: Major
>
> In org.apache.zookeeper.server.ZooKeeperServer, statements LOG.info(....)
> don't have LOG configuration controls.
> public ZooKeeperServer(FileTxnSnapLog txnLogFactory, int tickTime,
> int minSessionTimeout, int maxSessionTimeout, ZKDatabase zkDb) {
>
> ......
> LOG.info("Created server with tickTime " + tickTime
> + " minSessionTimeout " + getMinSessionTimeout()
> + " maxSessionTimeout " + getMaxSessionTimeout()
> + " datadir " + txnLogFactory.getDataDir()
> + " snapdir " + txnLogFactory.getSnapDir());
> ......
> }
> public void finishSessionInit(ServerCnxn cnxn, boolean valid)
> ......
> if (!valid) {
> LOG.info("Invalid session 0x"
> + Long.toHexString(cnxn.getSessionId())
> + " for client "
> + cnxn.getRemoteSocketAddress()
> + ", probably expired");
> cnxn.sendBuffer(ServerCnxnFactory.closeConn);
> } else {
> LOG.info("Established session 0x"
> + Long.toHexString(cnxn.getSessionId())
> + " with negotiated timeout " +
> cnxn.getSessionTimeout()
> + " for client "
> + cnxn.getRemoteSocketAddress());
> cnxn.enableRecv();
> }
> ......
> }
> Sensitive information about DataDir, SnapDir, SessionId and
> RemoteSocketAddress may be leaked. It is better to add LOG.isInfoEnabled()
> conditional statements:
> public ZooKeeperServer(FileTxnSnapLog txnLogFactory, int tickTime,
> int minSessionTimeout, int maxSessionTimeout, ZKDatabase zkDb) {
>
> ......
> if (LOG.isInfoEnabled())
> LOG.info("Created server with tickTime " + tickTime
> + " minSessionTimeout " + getMinSessionTimeout()
> + " maxSessionTimeout " + getMaxSessionTimeout()
> + " datadir " + txnLogFactory.getDataDir()
> + " snapdir " + txnLogFactory.getSnapDir());
> ......
> }
> public void finishSessionInit(ServerCnxn cnxn, boolean valid) {
> ......
> if (!valid) {
> if (LOG.isInfoEnabled())
> LOG.info("Invalid session 0x"
> + Long.toHexString(cnxn.getSessionId())
> + " for client "
> + cnxn.getRemoteSocketAddress()
> + ", probably expired");
> cnxn.sendBuffer(ServerCnxnFactory.closeConn);
> } else {
> if (LOG.isInfoEnabled())
> LOG.info("Established session 0x"
> + Long.toHexString(cnxn.getSessionId())
> + " with negotiated timeout " +
> cnxn.getSessionTimeout()
> + " for client "
> + cnxn.getRemoteSocketAddress());
> cnxn.enableRecv();
> }
> ......
> }
> The LOG.isInfoEnabled() conditional statement already exists in
> org.apache.zookeeper.server.persistence.FileTxnLog:
> public synchronized boolean append(TxnHeader hdr, Record txn) throws
> IOException {
> { ......
> if(LOG.isInfoEnabled()){
> LOG.info("Creating new log file: " +
> Util.makeLogName(hdr.getZxid()));
> }
> ......
> }
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
