From 775d98fb87dc3afeafd1c740ac8affae8b6cc05a Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Fri, 15 Feb 2019 08:56:27 -0800 Subject: [PATCH] Remove support for DCE Security UUID (V2) generation This change removes our support for generating V2 UUIDs from the package, since it seems that the current implementation is not up to spec in relation to the DCE 1.1 Specification[1][2]. In our implementation the data written in to the V1, V3-V5 UUID uses Big Endian byte order as specified in RFC-4122[3]. However, when looking at the DCE specification they make it read like the data should be written in not Little Endian but a format where you completely reverse the bites of the value from most significant first to least significant first: > Set the time_low field equal to the least significant 32-bits (bits numbered > 0 to 31 inclusive) of the time stamp in the same order of significance. If a > DCE Security version UUID is being created, then replace the time_low field > with the local user security attribute as defined by the DCE: Security > Services specification. There are more things later confirming the order: > Set the time_mid field equal to the bits numbered 32 to 47 inclusive of the > time stamp in the same order of significance. This timestamp handling instructions seem dangerous because the specification is telling us to overwrite the lest-significant bits of the UUID timestamp value with a deterministic value, meaning the remaining bits of the timestamp will now change much less frequently. Because the rest of the UUID consists of distinct / deterministic values, this will make the UUID much less unique. This also means that for the DCE Security UUIDs we would need completely different generating and parsing code than V1 and V3-V5 UUIDs. Considering that it seems like there would be quite a bit of work to support them correctly. To confirm that my understanding of the spec is correct, I tried to look at preexisting implementations to validate my understanding. Unfortunately, I've not had luck finding a UUID library that implements V2 UUIDs. That points to there being little to no value in continuing to try and support them, as there is not widespread availability of complementing implementations. This change will require a major version bump. [1] http://pubs.opengroup.org/onlinepubs/9629399/apdxa.htm#tagcjh_20_02_05 [2] http://pubs.opengroup.org/onlinepubs/9668899/chap5.htm#tagcjh_08_02_01_01 [3] https://tools.ietf.org/html/rfc4122#section-4.1.2 Signed-off-by: Tim Heckman --- README.md | 1 - generator.go | 36 +-------------------------- generator_test.go | 63 ----------------------------------------------- uuid.go | 18 ++++++++++---- 4 files changed, 14 insertions(+), 104 deletions(-) diff --git a/README.md b/README.md index efc3204..2685a83 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,6 @@ and parsing of UUIDs in different formats. This package supports the following UUID versions: * Version 1, based on timestamp and MAC address (RFC-4122) -* Version 2, based on timestamp, MAC address and POSIX UID/GID (DCE 1.1) * Version 3, based on MD5 hashing of a named value (RFC-4122) * Version 4, based on random numbers (RFC-4122) * Version 5, based on SHA-1 hashing of a named value (RFC-4122) diff --git a/generator.go b/generator.go index 4257761..2783d9e 100644 --- a/generator.go +++ b/generator.go @@ -30,7 +30,6 @@ import ( "hash" "io" "net" - "os" "sync" "time" ) @@ -47,21 +46,11 @@ type HWAddrFunc func() (net.HardwareAddr, error) // DefaultGenerator is the default UUID Generator used by this package. var DefaultGenerator Generator = NewGen() -var ( - posixUID = uint32(os.Getuid()) - posixGID = uint32(os.Getgid()) -) - // NewV1 returns a UUID based on the current timestamp and MAC address. func NewV1() (UUID, error) { return DefaultGenerator.NewV1() } -// NewV2 returns a DCE Security UUID based on the POSIX UID/GID. -func NewV2(domain byte) (UUID, error) { - return DefaultGenerator.NewV2(domain) -} - // NewV3 returns a UUID based on the MD5 hash of the namespace UUID and name. func NewV3(ns UUID, name string) UUID { return DefaultGenerator.NewV3(ns, name) @@ -80,7 +69,6 @@ func NewV5(ns UUID, name string) UUID { // Generator provides an interface for generating UUIDs. type Generator interface { NewV1() (UUID, error) - NewV2(domain byte) (UUID, error) NewV3(ns UUID, name string) UUID NewV4() (UUID, error) NewV5(ns UUID, name string) UUID @@ -164,28 +152,6 @@ func (g *Gen) NewV1() (UUID, error) { return u, nil } -// NewV2 returns a DCE Security UUID based on the POSIX UID/GID. -func (g *Gen) NewV2(domain byte) (UUID, error) { - u, err := g.NewV1() - if err != nil { - return Nil, err - } - - switch domain { - case DomainPerson: - binary.BigEndian.PutUint32(u[:], posixUID) - case DomainGroup: - binary.BigEndian.PutUint32(u[:], posixGID) - } - - u[9] = domain - - u.SetVersion(V2) - u.SetVariant(VariantRFC4122) - - return u, nil -} - // NewV3 returns a UUID based on the MD5 hash of the namespace UUID and name. func (g *Gen) NewV3(ns UUID, name string) UUID { u := newFromHash(md5.New(), ns, name) @@ -216,7 +182,7 @@ func (g *Gen) NewV5(ns UUID, name string) UUID { return u } -// Returns the epoch and clock sequence. +// getClockSequence returns the epoch and clock sequence. func (g *Gen) getClockSequence() (uint64, uint16, error) { var err error g.clockSequenceOnce.Do(func() { diff --git a/generator_test.go b/generator_test.go index 35b59b7..317956f 100644 --- a/generator_test.go +++ b/generator_test.go @@ -32,7 +32,6 @@ import ( func TestGenerator(t *testing.T) { t.Run("NewV1", testNewV1) - t.Run("NewV2", testNewV2) t.Run("NewV3", testNewV3) t.Run("NewV4", testNewV4) t.Run("NewV5", testNewV5) @@ -171,63 +170,6 @@ func testNewV1MissingNetworkFaultyRand(t *testing.T) { } } -func testNewV2(t *testing.T) { - t.Run("Basic", testNewV2Basic) - t.Run("DifferentAcrossCalls", testNewV2DifferentAcrossCalls) - t.Run("FaultyRand", testNewV2FaultyRand) -} - -func testNewV2Basic(t *testing.T) { - domains := []byte{ - DomainPerson, - DomainGroup, - DomainOrg, - } - for _, domain := range domains { - u, err := NewV2(domain) - if err != nil { - t.Errorf("NewV2(%d): %v", domain, err) - } - if got, want := u.Version(), V2; got != want { - t.Errorf("NewV2(%d) generated UUID with version %d, want %d", domain, got, want) - } - if got, want := u.Variant(), VariantRFC4122; got != want { - t.Errorf("NewV2(%d) generated UUID with variant %d, want %d", domain, got, want) - } - } -} - -func testNewV2DifferentAcrossCalls(t *testing.T) { - u1, err := NewV2(DomainOrg) - if err != nil { - t.Fatal(err) - } - u2, err := NewV2(DomainOrg) - if err != nil { - t.Fatal(err) - } - if u1 == u2 { - t.Errorf("generated identical UUIDs across calls: %v", u1) - } -} - -func testNewV2FaultyRand(t *testing.T) { - g := &Gen{ - epochFunc: time.Now, - hwAddrFunc: defaultHWAddrFunc, - rand: &faultyReader{ - readToFail: 0, // fail immediately - }, - } - u, err := g.NewV2(DomainPerson) - if err == nil { - t.Fatalf("got %v, want error", u) - } - if u != Nil { - t.Fatalf("got %v on error, want Nil", u) - } -} - func testNewV3(t *testing.T) { t.Run("Basic", testNewV3Basic) t.Run("EqualNames", testNewV3EqualNames) @@ -382,11 +324,6 @@ func BenchmarkGenerator(b *testing.B) { NewV1() } }) - b.Run("NewV2", func(b *testing.B) { - for i := 0; i < b.N; i++ { - NewV2(DomainOrg) - } - }) b.Run("NewV3", func(b *testing.B) { for i := 0; i < b.N; i++ { NewV3(NamespaceDNS, "www.example.com") diff --git a/uuid.go b/uuid.go index 29ef440..88707b8 100644 --- a/uuid.go +++ b/uuid.go @@ -19,11 +19,19 @@ // OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -// Package uuid provides implementations of the Universally Unique Identifier (UUID), as specified in RFC-4122 and DCE 1.1. +// Package uuid provides implementations of the Universally Unique Identifier +// (UUID), as specified in RFC-4122, // // RFC-4122[1] provides the specification for versions 1, 3, 4, and 5. // -// DCE 1.1[2] provides the specification for version 2. +// DCE 1.1[2] provides the specification for version 2, but version 2 support +// was removed from this package in v4 due to some concerns with the +// specification itself. Reading the spec, it seems that it would result in +// generating UUIDs that aren't very unique. In having read the spec it seemed +// that our implementation did not meet the spec. It also seems to be at-odds +// with RFC 4122, meaning we would need quite a bit of special code to support +// it. Lastly, there were no Version 2 implementations that we could find to +// ensure we were understanding the specification correctly. // // [1] https://tools.ietf.org/html/rfc4122 // [2] http://pubs.opengroup.org/onlinepubs/9696989899/chap5.htm#tagcjh_08_02_01_01 @@ -46,7 +54,7 @@ type UUID [Size]byte const ( _ byte = iota V1 // Version 1 (date-time and MAC address) - V2 // Version 2 (date-time and MAC address, DCE security version) + _ // Version 2 (date-time and MAC address, DCE security version) [removed] V3 // Version 3 (namespace name-based) V4 // Version 4 (random) V5 // Version 5 (namespace name-based) @@ -68,8 +76,8 @@ const ( ) // Timestamp is the count of 100-nanosecond intervals since 00:00:00.00, -// 15 October 1582 within a V1 UUID. This type has no meaning for V2-V5 -// UUIDs since they don't have an embedded timestamp. +// 15 October 1582 within a V1 UUID. This type has no meaning for other +// UUID versions since they don't have an embedded timestamp. type Timestamp uint64 const _100nsPerSecond = 10000000