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

Guid.NewGuid should guarantee a full 122 bits of entropy on non-Windows platforms #42752

Closed
GrabYourPitchforks opened this issue Sep 25, 2020 · 4 comments · Fixed by #42770
Closed

Comments

@GrabYourPitchforks
Copy link
Member

The Windows API CoCreateGuid as an implementation detail produces GUIDs with a full 122 bits of entropy. The historical reason for this is that app developers got used to using CoCreateGuid as a cheap way to produce random data, and they would sometimes use that random data for security purposes (such as for keys or other sensitive identifiers). Eventually the OS and app security teams grew tired of the work involved in auditing these callers, so they said, "fine, we're just going to wrap CryptGenRandom and call it a day, because 122 bits from a secure RNG is better than the current state of affairs." And since .NET's Guid.NewGuid() API wraps CoCreateGuid, .NET developers have historically also relied on the same behavior.

On non-Windows platforms, we do not guarantee that Guid.NewGuid() is backed by a secure RNG. This could introduce subtle security vulnerabilities for applications which were written for Windows and which made assumptions about how our APIs worked on Windows but which were eventually deployed on a non-Windows platform. See the thread at dotnet/corefx#37906 (comment) for some further conversation on this point.

I've also created dotnet/dotnet-api-docs#4910 for us to document Guid.NewGuid() as "not guaranteed to be seeded by a secure RNG" - but honestly I don't think documentation is good enough for this. This seems like a pit of failure due to the different security guarantees made across the different platforms. This could be resolved by bringing the "yes, it's really 122 bits of fresh, bespoke entropy!" guarantee cross-platform.

@danmoseley
Copy link
Member

@GrabYourPitchforks we do use a secure RNG on at least Linux and Windows, right? (per #13628 or is dev/urandom not secure by this definition)

Given that which platforms do we not use a secure RNG on? macOS?

@GrabYourPitchforks
Copy link
Member Author

The RandomNumberGenerator RNG is secure across all OSes, yes. This issue is only about the implementation of Guid.NewGuid, which may or may not use a secure RNG depending on the host OS.

@jkotas
Copy link
Member

jkotas commented Sep 25, 2020

We have two problems:

  • On Linux, we use /dev/urandom and have fallback path to fill in the rest with pseudo random generator when something goes wrong. We should be able to get rid of the fallback path, for guid generation at least.
  • On macOS, we use pseudo-random generator. I assume that this change was done to improve performance, but it was not such a great idea in the hindsight. We can switch macOS back to the same plan as Linux.

Would that be sufficient to address your concern?

@GrabYourPitchforks
Copy link
Member Author

@jkotas Yes, that should work. :)

@jkotas jkotas added this to the 6.0.0 milestone Sep 25, 2020
jkotas added a commit to jkotas/runtime that referenced this issue Sep 27, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants