From 354455e8be9ca9d01e969829282616ad863b6b01 Mon Sep 17 00:00:00 2001 From: Maisem Ali Date: Wed, 27 Sep 2023 23:01:09 -0700 Subject: [PATCH] ipn: use NodeCapMap in CheckFunnel These were missed when adding NodeCapMap and resulted in tsnet binaries not being able to turn on funnel. Fixes #9566 Signed-off-by: Maisem Ali --- cmd/derper/depaware.txt | 2 +- cmd/tailscale/cli/funnel.go | 4 +-- ipn/serve.go | 62 +++++++++++++++++++++++++------------ ipn/serve_test.go | 7 ++++- tsnet/tsnet.go | 2 +- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index a147524e1..df46c385a 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -270,7 +270,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa runtime/metrics from github.com/prometheus/client_golang/prometheus+ runtime/pprof from net/http/pprof runtime/trace from net/http/pprof - slices from tailscale.com/ipn+ + slices from tailscale.com/ipn/ipnstate+ sort from compress/flate+ strconv from compress/flate+ strings from bufio+ diff --git a/cmd/tailscale/cli/funnel.go b/cmd/tailscale/cli/funnel.go index 875c5f38e..9c0dea1b8 100644 --- a/cmd/tailscale/cli/funnel.go +++ b/cmd/tailscale/cli/funnel.go @@ -164,12 +164,12 @@ func (e *serveEnv) verifyFunnelEnabled(ctx context.Context, st *ipnstate.Status, // the feature flag on. // TODO(sonia,tailscale/corp#10577): Remove this fallback once the // control flag is turned on for all domains. - if err := ipn.CheckFunnelAccess(port, st.Self.Capabilities); err != nil { + if err := ipn.CheckFunnelAccess(port, st.Self); err != nil { return err } default: // Done with enablement, make sure the requested port is allowed. - if err := ipn.CheckFunnelPort(port, st.Self.Capabilities); err != nil { + if err := ipn.CheckFunnelPort(port, st.Self); err != nil { return err } } diff --git a/ipn/serve.go b/ipn/serve.go index 74975d6b0..b22a5bdb7 100644 --- a/ipn/serve.go +++ b/ipn/serve.go @@ -9,10 +9,10 @@ "net" "net/netip" "net/url" - "slices" "strconv" "strings" + "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" ) @@ -237,23 +237,21 @@ func (sc *ServeConfig) IsFunnelOn() bool { // 2. the node has the "funnel" nodeAttr // 3. the port is allowed for Funnel // -// The nodeAttrs arg should be the node's Self.Capabilities which should contain -// the attribute we're checking for and possibly warning-capabilities for -// Funnel. -func CheckFunnelAccess(port uint16, nodeAttrs []tailcfg.NodeCapability) error { - if !slices.Contains(nodeAttrs, tailcfg.CapabilityHTTPS) { +// The node arg should be the ipnstate.Status.Self node. +func CheckFunnelAccess(port uint16, node *ipnstate.PeerStatus) error { + if !node.HasCap(tailcfg.CapabilityHTTPS) { return errors.New("Funnel not available; HTTPS must be enabled. See https://tailscale.com/s/https.") } - if !slices.Contains(nodeAttrs, tailcfg.NodeAttrFunnel) { + if !node.HasCap(tailcfg.NodeAttrFunnel) { return errors.New("Funnel not available; \"funnel\" node attribute not set. See https://tailscale.com/s/no-funnel.") } - return CheckFunnelPort(port, nodeAttrs) + return CheckFunnelPort(port, node) } // CheckFunnelPort checks whether the given port is allowed for Funnel. // It uses the tailcfg.CapabilityFunnelPorts nodeAttr to determine the allowed // ports. -func CheckFunnelPort(wantedPort uint16, nodeAttrs []tailcfg.NodeCapability) error { +func CheckFunnelPort(wantedPort uint16, node *ipnstate.PeerStatus) error { deny := func(allowedPorts string) error { if allowedPorts == "" { return fmt.Errorf("port %d is not allowed for funnel", wantedPort) @@ -261,24 +259,50 @@ func CheckFunnelPort(wantedPort uint16, nodeAttrs []tailcfg.NodeCapability) erro return fmt.Errorf("port %d is not allowed for funnel; allowed ports are: %v", wantedPort, allowedPorts) } var portsStr string - for _, attr := range nodeAttrs { + parseAttr := func(attr string) (string, error) { + u, err := url.Parse(attr) + if err != nil { + return "", deny("") + } + portsStr := u.Query().Get("ports") + if portsStr == "" { + return "", deny("") + } + u.RawQuery = "" + if u.String() != string(tailcfg.CapabilityFunnelPorts) { + return "", deny("") + } + return portsStr, nil + } + for attr := range node.CapMap { attr := string(attr) if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) { continue } - u, err := url.Parse(attr) + var err error + portsStr, err = parseAttr(attr) if err != nil { - return deny("") + return err } - portsStr = u.Query().Get("ports") - if portsStr == "" { - return deny("") - } - u.RawQuery = "" - if u.String() != string(tailcfg.CapabilityFunnelPorts) { - return deny("") + break + } + if portsStr == "" { + for _, attr := range node.Capabilities { + attr := string(attr) + if !strings.HasPrefix(attr, string(tailcfg.CapabilityFunnelPorts)) { + continue + } + var err error + portsStr, err = parseAttr(attr) + if err != nil { + return err + } + break } } + if portsStr == "" { + return deny("") + } wantedPortString := strconv.Itoa(int(wantedPort)) for _, ps := range strings.Split(portsStr, ",") { if ps == "" { diff --git a/ipn/serve_test.go b/ipn/serve_test.go index 87ae2eba4..2576b2b66 100644 --- a/ipn/serve_test.go +++ b/ipn/serve_test.go @@ -5,6 +5,7 @@ import ( "testing" + "tailscale.com/ipn/ipnstate" "tailscale.com/tailcfg" ) @@ -26,7 +27,11 @@ func TestCheckFunnelAccess(t *testing.T) { {3000, caps(portAttr, tailcfg.CapabilityHTTPS, tailcfg.NodeAttrFunnel), true}, } for _, tt := range tests { - err := CheckFunnelAccess(tt.port, tt.caps) + cm := tailcfg.NodeCapMap{} + for _, c := range tt.caps { + cm[c] = nil + } + err := CheckFunnelAccess(tt.port, &ipnstate.PeerStatus{CapMap: cm}) switch { case err != nil && tt.wantErr, err == nil && !tt.wantErr: diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index f49f2bd93..9e6d34f4c 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -926,7 +926,7 @@ func (s *Server) ListenFunnel(network, addr string, opts ...FunnelOption) (net.L // flow here instead of CheckFunnelAccess to allow the user to turn on Funnel // if not already on. Specifically when running from a terminal. // See cli.serveEnv.verifyFunnelEnabled. - if err := ipn.CheckFunnelAccess(uint16(port), st.Self.Capabilities); err != nil { + if err := ipn.CheckFunnelAccess(uint16(port), st.Self); err != nil { return nil, err }