From 10d130b845743c39a7c40421471fc47fb6b79153 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 21 Feb 2024 16:44:11 -0800 Subject: [PATCH] cmd/derper, derp, tailcfg: add admission controller URL option So derpers can check an external URL for whether to permit access to a certain public key. Updates tailscale/corp#17693 Change-Id: I8594de58f54a08be3e2dbef8bcd1ff9b728ab297 Co-authored-by: Maisem Ali Signed-off-by: Brad Fitzpatrick --- cmd/derper/derper.go | 14 ++++--- derp/derp_server.go | 96 +++++++++++++++++++++++++++++++++++--------- tailcfg/derpmap.go | 21 +++++++++- 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/cmd/derper/derper.go b/cmd/derper/derper.go index 04622ec89..f126c5d6c 100644 --- a/cmd/derper/derper.go +++ b/cmd/derper/derper.go @@ -49,11 +49,13 @@ var ( runSTUN = flag.Bool("stun", true, "whether to run a STUN server. It will bind to the same IP (if any) as the --addr flag value.") runDERP = flag.Bool("derp", true, "whether to run a DERP server. The only reason to set this false is if you're decommissioning a server but want to keep its bootstrap DNS functionality still running.") - meshPSKFile = flag.String("mesh-psk-file", defaultMeshPSKFile(), "if non-empty, path to file containing the mesh pre-shared key file. It should contain some hex string; whitespace is trimmed.") - meshWith = flag.String("mesh-with", "", "optional comma-separated list of hostnames to mesh with; the server's own hostname can be in the list") - bootstrapDNS = flag.String("bootstrap-dns-names", "", "optional comma-separated list of hostnames to make available at /bootstrap-dns") - unpublishedDNS = flag.String("unpublished-bootstrap-dns-names", "", "optional comma-separated list of hostnames to make available at /bootstrap-dns and not publish in the list") - verifyClients = flag.Bool("verify-clients", false, "verify clients to this DERP server through a local tailscaled instance.") + meshPSKFile = flag.String("mesh-psk-file", defaultMeshPSKFile(), "if non-empty, path to file containing the mesh pre-shared key file. It should contain some hex string; whitespace is trimmed.") + meshWith = flag.String("mesh-with", "", "optional comma-separated list of hostnames to mesh with; the server's own hostname can be in the list") + bootstrapDNS = flag.String("bootstrap-dns-names", "", "optional comma-separated list of hostnames to make available at /bootstrap-dns") + unpublishedDNS = flag.String("unpublished-bootstrap-dns-names", "", "optional comma-separated list of hostnames to make available at /bootstrap-dns and not publish in the list") + verifyClients = flag.Bool("verify-clients", false, "verify clients to this DERP server through a local tailscaled instance.") + verifyClientURL = flag.String("verify-client-url", "", "if non-empty, an admission controller URL for permitting client connections; see tailcfg.DERPAdmitClientRequest") + verifyFailOpen = flag.Bool("verify-client-url-fail-open", true, "whether we fail open if --verify-client-url is unreachable") acceptConnLimit = flag.Float64("accept-connection-limit", math.Inf(+1), "rate limit for accepting new connection") acceptConnBurst = flag.Int("accept-connection-burst", math.MaxInt, "burst limit for accepting new connection") @@ -147,6 +149,8 @@ func main() { s := derp.NewServer(cfg.PrivateKey, log.Printf) s.SetVerifyClient(*verifyClients) + s.SetVerifyClientURL(*verifyClientURL) + s.SetVerifyClientURLFailOpen(*verifyFailOpen) if *meshPSKFile != "" { b, err := os.ReadFile(*meshPSKFile) diff --git a/derp/derp_server.go b/derp/derp_server.go index b36669c8c..1a03012a4 100644 --- a/derp/derp_server.go +++ b/derp/derp_server.go @@ -7,6 +7,7 @@ package derp import ( "bufio" + "bytes" "context" "crypto/ed25519" crand "crypto/rand" @@ -40,6 +41,7 @@ import ( "tailscale.com/envknob" "tailscale.com/metrics" "tailscale.com/syncs" + "tailscale.com/tailcfg" "tailscale.com/tstime" "tailscale.com/tstime/rate" "tailscale.com/types/key" @@ -144,9 +146,13 @@ type Server struct { avgQueueDuration *uint64 // In milliseconds; accessed atomically tcpRtt metrics.LabelMap // histogram - // verifyClients only accepts client connections to the DERP server if the clientKey is a - // known peer in the network, as specified by a running tailscaled's client's LocalAPI. - verifyClients bool + // verifyClientsLocalTailscaled only accepts client connections to the DERP + // server if the clientKey is a known peer in the network, as specified by a + // running tailscaled's client's LocalAPI. + verifyClientsLocalTailscaled bool + + verifyClientsURL string + verifyClientsURLFailOpen bool mu sync.Mutex closed bool @@ -353,7 +359,20 @@ func (s *Server) SetMeshKey(v string) { // // It must be called before serving begins. func (s *Server) SetVerifyClient(v bool) { - s.verifyClients = v + s.verifyClientsLocalTailscaled = v +} + +// SetVerifyClientURL sets the admission controller URL to use for verifying clients. +// If empty, all clients are accepted (unless restricted by SetVerifyClient checking +// against tailscaled). +func (s *Server) SetVerifyClientURL(v string) { + s.verifyClientsURL = v +} + +// SetVerifyClientURLFailOpen sets whether to allow clients to connect if the +// admission controller URL is unreachable. +func (s *Server) SetVerifyClientURLFailOpen(v bool) { + s.verifyClientsURLFailOpen = v } // HasMeshKey reports whether the server is configured with a mesh key. @@ -691,7 +710,9 @@ func (s *Server) accept(ctx context.Context, nc Conn, brw *bufio.ReadWriter, rem if err != nil { return fmt.Errorf("receive client key: %v", err) } - if err := s.verifyClient(ctx, clientKey, clientInfo); err != nil { + + clientAP, _ := netip.ParseAddrPort(remoteAddr) + if err := s.verifyClient(ctx, clientKey, clientInfo, clientAP.Addr()); err != nil { return fmt.Errorf("client %x rejected: %v", clientKey, err) } @@ -1116,21 +1137,60 @@ func (c *sclient) requestMeshUpdate() { } } -func (s *Server) verifyClient(ctx context.Context, clientKey key.NodePublic, info *clientInfo) error { - if !s.verifyClients { - return nil +// verifyClient checks whether the client is allowed to connect to the derper, +// depending on how & whether the server's been configured to verify. +func (s *Server) verifyClient(ctx context.Context, clientKey key.NodePublic, info *clientInfo, clientIP netip.Addr) error { + // tailscaled-based verification: + if s.verifyClientsLocalTailscaled { + status, err := tailscale.Status(ctx) + if err != nil { + return fmt.Errorf("failed to query local tailscaled status: %w", err) + } + if clientKey == status.Self.PublicKey { + return nil + } + if _, exists := status.Peer[clientKey]; !exists { + return fmt.Errorf("client %v not in set of peers", clientKey) + } } - status, err := tailscale.Status(ctx) - if err != nil { - return fmt.Errorf("failed to query local tailscaled status: %w", err) + + // admission controller-based verification: + if s.verifyClientsURL != "" { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + jreq, err := json.Marshal(&tailcfg.DERPAdmitClientRequest{ + NodePublic: clientKey, + Source: clientIP, + }) + if err != nil { + return err + } + req, err := http.NewRequestWithContext(ctx, "POST", s.verifyClientsURL, bytes.NewReader(jreq)) + if err != nil { + return err + } + res, err := http.DefaultClient.Do(req) + if err != nil { + if s.verifyClientsURLFailOpen { + s.logf("admission controller unreachable; allowing client %v", clientKey) + return nil + } + return err + } + defer res.Body.Close() + if res.StatusCode != 200 { + return fmt.Errorf("admission controller: %v", res.Status) + } + var jres tailcfg.DERPAdmitClientResponse + if err := json.NewDecoder(io.LimitReader(res.Body, 4<<10)).Decode(&jres); err != nil { + return err + } + if !jres.Allow { + return fmt.Errorf("admission controller: %v/%v not allowed", clientKey, clientIP) + } + // TODO(bradfitz): add policy for configurable bandwidth rate per client? } - if clientKey == status.Self.PublicKey { - return nil - } - if _, exists := status.Peer[clientKey]; !exists { - return fmt.Errorf("client %v not in set of peers", clientKey) - } - // TODO(bradfitz): add policy for configurable bandwidth rate per client? return nil } diff --git a/tailcfg/derpmap.go b/tailcfg/derpmap.go index d95d26d57..5d050a37d 100644 --- a/tailcfg/derpmap.go +++ b/tailcfg/derpmap.go @@ -3,7 +3,12 @@ package tailcfg -import "sort" +import ( + "net/netip" + "sort" + + "tailscale.com/types/key" +) // DERPMap describes the set of DERP packet relay servers that are available. type DERPMap struct { @@ -176,3 +181,17 @@ type DERPNode struct { // DotInvalid is a fake DNS TLD used in tests for an invalid hostname. const DotInvalid = ".invalid" + +// DERPAdmitClientRequest is the JSON request body of a POST to derper's +// --verify-client-url admission controller URL. +type DERPAdmitClientRequest struct { + NodePublic key.NodePublic // key to query for admission + Source netip.Addr // derp client's IP address +} + +// DERPAdmitClientResponse is the response to a DERPAdmitClientRequest. +type DERPAdmitClientResponse struct { + Allow bool // whether to permit client + + // TODO(bradfitz,maisem): bandwidth limits, etc? +}