diff --git a/cmd/tailscale/cli/cert.go b/cmd/tailscale/cli/cert.go index 6d78a8d8a..bab83901f 100644 --- a/cmd/tailscale/cli/cert.go +++ b/cmd/tailscale/cli/cert.go @@ -23,7 +23,10 @@ import ( "github.com/peterbourgon/ff/v3/ffcli" "software.sslmate.com/src/go-pkcs12" "tailscale.com/atomicfile" + "tailscale.com/feature/buildfeatures" + "tailscale.com/health" "tailscale.com/ipn" + "tailscale.com/tsconst" "tailscale.com/version" ) @@ -112,6 +115,11 @@ func runCert(ctx context.Context, args []string) error { certArgs.certFile = fileBase + ".crt" certArgs.keyFile = fileBase + ".key" } + if buildfeatures.HasHealth { + watchCtx, cancel := context.WithCancel(ctx) + defer cancel() + go watchCertPendingHealth(watchCtx, domain) + } certPEM, keyPEM, err := localClient.CertPairWithValidity(ctx, domain, certArgs.minValidity) if err != nil { return err @@ -167,6 +175,44 @@ func runCert(ctx context.Context, args []string) error { return nil } +// watchCertPendingHealth subscribes to the IPN bus and prints the +// [tsconst.HealthWarnableTLSCertPending] warning to stderr if it appears +// for domain while a cert fetch is in flight. It returns once it has +// printed the warning or ctx is done. +// +// Subscription is delayed 1 second so we don't print anything when the +// daemon returns a cached cert quickly. +func watchCertPendingHealth(ctx context.Context, domain string) { + select { + case <-time.After(1 * time.Second): + case <-ctx.Done(): + return + } + watcher, err := localClient.WatchIPNBus(ctx, ipn.NotifyInitialHealthState|ipn.NotifyNoNetMap) + if err != nil { + return + } + defer watcher.Close() + for { + n, err := watcher.Next() + if err != nil { + return + } + if n.Health == nil { + continue + } + ws, ok := n.Health.Warnings[tsconst.HealthWarnableTLSCertPending] + if !ok { + continue + } + if !strings.Contains(ws.Args[health.ArgDomains], domain) { + continue + } + fmt.Fprintf(os.Stderr, "%s: %s\n", ws.Title, ws.Text) + return + } +} + func writeIfChanged(filename string, contents []byte, mode os.FileMode) (changed bool, err error) { if filename == "-" { Stdout.Write(contents) diff --git a/health/args.go b/health/args.go index e89f7676f..5f606bdf4 100644 --- a/health/args.go +++ b/health/args.go @@ -36,4 +36,8 @@ const ( // ArgServerName provides a Warnable with comma delimited list of the hostname of the servers involved in the unhealthy state. // If no nameservers were available to query, this will be an empty string. ArgDNSServers Arg = "dns-servers" + + // ArgDomains provides a Warnable with a comma-delimited list of domain + // names involved in the unhealthy state. + ArgDomains Arg = "domains" ) diff --git a/ipn/ipnlocal/cert.go b/ipn/ipnlocal/cert.go index ef4cdf728..5e6034654 100644 --- a/ipn/ipnlocal/cert.go +++ b/ipn/ipnlocal/cert.go @@ -35,6 +35,7 @@ import ( "tailscale.com/atomicfile" "tailscale.com/envknob" "tailscale.com/feature/buildfeatures" + "tailscale.com/health" "tailscale.com/hostinfo" "tailscale.com/ipn" "tailscale.com/ipn/store" @@ -43,8 +44,11 @@ import ( "tailscale.com/syncs" "tailscale.com/tailcfg" "tailscale.com/tempfork/acme" + "tailscale.com/tsconst" "tailscale.com/types/logger" "tailscale.com/util/clientmetric" + "tailscale.com/util/set" + "tailscale.com/util/slicesx" "tailscale.com/util/testenv" "tailscale.com/version" "tailscale.com/version/distro" @@ -52,6 +56,7 @@ import ( func init() { RegisterC2N("GET /tls-cert-status", handleC2NTLSCertStatus) + hookCertRefreshLoop.Set(certRefreshLoop) } // Process-wide cache. (A new *Handler is created per connection, @@ -75,6 +80,18 @@ var ( metricACMETLSALPN01Failure = clientmetric.NewCounter("cert_acme_tls_alpn01_failure") ) +// certPendingWarnable fires while ACME is fetching a TLS certificate for +// which no usable cached copy exists (initial issuance or after the cached +// cert has expired). Async renewal of a still-valid cert does not fire it. +var certPendingWarnable = health.Register(&health.Warnable{ + Code: tsconst.HealthWarnableTLSCertPending, + Title: "Fetching TLS certificate", + Severity: health.SeverityLow, + Text: func(args health.Args) string { + return fmt.Sprintf("Fetching TLS certificate via ACME for: %s", args[health.ArgDomains]) + }, +}) + type acmeChallengeType string const ( @@ -660,6 +677,16 @@ var getCertPEM = func(ctx context.Context, b *LocalBackend, cs certStore, logf l return nil, err } + // If we have no usable cached cert (either nothing on disk, or what is + // on disk has expired or otherwise failed verification), surface a + // health warning to the user for the duration of the ACME flow. We + // don't fire the warning when previous is non-nil because then we have + // a working cert and the renewal is happening behind the scenes. + if previous == nil { + b.setCertPending(domain, true) + defer b.setCertPending(domain, false) + } + ac, err := acmeClient(cs) if err != nil { return nil, err @@ -1207,3 +1234,119 @@ func handleC2NTLSCertStatus(b *LocalBackend, w http.ResponseWriter, r *http.Requ writeJSON(w, ret) } + +// setCertPending sets or clears the in-flight ACME issuance state for +// domain and updates the [certPendingWarnable] to reflect the current set +// of pending domains. +func (b *LocalBackend) setCertPending(domain string, pending bool) { + b.pendingCertDomainsMu.Lock() + defer b.pendingCertDomainsMu.Unlock() + if pending { + b.pendingCertDomains.Make() + b.pendingCertDomains.Add(domain) + } else { + b.pendingCertDomains.Delete(domain) + } + if b.pendingCertDomains.Len() == 0 { + b.health.SetHealthy(certPendingWarnable) + return + } + b.health.SetUnhealthy(certPendingWarnable, health.Args{ + health.ArgDomains: joinedPendingCertDomainsLocked(b.pendingCertDomains), + }) +} + +func joinedPendingCertDomainsLocked(s set.Set[string]) string { + ds := slicesx.MapKeys(s) + slices.Sort(ds) + return strings.Join(ds, ", ") +} + +// certRefreshInterval is how often the background loop iterates the set of +// applicable cert domains and pokes the renewal machinery. The loop is +// only started while there's at least one HTTPS Web entry in the +// ServeConfig, so this cadence doesn't tick on idle/mobile nodes. +const certRefreshInterval = time.Hour + +// certRefreshLoop periodically iterates the domains configured for Serve or +// Funnel HTTPS and calls GetCertPEM on each. The existing renewal machinery +// in getCertPEM decides whether anything needs to happen (ARI check or +// expiry-based fallback); the loop just ensures it runs even on nodes that +// see no inbound TLS traffic. +// +// The first iteration runs immediately so that a node coming back online +// with stale or absent certs starts ACME within seconds rather than +// waiting a full interval. +// +// Set as [hookCertRefreshLoop] in cert.go's init. +func certRefreshLoop(b *LocalBackend, ctx context.Context) { + if envknob.IsCertShareReadOnlyMode() { + b.logf("cert refresh loop: cert-share read-only mode; loop is a no-op") + return + } + + b.refreshApplicableCerts(ctx) + + ticker, tickerCh := b.clock.NewTicker(certRefreshInterval) + defer ticker.Stop() + for { + select { + case <-tickerCh: + b.refreshApplicableCerts(ctx) + case <-ctx.Done(): + return + } + } +} + +// refreshApplicableCerts is one iteration of the cert refresh loop. +// +// It enumerates the Serve/Funnel-configured HTTPS hostnames, keeps those +// that [LocalBackend.resolveCertDomain] accepts (CertDomain, wildcard, or +// BYO Funnel domain), and calls [LocalBackend.GetCertPEM] for each. The +// renewal decision is delegated to the existing logic in [getCertPEM]. +func (b *LocalBackend) refreshApplicableCerts(ctx context.Context) { + sc := b.ServeConfig() + if !sc.Valid() { + return + } + + want := set.Set[string]{} + consider := func(host string) { + if host == "" { + return + } + if _, err := b.resolveCertDomain(host); err != nil { + return + } + want.Add(host) + } + for hp := range sc.Webs() { + host, _, err := net.SplitHostPort(string(hp)) + if err != nil { + continue + } + consider(host) + } + for _, tcp := range sc.TCPs() { + consider(tcp.TerminateTLS()) + } + for _, svc := range sc.Services().All() { + for _, tcp := range svc.TCP().All() { + consider(tcp.TerminateTLS()) + } + } + if want.Len() == 0 { + return + } + + for d := range want { + b.goTracker.Go(func() { + ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) + defer cancel() + if _, err := b.GetCertPEM(ctx, d); err != nil { + b.logf("cert refresh: %s: %v", d, err) + } + }) + } +} diff --git a/ipn/ipnlocal/cert_test.go b/ipn/ipnlocal/cert_test.go index af39ea0bc..224ba834d 100644 --- a/ipn/ipnlocal/cert_test.go +++ b/ipn/ipnlocal/cert_test.go @@ -15,6 +15,7 @@ import ( "crypto/x509/pkix" "embed" "encoding/pem" + "maps" "math/big" "os" "path/filepath" @@ -24,10 +25,12 @@ import ( "github.com/google/go-cmp/cmp" "tailscale.com/envknob" + "tailscale.com/health" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" "tailscale.com/tempfork/acme" + "tailscale.com/tsconst" "tailscale.com/tstest" "tailscale.com/types/logger" "tailscale.com/types/netmap" @@ -848,3 +851,172 @@ func TestGetCertPEMWithValidity(t *testing.T) { }) } } + +func TestCertPendingWarnable(t *testing.T) { + b := newTestLocalBackend(t) + + // currentWarning returns the pending warning's rendered text and + // domain-list arg, or "", "" if the warnable is currently healthy. + currentWarning := func() (text, domains string) { + ws, ok := b.health.CurrentState().Warnings[tsconst.HealthWarnableTLSCertPending] + if !ok { + return "", "" + } + return ws.Text, ws.Args[health.ArgDomains] + } + + if b.health.IsUnhealthy(certPendingWarnable) { + t.Fatal("warnable unexpectedly unhealthy before any setCertPending") + } + + b.setCertPending("a.example.com", true) + if !b.health.IsUnhealthy(certPendingWarnable) { + t.Fatal("warnable not unhealthy after first setCertPending") + } + if text, domains := currentWarning(); domains != "a.example.com" || + text != "Fetching TLS certificate via ACME for: a.example.com" { + t.Errorf("after first setCertPending: text=%q domains=%q", text, domains) + } + + b.setCertPending("b.example.com", true) + if !b.health.IsUnhealthy(certPendingWarnable) { + t.Fatal("warnable not unhealthy after second setCertPending") + } + if text, domains := currentWarning(); domains != "a.example.com, b.example.com" || + text != "Fetching TLS certificate via ACME for: a.example.com, b.example.com" { + t.Errorf("after second setCertPending: text=%q domains=%q", text, domains) + } + + b.setCertPending("a.example.com", false) + if !b.health.IsUnhealthy(certPendingWarnable) { + t.Fatal("warnable cleared too early; one domain still pending") + } + if text, domains := currentWarning(); domains != "b.example.com" || + text != "Fetching TLS certificate via ACME for: b.example.com" { + t.Errorf("after clearing a.example.com: text=%q domains=%q", text, domains) + } + + b.setCertPending("b.example.com", false) + if b.health.IsUnhealthy(certPendingWarnable) { + t.Fatal("warnable still unhealthy after clearing all domains") + } + if text, domains := currentWarning(); text != "" || domains != "" { + t.Errorf("after clearing all domains: text=%q domains=%q", text, domains) + } +} + +func TestServeConfigUsesACMECerts(t *testing.T) { + tests := []struct { + name string + sc *ipn.ServeConfig + want bool + }{ + {"nil", nil, false}, + {"empty", &ipn.ServeConfig{}, false}, + { + name: "background_web", + sc: &ipn.ServeConfig{ + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + "node.ts.net:443": {}, + }, + }, + want: true, + }, + { + name: "tcp_forward_no_tls", + sc: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{443: {TCPForward: "127.0.0.1:443"}}, + }, + want: false, + }, + { + name: "tls_terminated_tcp", + sc: &ipn.ServeConfig{ + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {TCPForward: "127.0.0.1:443", TerminateTLS: "node.ts.net"}, + }, + }, + want: true, + }, + { + name: "service_tls_terminated_tcp", + sc: &ipn.ServeConfig{ + Services: map[tailcfg.ServiceName]*ipn.ServiceConfig{ + "svc:web": { + TCP: map[uint16]*ipn.TCPPortHandler{ + 443: {TCPForward: "127.0.0.1:443", TerminateTLS: "web.svc.ts.net"}, + }, + }, + }, + }, + want: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var v ipn.ServeConfigView + if tt.sc != nil { + v = tt.sc.View() + } + if got := serveConfigUsesACMECerts(v); got != tt.want { + t.Errorf("serveConfigUsesACMECerts = %v, want %v", got, tt.want) + } + }) + } +} + +func TestRefreshApplicableCerts(t *testing.T) { + const ( + certDomain = "node1.example.com" + byoDomain = "byo.example.org" + ) + b := newTestLocalBackend(t) + b.varRoot = t.TempDir() + + b.mu.Lock() + b.currentNode().SetNetMap(&netmap.NetworkMap{ + SelfNode: (&tailcfg.Node{}).View(), + DNS: tailcfg.DNSConfig{ + CertDomains: []string{certDomain}, + }, + }) + b.serveConfig = (&ipn.ServeConfig{ + Web: map[ipn.HostPort]*ipn.WebServerConfig{ + ipn.HostPort(certDomain + ":443"): {}, + ipn.HostPort(byoDomain + ":443"): {}, + // Not in CertDomains and no Funnel entry; must be filtered out. + ipn.HostPort("not-ours.other.tld:443"): {}, + }, + AllowFunnel: map[ipn.HostPort]bool{ + ipn.HostPort(byoDomain + ":443"): true, + }, + }).View() + b.mu.Unlock() + + gotCh := make(chan string, 4) + b.ConfigureCertsForTest(func(host string) (*TLSCertKeyPair, error) { + gotCh <- host + return &TLSCertKeyPair{}, nil + }) + + b.refreshApplicableCerts(context.Background()) + + want := set.Of(certDomain, byoDomain) + got := set.Set[string]{} + for got.Len() < want.Len() { + select { + case h := <-gotCh: + got.Add(h) + case <-time.After(5 * time.Second): + t.Fatalf("timed out waiting for refresh workers; got %v, want %v", got, want) + } + } + if !maps.Equal(got, want) { + t.Errorf("got fetches %v, want %v", got, want) + } + select { + case h := <-gotCh: + t.Errorf("unexpected extra fetch for %q", h) + default: + } +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index ac423bde2..31ca7f941 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -422,6 +422,18 @@ type LocalBackend struct { // (sending false). needsCaptiveDetection chan bool + // certRefreshCancel cancels the background TLS cert refresh loop that + // periodically pokes [LocalBackend.GetCertPEM] so renewals happen on + // idle nodes. It is protected by mu and is non-nil while the loop is + // running. + certRefreshCancel context.CancelFunc + + // pendingCertDomains tracks the set of domains for which an ACME + // issuance is currently in flight with no usable cached cert. It backs + // the tls-cert-pending health Warnable. Guarded by pendingCertDomainsMu. + pendingCertDomainsMu sync.Mutex + pendingCertDomains set.Set[string] + // overrideAlwaysOn is whether [pkey.AlwaysOn] is overridden by the user // and should have no impact on the WantRunning state until the policy changes, // or the user re-connects manually, switches to a different profile, etc. @@ -1176,6 +1188,11 @@ var ( hookCheckCaptivePortalLoop feature.Hook[func(*LocalBackend, context.Context)] ) +// hookCertRefreshLoop is set by the ACME-enabled cert code to a function +// that periodically refreshes TLS certs for Serve/Funnel-configured +// domains so renewals proceed even on otherwise-idle nodes. +var hookCertRefreshLoop feature.Hook[func(*LocalBackend, context.Context)] + func (b *LocalBackend) onHealthChange(change health.Change) { if !buildfeatures.HasHealth { return @@ -1274,6 +1291,11 @@ func (b *LocalBackend) Shutdown() { b.captiveCancel() } + if buildfeatures.HasACME && b.certRefreshCancel != nil { + b.certRefreshCancel() + b.certRefreshCancel = nil + } + b.stopReconnectTimerLocked() if b.loginFlags&controlclient.LoginEphemeral != 0 { @@ -6444,6 +6466,11 @@ func (b *LocalBackend) enterStateLocked(newState ipn.State) { b.goTracker.Go(func() { hookCheckCaptivePortalLoop.Get()(b, captiveCtx) }) } } + + // (Re)evaluate the background TLS cert refresh loop. It runs + // only while we're Running and ServeConfig has at least one + // HTTPS Web entry, so idle nodes don't hold a timer open. + b.updateCertRefreshLoopLocked() } else if oldState == ipn.Running { // Transitioning away from running. b.closePeerAPIListenersLocked() @@ -6457,6 +6484,8 @@ func (b *LocalBackend) enterStateLocked(newState ipn.State) { // that we always have a (canceled) context to wait on // in onHealthChange. } + + b.updateCertRefreshLoopLocked() } b.pauseOrResumeControlClientLocked() @@ -7188,6 +7217,65 @@ func (b *LocalBackend) setTCPPortsInterceptedFromNetmapAndPrefsLocked(prefs ipn. // The LocalBackend's mutex is held while calling. var hookMaybeMutateHostinfoLocked feature.Hooks[func(*LocalBackend, *tailcfg.Hostinfo, ipn.PrefsView) bool] +// updateCertRefreshLoopLocked starts or stops the background TLS cert +// refresh loop based on whether we currently have any HTTPS-serving +// hostname whose cert we should keep fresh. The loop runs only while: +// +// - ACME support is compiled in, +// - the node is in [ipn.Running], and +// - the current [ipn.ServeConfig] has at least one HTTPS Web entry. +// +// We deliberately don't keep an idle timer around on hosts that have no +// certs to maintain (e.g. mobile devices that never run Serve), so this +// must be called whenever any of those inputs change: state transitions +// and ServeConfig reloads. +// +// b.mu must be held. +func (b *LocalBackend) updateCertRefreshLoopLocked() { + if !buildfeatures.HasACME { + return + } + shouldRun := hookCertRefreshLoop.IsSet() && + b.state == ipn.Running && + serveConfigUsesACMECerts(b.serveConfig) + + switch { + case shouldRun && b.certRefreshCancel == nil: + ctx, cancel := context.WithCancel(b.ctx) + b.certRefreshCancel = cancel + b.goTracker.Go(func() { hookCertRefreshLoop.Get()(b, ctx) }) + case !shouldRun && b.certRefreshCancel != nil: + b.certRefreshCancel() + b.certRefreshCancel = nil + } +} + +// serveConfigUsesACMECerts reports whether sc has any entry that +// causes tailscaled to obtain ACME-managed TLS certs: an HTTPS Web +// entry (background, foreground, or service) or a TCP handler with +// TerminateTLS set (`tailscale serve --tls-terminated-tcp`). +func serveConfigUsesACMECerts(sc ipn.ServeConfigView) bool { + if !sc.Valid() { + return false + } + for range sc.Webs() { + return true + } + for _, tcp := range sc.TCPs() { + if tcp.TerminateTLS() != "" { + return true + } + } + for _, svc := range sc.Services().All() { + for _, tcp := range svc.TCP().All() { + if tcp.TerminateTLS() != "" { + return true + } + } + } + return false +} + // maybeSentHostinfoIfChangedLocked updates the hostinfo.ServicesHash, hostinfo.WireIngress and // hostinfo.IngressEnabled fields and kicks off a Hostinfo update if the values have changed. // diff --git a/ipn/ipnlocal/serve.go b/ipn/ipnlocal/serve.go index 58fa17eef..3a7db7939 100644 --- a/ipn/ipnlocal/serve.go +++ b/ipn/ipnlocal/serve.go @@ -1544,6 +1544,7 @@ func (b *LocalBackend) reloadServeConfigLocked(prefs ipn.PrefsView) { if err != nil { b.lastServeConfJSON = mem.B(nil) b.serveConfig = ipn.ServeConfigView{} + b.updateCertRefreshLoopLocked() return } if b.lastServeConfJSON.Equal(mem.B(confj)) { @@ -1554,6 +1555,7 @@ func (b *LocalBackend) reloadServeConfigLocked(prefs ipn.PrefsView) { if err := json.Unmarshal(confj, &conf); err != nil { b.logf("invalid ServeConfig %q in StateStore: %v", confKey, err) b.serveConfig = ipn.ServeConfigView{} + b.updateCertRefreshLoopLocked() return } @@ -1564,6 +1566,7 @@ func (b *LocalBackend) reloadServeConfigLocked(prefs ipn.PrefsView) { }) b.serveConfig = conf.View() + b.updateCertRefreshLoopLocked() } func (b *LocalBackend) setVIPServicesTCPPortsInterceptedLocked(svcPorts map[tailcfg.ServiceName][]uint16) { diff --git a/tsconst/health.go b/tsconst/health.go index 93c6550ef..dbcb8d29a 100644 --- a/tsconst/health.go +++ b/tsconst/health.go @@ -23,4 +23,5 @@ const ( HealthWarnableTestWarnable = "test-warnable" HealthWarnableApplyDiskConfig = "apply-disk-config" HealthWarnableWarmingUp = "warming-up" + HealthWarnableTLSCertPending = "tls-cert-pending" )