ipn/ipnlocal: make the redactNetmapPrivateKeys test recursive

Expand TestRedactNetmapPrivateKeys to cover all sub-structs of
NetworkMap and confirm that a) all fields are annotated as private or
public, and b) all private fields are getting redacted.

Updates tailscale/corp#32095

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

ipn/ipnlocal/c2n_test.go

@@ -8,24 +8,26 @@ import (
 	"cmp"
 	"crypto/x509"
 	"encoding/json"
-	"fmt"
 	"net/http/httptest"
 	"net/url"
 	"os"
 	"path/filepath"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
 	"tailscale.com/ipn/store/mem"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
+	"tailscale.com/types/ipproto"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
-	"tailscale.com/types/opt"
 	"tailscale.com/types/views"
 	"tailscale.com/util/must"
+	"tailscale.com/util/set"
+	"tailscale.com/wgengine/filter/filtertype"
 
 	gcmp "github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
@@ -142,88 +144,335 @@ func TestHandleC2NTLSCertStatus(t *testing.T) {
 
 }
 
-// reflectNonzero returns a non-zero value for a given reflect.Value.
+// eachStructField calls cb for each struct field in struct type tp, recursively.
-func reflectNonzero(t reflect.Type) reflect.Value {
+func eachStructField(tp reflect.Type, cb func(reflect.Type, reflect.StructField)) {
-	switch t.Kind() {
+	if !strings.HasPrefix(tp.PkgPath(), "tailscale.com/") {
-	case reflect.Bool:
+		// Stop traversing when we reach a non-tailscale type.
-		return reflect.ValueOf(true)
+		return
-	case reflect.String:
-		if reflect.TypeFor[opt.Bool]() == t {
-			return reflect.ValueOf("true").Convert(t)
 	}
-		return reflect.ValueOf("foo").Convert(t)
+
-	case reflect.Int64:
+	for i := range tp.NumField() {
-		return reflect.ValueOf(int64(1)).Convert(t)
+		cb(tp, tp.Field(i))
-	case reflect.Slice:
+
-		return reflect.MakeSlice(t, 1, 1)
+		switch tp.Field(i).Type.Kind() {
-	case reflect.Ptr:
-		return reflect.New(t.Elem())
-	case reflect.Map:
-		return reflect.MakeMap(t)
 		case reflect.Struct:
-		switch t {
+			eachStructField(tp.Field(i).Type, cb)
-		case reflect.TypeFor[key.NodePrivate]():
+		case reflect.Slice, reflect.Array, reflect.Ptr, reflect.Map:
-			return reflect.ValueOf(key.NewNode())
+			if tp.Field(i).Type.Elem().Kind() == reflect.Struct {
+				eachStructField(tp.Field(i).Type.Elem(), cb)
+			}
 		}
 	}
-	panic(fmt.Sprintf("unhandled %v", t))
 }
 
-// setFieldsToRedact sets fields in the given netmap to non-zero values
+// eachStructValue calls cb for each struct field in the struct value v, recursively.
-// according to the fieldMap, which maps field names to whether they
+func eachStructValue(v reflect.Value, cb func(reflect.Type, reflect.StructField, reflect.Value)) {
-// should be reset (true) or not (false).
+	if v.IsZero() {
-func setFieldsToRedact(t *testing.T, nm *netmap.NetworkMap, fieldMap map[string]bool) {
+		return
-	t.Helper()
+	}
-	v := reflect.ValueOf(nm).Elem()
+
 	for i := range v.NumField() {
-		name := v.Type().Field(i).Name
+		cb(v.Type(), v.Type().Field(i), v.Field(i))
-		f := v.Field(i)
+
-		if !f.CanSet() {
+		switch v.Type().Field(i).Type.Kind() {
-			continue
+		case reflect.Struct:
+			eachStructValue(v.Field(i), cb)
+		case reflect.Slice, reflect.Array, reflect.Ptr, reflect.Map:
+			if v.Field(i).Type().Elem().Kind() == reflect.Struct {
+				eachStructValue(v.Field(i).Addr().Elem(), cb)
 			}
-		shouldReset, ok := fieldMap[name]
-		if !ok {
-			t.Errorf("fieldMap missing field %q", name)
-		}
-		if shouldReset {
-			f.Set(reflectNonzero(f.Type()))
 		}
 	}
 }
 
+// TestRedactNetmapPrivateKeys tests that redactNetmapPrivateKeys redacts all private keys
+// and other private fields from a netmap.NetworkMap, and only those fields.
 func TestRedactNetmapPrivateKeys(t *testing.T) {
-	fieldMap := map[string]bool{
+	type field struct {
-		// Private fields (should be redacted):
+		t reflect.Type
-		"PrivateKey": true,
+		f string
+	}
+	f := func(t any, f string) field {
+		return field{reflect.TypeOf(t), f}
+	}
+	// fields is a map of all struct fields in netmap.NetworkMap and its
+	// sub-structs, marking each field as private (true) or public (false).
+	// If you add a new field to netmap.NetworkMap or its sub-structs,
+	// you must add it to this list, marking it as private or public.
+	fields := map[field]bool{
+		// Private fields to be redacted.
+		f(netmap.NetworkMap{}, "PrivateKey"): true,
 
-		// Public fields (should not be redacted):
+		// All other fields are public.
-		"AllCaps":           false,
+		f(netmap.NetworkMap{}, "AllCaps"):                  false,
-		"CollectServices":   false,
+		f(netmap.NetworkMap{}, "CollectServices"):          false,
-		"DERPMap":           false,
+		f(netmap.NetworkMap{}, "DERPMap"):                  false,
-		"DNS":               false,
+		f(netmap.NetworkMap{}, "DNS"):                      false,
-		"DisplayMessages":   false,
+		f(netmap.NetworkMap{}, "DisplayMessages"):          false,
-		"Domain":            false,
+		f(netmap.NetworkMap{}, "Domain"):                   false,
-		"DomainAuditLogID":  false,
+		f(netmap.NetworkMap{}, "DomainAuditLogID"):         false,
-		"Expiry":            false,
+		f(netmap.NetworkMap{}, "Expiry"):                   false,
-		"MachineKey":        false,
+		f(netmap.NetworkMap{}, "MachineKey"):               false,
-		"Name":              false,
+		f(netmap.NetworkMap{}, "Name"):                     false,
-		"NodeKey":           false,
+		f(netmap.NetworkMap{}, "NodeKey"):                  false,
-		"PacketFilter":      false,
+		f(netmap.NetworkMap{}, "PacketFilter"):             false,
-		"PacketFilterRules": false,
+		f(netmap.NetworkMap{}, "PacketFilterRules"):        false,
-		"Peers":             false,
+		f(netmap.NetworkMap{}, "Peers"):                    false,
-		"SSHPolicy":         false,
+		f(netmap.NetworkMap{}, "SSHPolicy"):                false,
-		"SelfNode":          false,
+		f(netmap.NetworkMap{}, "SelfNode"):                 false,
-		"TKAEnabled":        false,
+		f(netmap.NetworkMap{}, "TKAEnabled"):               false,
-		"TKAHead":           false,
+		f(netmap.NetworkMap{}, "TKAHead"):                  false,
-		"UserProfiles":      false,
+		f(netmap.NetworkMap{}, "UserProfiles"):             false,
+		f(filtertype.CapMatch{}, "Cap"):                    false,
+		f(filtertype.CapMatch{}, "Dst"):                    false,
+		f(filtertype.CapMatch{}, "Values"):                 false,
+		f(filtertype.Match{}, "Caps"):                      false,
+		f(filtertype.Match{}, "Dsts"):                      false,
+		f(filtertype.Match{}, "IPProto"):                   false,
+		f(filtertype.Match{}, "SrcCaps"):                   false,
+		f(filtertype.Match{}, "Srcs"):                      false,
+		f(filtertype.Match{}, "SrcsContains"):              false,
+		f(filtertype.NetPortRange{}, "Net"):                false,
+		f(filtertype.NetPortRange{}, "Ports"):              false,
+		f(filtertype.PortRange{}, "First"):                 false,
+		f(filtertype.PortRange{}, "Last"):                  false,
+		f(key.DiscoPublic{}, "k"):                          false,
+		f(key.MachinePublic{}, "k"):                        false,
+		f(key.NodePrivate{}, "_"):                          false,
+		f(key.NodePrivate{}, "k"):                          false,
+		f(key.NodePublic{}, "k"):                           false,
+		f(tailcfg.CapGrant{}, "CapMap"):                    false,
+		f(tailcfg.CapGrant{}, "Caps"):                      false,
+		f(tailcfg.CapGrant{}, "Dsts"):                      false,
+		f(tailcfg.DERPHomeParams{}, "RegionScore"):         false,
+		f(tailcfg.DERPMap{}, "HomeParams"):                 false,
+		f(tailcfg.DERPMap{}, "OmitDefaultRegions"):         false,
+		f(tailcfg.DERPMap{}, "Regions"):                    false,
+		f(tailcfg.DNSConfig{}, "CertDomains"):              false,
+		f(tailcfg.DNSConfig{}, "Domains"):                  false,
+		f(tailcfg.DNSConfig{}, "ExitNodeFilteredSet"):      false,
+		f(tailcfg.DNSConfig{}, "ExtraRecords"):             false,
+		f(tailcfg.DNSConfig{}, "FallbackResolvers"):        false,
+		f(tailcfg.DNSConfig{}, "Nameservers"):              false,
+		f(tailcfg.DNSConfig{}, "Proxied"):                  false,
+		f(tailcfg.DNSConfig{}, "Resolvers"):                false,
+		f(tailcfg.DNSConfig{}, "Routes"):                   false,
+		f(tailcfg.DNSConfig{}, "TempCorpIssue13969"):       false,
+		f(tailcfg.DNSRecord{}, "Name"):                     false,
+		f(tailcfg.DNSRecord{}, "Type"):                     false,
+		f(tailcfg.DNSRecord{}, "Value"):                    false,
+		f(tailcfg.DisplayMessageAction{}, "Label"):         false,
+		f(tailcfg.DisplayMessageAction{}, "URL"):           false,
+		f(tailcfg.DisplayMessage{}, "ImpactsConnectivity"): false,
+		f(tailcfg.DisplayMessage{}, "PrimaryAction"):       false,
+		f(tailcfg.DisplayMessage{}, "Severity"):            false,
+		f(tailcfg.DisplayMessage{}, "Text"):                false,
+		f(tailcfg.DisplayMessage{}, "Title"):               false,
+		f(tailcfg.FilterRule{}, "CapGrant"):                false,
+		f(tailcfg.FilterRule{}, "DstPorts"):                false,
+		f(tailcfg.FilterRule{}, "IPProto"):                 false,
+		f(tailcfg.FilterRule{}, "SrcBits"):                 false,
+		f(tailcfg.FilterRule{}, "SrcIPs"):                  false,
+		f(tailcfg.HostinfoView{}, "ж"):                     false,
+		f(tailcfg.Hostinfo{}, "AllowsUpdate"):              false,
+		f(tailcfg.Hostinfo{}, "App"):                       false,
+		f(tailcfg.Hostinfo{}, "AppConnector"):              false,
+		f(tailcfg.Hostinfo{}, "BackendLogID"):              false,
+		f(tailcfg.Hostinfo{}, "Cloud"):                     false,
+		f(tailcfg.Hostinfo{}, "Container"):                 false,
+		f(tailcfg.Hostinfo{}, "Desktop"):                   false,
+		f(tailcfg.Hostinfo{}, "DeviceModel"):               false,
+		f(tailcfg.Hostinfo{}, "Distro"):                    false,
+		f(tailcfg.Hostinfo{}, "DistroCodeName"):            false,
+		f(tailcfg.Hostinfo{}, "DistroVersion"):             false,
+		f(tailcfg.Hostinfo{}, "Env"):                       false,
+		f(tailcfg.Hostinfo{}, "ExitNodeID"):                false,
+		f(tailcfg.Hostinfo{}, "FrontendLogID"):             false,
+		f(tailcfg.Hostinfo{}, "GoArch"):                    false,
+		f(tailcfg.Hostinfo{}, "GoArchVar"):                 false,
+		f(tailcfg.Hostinfo{}, "GoVersion"):                 false,
+		f(tailcfg.Hostinfo{}, "Hostname"):                  false,
+		f(tailcfg.Hostinfo{}, "IPNVersion"):                false,
+		f(tailcfg.Hostinfo{}, "IngressEnabled"):            false,
+		f(tailcfg.Hostinfo{}, "Location"):                  false,
+		f(tailcfg.Hostinfo{}, "Machine"):                   false,
+		f(tailcfg.Hostinfo{}, "NetInfo"):                   false,
+		f(tailcfg.Hostinfo{}, "NoLogsNoSupport"):           false,
+		f(tailcfg.Hostinfo{}, "OS"):                        false,
+		f(tailcfg.Hostinfo{}, "OSVersion"):                 false,
+		f(tailcfg.Hostinfo{}, "Package"):                   false,
+		f(tailcfg.Hostinfo{}, "PushDeviceToken"):           false,
+		f(tailcfg.Hostinfo{}, "RequestTags"):               false,
+		f(tailcfg.Hostinfo{}, "RoutableIPs"):               false,
+		f(tailcfg.Hostinfo{}, "SSH_HostKeys"):              false,
+		f(tailcfg.Hostinfo{}, "Services"):                  false,
+		f(tailcfg.Hostinfo{}, "ServicesHash"):              false,
+		f(tailcfg.Hostinfo{}, "ShareeNode"):                false,
+		f(tailcfg.Hostinfo{}, "ShieldsUp"):                 false,
+		f(tailcfg.Hostinfo{}, "StateEncrypted"):            false,
+		f(tailcfg.Hostinfo{}, "TPM"):                       false,
+		f(tailcfg.Hostinfo{}, "Userspace"):                 false,
+		f(tailcfg.Hostinfo{}, "UserspaceRouter"):           false,
+		f(tailcfg.Hostinfo{}, "WireIngress"):               false,
+		f(tailcfg.Hostinfo{}, "WoLMACs"):                   false,
+		f(tailcfg.Location{}, "City"):                      false,
+		f(tailcfg.Location{}, "CityCode"):                  false,
+		f(tailcfg.Location{}, "Country"):                   false,
+		f(tailcfg.Location{}, "CountryCode"):               false,
+		f(tailcfg.Location{}, "Latitude"):                  false,
+		f(tailcfg.Location{}, "Longitude"):                 false,
+		f(tailcfg.Location{}, "Priority"):                  false,
+		f(tailcfg.NetInfo{}, "DERPLatency"):                false,
+		f(tailcfg.NetInfo{}, "FirewallMode"):               false,
+		f(tailcfg.NetInfo{}, "HairPinning"):                false,
+		f(tailcfg.NetInfo{}, "HavePortMap"):                false,
+		f(tailcfg.NetInfo{}, "LinkType"):                   false,
+		f(tailcfg.NetInfo{}, "MappingVariesByDestIP"):      false,
+		f(tailcfg.NetInfo{}, "OSHasIPv6"):                  false,
+		f(tailcfg.NetInfo{}, "PCP"):                        false,
+		f(tailcfg.NetInfo{}, "PMP"):                        false,
+		f(tailcfg.NetInfo{}, "PreferredDERP"):              false,
+		f(tailcfg.NetInfo{}, "UPnP"):                       false,
+		f(tailcfg.NetInfo{}, "WorkingICMPv4"):              false,
+		f(tailcfg.NetInfo{}, "WorkingIPv6"):                false,
+		f(tailcfg.NetInfo{}, "WorkingUDP"):                 false,
+		f(tailcfg.NetPortRange{}, "Bits"):                  false,
+		f(tailcfg.NetPortRange{}, "IP"):                    false,
+		f(tailcfg.NetPortRange{}, "Ports"):                 false,
+		f(tailcfg.NetPortRange{}, "_"):                     false,
+		f(tailcfg.NodeView{}, "ж"):                         false,
+		f(tailcfg.Node{}, "Addresses"):                     false,
+		f(tailcfg.Node{}, "AllowedIPs"):                    false,
+		f(tailcfg.Node{}, "Cap"):                           false,
+		f(tailcfg.Node{}, "CapMap"):                        false,
+		f(tailcfg.Node{}, "Capabilities"):                  false,
+		f(tailcfg.Node{}, "ComputedName"):                  false,
+		f(tailcfg.Node{}, "ComputedNameWithHost"):          false,
+		f(tailcfg.Node{}, "Created"):                       false,
+		f(tailcfg.Node{}, "DataPlaneAuditLogID"):           false,
+		f(tailcfg.Node{}, "DiscoKey"):                      false,
+		f(tailcfg.Node{}, "Endpoints"):                     false,
+		f(tailcfg.Node{}, "ExitNodeDNSResolvers"):          false,
+		f(tailcfg.Node{}, "Expired"):                       false,
+		f(tailcfg.Node{}, "HomeDERP"):                      false,
+		f(tailcfg.Node{}, "Hostinfo"):                      false,
+		f(tailcfg.Node{}, "ID"):                            false,
+		f(tailcfg.Node{}, "IsJailed"):                      false,
+		f(tailcfg.Node{}, "IsWireGuardOnly"):               false,
+		f(tailcfg.Node{}, "Key"):                           false,
+		f(tailcfg.Node{}, "KeyExpiry"):                     false,
+		f(tailcfg.Node{}, "KeySignature"):                  false,
+		f(tailcfg.Node{}, "LastSeen"):                      false,
+		f(tailcfg.Node{}, "LegacyDERPString"):              false,
+		f(tailcfg.Node{}, "Machine"):                       false,
+		f(tailcfg.Node{}, "MachineAuthorized"):             false,
+		f(tailcfg.Node{}, "Name"):                          false,
+		f(tailcfg.Node{}, "Online"):                        false,
+		f(tailcfg.Node{}, "PrimaryRoutes"):                 false,
+		f(tailcfg.Node{}, "SelfNodeV4MasqAddrForThisPeer"): false,
+		f(tailcfg.Node{}, "SelfNodeV6MasqAddrForThisPeer"): false,
+		f(tailcfg.Node{}, "Sharer"):                        false,
+		f(tailcfg.Node{}, "StableID"):                      false,
+		f(tailcfg.Node{}, "Tags"):                          false,
+		f(tailcfg.Node{}, "UnsignedPeerAPIOnly"):           false,
+		f(tailcfg.Node{}, "User"):                          false,
+		f(tailcfg.Node{}, "computedHostIfDifferent"):       false,
+		f(tailcfg.PortRange{}, "First"):                    false,
+		f(tailcfg.PortRange{}, "Last"):                     false,
+		f(tailcfg.SSHPolicy{}, "Rules"):                    false,
+		f(tailcfg.Service{}, "Description"):                false,
+		f(tailcfg.Service{}, "Port"):                       false,
+		f(tailcfg.Service{}, "Proto"):                      false,
+		f(tailcfg.Service{}, "_"):                          false,
+		f(tailcfg.TPMInfo{}, "FirmwareVersion"):            false,
+		f(tailcfg.TPMInfo{}, "Manufacturer"):               false,
+		f(tailcfg.TPMInfo{}, "Model"):                      false,
+		f(tailcfg.TPMInfo{}, "SpecRevision"):               false,
+		f(tailcfg.TPMInfo{}, "Vendor"):                     false,
+		f(tailcfg.UserProfileView{}, "ж"):                  false,
+		f(tailcfg.UserProfile{}, "DisplayName"):            false,
+		f(tailcfg.UserProfile{}, "ID"):                     false,
+		f(tailcfg.UserProfile{}, "LoginName"):              false,
+		f(tailcfg.UserProfile{}, "ProfilePicURL"):          false,
+		f(views.Slice[ipproto.Proto]{}, "ж"):               false,
+		f(views.Slice[tailcfg.FilterRule]{}, "ж"):          false,
 	}
 
-	nm := &netmap.NetworkMap{}
+	t.Run("field_list_is_complete", func(t *testing.T) {
-	setFieldsToRedact(t, nm, fieldMap)
+		seen := set.Set[field]{}
+		eachStructField(reflect.TypeOf(netmap.NetworkMap{}), func(rt reflect.Type, sf reflect.StructField) {
+			f := field{rt, sf.Name}
+			seen.Add(f)
+			if _, ok := fields[f]; !ok {
+				// Fail the test if netmap has a field not in the list. If you see this test
+				// failure, please add the new field to the fields map above, marking it as private or public.
+				t.Errorf("netmap field has not been declared as private or public: %v.%v", rt, sf.Name)
+			}
+		})
 
-	got, _ := redactNetmapPrivateKeys(nm)
+		for want := range fields {
-	if !reflect.DeepEqual(got, &netmap.NetworkMap{}) {
+			if !seen.Contains(want) {
-		t.Errorf("redacted netmap is not empty: %+v", got)
+				// Fail the test if the list has a field not in netmap. If you see this test
+				// failure, please remove the field from the fields map above.
+				t.Errorf("field declared that has not been found in netmap: %v.%v", want.t, want.f)
+			}
+		}
+	})
+
+	// tests is a list of test cases, each with a non-redacted netmap and the expected redacted netmap.
+	// If you add a new private field to netmap.NetworkMap or its sub-structs, please add a test case
+	// here that has that field set in nm, and the expected redacted value in wantRedacted.
+	tests := []struct {
+		name         string
+		nm           *netmap.NetworkMap
+		wantRedacted *netmap.NetworkMap
+	}{
+		{
+			name: "redact_private_key",
+			nm: &netmap.NetworkMap{
+				PrivateKey: key.NewNode(),
+			},
+			wantRedacted: &netmap.NetworkMap{},
+		},
+	}
+
+	// confirmedRedacted is a set of all private fields that have been covered by the tests above.
+	confirmedRedacted := set.Set[field]{}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			// Record which of the private fields are set in the non-redacted netmap.
+			eachStructValue(reflect.ValueOf(tt.nm).Elem(), func(tt reflect.Type, sf reflect.StructField, v reflect.Value) {
+				f := field{tt, sf.Name}
+				if shouldRedact := fields[f]; shouldRedact && !v.IsZero() {
+					confirmedRedacted.Add(f)
+				}
+			})
+
+			got, _ := redactNetmapPrivateKeys(tt.nm)
+			if !reflect.DeepEqual(got, tt.wantRedacted) {
+				t.Errorf("unexpected redacted netmap: %+v", got)
+			}
+
+			// Check that all private fields in the redacted netmap are zero.
+			eachStructValue(reflect.ValueOf(got).Elem(), func(tt reflect.Type, sf reflect.StructField, v reflect.Value) {
+				f := field{tt, sf.Name}
+				if shouldRedact := fields[f]; shouldRedact && !v.IsZero() {
+					t.Errorf("field not redacted: %v.%v", tt, sf.Name)
+				}
+			})
+		})
+	}
+
+	// Check that all private fields in netmap.NetworkMap and its sub-structs
+	// are covered by the tests above. If you see a test failure here,
+	// please add a test case above that has that field set in nm.
+	for f, shouldRedact := range fields {
+		if shouldRedact {
+			if !confirmedRedacted.Contains(f) {
+				t.Errorf("field not covered by tests: %v.%v", f.t, f.f)
+			}
+		}
 	}
 }