diff --git a/builtin/logical/ssh/backend.go b/builtin/logical/ssh/backend.go index cbb372a719..1570370451 100644 --- a/builtin/logical/ssh/backend.go +++ b/builtin/logical/ssh/backend.go @@ -56,6 +56,10 @@ func Backend(conf *logical.BackendConfig) (*backend, error) { caPrivateKeyStoragePath, keysStoragePrefix, }, + + AllowSnapshotRead: []string{ + "config/ca", + }, }, Paths: []*framework.Path{ diff --git a/builtin/logical/ssh/path_config_ca.go b/builtin/logical/ssh/path_config_ca.go index 4565d506db..b1130ee443 100644 --- a/builtin/logical/ssh/path_config_ca.go +++ b/builtin/logical/ssh/path_config_ca.go @@ -107,6 +107,9 @@ func pathConfigCA(b *backend) *framework.Path { OperationSuffix: "ca-configuration", }, }, + logical.RecoverOperation: &framework.PathOperation{ + Callback: b.pathConfigCARecover, + }, }, HelpSynopsis: `Set the SSH private key used for signing certificates.`, @@ -120,7 +123,9 @@ Read operations will return the public key, if already stored/generated.`, } func (b *backend) pathConfigCARead(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - publicKey, err := getCAPublicKey(ctx, req.Storage) + // prevent migration from deprecated paths on snapshot read as writes to a loaded snapshot storage are forbidden + allowMigration := !req.IsSnapshotReadOrList() + publicKey, err := getCAPublicKey(ctx, req.Storage, allowMigration) if err != nil { return nil, fmt.Errorf("failed to read CA public key: %w", err) } @@ -142,16 +147,22 @@ func (b *backend) pathConfigCADelete(ctx context.Context, req *logical.Request, if err := req.Storage.Delete(ctx, caPrivateKeyStoragePath); err != nil { return nil, err } + if err := req.Storage.Delete(ctx, caPrivateKeyStoragePathDeprecated); err != nil { + return nil, err + } if err := req.Storage.Delete(ctx, caPublicKeyStoragePath); err != nil { return nil, err } + if err := req.Storage.Delete(ctx, caPublicKeyStoragePathDeprecated); err != nil { + return nil, err + } if err := req.Storage.Delete(ctx, caManagedKeyStoragePath); err != nil { return nil, err } return nil, nil } -func readStoredKey(ctx context.Context, storage logical.Storage, keyType string) (*keyStorageEntry, error) { +func readStoredKeyEntry(ctx context.Context, storage logical.Storage, keyType string, allowMigration bool) (*logical.StorageEntry, error) { var path, deprecatedPath string switch keyType { case caPrivateKey: @@ -176,25 +187,39 @@ func readStoredKey(ctx context.Context, storage logical.Storage, keyType string) if err != nil { return nil, err } + if entry != nil { + // modify entry variable, both for possible migration and also to comply with the expected JSON entry for the caller entry, err = logical.StorageEntryJSON(path, keyStorageEntry{ Key: string(entry.Value), }) if err != nil { return nil, err } - if err := storage.Put(ctx, entry); err != nil { - return nil, err - } - if err = storage.Delete(ctx, deprecatedPath); err != nil { - return nil, err + // migrations are disable on recover, as we can't write to the loaded snapshot storage + if allowMigration { + if err := storage.Put(ctx, entry); err != nil { + return nil, err + } + if err = storage.Delete(ctx, deprecatedPath); err != nil { + return nil, err + } } } } + return entry, nil +} + +// readStoredKey reads a key from storage, returning nil if not found. +// ignore-nil-nil-function-check +func readStoredKey(ctx context.Context, storage logical.Storage, keyType string, allowMigration bool) (*keyStorageEntry, error) { + entry, err := readStoredKeyEntry(ctx, storage, keyType, allowMigration) + if err != nil { + return nil, err + } if entry == nil { return nil, nil } - var keyEntry keyStorageEntry if err := entry.DecodeJSON(&keyEntry); err != nil { return nil, err @@ -450,10 +475,10 @@ func (b *backend) createManagedKey(ctx context.Context, s logical.Storage, manag return nil } -func getCAPublicKey(ctx context.Context, storage logical.Storage) (string, error) { +func getCAPublicKey(ctx context.Context, storage logical.Storage, allowMigration bool) (string, error) { var publicKey string - storedKeyEntry, err := readStoredKey(ctx, storage, caPublicKey) + storedKeyEntry, err := readStoredKey(ctx, storage, caPublicKey, allowMigration) if err != nil { return "", err } @@ -495,12 +520,13 @@ func readManagedKey(ctx context.Context, storage logical.Storage) (*managedKeySt } func caKeysConfigured(ctx context.Context, s logical.Storage) (bool, error) { - publicKeyEntry, err := readStoredKey(ctx, s, caPublicKey) + const allowMigration = false // no need to allow migration when just checking for existence, we can do that later + publicKeyEntry, err := readStoredKey(ctx, s, caPublicKey, allowMigration) if err != nil { return false, fmt.Errorf("failed to read CA public key: %w", err) } - privateKeyEntry, err := readStoredKey(ctx, s, caPrivateKey) + privateKeyEntry, err := readStoredKey(ctx, s, caPrivateKey, allowMigration) if err != nil { return false, fmt.Errorf("failed to read CA private key: %w", err) } @@ -520,3 +546,64 @@ func caKeysConfigured(ctx context.Context, s logical.Storage) (bool, error) { return false, nil } + +// pathConfigCARecover recovers the CA from the target snapshot back to the live storage. +// ignore-nil-nil-function-check +func (b *backend) pathConfigCARecover(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + // check live storage for existing keys. Disallow recovery if CA is already configured for consistency with create operation + found, err := caKeysConfigured(ctx, req.Storage) + if err != nil { + return nil, err + } + if found { + return logical.ErrorResponse("keys are already configured; delete them before recovering the CA"), nil + } + + // fetch directly from the snapshot storage instead of following the usual restore procedure of getting the values + // from the req.Data, since those came from a previous CARead operation on the loaded snapshot, which only contains + // the public key. + snapshotStorage, err := logical.NewSnapshotStorageView(req) + if err != nil { + return nil, err + } + const allowMigration = false // prevent migration from deprecated paths as we can't allow writes on the snapshot storage + publicKeyEntry, err := readStoredKeyEntry(ctx, snapshotStorage, caPublicKey, allowMigration) + if err != nil { + return nil, fmt.Errorf("failed to read CA public key for restore: %w", err) + } + privateKeyEntry, err := readStoredKeyEntry(ctx, snapshotStorage, caPrivateKey, allowMigration) + if err != nil { + return nil, fmt.Errorf("failed to read CA private key for restore: %w", err) + } + managedKey, err := readManagedKey(ctx, snapshotStorage) + if err != nil { + return nil, fmt.Errorf("failed to read CA managed key for restore: %w", err) + } + + if publicKeyEntry == nil && privateKeyEntry == nil && managedKey == nil { + return logical.ErrorResponse("no CA keys found in snapshot storage to restore"), nil + } + + // it's possible that we've read the keys from a deprecated path in the snapshot, but it should be automatically + // upgraded to the new path anyway, so we don't care about restoring it back to the deprecated path + if publicKeyEntry != nil { + err = req.Storage.Put(ctx, publicKeyEntry) + if err != nil { + return nil, fmt.Errorf("failed to restore public key entry in storage: %w", err) + } + } + if privateKeyEntry != nil { + err = req.Storage.Put(ctx, privateKeyEntry) + if err != nil { + return nil, fmt.Errorf("failed to restore private key entry in storage: %w", err) + } + } + if managedKey != nil { + err = b.createManagedKey(ctx, req.Storage, managedKey.KeyName.String(), managedKey.KeyId.String()) + if err != nil { + return nil, fmt.Errorf("failed to restore managed key entry in storage: %w", err) + } + } + + return nil, nil +} diff --git a/builtin/logical/ssh/path_config_ca_test.go b/builtin/logical/ssh/path_config_ca_test.go index 42cc9b7c79..7ac7777032 100644 --- a/builtin/logical/ssh/path_config_ca_test.go +++ b/builtin/logical/ssh/path_config_ca_test.go @@ -10,7 +10,9 @@ import ( "strings" "testing" + "github.com/hashicorp/vault/sdk/helper/testhelpers/snapshots" "github.com/hashicorp/vault/sdk/logical" + "github.com/stretchr/testify/require" ) func TestSSH_ConfigCAStorageUpgrade(t *testing.T) { @@ -39,7 +41,7 @@ func TestSSH_ConfigCAStorageUpgrade(t *testing.T) { } // Reading it should return the key as well as upgrade the storage path - privateKeyEntry, err := readStoredKey(context.Background(), config.StorageView, caPrivateKey) + privateKeyEntry, err := readStoredKey(context.Background(), config.StorageView, caPrivateKey, true) if err != nil { t.Fatal(err) } @@ -73,7 +75,7 @@ func TestSSH_ConfigCAStorageUpgrade(t *testing.T) { } // Reading it should return the key as well as upgrade the storage path - publicKeyEntry, err := readStoredKey(context.Background(), config.StorageView, caPublicKey) + publicKeyEntry, err := readStoredKey(context.Background(), config.StorageView, caPublicKey, true) if err != nil { t.Fatal(err) } @@ -180,6 +182,28 @@ func TestSSH_ConfigCAUpdateDelete(t *testing.T) { if err != nil || (resp != nil && resp.IsError()) { t.Fatalf("bad: err: %v, resp:%v", err, resp) } + + // verify deletion of keys on deprecated path + err = config.StorageView.Put(context.Background(), &logical.StorageEntry{ + Key: caPublicKeyStoragePathDeprecated, + Value: []byte(testCAPublicKey), + }) + require.NoError(t, err) + err = config.StorageView.Put(context.Background(), &logical.StorageEntry{ + Key: caPrivateKeyStoragePathDeprecated, + Value: []byte(testCAPrivateKey), + }) + require.NoError(t, err) + caReq.Operation = logical.DeleteOperation + resp, err = b.HandleRequest(context.Background(), caReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("bad: err: %v, resp:%v", err, resp) + } + // ensure it was deleted + caReq.Operation = logical.ReadOperation + resp, err = b.HandleRequest(context.Background(), caReq) + require.NoError(t, err) + require.Error(t, resp.Error()) } func createDeleteHelper(t *testing.T, b logical.Backend, config *logical.BackendConfig, index int, keyType string, keyBits int) { @@ -340,7 +364,7 @@ func TestReadStoredKey(t *testing.T) { t.Fatalf("error writing public key: %s", err) } - publicKeyEntry, err := readStoredKey(context.Background(), storage, caPublicKey) + publicKeyEntry, err := readStoredKey(context.Background(), storage, caPublicKey, true) if err != nil { t.Fatalf("error reading public key: %s", err) } @@ -349,7 +373,7 @@ func TestReadStoredKey(t *testing.T) { t.Fatalf("returned key does not match: expected %s, got %s", tt.publicKey, publicKeyEntry.Key) } - privateKeyEntry, err := readStoredKey(context.Background(), storage, caPrivateKey) + privateKeyEntry, err := readStoredKey(context.Background(), storage, caPrivateKey, true) if err != nil { t.Fatalf("error reading private key: %s", err) } @@ -387,7 +411,7 @@ func TestGetCAPublicKey(t *testing.T) { t.Fatalf("error writing key: %s", err) } - key, err := getCAPublicKey(ctx, storage) + key, err := getCAPublicKey(ctx, storage, true) if err != nil { t.Fatalf("error retrieving public key: %s", err) } @@ -586,3 +610,106 @@ func readKey(ctx context.Context, s logical.Storage, path string) error { return nil } + +// TestCARecover verifies secret recovery of the SSH CA +func TestCARecover(t *testing.T) { + var err error + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + + b, err := Factory(context.Background(), config) + if err != nil { + t.Fatalf("Cannot create backend: %s", err) + } + tc := snapshots.NewSnapshotTestCase(t, b) + + // generate CA keys on the snapshot storage + _, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config/ca", + Storage: tc.SnapshotStorage(), + Data: map[string]interface{}{ + "public_key": testCAPublicKey, + "private_key": testCAPrivateKey, + }, + }) + require.NoError(t, err) + + // write different CA to the regular storage + _, err = b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.UpdateOperation, + Path: "config/ca", + Storage: tc.RegularStorage(), + Data: map[string]interface{}{ + "generate_signing_key": true, + }, + }) + require.NoError(t, err) + t.Run("read no side effects", func(t *testing.T) { + tc.RunRead(t, "config/ca") + }) + + t.Run("recover succeeds", func(t *testing.T) { + tc.DoRecover(t, "config/ca") + data, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "config/ca", + Storage: tc.SnapshotStorage(), + Data: map[string]interface{}{ + "public_key": "should be the actual public key but the SSH CA recovery doesn't really care as it reads directly from snapshot storage", + }, + }) + require.NoError(t, err) + require.NotNil(t, data) + require.Equal(t, testCAPublicKey, data.Data["public_key"]) + }) +} + +// TestCARecoverMigration is the same as TestCARecover, but recovering from a snapshot with the data in the deprecated +// storage paths, which ensures that the migration logic is skipped during recovery. +func TestCARecoverMigration(t *testing.T) { + var err error + config := logical.TestBackendConfig() + config.StorageView = &logical.InmemStorage{} + + b, err := Factory(context.Background(), config) + if err != nil { + t.Fatalf("Cannot create backend: %s", err) + } + tc := snapshots.NewSnapshotTestCase(t, b) + err = tc.SnapshotStorage().Put(context.Background(), &logical.StorageEntry{ + Key: caPublicKeyStoragePathDeprecated, + Value: []byte(testCAPublicKey), + }) + require.NoError(t, err) + err = tc.SnapshotStorage().Put(context.Background(), &logical.StorageEntry{ + Key: caPrivateKeyStoragePathDeprecated, + Value: []byte(testCAPrivateKey), + }) + require.NoError(t, err) + + require.NoError(t, err) + t.Run("read no side effects", func(t *testing.T) { + tc.RunRead(t, "config/ca") + }) + + t.Run("recover succeeds", func(t *testing.T) { + tc.DoRecover(t, "config/ca") + // ensure that even though the migration on read is disabled, by reading from the old path in the snapshot and + // creating a entry in the regular storage, that the entry ends up in the new path + entry, err := tc.RegularStorage().Get(context.Background(), caPublicKeyStoragePathDeprecated) + require.NoError(t, err) + require.Nil(t, entry) + data, err := b.HandleRequest(context.Background(), &logical.Request{ + Operation: logical.ReadOperation, + Path: "config/ca", + Storage: tc.SnapshotStorage(), + Data: map[string]interface{}{ + "public_key": "should be the actual public key but the SSH CA recovery doesn't really care as it reads directly from snapshot storage", + }, + }) + require.NoError(t, err) + require.NotNil(t, data) + require.Equal(t, testCAPublicKey, data.Data["public_key"]) + }) +} diff --git a/builtin/logical/ssh/path_fetch.go b/builtin/logical/ssh/path_fetch.go index 870405dfad..e01ca59b22 100644 --- a/builtin/logical/ssh/path_fetch.go +++ b/builtin/logical/ssh/path_fetch.go @@ -29,7 +29,8 @@ func pathFetchPublicKey(b *backend) *framework.Path { } func (b *backend) pathFetchPublicKey(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - publicKey, err := getCAPublicKey(ctx, req.Storage) + const allowMigration = true // only paths that support snapshot reads are + publicKey, err := getCAPublicKey(ctx, req.Storage, allowMigration) if err != nil { return nil, err } diff --git a/builtin/logical/ssh/path_issue_sign.go b/builtin/logical/ssh/path_issue_sign.go index 0bb2e8316e..38c63ec46f 100644 --- a/builtin/logical/ssh/path_issue_sign.go +++ b/builtin/logical/ssh/path_issue_sign.go @@ -565,7 +565,8 @@ func createKeyTypeToMapKey(keyType string, keyBits int) map[string][]string { func (b *backend) getCASigner(ctx context.Context, s logical.Storage) (ssh.Signer, error) { var signer ssh.Signer - storedKey, err := readStoredKey(ctx, s, caPrivateKey) + const allowMigration = true // migration from deprecated paths is allowed when signing + storedKey, err := readStoredKey(ctx, s, caPrivateKey, allowMigration) if err != nil { return nil, fmt.Errorf("error reading stored key: %w", err) } diff --git a/builtin/logical/ssh/path_roles.go b/builtin/logical/ssh/path_roles.go index ba3aec21e4..e52b5c067a 100644 --- a/builtin/logical/ssh/path_roles.go +++ b/builtin/logical/ssh/path_roles.go @@ -594,7 +594,8 @@ func (b *backend) checkUpgrade(ctx context.Context, s logical.Storage, n string, // signing key type as we want to make ssh-rsa an explicitly notated // algorithm choice. var publicKey ssh.PublicKey - publicKeyStr, err := getCAPublicKey(ctx, s) + const allowPathMigration = true + publicKeyStr, err := getCAPublicKey(ctx, s, allowPathMigration) if err != nil { b.Logger().Debug(fmt.Sprintf("failed to load public key entry while attempting to migrate: %v", err)) goto SKIPVERSION2 diff --git a/changelog/_8581.txt b/changelog/_8581.txt new file mode 100644 index 0000000000..b1306514a5 --- /dev/null +++ b/changelog/_8581.txt @@ -0,0 +1,3 @@ +```release-note:improvement +secrets/ssh: Add support for recovering the SSH plugin CA from a loaded snapshot (enterprise only). +``` \ No newline at end of file