www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Phobos randomUUID is not suitable to generate secrets

reply Cym13 <cpicard purrfect.fr> writes:
Hi there,

As always when I make an appearance it's that something has gone 
wrong. Either in a popular library or D itself... This time it's 
a little bit of both.

There are many projects out there that use UUIDs to generate 
secrets such as session tokens and password recovery tokens. 
Other projects use it to protect against things like symlink 
attacks by generating temporary file names, or reduce the risk of 
IDOR by generating user IDs etc.

The issue is, they shouldn't use Phobos' randomUUID() for that. 
The Phobos implementation of UUID4 doesn't use cryptographic 
randomness which leads to predictable UUIDs, putting all these 
projects at risk.

Now, the question is: who's at fault? Is it Phobos? The RFC 
doesn't enforce the use of cryptographic randomness and the 
library doesn't claim that these UUIDs are suitable for secret 
generation. Is it the developers then? Every other language that 
I can think of uses cryptographic randomness for UUID4 because 
this kind of use is very common, so I think it is normal for 
developers to expect it to work similarly in D.

I personally think that Phobos must be changed. We can nag 
projects all day long, but any new developer will come and fall 
into the same trap. Furthermore I have thought for a long time 
that Phobos needs to provide a standard interface to the system's 
CSPRNG (CryptGenRandom on Windows, getrandom() or /dev/urandom on 
Linux, /dev/random on Unix). This is a component that does not 
involve the difficulty of cryptographic code (since you're not 
recoding anything) and is absolutely critical to so many 
operations that are very common at the age of secure 
communications. People use the path of least resistance and at 
the moment it is incredibly harder to use secure randomness 
compared to reaching for std.random.uniform(). Clearly randomUUID 
itself fell into this case.

Then again some might argue that developers should read the RFC 
of anything they use and, well, as much as I disagree, the RFC is 
the RFC, wrong as it may be.

I obviously can't solve that conundrum on my own and there are 
far too many projects impacted for me to take the time and reach 
them all, so I've written an article showing how to predict 
Phobos UUIDs in practice. I hope this shows that this is a very 
practical and important issue that must be dealt with, and the 
best way for that would be to generate cryptographically secure 
UUIDs in Phobos.

https://breakpoint.purrfect.fr/article/cracking_phobos_uuid.html

tl;dr: At the moment predicting a future UUID takes only a few 
thousand requests. It's nothing when it comes to things like 
trying session cookies or password recovery tokens. If you are a 
project manager, check whether you use randomUUID to generate 
secrets and replace it by cryptographically secure UUID.

Even if you don't care about the details of the attack, there's a 
section at the end of the article that details how to fix the 
issue and (maybe more importantly) how *not* to fix the issue.

Enjoy your day!
Aug 31 2020
next sibling parent reply WebFreak001 <d.forum webfreak.org> writes:
On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 Hi there,

 As always when I make an appearance it's that something has 
 gone wrong. Either in a popular library or D itself... This 
 time it's a little bit of both.

 [...]


Thanks for the post! Read the crypto review before and surely 
enough this time again it was really fun to read through the 
whole post. I also love the random pictures in your posts :p

I'm not too sure if I ever use randomUUID now, but if it was used 
in vibe.d applications by default that's terrifying to me.
Aug 31 2020
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 8/31/20 9:17 AM, WebFreak001 wrote:

 On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 Hi there,

 As always when I make an appearance it's that something has gone 
 wrong. Either in a popular library or D itself... This time it's a 
 little bit of both.

 [...]


 
 Thanks for the post! Read the crypto review before and surely enough 
 this time again it was really fun to read through the whole post. I also 
 love the random pictures in your posts :p


I share this sentiment, great article!


 
 I'm not too sure if I ever use randomUUID now, but if it was used in 
 vibe.d applications by default that's terrifying to me.


I had to look it up to make sure. The session id producer uses what is 
recommended in the article:

