Skip to content

Commit

Permalink
kms: allow multi-region kms keys to be enabled (#47672)
Browse files Browse the repository at this point in the history 
* kms: allow multi-region kms keys to be enabled

* add unit tests

* fix logging key lint

---------

Co-authored-by: Nic Klaassen <[email protected]>
  • Loading branch information
@dboslee @nklaassen
dboslee and nklaassen authored Oct 28, 2024
1 parent 239723a commit 37c1358
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 55 deletions.
31 changes: 18 additions & 13 deletions lib/auth/keystore/aws_kms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,13 @@ const (
)

type awsKMSKeystore struct {
kms kmsClient
clusterName types.ClusterName
awsAccount string
awsRegion string
clock clockwork.Clock
logger *slog.Logger
kms kmsClient
clusterName types.ClusterName
awsAccount string
awsRegion string
multiRegionEnabled bool
clock clockwork.Clock
logger *slog.Logger
}

func newAWSKMSKeystore(ctx context.Context, cfg *servicecfg.AWSKMSConfig, opts *Options) (*awsKMSKeystore, error) {
Expand Down Expand Up @@ -99,12 +100,13 @@ func newAWSKMSKeystore(ctx context.Context, cfg *servicecfg.AWSKMSConfig, opts *
clock = clockwork.NewRealClock()
}
return &awsKMSKeystore{
clusterName: opts.ClusterName,
awsAccount: cfg.AWSAccount,
awsRegion: cfg.AWSRegion,
kms: kmsClient,
clock: clock,
logger: opts.Logger,
clusterName: opts.ClusterName,
awsAccount: cfg.AWSAccount,
awsRegion: cfg.AWSRegion,
multiRegionEnabled: cfg.MultiRegion.Enabled,
kms: kmsClient,
clock: clock,
logger: opts.Logger,
}, nil
}

Expand All @@ -126,7 +128,9 @@ func (a *awsKMSKeystore) generateKey(ctx context.Context, algorithm cryptosuites
return nil, nil, trace.Wrap(err)
}

a.logger.InfoContext(ctx, "Creating new AWS KMS keypair.", "algorithm", alg)
a.logger.InfoContext(ctx, "Creating new AWS KMS keypair.",
slog.Any("algorithm", algorithm),
slog.Bool("multi_region", a.multiRegionEnabled))

output, err := a.kms.CreateKey(ctx, &kms.CreateKeyInput{
Description: aws.String("Teleport CA key"),
Expand All @@ -138,6 +142,7 @@ func (a *awsKMSKeystore) generateKey(ctx context.Context, algorithm cryptosuites
TagValue: aws.String(a.clusterName.GetClusterName()),
},
},
MultiRegion: aws.Bool(a.multiRegionEnabled),
})
if err != nil {
return nil, nil, trace.Wrap(err)
Expand Down
58 changes: 58 additions & 0 deletions lib/auth/keystore/aws_kms_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,60 @@ func TestAWSKMS_RetryWhilePending(t *testing.T) {
require.Error(t, err)
}

// TestMultiRegionKeys asserts that a keystore created with multi-region enabled
// correctly passes this argument to the AWS client. This gives very little real
// coverage since the AWS KMS service here is faked, but at least we know the
// keystore passed the bool to the client correctly. TestBackends and
// TestManager are both able to run with a real AWS KMS client and you can
// confirm the keys are really multi-region there.
func TestMultiRegionKeys(t *testing.T) {
ctx := context.Background()
clock := clockwork.NewFakeClock()

const pageSize int = 4
fakeKMS := newFakeAWSKMSService(t, clock, "123456789012", "us-west-2", pageSize)
clusterName, err := services.NewClusterNameWithRandomID(types.ClusterNameSpecV2{ClusterName: "test-cluster"})
require.NoError(t, err)
opts := &Options{
ClusterName: clusterName,
HostUUID: "uuid",
AuthPreferenceGetter: &fakeAuthPreferenceGetter{types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_HSM_V1},
awsKMSClient: fakeKMS,
awsSTSClient: &fakeAWSSTSClient{
account: "123456789012",
},
clockworkOverride: clock,
}

for _, multiRegion := range []bool{false, true} {
t.Run(fmt.Sprint(multiRegion), func(t *testing.T) {
cfg := servicecfg.KeystoreConfig{
AWSKMS: servicecfg.AWSKMSConfig{
AWSAccount: "123456789012",
AWSRegion: "us-west-2",
MultiRegion: struct{ Enabled bool }{
Enabled: multiRegion,
},
},
}
keyStore, err := NewManager(ctx, &cfg, opts)
require.NoError(t, err)

sshKeyPair, err := keyStore.NewSSHKeyPair(ctx, cryptosuites.UserCASSH)
require.NoError(t, err)

keyID, err := parseAWSKMSKeyID(sshKeyPair.PrivateKey)
require.NoError(t, err)

if multiRegion {
assert.Contains(t, keyID.arn, "mrk-")
} else {
assert.NotContains(t, keyID.arn, "mrk-")
}
})
}
}

type fakeAWSKMSService struct {
keys []*fakeAWSKMSKey
clock clockwork.Clock
Expand Down Expand Up @@ -239,6 +293,10 @@ type fakeAWSKMSKey struct {

func (f *fakeAWSKMSService) CreateKey(_ context.Context, input *kms.CreateKeyInput, _ ...func(*kms.Options)) (*kms.CreateKeyOutput, error) {
id := uuid.NewString()
if aws.ToBool(input.MultiRegion) {
// AWS does this https://docs.aws.amazon.com/kms/latest/developerguide/concepts.html#key-id-key-ARN
id = "mrk-" + id
}
a := arn.ARN{
Partition: "aws",
Service: "kms",
Expand Down
103 changes: 61 additions & 42 deletions lib/auth/keystore/keystore_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,7 @@ func newTestPack(ctx context.Context, t *testing.T) *testPack {
})
}

// Test with real GCP account if environment is enabled.
if config, ok := gcpKMSTestConfig(t); ok {
opts := baseOpts
opts.kmsClient = nil
Expand All @@ -621,6 +622,7 @@ func newTestPack(ctx context.Context, t *testing.T) *testPack {
}.marshal(),
})
}
// Always test with fake GCP client.
fakeGCPKMSConfig := servicecfg.KeystoreConfig{
GCPKMS: servicecfg.GCPKMSConfig{
ProtectionLevel: "HSM",
Expand All @@ -640,61 +642,78 @@ func newTestPack(ctx context.Context, t *testing.T) *testPack {
}.marshal(),
})

if config, ok := awsKMSTestConfig(t); ok {
opts := baseOpts
// Unset the fake clients so this test can use the real AWS clients.
opts.awsKMSClient = nil
opts.awsSTSClient = nil

backend, err := newAWSKMSKeystore(ctx, &config.AWSKMS, &opts)
// Test AWS with and without multi-region keys
for _, multiRegion := range []bool{false, true} {
// Test with real AWS account if environment is enabled.
if config, ok := awsKMSTestConfig(t); ok {
config.AWSKMS.MultiRegion.Enabled = multiRegion
opts := baseOpts
// Unset the fake clients so this test can use the real AWS clients.
opts.awsKMSClient = nil
opts.awsSTSClient = nil

backend, err := newAWSKMSKeystore(ctx, &config.AWSKMS, &opts)
require.NoError(t, err)
name := "aws_kms"
if multiRegion {
name += "_multi_region"
}
backends = append(backends, &backendDesc{
name: name,
config: config,
opts: &opts,
backend: backend,
expectedKeyType: types.PrivateKeyType_AWS_KMS,
unusedRawKey: awsKMSKeyID{
arn: arn.ARN{
Partition: "aws",
Service: "kms",
Region: config.AWSKMS.AWSRegion,
AccountID: config.AWSKMS.AWSAccount,
Resource: "unused",
}.String(),
account: config.AWSKMS.AWSAccount,
region: config.AWSKMS.AWSRegion,
}.marshal(),
})
}

// Always test with fake AWS client.
fakeAWSKMSConfig := servicecfg.KeystoreConfig{
AWSKMS: servicecfg.AWSKMSConfig{
AWSAccount: "123456789012",
AWSRegion: "us-west-2",
MultiRegion: struct{ Enabled bool }{
Enabled: multiRegion,
},
},
}
fakeAWSKMSBackend, err := newAWSKMSKeystore(ctx, &fakeAWSKMSConfig.AWSKMS, &baseOpts)
require.NoError(t, err)
name := "fake_aws_kms"
if multiRegion {
name += "_multi_region"
}
backends = append(backends, &backendDesc{
name: "aws_kms",
config: config,
opts: &opts,
backend: backend,
name: name,
config: fakeAWSKMSConfig,
opts: &baseOpts,
backend: fakeAWSKMSBackend,
expectedKeyType: types.PrivateKeyType_AWS_KMS,
unusedRawKey: awsKMSKeyID{
arn: arn.ARN{
Partition: "aws",
Service: "kms",
Region: config.AWSKMS.AWSRegion,
AccountID: config.AWSKMS.AWSAccount,
Region: "us-west-2",
AccountID: "123456789012",
Resource: "unused",
}.String(),
account: config.AWSKMS.AWSAccount,
region: config.AWSKMS.AWSRegion,
account: "123456789012",
region: "us-west-2",
}.marshal(),
})
}

fakeAWSKMSConfig := servicecfg.KeystoreConfig{
AWSKMS: servicecfg.AWSKMSConfig{
AWSAccount: "123456789012",
AWSRegion: "us-west-2",
},
}
fakeAWSKMSBackend, err := newAWSKMSKeystore(ctx, &fakeAWSKMSConfig.AWSKMS, &baseOpts)
require.NoError(t, err)
backends = append(backends, &backendDesc{
name: "fake_aws_kms",
config: fakeAWSKMSConfig,
opts: &baseOpts,
backend: fakeAWSKMSBackend,
expectedKeyType: types.PrivateKeyType_AWS_KMS,
unusedRawKey: awsKMSKeyID{
arn: arn.ARN{
Partition: "aws",
Service: "kms",
Region: "us-west-2",
AccountID: "123456789012",
Resource: "unused",
}.String(),
account: "123456789012",
region: "us-west-2",
}.marshal(),
})

return &testPack{
backends: backends,
clock: clock,
Expand Down
1 change: 1 addition & 0 deletions lib/config/configuration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1169,6 +1169,7 @@ func applyAWSKMSConfig(kmsConfig *AWSKMS, cfg *servicecfg.Config) error {
return trace.BadParameter("must set region in ca_key_params.aws_kms")
}
cfg.Auth.KeyStore.AWSKMS.AWSRegion = kmsConfig.Region
cfg.Auth.KeyStore.AWSKMS.MultiRegion = kmsConfig.MultiRegion
return nil
}

Expand Down
5 changes: 5 additions & 0 deletions lib/config/fileconf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,11 @@ type AWSKMS struct {
Account string `yaml:"account"`
// Region is the AWS region to use.
Region string `yaml:"region"`
// MultiRegion contains configuration for multi-region AWS KMS.
MultiRegion struct {
// Enabled configures new keys to be multi-region.
Enabled bool
} `yaml:"multi_region,omitempty"`
}

// TrustedCluster struct holds configuration values under "trusted_clusters" key
Expand Down
5 changes: 5 additions & 0 deletions lib/service/servicecfg/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ type AWSKMSConfig struct {
AWSAccount string
// AWSRegion is the AWS region where the keys will reside.
AWSRegion string
// MultiRegion contains configuration for multi-region AWS KMS.
MultiRegion struct {
// Enabled configures new keys to be multi-region.
Enabled bool
}
}

// CheckAndSetDefaults checks that required parameters of the config are
Expand Down

0 comments on commit 37c1358

Please sign in to comment.