From 0950dc191deae223070ebb767530fa61f200555d Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Thu, 16 Jul 2026 12:53:57 +0000 Subject: [PATCH] ipn/store/kubestore: track ACME account key fingerprints on certs Stamp the SHA-256 of the ACME account key that issued each cert into an acme-account-fingerprint field on the cert Secret when writing in cert-share "rw" mode. Implement [ARIReplacesAllower] to compare the recorded fingerprint against the current in-memory account key: matching -> allow "replaces"; mismatching -> skip. For legacy certs (issued before this code and lacking the fingerprint field), record the sha256 of the pre-adoption per-pod key at store init when reconcileSharedACMEAccountKey adopts a foreign shared key, and treat those certs as mis-aligned when the tracker is set. This prevents guaranteed-reject "replaces" claims on the first renewal after a ProxyGroup joins a tailnet whose shared account was seeded by a different PG. Updates #18251 Updates #20288 Signed-off-by: chaosinthecrd --- ipn/store/kubestore/store_kube.go | 73 ++++++- ipn/store/kubestore/store_kube_test.go | 258 ++++++++++++++++++++++--- 2 files changed, 306 insertions(+), 25 deletions(-) diff --git a/ipn/store/kubestore/store_kube.go b/ipn/store/kubestore/store_kube.go index b5fd880a5..12d9c597d 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,12 @@ 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 @@ -68,6 +76,11 @@ type Store struct { 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 @@ -174,7 +187,15 @@ func (s *Store) reconcileSharedACMEAccountKey() error { sharedKey = sharedSecret.Data[sanitizeKey(s.acmeAccountField)] } if len(sharedKey) > 0 { - // Shared field already populated for this tailnet. Adopt it. + // 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 } @@ -201,6 +222,49 @@ func (s *Store) writeSharedACMEAccountKey(key []byte) error { return s.updateSecret(map[string][]byte{s.acmeAccountField: key}, s.acmeAccountsSecretName) } +// 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) } @@ -232,7 +296,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") @@ -253,6 +319,9 @@ 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 + } } if err := s.updateSecret(data, secretName); err != nil { return fmt.Errorf("error writing TLS cert and key to Secret: %w", err) diff --git a/ipn/store/kubestore/store_kube_test.go b/ipn/store/kubestore/store_kube_test.go index 6527acaf7..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" @@ -868,15 +869,16 @@ func TestSharedACMEAccountKey(t *testing.T) { 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, + 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}, }, { @@ -890,6 +892,19 @@ func TestSharedACMEAccountKey(t *testing.T) { 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", @@ -925,26 +940,26 @@ func TestSharedACMEAccountKey(t *testing.T) { 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, + 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, + 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}, }, @@ -1060,6 +1075,9 @@ func TestSharedACMEAccountKey(t *testing.T) { 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) + } }) } } @@ -1074,3 +1092,197 @@ func cloneMap(m map[string][]byte) map[string][]byte { } 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) + } + }) + } +}