Random portion should be using cryptographic randomness

[sehrope]

The random part of the sequential UUID is being generated with non-cryptographic random(). It should be using something like pg_strong_random(...) instead:

diff --git a/sequential_uuids.c b/sequential_uuids.c
index ae5dbac..275d0f4 100644
--- a/sequential_uuids.c
+++ b/sequential_uuids.c
@@ -149,12 +149,10 @@ uuid_time_nextval(PG_FUNCTION_ARGS)
        for (i = 0; i < prefix_bytes; i++)
                uuid->data[i] = p[prefix_bytes - 1 - i];

-       /*
-        * TODO optimize this by using larger chunks of the random value
-        * (it should be 4B in most cases)
-        */
-       for (i = prefix_bytes; i < UUID_LEN; i++)
-               uuid->data[i] = (random() % 256);
+       if(!pg_strong_random(uuid.data + prefix_bytes, UUID_LEN - prefix_bytes))
+               ereport(ERROR,
+                               (errcode(ERRCODE_INTERNAL_ERROR),
+                                errmsg("could not generate random values")));

        PG_RETURN_UUID_P(uuid);
 }

May want to consider setting the v4 UUID version flags as well:

https://github.com/postgres/postgres/blob/517bf2d9107f0d45c5fea2e3904e8d3b10ce6bb2/src/backend/utils/adt/uuid.c#L430-L435

I've never ran into anything that cares about the UUID version flags and it makes the values slightly less random since you need to replace those bytes, but worth considering to make it more spec friendly.

[tvondra]

Yes, I agree I should have used pg_strong_random, thanks for spotting this. Will fix.

Not sure about the version flags, though. The thing is, this does not quite match any of the generators defined in the RFC, so it seems a bit wrong to indicate this. But maybe that means we should set those bits to zero?

[sehrope]

I think it's still a random UUID, the random source for the first chunk just cycles faster and is not cryptographically random so setting the standard v4 flags should be fine.

By my reading, the UUID RFC (https://tools.ietf.org/html/rfc4122#section-4.5) does not mandate a particular quality for the random number source. Could do worse: https://xkcd.com/221/

[tvondra]

LOL

OK, makes sense.

[tvondra]

I've pushed fixes for both issues.

It however seems setting the variant/version bits may interfere with the prefix. It does tweak bytes 6 and 8, so if the prefix is larger than 6 bytes, this will reset some of the bytes, effectively "merging" some of the prefixes. So some of the blocks will be much larger (variant + version have 7 bits, so the "merged" block will be 127-times larger than desired).

I don't think it's extremely problematic, though, because it only affects cases with prefixes longer than 6 bytes, which means 281474976710656 blocks. That seems rather unlikely, because the functions only accept int parameters. So it's "can't happen in practice" I think.

[sehrope]

Nice. Yes I think the merging of the bits should be fine.

I tried out the latest and found a couple other bounds issues. I'm going to submit a PR for them in a bit.