Skip to content

Commit

Permalink
fix: improves kms key validation across providers.
Browse files Browse the repository at this point in the history 
Signed-off-by: ianhundere <[email protected]>
  • Loading branch information
@ianhundere
ianhundere committed Dec 14, 2024
1 parent 30bf931 commit 625ebfc
Show file tree
Hide file tree
Showing 2 changed files with 224 additions and 41 deletions.
45 changes: 38 additions & 7 deletions pkg/certmaker/certmaker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,19 @@ func ValidateKMSConfig(config KMSConfig) error {
if keyID == "" {
return nil
}
if !strings.HasPrefix(keyID, "arn:aws:kms:") && !strings.HasPrefix(keyID, "alias/") {
if strings.HasPrefix(keyID, "arn:aws:kms:") {
parts := strings.Split(keyID, ":")
if len(parts) < 6 {
return fmt.Errorf("invalid AWS KMS ARN format for %s", keyType)
}
if parts[3] != config.Region {
return fmt.Errorf("region in ARN (%s) does not match configured region (%s)", parts[3], config.Region)
}
} else if strings.HasPrefix(keyID, "alias/") {
if strings.TrimPrefix(keyID, "alias/") == "" {
return fmt.Errorf("alias name cannot be empty for %s", keyType)
}
} else {
return fmt.Errorf("awskms %s must start with 'arn:aws:kms:' or 'alias/'", keyType)
}
return nil
Expand All @@ -249,11 +261,20 @@ func ValidateKMSConfig(config KMSConfig) error {
if keyID == "" {
return nil
}
if !strings.HasPrefix(keyID, "projects/") {
return fmt.Errorf("gcpkms %s must start with 'projects/'", keyType)
requiredComponents := []struct {
component string
message string
}{
{"projects/", "must start with 'projects/'"},
{"/locations/", "must contain '/locations/'"},
{"/keyRings/", "must contain '/keyRings/'"},
{"/cryptoKeys/", "must contain '/cryptoKeys/'"},
{"/cryptoKeyVersions/", "must contain '/cryptoKeyVersions/'"},
}
if !strings.Contains(keyID, "/locations/") || !strings.Contains(keyID, "/keyRings/") {
return fmt.Errorf("invalid gcpkms key format for %s: %s", keyType, keyID)
for _, req := range requiredComponents {
if !strings.Contains(keyID, req.component) {
return fmt.Errorf("gcpkms %s %s", keyType, req.message)
}
}
return nil
}
Expand All @@ -269,20 +290,30 @@ func ValidateKMSConfig(config KMSConfig) error {

case "azurekms":
// Azure KMS validation
if config.Options == nil {
return fmt.Errorf("options map is required for Azure KMS")
}
if config.Options["tenant-id"] == "" {
return fmt.Errorf("tenant-id is required for Azure KMS")
}
validateAzureKeyID := func(keyID, keyType string) error {
if keyID == "" {
return nil
}
// Validate format: azurekms:name=<key-name>;vault=<vault-name>
if !strings.HasPrefix(keyID, "azurekms:name=") {
return fmt.Errorf("azurekms %s must start with 'azurekms:name='", keyType)
}
if !strings.Contains(keyID, ";vault=") {
nameStart := strings.Index(keyID, "name=") + 5
vaultIndex := strings.Index(keyID, ";vault=")
if vaultIndex == -1 {
return fmt.Errorf("azurekms %s must contain ';vault=' parameter", keyType)
}
if strings.TrimSpace(keyID[nameStart:vaultIndex]) == "" {
return fmt.Errorf("key name cannot be empty for %s", keyType)
}
if strings.TrimSpace(keyID[vaultIndex+7:]) == "" {
return fmt.Errorf("vault name cannot be empty for %s", keyType)
}
return nil
}
if err := validateAzureKeyID(config.RootKeyID, "RootKeyID"); err != nil {
Expand Down
220 changes: 186 additions & 34 deletions pkg/certmaker/certmaker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,24 +145,24 @@ func TestValidateKMSConfig(t *testing.T) {
wantError: "awskms LeafKeyID must start with 'arn:aws:kms:' or 'alias/'",
},
{
name: "GCP KMS invalid root key ID",
name: "GCP_KMS_invalid_root_key_ID",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "invalid-key-id",
},
wantError: "gcpkms RootKeyID must start with 'projects/'",
},
{
name: "GCP KMS invalid intermediate key ID",
name: "GCP_KMS_invalid_intermediate_key_ID",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key",
RootKeyID: "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key/cryptoKeyVersions/1",
IntermediateKeyID: "invalid-key-id",
},
wantError: "gcpkms IntermediateKeyID must start with 'projects/'",
},
{
name: "GCP KMS invalid leaf key ID",
name: "GCP_KMS_invalid_leaf_key_ID",
config: KMSConfig{
Type: "gcpkms",
LeafKeyID: "invalid-key-id",
Expand All @@ -173,70 +173,86 @@ func TestValidateKMSConfig(t *testing.T) {
name: "GCP KMS missing required parts",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "projects/my-project",
RootKeyID: "projects/test-project",
},
wantError: "invalid gcpkms key format",
wantError: "gcpkms RootKeyID must contain '/locations/'",
},
{
name: "Azure KMS missing tenant ID",
name: "Azure_KMS_missing_tenant_ID",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=my-key;vault=my-vault",
Options: map[string]string{},
},
wantError: "tenant-id is required for Azure KMS",
},
{
name: "Azure KMS invalid root key ID prefix",
name: "Azure_KMS_missing_vault_parameter",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "invalid-key-id",
RootKeyID: "azurekms:name=my-key",
Options: map[string]string{
"tenant-id": "tenant-id",
},
},
wantError: "azurekms RootKeyID must start with 'azurekms:name='",
wantError: "azurekms RootKeyID must contain ';vault=' parameter",
},
{
name: "Azure KMS missing vault parameter",
name: "unsupported KMS type",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=my-key",
Options: map[string]string{
"tenant-id": "tenant-id",
},
Type: "invalidkms",
RootKeyID: "key-id",
},
wantError: "azurekms RootKeyID must contain ';vault=' parameter",
wantError: "unsupported KMS type",
},
{
name: "Azure KMS invalid intermediate key ID",
name: "aws_kms_invalid_arn_format",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=my-key;vault=my-vault",
IntermediateKeyID: "invalid-key-id",
Options: map[string]string{
"tenant-id": "tenant-id",
},
Type: "awskms",
Region: "us-west-2",
RootKeyID: "arn:aws:kms:us-west-2:invalid",
},
wantError: "invalid AWS KMS ARN format for RootKeyID",
},
{
name: "aws_kms_region_mismatch",
config: KMSConfig{
Type: "awskms",
Region: "us-west-2",
RootKeyID: "arn:aws:kms:us-east-1:123456789012:key/test-key",
},
wantError: "region in ARN (us-east-1) does not match configured region (us-west-2)",
},
{
name: "aws_kms_empty_alias",
config: KMSConfig{
Type: "awskms",
Region: "us-west-2",
RootKeyID: "alias/",
},
wantError: "azurekms IntermediateKeyID must start with 'azurekms:name='",
wantError: "alias name cannot be empty for RootKeyID",
},
{
name: "Azure KMS invalid leaf key ID",
name: "azure_kms_empty_key_name",
config: KMSConfig{
Type: "azurekms",
LeafKeyID: "invalid-key-id",
RootKeyID: "azurekms:name=;vault=test-vault",
Options: map[string]string{
"tenant-id": "tenant-id",
"tenant-id": "test-tenant",
},
},
wantError: "azurekms LeafKeyID must start with 'azurekms:name='",
wantError: "key name cannot be empty for RootKeyID",
},
{
name: "unsupported KMS type",
name: "azure_kms_empty_vault_name",
config: KMSConfig{
Type: "invalidkms",
RootKeyID: "key-id",
Type: "azurekms",
RootKeyID: "azurekms:name=test-key;vault=",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
wantError: "unsupported KMS type",
wantError: "vault name cannot be empty for RootKeyID",
},
}

Expand All @@ -262,7 +278,7 @@ func TestValidateKMSConfig(t *testing.T) {
},
{
Type: "gcpkms",
RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key",
RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key/cryptoKeyVersions/1",
},
{
Type: "azurekms",
Expand Down Expand Up @@ -681,3 +697,139 @@ func TestInitKMS(t *testing.T) {
})
}
}

