Closed
Bug 476594
Opened 15 years ago
Closed 15 years ago
[SECURITY] Calling srand() at startup time in mod_perl makes the same token be generated over and over
Categories
(Bugzilla :: Bugzilla-General, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: mkanat, Assigned: mkanat)
References
Details
Attachments
(1 file)
716 bytes,
patch
|
justdave
:
review+
|
Details | Diff | Splinter Review |
On bmo, we were noticing that attachments always had the same token, over and over. We talked to the maintainer of mod_perl, and he pointed out that we might be calling srand() at startup time, which we are, in Bugzilla::Install::Localconfig. We can fix this for now by making LOCALCONFIG_VARS into a sub instead of a constant, but the right fix is to call srand() inside of a ChildInitHandler with a good seed.
Updated•15 years ago
|
Summary: Calling rand() at startup time in mod_perl makes the same token be generated over and over → Calling srand() at startup time in mod_perl makes the same token be generated over and over
Assignee | ||
Comment 1•15 years ago
|
||
We can work out a better solution for HEAD, later, but here's a quick patch that fixes this (and works the way that this always should have worked anyway).
Assignee: general → mkanat
Status: NEW → ASSIGNED
Attachment #360252 -
Flags: review?(justdave)
Attachment #360252 -
Flags: review?(LpSolit)
Assignee | ||
Comment 2•15 years ago
|
||
We need some sort of testcase that proves that we're generating sufficiently random tokens even if we ask for them very quickly.
Flags: testcase?
Assignee | ||
Updated•15 years ago
|
Blocks: bz-release-322
Comment 3•15 years ago
|
||
Just be careful with this for 2 reasons: This now means that srand() will be called implicitely by each httpd child the first time rand() is called. That's going to equate srand(time). This means that if you have 2 httpd processes started within the same second, they will too follow the same random sequence for their lifetime Anything that re-introduces a call to rand() at startup will cause this to happen again. 3rd-party modules, etc. The ideal solution is to add to httpd.conf: PerlChildInitHandler "sub { srand($some_good_randomness) }" This way, each time a new httpd child is spawned, it will pick itself some good new randomness. Picking what to use for $some_good_randomness is always the fun part. I like Math::TrulyRandom or /dev/u?random, but it is not a portable solution.
Comment 4•15 years ago
|
||
(In reply to comment #2) > We need some sort of testcase that proves that we're generating sufficiently > random tokens even if we ask for them very quickly. That's indicating that randomness is probably not the best way to go. A form of HMAC() would probably be a better bet, it's fast and doesn't drain the entropy from the server. So you could use a token like SHA1(db secret + SHA1(db secret + userid + attachmentid)) Doesn't matter, really. If I understand this feature correctly, you want links to have an unguessable part to them, not a random part.
Comment 5•15 years ago
|
||
Comment on attachment 360252 [details] [diff] [review] v1 This is applied on b.m.o now, and appears to work as advertised. All my tokens are unique now.
Attachment #360252 -
Flags: review?(justdave) → review+
Comment 6•15 years ago
|
||
I hadn't seen gozer's comments in here yet when I marked that though... he's got some good points. Long term we should probably get something in like he's suggesting, because we never know what else we might use that random token generator for, and we might need something really random at some point. But for now... (In reply to comment #4) > Doesn't matter, really. If I understand this feature correctly, you want links > to have an unguessable part to them, not a random part. This is correct.
Assignee | ||
Comment 7•15 years ago
|
||
(In reply to comment #3) > The ideal solution is to add to httpd.conf: > > PerlChildInitHandler "sub { srand($some_good_randomness) }" Yeah. That's what we're going to do for future trunk releases. We just want some time to make sure that our random goodness is actually good and works on all the platforms we support.
Updated•15 years ago
|
Flags: blocking3.2.2?
Assignee | ||
Updated•15 years ago
|
Flags: blocking3.2.2? → blocking3.2.2+
Assignee | ||
Comment 8•15 years ago
|
||
tip: Checking in Bugzilla/Install/Localconfig.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Localconfig.pm,v <-- Localconfig.pm new revision: 1.16; previous revision: 1.15 done 3.2: Checking in Bugzilla/Install/Localconfig.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Localconfig.pm,v <-- Localconfig.pm new revision: 1.12.2.2; previous revision: 1.12.2.1 done 3.0: Checking in Bugzilla/Install/Localconfig.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Localconfig.pm,v <-- Localconfig.pm new revision: 1.8.2.2; previous revision: 1.8.2.1 done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: approval3.2+
Flags: approval3.0+
Flags: approval+
Resolution: --- → FIXED
Summary: Calling srand() at startup time in mod_perl makes the same token be generated over and over → [SECURITY] Calling srand() at startup time in mod_perl makes the same token be generated over and over
Assignee | ||
Comment 9•15 years ago
|
||
Security advisory sent in bug 476618, unlocking this bug.
Group: bugzilla-security
Assignee | ||
Updated•15 years ago
|
Attachment #360252 -
Flags: review?(LpSolit)
You need to log in
before you can comment on or make changes to this bug.
Description
•