Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Implications of "caching randomness" #527

Closed
simlu opened this issue Oct 7, 2020 · 6 comments
Closed

Security Implications of "caching randomness" #527

simlu opened this issue Oct 7, 2020 · 6 comments

Comments

@simlu
Copy link

simlu commented Oct 7, 2020

There are some possible security implications of caching randomness as discussed here

Since the original ticket was about "mockability" of uuid, I'm opening this ticket for further discussion on the security implications.

The gist of the problem is:

  • Using crypto.randomBytes made it extremely hard (impossible) to know previous or predict future values generated by this library
  • In feat: improved v4 performance #435 this library started using a pool, which now makes it very easy to know previous and predict future values generated by looking at a memory dump

The question is should we care and why.

@broofa
Copy link
Member

broofa commented Oct 7, 2020

now makes it very easy to know previous and predict future

Please elaborate on what "very easy" means.

This may be naive, but doesn't the module-private nature of this pool (in-memory only, not exported) mean it's actually quite difficult to access? And if there's a viable method for doing so, the security ramifications would extend well beyond just this module?

@simlu
Copy link
Author

simlu commented Oct 7, 2020

@broofa There are a lot of tools out there: https://github.com/Rob--/memoryjs - It's not about "how hard it is", but if it's the correct thing to do from a security perspective. I mean we could also mark this library as "don't use for security relevant use cases", but my preference would be not to do that.

By very easy I meant, compared to predicting randomness as generated by the OS.

I might have another argument here:

Not a security concern, but (I think?) a legitimate problem: Consider the case where the program is running in a container. The program is used to generate entries that are put into a database where the uuid is generated as the primary unique key. The container is backed up in the running state for easy recovery. Time progresses and more db entries are created. Something goes wrong and the backup has to be played in. Since the uuids generated are not actually random a "unique key constraint" exception is risen.

@ctavan
Copy link
Member

ctavan commented Oct 7, 2020

Not a security concern, but (I think?) a legitimate problem: Consider the case where the program is running in a container. The program is used to generate entries that are put into a database where the uuid is generated as the primary unique key. The container is backed up in the running state for easy recovery. Time progresses and more db entries are created. Something goes wrong and the backup has to be played in. Since the uuids generated are not actually random a "unique key constraint" exception is risen.

This idea is interesting. I'm wondering though if preallocation of random bytes really introduces a new problem here that otherwise wouldn't exist. Just assume the container is frozen at the exact point in time after a UUID has been returned by the uuid module but before the database insert has been issued. Copying that snapshot and continuing it multiple times would also result in unique key constraint violations for every additional instance of the snapshot that is being continued. How does preallocation of random bytes change anything about this general problem that you may get with multiple continuations of the same snapshot of a container?

@simlu
Copy link
Author

simlu commented Oct 7, 2020

[...] How does preallocation of random bytes change anything about this general problem that you may get with multiple continuations of the same snapshot of a container?

@ctavan I'd say that the main difference is that with one approach you can easily "flush" (i.e. wait for all externally triggered operations to complete), while there is no easy way of clearing the uuid cache (which you might not even know about).

I'm no expert by any means and hence posted on stackexchange here. Let's see if we can get someone who is an expert!

@broofa
Copy link
Member

broofa commented Aug 11, 2022

Closing this out as part of a general issue-sweep. The (one) response to the StackExchange question seems to indicate that caching randomness isn't, in and of itself, that big a deal. Also, we don't have any real-world reports of the RNG cache causing problems.

Feel free to reopen if there's reason to.

@broofa broofa closed this as completed Aug 11, 2022
@simlu
Copy link
Author

simlu commented Aug 11, 2022

Sounds good. We've since moved away from this library to crypto.randomUUID, since uuid wasn't working with our test library anymore after state aka caching of randomness was introduced. So I don't really have a reason to persue this.

OK to remain closed 👍

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

No branches or pull requests

3 participants