Merge pull request #2464 from tailscale/dsnet/opaque-hash

util/deephash: make hash type opaque

ipn/ipnlocal/local.go

@@ -95,7 +95,7 @@ type LocalBackend struct {
 	serverURL             string           // tailcontrol URL
 	newDecompressor       func() (controlclient.Decompressor, error)
 
-	filterHash string
+	filterHash deephash.Sum
 
 	// The mutex protects the following elements.
 	mu             sync.Mutex
@@ -944,7 +944,7 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs)
 	localNets, _ := localNetsB.IPSet()
 	logNets, _ := logNetsB.IPSet()
 
-	changed := deephash.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
+	changed := deephash.Update(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
 	if !changed {
 		return
 	}

util/deephash/deephash.go

@@ -53,8 +53,17 @@ func (h *hasher) setBufioWriter(w *bufio.Writer) (old *bufio.Writer) {
 	return old
 }
 
-// Hash returns the raw SHA-256 (not hex) of v.
+// Sum is an opaque checksum type that is comparable.
-func (h *hasher) Hash(v interface{}) (hash [sha256.Size]byte) {
+type Sum struct {
+	sum [sha256.Size]byte
+}
+
+func (s Sum) String() string {
+	return hex.EncodeToString(s.sum[:])
+}
+
+// Hash returns the hash of v.
+func (h *hasher) Hash(v interface{}) (hash Sum) {
 	h.bw.Flush()
 	h.h.Reset()
 	h.print(reflect.ValueOf(v))
@@ -64,7 +73,7 @@ func (h *hasher) Hash(v interface{}) (hash [sha256.Size]byte) {
 	// concrete type exported and we don't want the 'hash' result
 	// parameter to escape to the heap:
 	h.h.Sum(h.scratch[:0])
-	copy(hash[:], h.scratch[:])
+	copy(hash.sum[:], h.scratch[:])
 	return
 }
 
@@ -72,8 +81,8 @@ var hasherPool = &sync.Pool{
 	New: func() interface{} { return newHasher() },
 }
 
-// Hash returns the raw SHA-256 hash of v.
+// Hash returns the hash of v.
-func Hash(v interface{}) [sha256.Size]byte {
+func Hash(v interface{}) Sum {
 	h := hasherPool.Get().(*hasher)
 	defer hasherPool.Put(h)
 	for k := range h.visited {
@@ -82,30 +91,14 @@ func Hash(v interface{}) [sha256.Size]byte {
 	return h.Hash(v)
 }
 
-// UpdateHash sets last to the hex-encoded hash of v and reports whether its value changed.
+// Update sets last to the hash of v and reports whether its value changed.
-func UpdateHash(last *string, v ...interface{}) (changed bool) {
+func Update(last *Sum, v ...interface{}) (changed bool) {
 	sum := Hash(v)
-	if sha256EqualHex(sum, *last) {
+	if sum == *last {
 		// unchanged.
 		return false
 	}
-	*last = hex.EncodeToString(sum[:])
+	*last = sum
-	return true
-}
-
-// sha256EqualHex reports whether hx is the hex encoding of sum.
-func sha256EqualHex(sum [sha256.Size]byte, hx string) bool {
-	if len(hx) != len(sum)*2 {
-		return false
-	}
-	const hextable = "0123456789abcdef"
-	j := 0
-	for _, v := range sum {
-		if hx[j] != hextable[v>>4] || hx[j+1] != hextable[v&0x0f] {
-			return false
-		}
-		j += 2
-	}
 	return true
 }
 

util/deephash/deephash_test.go

@@ -7,8 +7,6 @@ package deephash
 import (
 	"bufio"
 	"bytes"
-	"crypto/sha256"
-	"encoding/hex"
 	"fmt"
 	"reflect"
 	"testing"
@@ -233,7 +231,7 @@ func BenchmarkTailcfgNode(b *testing.B) {
 }
 
 func TestExhaustive(t *testing.T) {
-	seen := make(map[[sha256.Size]byte]bool)
+	seen := make(map[Sum]bool)
 	for i := 0; i < 100000; i++ {
 		s := Hash(i)
 		if seen[s] {
@@ -243,19 +241,6 @@ func TestExhaustive(t *testing.T) {
 	}
 }
 
-func TestSHA256EqualHex(t *testing.T) {
-	for i := 0; i < 1000; i++ {
-		sum := Hash(i)
-		hx := hex.EncodeToString(sum[:])
-		if !sha256EqualHex(sum, hx) {
-			t.Fatal("didn't match, should've")
-		}
-		if sha256EqualHex(sum, hx[:len(hx)-1]) {
-			t.Fatal("matched on wrong length")
-		}
-	}
-}
-
 // verify this doesn't loop forever, as it used to (Issue 2340)
 func TestMapCyclicFallback(t *testing.T) {
 	type T struct {

wgengine/userspace.go

@@ -107,9 +107,9 @@ type userspaceEngine struct {
 
 	wgLock              sync.Mutex // serializes all wgdev operations; see lock order comment below
 	lastCfgFull         wgcfg.Config
-	lastRouterSig       string // of router.Config
+	lastRouterSig       deephash.Sum // of router.Config
-	lastEngineSigFull   string // of full wireguard config
+	lastEngineSigFull   deephash.Sum // of full wireguard config
-	lastEngineSigTrim   string // of trimmed wireguard config
+	lastEngineSigTrim   deephash.Sum // of trimmed wireguard config
 	recvActivityAt      map[tailcfg.DiscoKey]time.Time
 	trimmedDisco        map[tailcfg.DiscoKey]bool // set of disco keys of peers currently excluded from wireguard config
 	sentActivityAt      map[netaddr.IP]*int64     // value is atomic int64 of unixtime
@@ -641,7 +641,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 		}
 	}
 
-	if !deephash.UpdateHash(&e.lastEngineSigTrim, &min, trimmedDisco, trackDisco, trackIPs) {
+	if !deephash.Update(&e.lastEngineSigTrim, &min, trimmedDisco, trackDisco, trackIPs) {
 		// No changes
 		return nil
 	}
@@ -767,8 +767,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 		listenPort = 0
 	}
 
-	engineChanged := deephash.UpdateHash(&e.lastEngineSigFull, cfg)
+	engineChanged := deephash.Update(&e.lastEngineSigFull, cfg)
-	routerChanged := deephash.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
+	routerChanged := deephash.Update(&e.lastRouterSig, routerCfg, dnsCfg)
 	if !engineChanged && !routerChanged && listenPort == e.magicConn.LocalPort() {
 		return ErrNoChanges
 	}