Use SecureRandom for server TRIDs#3163
Conversation
CydeWeys
left a comment
There was a problem hiding this comment.
@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
left a comment
There was a problem hiding this comment.
@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
ThreadLocalRandomhere? Does it really have to beSecureRandom? 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.
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