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

std.uuid.randomUUID() is insecure and has been deprecated #7618

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions changelog/uuid_deprecation.dd
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
std.uuid.randomUUID() is insecure and has been deprecated

The `randomUUID()` overload not taking any arguments has been deprecated.
It is dangerous, as it causes predictable, non-secure UUIDs and has not been
documented as doing that previously. Silently changing this function to use a secure
system RNG is not possible, as these RNGs may block at early boot when they are
not yet seeded, causing `randomUUID` to block as well.
See $(LINK2 https://forum.dlang.org/thread/zjmyxudvscfwrkrdabmt@forum.dlang.org, this forum thread)
for a detailed discussion of the problem.

Depending on whether the desired output needs to be cryptographically secure and
considering the potential blocking problem, users should use `rndGen` (insecure)
from std.random or a secure alternative like this:
----------------------------
auto insecureUUID = rndGen.randomUUID();
----------------------------
20 changes: 11 additions & 9 deletions std/process.d
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,9 @@ version (Posix) @system unittest
assert(wait(spawnProcess(envProg.path, env, Config.newEnv)) == 6);
}

version (unittest)
import std.uuid;

@system unittest // Stream redirection in spawnProcess().
{
import std.path : buildPath;
Expand All @@ -1615,11 +1618,11 @@ version (Posix) @system unittest
// Pipes
void testPipes(Config config)
{
import std.file, std.uuid, core.thread, std.exception;
import std.file, core.thread, std.exception;
auto pipei = pipe();
auto pipeo = pipe();
auto pipee = pipe();
auto done = buildPath(tempDir(), randomUUID().toString());
auto done = buildPath(tempDir(), rndGen.randomUUID().toString());
auto pid = spawnProcess([prog.path, "foo", "bar", done],
pipei.readEnd, pipeo.writeEnd, pipee.writeEnd, null, config);
pipei.writeEnd.writeln("input");
Expand All @@ -1636,15 +1639,15 @@ version (Posix) @system unittest
// Files
void testFiles(Config config)
{
import std.ascii, std.file, std.uuid, core.thread, std.exception;
auto pathi = buildPath(tempDir(), randomUUID().toString());
auto patho = buildPath(tempDir(), randomUUID().toString());
auto pathe = buildPath(tempDir(), randomUUID().toString());
import std.ascii, std.file, core.thread, std.exception;
auto pathi = buildPath(tempDir(), rndGen.randomUUID().toString());
auto patho = buildPath(tempDir(), rndGen.randomUUID().toString());
auto pathe = buildPath(tempDir(), rndGen.randomUUID().toString());
std.file.write(pathi, "INPUT"~std.ascii.newline);
auto filei = File(pathi, "r");
auto fileo = File(patho, "w");
auto filee = File(pathe, "w");
auto done = buildPath(tempDir(), randomUUID().toString());
auto done = buildPath(tempDir(), rndGen.randomUUID().toString());
auto pid = spawnProcess([prog.path, "bar", "baz", done], filei, fileo, filee, null, config);
if (config & Config.detached)
while (!done.exists) Thread.sleep(10.msecs);
Expand Down Expand Up @@ -3271,10 +3274,9 @@ private string uniqueTempPath() @safe
{
import std.file : tempDir;
import std.path : buildPath;
import std.uuid : randomUUID;
// Path should contain spaces to test escaping whitespace
return buildPath(tempDir(), "std.process temporary file " ~
randomUUID().toString());
rndGen.randomUUID().toString());
}


Expand Down
6 changes: 4 additions & 2 deletions std/range/package.d
Original file line number Diff line number Diff line change
Expand Up @@ -11315,16 +11315,18 @@ that break its sorted-ness, `SortedRange` will work erratically.
assert(s.groupBy.equal!equal([[100, 101, 102], [200, 201], [300]]));
}

version (unittest)
import std.uuid;

// Test on an input range
@system unittest
{
import std.conv : text;
import std.file : exists, remove, tempDir;
import std.path : buildPath;
import std.stdio : File;
import std.uuid : randomUUID;
auto name = buildPath(tempDir(), "test.std.range.line-" ~ text(__LINE__) ~
"." ~ randomUUID().toString());
"." ~ rndGen.randomUUID().toString());
auto f = File(name, "w");
scope(exit) if (exists(name)) remove(name);
// write a sorted range of lines to the file
Expand Down
75 changes: 60 additions & 15 deletions std/uuid.d
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ module std.uuid;
import std.uuid;

UUID[] ids;
ids ~= randomUUID();

// Note: this output is not cryptographically secure, the UUID can be predicted!
ids ~= rndGen.randomUUID();

ids ~= md5UUID("test.name.123");
ids ~= sha1UUID("test.name.123");

Expand Down Expand Up @@ -969,7 +972,8 @@ public struct UUID
UUID id;
assert(id.empty);

id = randomUUID;
// Not cryptographically secure, output is predictable
id = rndGen.randomUUID();
assert(!id.empty);

id = UUID(cast(ubyte[16]) [138, 179, 6, 14, 44, 186, 79,
Expand Down Expand Up @@ -1203,10 +1207,29 @@ public struct UUID
*
* This function is not supported at compile time.
*
* Params:
* randomGen = uniform RNG
* Warning:
* This function should not be used when UUIDs need to be cryptographically secure,
* as the output of this function can be predicted.
*
* Deprecated:
* The `randomUUID()` overload not taking any arguments has been deprecated.
* It is dangerous, as it causes predictable, non-secure UUIDs and has not been
* documented as doing that previously. Silently changing this function to use a secure
* system RNG is not possible, as these RNGs may block at early boot when they are
* not yet seeded, causing `randomUUID` to block as well.
* See $(LINK2 https://forum.dlang.org/thread/zjmyxudvscfwrkrdabmt@forum.dlang.org, this forum thread)
* for a detailed discussion of the problem.
*
* Depending on whether the desired output needs to be cryptographically secure and
* considering the potential blocking problem, users should use `rndGen` (insecure)
* from std.random or a secure alternative like this:
* ----------------------------
* auto insecureUUID = rndGen.randomUUID();
* ----------------------------
*
* See_Also: $(REF isUniformRNG, std,random)
*/
deprecated("randomUUID()` is cryptographically insecure, use `rndGen.randomUUID()` if this was intended")
@safe UUID randomUUID()
{
import std.random : rndGen;
Expand All @@ -1231,7 +1254,25 @@ public struct UUID
}
}

/// ditto
/// Provide a convenience import for the insecure default RNG from std.random
public import std.random : rndGen;

/**
* This function generates a random number based UUID from a random
* number generator.
*
* This function is not supported at compile time.
*
* Warning:
* When UUIDs are used in a context, which requires them to be
* cryptographically secure, the phobos prngs from std.random,
* including rndGen, should not be used. The output of these
* rngs can be predicted, leading to predictable UUIDs.
*
* Params:
* randomGen = uniform RNG
* See_Also: $(REF isUniformRNG, std,random)
*/
UUID randomUUID(RNG)(ref RNG randomGen)
if (isInputRange!RNG && isIntegral!(ElementType!RNG))
{
Expand Down Expand Up @@ -1268,29 +1309,33 @@ if (isInputRange!RNG && isIntegral!(ElementType!RNG))
{
import std.random : Xorshift192, unpredictableSeed;

//simple call
auto uuid = randomUUID();
// Using phobos std.random.rndGen (not cryptographically secure!)
auto uuid = rndGen.randomUUID();

//provide a custom RNG. Must be seeded manually.
// provide a custom RNG, which must be seeded manually.
// Xorshift192 is not cryptographically secure either.
Xorshift192 gen;

gen.seed(unpredictableSeed);
auto uuid3 = randomUUID(gen);
auto uuid3 = gen.randomUUID();
}

@safe unittest
{
import std.random : Xorshift192, unpredictableSeed;
//simple call
auto uuid = randomUUID();

//provide a custom RNG. Must be seeded manually.
// Using phobos std.random.rndGen (not cryptographically secure!)
auto uuid = rndGen.randomUUID();

// provide a custom RNG, which must be seeded manually.
// Xorshift192 is not cryptographically secure either.
Xorshift192 gen;

gen.seed(unpredictableSeed);
auto uuid3 = randomUUID(gen);
auto uuid3 = gen.randomUUID();

auto u1 = randomUUID();
auto u2 = randomUUID();
auto u1 = rndGen.randomUUID();
auto u2 = rndGen.randomUUID();
assert(u1 != u2);
assert(u1.variant == UUID.Variant.rfc4122);
assert(u1.uuidVersion == UUID.Version.randomNumberBased);
Expand Down