https://github.com/vibe-d/vibe.d/blob/master/crypto/vibe/crypto/cryptorand.d#L125

whew!

-Steve
Aug 31 2020
next sibling parent Cym13 <cpicard purrfect.fr> writes:
On Monday, 31 August 2020 at 16:10:33 UTC, Steven Schveighoffer 
wrote:

 On 8/31/20 9:17 AM, WebFreak001 wrote:

 
 Thanks for the post! Read the crypto review before and surely 
 enough this time again it was really fun to read through the 
 whole post. I also love the random pictures in your posts :p


 I share this sentiment, great article!


Thank you both :)



 I had to look it up to make sure. The session id producer uses 
 what is recommended in the article:

 https://github.com/vibe-d/vibe.d/blob/master/crypto/vibe/crypto/cryptorand.d#L125

 whew!


Yes, cryptorand.d is good. Also, I know I'm trying to detonate a 
bomb here, but I made sure it wasn't *that* big of a bomb first. 
If it got you looking for randomUUID in your projects or others 
half of my goal is accomplished already. There cannot be too many 
eyes on the matter IMHO.
Aug 31 2020
prev sibling parent Andy Balba <pwplus7 gmail.com> writes:
On Monday, 31 August 2020 at 16:10:33 UTC, Steven Schveighoffer 
wrote:

 On 8/31/20 9:17 AM, WebFreak001 wrote:

 On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 Hi there,

 As always when I make an appearance it's that something has 
 gone wrong. Either in a popular library or D itself... This 
 time it's a little bit of both.

 [...]


 
 Thanks for the post! Read the crypto review before and surely 
 enough this time again it was really fun to read through the 
 whole post. I also love the random pictures in your posts :p


 I share this sentiment, great article!


 
 I'm not too sure if I ever use randomUUID now, but if it was 
 used in vibe.d applications by default that's terrifying to me.


 I had to look it up to make sure. The session id producer uses 
 what is recommended in the article:

 https://github.com/vibe-d/vibe.d/blob/master/crypto/vibe/crypto/cryptorand.d#L125

 whew!

 -Steve


I'm trying to create a crypto random number generator based on 
extracting only what is essential from the above code, and the 
only thing blocking this effort are two compile errors : both 
related to 'undefines' for 'IOMode' and  blocking
i.e.

gdc -o aexe rand_crypto.d
rand_crypto.d:62:12: error: undefined identifier ‘IOMode’
      size_t read(scope ubyte[] dst, IOMode mode)  blocking;
             ^
rand_crypto.d:77:21: error: undefined identifier ‘IOMode’
      override size_t read(scope ubyte[] dst, IOMode mode)  safe;
                      ^
rand_crypto.d:29:29: error: undefined identifier ‘blocking’
       property bool empty()  blocking;

After numerous searches, I'm unable to get a definition of vibe 
'IOMode'
.. and haven't a clue on  blocking is about... Any help is 
greatly appreciated.
Sep 05 2020
prev sibling next sibling parent Petar Kirov [ZombineDev] <petar.p.kirov gmail.com> writes:
On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 [..]


Very well written article, thank you for your efforts!
Aug 31 2020
prev sibling next sibling parent reply wjoe <invalid example.com> writes:
On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 [...]


Very insightful article, thank you.

I don't think the right approach to solve any problem is to 
silently support misuse.

A better way, IMHO, would be to
1) Add a generateSecret() function to Phobos, and
2) Add a Warning to the docs and the use of the function that 
says something like: "A Universally Unique Identifier (UUID) is 
*not* a secret and shouldn't be used as such. This includes 
session cookies, password reset tokens, etc.. For such purposes 
use generateSecret() instead. This message can be disabled by 
providing version=IUnderstandThatUUIDsAreNotSecrets", and
3) Silently implement randomUUID() to use generateSecret() 
because people aren't listening.

