Skip to content

Use SecureRandom for server TRIDs#3163

Open
jicelhay wants to merge 3 commits into
google:masterfrom
jicelhay:servertrid
Open

Use SecureRandom for server TRIDs#3163
jicelhay wants to merge 3 commits into
google:masterfrom
jicelhay:servertrid

Conversation

@jicelhay

@jicelhay jicelhay commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

This introduces a thread-specific instance of SecureRandom to compute serverTrids "randomly" in requests. This was chosen over using new SecureRandom instances per request to avoid potentially depleting OS entropy pool and over injecting and calling the existing singleton SecureRandom to avoid locks under heavy QPS load.


This change is Reviewable

@jicelhay
jicelhay requested a review from CydeWeys July 20, 2026 18:40

@CydeWeys CydeWeys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CydeWeys made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on jicelhay).


core/src/main/java/google/registry/flows/ServerTridProviderImpl.java line 33 at r1 (raw file):

  @VisibleForTesting
  static final ThreadLocal<SecureRandom> secureRandom =

Can we just use ThreadLocalRandom here? Does it really have to be SecureRandom? It's still pretty expensive to generate bytes using SecureRandom.

@CydeWeys CydeWeys left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CydeWeys made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on jicelhay).


core/src/main/java/google/registry/flows/ServerTridProviderImpl.java line 33 at r1 (raw file):

Previously, CydeWeys (Ben McIlwain) wrote…

Can we just use ThreadLocalRandom here? Does it really have to be SecureRandom? It's still pretty expensive to generate bytes using SecureRandom.

Also some digging shows that ThreadLocal<SecureRandom> might not actually work how you expect, as the underlying implementation is using non-threaded global state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants