Skip to content

Commit

Permalink
PR feedback changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Keegan Witt <keeganwitt@gmail.com>
  • Loading branch information
@keeganwitt
keeganwitt committed Dec 5, 2023
1 parent a6957bb commit ece2cb2
Show file tree
Hide file tree
Showing 9 changed files with 319 additions and 129 deletions.
17 changes: 9 additions & 8 deletions doc/plugin_server_keymanager_aws_kms.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ The `aws_kms` key manager plugin leverages the AWS Key Management Service (KMS)

The plugin accepts the following configuration options:

| Key | Type | Required | Description | Default |
|-------------------|--------|---------------------------------------|-----------------------------------------------------------------------------------------|---------------------------------------------------------|
| access_key_id | string | see [AWS KMS Access](#aws-kms-access) | The Access Key Id used to authenticate to KMS | Value of the AWS_ACCESS_KEY_ID environment variable |
| secret_access_key | string | see [AWS KMS Access](#aws-kms-access) | The Secret Access Key used to authenticate to KMS | Value of the AWS_SECRET_ACCESS_KEY environment variable |
| region | string | yes | The region where the keys will be stored | |
| key_metadata_file | string | yes | A file path location where information about generated keys will be persisted | |
| key_metadata | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | |
| key_policy_file | string | no | A file path location to a custom key policy in JSON format | "" |
| Key | Type | Required | Description | Default |
|----------------------|--------|---------------------------------------|--------------------------------------------------------------------------------------------|---------------------------------------------------------|
| access_key_id | string | see [AWS KMS Access](#aws-kms-access) | The Access Key Id used to authenticate to KMS | Value of the AWS_ACCESS_KEY_ID environment variable |
| secret_access_key | string | see [AWS KMS Access](#aws-kms-access) | The Secret Access Key used to authenticate to KMS | Value of the AWS_SECRET_ACCESS_KEY environment variable |
| region | string | yes | The region where the keys will be stored | |
| key_metadata_file | string | yes | A file path location where information about generated keys will be persisted (deprecated) | |
| key_identifier_file | string | yes | A file path location where information about generated keys will be persisted | |
| key_identifier_value | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | |
| key_policy_file | string | no | A file path location to a custom key policy in JSON format | "" |

### Alias and Key Management

Expand Down
23 changes: 12 additions & 11 deletions doc/plugin_server_keymanager_azure_key_vault.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@ SPIRE.

The plugin accepts the following configuration options:

| Key | Type | Required | Description | Default |
|-------------------|---------|----------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------|---------|
| key_metadata_file | string | yes | A file path location where key metadata used by the plugin will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_metadata | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | "" |
| key_vault_uri | string | Yes | The Key Vault URI where the keys managed by this plugin reside. | "" |
| use_msi | boolean | [Deprecated](#authenticating-to-azure) | Whether or not to use MSI to authenticate to Azure Key Vault. | false |
| subscription_id | string | [Optional](#authenticating-to-azure) | The subscription id. | "" |
| app_id | string | [Optional](#authenticating-to-azure) | The application id. | "" |
| app_secret | string | [Optional](#authenticating-to-azure) | The application secret. | "" |
| tenant_id | string | [Optional](#authenticating-to-azure) | The tenant id. | "" |
| Key | Type | Required | Description | Default |
|----------------------|---------|----------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------|
| key_metadata_file | string | yes | A file path location where key metadata used by the plugin will be persisted (deprecated). See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_identifier_file | string | yes | A file path location where key metadata used by the plugin will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_identifier_value | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | "" |
| key_vault_uri | string | Yes | The Key Vault URI where the keys managed by this plugin reside. | "" |
| use_msi | boolean | [Deprecated](#authenticating-to-azure) | Whether or not to use MSI to authenticate to Azure Key Vault. | false |
| subscription_id | string | [Optional](#authenticating-to-azure) | The subscription id. | "" |
| app_id | string | [Optional](#authenticating-to-azure) | The application id. | "" |
| app_secret | string | [Optional](#authenticating-to-azure) | The application secret. | "" |
| tenant_id | string | [Optional](#authenticating-to-azure) | The tenant id. | "" |

### Authenticating to Azure

Expand Down Expand Up @@ -53,7 +54,7 @@ following table is provided for informational purposes only:

| Label | Description |
|-----------------|----------------------------------------------------------------------------------------------------------------------------------------|
| spire-server-td | A string representing the trust domain name of the server. |
| spire-server-td | A string representing the trust domain name of the server. |
| spire-server-id | Auto-generated ID that is unique to the server and is persisted in the _Key Metadata File_ (see the `key_metadata_file` configurable). |

If the _Key Metadata File_ is not found during server startup, the file is
Expand Down
17 changes: 9 additions & 8 deletions doc/plugin_server_keymanager_gcp_kms.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ The plugin accepts the following configuration options:
| Key | Type | Required | Description | Default |
|----------------------|--------|----------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------|
| key_policy_file | string | no | A file path location to a custom [IAM Policy (v3)](https://cloud.google.com/pubsub/docs/reference/rpc/google.iam.v1#google.iam.v1.Policy) in JSON format to be attached to created CryptoKeys. | "" |
| key_metadata_file | string | yes | A file path location where key metadata used by the plugin will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_metadata | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | "" |
| key_metadata_file | string | yes | A file path location where key metadata used by the plugin will be persisted (deprecated). See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_identifier_file | string | yes | A file path location where key metadata used by the plugin will be persisted. See "[Management of keys](#management-of-keys)" for more information. | "" |
| key_identifier_value | string | yes | A static identifier for the SPIRE server instance (used instead of `key_metadata_file`) | "" |
| key_ring | string | yes | Resource ID of the key ring where the keys managed by this plugin reside, in the format projects/\*/locations/\*/keyRings/\* | "" |
| service_account_file | string | no | Path to the service account file used to authenticate with the Cloud KMS API. | Value of `GOOGLE_APPLICATION_CREDENTIALS` environment variable. |

Expand Down Expand Up @@ -50,12 +51,12 @@ the service. All the labels are named with the `spire-` prefix.
Users don't need to interact with the labels managed by the plugin. The
following table is provided for informational purposes only:

| Label | Description |
| ----- | ----------- |
| spire-server-td | SHA-1 checksum of the trust domain name of the server. |
| spire-server-id | Auto-generated ID that is unique to the server and is persisted in the _Key Metadata File_ (see the `key_metadata_file` configurable). |
| spire-last-update | Unix time of the last time that the plugin updated the CryptoKey to keep it active. |
| spire-active | Indicates if the CryptoKey is still in use by the plugin. |
| Label | Description |
|-------------------|----------------------------------------------------------------------------------------------------------------------------------------|
| spire-server-td | SHA-1 checksum of the trust domain name of the server. |
| spire-server-id | Auto-generated ID that is unique to the server and is persisted in the _Key Metadata File_ (see the `key_metadata_file` configurable). |
| spire-last-update | Unix time of the last time that the plugin updated the CryptoKey to keep it active. |
| spire-active | Indicates if the CryptoKey is still in use by the plugin. |

If the _Key Metadata File_ is not found during server startup, the file is
recreated, with a new auto-generated server ID. Consequently, if the file is
Expand Down
58 changes: 43 additions & 15 deletions pkg/server/plugin/keymanager/awskms/awskms.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"os"
"path"
"regexp"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -92,12 +93,13 @@ type Plugin struct {

// Config provides configuration context for the plugin
type Config struct {
AccessKeyID string `hcl:"access_key_id" json:"access_key_id"`
SecretAccessKey string `hcl:"secret_access_key" json:"secret_access_key"`
Region string `hcl:"region" json:"region"`
KeyMetadataFile string `hcl:"key_metadata_file" json:"key_metadata_file"`
KeyMetadata string `hcl:"key_metadata" json:"key_metadata"`
KeyPolicyFile string `hcl:"key_policy_file" json:"key_policy_file"`
AccessKeyID string `hcl:"access_key_id" json:"access_key_id"`
SecretAccessKey string `hcl:"secret_access_key" json:"secret_access_key"`
Region string `hcl:"region" json:"region"`
KeyMetadataFile string `hcl:"key_metadata_file" json:"key_metadata_file"`
KeyIdentifierFile string `hcl:"key_identifier_file" json:"key_identifier_file"`
KeyIdentifierValue string `hcl:"key_identifier_value" json:"key_identifier_value"`
KeyPolicyFile string `hcl:"key_policy_file" json:"key_policy_file"`
}

// New returns an instantiated plugin
Expand Down Expand Up @@ -132,22 +134,30 @@ func (p *Plugin) Configure(ctx context.Context, req *configv1.ConfigureRequest)
return nil, err
}

var serverID = config.KeyMetadata
if config.KeyMetadata == "" {
if config.KeyPolicyFile != "" {
policyBytes, err := os.ReadFile(config.KeyPolicyFile)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to read file configured in 'key_policy_file': %v", err)
}
policyStr := string(policyBytes)
p.keyPolicy = &policyStr
}

var serverID = config.KeyIdentifierValue
if serverID == "" && config.KeyMetadataFile != "" {
p.log.Warn("`key_metadata_file` is deprecated in favor of `key_identifier_file` and will be removed in a future version")
serverID, err = getOrCreateServerID(config.KeyMetadataFile)
if err != nil {
return nil, err
}
p.log.Debug("Loaded server id", "server_id", serverID)
}

if config.KeyPolicyFile != "" {
policyBytes, err := os.ReadFile(config.KeyPolicyFile)
if serverID == "" && config.KeyIdentifierFile != "" {
serverID, err = getOrCreateServerID(config.KeyIdentifierFile)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to read file configured in 'key_policy_file': %v", err)
return nil, err
}
policyStr := string(policyBytes)
p.keyPolicy = &policyStr
p.log.Debug("Loaded server id", "server_id", serverID)
}

awsCfg, err := newAWSConfig(ctx, config)
Expand Down Expand Up @@ -836,9 +846,27 @@ func parseAndValidateConfig(c string) (*Config, error) {
return nil, status.Error(codes.InvalidArgument, "configuration is missing a region")
}

if config.KeyMetadataFile == "" && config.KeyMetadata == "" {
if config.KeyIdentifierValue != "" {
re := regexp.MustCompile(".*[^A-z0-9/_-].*")
if re.MatchString(config.KeyIdentifierValue) {
return nil, status.Error(codes.InvalidArgument, "Key identifier must contain only alphanumeric characters, forward slashes (/), underscores (_), and dashes (-)")
}
if strings.HasPrefix(config.KeyIdentifierValue, "alias/aws/") {
return nil, status.Error(codes.InvalidArgument, "Key identifier must not start with alias/aws/")
}
if len(config.KeyIdentifierValue) > 256 {
return nil, status.Error(codes.InvalidArgument, "Key identifier must not be longer than 256 characters")
}
}
if config.KeyMetadataFile == "" && config.KeyIdentifierFile == "" && config.KeyIdentifierValue == "" {
return nil, status.Error(codes.InvalidArgument, "configuration requires server id or server id file path")
}
if (config.KeyMetadataFile != "" || config.KeyIdentifierFile != "") && config.KeyIdentifierValue != "" {
return nil, status.Error(codes.InvalidArgument, "configuration must not contain both server id and server id file path")
}
if config.KeyMetadataFile != "" && config.KeyIdentifierFile != "" {
return nil, status.Error(codes.InvalidArgument, "configuration must not contain both `key_identifier_file` and deprecated `key_metadata_file`")
}

return config, nil
}
Expand Down
Loading

0 comments on commit ece2cb2

Please sign in to comment.