I believe a big part of any solution is to destroy assumptions 
and to make people aware of their misconceptions/mistakes.
Aug 31 2020
parent reply Cym13 <cpicard purrfect.fr> writes:
On Monday, 31 August 2020 at 14:02:00 UTC, wjoe wrote:

 On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 [...]


 Very insightful article, thank you.

 I don't think the right approach to solve any problem is to 
 silently support misuse.

 A better way, IMHO, would be to
 1) Add a generateSecret() function to Phobos, and
 2) Add a Warning to the docs and the use of the function that 
 says something like: "A Universally Unique Identifier (UUID) is 
 *not* a secret and shouldn't be used as such. This includes 
 session cookies, password reset tokens, etc.. For such purposes 
 use generateSecret() instead. This message can be disabled by 
 providing version=IUnderstandThatUUIDsAreNotSecrets", and
 3) Silently implement randomUUID() to use generateSecret() 
 because people aren't listening.

 I believe a big part of any solution is to destroy assumptions 
 and to make people aware of their misconceptions/mistakes.


It's not a bad battleplan even though a warning at function use 
might be a bit cumbersome :) One of my favourite initiatives for 
security, all languages considered, is Python's standard module 
"secrets" (see [1]). It exposes very few things: 1) the system's 
CSPRNG, 2) various token generators (for example a binary one and 
a url-base64 encoded one to reduced encoding mistakes) and 3) a 
constant-time string comparison function. That's it. Most of 
these things are actually defined elsewhere. But it worked well 
and now whenever a python project requires something that has to 
do with manipulating secrets people are redirected toward that 
module where they can use stuff with minimal chance to get it 
wrong.

[1] https://docs.python.org/3/library/secrets.html
Aug 31 2020
next sibling parent wjoe <invalid example.com> writes:
On Monday, 31 August 2020 at 14:14:12 UTC, Cym13 wrote:

 On Monday, 31 August 2020 at 14:02:00 UTC, wjoe wrote:

 On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:
 2) Add a Warning to the docs and the use of the function that 
 says something like: "A Universally Unique Identifier (UUID)


 It's not a bad battleplan even though a warning at function use 
 might be a bit cumbersome :)


This is true and on purpose. Originally I had
2) Add a Warning to the docs *and/or* the use of the function ...

But people may not read the docs daily/retroactively and how to 
make people aware of problems in existing and forgotten code 
otherwise ?


 [1] https://docs.python.org/3/library/secrets.html


This looks really good.
Aug 31 2020
prev sibling parent reply Kagamin <spam here.lot> writes:
On Monday, 31 August 2020 at 14:14:12 UTC, Cym13 wrote:

 [1] https://docs.python.org/3/library/secrets.html


or 
https://ruby-doc.org/stdlib-2.7.1/libdoc/securerandom/rdoc/SecureRandom.html
But then how do you know that session ids are secrets and not 
just ids?
Sep 02 2020
parent Cym13 <cpicard purrfect.fr> writes:
On Wednesday, 2 September 2020 at 11:14:43 UTC, Kagamin wrote:

 On Monday, 31 August 2020 at 14:14:12 UTC, Cym13 wrote:

 [1] https://docs.python.org/3/library/secrets.html


 or 
 https://ruby-doc.org/stdlib-2.7.1/libdoc/securerandom/rdoc/SecureRandom.html
 But then how do you know that session ids are secrets and not 
 just ids?


In almost all implementations, when a session ID is used it is 
the information that identifies the user as logged in using a 
given account. That information is typically sufficient to obtain 
access to said account, so session IDs are typically secret. Of 
course you can imagine or find a counter-example, but it doesn't 
weaken the point: randomUUID is not suitable to generate secrets.
Sep 03 2020
prev sibling next sibling parent James Blachly <james.blachly gmail.com> writes:
On 8/31/20 3:49 AM, Cym13 wrote:
...

