Bug 53830 - Better handling of Manager.randomFile default value on Windows
Summary: Better handling of Manager.randomFile default value on Windows
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.35
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-05 08:07 UTC by Konstantin Kolinko
Modified: 2012-10-05 11:45 UTC (History)
0 users



Attachments
2012-09-05_tc6_53830.patch (5.30 KB, patch)
2012-09-05 09:05 UTC, Konstantin Kolinko
Details | Diff
2012-09-05_tc6_53830_v2.patch (6.23 KB, patch)
2012-09-05 10:19 UTC, Konstantin Kolinko
Details | Diff
2012-09-05_tc55_53830_v2.patch (6.23 KB, patch)
2012-09-05 10:26 UTC, Konstantin Kolinko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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
http://markmail.org/thread/l3f3meujmac2jzqp
http://marc.info/?t=134616742400004&r=1&w=2

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
java.io.EOFException
        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]
2012-09-05_tc6_53830.patch

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

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]
2012-09-05_tc55_53830_v2.patch

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.
http://markmail.org/message/4zfhs6fii6vb7pf4

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:
 org.apache.catalina.session.ManagerBase.level=FINE

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.