Просмотр исходного кода

ipn/ipnlocal: don't fail profile unmarshal due to attestation keys (#18335)

Soft-fail on initial unmarshal and try again, ignoring the
AttestationKey. This helps in cases where something about the
attestation key storage (usually a TPM) is messed up. The old key will
be lost, but at least the node can start again.

Updates #18302
Updates #15830

Signed-off-by: Andrew Lytvynov <[email protected]>
Andrew Lytvynov 1 месяц назад
Родитель
Сommit
2e77b75e96
2 измененных файлов с 79 добавлено и 7 удалено
  1. 41 7
      ipn/ipnlocal/profiles.go
  2. 38 0
      ipn/ipnlocal/profiles_test.go

+ 41 - 7
ipn/ipnlocal/profiles.go

@@ -5,10 +5,12 @@ package ipnlocal
 
 import (
 	"cmp"
+	"crypto"
 	"crypto/rand"
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"runtime"
 	"slices"
 	"strings"
@@ -59,6 +61,9 @@ type profileManager struct {
 	// extHost is the bridge between [profileManager] and the registered [ipnext.Extension]s.
 	// It may be nil in tests. A nil pointer is a valid, no-op host.
 	extHost *ExtensionHost
+
+	// Override for key.NewEmptyHardwareAttestationKey used for testing.
+	newEmptyHardwareAttestationKey func() (key.HardwareAttestationKey, error)
 }
 
 // SetExtensionHost sets the [ExtensionHost] for the [profileManager].
@@ -660,13 +665,23 @@ func (pm *profileManager) loadSavedPrefs(k ipn.StateKey) (ipn.PrefsView, error)
 
 	// if supported by the platform, create an empty hardware attestation key to use when deserializing
 	// to avoid type exceptions from json.Unmarshaling into an interface{}.
-	hw, _ := key.NewEmptyHardwareAttestationKey()
+	hw, _ := pm.newEmptyHardwareAttestationKey()
 	savedPrefs.Persist = &persist.Persist{
 		AttestationKey: hw,
 	}
 
 	if err := ipn.PrefsFromBytes(bs, savedPrefs); err != nil {
-		return ipn.PrefsView{}, fmt.Errorf("parsing saved prefs: %v", err)
+		// Try loading again, this time ignoring the AttestationKey contents.
+		// If that succeeds, there's something wrong with the underlying
+		// attestation key mechanism (most likely the TPM changed), but we
+		// should at least proceed with client startup.
+		origErr := err
+		savedPrefs.Persist.AttestationKey = &noopAttestationKey{}
+		if err := ipn.PrefsFromBytes(bs, savedPrefs); err != nil {
+			return ipn.PrefsView{}, fmt.Errorf("parsing saved prefs: %w", err)
+		} else {
+			pm.logf("failed to parse savedPrefs with attestation key (error: %v) but parsing without the attestation key succeeded; will proceed without using the old attestation key", origErr)
+		}
 	}
 	pm.logf("using backend prefs for %q: %v", k, savedPrefs.Pretty())
 
@@ -912,11 +927,12 @@ func newProfileManagerWithGOOS(store ipn.StateStore, logf logger.Logf, ht *healt
 	metricProfileCount.Set(int64(len(knownProfiles)))
 
 	pm := &profileManager{
-		goos:          goos,
-		store:         store,
-		knownProfiles: knownProfiles,
-		logf:          logf,
-		health:        ht,
+		goos:                           goos,
+		store:                          store,
+		knownProfiles:                  knownProfiles,
+		logf:                           logf,
+		health:                         ht,
+		newEmptyHardwareAttestationKey: key.NewEmptyHardwareAttestationKey,
 	}
 
 	var initialProfile ipn.LoginProfileView
@@ -985,3 +1001,21 @@ var (
 	metricMigrationError   = clientmetric.NewCounter("profiles_migration_error")
 	metricMigrationSuccess = clientmetric.NewCounter("profiles_migration_success")
 )
+
+// noopAttestationKey is a key.HardwareAttestationKey that always successfully
+// unmarshals as a zero key.
+type noopAttestationKey struct{}
+
+func (n noopAttestationKey) Public() crypto.PublicKey {
+	panic("noopAttestationKey.Public should not be called; missing IsZero check somewhere?")
+}
+
+func (n noopAttestationKey) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) (signature []byte, err error) {
+	panic("noopAttestationKey.Sign should not be called; missing IsZero check somewhere?")
+}
+
+func (n noopAttestationKey) MarshalJSON() ([]byte, error)      { return nil, nil }
+func (n noopAttestationKey) UnmarshalJSON([]byte) error        { return nil }
+func (n noopAttestationKey) Close() error                      { return nil }
+func (n noopAttestationKey) Clone() key.HardwareAttestationKey { return n }
+func (n noopAttestationKey) IsZero() bool                      { return true }

+ 38 - 0
ipn/ipnlocal/profiles_test.go

@@ -4,6 +4,7 @@
 package ipnlocal
 
 import (
+	"errors"
 	"fmt"
 	"os/user"
 	"strconv"
@@ -1147,3 +1148,40 @@ func TestProfileStateChangeCallback(t *testing.T) {
 		})
 	}
 }
+
+func TestProfileBadAttestationKey(t *testing.T) {
+	store := new(mem.Store)
+	pm, err := newProfileManagerWithGOOS(store, t.Logf, health.NewTracker(eventbustest.NewBus(t)), "linux")
+	if err != nil {
+		t.Fatal(err)
+	}
+	fk := new(failingHardwareAttestationKey)
+	pm.newEmptyHardwareAttestationKey = func() (key.HardwareAttestationKey, error) {
+		return fk, nil
+	}
+	sk := ipn.StateKey(t.Name())
+	if err := pm.store.WriteState(sk, []byte(`{"Config": {"AttestationKey": {}}}`)); err != nil {
+		t.Fatal(err)
+	}
+	prefs, err := pm.loadSavedPrefs(sk)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ak := prefs.Persist().AsStruct().AttestationKey
+	if _, ok := ak.(noopAttestationKey); !ok {
+		t.Errorf("loaded attestation key of type %T, want noopAttestationKey", ak)
+	}
+	if !fk.unmarshalCalled {
+		t.Error("UnmarshalJSON was not called on failingHardwareAttestationKey")
+	}
+}
+
+type failingHardwareAttestationKey struct {
+	noopAttestationKey
+	unmarshalCalled bool
+}
+
+func (k *failingHardwareAttestationKey) UnmarshalJSON([]byte) error {
+	k.unmarshalCalled = true
+	return errors.New("failed to unmarshal attestation key!")
+}