Outstanding! I am certainly scouring my codebases :-)
Aug 31 2020
prev sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On Monday, 31 August 2020 at 07:49:24 UTC, Cym13 wrote:

 I obviously can't solve that conundrum on my own and there are 
 far too many projects impacted for me to take the time and 
 reach them all, so I've written an article showing how to 
 predict Phobos UUIDs in practice. I hope this shows that this 
 is a very practical and important issue that must be dealt 
 with, and the best way for that would be to generate 
 cryptographically secure UUIDs in Phobos.

 https://breakpoint.purrfect.fr/article/cracking_phobos_uuid.html

 tl;dr: At the moment predicting a future UUID takes only a few 
 thousand requests. It's nothing when it comes to things like 
 trying session cookies or password recovery tokens. If you are 
 a project manager, check whether you use randomUUID to generate 
 secrets and replace it by cryptographically secure UUID.


Thanks for highlighting the issue.  One query about your article 
-- you state:


 Phobos' randomUUID() follows these lines perfectly, using 
 non-secure randomness.


... but actually, the code example you posted doesn't inherently 
use non-secure randomness: it accepts any uniform RNG.  It's 
therefore the user's choice whether or not to provide an RNG that 
is cryptographically secure or not.

This by itself seems in line with the RFC as you describe it:


 Note that the RFC does not require the use of cryptographically
 secure random numbers, but it does warn against using UUIDs for
 sensitive values if normal randomness is used.


In other words, the code as written (and as posted in your 
article) gives the user the choice about what their requirements 
are.

There is however an issue about the overload that does not take 
an RNG as input, which defaults to using `rndGen` (the default 
RNG, which on most platforms is indeed the Mersenne Twister).

So it feels like fixing the issue is not about the implementation 
posted in your article, but about what RNG is used by default if 
the parameterless overload of `randomUUID` is called.

Or have I missed something?

A related issue is whether Phobos provides _any_ 
cryptographically secure RNG, whether of its own implementation 
or by access to some other API.
Sep 02 2020
parent reply Cym13 <cpicard purrfect.fr> writes:
On Wednesday, 2 September 2020 at 16:49:30 UTC, Joseph Rushton 
Wakeling wrote:

 Thanks for highlighting the issue.  One query about your 
 article -- you state:


 Phobos' randomUUID() follows these lines perfectly, using 
 non-secure randomness.


 ... but actually, the code example you posted doesn't 
 inherently use non-secure randomness: it accepts any uniform 
 RNG.  It's therefore the user's choice whether or not to 
 provide an RNG that is cryptographically secure or not.


Yes, you can provide it manually with a random number generator 
that is cryptographically secure. I've never seen that, not even 
once. All usage I've seen without exception just calls 
randomUUID() which uses the default PRNG which is the one 
attacked.

It is always possible to opt-into security, but security doesn't 
work if it's not by default as shown in multiple cases of 
projects that had vulnerable code.



 This by itself seems in line with the RFC as you describe it:


 Note that the RFC does not require the use of cryptographically
 secure random numbers, but it does warn against using UUIDs for
 sensitive values if normal randomness is used.


 In other words, the code as written (and as posted in your 
 article) gives the user the choice about what their 
 requirements are.


Indeed, as I said the code is perfectly in line with the RFC. The 
issue is that the RFC predates usage, and isn't in line with said 
usage. Nowadays people expect UUIDs to be cryptographically 
secure, whether that's a fair assumption to make or not. While I 
could see an argument to put the blame on developers, it would 
not be a pragmatic one: there will always be new people coming in 
and falling into the same pitfall. Avoiding that pitfall entirely 
is possible and not too hard, therefore I personnaly recommend 
fixing it rather than just shifting the blame onto developers 
even though I think there is some truth to that.

Security only works if it is by default.


 There is however an issue about the overload that does not take 
 an RNG as input, which defaults to using `rndGen` (the default 
 RNG, which on most platforms is indeed the Mersenne Twister).

 So it feels like fixing the issue is not about the 
 implementation posted in your article, but about what RNG is 
 used by default if the parameterless overload of `randomUUID` 
 is called.

 Or have I missed something?


