Work around golang/go#11833 predictable random issue in Id. (#447)

* Work around golang/go#11833 predictable random issue.

In certain circumstances crypto/rand.Reader will return non-random
bytes. The most likely case is near boot, and as init is run when
the go program is started, it's possible that a non-random seed
could be used. While this is very unlikely to ever be an issue,
it is a very easy fix and it is preferable to be resilient.

Instead of seeding the global math/rand rng during init, a separate
math/rand.Rand is seeded upon the first call to Id. This also avoids
polluting the global math/rand rng which might be seeded elsewhere.

If crypto/rand.Reader fails, math/rand.Int63 will be called to
provide a seed. This is better than the current fallback to a seed
of 1.

This change introduces no noticeable performance overhead as the
global math/rand rng already uses a sync.Mutex internally.

* Document lack of performance overhead from mutex in `func id()`

msg.go

@@ -16,22 +16,9 @@ import (
 	"math/big"
 	"math/rand"
 	"strconv"
+	"sync"
 )
 
-func init() {
-	// Initialize default math/rand source using crypto/rand to provide better
-	// security without the performance trade-off.
-	buf := make([]byte, 8)
-	_, err := crand.Read(buf)
-	if err != nil {
-		// Failed to read from cryptographic source, fallback to default initial
-		// seed (1) by returning early
-		return
-	}
-	seed := binary.BigEndian.Uint64(buf)
-	rand.Seed(int64(seed))
-}
-
 const maxCompressionOffset = 2 << 13 // We have 14 bits for the compression pointer
 
 var (
@@ -66,11 +53,45 @@ var (
 //	dns.Id = func() uint16 { return 3 }
 var Id func() uint16 = id
 
+var (
+	idLock sync.Mutex
+	idRand *rand.Rand
+)
+
 // id returns a 16 bits random number to be used as a
 // message id. The random provided should be good enough.
 func id() uint16 {
-	id32 := rand.Uint32()
-	return uint16(id32)
+	idLock.Lock()
+
+	if idRand == nil {
+		// This (partially) works around
+		// https://github.com/golang/go/issues/11833 by only
+		// seeding idRand upon the first call to id.
+
+		var seed int64
+		var buf [8]byte
+
+		if _, err := crand.Read(buf[:]); err == nil {
+			seed = int64(binary.LittleEndian.Uint64(buf[:]))
+		} else {
+			seed = rand.Int63()
+		}
+
+		idRand = rand.New(rand.NewSource(seed))
+	}
+
+	// The call to idRand.Uint32 must be within the
+	// mutex lock because *rand.Rand is not safe for
+	// concurrent use.
+	//
+	// There is no added performance overhead to calling
+	// idRand.Uint32 inside a mutex lock over just
+	// calling rand.Uint32 as the global math/rand rng
+	// is internally protected by a sync.Mutex.
+	id := uint16(idRand.Uint32())
+
+	idLock.Unlock()
+	return id
 }
 
 // MsgHdr is a a manually-unpacked version of (id, bits).