zookeeper-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Patrick Hunt (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (ZOOKEEPER-3488) Possible information leakage to log without LOG configuration control LOG.isInfoEnabled()
Date Tue, 06 Aug 2019 03:17:00 GMT

    [ 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)

Mime
View raw message