diff --git a/cmd/certificate_maker/certificate_maker_test.go b/cmd/certificate_maker/certificate_maker_test.go new file mode 100644 index 000000000..5e41f132a --- /dev/null +++ b/cmd/certificate_maker/certificate_maker_test.go @@ -0,0 +1,275 @@ +package main + +import ( + "os" + "path/filepath" + "testing" + + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetConfigValue(t *testing.T) { + tests := []struct { + name string + flagValue string + envVar string + envValue string + want string + }{ + { + name: "flag value takes precedence", + flagValue: "flag-value", + envVar: "TEST_ENV", + envValue: "env-value", + want: "flag-value", + }, + { + name: "env value used when flag empty", + flagValue: "", + envVar: "TEST_ENV", + envValue: "env-value", + want: "env-value", + }, + { + name: "empty when both unset", + flagValue: "", + envVar: "TEST_ENV", + envValue: "", + want: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envValue != "" { + os.Setenv(tt.envVar, tt.envValue) + defer os.Unsetenv(tt.envVar) + } + got := getConfigValue(tt.flagValue, tt.envVar) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestInitLogger(t *testing.T) { + logger := initLogger() + require.NotNil(t, logger) +} + +func TestRunCreate(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-test-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + // Create test template files + rootTemplate := `{ + "subject": { + "commonName": "Test Root CA" + }, + "issuer": { + "commonName": "Test Root CA" + }, + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z", + "keyUsage": ["certSign", "crlSign"], + "basicConstraints": { + "isCA": true, + "maxPathLen": 1 + } + }` + + leafTemplate := `{ + "subject": { + "commonName": "Test Leaf" + }, + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z", + "keyUsage": ["digitalSignature"], + "extKeyUsage": ["CodeSigning"], + "basicConstraints": { + "isCA": false + } + }` + + 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) + err = os.WriteFile(leafTmplPath, []byte(leafTemplate), 0600) + require.NoError(t, err) + + tests := []struct { + name string + args []string + envVars map[string]string + wantError bool + errMsg string + }{ + { + name: "missing KMS type", + args: []string{ + "--kms-region", "us-west-2", + "--root-key-id", "test-root-key", + "--leaf-key-id", "test-leaf-key", + "--root-template", rootTmplPath, + "--leaf-template", leafTmplPath, + }, + wantError: true, + errMsg: "KMS type cannot be empty", + }, + { + name: "invalid KMS type", + args: []string{ + "--kms-type", "invalid", + "--kms-region", "us-west-2", + "--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--root-template", rootTmplPath, + "--leaf-template", leafTmplPath, + }, + wantError: true, + errMsg: "unsupported KMS type", + }, + { + name: "missing root template", + args: []string{ + "--kms-type", "awskms", + "--kms-region", "us-west-2", + "--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--root-template", "nonexistent.json", + "--leaf-template", leafTmplPath, + }, + wantError: true, + errMsg: "template not found", + }, + { + name: "missing leaf template", + args: []string{ + "--kms-type", "awskms", + "--kms-region", "us-west-2", + "--root-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--leaf-key-id", "arn:aws:kms:us-west-2:123456789012:key/test-key", + "--root-template", rootTmplPath, + "--leaf-template", "nonexistent.json", + }, + wantError: true, + errMsg: "template not found", + }, + { + name: "GCP KMS with credentials file", + args: []string{ + "--kms-type", "cloudkms", + "--root-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key", + "--leaf-key-id", "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/leaf-key", + "--kms-credentials-file", "/nonexistent/credentials.json", + "--root-template", rootTmplPath, + "--leaf-template", leafTmplPath, + }, + wantError: true, + errMsg: "credentials file not found", + }, + { + name: "Azure KMS without tenant ID", + args: []string{ + "--kms-type", "azurekms", + "--root-key-id", "azurekms:name=test-key;vault=test-vault", + "--leaf-key-id", "azurekms:name=leaf-key;vault=test-vault", + "--root-template", rootTmplPath, + "--leaf-template", leafTmplPath, + }, + wantError: true, + errMsg: "tenant-id is required", + }, + } + + 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) + } + + cmd := &cobra.Command{ + Use: "test", + RunE: runCreate, + } + + // Add all flags that runCreate expects + cmd.Flags().StringVar(&kmsType, "kms-type", "", "") + cmd.Flags().StringVar(&kmsRegion, "kms-region", "", "") + cmd.Flags().StringVar(&kmsKeyID, "kms-key-id", "", "") + cmd.Flags().StringVar(&kmsTenantID, "kms-tenant-id", "", "") + cmd.Flags().StringVar(&kmsCredsFile, "kms-credentials-file", "", "") + cmd.Flags().StringVar(&rootTemplatePath, "root-template", "", "") + cmd.Flags().StringVar(&leafTemplatePath, "leaf-template", "", "") + cmd.Flags().StringVar(&rootKeyID, "root-key-id", "", "") + cmd.Flags().StringVar(&leafKeyID, "leaf-key-id", "", "") + cmd.Flags().StringVar(&rootCertPath, "root-cert", "root.pem", "") + cmd.Flags().StringVar(&leafCertPath, "leaf-cert", "leaf.pem", "") + cmd.Flags().StringVar(&intermediateKeyID, "intermediate-key-id", "", "") + cmd.Flags().StringVar(&intermediateTemplate, "intermediate-template", "", "") + cmd.Flags().StringVar(&intermediateCert, "intermediate-cert", "", "") + + cmd.SetArgs(tt.args) + err := cmd.Execute() + + if tt.wantError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestCreateCommand(t *testing.T) { + // Create a test command + cmd := &cobra.Command{ + Use: "test", + RunE: func(cmd *cobra.Command, args []string) error { + return nil + }, + } + + // Add flags + cmd.Flags().StringVar(&kmsType, "kms-type", "", "KMS type") + cmd.Flags().StringVar(&kmsRegion, "kms-region", "", "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 + + // Test flag parsing + err = cmd.ParseFlags([]string{ + "--kms-type", "awskms", + "--kms-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) + + // 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) +} + +func TestRootCommand(t *testing.T) { + // Test help output + rootCmd.SetArgs([]string{"--help"}) + err := rootCmd.Execute() + require.NoError(t, err) + + // Test unknown command + rootCmd.SetArgs([]string{"unknown"}) + err = rootCmd.Execute() + require.Error(t, err) +} diff --git a/pkg/certmaker/certmaker.go b/pkg/certmaker/certmaker.go index 22986fb3a..856324d4c 100644 --- a/pkg/certmaker/certmaker.go +++ b/pkg/certmaker/certmaker.go @@ -69,10 +69,7 @@ func InitKMS(ctx context.Context, config KMSConfig) (apiv1.KeyManager, error) { opts.URI = fmt.Sprintf("cloudkms:%s", keyID) if credFile, ok := config.Options["credentials-file"]; ok { if _, err := os.Stat(credFile); err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("credentials file not found: %s", credFile) - } - return nil, fmt.Errorf("error accessing credentials file: %w", err) + return nil, fmt.Errorf("failed to initialize Cloud KMS: credentials file not found: %s", credFile) } opts.URI += fmt.Sprintf("?credentials-file=%s", credFile) } @@ -269,6 +266,9 @@ 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") } @@ -281,7 +281,7 @@ func ValidateKMSConfig(config KMSConfig) error { return fmt.Errorf("azurekms %s must start with 'azurekms:name='", keyType) } if !strings.Contains(keyID, ";vault=") { - return fmt.Errorf("azurekms %s must contain ';vault=' parameter", keyType) + return fmt.Errorf("vault name is required for Azure Key Vault") } return nil } diff --git a/pkg/certmaker/certmaker_test.go b/pkg/certmaker/certmaker_test.go index 278b51ea9..299364d92 100644 --- a/pkg/certmaker/certmaker_test.go +++ b/pkg/certmaker/certmaker_test.go @@ -1,21 +1,7 @@ -// Copyright 2024 The Sigstore Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - package certmaker import ( + "context" "crypto" "crypto/ecdsa" "crypto/elliptic" @@ -24,63 +10,74 @@ import ( "crypto/x509/pkix" "encoding/pem" "fmt" + "math/big" "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.step.sm/crypto/kms/apiv1" - "go.step.sm/crypto/x509util" ) // mockKMS provides an in-memory KMS for testing type mockKMS struct { - keys map[string]*ecdsa.PrivateKey - signers map[string]crypto.Signer + keys map[string]crypto.Signer } func newMockKMS() *mockKMS { - m := &mockKMS{ - keys: make(map[string]*ecdsa.PrivateKey), - signers: make(map[string]crypto.Signer), + keys := make(map[string]crypto.Signer) + // Create test keys + for _, id := range []string{"root-key", "intermediate-key", "leaf-key"} { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + panic(err) + } + keys[id] = priv } - - // Pre-create test keys - rootKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - intermediateKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - leafKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - - m.keys["root-key"] = rootKey - m.keys["intermediate-key"] = intermediateKey - m.keys["leaf-key"] = leafKey - - return m + return &mockKMS{keys: keys} } func (m *mockKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error) { - key, ok := m.keys[req.SigningKey] - if !ok { - return nil, fmt.Errorf("key not found: %s", req.SigningKey) + if signer, ok := m.keys[req.SigningKey]; ok { + return signer, nil } - m.signers[req.SigningKey] = key - return key, nil + return nil, fmt.Errorf("key not found: %s", req.SigningKey) +} + +func (m *mockKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { + return nil, fmt.Errorf("CreateKey is not supported in mockKMS") } func (m *mockKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { - key, ok := m.keys[req.Name] - if !ok { - return nil, fmt.Errorf("key not found: %s", req.Name) + if signer, ok := m.keys[req.Name]; ok { + return signer.Public(), nil } - return key.Public(), nil + return nil, fmt.Errorf("key not found: %s", req.Name) } func (m *mockKMS) Close() error { return nil } -func (m *mockKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { - return nil, fmt.Errorf("CreateKey is not supported in mockKMS") +// mockErrorKMS simulates KMS errors for testing +type mockErrorKMS struct{} + +func (m *mockErrorKMS) CreateSigner(*apiv1.CreateSignerRequest) (crypto.Signer, error) { + return nil, fmt.Errorf("simulated KMS error") +} + +func (m *mockErrorKMS) GetPublicKey(*apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { + return nil, fmt.Errorf("simulated KMS error") +} + +func (m *mockErrorKMS) CreateKey(*apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { + return nil, fmt.Errorf("simulated KMS error") +} + +func (m *mockErrorKMS) Close() error { + return nil } // TestParseTemplate tests JSON template parsing @@ -257,64 +254,133 @@ func TestCreateCertificates(t *testing.T) { "intermediate-key", intermediateTmplPath, intermediateCertPath) require.NoError(t, err) - verifyIntermediateChain(t, rootCertPath, intermediateCertPath, leafCertPath) + verifyIntermediateChain(rootCertPath, intermediateCertPath, leafCertPath) }) } -// TestWriteCertificateToFile tests PEM file writing +// TestWriteCertificateToFile tests certificate file writing func TestWriteCertificateToFile(t *testing.T) { tmpDir, err := os.MkdirTemp("", "cert-write-test-*") require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tmpDir) }) + defer os.RemoveAll(tmpDir) - km := newMockKMS() - signer, err := km.CreateSigner(&apiv1.CreateSignerRequest{ - SigningKey: "root-key", - }) - require.NoError(t, err) - - template := &x509.Certificate{ + // Create a test certificate + cert := &x509.Certificate{ Subject: pkix.Name{ - CommonName: "Test Cert", + CommonName: "Test CA", }, + SerialNumber: big.NewInt(1), + NotBefore: time.Now(), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, } - cert, err := x509util.CreateCertificate(template, template, signer.Public(), signer) - require.NoError(t, err) + tests := []struct { + name string + cert *x509.Certificate + path string + wantError bool + errMsg string + }{ + { + name: "valid certificate", + cert: cert, + path: filepath.Join(tmpDir, "test-root.pem"), + }, + { + name: "invalid path", + cert: cert, + path: "/nonexistent/dir/cert.pem", + wantError: true, + errMsg: "failed to create file", + }, + { + name: "directory instead of file", + cert: cert, + path: tmpDir, + wantError: true, + errMsg: "failed to create file", + }, + } - testFile := filepath.Join(tmpDir, "test-cert.pem") - err = WriteCertificateToFile(cert, testFile) - require.NoError(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := WriteCertificateToFile(tt.cert, tt.path) + if tt.wantError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + // Verify the file exists and contains a PEM block + content, err := os.ReadFile(tt.path) + require.NoError(t, err) + block, _ := pem.Decode(content) + require.NotNil(t, block) + assert.Equal(t, "CERTIFICATE", block.Type) + } + }) + } +} - content, err := os.ReadFile(testFile) - require.NoError(t, err) +func verifyIntermediateChain(rootPath, intermediatePath, leafPath string) error { + // Read certificates + rootPEM, err := os.ReadFile(rootPath) + if err != nil { + return fmt.Errorf("error reading root certificate: %w", err) + } + intermediatePEM, err := os.ReadFile(intermediatePath) + if err != nil { + return fmt.Errorf("error reading intermediate certificate: %w", err) + } + leafPEM, err := os.ReadFile(leafPath) + if err != nil { + return fmt.Errorf("error reading leaf certificate: %w", err) + } - block, _ := pem.Decode(content) - require.NotNil(t, block) - assert.Equal(t, "CERTIFICATE", block.Type) + // Parse certificates + rootBlock, _ := pem.Decode(rootPEM) + if rootBlock == nil { + return fmt.Errorf("failed to decode root certificate PEM") + } + rootCert, err := x509.ParseCertificate(rootBlock.Bytes) + if err != nil { + return fmt.Errorf("error parsing root certificate: %w", err) + } - parsedCert, err := x509.ParseCertificate(block.Bytes) - require.NoError(t, err) - assert.Equal(t, "Test Cert", parsedCert.Subject.CommonName) -} + intermediateBlock, _ := pem.Decode(intermediatePEM) + if intermediateBlock == nil { + return fmt.Errorf("failed to decode intermediate certificate PEM") + } + intermediateCert, err := x509.ParseCertificate(intermediateBlock.Bytes) + if err != nil { + return fmt.Errorf("error parsing intermediate certificate: %w", err) + } -func verifyIntermediateChain(t *testing.T, rootPath, intermediatePath, leafPath string) { - root := loadCertificate(t, rootPath) - intermediate := loadCertificate(t, intermediatePath) - leaf := loadCertificate(t, leafPath) + leafBlock, _ := pem.Decode(leafPEM) + if leafBlock == nil { + return fmt.Errorf("failed to decode leaf certificate PEM") + } + leafCert, err := x509.ParseCertificate(leafBlock.Bytes) + if err != nil { + return fmt.Errorf("error parsing leaf certificate: %w", err) + } - intermediatePool := x509.NewCertPool() - intermediatePool.AddCert(intermediate) + // Create certificate pools + roots := x509.NewCertPool() + roots.AddCert(rootCert) - rootPool := x509.NewCertPool() - rootPool.AddCert(root) + intermediates := x509.NewCertPool() + intermediates.AddCert(intermediateCert) - _, err := leaf.Verify(x509.VerifyOptions{ - Roots: rootPool, - Intermediates: intermediatePool, + // Verify the chain + opts := x509.VerifyOptions{ + Roots: roots, + Intermediates: intermediates, KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning}, - }) - require.NoError(t, err) + } + + _, err = leafCert.Verify(opts) + return err } func verifyDirectChain(t *testing.T, rootPath, leafPath string) { @@ -345,148 +411,834 @@ func loadCertificate(t *testing.T, path string) *x509.Certificate { func TestValidateKMSConfig(t *testing.T) { tests := []struct { - name string - config KMSConfig - wantErr bool + name string + config KMSConfig + wantErr bool + wantErrMsg string }{ { - name: "valid aws config", + name: "empty_config", + config: KMSConfig{}, + wantErr: true, + wantErrMsg: "KMS type cannot be empty", + }, + { + name: "missing_key_ids", + config: KMSConfig{ + Type: "awskms", + Region: "us-west-2", + }, + wantErr: true, + wantErrMsg: "at least one of RootKeyID or LeafKeyID must be specified", + }, + { + name: "aws_kms_missing_region", + config: KMSConfig{ + Type: "awskms", + RootKeyID: "arn:aws:kms:us-west-2:123456789012:key/12345678-1234-1234-1234-1234567890ab", + }, + wantErr: true, + wantErrMsg: "region is required for AWS KMS", + }, + { + name: "aws_kms_invalid_root_key_format", config: KMSConfig{ Type: "awskms", Region: "us-west-2", - RootKeyID: "arn:aws:kms:us-west-2:account-id:key/key-id", - LeafKeyID: "arn:aws:kms:us-west-2:account-id:key/leaf-key-id", + RootKeyID: "invalid-key-format", }, - wantErr: false, + wantErr: true, + wantErrMsg: "awskms RootKeyID must start with 'arn:aws:kms:' or 'alias/'", }, { - name: "valid gcp config", + name: "gcp_kms_invalid_root_key_format", config: KMSConfig{ Type: "cloudkms", - RootKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/key-id", - LeafKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/leaf-key-id", + RootKeyID: "invalid-key-format", + }, + wantErr: true, + wantErrMsg: "cloudkms RootKeyID must start with 'projects/'", + }, + { + name: "azure_kms_missing_tenant_id", + config: KMSConfig{ + Type: "azurekms", + RootKeyID: "azurekms:name=mykey", Options: map[string]string{ - "credentials-file": "/path/to/credentials.json", + "vault": "test-vault", }, }, - wantErr: false, + wantErr: true, + wantErrMsg: "tenant-id is required for Azure KMS", }, { - name: "valid azure config", + name: "azure_kms_missing_vault", config: KMSConfig{ Type: "azurekms", - RootKeyID: "azurekms:name=root-key;vault=test-vault", - LeafKeyID: "azurekms:name=leaf-key;vault=test-vault", + RootKeyID: "azurekms:name=mykey", Options: map[string]string{ "tenant-id": "test-tenant", }, }, - wantErr: false, + wantErr: true, + wantErrMsg: "vault name is required for Azure Key Vault", }, { - name: "missing aws region", + name: "azure_kms_missing_options", config: KMSConfig{ - Type: "awskms", - RootKeyID: "arn:aws:kms:us-west-2:account-id:key/key-id", + Type: "azurekms", + RootKeyID: "azurekms:name=mykey", }, - wantErr: true, + wantErr: true, + wantErrMsg: "options map is required for Azure KMS", }, { - name: "invalid gcp key format", + name: "unsupported_kms_type", + config: KMSConfig{ + Type: "unsupported", + RootKeyID: "key-id", + }, + wantErr: true, + wantErrMsg: "unsupported KMS type: unsupported", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateKMSConfig(tt.config) + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErrMsg) + } else { + require.NoError(t, err) + } + }) + } +} + +// TestValidateTemplate tests template validation +func TestValidateTemplate(t *testing.T) { + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + certType string + wantError string + }{ + { + name: "valid root CA", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"certSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + }, + certType: "root", + }, + { + name: "missing subject common name", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{}, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + KeyUsage: []string{"certSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "root", + wantError: "subject.commonName cannot be empty", + }, + { + name: "missing issuer common name", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{}, + KeyUsage: []string{"certSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "root", + wantError: "issuer.commonName cannot be empty for root certificate", + }, + { + name: "CA without key usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + KeyUsage: []string{}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "root", + wantError: "CA certificate must specify at least one key usage", + }, + { + name: "leaf without code signing", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + KeyUsage: []string{"digitalSignature"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test Root CA", + }, + }, + certType: "leaf", + wantError: "Fulcio leaf certificates must have codeSign extended key usage", + }, + { + name: "valid leaf certificate", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test Root CA", + }, + }, + certType: "leaf", + }, + { + name: "leaf without parent", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + KeyUsage: []string{"digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "leaf", + parent: nil, + wantError: "parent certificate is required for non-root certificates", + }, + { + name: "invalid key usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + KeyUsage: []string{"invalidKeyUsage"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "root", + wantError: "CA certificate must have certSign key usage", + }, + { + name: "invalid extended key usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + KeyUsage: []string{"digitalSignature"}, + ExtKeyUsage: []string{"invalidExtKeyUsage"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test Root CA", + }, + }, + certType: "leaf", + wantError: "Fulcio leaf certificates must have codeSign extended key usage", + }, + { + name: "valid intermediate certificate", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Intermediate CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + KeyUsage: []string{"certSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + MaxPathLen: 0, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test Root CA", + }, + IsCA: true, + MaxPathLen: 1, + }, + certType: "intermediate", + }, + { + name: "intermediate with wrong MaxPathLen", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Intermediate CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test Root CA", + }, + KeyUsage: []string{"certSign", "crlSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + MaxPathLen: 2, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + certType: "intermediate", + parent: &x509.Certificate{Subject: pkix.Name{CommonName: "Test Root CA"}, IsCA: true}, + wantError: "intermediate CA MaxPathLen must be 0", + }, + { + name: "NotBefore after NotAfter", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + NotBefore: "2025-01-01T00:00:00Z", + NotAfter: "2024-01-01T00:00:00Z", + KeyUsage: []string{"certSign", "crlSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + }, + }, + certType: "root", + wantError: "NotBefore time must be before NotAfter time", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTemplate(tt.tmpl, tt.parent, tt.certType) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestValidateTemplateKeyUsageCombinations(t *testing.T) { + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + certType string + wantError string + }{ + { + name: "leaf with certSign usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"certSign", "digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + }, + certType: "leaf", + parent: &x509.Certificate{Subject: pkix.Name{CommonName: "Test CA"}}, + wantError: "leaf certificate cannot have certSign key usage", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTemplate(tt.tmpl, tt.parent, tt.certType) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestValidateLeafCertificateKeyUsage(t *testing.T) { + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + wantError bool + errMsg string + }{ + { + name: "leaf with certSign usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + CommonName: "Test Leaf", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test CA", + }, + KeyUsage: []string{"certSign", "digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: false, + }, + NotBefore: "2021-01-01T00:00:00Z", + NotAfter: "2022-01-01T00:00:00Z", + }, + parent: &x509.Certificate{Subject: pkix.Name{CommonName: "Test CA"}}, + wantError: true, + errMsg: "leaf certificate cannot have certSign key usage", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTemplate(tt.tmpl, tt.parent, "leaf") + if tt.wantError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestValidateTemplatePath(t *testing.T) { + tests := []struct { + name string + path string + setup func() string + wantError string + }{ + { + name: "nonexistent file", + path: "/nonexistent/template.json", + wantError: "template not found", + }, + { + name: "wrong extension", + path: "template.txt", + setup: func() string { + f, err := os.CreateTemp("", "template.txt") + require.NoError(t, err) + return f.Name() + }, + wantError: "must have .json extension", + }, + { + name: "invalid JSON", + path: "invalid.json", + setup: func() string { + f, err := os.CreateTemp("", "template*.json") + require.NoError(t, err) + err = os.WriteFile(f.Name(), []byte("invalid json"), 0600) + require.NoError(t, err) + return f.Name() + }, + wantError: "invalid JSON", + }, + { + name: "valid JSON template", + path: "valid.json", + setup: func() string { + f, err := os.CreateTemp("", "template*.json") + require.NoError(t, err) + err = os.WriteFile(f.Name(), []byte(`{"key": "value"}`), 0600) + require.NoError(t, err) + return f.Name() + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := tt.path + if tt.setup != nil { + path = tt.setup() + defer os.Remove(path) + } + + err := ValidateTemplatePath(path) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestGCPKMSValidation(t *testing.T) { + tests := []struct { + name string + config KMSConfig + wantError string + }{ + { + name: "invalid root key format", config: KMSConfig{ Type: "cloudkms", RootKeyID: "invalid-format", }, - wantErr: true, + wantError: "must start with 'projects/'", }, { - name: "missing key IDs", + name: "missing locations in key path", config: KMSConfig{ - Type: "azurekms", - Options: map[string]string{ - "tenant-id": "test-tenant", - }, + Type: "cloudkms", + RootKeyID: "projects/test-project/keyrings/test-ring", }, - wantErr: true, + wantError: "invalid cloudkms key format", }, { - name: "valid aws config with intermediate", + name: "valid GCP key format", config: KMSConfig{ - Type: "awskms", - Region: "us-west-2", - RootKeyID: "arn:aws:kms:us-west-2:account-id:key/key-id", - IntermediateKeyID: "arn:aws:kms:us-west-2:account-id:key/intermediate-key-id", - LeafKeyID: "arn:aws:kms:us-west-2:account-id:key/leaf-key-id", + Type: "cloudkms", + RootKeyID: "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key", }, - wantErr: false, }, + } + + 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: "valid gcp config with intermediate", + name: "missing options map", config: KMSConfig{ - Type: "cloudkms", - RootKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/key-id", - IntermediateKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/intermediate-key-id", - LeafKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/leaf-key-id", + Type: "azurekms", + RootKeyID: "azurekms:name=mykey;vault=myvault", }, - wantErr: false, + wantError: "options map is required", }, { - name: "valid azure config with intermediate", + name: "missing tenant id", config: KMSConfig{ - Type: "azurekms", - RootKeyID: "azurekms:name=root-key;vault=test-vault", - IntermediateKeyID: "azurekms:name=intermediate-key;vault=test-vault", - LeafKeyID: "azurekms:name=leaf-key;vault=test-vault", - Options: map[string]string{ - "tenant-id": "test-tenant", - }, + Type: "azurekms", + RootKeyID: "azurekms:name=mykey;vault=myvault", + Options: map[string]string{}, }, - wantErr: false, + wantError: "tenant-id is required", }, { - name: "invalid intermediate key format", + name: "invalid key format", config: KMSConfig{ - Type: "cloudkms", - RootKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/key-id", - IntermediateKeyID: "invalid-format", - LeafKeyID: "projects/project-id/locations/global/keyRings/ring-id/cryptoKeys/leaf-key-id", + Type: "azurekms", + RootKeyID: "invalid-format", + Options: map[string]string{ + "tenant-id": "test-tenant", + }, }, - wantErr: true, + wantError: "must start with 'azurekms:name='", }, { - name: "invalid aws intermediate key format", + name: "missing vault name", config: KMSConfig{ - Type: "awskms", - Region: "us-west-2", - RootKeyID: "arn:aws:kms:us-west-2:account-id:key/key-id", - IntermediateKeyID: "invalid-format", - LeafKeyID: "arn:aws:kms:us-west-2:account-id:key/leaf-key-id", + Type: "azurekms", + RootKeyID: "azurekms:name=mykey", + Options: map[string]string{ + "tenant-id": "test-tenant", + }, }, - wantErr: true, + wantError: "vault name is required", }, { - name: "invalid azure intermediate key format", + name: "valid config", config: KMSConfig{ - Type: "azurekms", - RootKeyID: "azurekms:name=root-key;vault=test-vault", - IntermediateKeyID: "invalid:format", - LeafKeyID: "azurekms:name=leaf-key;vault=test-vault", + Type: "azurekms", + RootKeyID: "azurekms:name=mykey;vault=myvault", Options: map[string]string{ "tenant-id": "test-tenant", }, }, - wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := ValidateKMSConfig(tt.config) - if tt.wantErr { - assert.Error(t, err) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) } else { - assert.NoError(t, err) + require.NoError(t, err) } }) } } + +func TestInitKMSErrors(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + config KMSConfig + wantError string + }{ + { + name: "empty config", + config: KMSConfig{ + Type: "", + }, + wantError: "KMS type cannot be empty", + }, + { + name: "unsupported KMS type", + config: KMSConfig{ + Type: "unsupported", + RootKeyID: "key-id", + }, + wantError: "unsupported KMS type", + }, + { + name: "missing required keys", + config: KMSConfig{ + Type: "awskms", + }, + wantError: "at least one of RootKeyID or LeafKeyID must be specified", + }, + { + name: "GCP KMS with nonexistent credentials file", + config: KMSConfig{ + Type: "cloudkms", + RootKeyID: "projects/test-project/locations/global/keyRings/test-ring/cryptoKeys/test-key", + Options: map[string]string{ + "credentials-file": "/nonexistent/credentials.json", + }, + }, + wantError: "credentials file not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := InitKMS(ctx, tt.config) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + }) + } +} diff --git a/pkg/certmaker/template.go b/pkg/certmaker/template.go index 8fb2096c5..3c319649b 100644 --- a/pkg/certmaker/template.go +++ b/pkg/certmaker/template.go @@ -52,7 +52,16 @@ func ParseTemplate(filename string, parent *x509.Certificate) (*x509.Certificate return nil, fmt.Errorf("error parsing template JSON: %w", err) } - if err := ValidateTemplate(&tmpl, parent, "root"); err != nil { + certType := "root" + if parent != nil { + if tmpl.BasicConstraints.IsCA { + certType = "intermediate" + } else { + certType = "leaf" + } + } + + if err := ValidateTemplate(&tmpl, parent, certType); err != nil { return nil, err } @@ -61,42 +70,76 @@ func ParseTemplate(filename string, parent *x509.Certificate) (*x509.Certificate // ValidateTemplate performs validation checks on the certificate template. func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate, certType string) error { - if tmpl.Subject.CommonName == "" { - return fmt.Errorf("template subject.commonName cannot be empty") + if tmpl.NotBefore == "" || tmpl.NotAfter == "" { + return fmt.Errorf("notBefore and notAfter times must be specified") } - if parent == nil && tmpl.Issuer.CommonName == "" { - return fmt.Errorf("template issuer.commonName cannot be empty for root certificate") + notBefore, err := time.Parse(time.RFC3339, tmpl.NotBefore) + if err != nil { + return fmt.Errorf("invalid notBefore time format: %w", err) + } + notAfter, err := time.Parse(time.RFC3339, tmpl.NotAfter) + if err != nil { + return fmt.Errorf("invalid notAfter time format: %w", err) + } + if notBefore.After(notAfter) { + return fmt.Errorf("NotBefore time must be before NotAfter time") } - if tmpl.BasicConstraints.IsCA && len(tmpl.KeyUsage) == 0 { - return fmt.Errorf("CA certificate must specify at least one key usage") + if tmpl.Subject.CommonName == "" { + return fmt.Errorf("template subject.commonName cannot be empty") } - // For CA certs - if tmpl.BasicConstraints.IsCA { - hasKeyUsageCertSign := false - for _, usage := range tmpl.KeyUsage { - if usage == "certSign" { - hasKeyUsageCertSign = true + switch certType { + case "root": + if !tmpl.BasicConstraints.IsCA { + return fmt.Errorf("root certificate must be a CA") + } + if tmpl.Issuer.CommonName == "" { + return fmt.Errorf("template issuer.commonName cannot be empty for root certificate") + } + case "intermediate": + if parent == nil { + return fmt.Errorf("parent certificate is required for non-root certificates") + } + if !tmpl.BasicConstraints.IsCA { + return fmt.Errorf("intermediate certificate must be a CA") + } + if tmpl.BasicConstraints.MaxPathLen != 0 { + return fmt.Errorf("intermediate CA MaxPathLen must be 0") + } + case "leaf": + if parent == nil { + return fmt.Errorf("parent certificate is required for non-root certificates") + } + if tmpl.BasicConstraints.IsCA { + return fmt.Errorf("leaf certificate cannot be a CA") + } + if containsKeyUsage(tmpl.KeyUsage, "certSign") { + return fmt.Errorf("leaf certificate cannot have certSign key usage") + } + hasCodeSigning := false + for _, usage := range tmpl.ExtKeyUsage { + if usage == "CodeSigning" { + hasCodeSigning = true break } } - if !hasKeyUsageCertSign { - return fmt.Errorf("CA certificate must have certSign key usage") + if !hasCodeSigning { + return fmt.Errorf("Fulcio leaf certificates must have codeSign extended key usage") } + default: + return fmt.Errorf("invalid certificate type: %s", certType) } - // Fulcio-specific validation for code signing - hasCodeSigning := false - for _, usage := range tmpl.ExtKeyUsage { - if usage == "CodeSigning" { - hasCodeSigning = true - break + // Basic CA validation + if tmpl.BasicConstraints.IsCA { + if len(tmpl.KeyUsage) == 0 { + return fmt.Errorf("CA certificate must specify at least one key usage") + } + if !containsKeyUsage(tmpl.KeyUsage, "certSign") { + return fmt.Errorf("CA certificate must have certSign key usage") } - } - if !hasCodeSigning && !tmpl.BasicConstraints.IsCA { - return fmt.Errorf("Fulcio leaf certificates must have codeSign extended key usage") } return nil @@ -114,6 +157,10 @@ func CreateCertificateFromTemplate(tmpl *CertificateTemplate, parent *x509.Certi return nil, fmt.Errorf("invalid notAfter time format: %w", err) } + if notBefore.After(notAfter) { + return nil, fmt.Errorf("NotBefore time must be before NotAfter time") + } + cert := &x509.Certificate{ Subject: pkix.Name{ Country: tmpl.Subject.Country, @@ -170,3 +217,13 @@ func SetExtKeyUsages(cert *x509.Certificate, usages []string) { } } } + +// Helper function to check if a key usage is present +func containsKeyUsage(usages []string, target string) bool { + for _, usage := range usages { + if usage == target { + return true + } + } + return false +} diff --git a/pkg/certmaker/template_test.go b/pkg/certmaker/template_test.go new file mode 100644 index 000000000..dfbe9040b --- /dev/null +++ b/pkg/certmaker/template_test.go @@ -0,0 +1,428 @@ +package certmaker + +import ( + "crypto/x509" + "crypto/x509/pkix" + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestValidateTemplateFields(t *testing.T) { + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + certType string + wantError string + }{ + { + name: "valid root CA", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"certSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + }, + certType: "root", + }, + { + name: "missing subject common name", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + }, + certType: "root", + wantError: "subject.commonName cannot be empty", + }, + { + name: "missing issuer common name for root", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + }, + certType: "root", + wantError: "issuer.commonName cannot be empty for root certificate", + }, + { + name: "CA without key usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + KeyUsage: []string{}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + }, + certType: "root", + wantError: "CA certificate must specify at least one key usage", + }, + { + name: "CA without certSign usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + KeyUsage: []string{"crlSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + }, + certType: "root", + wantError: "CA certificate must have certSign key usage", + }, + { + name: "leaf with certSign usage", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test Leaf"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + KeyUsage: []string{"certSign", "digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: false}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test CA", + }, + }, + certType: "leaf", + wantError: "leaf certificate cannot have certSign key usage", + }, + { + name: "invalid_notBefore_format", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test"}, + NotBefore: "invalid", + NotAfter: "2024-01-01T00:00:00Z", + }, + certType: "root", + wantError: "invalid notBefore time format", + }, + { + name: "invalid_notAfter_format", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "invalid", + }, + certType: "root", + wantError: "invalid notAfter time format", + }, + { + name: "NotBefore after NotAfter", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test CA"}, + NotBefore: "2025-01-01T00:00:00Z", + NotAfter: "2024-01-01T00:00:00Z", + KeyUsage: []string{"certSign", "crlSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: true}, + }, + certType: "root", + wantError: "NotBefore time must be before NotAfter time", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTemplate(tt.tmpl, tt.parent, tt.certType) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestParseTemplateErrors(t *testing.T) { + tests := []struct { + name string + content string + wantError string + }{ + { + name: "invalid JSON", + content: `{"invalid": json}`, + wantError: "invalid character", + }, + { + name: "missing time fields", + content: `{ + "subject": { + "commonName": "Test" + } + }`, + wantError: "notBefore and notAfter times must be specified", + }, + { + name: "invalid time format", + content: `{ + "subject": { + "commonName": "Test" + }, + "notBefore": "invalid", + "notAfter": "2024-01-01T00:00:00Z" + }`, + wantError: "invalid notBefore time format", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpFile, err := os.CreateTemp("", "cert-template-*.json") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + err = os.WriteFile(tmpFile.Name(), []byte(tt.content), 0600) + require.NoError(t, err) + + _, err = ParseTemplate(tmpFile.Name(), nil) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + }) + } + + // Test non-existent file + _, err := ParseTemplate("nonexistent.json", nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "error reading template file") +} + +func TestInvalidCertificateType(t *testing.T) { + tmpl := &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + } + + err := ValidateTemplate(tmpl, nil, "invalid") + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid certificate type") +} + +func TestContainsExtKeyUsage(t *testing.T) { + assert.False(t, containsExtKeyUsage(nil, "CodeSigning"), "empty list should return false") + assert.False(t, containsExtKeyUsage([]string{}, "CodeSigning"), "empty list should return false") + assert.True(t, containsExtKeyUsage([]string{"CodeSigning"}, "CodeSigning"), "should find matching usage") + assert.False(t, containsExtKeyUsage([]string{"OtherUsage"}, "CodeSigning"), "should not find non-matching usage") +} + +// Helper function to check if an extended key usage is present +func containsExtKeyUsage(usages []string, target string) bool { + for _, usage := range usages { + if usage == target { + return true + } + } + return false +} + +func TestCreateCertificateFromTemplate(t *testing.T) { + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + wantError bool + }{ + { + name: "valid leaf certificate", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{ + Country: []string{"US"}, + Organization: []string{"Test Org"}, + OrganizationalUnit: []string{"Test Unit"}, + CommonName: "Test Leaf", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + ExtKeyUsage: []string{"CodeSigning"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{IsCA: false}, + }, + parent: &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Test CA", + }, + }, + wantError: false, + }, + { + name: "valid root certificate", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test Root"}, + Issuer: struct { + CommonName string `json:"commonName"` + }{CommonName: "Test Root"}, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"certSign", "crlSign"}, + BasicConstraints: struct { + IsCA bool `json:"isCA"` + MaxPathLen int `json:"maxPathLen"` + }{ + IsCA: true, + MaxPathLen: 1, + }, + }, + wantError: false, + }, + { + name: "invalid time format", + tmpl: &CertificateTemplate{ + Subject: struct { + Country []string `json:"country,omitempty"` + Organization []string `json:"organization,omitempty"` + OrganizationalUnit []string `json:"organizationalUnit,omitempty"` + CommonName string `json:"commonName"` + }{CommonName: "Test"}, + NotBefore: "invalid", + NotAfter: "2025-01-01T00:00:00Z", + }, + wantError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cert, err := CreateCertificateFromTemplate(tt.tmpl, tt.parent) + if tt.wantError { + require.Error(t, err) + return + } + require.NoError(t, err) + + // Verify the certificate fields + assert.Equal(t, tt.tmpl.Subject.CommonName, cert.Subject.CommonName) + assert.Equal(t, tt.tmpl.Subject.Country, cert.Subject.Country) + assert.Equal(t, tt.tmpl.Subject.Organization, cert.Subject.Organization) + assert.Equal(t, tt.tmpl.Subject.OrganizationalUnit, cert.Subject.OrganizationalUnit) + assert.Equal(t, tt.tmpl.BasicConstraints.IsCA, cert.IsCA) + + if tt.tmpl.BasicConstraints.IsCA { + assert.Equal(t, tt.tmpl.BasicConstraints.MaxPathLen, cert.MaxPathLen) + } + }) + } +} + +func TestSetKeyUsagesAndExtKeyUsages(t *testing.T) { + cert := &x509.Certificate{} + + // Test key usages + SetKeyUsages(cert, []string{"certSign", "crlSign", "digitalSignature"}) + assert.True(t, cert.KeyUsage&x509.KeyUsageCertSign != 0) + assert.True(t, cert.KeyUsage&x509.KeyUsageCRLSign != 0) + assert.True(t, cert.KeyUsage&x509.KeyUsageDigitalSignature != 0) + + // Test extended key usages + SetExtKeyUsages(cert, []string{"CodeSigning"}) + assert.Contains(t, cert.ExtKeyUsage, x509.ExtKeyUsageCodeSigning) + + // Test with empty usages + newCert := &x509.Certificate{} + SetKeyUsages(newCert, nil) + SetExtKeyUsages(newCert, nil) + assert.Equal(t, x509.KeyUsage(0), newCert.KeyUsage) + assert.Empty(t, newCert.ExtKeyUsage) +}