From 692f84df8d0ca951fbf2cd640f9d87139ed215d4 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 9 Jul 2026 16:30:45 +0000 Subject: [PATCH] wgengine,wgcfg,feature/netlog: move network flow logging behind a feature hook wgcfg.Config.NetworkLogging carried the network flow logging identity inside the WireGuard config, where it was unrelated to WireGuard; it lived there mainly so that identity changes would defeat Reconfig's ErrNoChanges check and reach the netlog startup/shutdown logic. Remove the field and move the whole netlog lifecycle into a new feature/netlog package, installed on the engine via the new wgengine.HookNewNetLogger hook, like other feature/* packages. The logging identity now comes from LocalBackend's current netmap via the widened NetLogSource interface (replacing Engine.SetNetLogNodeSource), so nmcfg no longer parses audit log IDs into the config. The engine still calls the hook before its ErrNoChanges return and before router.Set (to capture initial packets), and again after router.Set (to capture final packets), preserving the previous ordering. Core wgengine no longer imports wgengine/netlog, so minimal builds drop it entirely. tailscaled keeps netlog via feature/condregister, and tsnet imports feature/condregister/netlog explicitly to keep netlog enabled by default in tsnet-based binaries (tsidp, k8s-operator). This is pulled out of a future change that removes wgcfg.Config.Peers, to make that PR smaller. Updates #12542 Updates #12614 Change-Id: I41ca7dfe43c51e977c41b5f8e934bd1f0e6e6e24 Signed-off-by: Brad Fitzpatrick --- cmd/k8s-operator/depaware.txt | 4 +- cmd/tailscaled/depaware-min.txt | 2 +- cmd/tailscaled/depaware-minbox.txt | 2 +- cmd/tailscaled/depaware.txt | 4 +- cmd/tsidp/depaware.txt | 4 +- feature/condregister/maybe_netlog.go | 6 + feature/condregister/netlog/doc.go | 8 ++ feature/condregister/netlog/maybe_netlog.go | 8 ++ feature/netlog/netlog.go | 119 ++++++++++++++++++++ ipn/ipnlocal/local.go | 43 +++++-- ipn/ipnlocal/state_test.go | 3 +- tsnet/depaware.txt | 4 +- tsnet/tsnet.go | 1 + wgengine/netlog_hooks.go | 86 ++++++++++++++ wgengine/userspace.go | 108 +++++++----------- wgengine/wgcfg/config.go | 11 -- wgengine/wgcfg/config_test.go | 3 +- wgengine/wgcfg/nmcfg/nmcfg.go | 22 ---- wgengine/wgcfg/wgcfg_clone.go | 12 +- wgengine/wgengine.go | 8 +- 20 files changed, 329 insertions(+), 129 deletions(-) create mode 100644 feature/condregister/maybe_netlog.go create mode 100644 feature/condregister/netlog/doc.go create mode 100644 feature/condregister/netlog/maybe_netlog.go create mode 100644 feature/netlog/netlog.go create mode 100644 wgengine/netlog_hooks.go diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 1177b46f1..4da89afa6 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -734,9 +734,11 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/feature/buildfeatures from tailscale.com/wgengine/magicsock+ tailscale.com/feature/c2n from tailscale.com/tsnet tailscale.com/feature/condlite/expvar from tailscale.com/wgengine/magicsock + tailscale.com/feature/condregister/netlog from tailscale.com/tsnet tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy @@ -921,7 +923,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ + tailscale.com/wgengine/netlog from tailscale.com/feature/netlog tailscale.com/wgengine/netstack from tailscale.com/tsnet tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index 83a007b6d..f9a87a608 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -62,6 +62,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/buildfeatures from tailscale.com/cmd/tailscaled+ tailscale.com/feature/condlite/expvar from tailscale.com/wgengine/magicsock tailscale.com/feature/condregister from tailscale.com/cmd/tailscaled + tailscale.com/feature/condregister/netlog from tailscale.com/feature/condregister tailscale.com/feature/condregister/portmapper from tailscale.com/feature/condregister tailscale.com/feature/condregister/useproxy from tailscale.com/feature/condregister tailscale.com/health from tailscale.com/control/controlclient+ @@ -195,7 +196,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index 2fd6523f3..e5dce3277 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/cmd/tailscaled/depaware-minbox.txt @@ -75,6 +75,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/condregister from tailscale.com/cmd/tailscaled tailscale.com/feature/condregister/awsparamstore from tailscale.com/cmd/tailscale/cli tailscale.com/feature/condregister/identityfederation from tailscale.com/cmd/tailscale/cli + tailscale.com/feature/condregister/netlog from tailscale.com/feature/condregister tailscale.com/feature/condregister/oauthkey from tailscale.com/cmd/tailscale/cli tailscale.com/feature/condregister/portmapper from tailscale.com/feature/condregister+ tailscale.com/feature/condregister/useproxy from tailscale.com/cmd/tailscale/cli+ @@ -216,7 +217,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 2d19823f0..4874769b4 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -298,6 +298,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/clientupdate from tailscale.com/feature/condregister tailscale.com/feature/condlite/expvar from tailscale.com/wgengine/magicsock tailscale.com/feature/condregister from tailscale.com/cmd/tailscaled + tailscale.com/feature/condregister/netlog from tailscale.com/feature/condregister tailscale.com/feature/condregister/portmapper from tailscale.com/feature/condregister tailscale.com/feature/condregister/useproxy from tailscale.com/feature/condregister tailscale.com/feature/conn25 from tailscale.com/feature/condregister @@ -306,6 +307,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/feature/drive from tailscale.com/feature/condregister L tailscale.com/feature/linkspeed from tailscale.com/feature/condregister L tailscale.com/feature/linuxdnsfight from tailscale.com/feature/condregister + tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/portlist from tailscale.com/feature/condregister tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper tailscale.com/feature/posture from tailscale.com/feature/condregister @@ -506,7 +508,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ + tailscale.com/wgengine/netlog from tailscale.com/feature/netlog tailscale.com/wgengine/netstack from tailscale.com/cmd/tailscaled tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/cmd/tailscaled+ diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index fa2b87575..33f885d4a 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -151,9 +151,11 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/feature/buildfeatures from tailscale.com/wgengine/magicsock+ tailscale.com/feature/c2n from tailscale.com/tsnet tailscale.com/feature/condlite/expvar from tailscale.com/wgengine/magicsock + tailscale.com/feature/condregister/netlog from tailscale.com/tsnet tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy @@ -319,7 +321,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ + tailscale.com/wgengine/netlog from tailscale.com/feature/netlog tailscale.com/wgengine/netstack from tailscale.com/tsnet tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ diff --git a/feature/condregister/maybe_netlog.go b/feature/condregister/maybe_netlog.go new file mode 100644 index 000000000..f7b72e155 --- /dev/null +++ b/feature/condregister/maybe_netlog.go @@ -0,0 +1,6 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package condregister + +import _ "tailscale.com/feature/condregister/netlog" diff --git a/feature/condregister/netlog/doc.go b/feature/condregister/netlog/doc.go new file mode 100644 index 000000000..ff939ce5a --- /dev/null +++ b/feature/condregister/netlog/doc.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package netlog registers support for network flow logging if it's +// not disabled via the ts_omit_netlog or ts_omit_logtail build tags. +// It is pulled out separately from the main condregister package so +// that tsnet can link it without linking every conditional feature. +package netlog diff --git a/feature/condregister/netlog/maybe_netlog.go b/feature/condregister/netlog/maybe_netlog.go new file mode 100644 index 000000000..ec7a79133 --- /dev/null +++ b/feature/condregister/netlog/maybe_netlog.go @@ -0,0 +1,8 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !ts_omit_netlog && !ts_omit_logtail + +package netlog + +import _ "tailscale.com/feature/netlog" diff --git a/feature/netlog/netlog.go b/feature/netlog/netlog.go new file mode 100644 index 000000000..2f85d671c --- /dev/null +++ b/feature/netlog/netlog.go @@ -0,0 +1,119 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package netlog registers the network flow logging feature and wires +// it into the WireGuard engine. The logger implementation itself lives +// in tailscale.com/wgengine/netlog. +package netlog + +import ( + "context" + "time" + + "tailscale.com/envknob" + "tailscale.com/feature" + "tailscale.com/types/logid" + "tailscale.com/wgengine" + "tailscale.com/wgengine/netlog" + "tailscale.com/wgengine/router" +) + +func init() { + feature.Register("netlog") + wgengine.HookNewNetLogger.Set(newNetLogger) +} + +// uploadTimeout is the maximum time to wait when shutting down the +// network logger as it uploads the final network log messages. +const uploadTimeout = 5 * time.Second + +// netLogger implements [wgengine.NetLogger] on top of [netlog.Logger], +// tracking the logging identity and running state across engine +// reconfigurations. One netLogger exists per engine. +type netLogger struct { + deps wgengine.NetLoggerDeps + logger netlog.Logger + + // The fields below are only accessed from Reconfig and + // ReconfigDone, which the engine serializes under its internal + // lock. + + // nodeID and domainID are the logging IDs from the last Reconfig, + // or zero if logging was not enabled then. + nodeID, domainID logid.PrivateID + // logExitFlows is whether exit node flows were logged as of the + // last Reconfig. + logExitFlows bool + // shouldRun is whether the last Reconfig wanted the logger + // running. ReconfigDone consults it to shut down a stopping + // logger after the router has been reconfigured. + shouldRun bool +} + +func newNetLogger(deps wgengine.NetLoggerDeps) wgengine.NetLogger { + return &netLogger{deps: deps} +} + +func (nl *netLogger) Reconfig(routerCfg *router.Config, routerChanged bool) (changed bool) { + logf := nl.deps.Logf + src := nl.deps.Source() + + var nodeID, domainID logid.PrivateID + var logExitFlows bool + if src != nil { + if nid, did, lef, ok := src.NetLogIDs(); ok { + nodeID, domainID, logExitFlows = nid, did, lef + } + } + idsValid := !nodeID.IsZero() && !domainID.IsZero() + idsWereValid := !nl.nodeID.IsZero() && !nl.domainID.IsZero() + changed = nodeID != nl.nodeID || domainID != nl.domainID || logExitFlows != nl.logExitFlows + idsChanged := idsValid && idsWereValid && changed + nl.nodeID, nl.domainID, nl.logExitFlows = nodeID, domainID, logExitFlows + + nl.shouldRun = idsValid && !routerCfg.Equal(&router.Config{}) && !envknob.NoLogsNoSupport() + + // Shut down the logger if the IDs changed while it was running, + // letting the startup logic below bring it back up with the new + // identity. + if idsChanged && nl.logger.Running() { + logf("wgengine: Reconfig: shutting down network logger") + nl.shutdown() + } + + // Start the logger before the engine configures the router so + // that it captures initial packets. + if nl.shouldRun && !nl.logger.Running() { + logf("wgengine: Reconfig: starting up network logger (node:%s tailnet:%s)", nodeID.Public(), domainID.Public()) + if err := nl.logger.Startup(logf, src, nodeID, domainID, nl.deps.Tun, nl.deps.Sock, nl.deps.NetMon, nl.deps.Health, nl.deps.Bus, logExitFlows); err != nil { + logf("wgengine: Reconfig: error starting up network logger: %v", err) + } + nl.logger.ReconfigRoutes(routerCfg) + } else if routerChanged && nl.logger.Running() { + nl.logger.ReconfigRoutes(routerCfg) + } + return changed +} + +func (nl *netLogger) ReconfigDone() { + // Shut down a stopping logger only now, after the engine has + // configured the router, so that it captures final packets. + if !nl.shouldRun && nl.logger.Running() { + nl.deps.Logf("wgengine: Reconfig: shutting down network logger") + nl.shutdown() + } +} + +func (nl *netLogger) Shutdown() { + nl.shutdown() +} + +// shutdown stops the logger, waiting up to uploadTimeout for it to +// flush its final messages. +func (nl *netLogger) shutdown() { + ctx, cancel := context.WithTimeout(context.Background(), uploadTimeout) + defer cancel() + if err := nl.logger.Shutdown(ctx); err != nil { + nl.deps.Logf("wgengine: error shutting down network logger: %v", err) + } +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 5fafc9043..50a49d04b 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -103,7 +103,6 @@ "tailscale.com/wgengine" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/magicsock" - "tailscale.com/wgengine/netlog" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" "tailscale.com/wgengine/wgcfg/nmcfg" @@ -636,7 +635,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo e.SetPeerByIPPacketFunc(b.lookupPeerByIP) e.SetPeerForIPFunc(b.peerForIP) e.SetPeerSessionStateFunc(b.onPeerWireGuardState) - e.SetNetLogNodeSource(netLogNodeSource{b}) + e.SetNetLogSource(netLogNodeSource{b}) e.SetWGPeerLookup(b.lookupPeerWireGuardString) b.dialer.SetResolveMagicDNS(b.resolveMagicDNS) @@ -8017,9 +8016,10 @@ func (n noiseRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) return n.lb.DoNoiseRequest(req) } -// netLogNodeSource adapts LocalBackend's nodeBackend to [netlog.NodeSource]. -// Each method consults [LocalBackend.currentNode] so that profile rotations -// are picked up automatically without re-installing the source. +// netLogNodeSource adapts LocalBackend to [wgengine.NetLogSource]. +// Each method consults [LocalBackend.currentNode] or the current netmap +// so that profile rotations are picked up automatically without +// re-installing the source. type netLogNodeSource struct { b *LocalBackend } @@ -8048,8 +8048,37 @@ func (s netLogNodeSource) NodeByAddr(addr netip.Addr) (_ tailcfg.NodeView, _ tai return nv, up, true } -// Compile-time assertion that netLogNodeSource implements [netlog.NodeSource]. -var _ netlog.NodeSource = netLogNodeSource{} +// NetLogIDs implements [wgengine.NetLogSource], returning the network +// flow logging identity from the current netmap. ok is false if the +// netmap does not enable network flow logging for this node. +func (s netLogNodeSource) NetLogIDs() (nodeID, domainID logid.PrivateID, logExitFlows bool, ok bool) { + nm := s.b.NetMap() + if nm == nil || !nm.SelfNode.Valid() { + return + } + if !nm.SelfNode.HasCap(tailcfg.CapabilityDataPlaneAuditLogs) { + return + } + if nm.SelfNode.DataPlaneAuditLogID() == "" || nm.DomainAuditLogID == "" { + return + } + nodeID, errNode := logid.ParsePrivateID(nm.SelfNode.DataPlaneAuditLogID()) + if errNode != nil { + s.b.logf("[v1] netlog: unable to parse node audit log ID: %v", errNode) + } + domainID, errDomain := logid.ParsePrivateID(nm.DomainAuditLogID) + if errDomain != nil { + s.b.logf("[v1] netlog: unable to parse domain audit log ID: %v", errDomain) + } + if errNode != nil || errDomain != nil { + return logid.PrivateID{}, logid.PrivateID{}, false, false + } + return nodeID, domainID, nm.SelfNode.HasCap(tailcfg.NodeAttrLogExitFlows), true +} + +// Compile-time assertion that netLogNodeSource implements +// [wgengine.NetLogSource]. +var _ wgengine.NetLogSource = netLogNodeSource{} // lookupPeerWireGuardString returns the Tailscale-conventional short string // (e.g. "[IMTBr]") for the peer whose wireguard-go-formatted public key diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 7c1c9a23b..7ce4f590c 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -47,7 +47,6 @@ "tailscale.com/wgengine" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/magicsock" - "tailscale.com/wgengine/netlog" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" "tailscale.com/wgengine/wgint" @@ -1989,7 +1988,7 @@ func (e *mockEngine) PeerKeyForIP(netip.Addr) (_ key.NodePublic, _ netip.Prefix, } func (e *mockEngine) SetPeerSessionStateFunc(func(key.NodePublic, wgengine.PeerWireGuardState)) { } -func (e *mockEngine) SetNetLogNodeSource(netlog.NodeSource) {} +func (e *mockEngine) SetNetLogSource(wgengine.NetLogSource) {} func (e *mockEngine) SetWGPeerLookup(func(wgString string) (tsString string, ok bool)) {} func (e *mockEngine) ProbeLocks() {} diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index a553a9e93..7162bb616 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -147,9 +147,11 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/feature/buildfeatures from tailscale.com/wgengine/magicsock+ tailscale.com/feature/c2n from tailscale.com/tsnet tailscale.com/feature/condlite/expvar from tailscale.com/wgengine/magicsock + tailscale.com/feature/condregister/netlog from tailscale.com/tsnet tailscale.com/feature/condregister/oauthkey from tailscale.com/tsnet tailscale.com/feature/condregister/portmapper from tailscale.com/tsnet tailscale.com/feature/condregister/useproxy from tailscale.com/tsnet + tailscale.com/feature/netlog from tailscale.com/feature/condregister/netlog tailscale.com/feature/oauthkey from tailscale.com/feature/condregister/oauthkey tailscale.com/feature/portmapper from tailscale.com/feature/condregister/portmapper tailscale.com/feature/useproxy from tailscale.com/feature/condregister/useproxy @@ -314,7 +316,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/filter/filtertype from tailscale.com/types/netmap+ 💣 tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ - tailscale.com/wgengine/netlog from tailscale.com/wgengine+ + tailscale.com/wgengine/netlog from tailscale.com/feature/netlog tailscale.com/wgengine/netstack from tailscale.com/tsnet tailscale.com/wgengine/netstack/gro from tailscale.com/net/tstun+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 5bc0dd79c..50e75417d 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -163,6 +163,7 @@ "tailscale.com/control/controlclient" "tailscale.com/envknob" _ "tailscale.com/feature/c2n" + _ "tailscale.com/feature/condregister/netlog" _ "tailscale.com/feature/condregister/oauthkey" _ "tailscale.com/feature/condregister/portmapper" _ "tailscale.com/feature/condregister/useproxy" diff --git a/wgengine/netlog_hooks.go b/wgengine/netlog_hooks.go new file mode 100644 index 000000000..129f2f96b --- /dev/null +++ b/wgengine/netlog_hooks.go @@ -0,0 +1,86 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package wgengine + +import ( + "net/netip" + + "tailscale.com/feature" + "tailscale.com/health" + "tailscale.com/net/netmon" + "tailscale.com/net/tstun" + "tailscale.com/tailcfg" + "tailscale.com/types/logger" + "tailscale.com/types/logid" + "tailscale.com/util/eventbus" + "tailscale.com/wgengine/magicsock" + "tailscale.com/wgengine/router" +) + +// NetLogSource provides the network flow logging feature what it needs +// from the rest of the system (in practice, ipnlocal.LocalBackend): +// on-demand node lookups and the current audit logging identity. +type NetLogSource interface { + // SelfNode returns the current self node and its owning user profile. + // The views are invalid if there is no current node. + SelfNode() (tailcfg.NodeView, tailcfg.UserProfileView) + + // NodeByAddr returns the node assigned the given address along with + // its owning user profile. + // ok is false if no node is known to own addr. + NodeByAddr(addr netip.Addr) (node tailcfg.NodeView, user tailcfg.UserProfileView, ok bool) + + // NetLogIDs returns the audit logging IDs from the current netmap, + // along with whether exit node flows should be logged. + // ok is false if network flow logging should not run. + NetLogIDs() (nodeID, domainID logid.PrivateID, logExitFlows bool, ok bool) +} + +// NetLogger is the engine's handle on the network flow logger +// lifecycle, implemented by the feature/netlog package. +// +// Reconfig and ReconfigDone are only called from [Engine.Reconfig], +// serialized by the engine's internal lock. Shutdown may be called +// concurrently with them from [Engine.Close]. +type NetLogger interface { + // Reconfig is called near the top of every [Engine.Reconfig], + // before its ErrNoChanges early return (so identity-only changes + // still take effect) and before the router is configured (so a + // starting logger captures initial packets). It reports whether + // network logging state changed, which suppresses the engine's + // ErrNoChanges early return. + Reconfig(routerCfg *router.Config, routerChanged bool) (changed bool) + + // ReconfigDone is called at the end of [Engine.Reconfig], after + // the router is configured, so that a logger stopping as a result + // of the preceding Reconfig call captures final packets. It may + // block to flush pending messages. + ReconfigDone() + + // Shutdown stops the logger. It may block to flush pending + // messages, bounded by an internal upload timeout. + Shutdown() +} + +// NetLoggerDeps are the engine-owned dependencies passed to +// [HookNewNetLogger] to construct a [NetLogger]. +type NetLoggerDeps struct { + Logf logger.Logf + Tun *tstun.Wrapper + Sock *magicsock.Conn + NetMon *netmon.Monitor + Health *health.Tracker + Bus *eventbus.Bus + + // Source returns the [NetLogSource] installed via + // [Engine.SetNetLogSource], or nil if none has been installed yet. + // It is a func because the source is installed after the engine + // (and thus the NetLogger) is constructed. + Source func() NetLogSource +} + +// HookNewNetLogger is set by the feature/netlog package to construct +// each engine's [NetLogger]. If unset, network flow logging is not +// linked into the binary and the engine skips all NetLogger calls. +var HookNewNetLogger feature.Hook[func(NetLoggerDeps) NetLogger] diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 9b21ef612..32683e3c7 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -58,7 +58,6 @@ "tailscale.com/version" "tailscale.com/wgengine/filter" "tailscale.com/wgengine/magicsock" - "tailscale.com/wgengine/netlog" "tailscale.com/wgengine/netstack/gro" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" @@ -66,10 +65,6 @@ "tailscale.com/wgengine/wglog" ) -// networkLoggerUploadTimeout is the maximum timeout to wait when -// shutting down the network logger as it uploads the last network log messages. -const networkLoggerUploadTimeout = 5 * time.Second - type userspaceEngine struct { // eventBus will eventually become required, but for now may be nil. eventBus *eventbus.Bus @@ -156,13 +151,16 @@ type userspaceEngine struct { // value of the ICMP identifier and sequence number concatenated. icmpEchoResponseCallback map[uint32]func() - // networkLogger logs statistics about network connections. - networkLogger netlog.Logger + // netlogger is the network flow logger handle constructed via + // [HookNewNetLogger], or nil if the feature/netlog package is not + // linked into the binary. + netlogger NetLogger - // netLogSource is the [netlog.NodeSource] installed via - // [Engine.SetNetLogNodeSource]; it is read when starting up the network - // logger from inside Reconfig. It may be nil if no source was installed. - netLogSource syncs.AtomicValue[netlog.NodeSource] + // netLogSource is the [NetLogSource] installed via + // [Engine.SetNetLogSource]; the netlogger reads it on each + // Reconfig via [NetLoggerDeps.Source]. It may be nil if no source + // was installed. + netLogSource syncs.AtomicValue[NetLogSource] // wgPeerLookup is the lookup function installed via // [Engine.SetWGPeerLookup]; it is consulted by [wgLogger] to translate @@ -454,6 +452,18 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) closePool.add(e.magicConn) e.magicConn.SetNetworkUp(e.netMon.InterfaceState().AnyInterfaceUp()) + if newNetLogger, ok := HookNewNetLogger.GetOk(); ok { + e.netlogger = newNetLogger(NetLoggerDeps{ + Logf: logf, + Tun: e.tundev, + Sock: e.magicConn, + NetMon: e.netMon, + Health: e.health, + Bus: e.eventBus, + Source: e.netLogSource.Load, + }) + } + tsTUNDev.SetDiscoKey(e.magicConn.DiscoPublicKey()) if conf.RespondToPing { @@ -758,9 +768,9 @@ func (e *userspaceEngine) SetPeerSessionStateFunc(fn func(key.NodePublic, PeerWi }) } -// SetNetLogNodeSource installs the [netlog.NodeSource] used by the engine's -// network logger. -func (e *userspaceEngine) SetNetLogNodeSource(src netlog.NodeSource) { +// SetNetLogSource installs the [NetLogSource] consulted by the engine's +// network flow logger. +func (e *userspaceEngine) SetNetLogSource(src NetLogSource) { e.netLogSource.Store(src) } @@ -864,17 +874,18 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, listenPortChanged := listenPort != e.magicConn.LocalPort() peerMTUChanged := peerMTUEnable != e.magicConn.PeerMTUEnabled() - if !engineChanged && !routerChanged && !dnsChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged { - return ErrNoChanges + + // Let the network flow logger react before the early return below, + // so that logging identity changes take effect even when nothing + // else changed, and before the router is configured, so that a + // starting logger captures initial packets. + netlogChanged := false + if e.netlogger != nil { + netlogChanged = e.netlogger.Reconfig(routerCfg, routerChanged) } - newLogIDs := cfg.NetworkLogging - oldLogIDs := e.lastCfgFull.NetworkLogging - netLogIDsNowValid := !newLogIDs.NodeID.IsZero() && !newLogIDs.DomainID.IsZero() - netLogIDsWasValid := !oldLogIDs.NodeID.IsZero() && !oldLogIDs.DomainID.IsZero() - netLogIDsChanged := netLogIDsNowValid && netLogIDsWasValid && newLogIDs != oldLogIDs - netLogRunning := netLogIDsNowValid && !routerCfg.Equal(&router.Config{}) - if !buildfeatures.HasNetLog || envknob.NoLogsNoSupport() { - netLogRunning = false + + if !engineChanged && !routerChanged && !dnsChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged && !netlogChanged { + return ErrNoChanges } // TODO(bradfitz,danderson): maybe delete this isDNSIPOverTailscale @@ -982,36 +993,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } } - // Shutdown the network logger because the IDs changed. - // Let it be started back up by subsequent logic. - if buildfeatures.HasNetLog && netLogIDsChanged && e.networkLogger.Running() { - e.logf("wgengine: Reconfig: shutting down network logger") - ctx, cancel := context.WithTimeout(context.Background(), networkLoggerUploadTimeout) - defer cancel() - if err := e.networkLogger.Shutdown(ctx); err != nil { - e.logf("wgengine: Reconfig: error shutting down network logger: %v", err) - } - } - - // Startup the network logger. - // Do this before configuring the router so that we capture initial packets. - if buildfeatures.HasNetLog && netLogRunning && !e.networkLogger.Running() { - nid := cfg.NetworkLogging.NodeID - tid := cfg.NetworkLogging.DomainID - logExitFlowEnabled := cfg.NetworkLogging.LogExitFlowEnabled - e.logf("wgengine: Reconfig: starting up network logger (node:%s tailnet:%s)", nid.Public(), tid.Public()) - src := e.netLogSource.Load() - if src == nil { - e.logf("wgengine: Reconfig: no NodeSource installed; network logger not started") - } else if err := e.networkLogger.Startup(e.logf, src, nid, tid, e.tundev, e.magicConn, e.netMon, e.health, e.eventBus, logExitFlowEnabled); err != nil { - e.logf("wgengine: Reconfig: error starting up network logger: %v", err) - } - e.networkLogger.ReconfigRoutes(routerCfg) - } - if routerChanged { e.logf("wgengine: Reconfig: configuring router") - e.networkLogger.ReconfigRoutes(routerCfg) err := e.router.Set(routerCfg) e.health.SetRouterHealth(err) if err != nil { @@ -1050,16 +1033,11 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, } } - // Shutdown the network logger. - // Do this after configuring the router so that we capture final packets. - // This attempts to flush out any log messages and may block. - if !netLogRunning && e.networkLogger.Running() { - e.logf("wgengine: Reconfig: shutting down network logger") - ctx, cancel := context.WithTimeout(context.Background(), networkLoggerUploadTimeout) - defer cancel() - if err := e.networkLogger.Shutdown(ctx); err != nil { - e.logf("wgengine: Reconfig: error shutting down network logger: %v", err) - } + // Let the network flow logger finish reacting after the router is + // configured, so that a stopping logger captures final packets. + // This may block to flush pending log messages. + if e.netlogger != nil { + e.netlogger.ReconfigDone() } if buildfeatures.HasBird && isSubnetRouterChanged && e.birdClient != nil { @@ -1252,10 +1230,8 @@ func (e *userspaceEngine) Close() { } close(e.waitCh) - ctx, cancel := context.WithTimeout(context.Background(), networkLoggerUploadTimeout) - defer cancel() - if err := e.networkLogger.Shutdown(ctx); err != nil { - e.logf("wgengine: Close: error shutting down network logger: %v", err) + if e.netlogger != nil { + e.netlogger.Shutdown() } } diff --git a/wgengine/wgcfg/config.go b/wgengine/wgcfg/config.go index bbaac94db..bd37c19f3 100644 --- a/wgengine/wgcfg/config.go +++ b/wgengine/wgcfg/config.go @@ -9,7 +9,6 @@ "slices" "tailscale.com/types/key" - "tailscale.com/types/logid" ) //go:generate go run tailscale.com/cmd/cloner -type=Config,Peer @@ -20,15 +19,6 @@ type Config struct { PrivateKey key.NodePrivate Addresses []netip.Prefix Peers []Peer - - // NetworkLogging enables network logging. - // It is disabled if either ID is the zero value. - // LogExitFlowEnabled indicates whether or not exit flows should be logged. - NetworkLogging struct { - NodeID logid.PrivateID - DomainID logid.PrivateID - LogExitFlowEnabled bool - } } func (c *Config) Equal(o *Config) bool { @@ -36,7 +26,6 @@ func (c *Config) Equal(o *Config) bool { return c == o } return c.PrivateKey.Equal(o.PrivateKey) && - c.NetworkLogging == o.NetworkLogging && slices.Equal(c.Addresses, o.Addresses) && slices.EqualFunc(c.Peers, o.Peers, Peer.Equal) } diff --git a/wgengine/wgcfg/config_test.go b/wgengine/wgcfg/config_test.go index c19292f1a..fc8fdcbbb 100644 --- a/wgengine/wgcfg/config_test.go +++ b/wgengine/wgcfg/config_test.go @@ -14,8 +14,7 @@ func TestConfigEqual(t *testing.T) { rt := reflect.TypeFor[Config]() for sf := range rt.Fields() { switch sf.Name { - case "Name", "NodeID", "PrivateKey", "Addresses", "Peers", - "NetworkLogging": + case "Name", "NodeID", "PrivateKey", "Addresses", "Peers": // These are compared in [Config.Equal]. default: t.Errorf("Have you added field %q to Config.Equal? Do so if not, and then update TestConfigEqual", sf.Name) diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go index f99b7b007..7f633e4b5 100644 --- a/wgengine/wgcfg/nmcfg/nmcfg.go +++ b/wgengine/wgcfg/nmcfg/nmcfg.go @@ -15,7 +15,6 @@ "tailscale.com/tailcfg" "tailscale.com/types/key" "tailscale.com/types/logger" - "tailscale.com/types/logid" "tailscale.com/types/netmap" "tailscale.com/wgengine/wgcfg" ) @@ -53,27 +52,6 @@ func WGCfg(pk key.NodePrivate, nm *netmap.NetworkMap, logf logger.Logf, flags ne Peers: make([]wgcfg.Peer, 0, len(nm.Peers)), } - // Setup log IDs for data plane audit logging. - if nm.SelfNode.Valid() { - canNetworkLog := nm.SelfNode.HasCap(tailcfg.CapabilityDataPlaneAuditLogs) - logExitFlowEnabled := nm.SelfNode.HasCap(tailcfg.NodeAttrLogExitFlows) - if canNetworkLog && nm.SelfNode.DataPlaneAuditLogID() != "" && nm.DomainAuditLogID != "" { - nodeID, errNode := logid.ParsePrivateID(nm.SelfNode.DataPlaneAuditLogID()) - if errNode != nil { - logf("[v1] wgcfg: unable to parse node audit log ID: %v", errNode) - } - domainID, errDomain := logid.ParsePrivateID(nm.DomainAuditLogID) - if errDomain != nil { - logf("[v1] wgcfg: unable to parse domain audit log ID: %v", errDomain) - } - if errNode == nil && errDomain == nil { - cfg.NetworkLogging.NodeID = nodeID - cfg.NetworkLogging.DomainID = domainID - cfg.NetworkLogging.LogExitFlowEnabled = logExitFlowEnabled - } - } - } - var skippedExitNode, skippedSubnetRouter, skippedExpired []tailcfg.NodeView for _, peer := range nm.Peers { diff --git a/wgengine/wgcfg/wgcfg_clone.go b/wgengine/wgcfg/wgcfg_clone.go index 7fc8c1e3f..32e7e31e5 100644 --- a/wgengine/wgcfg/wgcfg_clone.go +++ b/wgengine/wgcfg/wgcfg_clone.go @@ -9,7 +9,6 @@ "net/netip" "tailscale.com/types/key" - "tailscale.com/types/logid" ) // Clone makes a deep copy of Config. @@ -32,14 +31,9 @@ func (src *Config) Clone() *Config { // A compilation failure here means this code must be regenerated, with the command at the top of this file. var _ConfigCloneNeedsRegeneration = Config(struct { - PrivateKey key.NodePrivate - Addresses []netip.Prefix - Peers []Peer - NetworkLogging struct { - NodeID logid.PrivateID - DomainID logid.PrivateID - LogExitFlowEnabled bool - } + PrivateKey key.NodePrivate + Addresses []netip.Prefix + Peers []Peer }{}) // Clone makes a deep copy of Peer. diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 6b996fe81..7d389d64d 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -16,7 +16,6 @@ "tailscale.com/types/key" "tailscale.com/types/netmap" "tailscale.com/wgengine/filter" - "tailscale.com/wgengine/netlog" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" "tailscale.com/wgengine/wgint" @@ -198,12 +197,13 @@ type Engine interface { // look up which peer should handle an outbound packet by destination IP. SetPeerByIPPacketFunc(func(netip.Addr) (_ key.NodePublic, ok bool)) - // SetNetLogNodeSource installs the [netlog.NodeSource] used by the engine's - // network logger to look up Tailscale node info on demand. + // SetNetLogSource installs the [NetLogSource] consulted by the + // engine's network flow logger for node lookups and the current + // audit logging identity. // // It is expected to be called once during LocalBackend construction, // before any [Engine.Reconfig] call that starts up the network logger. - SetNetLogNodeSource(netlog.NodeSource) + SetNetLogSource(NetLogSource) // SetWGPeerLookup installs the function used by the engine's // wireguard-go log wrapper to rewrite peer references in log lines