You have not missed anything. Of course randomUUID itself just 
takes whatever the RNG provides. The point of the article was 
however centered arround randomUUID because while attacking RNGs 
directly is fairly common, I had received doubts that this could 
be attacked in practice in the context of UUIDs. I wanted to 
prove that, yes, even though the attack is harder, it is possible 
to attack a bad PRNG in this context.


 A related issue is whether Phobos provides _any_ 
 cryptographically secure RNG, whether of its own implementation 
 or by access to some other API.


Indeed, it is the heart of the issue. I think quite strongly that 
Phobos must provide access to the system's CSPRNG.
Sep 03 2020
parent reply Cym13 <cpicard purrfect.fr> writes:
On Thursday, 3 September 2020 at 13:23:39 UTC, Cym13 wrote:

 On Wednesday, 2 September 2020 at 16:49:30 UTC, Joseph Rushton 
 Wakeling wrote:

 ...




I feel that answer was confused, let me reformulate.

You are correct when you say that randomUUID() alone just takes 
whatever the RNG provides and isn't broken by itself. Should you 
provide cryptographically secure randomness it would be OK to 
use. However I have never seen randomUUID() used with a PRNG 
other than the default one (and I searched), which is what I 
attacked in that article. The reason why I focus on UUID 
generation and not just breaking the default PRNG on its own is 
twofold: first I saw much more bad usage of randomUUID() than of 
uniform() itself, and second some people had expressed doubts 
that it was exploitable at all in this context. My main goal with 
this article is not to change randomUUID itself (as you pointed 
out, it does not present any bug), but to change the default PRNG 
used for randomUUID() to be a cryptographically secure one 
because this is the only way to prevent more people to fall into 
the same trap of expecting it to be secure.
Sep 03 2020
parent reply Johannes Pfau <nospam example.com> writes:
Am Thu, 03 Sep 2020 13:31:01 +0000 schrieb Cym13:


 On Thursday, 3 September 2020 at 13:23:39 UTC, Cym13 wrote:

 On Wednesday, 2 September 2020 at 16:49:30 UTC, Joseph Rushton Wakeling
 wrote:

 ...




 
 I feel that answer was confused, let me reformulate.
 
 You are correct when you say that randomUUID() alone just takes whatever
 the RNG provides and isn't broken by itself. Should you provide
 cryptographically secure randomness it would be OK to use. However I
 have never seen randomUUID() used with a PRNG other than the default one
 (and I searched), which is what I attacked in that article. The reason
 why I focus on UUID generation and not just breaking the default PRNG on
 its own is twofold: first I saw much more bad usage of randomUUID() than
 of uniform() itself, and second some people had expressed doubts that it
 was exploitable at all in this context. My main goal with this article
 is not to change randomUUID itself (as you pointed out, it does not
 present any bug), but to change the default PRNG used for randomUUID()
 to be a cryptographically secure one because this is the only way to
 prevent more people to fall into the same trap of expecting it to be
 secure.


As the "original author" of that module I can probably add some 
background information on this issue:

The UUID module is mostly a 1:1 port from boost.uuid. I don't remember 
exactly which boost version was used and whether it included a function 
which used the library default random number generator. Current docs in 
boost however also use mt19937 without explicitly warning the user this 
is not cryptographically secure: https://www.boost.org/doc/libs/1_62_0/
libs/uuid/uuid.html#boost/uuid/random_generator.hpp

So this is where the problem originally came from, although it might have 
been my mistake to provide a default overload using the system RNG.


Unfortunately, we can not silently replace this overload to use a secure 
RNG: On linux, would we use random or urandom? And the system rng can 
block on low entropy, which could cause regressions in some applications. 
Also some applications (like vibe.d) would probably rather block a fiber 
than a thread, which complicates things more.


I propose the following:
1) Deprecate the version using the system RNG. Add a hint in the message 
that this function is not cryptographicallly secure. Add a reference to 
documentation how to update code.
2) Update documentation examples to show how to provide the phobos 
default RNG. State that this is not cryptographically secure.
3) Optionally: Implement a Secure, System based RNG. 

