From 8ebf2e419df7857ac8919baa05248789a8ffbf33 Mon Sep 17 00:00:00 2001
From: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Date: Tue, 10 Dec 2019 23:31:09 -0800
Subject: [PATCH] Use crypto/rand for random id generation. (#1044)

* Use crypto/rand for random id generation.

Fixes #1043 and #1037

* Panic on rare crypto/rand error.

* Fixes in response to review.
---
 msg.go | 52 ++++++++++------------------------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/msg.go b/msg.go
index e04fb5d77..293813005 100644
--- a/msg.go
+++ b/msg.go
@@ -11,14 +11,12 @@ package dns
 //go:generate go run msg_generate.go
 
 import (
-	crand "crypto/rand"
+	"crypto/rand"
 	"encoding/binary"
 	"fmt"
 	"math/big"
-	"math/rand"
 	"strconv"
 	"strings"
-	"sync"
 )
 
 const (
@@ -73,53 +71,23 @@ var (
 	ErrTime          error = &Error{err: "bad time"}      // ErrTime indicates a timing error in TSIG authentication.
 )
 
-// Id by default, returns a 16 bits random number to be used as a
-// message id. The random provided should be good enough. This being a
-// variable the function can be reassigned to a custom function.
-// For instance, to make it return a static value:
+// Id by default returns a 16-bit random number to be used as a message id. The
+// number is drawn from a cryptographically secure random number generator.
+// This being a variable the function can be reassigned to a custom function.
+// For instance, to make it return a static value for testing:
 //
 //	dns.Id = func() uint16 { return 3 }
 var Id = 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 {
-	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))
+	var output uint16
+	err := binary.Read(rand.Reader, binary.BigEndian, &output)
+	if err != nil {
+		panic("dns: reading random id failed: " + err.Error())
 	}
-
-	// 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
+	return output
 }
 
 // MsgHdr is a a manually-unpacked version of (id, bits).