1179856 - Increase length of all tokens value for greater security

Status: RESOLVED FIXED

(Bugzilla :: User Accounts, enhancement)

Description

[Gervase Markham [:gerv]]

[e-mail] writes in mozilla.support.bugzilla:

<quote>
We are running scans against Bugzilla 4.2.14 to assess security vulnerability. 

Two findings are rated high from this URL pattern: https://mybugzilla.example.com/index.cgi:

Session Management: Insufficient Session ID Entropy
Session Management: Insufficient Session ID Length

These are related to this:  https://www.owasp.org/index.php/Insufficient_Session-ID_Length
</quote>


The Bugzilla_logincookie value has the form of type "2sllLO4azN", which is a (by default) 10 character value in the set 0-9a-zA-Z, 62 possible values. This makes 62^10 possibilities, which is approximately 8.39x10^17, or 2^59.

https://www.owasp.org/index.php/Insufficient_Session-ID_Length seems to require 128 bits, but reading carefully, it actually only requires 64 bits of entropy. So we are not far from the standard. The randomness comes from irand, which comes from Bugzilla::RNG, which comes from Math::Random::ISAAC, which is cryptographically strong.

Having said that, I see no problem in, say, doubling the length. 

Gerv

Comment 1

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1179856_2.patch

check tokens and logincookies to 22 character tokens (from 10!)
22 chars is about 128bits of entropy (unless I got the math wrong).

Comment 2

[Gervase Markham [:gerv]]

Comment on attachment 8639103 [details] [diff] [review]
1179856_2.patch

Review of attachment 8639103 [details] [diff] [review]:
-----------------------------------------------------------------

r=gerv. Your maths is good :-)

Gerv

Comment 3

[Simon Green]

Should this really be back ported to 5.0? It might break applications that assume the cookie is currently ten characters.  It's not as if the current cookies are insecure, this patch just makes it more secure.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/bugzilla/bugzilla.git
   19a2eba..d373bb1  master -> master

Should this resolve the bug? I don't think we need this in 5.0 either.

Comment 5

[David Lawrence [:dkl]]

(In reply to Dylan William Hardison [:dylan] from comment #4)
> To ssh://[e-mail]/bugzilla/bugzilla.git
>    19a2eba..d373bb1  master -> master
> 
> Should this resolve the bug? I don't think we need this in 5.0 either.

I also feel we do not need this for 5.0.

dkl

Comment 6

[:glob ✱]

this patch actually updates the length of all tokens, not just the cookie.  updating summary for clarity.

Comment 7

[Gervase Markham [:gerv]]

I nominated for 5.0 so we could have the discussion; if people feel it's unnecessary, that's fine by me.

Gerv

Comment 8

[Frédéric Buclin]

(In reply to Gervase Markham [:gerv] from comment #7)
> I nominated for 5.0 so we could have the discussion; if people feel it's
> unnecessary, that's fine by me.

Our policy is to not change the DB schema on a stable branch, so backporting it to 5.0 was not an option anyway.