I'll open a pull request for steps 1-2 today, so that the immediate 
problem is solved. I'll also have a look how difficult it is to do 3).


-- 
Johannes
Sep 05 2020
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Saturday, 5 September 2020 at 10:41:34 UTC, Johannes Pfau 
wrote:

 Unfortunately, we can not silently replace this overload to use 
 a secure RNG: On linux, would we use random or urandom? And the 
 system rng can block on low entropy, which could cause 
 regressions in some applications. Also some applications (like 
 vibe.d) would probably rather block a fiber than a thread, 
 which complicates things more.


On linux, you would use /dev/urandom (or getrandom(2) if you 
don't need to support old kernel versions), since it is just as 
secure as /dev/random and does not block. [1]

[1] https://www.2uo.de/myths-about-urandom/
Sep 05 2020
parent Johannes Pfau <nospam example.com> writes:
Am Sat, 05 Sep 2020 13:06:14 +0000 schrieb Paul Backus:


 On Saturday, 5 September 2020 at 10:41:34 UTC, Johannes Pfau wrote:

 Unfortunately, we can not silently replace this overload to use a
 secure RNG: On linux, would we use random or urandom? And the system
 rng can block on low entropy, which could cause regressions in some
 applications. Also some applications (like vibe.d) would probably
 rather block a fiber than a thread, which complicates things more.


 
 On linux, you would use /dev/urandom (or getrandom(2) if you don't need
 to support old kernel versions), since it is just as secure as
 /dev/random and does not block. [1]
 
 [1] https://www.2uo.de/myths-about-urandom/


Hmm, good to know, thanks. Seems kinda stupid that urandom returns 
insecure data before it is first seeded at boot though, but I guess 
that's the way it is...

-- 
Johannes
Sep 05 2020
prev sibling next sibling parent Seb <seb wilzba.ch> writes:
On Saturday, 5 September 2020 at 10:41:34 UTC, Johannes Pfau 
wrote:

 3) Optionally: Implement a Secure, System based RNG.

 I'll open a pull request for steps 1-2 today, so that the 
 immediate problem is solved. I'll also have a look how 
 difficult it is to do 3).


Have a look at these two well-tested implementations:

https://github.com/libmir/mir-random/blob/master/source/mir/random/engine/package.d
https://github.com/vibe-d/vibe.d/blob/master/crypto/vibe/crypto/cryptorand.d

You could even copy-paste one of them into Phobos as Vibe.d's 
implementation is based on Mir's which in turn is Boost-licensed.
Sep 05 2020
prev sibling next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/5/20 6:41 AM, Johannes Pfau wrote:

 
 Unfortunately, we can not silently replace this overload to use a secure
 RNG: On linux, would we use random or urandom? And the system rng can
 block on low entropy, which could cause regressions in some applications.
 Also some applications (like vibe.d) would probably rather block a fiber
 than a thread, which complicates things more.


1. The default should be changed, even if it's not as performant. There 
is no promise about randomUUID's performance.

2. vibe.d does not depend on this, so there are no worries about 
blocking a thread.

-Steve
Sep 05 2020
parent reply Johannes Pfau <nospam example.com> writes:
Am Sat, 05 Sep 2020 21:17:59 -0400 schrieb Steven Schveighoffer:


 On 9/5/20 6:41 AM, Johannes Pfau wrote:

 
 Unfortunately, we can not silently replace this overload to use a
 secure RNG: On linux, would we use random or urandom? And the system
 rng can block on low entropy, which could cause regressions in some
 applications.
 Also some applications (like vibe.d) would probably rather block a
 fiber than a thread, which complicates things more.


 
 1. The default should be changed, even if it's not as performant. There
 is no promise about randomUUID's performance.
 
 2. vibe.d does not depend on this, so there are no worries about
 blocking a thread.
 
 -Steve


