upstream/vault
1
0
Fork 0
mirror of https://github.com/hashicorp/vault.git synced 2026-02-03 20:40:45 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

VAULT-37632 allow restoring SSH CA from loaded snapshot (#8581) (#9034)

Browse source 
* allow restoring ssh config/ca

* add some unit tests

* address PR review

* imports and test upgrades

* linter complaints

* add PR comment and linter fixes

* address review

Co-authored-by: Bruno Oliveira de Souza <bruno.souza@hashicorp.com>
This commit is contained in:
Vault Automation 2025-09-02 08:27:23 -06:00 committed by GitHub
parent e40eca1286
commit 66a27fd4bd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 244 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
builtin/logical/ssh/backend.go
View file

@ -56,6 +56,10 @@ func Backend(conf *logical.BackendConfig) (*backend, error) {
caPrivateKeyStoragePath,
keysStoragePrefix,
},
AllowSnapshotRead: []string{
"config/ca",
},
},
Paths: []*framework.Path{

111
builtin/logical/ssh/path_config_ca.go
View file

@ -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
}

137
builtin/logical/ssh/path_config_ca_test.go
View file

@ -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"])
})
}

3
builtin/logical/ssh/path_fetch.go
View file

@ -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
}

3
builtin/logical/ssh/path_issue_sign.go
View file

@ -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)
}

3
builtin/logical/ssh/path_roles.go
View file

@ -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

3
changelog/_8581.txt Normal file
View file

@ -0,0 +1,3 @@
```release-note:improvement
secrets/ssh: Add support for recovering the SSH plugin CA from a loaded snapshot (enterprise only).
```