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 <maisem@tailscale.com>
This commit is contained in:
Maisem Ali 2023-09-27 23:01:09 -07:00 committed by Maisem Ali
parent 5c2b2fa1f8
commit 354455e8be
5 changed files with 53 additions and 24 deletions

View File

@ -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+

View File

@ -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
}
}

View File

@ -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 == "" {

View File

@ -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:

View File

@ -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
}