Replace uuidv4 generator with `crypto.randomUUID()` #8600

dlarocque · 2024-10-25T13:48:39Z

The uuidv4 generator in util uses Math.random(), which does not provide strong uniqueness guarantees (https://www.bocoup.com/blog/random-numbers).

The places where the uuidv4 generator are used don't require strong uniqueness guarantees (nothing security related), but I think it's good to move away from this from util in case we try to use it in the future.

A better built-in alternative is crypto.randomUUID(), which does provide strong uniqueness guarantees. I believe the reason this wasn't used originally was because it wasn't supported in Node <15. Since this is a more modern JS built-in, it's only defined in secure contexts. Is this something we're concerned about? Are there any App Check users with apps running in non-secure environments?

Fixes #6462
Also closes b/283984828

changeset-bot · 2024-10-25T13:48:43Z

🦋 Changeset detected

Latest commit: 6ed7315

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@firebase/app-check	Patch
@firebase/util	Patch
@firebase/app-check-compat	Patch
firebase	Patch
@firebase/analytics-compat	Patch
@firebase/analytics	Patch
@firebase/app-compat	Patch
@firebase/app	Patch
@firebase/auth-compat	Patch
@firebase/auth	Patch
@firebase/component	Patch
@firebase/data-connect	Patch
@firebase/database-compat	Patch
@firebase/database-types	Patch
@firebase/database	Patch
@firebase/firestore-compat	Patch
@firebase/firestore	Patch
@firebase/functions-compat	Patch
@firebase/functions	Patch
@firebase/installations-compat	Patch
@firebase/installations	Patch
@firebase/messaging-compat	Patch
@firebase/messaging	Patch
@firebase/performance-compat	Patch
@firebase/performance	Patch
@firebase/remote-config-compat	Patch
@firebase/remote-config	Patch
@firebase/storage-compat	Patch
@firebase/storage	Patch
@firebase/vertexai	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

google-oss-bot · 2024-10-25T14:02:49Z

Size Report ¹

Affected Products

@firebase/app-check
Type Base (32bf021) Merge (3f8a971) Diff
browser 26.3 kB 26.3 kB +4 B (+0.0%)
main 27.3 kB 27.3 kB +6 B (+0.0%)
module 26.3 kB 26.3 kB +4 B (+0.0%)
@firebase/util
Type Base (32bf021) Merge (3f8a971) Diff
browser 23.4 kB 23.2 kB -168 B (-0.7%)
main 29.2 kB 29.0 kB -222 B (-0.8%)
module 23.4 kB 23.2 kB -168 B (-0.7%)

`bundle`

Type	Base (`32bf021`)	Merge (`3f8a971`)	Diff
app-check (CustomProvider)	37.5 kB	37.3 kB	-142 B (-0.4%)
app-check (ReCaptchaEnterpriseProvider)	40.0 kB	39.8 kB	-144 B (-0.4%)
app-check (ReCaptchaV3Provider)	39.9 kB	39.8 kB	-144 B (-0.4%)

firebase
Type Base (32bf021) Merge (3f8a971) Diff
firebase-app-check-compat.js 23.4 kB 23.2 kB -136 B (-0.6%)
firebase-app-check.js 25.0 kB 24.9 kB -107 B (-0.4%)
firebase-compat.js 797 kB 797 kB -133 B (-0.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/erZPTOadzN.html

github-actions · 2024-10-25T14:05:51Z

Changeset File Check ⚠️

Warning: This PR modifies files in the following packages but they have not been included in the changeset file:%0A - @firebase/data-connect%0A - @firebase/database%0A%0A Make sure this was intentional.

google-oss-bot · 2024-10-25T14:14:37Z

Size Analysis Report ¹

Affected Products

`@firebase/app-check`

`initializeAppCheck`

Size

Type	Base (`32bf021`)	Merge (`3f8a971`)	Diff
size	11.2 kB	11.2 kB	+1 B (+0.0%)
size-with-ext-deps	36.5 kB	36.3 kB	-144 B (-0.4%)

External Dependency

Module	Base (`32bf021`)	Merge (`3f8a971`)	Diff
`@firebase/util`	Deferred ErrorFactory base64 getGlobal getModularInstance isIndexedDBAvailable uuidv4	Deferred ErrorFactory base64 getGlobal getModularInstance isIndexedDBAvailable	- uuidv4

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/qay7dZYpqg.html

hsubox76 · 2024-11-05T22:23:23Z

Since this is a more modern JS built-in, it's only defined in secure contexts. Is this something we're concerned about? Are there any App Check users with apps running in non-secure environments?

What's the effect of this, if you deploy an app with App Check to a site and call it with HTTP, do you get an error?

The uuidv4 generator in util used `Math.random()`, which does not provide strong uniqueness guarantees (https://www.bocoup.com/blog/random-numbers). The places where the uuidv4 generator were used didn't require strong uniqueness guarantees (nothing security related), but I think it's good to move away from this from util in case we try to use it in the future. A better built-in alternative is `crypto.randomUUID()`, which does provide strong uniqueness guarantees. Since this is a more modern JS built-in, it's only [defined in secure contexts](https://blog.mozilla.org/security/2018/01/15/secure-contexts-everywhere/). Is this something we're concerned about? Are there any App Check users with apps running in non-secure environments?

dlarocque · 2025-01-07T15:31:18Z

Since this is a more modern JS built-in, it's only defined in secure contexts. Is this something we're concerned about? Are there any App Check users with apps running in non-secure environments?

What's the effect of this, if you deploy an app with App Check to a site and call it with HTTP, do you get an error?

@hsubox76 If you call crypto.UUID() on a site deployed with HTTP (non-secure context), you will get TypeError: crypto.randomUUID is not a function. Tested this here http://httpforever.com/.

It's worth mentioning that sites served locally on HTTP (http://localhost) are considered secure contexts.

Also, in App Check, crypto.randomUUID is only used for testing/debugging in readOrCreateDebugTokenFromStorage.

yuchenshi

Seems only used in tests in data-connect and database too. I support this change for these reasons

It's really a small change touching just a few lines of code so the risk is low
We can avoid rolling our own UUID implementation

I don't think the security considerations apply to our use cases. When we do need a secure implementation in the future, we'll likely have to do a security review anyway.

dlarocque requested review from hsubox76, a team, maneesht and aashishpatil-g as code owners October 25, 2024 13:48

dlarocque requested review from a team as code owners October 25, 2024 14:03

dlarocque and others added 3 commits January 7, 2025 10:13

Update API reports

fcec824

Add changeset

6ed7315

dlarocque force-pushed the dl/uuid branch from 1e712d0 to 6ed7315 Compare January 7, 2025 15:14

yuchenshi approved these changes Jan 7, 2025

View reviewed changes

Add comment about availability restricted to secure contexts

e93f105

hsubox76 approved these changes Jan 8, 2025

View reviewed changes

dlarocque merged commit 25a6204 into main Jan 8, 2025
50 checks passed

dlarocque deleted the dl/uuid branch January 8, 2025 16:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace uuidv4 generator with `crypto.randomUUID()` #8600

Replace uuidv4 generator with `crypto.randomUUID()` #8600

dlarocque commented Oct 25, 2024 •

edited

Loading

changeset-bot bot commented Oct 25, 2024 •

edited

Loading

google-oss-bot commented Oct 25, 2024 •

edited

Loading

`@firebase/app-check`

`@firebase/util`

`bundle`

`firebase`

github-actions bot commented Oct 25, 2024

google-oss-bot commented Oct 25, 2024 •

edited

Loading

`@firebase/app-check`

`initializeAppCheck`

hsubox76 commented Nov 5, 2024

dlarocque commented Jan 7, 2025 •

edited

Loading

yuchenshi left a comment •

edited

Loading

Replace uuidv4 generator with crypto.randomUUID() #8600

Replace uuidv4 generator with crypto.randomUUID() #8600

Conversation

dlarocque commented Oct 25, 2024 • edited Loading

changeset-bot bot commented Oct 25, 2024 • edited Loading

🦋 Changeset detected

google-oss-bot commented Oct 25, 2024 • edited Loading

Size Report 1

Affected Products

@firebase/app-check

@firebase/util

bundle

firebase

Test Logs

github-actions bot commented Oct 25, 2024

Changeset File Check ⚠️

google-oss-bot commented Oct 25, 2024 • edited Loading

Size Analysis Report 1

Affected Products

@firebase/app-check

initializeAppCheck

Test Logs

hsubox76 commented Nov 5, 2024

dlarocque commented Jan 7, 2025 • edited Loading

yuchenshi left a comment • edited Loading

Choose a reason for hiding this comment

Replace uuidv4 generator with `crypto.randomUUID()` #8600

Replace uuidv4 generator with `crypto.randomUUID()` #8600

dlarocque commented Oct 25, 2024 •

edited

Loading

changeset-bot bot commented Oct 25, 2024 •

edited

Loading

google-oss-bot commented Oct 25, 2024 •

edited

Loading

Size Report ¹

`@firebase/app-check`

`@firebase/util`

`bundle`

`firebase`

google-oss-bot commented Oct 25, 2024 •

edited

Loading

Size Analysis Report ¹

`@firebase/app-check`

`initializeAppCheck`

dlarocque commented Jan 7, 2025 •

edited

Loading

yuchenshi left a comment •

edited

Loading