Bug 53830

Summary: Better handling of Manager.randomFile default value on Windows
Product: Tomcat 6 Reporter: Konstantin Kolinko <knst.kolinko>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: 6.0.35   
Target Milestone: default   
Hardware: PC   
OS: Windows XP   
Attachments: 2012-09-05_tc6_53830.patch

Description Konstantin Kolinko 2012-09-05 08:07:20 UTC
See the following thread on the users list:
"Windows Path Not Found for urandom", started on Aug 28, 2012

This issue is specific to Tomcat 6 and earlier.
In Tomcat 7 the session id generation was reimplemented and this feature was removed.

The default value of o.a.c.session.ManagerBase.devRandomSource is "/dev/urandom".

1) The value is unsuitable for Windows, where the file does not exist.

I think it would be better to statically test the value once:
- File.isAbsolute(), to avoid looking for "C:\dev\urandom"
- File.exists()

2) On each call to ManagerBase.getRandomBytes(..) it tries to reopen the file by calling setRandomFile(..).

There is try/catch(IOException) that clears the name and prevents subsequent attempts to open the file, but it does not happen in case of simple if(!f.exists()) return.

Stacktrace (from the mail thread, see details there):
Aug 29, 2012 11:52:29 AM org.apache.catalina.session.ManagerBase setRandomFile
WARNING: Error reading /dev/urandom
        at java.io.DataInputStream.readFully(DataInputStream.java:180)
        at java.io.DataInputStream.readLong(DataInputStream.java:399)
        at org.apache.catalina.session.ManagerBase.setRandomFile(ManagerBase.java:548)
        at org.apache.catalina.session.ManagerBase.getRandomBytes(ManagerBase.java:993)
        at org.apache.catalina.session.ManagerBase.init(ManagerBase.java:767)
        at org.apache.catalina.session.StandardManager.start(StandardManager.java:630)
        at org.apache.catalina.core.ContainerBase.setManager(ContainerBase.java:446)
        at org.apache.catalina.core.StandardContext.start(StandardContext.java:4631)
        at org.apache.catalina.core.ContainerBase.addChildInternal(ContainerBase.java:799)
        at org.apache.catalina.core.ContainerBase.addChild(ContainerBase.java:779)
        at org.apache.catalina.core.StandardHost.addChild(StandardHost.java:601)
        at org.apache.catalina.startup.HostConfig.deployDescriptor(HostConfig.java:675)
        at org.apache.catalina.startup.HostConfig.deployDescriptors(HostConfig.java:601)
        at org.apache.catalina.startup.HostConfig.deployApps(HostConfig.java:502)
        at org.apache.catalina.startup.HostConfig.start(HostConfig.java:1317)
        at org.apache.catalina.startup.HostConfig.lifecycleEvent(HostConfig.java:324)
        at org.apache.catalina.util.LifecycleSupport.fireLifecycleEvent(LifecycleSupport.java:142)
        at org.apache.catalina.core.ContainerBase.start(ContainerBase.java:1065)
        at org.apache.catalina.core.StandardHost.start(StandardHost.java:840)
        at org.apache.catalina.core.ContainerBase.start(ContainerBase.java:1057)
        at org.apache.catalina.core.StandardEngine.start(StandardEngine.java:463)
        at org.apache.catalina.core.StandardService.start(StandardService.java:525)
        at org.apache.catalina.core.StandardServer.start(StandardServer.java:754)
        at org.apache.catalina.startup.Catalina.start(Catalina.java:595)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
        at java.lang.reflect.Method.invoke(Method.java:597)
        at org.apache.catalina.startup.Bootstrap.start(Bootstrap.java:289)
        at org.apache.catalina.startup.Bootstrap.main(Bootstrap.java:414)
Comment 1 Konstantin Kolinko 2012-09-05 09:05:29 UTC
Created attachment 29329 [details]

Patch for Tomcat 6
Comment 2 Konstantin Kolinko 2012-09-05 10:19:17 UTC
Created attachment 29330 [details]

Corrected patch.
1) Properly close old randomIS stream when setRandomFile() is called at runtime to replace the random file.
2) Documentation: The attribute is now documented both for StandardManager and for PersistentManager.
Comment 3 Konstantin Kolinko 2012-09-05 10:26:26 UTC
Created attachment 29331 [details]

Patch for Tomcat 5.5
Comment 4 Christopher Schultz 2012-09-05 15:04:18 UTC
RE:documentation of the attribute name, Jeffrey reported that setting the "randomFile" attribute on the <Manager> had no effect: /dev/urandom was still used. I'm not sure why that was (it could have been a misconfiguration), but please check that setting "randomFile" actually has an effect.
Comment 5 Konstantin Kolinko 2012-09-05 19:00:36 UTC
(In reply to comment #4)
> RE:documentation of the attribute name, Jeffrey reported that setting the
> "randomFile" attribute on the <Manager> had no effect: /dev/urandom was
> still used. I'm not sure why that was (it could have been a
> misconfiguration), but please check that setting "randomFile" actually has
> an effect.

Regarding 6.0.35:
I do not know why it did not work for Jeffrey.

a) A known issue is that if the value is a non-existent file, then in 6.0.35 setting the value would not have much effect. ManagerBase silently accepts the file name and then it will try to reopen it, like it does with the default value of /dev/urandom.
Anyway, whether the value was set can be confirmed via JMX.

b) Maybe a typo, or it was set in a wrong place?

For 6.0.35 and earlier running on Windows I would suggest to set randomFile attribute to point to an existing file, containing 8 bytes.

The initial 8 bytes are read during readLong() call in setRandomFile(). Having a non-empty file avoids logging an IOException there. An attempt to read more bytes in ManagerBase#getRandomBytes() will result in IOException, which will be caught and will set the devRandomSource field to null.

Using a longer file is not recommended, as it will affect the randomness of session ids.

Regarding 6.0 + patch, I tested setting the value
a) Using JMX
b) In conf/context.xml:
 <Manager randomFile="${catalina.base}/conf/server.xml"/>
In conf/logging.properties:

In logs/catalina.2012-09-05.log the following is logged:
 05.09.2012 22:45:17 org.apache.catalina.session.ManagerBase doSetRandomFile
 FINE: Opening C:\[redacted]/conf/server.xml
Comment 6 Mark Thomas 2012-10-01 09:51:13 UTC
Fixed in 5.5.x and will be included in 5.5.36 onwards.
Comment 7 Mark Thomas 2012-10-05 11:45:16 UTC
Fixed in 6.0.x and will be included in 6.0.36 onwards.