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

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

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

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.

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 ..."