From 713e53c6b60558ebf17d2208f2cbd720ecc569e2 Mon Sep 17 00:00:00 2001 From: ianhundere <138915+ianhundere@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:25:54 -0500 Subject: [PATCH] fix: improves things and follows lessons learned troubleshooting tsa vers of this tool. --- cmd/certificate_maker/certificate_maker.go | 7 +- pkg/certmaker/certmaker.go | 42 +++--- pkg/certmaker/certmaker_test.go | 99 ++++---------- pkg/certmaker/template.go | 123 +++++++++++++++--- .../templates/intermediate-template.json | 2 +- 5 files changed, 153 insertions(+), 120 deletions(-) diff --git a/cmd/certificate_maker/certificate_maker.go b/cmd/certificate_maker/certificate_maker.go index 1948249e..5463f3bb 100644 --- a/cmd/certificate_maker/certificate_maker.go +++ b/cmd/certificate_maker/certificate_maker.go @@ -22,12 +22,15 @@ import ( "encoding/json" "fmt" "os" + "time" "github.com/sigstore/timestamp-authority/pkg/certmaker" "github.com/spf13/cobra" "go.uber.org/zap" ) +// CLI flags and env vars for config. +// Supports AWS KMS, Google Cloud KMS, and Azure Key Vault configurations. var ( logger *zap.Logger version string @@ -96,6 +99,9 @@ func init() { } func runCreate(cmd *cobra.Command, args []string) error { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + // Build KMS config from flags and environment config := certmaker.KMSConfig{ Type: getConfigValue(kmsType, "KMS_TYPE"), @@ -120,7 +126,6 @@ func runCreate(cmd *cobra.Command, args []string) error { } } - ctx := context.Background() km, err := certmaker.InitKMS(ctx, config) if err != nil { return fmt.Errorf("failed to initialize KMS: %w", err) diff --git a/pkg/certmaker/certmaker.go b/pkg/certmaker/certmaker.go index 7c2e399b..ea2693dc 100644 --- a/pkg/certmaker/certmaker.go +++ b/pkg/certmaker/certmaker.go @@ -20,7 +20,6 @@ package certmaker import ( "context" "crypto/x509" - "encoding/json" "encoding/pem" "fmt" "os" @@ -33,6 +32,7 @@ import ( "go.step.sm/crypto/x509util" ) +// KMSConfig holds config for KMS providers. type KMSConfig struct { Type string // KMS provider type: "awskms", "cloudkms", "azurekms" Region string // AWS region or Cloud location @@ -41,6 +41,8 @@ type KMSConfig struct { Options map[string]string // Provider-specific options } +// InitKMS initializes KMS provider based on the given config, KMSConfig. +// Supports AWS KMS, Google Cloud KMS, and Azure Key Vault. func InitKMS(ctx context.Context, config KMSConfig) (apiv1.KeyManager, error) { if err := ValidateKMSConfig(config); err != nil { return nil, fmt.Errorf("invalid KMS configuration: %w", err) @@ -102,13 +104,17 @@ func CreateCertificates(km apiv1.KeyManager, config KMSConfig, rootTemplatePath, return fmt.Errorf("error creating root signer: %w", err) } - // Create root certificate + // Create root cert rootCert, err := x509util.CreateCertificate(rootTmpl, rootTmpl, rootSigner.Public(), rootSigner) if err != nil { return fmt.Errorf("error creating root certificate: %w", err) } - // Parse intermediate template + if err := WriteCertificateToFile(rootCert, rootCertPath); err != nil { + return fmt.Errorf("error writing root certificate: %w", err) + } + + // Create intermediate cert intermediateTmpl, err := ParseTemplate(intermediateTemplatePath, rootCert) if err != nil { return fmt.Errorf("error parsing intermediate template: %w", err) @@ -127,16 +133,11 @@ func CreateCertificates(km apiv1.KeyManager, config KMSConfig, rootTemplatePath, return fmt.Errorf("error creating intermediate signer: %w", err) } - // Create intermediate certificate intermediateCert, err := x509util.CreateCertificate(intermediateTmpl, rootCert, intermediateSigner.Public(), rootSigner) if err != nil { return fmt.Errorf("error creating intermediate certificate: %w", err) } - if err := WriteCertificateToFile(rootCert, rootCertPath); err != nil { - return fmt.Errorf("error writing root certificate: %w", err) - } - if err := WriteCertificateToFile(intermediateCert, intermCertPath); err != nil { return fmt.Errorf("error writing intermediate certificate: %w", err) } @@ -144,10 +145,12 @@ func CreateCertificates(km apiv1.KeyManager, config KMSConfig, rootTemplatePath, // Verify certificate chain pool := x509.NewCertPool() pool.AddCert(rootCert) - if _, err := intermediateCert.Verify(x509.VerifyOptions{ - Roots: pool, - }); err != nil { - return fmt.Errorf("CA.Intermediate.Verify() error = %v", err) + opts := x509.VerifyOptions{ + Roots: pool, + KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageTimeStamping}, + } + if _, err := intermediateCert.Verify(opts); err != nil { + return fmt.Errorf("certificate chain verification failed: %w", err) } return nil @@ -170,6 +173,11 @@ func WriteCertificateToFile(cert *x509.Certificate, filename string) error { return fmt.Errorf("failed to write certificate to file %s: %w", filename, err) } + certType := "root" + if cert.Subject.OrganizationalUnit != nil && cert.Subject.OrganizationalUnit[0] == "TSA Intermediate CA" { + certType = "intermediate" + } + fmt.Printf("Your %s certificate has been saved in %s.\n", certType, filename) return nil } @@ -216,15 +224,5 @@ func ValidateTemplatePath(path string) error { return fmt.Errorf("template file must have .json extension: %s", path) } - content, err := os.ReadFile(path) - if err != nil { - return fmt.Errorf("error reading template file: %w", err) - } - - var js json.RawMessage - if err := json.Unmarshal(content, &js); err != nil { - return fmt.Errorf("invalid JSON in template file: %w", err) - } - return nil } diff --git a/pkg/certmaker/certmaker_test.go b/pkg/certmaker/certmaker_test.go index ee7dc742..b5813fd5 100644 --- a/pkg/certmaker/certmaker_test.go +++ b/pkg/certmaker/certmaker_test.go @@ -124,112 +124,61 @@ func TestParseTemplate(t *testing.T) { // TestCreateCertificates tests certificate chain creation func TestCreateCertificates(t *testing.T) { - t.Run("Fulcio", func(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "cert-test-fulcio-*") + t.Run("TSA", func(t *testing.T) { + tmpDir, err := os.MkdirTemp("", "cert-test-tsa-*") require.NoError(t, err) t.Cleanup(func() { os.RemoveAll(tmpDir) }) // Root template (same for both) rootContent := `{ "subject": { - "commonName": "https://blah.com" + "country": ["US"], + "organization": ["Sigstore"], + "organizationalUnit": ["Timestamp Authority Root CA"], + "commonName": "https://tsa.com" }, "issuer": { - "commonName": "https://blah.com" - }, - "keyUsage": [ - "certSign", - "crlSign" - ], - "extKeyUsage": [ - "CodeSigning" - ], - "basicConstraints": { - "isCA": true, - "maxPathLen": 0 + "commonName": "https://tsa.com" }, "notBefore": "2024-01-01T00:00:00Z", - "notAfter": "2025-01-01T00:00:00Z" - }` - - // Fulcio intermediate template - intermediateContent := `{ - "subject": { - "commonName": "https://blah.com" - }, - "issuer": { - "commonName": "https://blah.com" - }, - "keyUsage": [ - "certSign", - "crlSign" - ], - "extKeyUsage": [ - "CodeSigning" - ], + "notAfter": "2034-01-01T00:00:00Z", "basicConstraints": { "isCA": true, - "maxPathLen": 0 - }, - "notBefore": "2024-01-01T00:00:00Z", - "notAfter": "2025-01-01T00:00:00Z" - }` - - testCertificateCreation(t, tmpDir, rootContent, intermediateContent) - }) - - t.Run("TSA", func(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "cert-test-tsa-*") - require.NoError(t, err) - t.Cleanup(func() { os.RemoveAll(tmpDir) }) - - // Root template (same for both) - rootContent := `{ - "subject": { - "commonName": "https://blah.com" - }, - "issuer": { - "commonName": "https://blah.com" + "maxPathLen": 1 }, "keyUsage": [ "certSign", "crlSign" - ], - "extKeyUsage": [ - "CodeSigning" - ], - "basicConstraints": { - "isCA": true, - "maxPathLen": 0 - }, - "notBefore": "2024-01-01T00:00:00Z", - "notAfter": "2025-01-01T00:00:00Z" + ] }` // TSA intermediate template intermediateContent := `{ "subject": { - "commonName": "https://blah.com" + "country": ["US"], + "organization": ["Sigstore"], + "organizationalUnit": ["Timestamp Authority"], + "commonName": "https://tsa.com" }, "issuer": { - "commonName": "https://blah.com" + "commonName": "https://tsa.com" }, - "keyUsage": [ - "certSign", - "crlSign" - ], + "notBefore": "2024-01-01T00:00:00Z", + "notAfter": "2034-01-01T00:00:00Z", "basicConstraints": { - "isCA": false + "isCA": false, + "maxPathLen": 0 }, + "keyUsage": [ + "digitalSignature" + ], "extensions": [ { "id": "2.5.29.37", "critical": true, - "value": "asn1Seq (asn1Enc oid:1.3.6.1.5.5.7.3.8) | toJson" + "value": {{ asn1Seq (asn1Enc "oid:1.3.6.1.5.5.7.3.8") | toJson }} } - ], - "notBefore": "2024-01-01T00:00:00Z", - "notAfter": "2025-01-01T00:00:00Z" + ] }` testCertificateCreation(t, tmpDir, rootContent, intermediateContent) diff --git a/pkg/certmaker/template.go b/pkg/certmaker/template.go index 0c339ece..566d36b9 100644 --- a/pkg/certmaker/template.go +++ b/pkg/certmaker/template.go @@ -5,15 +5,24 @@ package certmaker import ( + "bytes" "crypto/x509" "crypto/x509/pkix" + "encoding/base64" "encoding/json" "fmt" "math/big" "os" + "strconv" + "strings" + "text/template" "time" + + "go.step.sm/crypto/x509util" ) +// CertificateTemplate defines the JSON structure for X.509 certificate templates +// including subject, issuer, validity period, and certificate constraints. type CertificateTemplate struct { Subject struct { Country []string `json:"country,omitempty"` @@ -27,7 +36,6 @@ type CertificateTemplate struct { NotBefore string `json:"notBefore"` NotAfter string `json:"notAfter"` KeyUsage []string `json:"keyUsage"` - ExtKeyUsage []string `json:"extKeyUsage,omitempty"` BasicConstraints struct { IsCA bool `json:"isCA"` MaxPathLen int `json:"maxPathLen"` @@ -39,6 +47,11 @@ type CertificateTemplate struct { } `json:"extensions,omitempty"` } +// TemplateData holds context data passed to the template parser +type TemplateData struct { + Parent *x509.Certificate +} + // ParseTemplate creates an x509 certificate from JSON template func ParseTemplate(filename string, parent *x509.Certificate) (*x509.Certificate, error) { content, err := os.ReadFile(filename) @@ -46,18 +59,37 @@ func ParseTemplate(filename string, parent *x509.Certificate) (*x509.Certificate return nil, fmt.Errorf("error reading template file: %w", err) } - var tmpl CertificateTemplate - if err := json.Unmarshal(content, &tmpl); err != nil { - return nil, fmt.Errorf("error parsing template JSON: %w", err) + data := &TemplateData{ + Parent: parent, + } + + // Borrows x509util functions to create template + tmpl, err := template.New("cert").Funcs(x509util.GetFuncMap()).Parse(string(content)) + if err != nil { + return nil, fmt.Errorf("intermediate template error: %w", err) + } + + var buf bytes.Buffer + if err := tmpl.Execute(&buf, data); err != nil { + return nil, fmt.Errorf("intermediate template error: %w", err) + } + + // Parse template as JSON + var certTmpl CertificateTemplate + if err := json.Unmarshal(buf.Bytes(), &certTmpl); err != nil { + return nil, fmt.Errorf("intermediate template error: invalid JSON after template execution: %w", err) } - if err := ValidateTemplate(&tmpl, parent); err != nil { - return nil, err + if err := ValidateTemplate(&certTmpl, parent); err != nil { + return nil, fmt.Errorf("template validation error: %w", err) } - return CreateCertificateFromTemplate(&tmpl, parent) + return CreateCertificateFromTemplate(&certTmpl, parent) } +// ValidateTemplate performs validation checks on the certificate template. +// CA certs: verifies proper key usage is set. +// non-CA certs: verifies digitalSignature usage is set. func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) error { if tmpl.Subject.CommonName == "" { return fmt.Errorf("template subject.commonName cannot be empty") @@ -67,11 +99,11 @@ func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) error return fmt.Errorf("template issuer.commonName cannot be empty for root certificate") } - if tmpl.BasicConstraints.IsCA && len(tmpl.KeyUsage) == 0 { - return fmt.Errorf("CA certificate must specify at least one key usage") - } - + // For CA certs if tmpl.BasicConstraints.IsCA { + if len(tmpl.KeyUsage) == 0 { + return fmt.Errorf("CA certificate must specify at least one key usage") + } hasKeyUsageCertSign := false for _, usage := range tmpl.KeyUsage { if usage == "certSign" { @@ -82,11 +114,42 @@ func ValidateTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) error if !hasKeyUsageCertSign { return fmt.Errorf("CA certificate must have certSign key usage") } + } else { + // For non-CA certs + if len(tmpl.KeyUsage) == 0 { + return fmt.Errorf("certificate must specify at least one key usage") + } + hasDigitalSignature := false + for _, usage := range tmpl.KeyUsage { + if usage == "digitalSignature" { + hasDigitalSignature = true + break + } + } + if !hasDigitalSignature { + return fmt.Errorf("timestamp authority certificate must have digitalSignature key usage") + } + } + + // Validate extensions + for _, ext := range tmpl.Extensions { + if ext.ID == "" { + return fmt.Errorf("extension ID cannot be empty") + } + // Validate OID format + for _, n := range strings.Split(ext.ID, ".") { + if _, err := strconv.Atoi(n); err != nil { + return fmt.Errorf("invalid OID component in extension: %s", ext.ID) + } + } } return nil } +// CreateCertificateFromTemplate generates an x509.Certificate from the provided template +// applying all specified attributes including subject, issuer, validity period, +// constraints and extensions. func CreateCertificateFromTemplate(tmpl *CertificateTemplate, parent *x509.Certificate) (*x509.Certificate, error) { notBefore, err := time.Parse(time.RFC3339, tmpl.NotBefore) if err != nil { @@ -116,6 +179,7 @@ func CreateCertificateFromTemplate(tmpl *CertificateTemplate, parent *x509.Certi NotAfter: notAfter, BasicConstraintsValid: true, IsCA: tmpl.BasicConstraints.IsCA, + ExtraExtensions: []pkix.Extension{}, } if tmpl.BasicConstraints.IsCA { @@ -124,11 +188,37 @@ func CreateCertificateFromTemplate(tmpl *CertificateTemplate, parent *x509.Certi } SetKeyUsages(cert, tmpl.KeyUsage) - SetExtKeyUsages(cert, tmpl.ExtKeyUsage) + + // Sets extensions + for _, ext := range tmpl.Extensions { + var oid []int + for _, n := range strings.Split(ext.ID, ".") { + i, err := strconv.Atoi(n) + if err != nil { + return nil, fmt.Errorf("invalid OID in extension: %s", ext.ID) + } + oid = append(oid, i) + } + + extension := pkix.Extension{ + Id: oid, + Critical: ext.Critical, + } + + value, err := base64.StdEncoding.DecodeString(ext.Value) + if err != nil { + return nil, fmt.Errorf("error decoding extension value: %w", err) + } + extension.Value = value + + cert.ExtraExtensions = append(cert.ExtraExtensions, extension) + } return cert, nil } +// SetKeyUsages applies the specified key usage to cert(s) +// supporting certSign, crlSign, and digitalSignature usages. func SetKeyUsages(cert *x509.Certificate, usages []string) { for _, usage := range usages { switch usage { @@ -141,12 +231,3 @@ func SetKeyUsages(cert *x509.Certificate, usages []string) { } } } - -func SetExtKeyUsages(cert *x509.Certificate, usages []string) { - for _, usage := range usages { - switch usage { - case "timeStamping": - cert.ExtKeyUsage = append(cert.ExtKeyUsage, x509.ExtKeyUsageTimeStamping) - } - } -} diff --git a/pkg/certmaker/templates/intermediate-template.json b/pkg/certmaker/templates/intermediate-template.json index 7aaff054..b6a5c82a 100644 --- a/pkg/certmaker/templates/intermediate-template.json +++ b/pkg/certmaker/templates/intermediate-template.json @@ -28,7 +28,7 @@ { "id": "2.5.29.37", "critical": true, - "value": "asn1Seq (asn1Enc oid:1.3.6.1.5.5.7.3.8) | toJson" + "value": {{ asn1Seq (asn1Enc "oid:1.3.6.1.5.5.7.3.8") | toJson }} } ] } \ No newline at end of file