From 02345bb100e0ec63713bee641b4e2ffd7ff6746e Mon Sep 17 00:00:00 2001 From: ianhundere <138915+ianhundere@users.noreply.github.com> Date: Wed, 4 Dec 2024 01:14:29 -0500 Subject: [PATCH] fix: adds further test coverage. Signed-off-by: ianhundere <138915+ianhundere@users.noreply.github.com> --- .../certificate_maker_test.go | 290 +++++++++ pkg/certmaker/certmaker_test.go | 612 ++++++++++++------ pkg/certmaker/template.go | 23 + pkg/certmaker/template_test.go | 335 ++++++++++ 4 files changed, 1064 insertions(+), 196 deletions(-) create mode 100644 cmd/certificate_maker/certificate_maker_test.go create mode 100644 pkg/certmaker/template_test.go diff --git a/cmd/certificate_maker/certificate_maker_test.go b/cmd/certificate_maker/certificate_maker_test.go new file mode 100644 index 000000000..c6765e481 --- /dev/null +++ b/cmd/certificate_maker/certificate_maker_test.go @@ -0,0 +1,290 @@ +// 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 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 TSA Root CA" + }, + "issuer": { + "commonName": "Test TSA 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 TSA" + }, + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z", + "keyUsage": ["digitalSignature"], + "extKeyUsage": ["TimeStamping"], + "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_test.go b/pkg/certmaker/certmaker_test.go index 6dc1f9576..b6e0eb55f 100644 --- a/pkg/certmaker/certmaker_test.go +++ b/pkg/certmaker/certmaker_test.go @@ -16,6 +16,7 @@ package certmaker import ( + "context" "crypto" "crypto/ecdsa" "crypto/elliptic" @@ -24,9 +25,11 @@ import ( "crypto/x509/pkix" "encoding/pem" "fmt" + "math/big" "os" "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -34,14 +37,16 @@ import ( "go.step.sm/crypto/x509util" ) -// mockKMS provides an in-memory KMS for testing -type mockKMS struct { +// mockKMSProvider is a mock implementation of apiv1.KeyManager +type mockKMSProvider struct { + name string keys map[string]*ecdsa.PrivateKey signers map[string]crypto.Signer } -func newMockKMS() *mockKMS { - m := &mockKMS{ +func newMockKMSProvider() *mockKMSProvider { + m := &mockKMSProvider{ + name: "test", keys: make(map[string]*ecdsa.PrivateKey), signers: make(map[string]crypto.Signer), } @@ -58,7 +63,11 @@ func newMockKMS() *mockKMS { return m } -func (m *mockKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, error) { +func (m *mockKMSProvider) CreateKey(*apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { + return nil, fmt.Errorf("not implemented") +} + +func (m *mockKMSProvider) 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) @@ -67,7 +76,7 @@ func (m *mockKMS) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e return key, nil } -func (m *mockKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey, error) { +func (m *mockKMSProvider) 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) @@ -75,50 +84,310 @@ func (m *mockKMS) GetPublicKey(req *apiv1.GetPublicKeyRequest) (crypto.PublicKey return key.Public(), nil } -func (m *mockKMS) Close() error { +func (m *mockKMSProvider) Close() error { return nil } -func (m *mockKMS) CreateKey(req *apiv1.CreateKeyRequest) (*apiv1.CreateKeyResponse, error) { - return nil, fmt.Errorf("CreateKey is not supported in mockKMS") +func TestValidateKMSConfig(t *testing.T) { + tests := []struct { + name string + config KMSConfig + wantError string + }{ + { + name: "empty KMS type", + config: KMSConfig{ + RootKeyID: "key-id", + }, + wantError: "KMS type cannot be empty", + }, + { + name: "missing key IDs", + config: KMSConfig{ + Type: "awskms", + }, + wantError: "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/1234abcd-12ab-34cd-56ef-1234567890ab", + }, + wantError: "region is required for AWS KMS", + }, + { + name: "AWS KMS invalid root key ID", + config: KMSConfig{ + Type: "awskms", + Region: "us-west-2", + RootKeyID: "invalid-key-id", + }, + wantError: "awskms RootKeyID must start with 'arn:aws:kms:' or 'alias/'", + }, + { + name: "AWS KMS invalid intermediate key ID", + config: KMSConfig{ + Type: "awskms", + Region: "us-west-2", + RootKeyID: "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab", + IntermediateKeyID: "invalid-key-id", + }, + wantError: "awskms IntermediateKeyID must start with 'arn:aws:kms:' or 'alias/'", + }, + { + name: "AWS KMS invalid leaf key ID", + config: KMSConfig{ + Type: "awskms", + Region: "us-west-2", + LeafKeyID: "invalid-key-id", + }, + wantError: "awskms LeafKeyID must start with 'arn:aws:kms:' or 'alias/'", + }, + { + name: "GCP KMS invalid root key ID", + config: KMSConfig{ + Type: "cloudkms", + RootKeyID: "invalid-key-id", + }, + wantError: "cloudkms RootKeyID must start with 'projects/'", + }, + { + name: "GCP KMS invalid intermediate key ID", + config: KMSConfig{ + Type: "cloudkms", + RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key", + IntermediateKeyID: "invalid-key-id", + }, + wantError: "cloudkms IntermediateKeyID must start with 'projects/'", + }, + { + name: "GCP KMS invalid leaf key ID", + config: KMSConfig{ + Type: "cloudkms", + LeafKeyID: "invalid-key-id", + }, + wantError: "cloudkms LeafKeyID must start with 'projects/'", + }, + { + name: "GCP KMS missing required parts", + config: KMSConfig{ + Type: "cloudkms", + RootKeyID: "projects/my-project", + }, + wantError: "invalid cloudkms key format", + }, + { + name: "Azure KMS missing tenant ID", + config: KMSConfig{ + Type: "azurekms", + RootKeyID: "azurekms:name=my-key;vault=my-vault", + }, + wantError: "tenant-id is required for Azure KMS", + }, + { + name: "Azure KMS invalid root key ID prefix", + config: KMSConfig{ + Type: "azurekms", + RootKeyID: "invalid-key-id", + Options: map[string]string{ + "tenant-id": "tenant-id", + }, + }, + wantError: "azurekms RootKeyID must start with 'azurekms:name='", + }, + { + name: "Azure KMS missing vault parameter", + config: KMSConfig{ + Type: "azurekms", + RootKeyID: "azurekms:name=my-key", + Options: map[string]string{ + "tenant-id": "tenant-id", + }, + }, + wantError: "azurekms RootKeyID must contain ';vault=' parameter", + }, + { + name: "Azure KMS invalid intermediate key ID", + config: KMSConfig{ + Type: "azurekms", + RootKeyID: "azurekms:name=my-key;vault=my-vault", + IntermediateKeyID: "invalid-key-id", + Options: map[string]string{ + "tenant-id": "tenant-id", + }, + }, + wantError: "azurekms IntermediateKeyID must start with 'azurekms:name='", + }, + { + name: "Azure KMS invalid leaf key ID", + config: KMSConfig{ + Type: "azurekms", + LeafKeyID: "invalid-key-id", + Options: map[string]string{ + "tenant-id": "tenant-id", + }, + }, + wantError: "azurekms LeafKeyID must start with 'azurekms:name='", + }, + { + name: "unsupported KMS type", + config: KMSConfig{ + Type: "invalidkms", + RootKeyID: "key-id", + }, + wantError: "unsupported KMS type", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateKMSConfig(tt.config) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + }) + } + + // Test valid configurations + validConfigs := []KMSConfig{ + { + Type: "awskms", + Region: "us-west-2", + RootKeyID: "arn:aws:kms:us-west-2:123456789012:key/1234abcd-12ab-34cd-56ef-1234567890ab", + }, + { + Type: "awskms", + Region: "us-west-2", + LeafKeyID: "alias/my-key", + }, + { + Type: "cloudkms", + RootKeyID: "projects/my-project/locations/global/keyRings/my-keyring/cryptoKeys/my-key", + }, + { + Type: "azurekms", + RootKeyID: "azurekms:name=my-key;vault=my-vault", + Options: map[string]string{ + "tenant-id": "tenant-id", + }, + }, + } + + for _, config := range validConfigs { + t.Run(fmt.Sprintf("valid %s config", config.Type), func(t *testing.T) { + err := ValidateKMSConfig(config) + require.NoError(t, err) + }) + } } -// TestParseTemplate tests JSON template parsing -func TestParseTemplate(t *testing.T) { - tmpFile, err := os.CreateTemp("", "cert-template-*.json") +func TestValidateTemplatePath(t *testing.T) { + // Create a temporary directory for test files + tmpDir, err := os.MkdirTemp("", "template-test-*") require.NoError(t, err) - defer os.Remove(tmpFile.Name()) + defer os.RemoveAll(tmpDir) - templateContent := `{ - "subject": { - "commonName": "Test CA" + // Create a valid JSON file + validPath := filepath.Join(tmpDir, "valid.json") + err = os.WriteFile(validPath, []byte("{}"), 0600) + require.NoError(t, err) + + // Create a non-JSON file + nonJSONPath := filepath.Join(tmpDir, "invalid.txt") + err = os.WriteFile(nonJSONPath, []byte("{}"), 0600) + require.NoError(t, err) + + tests := []struct { + name string + path string + wantError string + }{ + { + name: "valid JSON file", + path: validPath, }, - "issuer": { - "commonName": "Test CA" + { + name: "non-existent file", + path: "/nonexistent/template.json", + wantError: "template not found", }, - "keyUsage": [ - "certSign", - "crlSign" - ], - "basicConstraints": { - "isCA": true, - "maxPathLen": 0 + { + name: "wrong extension", + path: nonJSONPath, + wantError: "template file must have .json extension", }, - "notBefore": "2024-01-01T00:00:00Z", - "notAfter": "2025-01-01T00:00:00Z" - }` + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateTemplatePath(tt.path) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestWriteCertificateToFile(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-write-test-*") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tmpDir) }) - err = os.WriteFile(tmpFile.Name(), []byte(templateContent), 0600) + // Create a key pair + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) - tmpl, err := ParseTemplate(tmpFile.Name(), nil) + // Create a certificate template + template := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + CommonName: "Test Cert", + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + BasicConstraintsValid: true, + IsCA: true, + MaxPathLen: 0, + MaxPathLenZero: true, + KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageCRLSign, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageCodeSigning}, + SignatureAlgorithm: x509.ECDSAWithSHA256, + PublicKeyAlgorithm: x509.ECDSA, + } + + // Create a self-signed certificate + cert, err := x509util.CreateCertificate(template, template, key.Public(), key) require.NoError(t, err) - assert.Equal(t, "Test CA", tmpl.Subject.CommonName) - assert.True(t, tmpl.IsCA) - assert.Equal(t, 0, tmpl.MaxPathLen) + + t.Run("success", func(t *testing.T) { + testFile := filepath.Join(tmpDir, "test-cert.pem") + err = WriteCertificateToFile(cert, testFile) + require.NoError(t, err) + + content, err := os.ReadFile(testFile) + require.NoError(t, err) + + block, _ := pem.Decode(content) + require.NotNil(t, block) + assert.Equal(t, "CERTIFICATE", block.Type) + + parsedCert, err := x509.ParseCertificate(block.Bytes) + require.NoError(t, err) + assert.Equal(t, "Test Cert", parsedCert.Subject.CommonName) + }) + + t.Run("error writing to file", func(t *testing.T) { + // Try to write to a non-existent directory + testFile := filepath.Join(tmpDir, "nonexistent", "test-cert.pem") + err = WriteCertificateToFile(cert, testFile) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to create file") + }) } -// TestCreateCertificates tests certificate chain creation func TestCreateCertificates(t *testing.T) { rootContent := `{ "subject": { @@ -170,7 +439,7 @@ func TestCreateCertificates(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { os.RemoveAll(tmpDir) }) - km := newMockKMS() + km := newMockKMSProvider() config := KMSConfig{ Type: "mockkms", RootKeyID: "root-key", @@ -195,7 +464,11 @@ func TestCreateCertificates(t *testing.T) { "", "", "") require.NoError(t, err) - verifyDirectChain(t, rootCertPath, leafCertPath) + // Verify certificates were created + _, err = os.Stat(rootCertPath) + require.NoError(t, err) + _, err = os.Stat(leafCertPath) + require.NoError(t, err) }) t.Run("TSA with intermediate", func(t *testing.T) { @@ -225,7 +498,7 @@ func TestCreateCertificates(t *testing.T) { ] }` - km := newMockKMS() + km := newMockKMSProvider() config := KMSConfig{ Type: "mockkms", RootKeyID: "root-key", @@ -254,210 +527,157 @@ func TestCreateCertificates(t *testing.T) { "intermediate-key", intermediateTmplPath, intermediateCertPath) require.NoError(t, err) - verifyIntermediateChain(t, rootCertPath, intermediateCertPath, leafCertPath) + // Verify certificates were created + _, err = os.Stat(rootCertPath) + require.NoError(t, err) + _, err = os.Stat(intermediateCertPath) + require.NoError(t, err) + _, err = os.Stat(leafCertPath) + require.NoError(t, err) }) -} -// TestWriteCertificateToFile tests PEM file writing -func TestWriteCertificateToFile(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "cert-write-test-*") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tmpDir) }) + t.Run("invalid root template path", func(t *testing.T) { + km := newMockKMSProvider() + config := KMSConfig{ + Type: "mockkms", + RootKeyID: "root-key", + LeafKeyID: "leaf-key", + Options: make(map[string]string), + } - km := newMockKMS() - signer, err := km.CreateSigner(&apiv1.CreateSignerRequest{ - SigningKey: "root-key", + err := CreateCertificates(km, config, + "/nonexistent/root.json", "/nonexistent/leaf.json", + "/nonexistent/root.pem", "/nonexistent/leaf.pem", + "", "", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "error reading template file") }) - require.NoError(t, err) - template := &x509.Certificate{ - Subject: pkix.Name{ - CommonName: "Test Cert", - }, - } + t.Run("invalid intermediate template path", func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-test-tsa-*") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tmpDir) }) - cert, err := x509util.CreateCertificate(template, template, signer.Public(), signer) - require.NoError(t, err) + km := newMockKMSProvider() + config := KMSConfig{ + Type: "mockkms", + RootKeyID: "root-key", + IntermediateKeyID: "intermediate-key", + LeafKeyID: "leaf-key", + Options: make(map[string]string), + } - testFile := filepath.Join(tmpDir, "test-cert.pem") - err = WriteCertificateToFile(cert, testFile) - require.NoError(t, err) + rootTmplPath := filepath.Join(tmpDir, "root-template.json") + leafTmplPath := filepath.Join(tmpDir, "leaf-template.json") + rootCertPath := filepath.Join(tmpDir, "root.pem") + leafCertPath := filepath.Join(tmpDir, "leaf.pem") - content, err := os.ReadFile(testFile) - require.NoError(t, err) + err = os.WriteFile(rootTmplPath, []byte(rootContent), 0600) + require.NoError(t, err) + err = os.WriteFile(leafTmplPath, []byte(leafContent), 0600) + require.NoError(t, err) + + err = CreateCertificates(km, config, + rootTmplPath, leafTmplPath, + rootCertPath, leafCertPath, + "intermediate-key", "/nonexistent/intermediate.json", "/nonexistent/intermediate.pem") + require.Error(t, err) + assert.Contains(t, err.Error(), "error reading template file") + }) - block, _ := pem.Decode(content) - require.NotNil(t, block) - assert.Equal(t, "CERTIFICATE", block.Type) + t.Run("invalid leaf template path", func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-test-tsa-*") + require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(tmpDir) }) - parsedCert, err := x509.ParseCertificate(block.Bytes) - require.NoError(t, err) - assert.Equal(t, "Test Cert", parsedCert.Subject.CommonName) + km := newMockKMSProvider() + config := KMSConfig{ + Type: "mockkms", + RootKeyID: "root-key", + LeafKeyID: "leaf-key", + Options: make(map[string]string), + } + + rootTmplPath := filepath.Join(tmpDir, "root-template.json") + rootCertPath := filepath.Join(tmpDir, "root.pem") + leafCertPath := filepath.Join(tmpDir, "leaf.pem") + + err = os.WriteFile(rootTmplPath, []byte(rootContent), 0600) + require.NoError(t, err) + + err = CreateCertificates(km, config, + rootTmplPath, "/nonexistent/leaf.json", + rootCertPath, leafCertPath, + "", "", "") + require.Error(t, err) + assert.Contains(t, err.Error(), "error reading template file") + }) } -func TestValidateKMSConfig(t *testing.T) { +func TestInitKMS(t *testing.T) { + ctx := context.Background() tests := []struct { - name string - config KMSConfig - wantErr bool + name string + config KMSConfig + wantError string }{ { - name: "valid aws config", + name: "AWS KMS", 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", - }, - wantErr: false, - }, - { - name: "valid gcp config", - 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", - Options: map[string]string{ - "credentials-file": "/path/to/credentials.json", - }, - }, - wantErr: false, - }, - { - name: "valid azure config", - config: KMSConfig{ - Type: "azurekms", - RootKeyID: "azurekms:name=root-key;vault=test-vault", - LeafKeyID: "azurekms:name=leaf-key;vault=test-vault", + RootKeyID: "test-key", Options: map[string]string{ - "tenant-id": "test-tenant", + "access-key-id": "test-access-key", + "secret-access-key": "test-secret-key", }, }, - wantErr: false, }, { - name: "missing aws region", - config: KMSConfig{ - Type: "awskms", - RootKeyID: "arn:aws:kms:us-west-2:account-id:key/key-id", - }, - wantErr: true, - }, - { - name: "invalid gcp key format", + name: "GCP KMS", config: KMSConfig{ Type: "cloudkms", - RootKeyID: "invalid-format", - }, - wantErr: true, - }, - { - name: "missing key IDs", - config: KMSConfig{ - Type: "azurekms", + RootKeyID: "test-key", Options: map[string]string{ - "tenant-id": "test-tenant", + "credentials-file": "/path/to/credentials.json", }, }, - wantErr: true, - }, - { - name: "valid aws config with intermediate", - 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", - }, - wantErr: false, - }, - { - name: "valid gcp config with intermediate", - 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", - }, - wantErr: false, }, { - name: "valid azure config with intermediate", + name: "Azure KMS", 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", + Type: "azurekms", + RootKeyID: "test-key", Options: map[string]string{ - "tenant-id": "test-tenant", + "tenant-id": "test-tenant", + "client-id": "test-client", + "client-secret": "test-secret", }, }, - wantErr: false, }, { - name: "invalid intermediate key format", + name: "unsupported KMS type", 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: "unsupportedkms", + RootKeyID: "test-key", }, - wantErr: true, + wantError: "unsupported KMS type", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidateKMSConfig(tt.config) - if tt.wantErr { - assert.Error(t, err) + km, err := InitKMS(ctx, tt.config) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + assert.Nil(t, km) } else { - assert.NoError(t, err) + // Since we can't actually connect to KMS providers in tests, + // we expect an error but not the "unsupported KMS type" error + require.Error(t, err) + assert.NotContains(t, err.Error(), "unsupported KMS type") } }) } } - -func verifyDirectChain(t *testing.T, rootPath, leafPath string) { - root := loadCertificate(t, rootPath) - leaf := loadCertificate(t, leafPath) - - rootPool := x509.NewCertPool() - rootPool.AddCert(root) - - _, err := leaf.Verify(x509.VerifyOptions{ - Roots: rootPool, - }) - require.NoError(t, err) -} - -func verifyIntermediateChain(t *testing.T, rootPath, intermediatePath, leafPath string) { - root := loadCertificate(t, rootPath) - intermediate := loadCertificate(t, intermediatePath) - leaf := loadCertificate(t, leafPath) - - intermediatePool := x509.NewCertPool() - intermediatePool.AddCert(intermediate) - - rootPool := x509.NewCertPool() - rootPool.AddCert(root) - - _, err := leaf.Verify(x509.VerifyOptions{ - Roots: rootPool, - Intermediates: intermediatePool, - }) - require.NoError(t, err) -} - -func loadCertificate(t *testing.T, path string) *x509.Certificate { - data, err := os.ReadFile(path) - require.NoError(t, err) - - block, _ := pem.Decode(data) - require.NotNil(t, block) - - cert, err := x509.ParseCertificate(block.Bytes) - require.NoError(t, err) - return cert -} diff --git a/pkg/certmaker/template.go b/pkg/certmaker/template.go index 0ff7d4884..1a8bd946c 100644 --- a/pkg/certmaker/template.go +++ b/pkg/certmaker/template.go @@ -88,6 +88,23 @@ func ParseTemplate(filename string, parent *x509.Certificate) (*x509.Certificate // ValidateTemplate performs validation checks on the certificate template. func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) error { + // Validate time fields first + if tmpl.NotBefore == "" { + return fmt.Errorf("notBefore time must be specified") + } + if tmpl.NotAfter == "" { + return fmt.Errorf("notAfter time must be specified") + } + + // Then validate time formats + if _, err := time.Parse(time.RFC3339, tmpl.NotBefore); err != nil { + return fmt.Errorf("invalid notBefore time format: %w", err) + } + if _, err := time.Parse(time.RFC3339, tmpl.NotAfter); err != nil { + return fmt.Errorf("invalid notAfter time format: %w", err) + } + + // Then validate other fields if tmpl.Subject.CommonName == "" { return fmt.Errorf("template subject.commonName cannot be empty") } @@ -141,6 +158,12 @@ func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) error } } + notBefore, _ := time.Parse(time.RFC3339, tmpl.NotBefore) + notAfter, _ := time.Parse(time.RFC3339, tmpl.NotAfter) + if notBefore.After(notAfter) { + return fmt.Errorf("NotBefore time must be before NotAfter time") + } + return nil } diff --git a/pkg/certmaker/template_test.go b/pkg/certmaker/template_test.go new file mode 100644 index 000000000..77442fe82 --- /dev/null +++ b/pkg/certmaker/template_test.go @@ -0,0 +1,335 @@ +// 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 ( + "crypto/x509" + "crypto/x509/pkix" + "encoding/base64" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestParseTemplate(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-template-*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + // Create a parent certificate for template data + parent := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Parent CA", + }, + } + + tests := []struct { + name string + content string + parent *x509.Certificate + wantError string + }{ + { + name: "valid template", + content: `{ + "subject": { + "commonName": "Test TSA" + }, + "issuer": { + "commonName": "Test TSA" + }, + "keyUsage": [ + "digitalSignature" + ], + "basicConstraints": { + "isCA": false + }, + "extensions": [ + { + "id": "2.5.29.37", + "critical": true, + "value": "MCQwIgYDVR0lBBswGQYIKwYBBQUHAwgGDSsGAQQBgjcUAgICAf8=" + } + ], + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z" + }`, + parent: parent, + }, + { + name: "missing required fields", + content: `{ + "issuer": {"commonName": "Test TSA"}, + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z" + }`, + wantError: "subject.commonName cannot be empty", + }, + { + name: "invalid time format", + content: `{ + "subject": {"commonName": "Test TSA"}, + "issuer": {"commonName": "Test TSA"}, + "notBefore": "invalid", + "notAfter": "2025-01-01T00:00:00Z" + }`, + wantError: "invalid notBefore time format", + }, + { + name: "missing digital signature usage", + content: `{ + "subject": {"commonName": "Test TSA"}, + "issuer": {"commonName": "Test TSA"}, + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2025-01-01T00:00:00Z", + "keyUsage": ["certSign"], + "basicConstraints": {"isCA": false} + }`, + wantError: "timestamp authority certificate must have digitalSignature key usage", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create temp file for template + tmpFile := filepath.Join(tmpDir, "template.json") + err := os.WriteFile(tmpFile, []byte(tt.content), 0600) + require.NoError(t, err) + + cert, err := ParseTemplate(tmpFile, tt.parent) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + assert.Nil(t, cert) + } else { + require.NoError(t, err) + require.NotNil(t, cert) + } + }) + } +} + +func TestValidateTemplate(t *testing.T) { + // Create a parent certificate for testing + parent := &x509.Certificate{ + Subject: pkix.Name{ + CommonName: "Parent CA", + }, + } + + tests := []struct { + name string + tmpl *CertificateTemplate + parent *x509.Certificate + wantError string + }{ + { + name: "valid TSA template", + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + Extensions: []struct { + ID string `json:"id"` + Critical bool `json:"critical"` + Value string `json:"value"` + }{ + { + ID: "2.5.29.37", + Critical: true, + Value: base64.StdEncoding.EncodeToString([]byte{0x30, 0x24, 0x30, 0x22, 0x06, 0x08, 0x2B, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, 0x08}), + }, + }, + }, + parent: parent, + }, + { + name: "empty notBefore time", + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + }, + wantError: "notBefore time must be specified", + }, + { + name: "empty notAfter time", + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotBefore: "2024-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + }, + wantError: "notAfter time must be specified", + }, + { + 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 TSA", + }, + NotBefore: "invalid", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + }, + wantError: "invalid notBefore time format", + }, + { + name: "invalid extension OID", + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + Extensions: []struct { + ID string `json:"id"` + Critical bool `json:"critical"` + Value string `json:"value"` + }{ + { + ID: "invalid.oid", + Critical: true, + Value: "AQID", + }, + }, + }, + wantError: "invalid OID component in extension", + }, + { + name: "empty extension ID", + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotBefore: "2024-01-01T00:00:00Z", + NotAfter: "2025-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + Extensions: []struct { + ID string `json:"id"` + Critical bool `json:"critical"` + Value string `json:"value"` + }{ + { + ID: "", + Critical: true, + Value: "AQID", + }, + }, + }, + wantError: "extension ID cannot be empty", + }, + { + 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 TSA", + }, + Issuer: struct { + CommonName string `json:"commonName"` + }{ + CommonName: "Test TSA", + }, + NotBefore: "2025-01-01T00:00:00Z", // Later than NotAfter + NotAfter: "2024-01-01T00:00:00Z", + KeyUsage: []string{"digitalSignature"}, + }, + 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) + if tt.wantError != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantError) + } else { + require.NoError(t, err) + } + }) + } +}