Page MenuHomePhabricator

mw.user.generateRandomSessionId not so random
Closed, ResolvedPublic

Description

We have been using generateRandomSessionId [1] to aid our research on sendBeacon support:

https://gerrit.wikimedia.org/r/#/c/175953/13/modules/ext.wikimediaEvents.sendBeacon.js

And found it to be not so "random".

We have about 56.000 records in which our client side generated id should be unique, but on those there are 29 duplicates. (Please note that this is nothing to do with sendBeacon itself as these numbers come from the control experiment that is contrasting "regular" event data with "send beacon" data)

We have looked into the browsers that are produccing duplicated data and for the most part is Safari in its many flavors (we can provide more detail data upon request).

Perhaps we should consider using: http://caniuse.com/#search=getRandomValues when supported?

We have enough "good" unique data to do our experiment so that is no issue, this is just an FYI.

[1] https://git.wikimedia.org/blob/mediawiki%2Fcore.git/ed646ee688057e6022a00e2965f3ca7095cc9ec8/resources%2Fsrc%2Fmediawiki%2Fmediawiki.user.js#L51

Event Timeline

Nuria created this task.Dec 13 2014, 2:26 AM
Nuria raised the priority of this task from to Needs Triage.
Nuria updated the task description. (Show Details)
Nuria added a project: MediaWiki-JavaScript.
Nuria changed Security from none to None.
Nuria added subscribers: Nuria, Mattflaschen-WMF.
Nuria updated the task description. (Show Details)Dec 13 2014, 2:28 AM
Aklapper triaged this task as Normal priority.Dec 15 2014, 1:05 PM

This is a more acute problem in mobile where Safari is much more prevalent (in our user base on enwiki at least)

gerritbot added a subscriber: gerritbot.

Change 187876 had a related patch set uploaded (by Nuria):
Using cryptoAPI if available in generateRandomSessionId

https://gerrit.wikimedia.org/r/187876

Patch-For-Review

Nuria claimed this task.Feb 2 2015, 3:10 PM
Krinkle renamed this task from generateRandomSessionId on mediawiki.user.js not so random to mw.user.generateRandomSessionId not so random .Feb 14 2015, 10:26 AM
Krinkle closed this task as Resolved.
Krinkle removed a project: Patch-For-Review.
Krinkle added a subscriber: Krinkle.

Has anyone upstreamed this bug to Apple?

Nuria added a comment.Feb 26 2015, 6:34 PM

I believe this is a known issue and not per se a bug, Math.random is not supposed to be cryptographically strong, one of many reports on the subject: https://code.google.com/p/chromium/issues/detail?id=246054