util/deephash: make hash type opaque

The fact that Hash returns a [sha256.Size]byte leaks details about
the underlying hash implementation. This could very well be any other
hashing algorithm with a possible different block size.

Abstract this implementation detail away by declaring an opaque type
that is comparable. While we are changing the signature of UpdateHash,
rename it to just Update to reduce stutter (e.g., deephash.Update).

Signed-off-by: Joe Tsai <joetsai@digital-static.net>

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
 	}