diff --git a/cmd/k8s-operator/deploy/chart/templates/deployment.yaml b/cmd/k8s-operator/deploy/chart/templates/deployment.yaml index 8f835be39..d8c3803d5 100644 --- a/cmd/k8s-operator/deploy/chart/templates/deployment.yaml +++ b/cmd/k8s-operator/deploy/chart/templates/deployment.yaml @@ -124,6 +124,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.uid + - name: OPERATOR_SHARED_ACME_ACCOUNT_KEY + value: {{ .Values.operatorConfig.sharedACMEAccountKey | quote }} {{- with .Values.operatorConfig.extraEnv }} {{- toYaml . | nindent 12 }} {{- end }} diff --git a/cmd/k8s-operator/deploy/chart/values.yaml b/cmd/k8s-operator/deploy/chart/values.yaml index f5591e856..6cb0628dd 100644 --- a/cmd/k8s-operator/deploy/chart/values.yaml +++ b/cmd/k8s-operator/deploy/chart/values.yaml @@ -87,6 +87,13 @@ operatorConfig: # - name: EXTRA_VAR2 # value: "value2" + # Default for the tailscale.com/share-acme-account annotation on new + # ProxyGroups. When true, the operator provisions a shared per-tailnet + # ACME account key Secret and configures proxies to use it, preserving + # Let's Encrypt's ARI "replaces" renewal exemption across pod restarts + # and ProxyGroup recreation. See #18251. + sharedACMEAccountKey: false + # In the case that you already have a tailscale ingressclass in your cluster (or vcluster), you can disable the creation here ingressClass: # Allows for customization of the ingress class name used by the operator to identify ingresses to reconcile. This does diff --git a/cmd/k8s-operator/deploy/manifests/operator.yaml b/cmd/k8s-operator/deploy/manifests/operator.yaml index 1dfbf7126..03502be4c 100644 --- a/cmd/k8s-operator/deploy/manifests/operator.yaml +++ b/cmd/k8s-operator/deploy/manifests/operator.yaml @@ -6874,6 +6874,8 @@ spec: valueFrom: fieldRef: fieldPath: metadata.uid + - name: OPERATOR_SHARED_ACME_ACCOUNT_KEY + value: "false" image: tailscale/k8s-operator:stable imagePullPolicy: Always name: operator diff --git a/cmd/k8s-operator/operator.go b/cmd/k8s-operator/operator.go index 377651810..1663046f4 100644 --- a/cmd/k8s-operator/operator.go +++ b/cmd/k8s-operator/operator.go @@ -96,6 +96,7 @@ func main() { tsFirewallMode = defaultEnv("PROXY_FIREWALL_MODE", "") defaultProxyClass = defaultEnv("PROXY_DEFAULT_CLASS", "") isDefaultLoadBalancer = defaultBool("OPERATOR_DEFAULT_LOAD_BALANCER", false) + sharedACMEAccountKey = defaultBool("OPERATOR_SHARED_ACME_ACCOUNT_KEY", false) loginServer = strings.TrimSuffix(defaultEnv("OPERATOR_LOGIN_SERVER", ""), "/") ingressClassName = defaultEnv("OPERATOR_INGRESS_CLASS_NAME", "tailscale") operatorSAName = defaultEnv("OPERATOR_SERVICE_ACCOUNT_NAME", "operator") @@ -170,6 +171,7 @@ func main() { defaultProxyClass: defaultProxyClass, loginServer: loginServer, ingressClassName: ingressClassName, + sharedACMEAccountKey: sharedACMEAccountKey, }) } @@ -752,6 +754,7 @@ func runReconcilers(opts reconcilerOpts) { proxyClassFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(proxyClassHandlerForProxyGroup(mgr.GetClient(), startlog)) nodeFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(nodeHandlerForProxyGroup(mgr.GetClient(), opts.defaultProxyClass, startlog)) saFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(serviceAccountHandlerForProxyGroup(mgr.GetClient(), startlog)) + acmeSecretFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(acmeAccountsSecretHandlerForProxyGroup(mgr.GetClient(), opts.tailscaleNamespace, opts.sharedACMEAccountKey, startlog)) err = builder.ControllerManagedBy(mgr). For(&tsapi.ProxyGroup{}). Named("proxygroup-reconciler"). @@ -760,6 +763,9 @@ func runReconcilers(opts reconcilerOpts) { Watches(&corev1.ConfigMap{}, ownedByProxyGroupFilter). Watches(&corev1.ServiceAccount{}, saFilterForProxyGroup). Watches(&corev1.Secret{}, ownedByProxyGroupFilter). + // The shared ACME accounts Secret has no ProxyGroup owner ref, so + // watch it by name to react to its deletion/recreation. + Watches(&corev1.Secret{}, acmeSecretFilterForProxyGroup). Watches(&rbacv1.Role{}, ownedByProxyGroupFilter). Watches(&rbacv1.RoleBinding{}, ownedByProxyGroupFilter). Watches(&tsapi.ProxyClass{}, proxyClassFilterForProxyGroup). @@ -780,6 +786,8 @@ func runReconcilers(opts reconcilerOpts) { loginServer: opts.tsServer.ControlURL, authKeyRateLimits: make(map[string]*rate.Limiter), authKeyReissuing: make(map[string]bool), + + sharedACMEAccountKey: opts.sharedACMEAccountKey, }) if err != nil { startlog.Fatalf("could not create ProxyGroup reconciler: %v", err) @@ -835,6 +843,13 @@ type reconcilerOpts struct { // ingressClassName is the name of the ingress class used by reconcilers of Ingress resources. This defaults // to "tailscale" but can be customised. ingressClassName string + // sharedACMEAccountKey is the operator-wide default for the + // shared-ACME-account feature. When true, every ProxyGroup uses the + // shared per-tailnet account key unless the ProxyGroup explicitly + // opts out via tailscale.com/share-acme-account=false. When false, + // ProxyGroups opt in individually via + // tailscale.com/share-acme-account=true. + sharedACMEAccountKey bool // operatorSAName is the name of the ServiceAccount that the operator pod runs as. It is used as the target // ServiceAccount when minting tokens via the Kubernetes TokenRequest API for Tailnets that authenticate using // workload identity federation. @@ -1230,6 +1245,30 @@ func serviceAccountHandlerForProxyGroup(cl client.Client, logger *zap.SugaredLog } } +// acmeAccountsSecretHandlerForProxyGroup enqueues ProxyGroups that use the +// shared ACME account when the shared ACME accounts Secret changes. The +// Secret carries no owner reference, so the owner-based Secret watch never +// matches it. +func acmeAccountsSecretHandlerForProxyGroup(cl client.Client, tsNamespace string, sharedACMEAccountDefault bool, logger *zap.SugaredLogger) handler.MapFunc { + return func(ctx context.Context, o client.Object) []reconcile.Request { + if o.GetName() != kubetypes.ACMEAccountsSecretName || o.GetNamespace() != tsNamespace { + return nil + } + pgList := new(tsapi.ProxyGroupList) + if err := cl.List(ctx, pgList); err != nil { + logger.Debugf("error listing ProxyGroups for shared ACME accounts Secret: %v", err) + return nil + } + reqs := make([]reconcile.Request, 0, len(pgList.Items)) + for _, pg := range pgList.Items { + if sharedACMEAccountEnabled(&pg, sharedACMEAccountDefault) { + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&pg)}) + } + } + return reqs + } +} + // serviceHandlerForIngress returns a handler for Service events for ingress // reconciler that ensures that if the Service associated with an event is of // interest to the reconciler, the associated Ingress(es) gets be reconciled. diff --git a/cmd/k8s-operator/proxygroup.go b/cmd/k8s-operator/proxygroup.go index c4da525dd..b4955d7af 100644 --- a/cmd/k8s-operator/proxygroup.go +++ b/cmd/k8s-operator/proxygroup.go @@ -56,6 +56,7 @@ reasonProxyGroupCreating = "ProxyGroupCreating" reasonProxyGroupInvalid = "ProxyGroupInvalid" reasonProxyGroupTailnetUnavailable = "ProxyGroupTailnetUnavailable" + reasonACMEAccountsPendingDeletion = "ACMEAccountsPendingDeletion" // Copied from k8s.io/apiserver/pkg/registry/generic/registry/store.go@cccad306d649184bf2a0e319ba830c53f65c445c optimisticLockErrorMsg = "the object has been modified; please apply your changes to the latest version and try again" @@ -102,6 +103,14 @@ type ProxyGroupReconciler struct { apiServerProxyGroups set.Slice[types.UID] // for kube-apiserver proxygroups gauge authKeyRateLimits map[string]*rate.Limiter // per-ProxyGroup rate limiters for auth key re-issuance. authKeyReissuing map[string]bool + + // sharedACMEAccountKey is the operator-wide default for the + // shared-ACME-account feature. When true, every ProxyGroup uses the + // shared per-tailnet account key unless the ProxyGroup explicitly + // opts out via tailscale.com/share-acme-account=false. When false, + // only ProxyGroups annotated with tailscale.com/share-acme-account=true + // use it. + sharedACMEAccountKey bool } func (r *ProxyGroupReconciler) logger(name string) *zap.SugaredLogger { @@ -354,7 +363,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, tsClient tscl } } - role := pgRole(pg, r.tsNamespace) + role := pgRole(pg, r.tsNamespace, r.sharedACMEAccountEnabledFor(pg)) if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, role, func(r *rbacv1.Role) { r.ObjectMeta.Labels = role.ObjectMeta.Labels r.ObjectMeta.Annotations = role.ObjectMeta.Annotations @@ -394,13 +403,36 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, tsClient tscl }); err != nil { return r.notReadyErrf(pg, logger, "error provisioning ingress ConfigMap %q: %w", cm.Name, err) } + + // Ensure the shared ACME accounts Secret exists (with finalizer) + // when this ProxyGroup opts into the feature. Proxy pods + // populate its fields on first cert issuance. See #18251. + if r.sharedACMEAccountEnabledFor(pg) { + acmeSecret := pgACMEAccountSecret(r.tsNamespace) + if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, acmeSecret, func(existing *corev1.Secret) { + if !existing.DeletionTimestamp.IsZero() { + // Deletion can't be undone; warn so the account keys + // get backed up before the finalizer is removed. + msg := fmt.Sprintf("shared ACME accounts Secret %q is marked for deletion but retained by the %q finalizer. Its data remains readable until the finalizer is removed - back it up first to preserve the ACME account keys.", existing.Name, kubetypes.ACMEAccountsFinalizer) + r.recorder.Event(existing, corev1.EventTypeWarning, reasonACMEAccountsPendingDeletion, msg) + logger.Warn(msg) + return + } + existing.Labels = acmeSecret.Labels + if !slices.Contains(existing.Finalizers, kubetypes.ACMEAccountsFinalizer) { + existing.Finalizers = append(existing.Finalizers, kubetypes.ACMEAccountsFinalizer) + } + }); err != nil { + return r.notReadyErrf(pg, logger, "error provisioning shared ACME accounts Secret %q: %w", acmeSecret.Name, err) + } + } } defaultImage := r.tsProxyImage if pg.Spec.Type == tsapi.ProxyGroupTypeKubernetesAPIServer { defaultImage = r.k8sProxyImage } - ss, err := pgStatefulSet(pg, r.tsNamespace, defaultImage, r.tsFirewallMode, tailscaledPort, proxyClass) + ss, err := pgStatefulSet(pg, r.tsNamespace, defaultImage, r.tsFirewallMode, tailscaledPort, proxyClass, r.sharedACMEAccountEnabledFor(pg)) if err != nil { return r.notReadyErrf(pg, logger, "error generating StatefulSet spec: %w", err) } @@ -1347,6 +1379,25 @@ func notReady(reason, msg string) (map[string][]netip.AddrPort, *notReadyReason, }, nil } +// sharedACMEAccountEnabledFor reports whether the shared-ACME-account +// feature should be applied to pg. The per-PG +// tailscale.com/share-acme-account annotation wins when set; otherwise +// the operator's OPERATOR_SHARED_ACME_ACCOUNT_KEY setting is the default +// for every ProxyGroup. +func (r *ProxyGroupReconciler) sharedACMEAccountEnabledFor(pg *tsapi.ProxyGroup) bool { + return sharedACMEAccountEnabled(pg, r.sharedACMEAccountKey) +} + +// sharedACMEAccountEnabled reports whether pg should use the shared ACME +// account, with the tailscale.com/share-acme-account annotation overriding +// the operator-wide default. +func sharedACMEAccountEnabled(pg *tsapi.ProxyGroup, operatorDefault bool) bool { + if v, ok := pg.Annotations[AnnotationShareACMEAccount]; ok { + return v == "true" + } + return operatorDefault +} + func (r *ProxyGroupReconciler) notReadyErrf(pg *tsapi.ProxyGroup, logger *zap.SugaredLogger, format string, a ...any) (map[string][]netip.AddrPort, *notReadyReason, error) { err := fmt.Errorf(format, a...) if strings.Contains(err.Error(), optimisticLockErrorMsg) { diff --git a/cmd/k8s-operator/proxygroup_specs.go b/cmd/k8s-operator/proxygroup_specs.go index cc54656bb..fc9b532b5 100644 --- a/cmd/k8s-operator/proxygroup_specs.go +++ b/cmd/k8s-operator/proxygroup_specs.go @@ -64,8 +64,12 @@ func pgNodePortService(pg *tsapi.ProxyGroup, name string, namespace string) *cor } // Returns the base StatefulSet definition for a ProxyGroup. A ProxyClass may be -// applied over the top after. -func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, tsFirewallMode string, port *uint16, proxyClass *tsapi.ProxyClass) (*appsv1.StatefulSet, error) { +// applied over the top after. shareACMEAccount, when true, injects the env +// vars that route the pod's ACME account key to the shared per-tailnet +// Secret and drops TS_DEBUG_ACME_FORCE_RENEWAL so ARI-based renewals are +// attempted; the caller is responsible for checking the operator setting +// and the PG opt-in annotation. +func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, tsFirewallMode string, port *uint16, proxyClass *tsapi.ProxyClass, shareACMEAccount bool) (*appsv1.StatefulSet, error) { if pg.Spec.Type == tsapi.ProxyGroupTypeKubernetesAPIServer { return kubeAPIServerStatefulSet(pg, namespace, image, port) } @@ -187,14 +191,6 @@ func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, tsFirewallMode string Name: "TS_EXPERIMENTAL_VERSIONED_CONFIG_DIR", Value: "/etc/tsconfig/$(POD_NAME)", }, - { - // This ensures that cert renewals can succeed if ACME account - // keys have changed since issuance. We cannot guarantee or - // validate that the account key has not changed, see - // https://github.com/tailscale/tailscale/issues/18251 - Name: "TS_DEBUG_ACME_FORCE_RENEWAL", - Value: "true", - }, } if port != nil { @@ -252,6 +248,29 @@ func pgStatefulSet(pg *tsapi.ProxyGroup, namespace, image, tsFirewallMode string Value: "true", }, ) + if shareACMEAccount { + envs = append(envs, + corev1.EnvVar{ + Name: "TS_ACME_ACCOUNT_SECRET_NAME", + Value: kubetypes.ACMEAccountsSecretName, + }, + corev1.EnvVar{ + Name: "TS_ACME_ACCOUNT_FIELD", + Value: pgACMEAccountField(pg), + }, + ) + } else { + // Without a shared account key we cannot guarantee that + // the account key that issued the previous cert is the + // same one attempting renewal. Force plain new-order flow + // so renewals do not silently fail on rejected ARI + // "replaces" claims. See + // https://github.com/tailscale/tailscale/issues/18251. + envs = append(envs, corev1.EnvVar{ + Name: "TS_DEBUG_ACME_FORCE_RENEWAL", + Value: "true", + }) + } } return append(c.Env, envs...) }() @@ -407,7 +426,7 @@ func pgServiceAccount(pg *tsapi.ProxyGroup, namespace string) *corev1.ServiceAcc } } -func pgRole(pg *tsapi.ProxyGroup, namespace string) *rbacv1.Role { +func pgRole(pg *tsapi.ProxyGroup, namespace string, shareACMEAccount bool) *rbacv1.Role { return &rbacv1.Role{ ObjectMeta: metav1.ObjectMeta{ Name: pg.Name, @@ -439,6 +458,12 @@ func pgRole(pg *tsapi.ProxyGroup, namespace string) *rbacv1.Role { pgPodName(pg.Name, i), // State. ) } + // Ingress ProxyGroup write replicas need access to the + // shared ACME account Secret so they can read the + // per-tailnet account key and write it on first use. + if pg.Spec.Type == tsapi.ProxyGroupTypeIngress && shareACMEAccount { + secrets = append(secrets, kubetypes.ACMEAccountsSecretName) + } return secrets }(), }, @@ -477,6 +502,35 @@ func pgRoleBinding(pg *tsapi.ProxyGroup, namespace string) *rbacv1.RoleBinding { } } +// pgACMEAccountField returns the field name used inside the shared +// tailscale-acme-accounts Secret for this ProxyGroup's tailnet. The blank +// tailnet (operator-default credentials) is represented by a reserved +// identifier so it gets a stable, unique field. +func pgACMEAccountField(pg *tsapi.ProxyGroup) string { + tn := pg.Spec.Tailnet + if tn == "" { + tn = kubetypes.ACMEAccountDefaultKey + } + return tn + kubetypes.ACMEAccountKeySuffix +} + +// pgACMEAccountSecret returns the shared per-tailnet ACME account key +// Secret, keyed by tailnet inside its data. Not owned by any ProxyGroup +// so it outlives ProxyGroup deletion. +func pgACMEAccountSecret(namespace string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: kubetypes.ACMEAccountsSecretName, + Namespace: namespace, + Labels: map[string]string{ + kubetypes.LabelManaged: "true", + }, + // Block accidental deletion. + Finalizers: []string{kubetypes.ACMEAccountsFinalizer}, + }, + } +} + // kube-apiserver proxies in auth mode use a static ServiceAccount. Everything // else uses a per-ProxyGroup ServiceAccount. func pgServiceAccountName(pg *tsapi.ProxyGroup) string { diff --git a/cmd/k8s-operator/proxygroup_test.go b/cmd/k8s-operator/proxygroup_test.go index 5432df940..ebd452559 100644 --- a/cmd/k8s-operator/proxygroup_test.go +++ b/cmd/k8s-operator/proxygroup_test.go @@ -1136,14 +1136,15 @@ func TestProxyGroupTypes(t *testing.T) { zl, _ := zap.NewDevelopment() reconciler := &ProxyGroupReconciler{ - tsNamespace: tsNamespace, - tsProxyImage: testProxyImage, - Client: fc, - log: zl.Sugar(), - clients: tsclient.NewProvider(&fakeTSClient{}), - clock: tstest.NewClock(tstest.ClockOpts{}), - authKeyRateLimits: make(map[string]*rate.Limiter), - authKeyReissuing: make(map[string]bool), + tsNamespace: tsNamespace, + tsProxyImage: testProxyImage, + Client: fc, + log: zl.Sugar(), + clients: tsclient.NewProvider(&fakeTSClient{}), + clock: tstest.NewClock(tstest.ClockOpts{}), + authKeyRateLimits: make(map[string]*rate.Limiter), + authKeyReissuing: make(map[string]bool), + sharedACMEAccountKey: true, } t.Run("egress_type", func(t *testing.T) { @@ -1263,6 +1264,9 @@ func TestProxyGroupTypes(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "test-ingress", UID: "test-ingress-uid", + Annotations: map[string]string{ + AnnotationShareACMEAccount: "true", + }, }, Spec: tsapi.ProxyGroupSpec{ Type: tsapi.ProxyGroupTypeIngress, @@ -1283,6 +1287,44 @@ func TestProxyGroupTypes(t *testing.T) { verifyEnvVar(t, sts, "TS_INTERNAL_APP", kubetypes.AppProxyGroupIngress) verifyEnvVar(t, sts, "TS_SERVE_CONFIG", "/etc/proxies/serve-config.json") verifyEnvVar(t, sts, "TS_EXPERIMENTAL_CERT_SHARE", "true") + verifyEnvVar(t, sts, "TS_ACME_ACCOUNT_SECRET_NAME", kubetypes.ACMEAccountsSecretName) + // pg.Spec.Tailnet is empty here so the default tailnet field is used. + verifyEnvVar(t, sts, "TS_ACME_ACCOUNT_FIELD", kubetypes.ACMEAccountDefaultKey+kubetypes.ACMEAccountKeySuffix) + // TS_DEBUG_ACME_FORCE_RENEWAL must NOT be set when the PG is + // opted in to the shared ACME account. + for _, e := range sts.Spec.Template.Spec.Containers[0].Env { + if e.Name == "TS_DEBUG_ACME_FORCE_RENEWAL" { + t.Errorf("TS_DEBUG_ACME_FORCE_RENEWAL must not be set on ingress ProxyGroup pods that share an ACME account") + } + } + + // Verify the shared ACME accounts Secret exists and has the + // deletion finalizer (see tailscale/tailscale#18251). + acmeSecret := &corev1.Secret{} + if err := fc.Get(t.Context(), client.ObjectKey{Namespace: tsNamespace, Name: kubetypes.ACMEAccountsSecretName}, acmeSecret); err != nil { + t.Errorf("failed to get shared ACME accounts Secret: %v", err) + } + if !slices.Contains(acmeSecret.Finalizers, kubetypes.ACMEAccountsFinalizer) { + t.Errorf("shared ACME accounts Secret missing finalizer %q (got %v)", kubetypes.ACMEAccountsFinalizer, acmeSecret.Finalizers) + } + + // Verify the per-ProxyGroup Role grants access to the shared + // ACME accounts Secret (write replicas need it to read/write the + // per-tailnet account key). + role := &rbacv1.Role{} + if err := fc.Get(t.Context(), client.ObjectKey{Namespace: tsNamespace, Name: pg.Name}, role); err != nil { + t.Fatalf("failed to get ProxyGroup Role: %v", err) + } + var sawACMEAccess bool + for _, rule := range role.Rules { + if slices.Contains(rule.Verbs, "patch") && slices.Contains(rule.ResourceNames, kubetypes.ACMEAccountsSecretName) { + sawACMEAccess = true + break + } + } + if !sawACMEAccess { + t.Errorf("ProxyGroup Role does not grant patch access to %q", kubetypes.ACMEAccountsSecretName) + } // Verify ConfigMap volume mount cmName := fmt.Sprintf("%s-ingress-config", pg.Name) @@ -1312,6 +1354,60 @@ func TestProxyGroupTypes(t *testing.T) { } }) + t.Run("ingress_type_shared_acme_opt_out", func(t *testing.T) { + // The reconciler has sharedACMEAccountKey=true, so ingress PGs + // default to shared. Explicit tailscale.com/share-acme-account=false + // must opt this PG out: no shared-Secret env vars, no Role + // access to the shared Secret, and TS_DEBUG_ACME_FORCE_RENEWAL + // must still be set so ARI "replaces" doesn't silently fail. + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress-optout", + UID: "test-ingress-optout-uid", + Annotations: map[string]string{ + AnnotationShareACMEAccount: "false", + }, + }, + Spec: tsapi.ProxyGroupSpec{ + Type: tsapi.ProxyGroupTypeIngress, + Replicas: new(int32(0)), + }, + } + if err := fc.Create(t.Context(), pg); err != nil { + t.Fatal(err) + } + expectReconciled(t, reconciler, "", pg.Name) + + sts := &appsv1.StatefulSet{} + if err := fc.Get(t.Context(), client.ObjectKey{Namespace: tsNamespace, Name: pg.Name}, sts); err != nil { + t.Fatalf("failed to get StatefulSet: %v", err) + } + for _, e := range sts.Spec.Template.Spec.Containers[0].Env { + switch e.Name { + case "TS_ACME_ACCOUNT_SECRET_NAME", "TS_ACME_ACCOUNT_FIELD": + t.Errorf("env %q unexpectedly present on opt-out PG", e.Name) + } + } + var sawForceRenewal bool + for _, e := range sts.Spec.Template.Spec.Containers[0].Env { + if e.Name == "TS_DEBUG_ACME_FORCE_RENEWAL" { + sawForceRenewal = true + } + } + if !sawForceRenewal { + t.Errorf("TS_DEBUG_ACME_FORCE_RENEWAL must be set on opt-out PG (avoids silent ARI \"replaces\" rejection)") + } + role := &rbacv1.Role{} + if err := fc.Get(t.Context(), client.ObjectKey{Namespace: tsNamespace, Name: pg.Name}, role); err != nil { + t.Fatalf("failed to get ProxyGroup Role: %v", err) + } + for _, rule := range role.Rules { + if slices.Contains(rule.ResourceNames, kubetypes.ACMEAccountsSecretName) { + t.Errorf("opt-out PG Role must not grant access to %q", kubetypes.ACMEAccountsSecretName) + } + } + }) + t.Run("kubernetes_api_server_type", func(t *testing.T) { pg := &tsapi.ProxyGroup{ ObjectMeta: metav1.ObjectMeta{ @@ -1331,7 +1427,7 @@ func TestProxyGroupTypes(t *testing.T) { } expectReconciled(t, reconciler, "", pg.Name) - verifyProxyGroupCounts(t, reconciler, 1, 2, 1) + verifyProxyGroupCounts(t, reconciler, 2, 2, 1) sts := &appsv1.StatefulSet{} if err := fc.Get(t.Context(), client.ObjectKey{Namespace: tsNamespace, Name: pg.Name}, sts); err != nil { @@ -2036,10 +2132,11 @@ func verifyEnvVarNotPresent(t *testing.T, sts *appsv1.StatefulSet, name string) func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.ProxyGroup, shouldExist bool, proxyClass *tsapi.ProxyClass) { t.Helper() - role := pgRole(pg, tsNamespace) + shareACMEAccount := pg.Annotations[AnnotationShareACMEAccount] == "true" + role := pgRole(pg, tsNamespace, shareACMEAccount) roleBinding := pgRoleBinding(pg, tsNamespace) serviceAccount := pgServiceAccount(pg, tsNamespace) - statefulSet, err := pgStatefulSet(pg, tsNamespace, testProxyImage, "auto", nil, proxyClass) + statefulSet, err := pgStatefulSet(pg, tsNamespace, testProxyImage, "auto", nil, proxyClass, shareACMEAccount) if err != nil { t.Fatal(err) } diff --git a/cmd/k8s-operator/sts.go b/cmd/k8s-operator/sts.go index f1e10240b..f25f83185 100644 --- a/cmd/k8s-operator/sts.go +++ b/cmd/k8s-operator/sts.go @@ -68,6 +68,12 @@ AnnotationProxyGroup = "tailscale.com/proxy-group" + // AnnotationShareACMEAccount opts a single ProxyGroup into ("true") + // or out of ("false") using the shared per-tailnet ACME account key. + // When absent, OPERATOR_SHARED_ACME_ACCOUNT_KEY on the operator is + // the default. See tailscale/tailscale#18251. + AnnotationShareACMEAccount = "tailscale.com/share-acme-account" + // Annotations settable by users on ingresses. AnnotationFunnel = "tailscale.com/funnel" AnnotationHTTPRedirect = "tailscale.com/http-redirect" diff --git a/feature/acme/cert.go b/feature/acme/cert.go index 130a8d171..0d9b7408e 100644 --- a/feature/acme/cert.go +++ b/feature/acme/cert.go @@ -360,17 +360,30 @@ func (e *extension) domainRenewalTimeByARI(b *ipnlocal.LocalBackend, cs certStor return nil, fmt.Errorf("unexpected ACME account status %q", a.Status) } - // If we have a previous cert, include it in the order. Assuming we're - // within the ARI renewal window this should exclude us from LE rate - // limits. - // Note that this order extension will fail renewals if the ACME account key has changed - // since the last issuance, see - // https://github.com/tailscale/tailscale/issues/18251 + // If we have a previous cert, include it in the order via the ARI + // "replaces" extension so LE classifies the new cert as a renewal + // and exempts it from the per-registered-domain rate limit. Stores + // that can tell the current ACME account did not issue the previous + // cert opt out via [ARIReplacesAllower]; see #18251. var opts []xacme.OrderOption if previous != nil && !envknob.Bool("TS_DEBUG_ACME_FORCE_RENEWAL") { prevCrt, err := parseCertificate(previous) if err == nil { - opts = append(opts, xacme.WithOrderReplacesCert(prevCrt)) + useReplaces := true + if a, ok := cs.(ARIReplacesAllower); ok { + if allowed, err := a.ShouldUseARIReplacesForRenewal(domain); err != nil { + // Fail open: an error means we couldn't check + // eligibility, not that the account is misaligned. + logf("acme: failed to check ARI 'replaces' eligibility for %q, defaulting to use it: %v", domain, err) + } else { + useReplaces = allowed + } + } + if useReplaces { + opts = append(opts, xacme.WithOrderReplacesCert(prevCrt)) + } else { + logf("account key mismatch for previous cert; skipping ARI 'replaces' hint") + } } } diff --git a/feature/acme/certstore.go b/feature/acme/certstore.go index 43ff87be6..a7b5aea0d 100644 --- a/feature/acme/certstore.go +++ b/feature/acme/certstore.go @@ -187,6 +187,25 @@ type TLSCertKeyReader interface { ReadTLSCertAndKey(domain string) ([]byte, []byte, error) } +// ARIReplacesAllower is optionally implemented by state stores to opt +// out of the ARI "replaces" hint on a per-domain basis at renewal time. +// When implemented and returning false, the renewal path submits a plain +// newOrder instead of claiming renewal exemption via "replaces". +// +// Used by the k8s cert-share store to prevent submitting a "replaces" +// claim that Let's Encrypt would reject because the current ACME +// account did not issue the previous cert (which can happen when a +// shared per-tailnet account key is adopted after a cert was already +// issued by a per-pod account). See #18251. +type ARIReplacesAllower interface { + // ShouldUseARIReplacesForRenewal reports whether the renewal order + // for domain should carry the ARI "replaces" hint. It is advisory: + // a non-nil error means eligibility could not be determined, not + // that the hint should be skipped, and callers fail open (use the + // hint) in that case. + ShouldUseARIReplacesForRenewal(domain string) (bool, error) +} + func (s certStateStore) Read(domain string, now time.Time) (*ipnlocal.TLSCertKeyPair, error) { // If we're using a store that supports atomic reads, use that if kr, ok := s.StateStore.(TLSCertKeyReader); ok { @@ -215,6 +234,16 @@ func (s certStateStore) Read(domain string, now time.Time) (*ipnlocal.TLSCertKey return &ipnlocal.TLSCertKeyPair{CertPEM: certPEM, KeyPEM: keyPEM, Cached: true}, nil } +// ShouldUseARIReplacesForRenewal delegates to the underlying state store +// if it implements [ARIReplacesAllower]. Stores that don't opt in default +// to true (attempt "replaces"), preserving the pre-hook behaviour. +func (s certStateStore) ShouldUseARIReplacesForRenewal(domain string) (bool, error) { + if a, ok := s.StateStore.(ARIReplacesAllower); ok { + return a.ShouldUseARIReplacesForRenewal(domain) + } + return true, nil +} + func (s certStateStore) WriteCert(domain string, cert []byte) error { return ipn.WriteState(s.StateStore, ipn.StateKey(domain+".crt"), cert) } diff --git a/ipn/store/kubestore/store_kube.go b/ipn/store/kubestore/store_kube.go index 50ea1e287..bfc287baa 100644 --- a/ipn/store/kubestore/store_kube.go +++ b/ipn/store/kubestore/store_kube.go @@ -5,7 +5,9 @@ package kubestore import ( + "bytes" "context" + "crypto/sha256" "encoding/json" "fmt" "net" @@ -47,6 +49,17 @@ func init() { keyTLSCert = "tls.crt" keyTLSKey = "tls.key" + // keyACMEAcctFP is the cert Secret field that records the SHA-256 + // fingerprint of the PEM-encoded ACME account key that issued the + // cert. The renewal path uses this to decide whether to include the + // ARI "replaces" hint: only if the current account key matches + // (otherwise Let's Encrypt rejects the claim). + keyACMEAcctFP = "acme-account-fingerprint" + + // acmeAccountStateKey is the ipn.StateStore key under which tailscaled + // stores its ACME account private key. Mirrors the acmePEMName constant + // in ipn/ipnlocal/cert.go. Duplicated here to avoid an import cycle. + acmeAccountStateKey = "acme-account.key.pem" ) // Store is an ipn.StateStore that uses a Kubernetes Secret for persistence. @@ -57,6 +70,17 @@ type Store struct { certShareMode string // 'ro', 'rw', or empty podName string + // acmeAccountsSecretName, when non-empty in "rw" cert share mode, + // routes reads and writes of acmeAccountStateKey to acmeAccountField + // inside this shared per-tailnet Secret. See #18251. + acmeAccountsSecretName string + acmeAccountField string + + // preAdoptedLocalKey is the SHA-256 of the per-pod ACME account key + // that was in the local state Secret before we adopted a foreign + // shared key. Non-nil only when adoption changed the key. + preAdoptedLocalKey []byte + logf logger.Logf // memory holds the latest tailscale state. Writes write state to a kube @@ -106,6 +130,17 @@ func newWithClient(logf logger.Logf, c kubeclient.Client, secretName string) (*S s.certShareMode = "ro" } + // Configure shared ACME account lookup. Only meaningful for the cert + // issuer (cert share "rw") — read replicas never issue. + if s.certShareMode == "rw" { + s.acmeAccountsSecretName = os.Getenv("TS_ACME_ACCOUNT_SECRET_NAME") + s.acmeAccountField = os.Getenv("TS_ACME_ACCOUNT_FIELD") + if s.acmeAccountsSecretName != "" && s.acmeAccountField == "" { + s.logf("[unexpected] TS_ACME_ACCOUNT_SECRET_NAME set without TS_ACME_ACCOUNT_FIELD; ignoring shared ACME account configuration") + s.acmeAccountsSecretName = "" + } + } + // Load latest state from kube Secret if it already exists. if err := s.loadState(); err != nil && err != ipn.ErrStateNotExist { return nil, fmt.Errorf("error loading state from kube Secret: %w", err) @@ -126,9 +161,135 @@ func newWithClient(logf logger.Logf, c kubeclient.Client, secretName string) (*S if s.certShareMode == "ro" { go s.runCertReload(context.Background()) } + if s.acmeAccountsSecretName != "" { + if err := s.reconcileSharedACMEAccountKey(); err != nil { + // Non-fatal: the cert loop will retry on next issuance. + s.logf("kubestore: reconciling shared ACME account key: %v", err) + } + } return s, nil } +// reconcileSharedACMEAccountKey aligns the in-memory ACME account key with +// the shared per-tailnet field: adopt the shared value if present, otherwise +// copy the local per-pod key up so upgrading deployments keep renewal +// continuity. +func (s *Store) reconcileSharedACMEAccountKey() error { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + sharedSecret, err := s.client.GetSecret(ctx, s.acmeAccountsSecretName) + if err != nil && !kubeclient.IsNotFoundErr(err) { + return fmt.Errorf("reading shared ACME accounts Secret %q: %w", s.acmeAccountsSecretName, err) + } + var sharedKey []byte + if sharedSecret != nil { + sharedKey = sharedSecret.Data[sanitizeKey(s.acmeAccountField)] + } + if len(sharedKey) > 0 { + // Shared field already populated for this tailnet. Adopt it. If + // our local per-pod key differs, remember its fingerprint so + // legacy certs on this pod (issued before we started stamping + // fingerprints) can be recognised as mis-aligned on renewal. + localKey, err := s.memory.ReadState(ipn.StateKey(acmeAccountStateKey)) + if err == nil && len(localKey) > 0 && !bytes.Equal(localKey, sharedKey) { + sum := sha256.Sum256(localKey) + s.preAdoptedLocalKey = sum[:] + } + s.memory.WriteState(ipn.StateKey(acmeAccountStateKey), sharedKey) + return nil + } + + // Shared field is empty. If we have a per-pod key from the state + // Secret, copy it up so existing renewals stay exempt. + localKey, err := s.memory.ReadState(ipn.StateKey(acmeAccountStateKey)) + if err != nil || len(localKey) == 0 { + // Nothing local either; the cert loop will generate one on first + // use and route the write through writeSharedACMEAccountKey. + return nil + } + if err := s.writeSharedACMEAccountKey(localKey); err != nil { + return fmt.Errorf("copying per-pod ACME account key to shared Secret: %w", err) + } + s.logf("kubestore: migrated per-pod ACME account key into shared Secret %q field %q", s.acmeAccountsSecretName, s.acmeAccountField) + return nil +} + +// writeSharedACMEAccountKey writes key to acmeAccountField inside +// acmeAccountsSecretName, using whichever access pattern (patch or update) +// this Store has permission for. +func (s *Store) writeSharedACMEAccountKey(key []byte) error { + return s.updateSecret(map[string][]byte{s.acmeAccountField: key}, s.acmeAccountsSecretName) +} + +// maybeRestoreSharedACMEAccountKey writes the in-memory ACME account key to +// the shared Secret's per-tailnet field if that field is empty or missing, +// e.g. because the Secret was deleted and recreated while this process was +// running. It never overwrites an existing shared key. +func (s *Store) maybeRestoreSharedACMEAccountKey() error { + if s.acmeAccountsSecretName == "" { + return nil + } + key, err := s.memory.ReadState(ipn.StateKey(acmeAccountStateKey)) + if err != nil || len(key) == 0 { + return nil + } + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + shared, err := s.client.GetSecret(ctx, s.acmeAccountsSecretName) + if err != nil && !kubeclient.IsNotFoundErr(err) { + return fmt.Errorf("reading shared ACME accounts Secret %q: %w", s.acmeAccountsSecretName, err) + } + if shared != nil && len(shared.Data[sanitizeKey(s.acmeAccountField)]) > 0 { + return nil + } + s.logf("kubestore: shared ACME accounts Secret %q field %q is empty; restoring account key from memory", s.acmeAccountsSecretName, s.acmeAccountField) + return s.writeSharedACMEAccountKey(key) +} + +// acmeAccountKeyFingerprint returns the SHA-256 of the current in-memory +// PEM-encoded ACME account key, or (nil, false) if the key isn't set. +func acmeAccountKeyFingerprint(m *mem.Store) ([]byte, bool) { + key, err := m.ReadState(ipn.StateKey(acmeAccountStateKey)) + if err != nil || len(key) == 0 { + return nil, false + } + sum := sha256.Sum256(key) + return sum[:], true +} + +// ShouldUseARIReplacesForRenewal reports whether the current ACME account +// key matches the one that issued the cert for domain. See #18251. +func (s *Store) ShouldUseARIReplacesForRenewal(domain string) (bool, error) { + if s.certShareMode != "rw" { + return true, nil + } + curFP, ok := acmeAccountKeyFingerprint(&s.memory) + if !ok { + // No current account key in memory yet; nothing to compare. + return true, nil + } + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + sec, err := s.client.GetSecret(ctx, domain) + if err != nil { + if kubeclient.IsNotFoundErr(err) { + return true, nil + } + return true, fmt.Errorf("getting TLS Secret %q: %w", domain, err) + } + certFP := sec.Data[keyACMEAcctFP] + if len(certFP) == 0 { + // Legacy cert, no fingerprint stamp. Assume misaligned only if + // we adopted a foreign shared key. + if len(s.preAdoptedLocalKey) > 0 { + return false, nil + } + return true, nil + } + return bytes.Equal(certFP, curFP), nil +} + func (s *Store) SetDialer(d func(ctx context.Context, network, address string) (net.Conn, error)) { s.client.SetDialer(d) } @@ -147,6 +308,12 @@ func (s *Store) WriteState(id ipn.StateKey, bs []byte) (err error) { s.memory.WriteState(ipn.StateKey(sanitizeKey(id)), bs) } }() + if s.acmeAccountsSecretName != "" && string(id) == acmeAccountStateKey { + if bs == nil { + return s.removeSecretField(s.acmeAccountField, s.acmeAccountsSecretName) + } + return s.writeSharedACMEAccountKey(bs) + } if bs == nil { return s.removeSecretField(string(id), s.secretName) } @@ -154,7 +321,9 @@ func (s *Store) WriteState(id ipn.StateKey, bs []byte) (err error) { } // WriteTLSCertAndKey writes a TLS cert and key to domain.crt, domain.key fields -// of a Tailscale Kubernetes node's state Secret. +// of a Tailscale Kubernetes node's state Secret. In cert-share "rw" mode it +// also stamps acme-account-fingerprint alongside the cert so the renewal path +// can tell whether the current ACME account key issued this cert. func (s *Store) WriteTLSCertAndKey(domain string, cert, key []byte) (err error) { if s.certShareMode == "ro" { s.logf("[unexpected] TLS cert and key write in read-only mode") @@ -175,6 +344,14 @@ func (s *Store) WriteTLSCertAndKey(domain string, cert, key []byte) (err error) keyTLSCert: cert, keyTLSKey: key, } + if fp, ok := acmeAccountKeyFingerprint(&s.memory); ok { + data[keyACMEAcctFP] = fp + } + // The shared Secret may have been deleted and recreated empty + // while we were running; re-assert the account key if so. + if err := s.maybeRestoreSharedACMEAccountKey(); err != nil { + s.logf("kubestore: restoring shared ACME account key: %v", err) + } } if err := s.updateSecret(data, secretName); err != nil { return fmt.Errorf("error writing TLS cert and key to Secret: %w", err) @@ -489,7 +666,9 @@ func (s *Store) loadCerts(ctx context.Context, sel map[string]string) error { // canCreateSecret returns true if this node should be allowed to create the given // Secret in its namespace. func (s *Store) canCreateSecret(secret string) bool { - // Only allow creating the state Secret (and not TLS Secrets). + // Only allow creating the state Secret (and not TLS Secrets). The + // shared ACME accounts Secret is precreated by the operator, so write + // replicas never need create permission for it. return secret == s.secretName } @@ -498,7 +677,8 @@ func (s *Store) canCreateSecret(secret string) bool { func (s *Store) canPatchSecret(secret string) bool { // For backwards compatibility reasons, setups where the proxies are not // given PATCH permissions for state Secrets are allowed. For TLS - // Secrets, we should always have PATCH permissions. + // Secrets and the shared ACME accounts Secret, we should always have + // PATCH permissions. if secret == s.secretName { return s.canPatch } diff --git a/ipn/store/kubestore/store_kube_test.go b/ipn/store/kubestore/store_kube_test.go index 5f65116bc..14b69787e 100644 --- a/ipn/store/kubestore/store_kube_test.go +++ b/ipn/store/kubestore/store_kube_test.go @@ -6,6 +6,7 @@ import ( "bytes" "context" + "crypto/sha256" "encoding/json" "fmt" "strings" @@ -847,3 +848,441 @@ func TestNewWithClient(t *testing.T) { }) } } + +func TestSharedACMEAccountKey(t *testing.T) { + const ( + stateSecretName = "ingress-proxies-0" + sharedSecretName = kubetypes.ACMEAccountsSecretName + sharedField = "my-tailnet" + kubetypes.ACMEAccountKeySuffix + ) + existingKey := []byte("-----BEGIN PRIVATE KEY-----\nexisting\n-----END PRIVATE KEY-----") + freshKey := []byte("-----BEGIN PRIVATE KEY-----\nfresh\n-----END PRIVATE KEY-----") + + tests := []struct { + name string + certMode string + stateSecret map[string][]byte + sharedSecret map[string][]byte // nil = Secret does not exist + envSecretName string + envField string + writeAfterInit []byte // if non-nil, call WriteState(acmeAccountStateKey, …) after init + wantMemoryACME []byte + wantSharedSecret map[string][]byte + wantStateSecret map[string][]byte // optional: when set, asserts state Secret was not touched on the ACME path + wantPreAdopted []byte // expected s.preAdoptedLocalKey (sha256 of pre-adoption local key) + }{ + { + name: "adopts_shared_key_when_present", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{}, + sharedSecret: map[string][]byte{sharedField: existingKey}, + wantMemoryACME: existingKey, + wantSharedSecret: map[string][]byte{sharedField: existingKey}, + }, + { + name: "shared_overrides_per_pod_key", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{ + acmeAccountStateKey: freshKey, // would be stale + }, + sharedSecret: map[string][]byte{sharedField: existingKey}, + wantMemoryACME: existingKey, + wantSharedSecret: map[string][]byte{sharedField: existingKey}, + wantPreAdopted: sha256Sum(freshKey), + }, + { + name: "adopting_matching_local_leaves_preadopted_nil", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{ + acmeAccountStateKey: existingKey, // matches shared + }, + sharedSecret: map[string][]byte{sharedField: existingKey}, + wantMemoryACME: existingKey, + wantSharedSecret: map[string][]byte{sharedField: existingKey}, + }, + { + name: "migrates_per_pod_key_when_shared_field_empty", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{ + acmeAccountStateKey: existingKey, + }, + sharedSecret: map[string][]byte{}, // exists but no field for this tailnet + wantMemoryACME: existingKey, + wantSharedSecret: map[string][]byte{sharedField: existingKey}, + }, + { + name: "no_op_when_no_keys_anywhere", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{}, + sharedSecret: map[string][]byte{}, + wantMemoryACME: nil, + wantSharedSecret: map[string][]byte{}, + }, + { + name: "ro_mode_ignores_env_vars", + certMode: "ro", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{ + acmeAccountStateKey: freshKey, + }, + sharedSecret: map[string][]byte{sharedField: existingKey}, + wantMemoryACME: freshKey, // per-pod copy stays; shared Secret never consulted + }, + { + name: "write_routes_to_shared_secret", + certMode: "rw", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{}, + sharedSecret: map[string][]byte{}, + writeAfterInit: freshKey, + wantMemoryACME: freshKey, + wantSharedSecret: map[string][]byte{sharedField: freshKey}, + wantStateSecret: map[string][]byte{}, // state Secret untouched by the ACME write + }, + { + name: "write_in_ro_mode_goes_to_state_secret", + certMode: "ro", + envSecretName: sharedSecretName, + envField: sharedField, + stateSecret: map[string][]byte{}, + sharedSecret: map[string][]byte{}, + writeAfterInit: freshKey, + wantMemoryACME: freshKey, + wantSharedSecret: map[string][]byte{}, // env vars ignored in ro mode + wantStateSecret: map[string][]byte{acmeAccountStateKey: freshKey}, + }, + { + name: "no_env_vars_routes_to_state_secret", + certMode: "rw", + envSecretName: "", // shared account disabled + envField: "", + stateSecret: map[string][]byte{ + acmeAccountStateKey: existingKey, + }, + sharedSecret: nil, // does not exist; should not be touched + wantMemoryACME: existingKey, + wantStateSecret: map[string][]byte{acmeAccountStateKey: existingKey}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv("TS_CERT_SHARE_MODE", tt.certMode) + t.Setenv("TS_ACME_ACCOUNT_SECRET_NAME", tt.envSecretName) + t.Setenv("TS_ACME_ACCOUNT_FIELD", tt.envField) + t.Setenv("POD_NAME", stateSecretName) + + stateData := cloneMap(tt.stateSecret) + sharedData := cloneMap(tt.sharedSecret) + sharedExists := tt.sharedSecret != nil + + client := &kubeclient.FakeClient{ + GetSecretImpl: func(ctx context.Context, name string) (*kubeapi.Secret, error) { + switch name { + case stateSecretName: + return &kubeapi.Secret{Data: stateData}, nil + case sharedSecretName: + if !sharedExists { + return nil, &kubeapi.Status{Code: 404} + } + return &kubeapi.Secret{Data: sharedData}, nil + } + return nil, &kubeapi.Status{Code: 404} + }, + CheckSecretPermissionsImpl: func(ctx context.Context, name string) (bool, bool, error) { + return true, true, nil + }, + JSONPatchResourceImpl: func(ctx context.Context, name, resourceType string, patches []kubeclient.JSONPatch) error { + var target *map[string][]byte + switch name { + case stateSecretName: + target = &stateData + case sharedSecretName: + target = &sharedData + sharedExists = true + default: + t.Errorf("unexpected patch target Secret %q", name) + return nil + } + if *target == nil { + *target = map[string][]byte{} + } + for _, p := range patches { + if p.Op == "add" && p.Path == "/data" { + *target = p.Value.(map[string][]byte) + } else if p.Op == "add" && strings.HasPrefix(p.Path, "/data/") { + key := strings.TrimPrefix(p.Path, "/data/") + (*target)[key] = p.Value.([]byte) + } + } + return nil + }, + CreateSecretImpl: func(ctx context.Context, s *kubeapi.Secret) error { + switch s.Name { + case stateSecretName: + stateData = s.Data + case sharedSecretName: + sharedData = s.Data + sharedExists = true + default: + t.Errorf("unexpected create target Secret %q", s.Name) + } + return nil + }, + ListSecretsImpl: func(ctx context.Context, selector map[string]string) (*kubeapi.SecretList, error) { + // Used by ro-mode TLS Secret preload; irrelevant to this test. + return &kubeapi.SecretList{}, nil + }, + } + + s, err := newWithClient(t.Logf, client, stateSecretName) + if err != nil { + t.Fatalf("newWithClient: %v", err) + } + + if tt.writeAfterInit != nil { + if err := s.WriteState(ipn.StateKey(acmeAccountStateKey), tt.writeAfterInit); err != nil { + t.Fatalf("WriteState(ACME key): %v", err) + } + } + + gotMemACME, err := s.memory.ReadState(ipn.StateKey(acmeAccountStateKey)) + if err != nil && tt.wantMemoryACME != nil { + t.Errorf("memory ReadState(ACME key): %v", err) + } + if !bytes.Equal(gotMemACME, tt.wantMemoryACME) { + t.Errorf("memory ACME key = %q, want %q", gotMemACME, tt.wantMemoryACME) + } + if tt.wantSharedSecret != nil { + if diff := cmp.Diff(sharedData, tt.wantSharedSecret); diff != "" { + t.Errorf("shared Secret contents mismatch (-got +want):\n%s", diff) + } + } + if tt.wantStateSecret != nil { + if diff := cmp.Diff(stateData, tt.wantStateSecret); diff != "" { + t.Errorf("state Secret contents mismatch (-got +want):\n%s", diff) + } + } + if !bytes.Equal(s.preAdoptedLocalKey, tt.wantPreAdopted) { + t.Errorf("preAdoptedLocalKey = %x, want %x", s.preAdoptedLocalKey, tt.wantPreAdopted) + } + }) + } +} + +func cloneMap(m map[string][]byte) map[string][]byte { + if m == nil { + return nil + } + out := make(map[string][]byte, len(m)) + for k, v := range m { + out[k] = append([]byte(nil), v...) + } + return out +} + +func sha256Sum(b []byte) []byte { + sum := sha256.Sum256(b) + return sum[:] +} + +func TestShouldUseARIReplacesForRenewal(t *testing.T) { + const domain = "app.tailnetxyz.ts.net" + acmeKey := []byte("-----BEGIN PRIVATE KEY-----\ncurrent\n-----END PRIVATE KEY-----") + otherKey := []byte("-----BEGIN PRIVATE KEY-----\nother\n-----END PRIVATE KEY-----") + curFP := sha256Sum(acmeKey) + otherFP := sha256Sum(otherKey) + + tests := []struct { + name string + certShareMode string + acmeInMemory []byte // per-pod ACME key present in memory + preAdopted []byte // sha256 of pre-adoption local key (foreign-key path) + certSecret map[string][]byte + certGetErr error + want bool + wantErr bool + }{ + { + name: "non_rw_mode_returns_true", + certShareMode: "", + want: true, + }, + { + name: "ro_mode_returns_true", + certShareMode: "ro", + want: true, + }, + { + name: "no_acme_key_returns_true", + certShareMode: "rw", + want: true, + }, + { + name: "cert_not_found_returns_true", + certShareMode: "rw", + acmeInMemory: acmeKey, + certGetErr: &kubeapi.Status{Code: 404}, + want: true, + }, + { + name: "cert_get_error_returns_true_with_err", + certShareMode: "rw", + acmeInMemory: acmeKey, + certGetErr: fmt.Errorf("api down"), + want: true, + wantErr: true, + }, + { + name: "fingerprint_matches", + certShareMode: "rw", + acmeInMemory: acmeKey, + certSecret: map[string][]byte{keyACMEAcctFP: curFP}, + want: true, + }, + { + name: "fingerprint_differs", + certShareMode: "rw", + acmeInMemory: acmeKey, + certSecret: map[string][]byte{keyACMEAcctFP: otherFP}, + want: false, + }, + { + name: "legacy_cert_no_preadopted_returns_true", + certShareMode: "rw", + acmeInMemory: acmeKey, + certSecret: map[string][]byte{}, // no fingerprint field + want: true, + }, + { + name: "legacy_cert_with_preadopted_returns_false", + certShareMode: "rw", + acmeInMemory: acmeKey, + preAdopted: sha256Sum(otherKey), + certSecret: map[string][]byte{}, // no fingerprint field + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &kubeclient.FakeClient{ + GetSecretImpl: func(ctx context.Context, name string) (*kubeapi.Secret, error) { + if tt.certGetErr != nil { + return nil, tt.certGetErr + } + return &kubeapi.Secret{Data: tt.certSecret}, nil + }, + } + s := &Store{ + client: client, + certShareMode: tt.certShareMode, + memory: mem.Store{}, + preAdoptedLocalKey: tt.preAdopted, + logf: t.Logf, + } + if len(tt.acmeInMemory) > 0 { + s.memory.WriteState(ipn.StateKey(acmeAccountStateKey), tt.acmeInMemory) + } + + got, err := s.ShouldUseARIReplacesForRenewal(domain) + if (err != nil) != tt.wantErr { + t.Errorf("err = %v, wantErr = %v", err, tt.wantErr) + } + if got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} + +func TestWriteTLSCertAndKeyStampsFingerprint(t *testing.T) { + const domain = "app.tailnetxyz.ts.net" + acmeKey := []byte("-----BEGIN PRIVATE KEY-----\naccount\n-----END PRIVATE KEY-----") + wantFP := sha256Sum(acmeKey) + + tests := []struct { + name string + certMode string + acmeInMemory []byte + wantFP []byte // expected value of keyACMEAcctFP field; nil means field must be absent + }{ + { + name: "rw_mode_with_acme_key_stamps_fingerprint", + certMode: "rw", + acmeInMemory: acmeKey, + wantFP: wantFP, + }, + { + name: "rw_mode_no_acme_key_no_stamp", + certMode: "rw", + }, + { + name: "non_share_mode_no_stamp", + certMode: "", + acmeInMemory: acmeKey, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + secret := map[string][]byte{} + client := &kubeclient.FakeClient{ + GetSecretImpl: func(ctx context.Context, name string) (*kubeapi.Secret, error) { + return &kubeapi.Secret{Data: secret}, nil + }, + CheckSecretPermissionsImpl: func(ctx context.Context, name string) (bool, bool, error) { + return true, true, nil + }, + JSONPatchResourceImpl: func(ctx context.Context, name, resourceType string, patches []kubeclient.JSONPatch) error { + for _, p := range patches { + if p.Op == "add" && p.Path == "/data" { + secret = p.Value.(map[string][]byte) + } else if p.Op == "add" && strings.HasPrefix(p.Path, "/data/") { + secret[strings.TrimPrefix(p.Path, "/data/")] = p.Value.([]byte) + } + } + return nil + }, + } + s := &Store{ + client: client, + canPatch: true, + secretName: "ts-state", + certShareMode: tt.certMode, + memory: mem.Store{}, + logf: t.Logf, + } + if len(tt.acmeInMemory) > 0 { + s.memory.WriteState(ipn.StateKey(acmeAccountStateKey), tt.acmeInMemory) + } + + if err := s.WriteTLSCertAndKey(domain, []byte("cert"), []byte("key")); err != nil { + t.Fatalf("WriteTLSCertAndKey: %v", err) + } + + gotFP, present := secret[keyACMEAcctFP] + if tt.wantFP == nil { + if present { + t.Errorf("unexpected fingerprint stamped: %x", gotFP) + } + return + } + if !bytes.Equal(gotFP, tt.wantFP) { + t.Errorf("fingerprint = %x, want %x", gotFP, tt.wantFP) + } + }) + } +} diff --git a/kube/kubetypes/types.go b/kube/kubetypes/types.go index b99980ef4..d4653f374 100644 --- a/kube/kubetypes/types.go +++ b/kube/kubetypes/types.go @@ -65,6 +65,28 @@ LabelSecretTypeState = "state" LabelSecretTypeCerts = "certs" + // ACMEAccountsSecretName is the name of the Secret in the operator + // namespace that holds shared ACME account private keys, one field per + // tailnet. Sharing one account key across all replicas of every + // ProxyGroup attached to a tailnet preserves Let's Encrypt's renewal + // exemption (via the ARI "replaces" extension) across pod restarts, + // ProxyGroup recreation, and ingress migration. + ACMEAccountsSecretName = "tailscale-acme-accounts" + + // ACMEAccountDefaultKey is the field used inside ACMEAccountsSecretName + // for the "default" tailnet (i.e., when ProxyGroup.spec.tailnet is empty + // and the operator uses its own credentials). + ACMEAccountDefaultKey = "_default" + + // ACMEAccountKeySuffix is appended to the tailnet identifier to form a + // field name within ACMEAccountsSecretName. + ACMEAccountKeySuffix = ".acme-account.key.pem" + + // ACMEAccountsFinalizer blocks accidental deletion of + // ACMEAccountsSecretName; losing those keys breaks ARI "replaces" + // renewals for every cert in the tailnet. See #18251. + ACMEAccountsFinalizer = "tailscale.com/acme-account-protection" + KubeAPIServerConfigFile = "config.hujson" APIServerProxyModeAuth APIServerProxyMode = "auth" APIServerProxyModeNoAuth APIServerProxyMode = "noauth"