diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index c59df833d..214d3a4e4 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3988,6 +3988,12 @@ func (b *LocalBackend) wantIngressLocked() bool { return b.serveConfig.Valid() && b.serveConfig.HasAllowFunnel() } +// hasIngressEnabledLocked reports whether the node has any funnel endpoint enabled. This bool is sent to control (in +// Hostinfo.IngressEnabled) to determine whether 'Funnel' badge should be displayed on this node in the admin panel. +func (b *LocalBackend) hasIngressEnabledLocked() bool { + return b.serveConfig.Valid() && b.serveConfig.IsFunnelOn() +} + // setPrefsLockedOnEntry requires b.mu be held to call it, but it // unlocks b.mu when done. newp ownership passes to this function. // It returns a read-only copy of the new prefs. @@ -5086,7 +5092,12 @@ func (b *LocalBackend) applyPrefsToHostinfoLocked(hi *tailcfg.Hostinfo, prefs ip // if this is accidentally false, then control may not configure DNS // properly. This exists as an optimization to control to program fewer DNS // records that have ingress enabled but are not actually being used. + // TODO(irbekrm): once control knows that if hostinfo.IngressEnabled is true, + // then wireIngress can be considered true, don't send wireIngress in that case. hi.WireIngress = b.wantIngressLocked() + // The Hostinfo.IngressEnabled field is used to communicate to control whether + // the funnel is actually enabled. + hi.IngressEnabled = b.hasIngressEnabledLocked() hi.AppConnector.Set(prefs.AppConnector().Advertise) } @@ -6009,14 +6020,37 @@ func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn. b.updateServeTCPPortNetMapAddrListenersLocked(servePorts) } } - // Kick off a Hostinfo update to control if WireIngress changed. - if wire := b.wantIngressLocked(); b.hostinfo != nil && b.hostinfo.WireIngress != wire { + + // Update funnel info in hostinfo and kick off control update if needed. + b.updateIngressLocked() + b.setTCPPortsIntercepted(handlePorts) +} + +// updateIngressLocked updates the hostinfo.WireIngress and hostinfo.IngressEnabled fields and kicks off a Hostinfo +// update if the values have changed. +// TODO(irbekrm): once control knows that if hostinfo.IngressEnabled is true, then wireIngress can be considered true, +// we can stop sending hostinfo.WireIngress in that case. +// +// b.mu must be held. +func (b *LocalBackend) updateIngressLocked() { + if b.hostinfo == nil { + return + } + hostInfoChanged := false + if wire := b.wantIngressLocked(); b.hostinfo.WireIngress != wire { b.logf("Hostinfo.WireIngress changed to %v", wire) b.hostinfo.WireIngress = wire + hostInfoChanged = true + } + if ie := b.hasIngressEnabledLocked(); b.hostinfo.IngressEnabled != ie { + b.logf("Hostinfo.IngressEnabled changed to %v", ie) + b.hostinfo.IngressEnabled = ie + hostInfoChanged = true + } + // Kick off a Hostinfo update to control if ingress status has changed. + if hostInfoChanged { b.goTracker.Go(b.doSetHostinfoFilterServices) } - - b.setTCPPortsIntercepted(handlePorts) } // setServeProxyHandlersLocked ensures there is an http proxy handler for each diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 5e8a3172c..348bdcab3 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -4838,3 +4838,154 @@ func TestUpdatePrefsOnSysPolicyChange(t *testing.T) { }) } } + +func TestUpdateIngressLocked(t *testing.T) { + tests := []struct { + name string + hi *tailcfg.Hostinfo + sc *ipn.ServeConfig + wantIngress bool + wantWireIngress bool + wantControlUpdate bool + }{ + { + name: "no_hostinfo_no_serve_config", + hi: nil, + }, + { + name: "empty_hostinfo_no_serve_config", + hi: &tailcfg.Hostinfo{}, + }, + { + name: "empty_hostinfo_funnel_enabled", + hi: &tailcfg.Hostinfo{}, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": true, + }, + }, + wantIngress: true, + wantWireIngress: true, + wantControlUpdate: true, + }, + { + name: "empty_hostinfo_funnel_disabled", + hi: &tailcfg.Hostinfo{}, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": false, + }, + }, + wantWireIngress: true, // true if there is any AllowFunnel block + wantControlUpdate: true, + }, + { + name: "empty_hostinfo_no_funnel", + hi: &tailcfg.Hostinfo{}, + sc: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 80: {HTTPS: true}, + }, + }, + }, + { + name: "funnel_enabled_no_change", + hi: &tailcfg.Hostinfo{ + IngressEnabled: true, + WireIngress: true, + }, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": true, + }, + }, + wantIngress: true, + wantWireIngress: true, + }, + { + name: "funnel_disabled_no_change", + hi: &tailcfg.Hostinfo{ + WireIngress: true, + }, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": false, + }, + }, + wantWireIngress: true, // true if there is any AllowFunnel block + }, + { + name: "funnel_changes_to_disabled", + hi: &tailcfg.Hostinfo{ + IngressEnabled: true, + WireIngress: true, + }, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": false, + }, + }, + wantWireIngress: true, // true if there is any AllowFunnel block + wantControlUpdate: true, + }, + { + name: "funnel_changes_to_enabled", + hi: &tailcfg.Hostinfo{ + WireIngress: true, + }, + sc: &ipn.ServeConfig{ + AllowFunnel: map[ipn.HostPort]bool{ + "tailnet.xyz:443": true, + }, + }, + wantWireIngress: true, + wantIngress: true, + wantControlUpdate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b := newTestLocalBackend(t) + b.hostinfo = tt.hi + b.serveConfig = tt.sc.View() + allDone := make(chan bool, 1) + defer b.goTracker.AddDoneCallback(func() { + b.mu.Lock() + defer b.mu.Unlock() + if b.goTracker.RunningGoroutines() > 0 { + return + } + select { + case allDone <- true: + default: + } + })() + + was := b.goTracker.StartedGoroutines() + b.updateIngressLocked() + + if tt.hi != nil { + if tt.hi.IngressEnabled != tt.wantIngress { + t.Errorf("IngressEnabled = %v, want %v", tt.hi.IngressEnabled, tt.wantIngress) + } + if tt.hi.WireIngress != tt.wantWireIngress { + t.Errorf("WireIngress = %v, want %v", tt.hi.WireIngress, tt.wantWireIngress) + } + } + + startedGoroutine := b.goTracker.StartedGoroutines() != was + if startedGoroutine != tt.wantControlUpdate { + t.Errorf("control update triggered = %v, want %v", startedGoroutine, tt.wantControlUpdate) + } + + if startedGoroutine { + select { + case <-time.After(5 * time.Second): + t.Fatal("timed out waiting for goroutine to finish") + case <-allDone: + } + } + }) + } +} diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 9b26e8883..937f619e6 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -155,7 +155,8 @@ type CapabilityVersion int // - 110: 2024-12-12: removed never-before-used Tailscale SSH public key support (#14373) // - 111: 2025-01-14: Client supports a peer having Node.HomeDERP (issue #14636) // - 112: 2025-01-14: Client interprets AllowedIPs of nil as meaning same as Addresses -const CurrentCapabilityVersion CapabilityVersion = 112 +// - 113: 2025-01-20: Client communicates to control whether funnel is enabled by sending Hostinfo.IngressEnabled (#14688) +const CurrentCapabilityVersion CapabilityVersion = 113 // ID is an integer ID for a user, node, or login allocated by the // control plane. @@ -869,6 +870,7 @@ type Hostinfo struct { ShareeNode bool `json:",omitempty"` // indicates this node exists in netmap because it's owned by a shared-to user NoLogsNoSupport bool `json:",omitempty"` // indicates that the user has opted out of sending logs and support WireIngress bool `json:",omitempty"` // indicates that the node wants the option to receive ingress connections + IngressEnabled bool `json:",omitempty"` // if the node has any funnel endpoint enabled AllowsUpdate bool `json:",omitempty"` // indicates that the node has opted-in to admin-console-drive remote updates Machine string `json:",omitempty"` // the current host's machine type (uname -m) GoArch string `json:",omitempty"` // GOARCH value (of the built binary) diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 42cef1598..f7126ca41 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -166,6 +166,7 @@ var _HostinfoCloneNeedsRegeneration = Hostinfo(struct { ShareeNode bool NoLogsNoSupport bool WireIngress bool + IngressEnabled bool AllowsUpdate bool Machine string GoArch string diff --git a/tailcfg/tailcfg_test.go b/tailcfg/tailcfg_test.go index 560e28933..da5873847 100644 --- a/tailcfg/tailcfg_test.go +++ b/tailcfg/tailcfg_test.go @@ -51,6 +51,7 @@ func TestHostinfoEqual(t *testing.T) { "ShareeNode", "NoLogsNoSupport", "WireIngress", + "IngressEnabled", "AllowsUpdate", "Machine", "GoArch", @@ -251,6 +252,26 @@ func TestHostinfoEqual(t *testing.T) { &Hostinfo{}, false, }, + { + &Hostinfo{IngressEnabled: true}, + &Hostinfo{}, + false, + }, + { + &Hostinfo{IngressEnabled: true}, + &Hostinfo{IngressEnabled: true}, + true, + }, + { + &Hostinfo{IngressEnabled: false}, + &Hostinfo{}, + true, + }, + { + &Hostinfo{IngressEnabled: false}, + &Hostinfo{IngressEnabled: true}, + false, + }, } for i, tt := range tests { got := tt.a.Equal(tt.b) diff --git a/tailcfg/tailcfg_view.go b/tailcfg/tailcfg_view.go index 3770f272f..55c244fbf 100644 --- a/tailcfg/tailcfg_view.go +++ b/tailcfg/tailcfg_view.go @@ -283,6 +283,7 @@ func (v HostinfoView) ShieldsUp() bool { return v.ж.Shie func (v HostinfoView) ShareeNode() bool { return v.ж.ShareeNode } func (v HostinfoView) NoLogsNoSupport() bool { return v.ж.NoLogsNoSupport } func (v HostinfoView) WireIngress() bool { return v.ж.WireIngress } +func (v HostinfoView) IngressEnabled() bool { return v.ж.IngressEnabled } func (v HostinfoView) AllowsUpdate() bool { return v.ж.AllowsUpdate } func (v HostinfoView) Machine() string { return v.ж.Machine } func (v HostinfoView) GoArch() string { return v.ж.GoArch } @@ -324,6 +325,7 @@ var _HostinfoViewNeedsRegeneration = Hostinfo(struct { ShareeNode bool NoLogsNoSupport bool WireIngress bool + IngressEnabled bool AllowsUpdate bool Machine string GoArch string