1) This is not about performance. geturandom, /dev/urandom on FreeBSD and 
other cryptographic random number generators can block if they have not 
been seeded with enough entropy yet. There are well-known bugs caused by 
this when programs in early boot use these interfaces, block and 
therefore cause the whole system boot to fail: https://bugs.debian.org/
cgi-bin/bugreport.cgi?bug=897572
https://wiki.debian.org/BoottimeEntropyStarvation

On small embeeded systems with less entropy sources, it may take even 
longer to properly seed the system random number generators.

Silently changin randomUUID() to use such an interface means some 
programs which do not care about UUIDs being secure might block, which 
could cause catastrophic effects as in the above bug reports. Although 
it's unlikely such low-level tools are written in D, we can not simply 
assume that.

Because of that, the only valid solution is to remove the default 
overload and let the user make an informed decision.


-- 
Johannes
Sep 06 2020
next sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 9/6/20 4:12 AM, Johannes Pfau wrote:

 Am Sat, 05 Sep 2020 21:17:59 -0400 schrieb Steven Schveighoffer:
 

 On 9/5/20 6:41 AM, Johannes Pfau wrote:

 Unfortunately, we can not silently replace this overload to use a
 secure RNG: On linux, would we use random or urandom? And the system
 rng can block on low entropy, which could cause regressions in some
 applications.
 Also some applications (like vibe.d) would probably rather block a
 fiber than a thread, which complicates things more.


 1. The default should be changed, even if it's not as performant. There
 is no promise about randomUUID's performance.

 2. vibe.d does not depend on this, so there are no worries about
 blocking a thread.


 
 1) This is not about performance. geturandom, /dev/urandom on FreeBSD and
 other cryptographic random number generators can block if they have not
 been seeded with enough entropy yet. There are well-known bugs caused by
 this when programs in early boot use these interfaces, block and
 therefore cause the whole system boot to fail: https://bugs.debian.org/
 cgi-bin/bugreport.cgi?bug=897572
 https://wiki.debian.org/BoottimeEntropyStarvation
 
 On small embeeded systems with less entropy sources, it may take even
 longer to properly seed the system random number generators.
 
 Silently changin randomUUID() to use such an interface means some
 programs which do not care about UUIDs being secure might block, which
 could cause catastrophic effects as in the above bug reports. Although
 it's unlikely such low-level tools are written in D, we can not simply
 assume that.
 
 Because of that, the only valid solution is to remove the default
 overload and let the user make an informed decision.


How is that the only valid solution? Another valid solution is -- change 
the (most likely not-existing) application that absolutely cannot block 
to use a defined PRNG that doesn't block, while benefiting the security 
of every other application that is not hampered by system startup 
entropy starvation.

This isn't even changing something that will break code or create a 
situation where randomUUID will produce a different code path, or 
different outputs that may break something. I see no reason to go 
through this pain for a theoretical problem that likely doesn't exist.

I will note that the most likely result from the changes you are 
proposing is that people go from using randomUUID() to 
rndGen.randomUUID(), and I don't see that being the correct result. 
That's even what your PR recommends!

It should be: "If you don't care about the random number generator that 
is going to generate your UUID, I'm going to use a secure one."

The more I think about this, the more I feel like the solution you have 
arrived at is completely backwards.

-Steve
Sep 06 2020
prev sibling parent reply Cym13 <cpicard purrfect.fr> writes:
On Sunday, 6 September 2020 at 08:12:56 UTC, Johannes Pfau wrote:

 1) This is not about performance. geturandom, /dev/urandom on 
 FreeBSD and other cryptographic random number generators can 
 block if they have not been seeded with enough entropy yet. 
 There are well-known bugs caused by this when programs in early 
 boot use these interfaces, block and therefore cause the whole 
 system boot to fail: https://bugs.debian.org/ 
 cgi-bin/bugreport.cgi?bug=897572 
 https://wiki.debian.org/BoottimeEntropyStarvation

 On small embeeded systems with less entropy sources, it may 
 take even longer to properly seed the system random number 
 generators.