func TestGCPKMSValidation(t *testing.T) {
tests := []struct {
name string
config KMSConfig
wantError string
}{
{
name: "gcp_kms_invalid_root_key_format",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "invalid-key-id",
},
wantError: "must start with 'projects/'",
},
{
name: "missing_required_components",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "projects/test-project",
},
wantError: "must contain '/locations/'",
},
{
name: "valid_GCP_key_format",
config: KMSConfig{
Type: "gcpkms",
RootKeyID: "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key/cryptoKeyVersions/1",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateKMSConfig(tt.config)
if tt.wantError != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantError)
} else {
require.NoError(t, err)
}
})
}
}

func TestAzureKMSValidation(t *testing.T) {
tests := []struct {
name string
config KMSConfig
wantError string
}{
{
name: "missing_options_map",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=mykey;vault=myvault",
},
wantError: "options map is required for Azure KMS",
},
{
name: "missing_tenant_id",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=mykey;vault=myvault",
Options: map[string]string{},
},
wantError: "tenant-id is required for Azure KMS",
},
{
name: "invalid_key_format",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "invalid-format",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
wantError: "azurekms RootKeyID must start with 'azurekms:name='",
},
{
name: "missing_vault_name",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=mykey",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
wantError: "azurekms RootKeyID must contain ';vault=' parameter",
},
{
name: "empty_key_name",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=;vault=test-vault",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
wantError: "key name cannot be empty for RootKeyID",
},
{
name: "empty_vault_name",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=test-key;vault=",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
wantError: "vault name cannot be empty for RootKeyID",
},
{
name: "valid_config",
config: KMSConfig{
Type: "azurekms",
RootKeyID: "azurekms:name=mykey;vault=myvault",
Options: map[string]string{
"tenant-id": "test-tenant",
},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateKMSConfig(tt.config)
if tt.wantError != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantError)
} else {
require.NoError(t, err)
}
})
}
}

0 comments on commit 625ebfc

Please sign in to comment.