Implement crypto.randomUUID()
Categories
(Core :: DOM: Web Crypto, enhancement, P3)
Tracking
()
People
(Reporter: cpeterson, Assigned: cpeterson)
References
Details
(Keywords: dev-doc-complete)
Attachments
(3 files)
WICG draft specification for crypto.randomUUID()
:
https://wicg.github.io/uuid/
Assignee | ||
Comment 1•3 years ago
|
||
I have a patch to implement crypto.randomUUID()
and fix the WPT failures.
Updated•3 years ago
|
Comment 2•3 years ago
|
||
I know of at least two ways to generate UUIDs in Gecko:
- Via nsUUIDGenerator
- Via the Rust uuid crate, exposed to C++ via GkRustUtils: https://searchfox.org/mozilla-central/rev/4f05a46731c1f7f111ec7a41ce38a34594aa0d37/xpcom/base/GkRustUtils.h#14
It might be nice to unify them. The easiest would probably be to pipe nsUUIDGenerator into the Rust crate, assuming we think it's of sufficient quality.
Comment 3•3 years ago
|
||
So I don't want to make a big deal of this, but I am not happy with either of those implementations. nsUUIDGenerator isn't guaranteed to produce cryptographic random numbers (it invokes random()
in some cases). It's probably not at the level of a security problem, as we mostly use arc4random()
, but it's not something I would trust.
I traced the rust version back to the uuid crate, which just uses thread_rng()
from the rand crate. (BTW, that uuid crate is overkill for our needs, but it's probably deeply entrenched already, so no point trying to fix that, especially given how small it is.) I don't know if we override the default thread PRNG. It looks like we just use the default from the rand crate, which looks to use os-specific functions. We should be using the same CSPRNG we have for crypto.getRandomBytes
. That is, the one in NSS that we know is good.
Assignee | ||
Comment 4•3 years ago
|
||
(In reply to Martin Thomson [:mt:] from comment #3)
I traced the rust version back to the uuid crate, which just uses
thread_rng()
from the rand crate. (BTW, that uuid crate is overkill for our needs, but it's probably deeply entrenched already, so no point trying to fix that, especially given how small it is.) I don't know if we override the default thread PRNG. It looks like we just use the default from the rand crate, which looks to use os-specific functions. We should be using the same CSPRNG we have forcrypto.getRandomBytes
. That is, the one in NSS that we know is good.
In that case, I will look into implementing randomUUID
using the same CSPRNG we use for getRandomBytes
(nsRandomGenerator::GetRandomBytes
). I will probably update nsUUIDGenerator
itself to use nsRandomGenerator
and then implement randomUUID
using nsUUIDGenerator.
Comment 5•3 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #4)
(In reply to Martin Thomson [:mt:] from comment #3)
I traced the rust version back to the uuid crate, which just uses
thread_rng()
from the rand crate. (BTW, that uuid crate is overkill for our needs, but it's probably deeply entrenched already, so no point trying to fix that, especially given how small it is.) I don't know if we override the default thread PRNG. It looks like we just use the default from the rand crate, which looks to use os-specific functions. We should be using the same CSPRNG we have forcrypto.getRandomBytes
. That is, the one in NSS that we know is good.In that case, I will look into implementing
randomUUID
using the same CSPRNG we use forgetRandomBytes
(nsRandomGenerator::GetRandomBytes
). I will probably updatensUUIDGenerator
itself to usensRandomGenerator
and then implementrandomUUID
using nsUUIDGenerator.
And once we do that, probably worth also eliminating GkRustUtils_GenerateUUID and rerouting its one caller to the new path.
Assignee | ||
Comment 6•3 years ago
|
||
(In reply to Bobby Holley (:bholley) from comment #5)
And once we do that, probably worth also eliminating GkRustUtils_GenerateUUID and rerouting its one caller to the new path.
I filed bug 1724152 to replace GkRustUtils_GenerateUUID
.
Updated•3 years ago
|
Assignee | ||
Comment 7•3 years ago
|
||
(In reply to Chris Peterson [:cpeterson] from comment #4)
In that case, I will look into implementing
randomUUID
using the same CSPRNG we use forgetRandomBytes
(nsRandomGenerator::GetRandomBytes
).
This new patch is taking longer than expected. Firefox generates UUIDs before NSS is initialized. Instead of returning an error, NSS will quietly self-initialize in "nocertdb" mode because the user's profile directory path is not known yet so early in Firefox startup. In "nocertdb" mode, tests can't insert test certs into the certdb necessary to establish HTTPS connections to local test servers.
I need to find a workaround, such as finding the user's profile directory path sooner during startup, using NSS without the user's profile directory path, or special casing UUID generation during early startup to fall back to a different RNG. These approaches will surely have zero unanticipated side effects. :)
Comment 8•3 years ago
|
||
I think this issue might be a duplicate of bug 1700675, which has patches for migrating nsUUIDGenerator
to use RandomUint64OrDie()
(a CSPRNG which doesn't require NSS), and removing the use of GkRustUtils for generating UUIDs.
Assignee | ||
Comment 9•3 years ago
•
|
||
Hi Dana, in this bug I am implementing a new crypto.randomUUID()
Web API that Chrome has shipped. In comment 3, Martin recommended that UUID generation (nsUUIDGenerator
) use NSS's CSRNG (via nsRandomGenerator
like crypto.getRandomValues()
does). nsUUIDGenerator
currently uses the OS's UUID APIs on Windows and macOS, but falls back to the standard C library's weak random()
on Linux.
Unfortunately, changing nsUUIDGenerator
to use nsRandomGenerator
is not straightforward because nsUUIDGenerator
is called very early during Firefox startup, before NSS is initialized. Calling nsRandomGenerator
during early startup does return random data, but it causes NSS to quietly auto-initialize in "nocertdb" mode which causes hard to debug test failures.
Do you like any of these alternatives?
- Change
nsUUIDGenerator
to use the OS's strong RNG (e.g./dev/urandom
,arc4random()
, etc via MFBT's RandomNum code). This is similar to the patches in bug 1700675 (r+'d but never landed) to work around a different problem. - Change
nsUUIDGenerator
to use the OS's RNG during early startup andnsRandomGenerator
after NSS has been initialized. - Change
nsUUIDGenerator
to usensRandomGenerator
, but changensRandomGenerator
to use the OS's RNG during early startup and NSS after it has been initialized. - None of the above: use some other option or don't implement
crypt.randomUUID()
at all.
I like #3 because it would make nsRandomGenerator
available during early startup, but still use NSS when it's available. You might not like #3 if you intend nsRandomGenerator
to strictly be a wrapper around NSS's CSRNG.
I would actually prefer #2 here - nsRandomGenerator
should really be guaranteed to be using NSS's CSPRNG. My understanding of a quick read of the randomUUID
spec is that using /dev/urandom
would be acceptable, so it should be okay to have that fallback.
Assignee | ||
Comment 11•3 years ago
|
||
(In reply to Dana Keeler (she/her) (use needinfo) (:keeler for reviews) from comment #10)
I would actually prefer #2 here -
nsRandomGenerator
should really be guaranteed to be using NSS's CSPRNG. My understanding of a quick read of therandomUUID
spec is that using/dev/urandom
would be acceptable, so it should be okay to have that fallback.
Thanks. I'm relieved you preferred option #2 because I found some cases where nsRandomGenerator
's main thread initialization requirement would add new complications to nsUUIDGenerator
. Option #2 avoids that problem.
I have patches that I will clean up and post soon.
Comment 12•3 years ago
|
||
(In reply to Dana, Chris)
My understanding of a quick read of the randomUUID spec is that using /dev/urandom would be acceptable, so it should be okay to have that fallback.
My expectation in the UUID spec, was that a source of randomness similar to crypto.getRandomBytes
would be used. The Web Crypto spec indicates for crypto.getRandomBytes
:
Implementations should generate cryptographically random values using well-established cryptographic pseudo-random number generators seeded with high-quality entropy, such as from an operating-system entropy source (e.g., "/dev/urandom"). This specification provides no lower-bound on the information theoretic entropy present in cryptographically random values, but implementations should make a best effort to provide as much entropy as practicable.
So /dev/urandom
should be good 👍
I'm excited to see this work in motion 🎉
Assignee | ||
Comment 13•3 years ago
|
||
Generalize RandomUint64() into a new GenerateRandomBytes() function, which will be used in the next changeset to replace nsUUIDGenerator's various platform-specific implementations with one simple implementation.
Assignee | ||
Comment 14•3 years ago
|
||
This changeset is an adaptation of the accepted changes (but not landed) for bug 1700675:
https://phabricator.services.mozilla.com/D118714
Also, clean up some old (1998!) JavaDoc comments that just make the code more difficult to read.
Depends on D124311
Assignee | ||
Comment 15•3 years ago
|
||
Google shipped crypto.randomUUID() in Chrome 92.
WICG draft specification for crypto.randomUUID():
https://wicg.github.io/uuid/
Depends on D124312
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 16•3 years ago
|
||
Pushed by cpeterson@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eaa49cc4154b mfbt: Add new GenerateRandomBytes() function. r=cmartin https://hg.mozilla.org/integration/autoland/rev/a1b5d8bd40fc Move nsUUIDGenerator logic to new nsID::GenerateUUID() factory functions that use nsRandomGenerator. r=keeler https://hg.mozilla.org/integration/autoland/rev/ad6b007fd7af Implement crypto.randomUUID(). r=keeler,nika
Comment 17•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eaa49cc4154b
https://hg.mozilla.org/mozilla-central/rev/a1b5d8bd40fc
https://hg.mozilla.org/mozilla-central/rev/ad6b007fd7af
Comment 18•3 years ago
|
||
Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.
Updated•3 years ago
|
Updated•3 years ago
|
Comment 19•3 years ago
|
||
FYI, docs tracking for FF95 work on this method can be found here: https://github.com/mdn/content/issues/10143#issuecomment-955945183
There isn't very much. This was already documented so the main change is update of the browser compatibility information.
Assignee | ||
Updated•2 years ago
|
Description
•