From 255253881e76a5b0932987aeddc21f489190720d Mon Sep 17 00:00:00 2001 From: Irbe Krumina Date: Fri, 22 Nov 2024 14:34:27 +0000 Subject: [PATCH] cmd/containerboot: Ingress proxies now only advertise an HTTPS endpoint when it's ready L7 Ingress proxies now set a new https_endpoint field to their state Secret when the serve config has been loaded. This gets unset if serve config can not be set. L7 Ingress proxies now attempt to determine if HTTPS is disabled for the tailnet (by looking at cert domains in netmap) and log an error. Signed-off-by: Irbe Krumina --- cmd/containerboot/kube.go | 14 +++++++- cmd/containerboot/main.go | 29 ++++++++++++---- cmd/containerboot/main_test.go | 36 ++++++++++++-------- cmd/containerboot/serve.go | 62 ++++++++++++++++++++++++++++------ 4 files changed, 107 insertions(+), 34 deletions(-) diff --git a/cmd/containerboot/kube.go b/cmd/containerboot/kube.go index c570140f6..2293dd066 100644 --- a/cmd/containerboot/kube.go +++ b/cmd/containerboot/kube.go @@ -74,7 +74,19 @@ func (kc *kubeClient) storeDeviceEndpoints(ctx context.Context, fqdn string, add kubetypes.KeyDeviceIPs: deviceIPs, }, } - return kc.StrategicMergePatchSecret(ctx, secretName, s, "tailscale-container") + return kc.StrategicMergePatchSecret(ctx, kc.stateSecret, s, "tailscale-container") +} + +// storeHTTPSEndpoint writes an HTTPS endpoint exposed by this device via 'tailscale serve' to the named Kubernetes +// Secret. In practice this will be the same value that gets written to 'device_fqdn', but this should only be called +// when the serve config has been successfully set up. +func (kc *kubeClient) storeHTTPSEndpoint(ctx context.Context, ep string) error { + s := &kubeapi.Secret{ + Data: map[string][]byte{ + kubetypes.KeyHTTPSEndpoint: []byte(ep), + }, + } + return kc.StrategicMergePatchSecret(ctx, kc.stateSecret, s, "tailscale-container") } // deleteAuthKey deletes the 'authkey' field of the given kube diff --git a/cmd/containerboot/main.go b/cmd/containerboot/main.go index 7f1340e98..91334b138 100644 --- a/cmd/containerboot/main.go +++ b/cmd/containerboot/main.go @@ -289,12 +289,16 @@ func main() { } } + // Remove any serve config and advertised HTTPS endpoint that may have been set by a previous run of + // containerboot, but only if we're providing a new one. if cfg.ServeConfigPath != "" { - // Remove any serve config that may have been set by a previous run of - // containerboot, but only if we're providing a new one. + log.Printf("serve proxy: unsetting previous config") if err := client.SetServeConfig(ctx, new(ipn.ServeConfig)); err != nil { log.Fatalf("failed to unset serve config: %v", err) } + if err := kc.storeHTTPSEndpoint(ctx, ""); err != nil { + log.Fatalf("failed to update HTTPS endpoint in tailscale state: %v", err) + } } if hasKubeStateStore(cfg) && isTwoStepConfigAuthOnce(cfg) { @@ -334,10 +338,12 @@ func main() { h = &healthz{} // http server for the healthz endpoint healthzRunner = sync.OnceFunc(func() { runHealthz(cfg.HealthCheckAddrPort, h) }) + // triggerWatchServeConfigChanges = sync.OnceFunc(func() { + // go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client, kc) + // }) + triggerWatchServeConfigChanges sync.Once ) - if cfg.ServeConfigPath != "" { - go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client) - } + var nfr linuxfw.NetfilterRunner if isL3Proxy(cfg) { nfr, err = newNetfilterRunner(log.Printf) @@ -511,8 +517,11 @@ func main() { resetTimer(false) backendAddrs = newBackendAddrs } - if cfg.ServeConfigPath != "" && len(n.NetMap.DNS.CertDomains) != 0 { - cd := n.NetMap.DNS.CertDomains[0] + if cfg.ServeConfigPath != "" { + cd := certDomainFromNetmap(n.NetMap) + if cd == "" { + cd = kubetypes.ValueNoHTTPS + } prev := certDomain.Swap(ptr.To(cd)) if prev == nil || *prev != cd { select { @@ -559,6 +568,12 @@ func main() { } } + if cfg.ServeConfigPath != "" { + triggerWatchServeConfigChanges.Do(func() { + go watchServeConfigChanges(ctx, cfg.ServeConfigPath, certDomainChanged, certDomain, client, kc) + }) + } + if cfg.HealthCheckAddrPort != "" { h.Lock() h.hasAddrs = len(addrs) != 0 diff --git a/cmd/containerboot/main_test.go b/cmd/containerboot/main_test.go index 5c92787ce..5d07b2937 100644 --- a/cmd/containerboot/main_test.go +++ b/cmd/containerboot/main_test.go @@ -102,6 +102,8 @@ func TestContainerBoot(t *testing.T) { argFile := filepath.Join(d, "args") runningSockPath := filepath.Join(d, "tmp/tailscaled.sock") + capver := fmt.Sprintf("%d", tailcfg.CurrentCapabilityVersion) + type phase struct { // If non-nil, send this IPN bus notification (and remember it as the // initial update for any future new watchers, then wait for all the @@ -453,10 +455,11 @@ type phase struct { { Notify: runningNotify, WantKubeSecret: map[string]string{ - "authkey": "tskey-key", - "device_fqdn": "test-node.test.ts.net", - "device_id": "myID", - "device_ips": `["100.64.0.1"]`, + "authkey": "tskey-key", + "device_fqdn": "test-node.test.ts.net", + "device_id": "myID", + "device_ips": `["100.64.0.1"]`, + "tailscale_capver": capver, }, }, }, @@ -546,9 +549,10 @@ type phase struct { "/usr/bin/tailscale --socket=/tmp/tailscaled.sock set --accept-dns=false", }, WantKubeSecret: map[string]string{ - "device_fqdn": "test-node.test.ts.net", - "device_id": "myID", - "device_ips": `["100.64.0.1"]`, + "device_fqdn": "test-node.test.ts.net", + "device_id": "myID", + "device_ips": `["100.64.0.1"]`, + "tailscale_capver": capver, }, }, }, @@ -575,10 +579,11 @@ type phase struct { { Notify: runningNotify, WantKubeSecret: map[string]string{ - "authkey": "tskey-key", - "device_fqdn": "test-node.test.ts.net", - "device_id": "myID", - "device_ips": `["100.64.0.1"]`, + "authkey": "tskey-key", + "device_fqdn": "test-node.test.ts.net", + "device_id": "myID", + "device_ips": `["100.64.0.1"]`, + "tailscale_capver": capver, }, }, { @@ -593,10 +598,11 @@ type phase struct { }, }, WantKubeSecret: map[string]string{ - "authkey": "tskey-key", - "device_fqdn": "new-name.test.ts.net", - "device_id": "newID", - "device_ips": `["100.64.0.1"]`, + "authkey": "tskey-key", + "device_fqdn": "new-name.test.ts.net", + "device_id": "newID", + "device_ips": `["100.64.0.1"]`, + "tailscale_capver": capver, }, }, }, diff --git a/cmd/containerboot/serve.go b/cmd/containerboot/serve.go index 6c22b3eeb..39fb7f9f2 100644 --- a/cmd/containerboot/serve.go +++ b/cmd/containerboot/serve.go @@ -19,6 +19,8 @@ "github.com/fsnotify/fsnotify" "tailscale.com/client/tailscale" "tailscale.com/ipn" + "tailscale.com/kube/kubetypes" + "tailscale.com/types/netmap" ) // watchServeConfigChanges watches path for changes, and when it sees one, reads @@ -26,21 +28,21 @@ // applies it to lc. It exits when ctx is canceled. cdChanged is a channel that // is written to when the certDomain changes, causing the serve config to be // re-read and applied. -func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan bool, certDomainAtomic *atomic.Pointer[string], lc *tailscale.LocalClient) { +func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan bool, certDomainAtomic *atomic.Pointer[string], lc *tailscale.LocalClient, kc *kubeClient) { if certDomainAtomic == nil { - panic("cd must not be nil") + panic("certDomainAtomic must not be nil") } var tickChan <-chan time.Time var eventChan <-chan fsnotify.Event if w, err := fsnotify.NewWatcher(); err != nil { - log.Printf("failed to create fsnotify watcher, timer-only mode: %v", err) + log.Printf("serve proxy: failed to create fsnotify watcher, timer-only mode: %v", err) ticker := time.NewTicker(5 * time.Second) defer ticker.Stop() tickChan = ticker.C } else { defer w.Close() if err := w.Add(filepath.Dir(path)); err != nil { - log.Fatalf("failed to add fsnotify watch: %v", err) + log.Fatalf("serve proxy: failed to add fsnotify watch: %v", err) } eventChan = w.Events } @@ -59,24 +61,62 @@ func watchServeConfigChanges(ctx context.Context, path string, cdChanged <-chan // k8s handles these mounts. So just re-read the file and apply it // if it's changed. } - if certDomain == "" { - continue - } sc, err := readServeConfig(path, certDomain) if err != nil { - log.Fatalf("failed to read serve config: %v", err) + log.Fatalf("serve proxy: failed to read serve config: %v", err) } if prevServeConfig != nil && reflect.DeepEqual(sc, prevServeConfig) { continue } - log.Printf("Applying serve config") - if err := lc.SetServeConfig(ctx, sc); err != nil { - log.Fatalf("failed to set serve config: %v", err) + validateHTTPSServe(certDomain, sc) + if err := updateServeConfig(ctx, sc, certDomain, kc, lc); err != nil { + log.Fatalf("serve proxy: error updating serve config: %v", err) } prevServeConfig = sc } } +func certDomainFromNetmap(nm *netmap.NetworkMap) string { + if len(nm.DNS.CertDomains) == 0 { + return "" + } + return nm.DNS.CertDomains[0] +} + +func updateServeConfig(ctx context.Context, sc *ipn.ServeConfig, certDomain string, kc *kubeClient, lc *tailscale.LocalClient) error { + defer func() { + if err := kc.storeHTTPSEndpoint(ctx, certDomain); err != nil { + log.Printf("[unexpected]: serve proxy: error storing HTTPS endpoint: %v", err) + } + }() + // TODO(irbekrm): This means that serve config that does not expose HTTPS endpoint will not be set for a tailnet + // that does not have HTTPS enabled. We probably want to fix this. + if certDomain == kubetypes.ValueNoHTTPS { + return nil + } + log.Printf("serve proxy: applying serve config") + return lc.SetServeConfig(ctx, sc) +} + +func validateHTTPSServe(certDomain string, sc *ipn.ServeConfig) { + if certDomain != kubetypes.ValueNoHTTPS || !hasHTTPSEndpoint(sc) { + return + } + log.Printf( + `serve proxy: this node is configured as a proxy that exposes an HTTPS endpoint to tailnet, + (perhaps a Kubernetes operator Ingress proxy) but it is not able to issue TLS certs, so this will likely not work. + To make it work, ensure that HTTPS is enabled for you tailnet, see https://tailscale.com/kb/1153/enabling-https or this will not work.`) +} + +func hasHTTPSEndpoint(cfg *ipn.ServeConfig) bool { + for _, tcpCfg := range cfg.TCP { + if tcpCfg.HTTPS { + return true + } + } + return false +} + // readServeConfig reads the ipn.ServeConfig from path, replacing // ${TS_CERT_DOMAIN} with certDomain. func readServeConfig(path, certDomain string) (*ipn.ServeConfig, error) {