diff --git a/client/systray/systray.go b/client/systray/systray.go index 518b2e989..bc099a1ec 100644 --- a/client/systray/systray.go +++ b/client/systray/systray.go @@ -512,7 +512,7 @@ func (menu *Menu) watchIPNBus() { } func (menu *Menu) watchIPNBusInner() error { - watcher, err := menu.lc.WatchIPNBus(menu.bgCtx, ipn.NotifyNoPrivateKeys) + watcher, err := menu.lc.WatchIPNBus(menu.bgCtx, 0) if err != nil { return fmt.Errorf("watching ipn bus: %w", err) } diff --git a/cmd/sniproxy/sniproxy.go b/cmd/sniproxy/sniproxy.go index c020b4a1f..2115c8095 100644 --- a/cmd/sniproxy/sniproxy.go +++ b/cmd/sniproxy/sniproxy.go @@ -141,7 +141,7 @@ func run(ctx context.Context, ts *tsnet.Server, wgPort int, hostname string, pro // in the netmap. // We set the NotifyInitialNetMap flag so we will always get woken with the // current netmap, before only being woken on changes. - bus, err := lc.WatchIPNBus(ctx, ipn.NotifyWatchEngineUpdates|ipn.NotifyInitialNetMap|ipn.NotifyNoPrivateKeys) + bus, err := lc.WatchIPNBus(ctx, ipn.NotifyWatchEngineUpdates|ipn.NotifyInitialNetMap) if err != nil { log.Fatalf("watching IPN bus: %v", err) } diff --git a/cmd/stund/depaware.txt b/cmd/stund/depaware.txt index bd8eebb7b..7b3d05f94 100644 --- a/cmd/stund/depaware.txt +++ b/cmd/stund/depaware.txt @@ -82,8 +82,9 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar tailscale.com/util/mak from tailscale.com/syncs+ tailscale.com/util/nocasemaps from tailscale.com/types/ipproto tailscale.com/util/rands from tailscale.com/tsweb + tailscale.com/util/set from tailscale.com/types/key tailscale.com/util/slicesx from tailscale.com/tailcfg - tailscale.com/util/testenv from tailscale.com/types/logger + tailscale.com/util/testenv from tailscale.com/types/logger+ tailscale.com/util/vizerror from tailscale.com/tailcfg+ tailscale.com/version from tailscale.com/envknob+ tailscale.com/version/distro from tailscale.com/envknob @@ -94,7 +95,7 @@ tailscale.com/cmd/stund dependencies: (generated by github.com/tailscale/depawar golang.org/x/crypto/nacl/box from tailscale.com/types/key golang.org/x/crypto/nacl/secretbox from golang.org/x/crypto/nacl/box golang.org/x/crypto/salsa20/salsa from golang.org/x/crypto/nacl/box+ - golang.org/x/exp/constraints from tailscale.com/tsweb/varz + golang.org/x/exp/constraints from tailscale.com/tsweb/varz+ golang.org/x/sys/cpu from golang.org/x/crypto/blake2b+ LD golang.org/x/sys/unix from github.com/prometheus/procfs+ W golang.org/x/sys/windows from github.com/prometheus/client_golang/prometheus diff --git a/cmd/tailscale/cli/debug.go b/cmd/tailscale/cli/debug.go index 2836ae298..ffed51a63 100644 --- a/cmd/tailscale/cli/debug.go +++ b/cmd/tailscale/cli/debug.go @@ -258,7 +258,6 @@ func debugCmd() *ffcli.Command { fs.BoolVar(&watchIPNArgs.netmap, "netmap", true, "include netmap in messages") fs.BoolVar(&watchIPNArgs.initial, "initial", false, "include initial status") fs.BoolVar(&watchIPNArgs.rateLimit, "rate-limit", true, "rate limit messags") - fs.BoolVar(&watchIPNArgs.showPrivateKey, "show-private-key", false, "include node private key in printed netmap") fs.IntVar(&watchIPNArgs.count, "count", 0, "exit after printing this many statuses, or 0 to keep going forever") return fs })(), @@ -270,7 +269,6 @@ func debugCmd() *ffcli.Command { ShortHelp: "Print the current network map", FlagSet: (func() *flag.FlagSet { fs := newFlagSet("netmap") - fs.BoolVar(&netmapArgs.showPrivateKey, "show-private-key", false, "include node private key in printed netmap") return fs })(), }, @@ -614,11 +612,10 @@ func runPrefs(ctx context.Context, args []string) error { } var watchIPNArgs struct { - netmap bool - initial bool - showPrivateKey bool - rateLimit bool - count int + netmap bool + initial bool + rateLimit bool + count int } func runWatchIPN(ctx context.Context, args []string) error { @@ -626,9 +623,6 @@ func runWatchIPN(ctx context.Context, args []string) error { if watchIPNArgs.initial { mask = ipn.NotifyInitialState | ipn.NotifyInitialPrefs | ipn.NotifyInitialNetMap } - if !watchIPNArgs.showPrivateKey { - mask |= ipn.NotifyNoPrivateKeys - } if watchIPNArgs.rateLimit { mask |= ipn.NotifyRateLimit } @@ -652,18 +646,11 @@ func runWatchIPN(ctx context.Context, args []string) error { return nil } -var netmapArgs struct { - showPrivateKey bool -} - func runNetmap(ctx context.Context, args []string) error { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() var mask ipn.NotifyWatchOpt = ipn.NotifyInitialNetMap - if !netmapArgs.showPrivateKey { - mask |= ipn.NotifyNoPrivateKeys - } watcher, err := localClient.WatchIPNBus(ctx, mask) if err != nil { return err diff --git a/cmd/tailscale/cli/serve_v2.go b/cmd/tailscale/cli/serve_v2.go index e194b1e10..1ce14cf09 100644 --- a/cmd/tailscale/cli/serve_v2.go +++ b/cmd/tailscale/cli/serve_v2.go @@ -475,7 +475,7 @@ func (e *serveEnv) runServeCombined(subcmd serveMode) execFunc { // if foreground mode, create a WatchIPNBus session // and use the nested config for all following operations // TODO(marwan-at-work): nested-config validations should happen here or previous to this point. - watcher, err = e.lc.WatchIPNBus(ctx, ipn.NotifyInitialState|ipn.NotifyNoPrivateKeys) + watcher, err = e.lc.WatchIPNBus(ctx, ipn.NotifyInitialState) if err != nil { return err } diff --git a/cmd/tsidp/tsidp.go b/cmd/tsidp/tsidp.go index c02b09745..7093ab9ee 100644 --- a/cmd/tsidp/tsidp.go +++ b/cmd/tsidp/tsidp.go @@ -287,7 +287,7 @@ func serveOnLocalTailscaled(ctx context.Context, lc *local.Client, st *ipnstate. // We watch the IPN bus just to get a session ID. The session expires // when we stop watching the bus, and that auto-deletes the foreground // serve/funnel configs we are creating below. - watcher, err := lc.WatchIPNBus(ctx, ipn.NotifyInitialState|ipn.NotifyNoPrivateKeys) + watcher, err := lc.WatchIPNBus(ctx, ipn.NotifyInitialState) if err != nil { return nil, nil, fmt.Errorf("could not set up ipn bus watcher: %v", err) } diff --git a/control/controlclient/map.go b/control/controlclient/map.go index eafdb2d56..a9db25517 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -867,7 +867,6 @@ func (ms *mapSession) netmap() *netmap.NetworkMap { nm := &netmap.NetworkMap{ NodeKey: ms.publicNodeKey, - PrivateKey: ms.privateNodeKey, MachineKey: ms.machinePubKey, Peers: peerViews, UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfileView), diff --git a/ipn/backend.go b/ipn/backend.go index 91cf81ca5..b4ba958c5 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -74,7 +74,7 @@ const ( NotifyInitialPrefs NotifyWatchOpt = 1 << 2 // if set, the first Notify message (sent immediately) will contain the current Prefs NotifyInitialNetMap NotifyWatchOpt = 1 << 3 // if set, the first Notify message (sent immediately) will contain the current NetMap - NotifyNoPrivateKeys NotifyWatchOpt = 1 << 4 // if set, private keys that would normally be sent in updates are zeroed out + NotifyNoPrivateKeys NotifyWatchOpt = 1 << 4 // (no-op) it used to redact private keys; now they always are and this does nothing NotifyInitialDriveShares NotifyWatchOpt = 1 << 5 // if set, the first Notify message (sent immediately) will contain the current Taildrive Shares NotifyInitialOutgoingFiles NotifyWatchOpt = 1 << 6 // if set, the first Notify message (sent immediately) will contain the current Taildrop OutgoingFiles diff --git a/ipn/ipnlocal/c2n.go b/ipn/ipnlocal/c2n.go index 0c228060f..b5e722b97 100644 --- a/ipn/ipnlocal/c2n.go +++ b/ipn/ipnlocal/c2n.go @@ -179,7 +179,6 @@ func handleC2NDebugNetMap(b *LocalBackend, w http.ResponseWriter, r *http.Reques } field.SetZero() } - nm, _ = redactNetmapPrivateKeys(nm) return json.Marshal(nm) } diff --git a/ipn/ipnlocal/c2n_test.go b/ipn/ipnlocal/c2n_test.go index 877d102d0..420633c87 100644 --- a/ipn/ipnlocal/c2n_test.go +++ b/ipn/ipnlocal/c2n_test.go @@ -13,21 +13,17 @@ import ( "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/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" @@ -144,338 +140,6 @@ func TestHandleC2NTLSCertStatus(t *testing.T) { } -// eachStructField calls cb for each struct field in struct type tp, recursively. -func eachStructField(tp reflect.Type, cb func(reflect.Type, reflect.StructField)) { - if !strings.HasPrefix(tp.PkgPath(), "tailscale.com/") { - // Stop traversing when we reach a non-tailscale type. - return - } - - for i := range tp.NumField() { - cb(tp, tp.Field(i)) - - switch tp.Field(i).Type.Kind() { - case reflect.Struct: - eachStructField(tp.Field(i).Type, cb) - case reflect.Slice, reflect.Array, reflect.Ptr, reflect.Map: - if tp.Field(i).Type.Elem().Kind() == reflect.Struct { - eachStructField(tp.Field(i).Type.Elem(), cb) - } - } - } -} - -// eachStructValue calls cb for each struct field in the struct value v, recursively. -func eachStructValue(v reflect.Value, cb func(reflect.Type, reflect.StructField, reflect.Value)) { - if v.IsZero() { - return - } - - for i := range v.NumField() { - cb(v.Type(), v.Type().Field(i), v.Field(i)) - - switch v.Type().Field(i).Type.Kind() { - 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) - } - } - } -} - -// 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) { - type field struct { - t reflect.Type - 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, - - // All other fields are public. - f(netmap.NetworkMap{}, "AllCaps"): false, - f(netmap.NetworkMap{}, "CollectServices"): false, - f(netmap.NetworkMap{}, "DERPMap"): false, - f(netmap.NetworkMap{}, "DNS"): false, - f(netmap.NetworkMap{}, "DisplayMessages"): false, - f(netmap.NetworkMap{}, "Domain"): false, - f(netmap.NetworkMap{}, "DomainAuditLogID"): false, - f(netmap.NetworkMap{}, "Expiry"): false, - f(netmap.NetworkMap{}, "MachineKey"): false, - f(netmap.NetworkMap{}, "Name"): false, - f(netmap.NetworkMap{}, "NodeKey"): false, - f(netmap.NetworkMap{}, "PacketFilter"): false, - f(netmap.NetworkMap{}, "PacketFilterRules"): false, - f(netmap.NetworkMap{}, "Peers"): false, - f(netmap.NetworkMap{}, "SSHPolicy"): false, - f(netmap.NetworkMap{}, "SelfNode"): false, - f(netmap.NetworkMap{}, "TKAEnabled"): false, - f(netmap.NetworkMap{}, "TKAHead"): 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{}, "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{}, "FamilyIndicator"): 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, - } - - t.Run("field_list_is_complete", func(t *testing.T) { - 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) - } - }) - - for want := range fields { - if !seen.Contains(want) { - // 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) - } - } - } -} - func TestHandleC2NDebugNetmap(t *testing.T) { nm := &netmap.NetworkMap{ Name: "myhost", @@ -495,10 +159,7 @@ func TestHandleC2NDebugNetmap(t *testing.T) { Hostinfo: (&tailcfg.Hostinfo{Hostname: "peer1"}).View(), }).View(), }, - PrivateKey: key.NewNode(), } - withoutPrivateKey := *nm - withoutPrivateKey.PrivateKey = key.NodePrivate{} for _, tt := range []struct { name string @@ -507,12 +168,12 @@ func TestHandleC2NDebugNetmap(t *testing.T) { }{ { name: "simple_get", - want: &withoutPrivateKey, + want: nm, }, { name: "post_no_omit", req: &tailcfg.C2NDebugNetmapRequest{}, - want: &withoutPrivateKey, + want: nm, }, { name: "post_omit_peers_and_name", @@ -524,7 +185,7 @@ func TestHandleC2NDebugNetmap(t *testing.T) { { name: "post_omit_nonexistent_field", req: &tailcfg.C2NDebugNetmapRequest{OmitFields: []string{"ThisFieldDoesNotExist"}}, - want: &withoutPrivateKey, + want: nm, }, } { t.Run(tt.name, func(t *testing.T) { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 41d110400..9de1f3d85 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3052,9 +3052,6 @@ func (b *LocalBackend) WatchNotifications(ctx context.Context, mask ipn.NotifyWa func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.Actor, mask ipn.NotifyWatchOpt, onWatchAdded func(), fn func(roNotify *ipn.Notify) (keepGoing bool)) { ch := make(chan *ipn.Notify, 128) sessionID := rands.HexString(16) - if mask&ipn.NotifyNoPrivateKeys != 0 { - fn = filterPrivateKeys(fn) - } if mask&ipn.NotifyHealthActions == 0 { // if UI does not support PrimaryAction in health warnings, append // action URLs to the warning text instead. @@ -3154,39 +3151,6 @@ func (b *LocalBackend) WatchNotificationsAs(ctx context.Context, actor ipnauth.A sender.Run(ctx, ch) } -// filterPrivateKeys returns an IPN listener func that wraps the supplied IPN -// listener and zeroes out the PrivateKey in the NetMap passed to the wrapped -// listener. -func filterPrivateKeys(fn func(roNotify *ipn.Notify) (keepGoing bool)) func(*ipn.Notify) bool { - return func(n *ipn.Notify) bool { - redacted, changed := redactNetmapPrivateKeys(n.NetMap) - if !changed { - return fn(n) - } - - // The netmap in n is shared across all watchers, so to mutate it for a - // single watcher we have to clone the notify and the netmap. We can - // make shallow clones, at least. - n2 := *n - n2.NetMap = redacted - return fn(&n2) - } -} - -// redactNetmapPrivateKeys returns a copy of nm with private keys zeroed out. -// If no change was needed, it returns nm unmodified. -func redactNetmapPrivateKeys(nm *netmap.NetworkMap) (redacted *netmap.NetworkMap, changed bool) { - if nm == nil || nm.PrivateKey.IsZero() { - return nm, false - } - - // The netmap might be shared across watchers, so make at least a shallow - // clone before mutating it. - nm2 := *nm - nm2.PrivateKey = key.NodePrivate{} - return &nm2, true -} - // appendHealthActions returns an IPN listener func that wraps the supplied IPN // listener func and transforms health messages passed to the wrapped listener. // If health messages with PrimaryActions are present, it appends the label & @@ -5087,7 +5051,12 @@ func (b *LocalBackend) authReconfigLocked() { } } - cfg, err := nmcfg.WGCfg(nm, b.logf, flags, prefs.ExitNodeID()) + priv := b.pm.CurrentPrefs().Persist().PrivateNodeKey() + if !priv.IsZero() && priv.Public() != nm.NodeKey { + priv = key.NodePrivate{} + } + + cfg, err := nmcfg.WGCfg(priv, nm, b.logf, flags, prefs.ExitNodeID()) if err != nil { b.logf("wgcfg: %v", err) return diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 962335046..5df0ae5bb 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -20,6 +20,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "testing" "time" @@ -49,6 +50,7 @@ import ( "tailscale.com/tsd" "tailscale.com/tstest" "tailscale.com/tstest/deptest" + "tailscale.com/tstest/typewalk" "tailscale.com/types/appctype" "tailscale.com/types/dnstype" "tailscale.com/types/ipproto" @@ -57,6 +59,7 @@ import ( "tailscale.com/types/logid" "tailscale.com/types/netmap" "tailscale.com/types/opt" + "tailscale.com/types/persist" "tailscale.com/types/ptr" "tailscale.com/types/views" "tailscale.com/util/dnsname" @@ -7112,3 +7115,104 @@ func eqUpdate(want appctype.RouteUpdate) func(appctype.RouteUpdate) error { return nil } } + +type fakeAttestationKey struct{ key.HardwareAttestationKey } + +func (f *fakeAttestationKey) Clone() key.HardwareAttestationKey { + return &fakeAttestationKey{} +} + +// TestStripKeysFromPrefs tests that LocalBackend's [stripKeysFromPrefs] (as used +// by sendNotify etc) correctly removes all private keys from an ipn.Notify. +// +// It does so by testing the the two ways that Notifys are sent: via sendNotify, +// and via extension hooks. +func TestStripKeysFromPrefs(t *testing.T) { + // genNotify generates a sample ipn.Notify with various private keys set + // at a certain path through the Notify data structure. + genNotify := map[string]func() ipn.Notify{ + "Notify.Prefs.ж.Persist.PrivateNodeKey": func() ipn.Notify { + return ipn.Notify{ + Prefs: ptr.To((&ipn.Prefs{ + Persist: &persist.Persist{PrivateNodeKey: key.NewNode()}, + }).View()), + } + }, + "Notify.Prefs.ж.Persist.OldPrivateNodeKey": func() ipn.Notify { + return ipn.Notify{ + Prefs: ptr.To((&ipn.Prefs{ + Persist: &persist.Persist{OldPrivateNodeKey: key.NewNode()}, + }).View()), + } + }, + "Notify.Prefs.ж.Persist.NetworkLockKey": func() ipn.Notify { + return ipn.Notify{ + Prefs: ptr.To((&ipn.Prefs{ + Persist: &persist.Persist{NetworkLockKey: key.NewNLPrivate()}, + }).View()), + } + }, + "Notify.Prefs.ж.Persist.AttestationKey": func() ipn.Notify { + return ipn.Notify{ + Prefs: ptr.To((&ipn.Prefs{ + Persist: &persist.Persist{AttestationKey: new(fakeAttestationKey)}, + }).View()), + } + }, + } + + private := key.PrivateTypesForTest() + + for path := range typewalk.MatchingPaths(reflect.TypeFor[ipn.Notify](), private.Contains) { + t.Run(path.Name, func(t *testing.T) { + gen, ok := genNotify[path.Name] + if !ok { + t.Fatalf("no genNotify function for path %q", path.Name) + } + withKey := gen() + + if path.Walk(reflect.ValueOf(withKey)).IsZero() { + t.Fatalf("generated notify does not have non-zero value at path %q", path.Name) + } + + h := &ExtensionHost{} + ch := make(chan *ipn.Notify, 1) + b := &LocalBackend{ + extHost: h, + notifyWatchers: map[string]*watchSession{ + "test": {ch: ch}, + }, + } + + var okay atomic.Int32 + testNotify := func(via string) func(*ipn.Notify) { + return func(n *ipn.Notify) { + if n == nil { + t.Errorf("notify from %s is nil", via) + return + } + if !path.Walk(reflect.ValueOf(*n)).IsZero() { + t.Errorf("notify from %s has non-zero value at path %q; key not stripped", via, path.Name) + } else { + okay.Add(1) + } + } + } + + h.Hooks().MutateNotifyLocked.Add(testNotify("MutateNotifyLocked hook")) + + b.send(withKey) + + select { + case n := <-ch: + testNotify("watchSession")(n) + default: + t.Errorf("no notify sent to watcher channel") + } + + if got := okay.Load(); got != 2 { + t.Errorf("notify passed validation %d times; want 2", got) + } + }) + } +} diff --git a/ipn/ipnlocal/profiles.go b/ipn/ipnlocal/profiles.go index 9c2176378..40a3c9887 100644 --- a/ipn/ipnlocal/profiles.go +++ b/ipn/ipnlocal/profiles.go @@ -24,6 +24,7 @@ import ( "tailscale.com/types/persist" "tailscale.com/util/clientmetric" "tailscale.com/util/eventbus" + "tailscale.com/util/testenv" ) var debug = envknob.RegisterBool("TS_DEBUG_PROFILES") @@ -849,6 +850,7 @@ func (pm *profileManager) CurrentPrefs() ipn.PrefsView { // ReadStartupPrefsForTest reads the startup prefs from disk. It is only used for testing. func ReadStartupPrefsForTest(logf logger.Logf, store ipn.StateStore) (ipn.PrefsView, error) { + testenv.AssertInTest() bus := eventbus.New() defer bus.Close() ht := health.NewTracker(bus) // in tests, don't care about the health status diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index de5ff53ac..ddd55234a 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -877,14 +877,6 @@ func (h *Handler) serveWatchIPNBus(w http.ResponseWriter, r *http.Request) { } mask = ipn.NotifyWatchOpt(v) } - // Users with only read access must request private key filtering. If they - // don't filter out private keys, require write access. - if (mask & ipn.NotifyNoPrivateKeys) == 0 { - if !h.PermitWrite { - http.Error(w, "watch IPN bus access denied, must set ipn.NotifyNoPrivateKeys when not running as admin/root or operator", http.StatusForbidden) - return - } - } w.Header().Set("Content-Type", "application/json") ctx := r.Context() diff --git a/ipn/localapi/localapi_test.go b/ipn/localapi/localapi_test.go index fa24717f7..d00b4117b 100644 --- a/ipn/localapi/localapi_test.go +++ b/ipn/localapi/localapi_test.go @@ -263,13 +263,17 @@ func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) { }) } +// TestServeWatchIPNBus used to test that various WatchIPNBus mask flags +// changed the permissions required to access the endpoint. +// However, since the removal of the NotifyNoPrivateKeys flag requirement +// for read-only users, this test now only verifies that the endpoint +// behaves correctly based on the PermitRead and PermitWrite settings. func TestServeWatchIPNBus(t *testing.T) { tstest.Replace(t, &validLocalHostForTesting, true) tests := []struct { desc string permitRead, permitWrite bool - mask ipn.NotifyWatchOpt // extra bits in addition to ipn.NotifyInitialState wantStatus int }{ { @@ -279,20 +283,13 @@ func TestServeWatchIPNBus(t *testing.T) { wantStatus: http.StatusForbidden, }, { - desc: "read-initial-state", + desc: "read-only", permitRead: true, permitWrite: false, - wantStatus: http.StatusForbidden, - }, - { - desc: "read-initial-state-no-private-keys", - permitRead: true, - permitWrite: false, - mask: ipn.NotifyNoPrivateKeys, wantStatus: http.StatusOK, }, { - desc: "read-initial-state-with-private-keys", + desc: "read-and-write", permitRead: true, permitWrite: true, wantStatus: http.StatusOK, @@ -311,7 +308,7 @@ func TestServeWatchIPNBus(t *testing.T) { c := s.Client() ctx, cancel := context.WithCancel(context.Background()) - req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/localapi/v0/watch-ipn-bus?mask=%d", s.URL, ipn.NotifyInitialState|tt.mask), nil) + req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s/localapi/v0/watch-ipn-bus?mask=%d", s.URL, ipn.NotifyInitialState), nil) if err != nil { t.Fatal(err) } diff --git a/tsconsensus/monitor.go b/tsconsensus/monitor.go index 61a5a74a0..2aa4c863b 100644 --- a/tsconsensus/monitor.go +++ b/tsconsensus/monitor.go @@ -102,15 +102,13 @@ func (m *monitor) handleSummaryStatus(w http.ResponseWriter, r *http.Request) { } func (m *monitor) handleNetmap(w http.ResponseWriter, r *http.Request) { - var mask ipn.NotifyWatchOpt = ipn.NotifyInitialNetMap - mask |= ipn.NotifyNoPrivateKeys lc, err := m.ts.LocalClient() if err != nil { log.Printf("monitor: error LocalClient: %v", err) http.Error(w, "", http.StatusInternalServerError) return } - watcher, err := lc.WatchIPNBus(r.Context(), mask) + watcher, err := lc.WatchIPNBus(r.Context(), ipn.NotifyInitialNetMap) if err != nil { log.Printf("monitor: error WatchIPNBus: %v", err) http.Error(w, "", http.StatusInternalServerError) diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 2944f6359..14747650f 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -350,7 +350,7 @@ func (s *Server) Up(ctx context.Context) (*ipnstate.Status, error) { return nil, fmt.Errorf("tsnet.Up: %w", err) } - watcher, err := lc.WatchIPNBus(ctx, ipn.NotifyInitialState|ipn.NotifyNoPrivateKeys) + watcher, err := lc.WatchIPNBus(ctx, ipn.NotifyInitialState) if err != nil { return nil, fmt.Errorf("tsnet.Up: %w", err) } diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 64f49c7b8..9d75cfc29 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -2128,16 +2128,10 @@ func TestC2NDebugNetmap(t *testing.T) { var current netmap.NetworkMap must.Do(json.Unmarshal(resp.Current, ¤t)) - if !current.PrivateKey.IsZero() { - t.Errorf("current netmap has non-zero private key: %v", current.PrivateKey) - } // Check candidate netmap if we sent a map response. if cand != nil { var candidate netmap.NetworkMap must.Do(json.Unmarshal(resp.Candidate, &candidate)) - if !candidate.PrivateKey.IsZero() { - t.Errorf("candidate netmap has non-zero private key: %v", candidate.PrivateKey) - } if diff := cmp.Diff(current.SelfNode, candidate.SelfNode); diff != "" { t.Errorf("SelfNode differs (-current +candidate):\n%s", diff) } diff --git a/tstest/typewalk/typewalk.go b/tstest/typewalk/typewalk.go new file mode 100644 index 000000000..b22505351 --- /dev/null +++ b/tstest/typewalk/typewalk.go @@ -0,0 +1,106 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package typewalk provides utilities to walk Go types using reflection. +package typewalk + +import ( + "iter" + "reflect" + "strings" +) + +// Path describes a path via a type where a private key may be found, +// along with a function to test whether a reflect.Value at that path is +// non-zero. +type Path struct { + // Name is the path from the root type, suitable for using as a t.Run name. + Name string + + // Walk returns the reflect.Value at the end of the path, given a root + // reflect.Value. + Walk func(root reflect.Value) (leaf reflect.Value) +} + +// MatchingPaths returns a sequence of [Path] for all paths +// within the given type that end in a type matching match. +func MatchingPaths(rt reflect.Type, match func(reflect.Type) bool) iter.Seq[Path] { + // valFromRoot is a function that, given a reflect.Value of the root struct, + // returns the reflect.Value at some path within it. + type valFromRoot func(reflect.Value) reflect.Value + + return func(yield func(Path) bool) { + var walk func(reflect.Type, valFromRoot) + var path []string + var done bool + seen := map[reflect.Type]bool{} + + walk = func(t reflect.Type, getV valFromRoot) { + if seen[t] { + return + } + seen[t] = true + defer func() { seen[t] = false }() + if done { + return + } + if match(t) { + if !yield(Path{ + Name: strings.Join(path, "."), + Walk: getV, + }) { + done = true + } + return + } + switch t.Kind() { + case reflect.Ptr, reflect.Slice, reflect.Array: + walk(t.Elem(), func(root reflect.Value) reflect.Value { + v := getV(root) + return v.Elem() + }) + case reflect.Struct: + for i := range t.NumField() { + sf := t.Field(i) + fieldName := sf.Name + if fieldName == "_" { + continue + } + path = append(path, fieldName) + walk(sf.Type, func(root reflect.Value) reflect.Value { + return getV(root).FieldByName(fieldName) + }) + path = path[:len(path)-1] + if done { + return + } + } + case reflect.Map: + walk(t.Elem(), func(root reflect.Value) reflect.Value { + v := getV(root) + if v.Len() == 0 { + return reflect.Zero(t.Elem()) + } + iter := v.MapRange() + iter.Next() + return iter.Value() + }) + if done { + return + } + walk(t.Key(), func(root reflect.Value) reflect.Value { + v := getV(root) + if v.Len() == 0 { + return reflect.Zero(t.Key()) + } + iter := v.MapRange() + iter.Next() + return iter.Key() + }) + } + } + + path = append(path, rt.Name()) + walk(rt, func(v reflect.Value) reflect.Value { return v }) + } +} diff --git a/types/key/util.go b/types/key/util.go index bdb2a06f6..50fac8275 100644 --- a/types/key/util.go +++ b/types/key/util.go @@ -10,9 +10,12 @@ import ( "errors" "fmt" "io" + "reflect" "slices" "go4.org/mem" + "tailscale.com/util/set" + "tailscale.com/util/testenv" ) // rand fills b with cryptographically strong random bytes. Panics if @@ -115,3 +118,18 @@ func debug32(k [32]byte) string { dst[6] = ']' return string(dst[:7]) } + +// PrivateTypesForTest returns the set of private key types +// in this package, for testing purposes. +func PrivateTypesForTest() set.Set[reflect.Type] { + testenv.AssertInTest() + return set.Of( + reflect.TypeFor[ChallengePrivate](), + reflect.TypeFor[ControlPrivate](), + reflect.TypeFor[DiscoPrivate](), + reflect.TypeFor[MachinePrivate](), + reflect.TypeFor[NodePrivate](), + reflect.TypeFor[NLPrivate](), + reflect.TypeFor[HardwareAttestationKey](), + ) +} diff --git a/types/netmap/netmap.go b/types/netmap/netmap.go index cc6bec1db..0a2f3ea71 100644 --- a/types/netmap/netmap.go +++ b/types/netmap/netmap.go @@ -26,11 +26,10 @@ import ( // The fields should all be considered read-only. They might // alias parts of previous NetworkMap values. type NetworkMap struct { - SelfNode tailcfg.NodeView - AllCaps set.Set[tailcfg.NodeCapability] // set version of SelfNode.Capabilities + SelfNode.CapMap - NodeKey key.NodePublic - PrivateKey key.NodePrivate - Expiry time.Time + SelfNode tailcfg.NodeView + AllCaps set.Set[tailcfg.NodeCapability] // set version of SelfNode.Capabilities + SelfNode.CapMap + NodeKey key.NodePublic + Expiry time.Time // Name is the DNS name assigned to this node. // It is the MapResponse.Node.Name value and ends with a period. Name string diff --git a/types/netmap/netmap_test.go b/types/netmap/netmap_test.go index 40f504741..ee4fecdb4 100644 --- a/types/netmap/netmap_test.go +++ b/types/netmap/netmap_test.go @@ -6,11 +6,13 @@ package netmap import ( "encoding/hex" "net/netip" + "reflect" "testing" "go4.org/mem" "tailscale.com/net/netaddr" "tailscale.com/tailcfg" + "tailscale.com/tstest/typewalk" "tailscale.com/types/key" ) @@ -316,3 +318,10 @@ func TestPeerIndexByNodeID(t *testing.T) { } } } + +func TestNoPrivateKeyMaterial(t *testing.T) { + private := key.PrivateTypesForTest() + for path := range typewalk.MatchingPaths(reflect.TypeFor[NetworkMap](), private.Contains) { + t.Errorf("NetworkMap contains private key material at path: %q", path.Name) + } +} diff --git a/wgengine/bench/wg.go b/wgengine/bench/wg.go index f0fa38bf9..ce6add866 100644 --- a/wgengine/bench/wg.go +++ b/wgengine/bench/wg.go @@ -111,9 +111,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. Endpoints: epFromTyped(st.LocalAddrs), } e2.SetNetworkMap(&netmap.NetworkMap{ - NodeKey: k2.Public(), - PrivateKey: k2, - Peers: []tailcfg.NodeView{n.View()}, + NodeKey: k2.Public(), + Peers: []tailcfg.NodeView{n.View()}, }) p := wgcfg.Peer{ @@ -143,9 +142,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. Endpoints: epFromTyped(st.LocalAddrs), } e1.SetNetworkMap(&netmap.NetworkMap{ - NodeKey: k1.Public(), - PrivateKey: k1, - Peers: []tailcfg.NodeView{n.View()}, + NodeKey: k1.Public(), + Peers: []tailcfg.NodeView{n.View()}, }) p := wgcfg.Peer{ diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index e91dac2ec..09c54f504 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -308,8 +308,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM buildNetmapLocked := func(myIdx int) *netmap.NetworkMap { me := ms[myIdx] nm := &netmap.NetworkMap{ - PrivateKey: me.privateKey, - NodeKey: me.privateKey.Public(), + NodeKey: me.privateKey.Public(), SelfNode: (&tailcfg.Node{ Addresses: []netip.Prefix{netip.PrefixFrom(netaddr.IPv4(1, 0, 0, byte(myIdx+1)), 32)}, }).View(), @@ -356,7 +355,7 @@ func meshStacks(logf logger.Logf, mutateNetmap func(idx int, nm *netmap.NetworkM peerSet.Add(peer.Key()) } m.conn.UpdatePeers(peerSet) - wg, err := nmcfg.WGCfg(nm, logf, 0, "") + wg, err := nmcfg.WGCfg(ms[i].privateKey, nm, logf, 0, "") if err != nil { // We're too far from the *testing.T to be graceful, // blow up. Shouldn't happen anyway. @@ -2201,9 +2200,8 @@ func TestIsWireGuardOnlyPeer(t *testing.T) { defer m.Close() nm := &netmap.NetworkMap{ - Name: "ts", - PrivateKey: m.privateKey, - NodeKey: m.privateKey.Public(), + Name: "ts", + NodeKey: m.privateKey.Public(), SelfNode: (&tailcfg.Node{ Addresses: []netip.Prefix{tsaip}, }).View(), @@ -2224,7 +2222,7 @@ func TestIsWireGuardOnlyPeer(t *testing.T) { } m.conn.onNodeViewsUpdate(nv) - cfg, err := nmcfg.WGCfg(nm, t.Logf, netmap.AllowSubnetRoutes, "") + cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") if err != nil { t.Fatal(err) } @@ -2266,9 +2264,8 @@ func TestIsWireGuardOnlyPeerWithMasquerade(t *testing.T) { defer m.Close() nm := &netmap.NetworkMap{ - Name: "ts", - PrivateKey: m.privateKey, - NodeKey: m.privateKey.Public(), + Name: "ts", + NodeKey: m.privateKey.Public(), SelfNode: (&tailcfg.Node{ Addresses: []netip.Prefix{tsaip}, }).View(), @@ -2290,7 +2287,7 @@ func TestIsWireGuardOnlyPeerWithMasquerade(t *testing.T) { } m.conn.onNodeViewsUpdate(nv) - cfg, err := nmcfg.WGCfg(nm, t.Logf, netmap.AllowSubnetRoutes, "") + cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") if err != nil { t.Fatal(err) } @@ -2334,7 +2331,7 @@ func applyNetworkMap(t *testing.T, m *magicStack, nm *netmap.NetworkMap) { m.conn.noV6.Store(true) // Turn the network map into a wireguard config (for the tailscale internal wireguard device). - cfg, err := nmcfg.WGCfg(nm, t.Logf, netmap.AllowSubnetRoutes, "") + cfg, err := nmcfg.WGCfg(m.privateKey, nm, t.Logf, netmap.AllowSubnetRoutes, "") if err != nil { t.Fatal(err) } @@ -2403,9 +2400,8 @@ func TestIsWireGuardOnlyPickEndpointByPing(t *testing.T) { wgEpV6 := netip.MustParseAddrPort(v6.LocalAddr().String()) nm := &netmap.NetworkMap{ - Name: "ts", - PrivateKey: m.privateKey, - NodeKey: m.privateKey.Public(), + Name: "ts", + NodeKey: m.privateKey.Public(), SelfNode: (&tailcfg.Node{ Addresses: []netip.Prefix{tsaip}, }).View(), diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index 28d5345d6..487e78d81 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -12,6 +12,7 @@ import ( "strings" "tailscale.com/tailcfg" + "tailscale.com/types/key" "tailscale.com/types/logger" "tailscale.com/types/logid" "tailscale.com/types/netmap" @@ -41,9 +42,9 @@ func cidrIsSubnet(node tailcfg.NodeView, cidr netip.Prefix) bool { } // WGCfg returns the NetworkMaps's WireGuard configuration. -func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, exitNode tailcfg.StableNodeID) (*wgcfg.Config, error) { +func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags, exitNode tailcfg.StableNodeID) (*wgcfg.Config, error) { cfg := &wgcfg.Config{ - PrivateKey: nm.PrivateKey, + PrivateKey: pk, Addresses: nm.GetAddresses().AsSlice(), Peers: make([]wgcfg.Peer, 0, len(nm.Peers)), }