nu7hatch/gouuid Duplicate sequentially generated V4 UUIDs

[jaytaylor]

README from fork https://github.com/jaytaylor/uuid:

DEPRECATED DUE TO POOR RELIABILITY

PLEASE DO DO NOT USE!

This package is deprecated due to my having observed it generating duplicate UUID's in the wild from uuid.NewV4().

Recommendation

The current recommended UUID package for go is satori/go.uuid.

Also, RE: nu7hatch

If I could, I would also nuke nu7hatch/go-uuid from existence, as the core of this package is identical to it.

[rosenhouse]

hey @jaytaylor I'm looking at the V4 implementation in satori/go.uuid and the one here. I'm not seeing any meaningful differences. Could you explain what about the V4 code path is different between the two packages?

I found this issue via cloudfoundry/cf-smoke-tests#13 where I'm cross-posting the same question.

Thanks!

[jaytaylor]

IIRC the satori one returns a _UUID while the nu7hatch one returns a tulle of (_UUID, error).

Thus the implementations and critical paths are different in that the satori one promises to never return an error.

On Nov 24, 2015, at 6:52 PM, Gabe Rosenhouse notifications@github.com wrote:

hey @jaytaylor I'm looking at the V4 implementation in satori/go.uuid and the one here. I'm not seeing any meaningful differences. Could you explain what about the V4 code path is different between the two packages?

I found this issue via cloudfoundry/cf-smoke-tests#13 where I'm cross-posting the same question.

—
Reply to this email directly or view it on GitHub.

[ryanmoran]

I am not sure I follow the reasoning here at all.

A simplified version of the "offending" code is offered below:

package uuid

import "crypto/rand"

type UUID [16]byte

func NewV4() (*UUID, error) {
    u = new(UUID)

     _, err = rand.Read(u[:])
    if err != nil {
        return
    }

    return
}

And here is the same code in the satori/go.uuid codebase:

package uuid

import "crypto/rand"

type UUID [16]byte

func NewV4() UUID {
    u := UUID{}
    safeRandom(u[:])
    return u
}

func safeRandom(dest []byte) {
    if _, err := rand.Read(dest); err != nil {
        panic(err)
    }
}

So, the satori/go.uuid package is somehow safer because it does not return an error?
But that is just because it hides the error in the safeRandom function that panics.

As we can also see, both packages are simply providing wrappers around the stdlib crypto/rand package.
So if you have some issue with the "randomness" of this library, then your concern should be directed at the stdlib package.
Both libraries consume and use the same stdlib package and so could not provide any difference in "randomness".

So, maybe we should take a look at the "crypto/rand" package.

// Copyright 2010 The Go Authors.  All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// Package rand implements a cryptographically secure
// pseudorandom number generator.
package rand

import "io"

// Reader is a global, shared instance of a cryptographically
// strong pseudo-random generator.
//
// On Unix-like systems, Reader reads from /dev/urandom.
// On Linux, Reader uses getrandom(2) if available, /dev/urandom otherwise.
// On Windows systems, Reader uses the CryptGenRandom API.
var Reader io.Reader

// Read is a helper function that calls Reader.Read using io.ReadFull.
// On return, n == len(b) if and only if err == nil.
func Read(b []byte) (n int, err error) {
    return io.ReadFull(Reader, b)
}

As we can see in the comment of this package, the Read function will call out to a cryptographically strong pseudo-random generator.
But what does that mean? What kind of guarantees does it make?

From the Wikipedia article describing CSPRNGs
we can see that there are issues in generating truly random numbers.

This paragraph seems important:

Ideally, the generation of random numbers in CSPRNGs uses entropy obtained from a
high-quality source, which might be a hardware random number generator or perhaps
unpredictable system processes — though unexpected correlations have been found in
several such ostensibly independent processes. From an information-theoretic point of
view, the amount of randomness, the entropy that can be generated, is equal to the
entropy provided by the system. But sometimes, in practical situations, more random
numbers are needed than there is entropy available. Also the processes to extract
randomness from a running system are slow in actual practice. In such instances, a
CSPRNG can sometimes be used. A CSPRNG can "stretch" the available entropy over more bits.

From here we can see that it really comes down to the hardware that your code is running on.
Some hardware will unfortunately have less access to available entropy.
For example, virtualized hardware, running in the cloud can exhibit entropy starvation.

There is pretty much nothing this library can do to increase the pool of entropy.
And both of these libraries have almost identical implementations (ignoring the strange use of "safe" to describe a codepath that could crash your process).

This just seems like a groundless issue to me. Am I missing something here?

[rosenhouse]

Right.

There's nothing magic about the satori implementation of V4. It just opts to crash on error instead of returning an error to the caller.

It is an interesting design choice for a library author. It contradicts the recommendations of the Go maintainers, who write in Effective Go:

. . . real library functions should avoid panic

[jaytaylor]

I agree that seems like an odd choice. Would be curious and interested to know which conditions the panic could be triggered IRL.

On Dec 5, 2015, at 4:24 PM, Gabe Rosenhouse notifications@github.com wrote:

Right.

There's nothing magic about the satori implementation of V4. It just opts to crash on error instead of returning an error to the caller.

It is an interesting design choice for a library author. It contradicts the recommendations of the Go maintainers, who write in Effective Go:

. . . real library functions should avoid panic

—
Reply to this email directly or view it on GitHub.