Booting on real systems is rarely an issue besides the very first 
boot because the system seeds itself from a random blob that it 
regularily saves on disk. But yes, on some systems and especially 
embedded ones it can be an issue.

Phobos can't reasonnably expect to manage all cases perfectly by 
default. There's no solution that fits all. But there's a 
solution that fits 99% of cases, and that's what should be 
implemented. This does not present any change for people in the 
minority case that already had to implement something specific 
before.


 Silently changin randomUUID() to use such an interface means 
 some programs which do not care about UUIDs being secure might 
 block, which could cause catastrophic effects as in the above 
 bug reports. Although it's unlikely such low-level tools are 
 written in D, we can not simply assume that.

 Because of that, the only valid solution is to remove the 
 default overload and let the user make an informed decision.


Informed decision by itself doesn't work in security. What works 
is providing sane defaults that cover the 99% case (which is what 
defaults are for) and leaving open the possibility to use 
something else. I see no reasonnable reason to take something 
that isn't safe and replacing it by something that works just as 
well and is safer.

An important note here: I'm not even advocating to advertise that 
UUID are secure (I would prefer a dedicated, separate function to 
generate secure tokens of more than 122 bits), but we need to 
make sure defaults are sane. There's nothing to be lost in this 
case: it's replacing an "always unsecure" by "secure in all cases 
except that very specific one".

Most people are not able to make an informed decision when it 
comes to security. It's a bit like informed consent in the 
medical field: sure you can give your opinion but it doesn't 
change the fact that you would need years of training to actually 
understand the consequences of what you're choosing. I'm not 
saying not to explain the situation, but it should never be a 
justification for not providing safe defaults. The lack of safe 
defaults is the reason why this whole thing became an issue in 
the first place.

Leave corner cases where they are: corner cases. They should not 
impact the default.
Sep 07 2020
parent Cym13 <cpicard purrfect.fr> writes:
On Monday, 7 September 2020 at 07:16:17 UTC, Cym13 wrote:

 I see no reasonnable reason to take something that isn't safe 
 and replacing it by something that works just as well and is 
 safer.


I of course meant: "I see no reasonnable reason *not* to take ..."
Sep 07 2020
prev sibling parent Johannes Pfau <nospam example.com> writes:
Am Sat, 05 Sep 2020 10:41:34 +0000 schrieb Johannes Pfau:


 
 I propose the following:
 1) Deprecate the version using the system RNG. Add a hint in the message
 that this function is not cryptographicallly secure. Add a reference to
 documentation how to update code.
 2) Update documentation examples to show how to provide the phobos
 default RNG. State that this is not cryptographically secure.
 3) Optionally: Implement a Secure, System based RNG.
 
 I'll open a pull request for steps 1-2 today, so that the immediate
 problem is solved. I'll also have a look how difficult it is to do 3).



PR for 1), 2) is here: https://github.com/dlang/phobos/pull/7618
Testing is mostly ok, the only problem is that the deprecation triggers
on import std.uuid : randomUUID even if the 0-parameter overload is not 
used. The dscanner test however complains in one of the unittests when
using a local, non-selective import..

Initial PR for 3) is at https://github.com/dlang/phobos/pull/7619 but 
faces some more serious complications:
* how to handle static initialization of __gshared variables
* how to do file IO. std.stdio.File is not  nogc, reinventing the wheel 
and using low-level posix/C APIs as in vibe.d or MIR is usually also not 
welcome in phobos.
* (how to detect kernel / glibc version. So we want to do syscalls 
manually, like vibe.d, ignoring the glibc version? Then we still have to 
check if the kernel supports the syscall. Sounds annoying, so I used the /
dev/urandom implementation for linux for now).

I unfortunately don't have much time to work on 3), so if anyone wants to 
chime in there, feel free to enhance that code in the PR ;-)


-- 
Johannes
Sep 06 2020