On 29/11/2010 13:41, Tim Funk wrote: > Sorry for the additional noise ... my svn emails are in a different > folder from dev emails. I just noticed ...
Good to see we were thinking along the same lines. I still want to get to the bottom of the really poor performance on my Mac. Before I do that, I want to take a look at the recent 7.0.5 bugs to see if any are serious enough to change my vote from Beta. Mark > > svn commit: r1039882 - > /tomcat/trunk/java/org/apache/catalina/session/ManagerBase.java > > -Tim > > On 11/29/2010 7:40 AM, Tim Funk wrote: >> I checked the svn history of why MD5 (hashing was used) and the picture >> is incomplete. (unless someone asks craig since I think he was the >> author) >> >> But it appears like this ... >> Tomcat 3.X use Math.random() and some misc crap to generate its session >> id. It had a comment (paraphrased), "not secure for banking/military use >> for creating session ID. " >> >> Then in tomcat 4.0 - we see the code as it is now. It has always tried >> to use SecureRandom. What is interesting is SecureRandom's javadocs in >> 1.2 (JDK) are different than 1.4. (And maybe 1.3) By the time we get to >> Java 1.4 - SecureRandom is advertised as cryptographically strong. So >> back in the day of 1.2 - there was no guarantee there. Or maybe is >> wasn't cross platform guaranteed. >> >> So back in the day ... you might be able to get a stream of session ids >> ... and then determine where you are in the sequence of random number >> generation. (Recall that randoms aren't really random, its just an >> algorithm on a seed) An "easy" way to thwart this attack - is to hash >> the numbers. Making it orders of magnitude harder determine what the >> numbers generated the session ID. (For this type of deception, any >> hashing function is still OK even with the collision issues) >> >> So that leaves us to where we are now. Interestingly enough ... RFC1750 >> (as listed in the SecureRandom javadoc) discourages the use of current >> time as a seed because the window to guess the seed is now orders of >> magnitude smaller. >> >> Since all instances of Random are self seeding, it may be best (ask you >> local JVM expert for opinion) to allow the JVM to decide the seed. In >> which case - it may go to the hardware, use /dev/urandom, etc. Which >> would be much better than anything we can do. >> >> As for hash or not to hash. I am unsure. If the seed IS quality, and the >> random algorithm is quality - then there is no need to hash. But if we >> allow reality to intervene - then we might accept that some platforms >> might not have a quality seed, or one of the algorithms might come under >> attack and no longer be good. In which case - hashing becomes a good >> defense. >> >> >> So to collect all the thoughts above ... it might be nice to do the >> following >> - Force use of SecureRandom (and still allow it to be extended) >> - Turn hashing off (but leave it as an option) >> - When initializing SecureRandom - do nothing. Let the JVM take care >> of it. >> - Allow /dev/urandom to override the previous statement. >> >> Then if any of the above is unacceptable ... the user can just provide >> their own extended version of SecureRandom. >> >> >> -Tim >> >> On 11/26/2010 1:46 PM, Remy Maucherat wrote: >>> On Thu, 2010-11-25 at 16:33 +0000, Mark Thomas wrote: >>>> I wouldn't call it bad. It doesn't do any harm (apart from adding a >>>> very >>>> small amount of overhead), and it would help if the random source >>>> selected ended up not being that random. >>>> >>>> I thought the trade-off of protection against bad choices of random >>>> sources was worth the minimal overhead added. I'm not against removing >>>> it entirely, if there is consensus to do so. >>> >>> MD5 is now known as a bad hash (it was fine at the time the code was >>> written), so does it actually improve anything ? >>> >>> Also, isn't SecureRandom always available now ? This is 10+ years old >>> code, probably. >>> >>>> For SecureRandom, yes. I did test this locally and it achieves >>>> thread-safety by using an internal sync and it did create a significant >>>> bottleneck. That is why I went the parallel route. Reading from a >>>> stream >>>> has a similar sync so again that is why I went the parallel route. >>> >>> Ok. The internal lock is a much smaller sync than the old sync block, so >>> it would be a bit better. It is possible it is noticeable, though. The >>> question is if this yields a good enough session creation rate. >>> >>>> How about this as an approach to reduce the complexity: >>>> 1. Remove the MD5 code (optional) >>>> 2. Default to /dev/urandom then SecureRandom. Don't fall back to >>>> Random. >>>> 3. Provide a class that implements Random that reads data from a file >>>> 4. If randomFile is specified, use the the class from 3 as the Random >>>> source >>>> >>>> That should reduce the current 3 Queues to one. I doubt it will improve >>>> performance much but it should make the code clearer. >>>> >>>> Thoughts? >>> >>> I don't know what the best solution is. /dev/urandom could also only be >>> used as seed only to a SecureRandom, this is more Javaish. >>> So about the MD5, I don't think it is useful anymore. >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >> >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
