fix: improves kms key validation across providers.

Signed-off-by: ianhundere <[email protected]>

cmd/certificate_maker/certificate_maker_test.go

@@ -18,11 +18,10 @@ package main
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestGetConfigValue(t *testing.T) {
@@ -84,22 +83,27 @@ func TestGetConfigValue(t *testing.T) {
 				defer os.Unsetenv(tt.envVar)
 			}
 			got := getConfigValue(tt.flagValue, tt.envVar)
-			assert.Equal(t, tt.want, got)
+			if got != tt.want {
+				t.Errorf("got %v, want %v", got, tt.want)
+			}
 		})
 	}
 }
 
 func TestInitLogger(t *testing.T) {
 	logger := initLogger()
-	require.NotNil(t, logger)
+	if logger == nil {
+		t.Errorf("logger is nil")
+	}
 }
 
 func TestRunCreate(t *testing.T) {
 	tmpDir, err := os.MkdirTemp("", "cert-test-*")
-	require.NoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	defer os.RemoveAll(tmpDir)
 
-	// Create test template files
 	rootTemplate := `{
 		"subject": {
 			"commonName": "Test TSA Root CA"
@@ -132,9 +136,13 @@ func TestRunCreate(t *testing.T) {
 	rootTmplPath := filepath.Join(tmpDir, "root-template.json")
 	leafTmplPath := filepath.Join(tmpDir, "leaf-template.json")
 	err = os.WriteFile(rootTmplPath, []byte(rootTemplate), 0600)
-	require.NoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 	err = os.WriteFile(leafTmplPath, []byte(leafTemplate), 0600)
-	require.NoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
 	tests := []struct {
 		name      string
@@ -223,7 +231,6 @@ func TestRunCreate(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Set environment variables
 			for k, v := range tt.envVars {
 				os.Setenv(k, v)
 				defer os.Unsetenv(k)
@@ -234,7 +241,6 @@ func TestRunCreate(t *testing.T) {
 				RunE: runCreate,
 			}
 
-			// Add all flags that runCreate expects
 			cmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS provider type (awskms, gcpkms, azurekms)")
 			cmd.Flags().StringVar(&kmsRegion, "aws-region", "", "AWS KMS region")
 			cmd.Flags().StringVar(&kmsKeyID, "kms-key-id", "", "KMS key identifier")
@@ -254,58 +260,72 @@ func TestRunCreate(t *testing.T) {
 			err := cmd.Execute()
 
 			if tt.wantError {
-				require.Error(t, err)
-				assert.Contains(t, err.Error(), tt.errMsg)
+				if err == nil {
+					t.Errorf("expected error, but got nil")
+				} else if !strings.Contains(err.Error(), tt.errMsg) {
+					t.Errorf("error %q should contain %q", err.Error(), tt.errMsg)
+				}
 			} else {
-				require.NoError(t, err)
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
 			}
 		})
 	}
 }
 
 func TestCreateCommand(t *testing.T) {
-	// Create a test command
 	cmd := &cobra.Command{
 		Use: "test",
 		RunE: func(_ *cobra.Command, _ []string) error {
 			return nil
 		},
 	}
 
-	// Add flags
 	cmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS type")
 	cmd.Flags().StringVar(&kmsRegion, "aws-region", "", "AWS KMS region")
 	cmd.Flags().StringVar(&rootKeyID, "root-key-id", "", "Root key ID")
 	cmd.Flags().StringVar(&leafKeyID, "leaf-key-id", "", "Leaf key ID")
 
-	// Test missing required flags
 	err := cmd.Execute()
-	require.NoError(t, err) // No required flags set yet
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
-	// Test flag parsing
 	err = cmd.ParseFlags([]string{
 		"--kms-type", "awskms",
 		"--aws-region", "us-west-2",
 		"--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab",
 		"--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654",
 	})
-	require.NoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
-	// Verify flag values
-	assert.Equal(t, "awskms", kmsType)
-	assert.Equal(t, "us-west-2", kmsRegion)
-	assert.Equal(t, "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab", rootKeyID)
-	assert.Equal(t, "arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654", leafKeyID)
+	if kmsType != "awskms" {
+		t.Errorf("got kms-type %v, want awskms", kmsType)
+	}
+	if kmsRegion != "us-west-2" {
+		t.Errorf("got aws-region %v, want us-west-2", kmsRegion)
+	}
+	if rootKeyID != "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab" {
+		t.Errorf("got root-key-id %v, want arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab", rootKeyID)
+	}
+	if leafKeyID != "arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654" {
+		t.Errorf("got leaf-key-id %v, want arn:aws:kms:us-west-2:123456789012:key/9876fedc-ba98-7654-3210-fedcba987654", leafKeyID)
+	}
 }
 
 func TestRootCommand(t *testing.T) {
-	// Test help output
 	rootCmd.SetArgs([]string{"--help"})
 	err := rootCmd.Execute()
-	require.NoError(t, err)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
 
-	// Test unknown command
 	rootCmd.SetArgs([]string{"unknown"})
 	err = rootCmd.Execute()
-	require.Error(t, err)
+	if err == nil {
+		t.Errorf("expected error, but got nil")
+	}
 }

go.mod

@@ -31,7 +31,6 @@ require (
 	github.com/spf13/cobra v1.8.1
 	github.com/spf13/pflag v1.0.5
 	github.com/spf13/viper v1.19.0
-	github.com/stretchr/testify v1.9.0
 	github.com/urfave/negroni v1.0.0
 	go.step.sm/crypto v0.55.0
 	go.uber.org/zap v1.27.0
@@ -47,7 +46,7 @@ require (
 	cloud.google.com/go/auth/oauth2adapt v0.2.6 // indirect
 	cloud.google.com/go/compute/metadata v0.5.2 // indirect
 	cloud.google.com/go/iam v1.2.2 // indirect
-	cloud.google.com/go/kms v1.20.1 // indirect
+	cloud.google.com/go/kms v1.20.2 // indirect
 	cloud.google.com/go/longrunning v0.6.2 // indirect
 	dario.cat/mergo v1.0.1 // indirect
 	filippo.io/edwards25519 v1.1.0 // indirect
@@ -56,9 +55,9 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys v0.10.0 // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.1 // indirect
-	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.1.0 // indirect
-	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0 // indirect
-	github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.3.0 // indirect
+	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0 // indirect
+	github.com/AzureAD/microsoft-authentication-library-for-go v1.3.1 // indirect
 	github.com/Masterminds/goutils v1.1.1 // indirect
 	github.com/Masterminds/semver/v3 v3.3.0 // indirect
 	github.com/Masterminds/sprig/v3 v3.3.0 // indirect
@@ -82,7 +81,6 @@ require (
 	github.com/cenkalti/backoff/v4 v4.3.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/common-nighthawk/go-figure v0.0.0-20210622060536-734e95fb86be // indirect
-	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
 	github.com/docker/go-units v0.5.0 // indirect
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
@@ -133,7 +131,6 @@ require (
 	github.com/opentracing/opentracing-go v1.2.0 // indirect
 	github.com/pelletier/go-toml/v2 v2.2.2 // indirect
 	github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
-	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/client_model v0.6.1 // indirect
 	github.com/prometheus/common v0.55.0 // indirect
 	github.com/prometheus/procfs v0.15.1 // indirect

go.sum

@@ -31,10 +31,10 @@ github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys v0.10.0 h1:m/sWOGCREuSBqg2
 github.com/Azure/azure-sdk-for-go/sdk/keyvault/azkeys v0.10.0/go.mod h1:Pu5Zksi2KrU7LPbZbNINx6fuVrUp/ffvpxdDj+i8LeE=
 github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.1 h1:FbH3BbSb4bvGluTesZZ+ttN/MDsnMmQP36OSnDuSXqw=
 github.com/Azure/azure-sdk-for-go/sdk/keyvault/internal v0.7.1/go.mod h1:9V2j0jn9jDEkCkv8w/bKTNppX/d0FVA1ud77xCIP4KA=
-github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.1.0 h1:DRiANoJTiW6obBQe3SqZizkuV1PEgfiiGivmVocDy64=
-github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.1.0/go.mod h1:qLIye2hwb/ZouqhpSD9Zn3SJipvpEnz1Ywl3VUk9Y0s=
-github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0 h1:D3occbWoio4EBLkbkevetNMAVX197GkzbUMtqjGWn80=
-github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.0.0/go.mod h1:bTSOgj05NGRuHHhQwAdPnYr9TOdNmKlZTgGLL6nyAdI=
+github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.3.0 h1:7rKG7UmnrxX4N53TFhkYqjc+kVUZuw0fL8I3Fh+Ld9E=
+github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.3.0/go.mod h1:Wjo+24QJVhhl/L7jy6w9yzFF2yDOf3cKECAa8ecf9vE=
+github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0 h1:eXnN9kaS8TiDwXjoie3hMRLuwdUBUMW9KRgOqB3mCaw=
+github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.1.0/go.mod h1:XIpam8wumeZ5rVMuhdDQLMfIPDf1WO3IzrCRO3e3e3o=
 github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJTmL004Abzc5wDB5VtZG2PJk5ndYDgVacGqfirKxjM=
 github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE=
 github.com/AzureAD/microsoft-authentication-library-for-go v1.3.1 h1:gUDtaZk8heteyfdmv+pcfHvhR9llnh7c7GMwZ8RVG04=
@@ -322,16 +322,16 @@ github.com/secure-systems-lab/go-securesystemslib v0.8.0 h1:mr5An6X45Kb2nddcFlbm
 github.com/secure-systems-lab/go-securesystemslib v0.8.0/go.mod h1:UH2VZVuJfCYR8WgMlCU1uFsOUU+KeyrTWcSS73NBOzU=
 github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k=
 github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME=
-github.com/sigstore/sigstore v1.8.10 h1:r4t+TYzJlG9JdFxMy+um9GZhZ2N1hBTyTex0AHEZxFs=
-github.com/sigstore/sigstore v1.8.10/go.mod h1:BekjqxS5ZtHNJC4u3Q3Stvfx2eyisbW/lUZzmPU2u4A=
-github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.10 h1:e5GfVngPjGap/N3ODefayt7vKIPS1/v3hWLZ9+4MrN4=
-github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.10/go.mod h1:HOr3AdFPKdND2FNl/sUD5ZifPl1OMJvrbf9xIaaWcus=
-github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.10 h1:9tZEpfIL/ewAG9G87AHe3aVoy8Ujos2F1qLfCckX6jQ=
-github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.10/go.mod h1:VnIAcitund62R45ezK/dtUeEhuRtB3LsAgJ8m0H34zc=
-github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.8.10 h1:Xre51HdjIIaVo5ox5zyL+6h0tkrx7Ke9Neh7fLmmZK0=
-github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.8.10/go.mod h1:VNfdklQDbyGJog8S7apdxiEfmYmCkKyxrsCL9xprkTY=
-github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.10 h1:HjfjL3x3dP2kaGqQHVog974cTcKfzFaGjfZyLQ9KXrg=
-github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.10/go.mod h1:jaeEjkTW1p3gUyPjz9lTcT4TydCs208FoyAwIs6bIT4=
+github.com/sigstore/sigstore v1.8.11 h1:tEqeQqbT+awtM87ec9KEeSUxT/AFvJNawneYJyAkFrQ=
+github.com/sigstore/sigstore v1.8.11/go.mod h1:fdrFQosxCQ4wTL5H1NrZcQkqQ72AQbPjtpcL2QOGKV0=
+github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.11 h1:4jIEBOtqDZHyQNQSw/guGmIY0y3CVdOGQu3l2FNlqpY=
+github.com/sigstore/sigstore/pkg/signature/kms/aws v1.8.11/go.mod h1:rzfk1r8p6Mgjp5tidjzNC+/Kh1h6Eh/ON7xI7ApqBSM=
+github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.11 h1:GXL/OitAMBbLg61nbbk0bXOgOIgDgyFE+9T2Ng3P3o8=
+github.com/sigstore/sigstore/pkg/signature/kms/azure v1.8.11/go.mod h1:a9KhG9LZJFcGJB2PtFga1jUIUB0gr0Ix44TDMMXUjJU=
+github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.8.11 h1:jxKeAMOzaxjwEfmpMMYxF5Vf35tEhQOUXURaUx0ctgo=
+github.com/sigstore/sigstore/pkg/signature/kms/gcp v1.8.11/go.mod h1:fIAOBcL2s+Vq2Fp9WZByUDdWAmhNuZkJGLCUVUjkdtI=
+github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.11 h1:nH6Cpsz9c7v8jpGiJcH+3+zijfdJha+9mK07MAzZjbc=
+github.com/sigstore/sigstore/pkg/signature/kms/hashivault v1.8.11/go.mod h1:bTBdhPvdaDsHccD9zsSHe/q4ah2OXkdfL/qK7JCuRno=
 github.com/smallstep/assert v0.0.0-20200723003110-82e2b9b3b262 h1:unQFBIznI+VYD1/1fApl1A+9VcBk+9dcqGfnePY87LY=
 github.com/smallstep/assert v0.0.0-20200723003110-82e2b9b3b262/go.mod h1:MyOHs9Po2fbM1LHej6sBUT8ozbxmMOFG+E+rx/GSGuc=
 github.com/sourcegraph/conc v0.3.0 h1:OQTbbt6P72L20UqAkXXuLOj79LfEanQ+YQFNpLA9ySo=

pkg/certmaker/certmaker.go

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