Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ipn/ipnlocal, control/controlclient: keep map poll alive on NoLogsNoSupport + flow logs #11825

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ipn/ipnlocal, control/controlclient: keep map poll alive on NoLogsNoSupport + flow logs #11825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions control/controlclient/map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,16 @@ type mapSession struct {
lastTKAInfo *tailcfg.TKAInfo
lastNetmapSummary string // from NetworkMap.VeryConcise
lastMaxExpiry time.Duration

// breakPeersForFlowLogs is whether we've detected a situation
// that's incompatible with flow logs: the tailnet admin has enabled
// network flow logs, but the local machine has been configured to
// not log. In this case, to be conservative, we drop all peers
// but keep the map poll connection open. This way, the admin can
// still get back into the machine later if they first disable flow
// logs. But we otherwise disable connectivity over Tailscale.
// See https://github.com/tailscale/tailscale/issues/11793
breakPeersForFlowLogs bool
}

// newMapSession returns a mostly unconfigured new mapSession.
Expand Down Expand Up @@ -181,6 +191,8 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t
}
}
ms.controlKnobs.UpdateFromNodeAttributes(resp.Node.CapMap)

ms.breakPeersForFlowLogs = envknob.NoLogsNoSupport() && resp.Node.CapMap.Contains(tailcfg.CapabilityDataPlaneAuditLogs)
}

// Call Node.InitDisplayNames on any changed nodes.
Expand Down Expand Up @@ -219,6 +231,9 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t
}

func (ms *mapSession) tryHandleIncrementally(res *tailcfg.MapResponse) bool {
if ms.breakPeersForFlowLogs {
return false
}
if ms.controlKnobs != nil && ms.controlKnobs.DisableDeltaUpdates.Load() {
return false
}
Expand Down Expand Up @@ -805,6 +820,10 @@ func (ms *mapSession) netmap() *netmap.NetworkMap {
MaxKeyDuration: ms.lastMaxExpiry,
}

if ms.breakPeersForFlowLogs {
nm.Peers = nil
}

if ms.lastTKAInfo != nil && ms.lastTKAInfo.Head != "" {
if err := nm.TKAHead.UnmarshalText([]byte(ms.lastTKAInfo.Head)); err != nil {
ms.logf("error unmarshalling TKAHead: %v", err)
Expand Down
19 changes: 6 additions & 13 deletions ipn/ipnlocal/local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,6 +1013,8 @@ func (b *LocalBackend) peerCapsLocked(src netip.Addr) tailcfg.PeerCapMap {
return nil
}

var noLogsNoSupportNotifyOnce sync.Once

// SetControlClientStatus is the callback invoked by the control client whenever it posts a new status.
// Among other things, this is where we update the netmap, packet filters, DNS and DERP maps.
func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st controlclient.Status) {
Expand Down Expand Up @@ -1216,20 +1218,11 @@ func (b *LocalBackend) SetControlClientStatus(c controlclient.Client, st control

if st.NetMap != nil {
if envknob.NoLogsNoSupport() && st.NetMap.HasCap(tailcfg.CapabilityDataPlaneAuditLogs) {
msg := "tailnet requires logging to be enabled. Remove --no-logs-no-support from tailscaled command line."
msg := "tailnet requires logging to be enabled. Remove --no-logs-no-support from the tailscaled command line."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this message get surfaced in the admin panel UI?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, in theory. That's what the following line should be doing indirectly. It sets health as unhappy on the device and nodes should be telling control about their health state. But we don't do much with that server-side yet.

health.SetLocalLogConfigHealth(errors.New(msg))
// Connecting to this tailnet without logging is forbidden; boot us outta here.
b.mu.Lock()
prefs.WantRunning = false
p := prefs.View()
if err := b.pm.SetPrefs(p, ipn.NetworkProfile{
MagicDNSName: st.NetMap.MagicDNSSuffix(),
DomainName: st.NetMap.DomainName(),
}); err != nil {
b.logf("Failed to save new controlclient state: %v", err)
}
b.mu.Unlock()
b.send(ipn.Notify{ErrMessage: &msg, Prefs: &p})
noLogsNoSupportNotifyOnce.Do(func() {
b.send(ipn.Notify{ErrMessage: ptr.To(msg)})
})
return
}
if netMap != nil {
Expand Down
Loading