From 7ebe72bbfe342688a325325ea79e46970d017eb7 Mon Sep 17 00:00:00 2001 From: Herman Slatman Date: Wed, 3 Dec 2025 19:30:00 +0100 Subject: [PATCH] caddypki: Add support for multiple intermediates in signing chain (#7057) * caddypki: Add support for multiple intermediates in signing chain * Move intermediate lifetime configuration check In #7272 a check was changed to ensure that generated intermediate certificates would always use a lifetime that falls within the lifetime of the root. However, when a root and intermediate(s) are supplied, the configuration value was being used instead of the actual lifetimes of the certificates. The check was moved to only be performed when an intermediate is generated; not when loaded from disk. * Add tests for `pemDecodeCertificateChain` and `pemDecodeCertificate` * Use `crypto.Signer` instead of `any` in appropriate places * Use latest Smallstep packages --------- Co-authored-by: Matt Holt --- go.mod | 2 +- modules/caddypki/adminapi.go | 13 +- modules/caddypki/ca.go | 73 +++--- modules/caddypki/crypto.go | 65 ++++- modules/caddypki/crypto_test.go | 314 ++++++++++++++++++++++++ modules/caddypki/maintain.go | 10 +- modules/caddytls/capools.go | 6 +- modules/caddytls/internalissuer.go | 3 +- modules/caddytls/internalissuer_test.go | 262 ++++++++++++++++++++ 9 files changed, 700 insertions(+), 48 deletions(-) create mode 100644 modules/caddypki/crypto_test.go create mode 100644 modules/caddytls/internalissuer_test.go diff --git a/go.mod b/go.mod index 29198a510..5269a0841 100644 --- a/go.mod +++ b/go.mod @@ -35,6 +35,7 @@ require ( go.opentelemetry.io/contrib/propagators/autoprop v0.63.0 go.opentelemetry.io/otel v1.38.0 go.opentelemetry.io/otel/sdk v1.38.0 + go.step.sm/crypto v0.74.0 go.uber.org/automaxprocs v1.6.0 go.uber.org/zap v1.27.1 go.uber.org/zap/exp v0.3.0 @@ -166,7 +167,6 @@ require ( go.opentelemetry.io/otel/metric v1.38.0 // indirect go.opentelemetry.io/otel/trace v1.38.0 go.opentelemetry.io/proto/otlp v1.7.1 // indirect - go.step.sm/crypto v0.74.0 go.uber.org/multierr v1.11.0 // indirect golang.org/x/mod v0.29.0 // indirect golang.org/x/sys v0.38.0 diff --git a/modules/caddypki/adminapi.go b/modules/caddypki/adminapi.go index 463e31f35..c37b8d7b6 100644 --- a/modules/caddypki/adminapi.go +++ b/modules/caddypki/adminapi.go @@ -222,11 +222,16 @@ func rootAndIntermediatePEM(ca *CA) (root, inter []byte, err error) { if err != nil { return root, inter, err } - inter, err = pemEncodeCert(ca.IntermediateCertificate().Raw) - if err != nil { - return root, inter, err + + for _, interCert := range ca.IntermediateCertificateChain() { + pemBytes, err := pemEncodeCert(interCert.Raw) + if err != nil { + return nil, nil, err + } + inter = append(inter, pemBytes...) } - return root, inter, err + + return } // caInfo is the response structure for the CA info API endpoint. diff --git a/modules/caddypki/ca.go b/modules/caddypki/ca.go index 5b17518ca..8f6fd3afe 100644 --- a/modules/caddypki/ca.go +++ b/modules/caddypki/ca.go @@ -75,10 +75,11 @@ type CA struct { // and module provisioning. ID string `json:"-"` - storage certmagic.Storage - root, inter *x509.Certificate - interKey any // TODO: should we just store these as crypto.Signer? - mu *sync.RWMutex + storage certmagic.Storage + root *x509.Certificate + interChain []*x509.Certificate + interKey crypto.Signer + mu *sync.RWMutex rootCertPath string // mainly used for logging purposes if trusting log *zap.Logger @@ -127,14 +128,16 @@ func (ca *CA) Provision(ctx caddy.Context, id string, log *zap.Logger) error { } // load the certs and key that will be used for signing - var rootCert, interCert *x509.Certificate + var rootCert *x509.Certificate + var rootCertChain, interCertChain []*x509.Certificate var rootKey, interKey crypto.Signer var err error if ca.Root != nil { if ca.Root.Format == "" || ca.Root.Format == "pem_file" { ca.rootCertPath = ca.Root.Certificate } - rootCert, rootKey, err = ca.Root.Load() + rootCertChain, rootKey, err = ca.Root.Load() + rootCert = rootCertChain[0] } else { ca.rootCertPath = "storage:" + ca.storageKeyRootCert() rootCert, rootKey, err = ca.loadOrGenRoot() @@ -142,21 +145,23 @@ func (ca *CA) Provision(ctx caddy.Context, id string, log *zap.Logger) error { if err != nil { return err } - actualRootLifetime := time.Until(rootCert.NotAfter) - if time.Duration(ca.IntermediateLifetime) >= actualRootLifetime { - return fmt.Errorf("intermediate certificate lifetime must be less than actual root certificate lifetime (%s)", actualRootLifetime) - } + if ca.Intermediate != nil { - interCert, interKey, err = ca.Intermediate.Load() + interCertChain, interKey, err = ca.Intermediate.Load() } else { - interCert, interKey, err = ca.loadOrGenIntermediate(rootCert, rootKey) + actualRootLifetime := time.Until(rootCert.NotAfter) + if time.Duration(ca.IntermediateLifetime) >= actualRootLifetime { + return fmt.Errorf("intermediate certificate lifetime must be less than actual root certificate lifetime (%s)", actualRootLifetime) + } + + interCertChain, interKey, err = ca.loadOrGenIntermediate(rootCert, rootKey) } if err != nil { return err } ca.mu.Lock() - ca.root, ca.inter, ca.interKey = rootCert, interCert, interKey + ca.root, ca.interChain, ca.interKey = rootCert, interCertChain, interKey ca.mu.Unlock() return nil @@ -172,21 +177,21 @@ func (ca CA) RootCertificate() *x509.Certificate { // RootKey returns the CA's root private key. Since the root key is // not cached in memory long-term, it needs to be loaded from storage, // which could yield an error. -func (ca CA) RootKey() (any, error) { +func (ca CA) RootKey() (crypto.Signer, error) { _, rootKey, err := ca.loadOrGenRoot() return rootKey, err } -// IntermediateCertificate returns the CA's intermediate -// certificate (public key). -func (ca CA) IntermediateCertificate() *x509.Certificate { +// IntermediateCertificateChain returns the CA's intermediate +// certificate chain. +func (ca CA) IntermediateCertificateChain() []*x509.Certificate { ca.mu.RLock() defer ca.mu.RUnlock() - return ca.inter + return ca.interChain } // IntermediateKey returns the CA's intermediate private key. -func (ca CA) IntermediateKey() any { +func (ca CA) IntermediateKey() crypto.Signer { ca.mu.RLock() defer ca.mu.RUnlock() return ca.interKey @@ -207,26 +212,27 @@ func (ca *CA) NewAuthority(authorityConfig AuthorityConfig) (*authority.Authorit // cert/key directly, since it's unlikely to expire // while Caddy is running (long lifetime) var issuerCert *x509.Certificate - var issuerKey any + var issuerKey crypto.Signer issuerCert = rootCert var err error issuerKey, err = ca.RootKey() if err != nil { return nil, fmt.Errorf("loading signing key: %v", err) } - signerOption = authority.WithX509Signer(issuerCert, issuerKey.(crypto.Signer)) + signerOption = authority.WithX509Signer(issuerCert, issuerKey) } else { // if we're signing with intermediate, we need to make // sure it's always fresh, because the intermediate may // renew while Caddy is running (medium lifetime) signerOption = authority.WithX509SignerFunc(func() ([]*x509.Certificate, crypto.Signer, error) { - issuerCert := ca.IntermediateCertificate() - issuerKey := ca.IntermediateKey().(crypto.Signer) + issuerChain := ca.IntermediateCertificateChain() + issuerCert := issuerChain[0] + issuerKey := ca.IntermediateKey() ca.log.Debug("using intermediate signer", zap.String("serial", issuerCert.SerialNumber.String()), zap.String("not_before", issuerCert.NotBefore.String()), zap.String("not_after", issuerCert.NotAfter.String())) - return []*x509.Certificate{issuerCert}, issuerKey, nil + return issuerChain, issuerKey, nil }) } @@ -252,7 +258,11 @@ func (ca *CA) NewAuthority(authorityConfig AuthorityConfig) (*authority.Authorit func (ca CA) loadOrGenRoot() (rootCert *x509.Certificate, rootKey crypto.Signer, err error) { if ca.Root != nil { - return ca.Root.Load() + rootChain, rootSigner, err := ca.Root.Load() + if err != nil { + return nil, nil, err + } + return rootChain[0], rootSigner, nil } rootCertPEM, err := ca.storage.Load(ca.ctx, ca.storageKeyRootCert()) if err != nil { @@ -268,7 +278,7 @@ func (ca CA) loadOrGenRoot() (rootCert *x509.Certificate, rootKey crypto.Signer, } if rootCert == nil { - rootCert, err = pemDecodeSingleCert(rootCertPEM) + rootCert, err = pemDecodeCertificate(rootCertPEM) if err != nil { return nil, nil, fmt.Errorf("parsing root certificate PEM: %v", err) } @@ -314,7 +324,8 @@ func (ca CA) genRoot() (rootCert *x509.Certificate, rootKey crypto.Signer, err e return rootCert, rootKey, nil } -func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey crypto.Signer) (interCert *x509.Certificate, interKey crypto.Signer, err error) { +func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey crypto.Signer) (interCertChain []*x509.Certificate, interKey crypto.Signer, err error) { + var interCert *x509.Certificate interCertPEM, err := ca.storage.Load(ca.ctx, ca.storageKeyIntermediateCert()) if err != nil { if !errors.Is(err, fs.ErrNotExist) { @@ -326,10 +337,12 @@ func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey crypto.Si if err != nil { return nil, nil, fmt.Errorf("generating new intermediate cert: %v", err) } + + interCertChain = append(interCertChain, interCert) } - if interCert == nil { - interCert, err = pemDecodeSingleCert(interCertPEM) + if len(interCertChain) == 0 { + interCertChain, err = pemDecodeCertificateChain(interCertPEM) if err != nil { return nil, nil, fmt.Errorf("decoding intermediate certificate PEM: %v", err) } @@ -346,7 +359,7 @@ func (ca CA) loadOrGenIntermediate(rootCert *x509.Certificate, rootKey crypto.Si } } - return interCert, interKey, nil + return interCertChain, interKey, nil } func (ca CA) genIntermediate(rootCert *x509.Certificate, rootKey crypto.Signer) (interCert *x509.Certificate, interKey crypto.Signer, err error) { diff --git a/modules/caddypki/crypto.go b/modules/caddypki/crypto.go index 324a4fcfa..715155eb2 100644 --- a/modules/caddypki/crypto.go +++ b/modules/caddypki/crypto.go @@ -17,15 +17,20 @@ package caddypki import ( "bytes" "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/rsa" "crypto/x509" "encoding/pem" + "errors" "fmt" "os" "github.com/caddyserver/certmagic" + "go.step.sm/crypto/pemutil" ) -func pemDecodeSingleCert(pemDER []byte) (*x509.Certificate, error) { +func pemDecodeCertificate(pemDER []byte) (*x509.Certificate, error) { pemBlock, remaining := pem.Decode(pemDER) if pemBlock == nil { return nil, fmt.Errorf("no PEM block found") @@ -39,6 +44,15 @@ func pemDecodeSingleCert(pemDER []byte) (*x509.Certificate, error) { return x509.ParseCertificate(pemBlock.Bytes) } +func pemDecodeCertificateChain(pemDER []byte) ([]*x509.Certificate, error) { + chain, err := pemutil.ParseCertificateBundle(pemDER) + if err != nil { + return nil, fmt.Errorf("failed parsing certificate chain: %w", err) + } + + return chain, nil +} + func pemEncodeCert(der []byte) ([]byte, error) { return pemEncode("CERTIFICATE", der) } @@ -70,15 +84,18 @@ type KeyPair struct { Format string `json:"format,omitempty"` } -// Load loads the certificate and key. -func (kp KeyPair) Load() (*x509.Certificate, crypto.Signer, error) { +// Load loads the certificate chain and (optional) private key from +// the corresponding files, using the configured format. If a +// private key is read, it will be verified to belong to the first +// certificate in the chain. +func (kp KeyPair) Load() ([]*x509.Certificate, crypto.Signer, error) { switch kp.Format { case "", "pem_file": certData, err := os.ReadFile(kp.Certificate) if err != nil { return nil, nil, err } - cert, err := pemDecodeSingleCert(certData) + chain, err := pemDecodeCertificateChain(certData) if err != nil { return nil, nil, err } @@ -93,11 +110,49 @@ func (kp KeyPair) Load() (*x509.Certificate, crypto.Signer, error) { if err != nil { return nil, nil, err } + if err := verifyKeysMatch(chain[0], key); err != nil { + return nil, nil, err + } } - return cert, key, nil + return chain, key, nil default: return nil, nil, fmt.Errorf("unsupported format: %s", kp.Format) } } + +// verifyKeysMatch verifies that the public key in the [x509.Certificate] matches +// the public key of the [crypto.Signer]. +func verifyKeysMatch(crt *x509.Certificate, signer crypto.Signer) error { + switch pub := crt.PublicKey.(type) { + case *rsa.PublicKey: + pk, ok := signer.Public().(*rsa.PublicKey) + if !ok { + return fmt.Errorf("private key type %T does not match issuer public key type %T", signer.Public(), pub) + } + if !pub.Equal(pk) { + return errors.New("private key does not match issuer public key") + } + case *ecdsa.PublicKey: + pk, ok := signer.Public().(*ecdsa.PublicKey) + if !ok { + return fmt.Errorf("private key type %T does not match issuer public key type %T", signer.Public(), pub) + } + if !pub.Equal(pk) { + return errors.New("private key does not match issuer public key") + } + case ed25519.PublicKey: + pk, ok := signer.Public().(ed25519.PublicKey) + if !ok { + return fmt.Errorf("private key type %T does not match issuer public key type %T", signer.Public(), pub) + } + if !pub.Equal(pk) { + return errors.New("private key does not match issuer public key") + } + default: + return fmt.Errorf("unsupported key type: %T", pub) + } + + return nil +} diff --git a/modules/caddypki/crypto_test.go b/modules/caddypki/crypto_test.go new file mode 100644 index 000000000..a07763d14 --- /dev/null +++ b/modules/caddypki/crypto_test.go @@ -0,0 +1,314 @@ +// Copyright 2015 Matthew Holt and The Caddy 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 caddypki + +import ( + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "os" + "path/filepath" + "testing" + "time" + + "go.step.sm/crypto/keyutil" + "go.step.sm/crypto/pemutil" +) + +func TestKeyPair_Load(t *testing.T) { + rootSigner, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Failed creating signer: %v", err) + } + + tmpl := &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-root"}, + IsCA: true, + MaxPathLen: 3, + } + rootBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, rootSigner.Public(), rootSigner) + if err != nil { + t.Fatalf("Creating root certificate failed: %v", err) + } + + root, err := x509.ParseCertificate(rootBytes) + if err != nil { + t.Fatalf("Parsing root certificate failed: %v", err) + } + + intermediateSigner, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Creating intermedaite signer failed: %v", err) + } + + intermediateBytes, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-first-intermediate"}, + IsCA: true, + MaxPathLen: 2, + NotAfter: time.Now().Add(time.Hour), + }, root, intermediateSigner.Public(), rootSigner) + if err != nil { + t.Fatalf("Creating intermediate certificate failed: %v", err) + } + + intermediate, err := x509.ParseCertificate(intermediateBytes) + if err != nil { + t.Fatalf("Parsing intermediate certificate failed: %v", err) + } + + var chainContents []byte + chain := []*x509.Certificate{intermediate, root} + for _, cert := range chain { + b, err := pemutil.Serialize(cert) + if err != nil { + t.Fatalf("Failed serializing intermediate certificate: %v", err) + } + chainContents = append(chainContents, pem.EncodeToMemory(b)...) + } + + dir := t.TempDir() + rootCertFile := filepath.Join(dir, "root.pem") + if _, err = pemutil.Serialize(root, pemutil.WithFilename(rootCertFile)); err != nil { + t.Fatalf("Failed serializing root certificate: %v", err) + } + rootKeyFile := filepath.Join(dir, "root.key") + if _, err = pemutil.Serialize(rootSigner, pemutil.WithFilename(rootKeyFile)); err != nil { + t.Fatalf("Failed serializing root key: %v", err) + } + intermediateCertFile := filepath.Join(dir, "intermediate.pem") + if _, err = pemutil.Serialize(intermediate, pemutil.WithFilename(intermediateCertFile)); err != nil { + t.Fatalf("Failed serializing intermediate certificate: %v", err) + } + intermediateKeyFile := filepath.Join(dir, "intermediate.key") + if _, err = pemutil.Serialize(intermediateSigner, pemutil.WithFilename(intermediateKeyFile)); err != nil { + t.Fatalf("Failed serializing intermediate key: %v", err) + } + chainFile := filepath.Join(dir, "chain.pem") + if err := os.WriteFile(chainFile, chainContents, 0644); err != nil { + t.Fatalf("Failed writing intermediate chain: %v", err) + } + + t.Run("ok/single-certificate-without-signer", func(t *testing.T) { + kp := KeyPair{ + Certificate: rootCertFile, + } + chain, signer, err := kp.Load() + if err != nil { + t.Fatalf("Failed loading KeyPair: %v", err) + } + if len(chain) != 1 { + t.Errorf("Expected 1 certificate in chain; got %d", len(chain)) + } + if signer != nil { + t.Error("Expected no signer to be returned") + } + }) + + t.Run("ok/single-certificate-with-signer", func(t *testing.T) { + kp := KeyPair{ + Certificate: rootCertFile, + PrivateKey: rootKeyFile, + } + chain, signer, err := kp.Load() + if err != nil { + t.Fatalf("Failed loading KeyPair: %v", err) + } + if len(chain) != 1 { + t.Errorf("Expected 1 certificate in chain; got %d", len(chain)) + } + if signer == nil { + t.Error("Expected signer to be returned") + } + }) + + t.Run("ok/multiple-certificates-with-signer", func(t *testing.T) { + kp := KeyPair{ + Certificate: chainFile, + PrivateKey: intermediateKeyFile, + } + chain, signer, err := kp.Load() + if err != nil { + t.Fatalf("Failed loading KeyPair: %v", err) + } + if len(chain) != 2 { + t.Errorf("Expected 2 certificates in chain; got %d", len(chain)) + } + if signer == nil { + t.Error("Expected signer to be returned") + } + }) + + t.Run("fail/non-matching-public-key", func(t *testing.T) { + kp := KeyPair{ + Certificate: intermediateCertFile, + PrivateKey: rootKeyFile, + } + chain, signer, err := kp.Load() + if err == nil { + t.Error("Expected loading KeyPair to return an error") + } + if chain != nil { + t.Error("Expected no chain to be returned") + } + if signer != nil { + t.Error("Expected no signer to be returned") + } + }) +} + +func Test_pemDecodeCertificate(t *testing.T) { + signer, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Failed creating signer: %v", err) + } + + tmpl := &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-cert"}, + IsCA: true, + MaxPathLen: 3, + } + derBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, signer.Public(), signer) + if err != nil { + t.Fatalf("Creating root certificate failed: %v", err) + } + cert, err := x509.ParseCertificate(derBytes) + if err != nil { + t.Fatalf("Parsing root certificate failed: %v", err) + } + + pemBlock, err := pemutil.Serialize(cert) + if err != nil { + t.Fatalf("Failed serializing certificate: %v", err) + } + pemData := pem.EncodeToMemory(pemBlock) + + t.Run("ok", func(t *testing.T) { + cert, err := pemDecodeCertificate(pemData) + if err != nil { + t.Fatalf("Failed decoding PEM data: %v", err) + } + if cert == nil { + t.Errorf("Expected a certificate in PEM data") + } + }) + + t.Run("fail/no-pem-data", func(t *testing.T) { + cert, err := pemDecodeCertificate(nil) + if err == nil { + t.Fatalf("Expected pemDecodeCertificate to return an error") + } + if cert != nil { + t.Errorf("Expected pemDecodeCertificate to return nil") + } + }) + + t.Run("fail/multiple", func(t *testing.T) { + multiplePEMData := append(pemData, pemData...) + cert, err := pemDecodeCertificate(multiplePEMData) + if err == nil { + t.Fatalf("Expected pemDecodeCertificate to return an error") + } + if cert != nil { + t.Errorf("Expected pemDecodeCertificate to return nil") + } + }) + + t.Run("fail/no-pem-certificate", func(t *testing.T) { + pkData := pem.EncodeToMemory(&pem.Block{ + Type: "PRIVATE KEY", + Bytes: []byte("some-bogus-private-key"), + }) + cert, err := pemDecodeCertificate(pkData) + if err == nil { + t.Fatalf("Expected pemDecodeCertificate to return an error") + } + if cert != nil { + t.Errorf("Expected pemDecodeCertificate to return nil") + } + }) +} + +func Test_pemDecodeCertificateChain(t *testing.T) { + signer, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Failed creating signer: %v", err) + } + + tmpl := &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-cert"}, + IsCA: true, + MaxPathLen: 3, + } + derBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, signer.Public(), signer) + if err != nil { + t.Fatalf("Creating root certificate failed: %v", err) + } + cert, err := x509.ParseCertificate(derBytes) + if err != nil { + t.Fatalf("Parsing root certificate failed: %v", err) + } + + pemBlock, err := pemutil.Serialize(cert) + if err != nil { + t.Fatalf("Failed serializing certificate: %v", err) + } + pemData := pem.EncodeToMemory(pemBlock) + + t.Run("ok/single", func(t *testing.T) { + certs, err := pemDecodeCertificateChain(pemData) + if err != nil { + t.Fatalf("Failed decoding PEM data: %v", err) + } + if len(certs) != 1 { + t.Errorf("Expected 1 certificate in PEM data; got %d", len(certs)) + } + }) + + t.Run("ok/multiple", func(t *testing.T) { + multiplePEMData := append(pemData, pemData...) + certs, err := pemDecodeCertificateChain(multiplePEMData) + if err != nil { + t.Fatalf("Failed decoding PEM data: %v", err) + } + if len(certs) != 2 { + t.Errorf("Expected 2 certificates in PEM data; got %d", len(certs)) + } + }) + + t.Run("fail/no-pem-certificate", func(t *testing.T) { + pkData := pem.EncodeToMemory(&pem.Block{ + Type: "PRIVATE KEY", + Bytes: []byte("some-bogus-private-key"), + }) + certs, err := pemDecodeCertificateChain(pkData) + if err == nil { + t.Fatalf("Expected pemDecodeCertificateChain to return an error") + } + if len(certs) != 0 { + t.Errorf("Expected 0 certificates in PEM data; got %d", len(certs)) + } + }) + + t.Run("fail/no-der-certificate", func(t *testing.T) { + certs, err := pemDecodeCertificateChain([]byte("invalid-der-data")) + if err == nil { + t.Fatalf("Expected pemDecodeCertificateChain to return an error") + } + if len(certs) != 0 { + t.Errorf("Expected 0 certificates in PEM data; got %d", len(certs)) + } + }) +} diff --git a/modules/caddypki/maintain.go b/modules/caddypki/maintain.go index 31e453ff9..091e71243 100644 --- a/modules/caddypki/maintain.go +++ b/modules/caddypki/maintain.go @@ -66,16 +66,16 @@ func (p *PKI) renewCertsForCA(ca *CA) error { if needsRenewal(ca.root) { // TODO: implement root renewal (use same key) log.Warn("root certificate expiring soon (FIXME: ROOT RENEWAL NOT YET IMPLEMENTED)", - zap.Duration("time_remaining", time.Until(ca.inter.NotAfter)), + zap.Duration("time_remaining", time.Until(ca.interChain[0].NotAfter)), ) } } // only maintain the intermediate if it's not manually provided in the config if ca.Intermediate == nil { - if needsRenewal(ca.inter) { + if needsRenewal(ca.interChain[0]) { log.Info("intermediate expires soon; renewing", - zap.Duration("time_remaining", time.Until(ca.inter.NotAfter)), + zap.Duration("time_remaining", time.Until(ca.interChain[0].NotAfter)), ) rootCert, rootKey, err := ca.loadOrGenRoot() @@ -86,10 +86,10 @@ func (p *PKI) renewCertsForCA(ca *CA) error { if err != nil { return fmt.Errorf("generating new certificate: %v", err) } - ca.inter, ca.interKey = interCert, interKey + ca.interChain, ca.interKey = []*x509.Certificate{interCert}, interKey log.Info("renewed intermediate", - zap.Time("new_expiration", ca.inter.NotAfter), + zap.Time("new_expiration", ca.interChain[0].NotAfter), ) } } diff --git a/modules/caddytls/capools.go b/modules/caddytls/capools.go index c73bc4832..5ed6a82a9 100644 --- a/modules/caddytls/capools.go +++ b/modules/caddytls/capools.go @@ -257,7 +257,7 @@ func (PKIIntermediateCAPool) CaddyModule() caddy.ModuleInfo { } } -// Loads the PKI app and load the intermediate certificates into the certificate pool +// Loads the PKI app and loads the intermediate certificates into the certificate pool func (p *PKIIntermediateCAPool) Provision(ctx caddy.Context) error { pkiApp, err := ctx.AppIfConfigured("pki") if err != nil { @@ -274,7 +274,9 @@ func (p *PKIIntermediateCAPool) Provision(ctx caddy.Context) error { caPool := x509.NewCertPool() for _, ca := range p.ca { - caPool.AddCert(ca.IntermediateCertificate()) + for _, c := range ca.IntermediateCertificateChain() { + caPool.AddCert(c) + } } p.pool = caPool return nil diff --git a/modules/caddytls/internalissuer.go b/modules/caddytls/internalissuer.go index be779757a..cad19f063 100644 --- a/modules/caddytls/internalissuer.go +++ b/modules/caddytls/internalissuer.go @@ -115,7 +115,8 @@ func (iss InternalIssuer) Issue(ctx context.Context, csr *x509.CertificateReques if iss.SignWithRoot { issuerCert = iss.ca.RootCertificate() } else { - issuerCert = iss.ca.IntermediateCertificate() + chain := iss.ca.IntermediateCertificateChain() + issuerCert = chain[0] } // ensure issued certificate does not expire later than its issuer diff --git a/modules/caddytls/internalissuer_test.go b/modules/caddytls/internalissuer_test.go new file mode 100644 index 000000000..d39d8373b --- /dev/null +++ b/modules/caddytls/internalissuer_test.go @@ -0,0 +1,262 @@ +// Copyright 2015 Matthew Holt and The Caddy 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 caddytls + +import ( + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/modules/caddypki" + "go.uber.org/zap" + + "go.step.sm/crypto/keyutil" + "go.step.sm/crypto/pemutil" +) + +func TestInternalIssuer_Issue(t *testing.T) { + rootSigner, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Creating root signer failed: %v", err) + } + + tmpl := &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-root"}, + IsCA: true, + MaxPathLen: 3, + NotAfter: time.Now().Add(7 * 24 * time.Hour), + NotBefore: time.Now().Add(-7 * 24 * time.Hour), + } + rootBytes, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, rootSigner.Public(), rootSigner) + if err != nil { + t.Fatalf("Creating root certificate failed: %v", err) + } + + root, err := x509.ParseCertificate(rootBytes) + if err != nil { + t.Fatalf("Parsing root certificate failed: %v", err) + } + + firstIntermediateSigner, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Creating intermedaite signer failed: %v", err) + } + + firstIntermediateBytes, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-first-intermediate"}, + IsCA: true, + MaxPathLen: 2, + NotAfter: time.Now().Add(24 * time.Hour), + NotBefore: time.Now().Add(-24 * time.Hour), + }, root, firstIntermediateSigner.Public(), rootSigner) + if err != nil { + t.Fatalf("Creating intermediate certificate failed: %v", err) + } + + firstIntermediate, err := x509.ParseCertificate(firstIntermediateBytes) + if err != nil { + t.Fatalf("Parsing intermediate certificate failed: %v", err) + } + + secondIntermediateSigner, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Creating second intermedaite signer failed: %v", err) + } + + secondIntermediateBytes, err := x509.CreateCertificate(rand.Reader, &x509.Certificate{ + Subject: pkix.Name{CommonName: "test-second-intermediate"}, + IsCA: true, + MaxPathLen: 2, + NotAfter: time.Now().Add(24 * time.Hour), + NotBefore: time.Now().Add(-24 * time.Hour), + }, firstIntermediate, secondIntermediateSigner.Public(), firstIntermediateSigner) + if err != nil { + t.Fatalf("Creating second intermediate certificate failed: %v", err) + } + + secondIntermediate, err := x509.ParseCertificate(secondIntermediateBytes) + if err != nil { + t.Fatalf("Parsing second intermediate certificate failed: %v", err) + } + + dir := t.TempDir() + storageDir := filepath.Join(dir, "certmagic") + rootCertFile := filepath.Join(dir, "root.pem") + if _, err = pemutil.Serialize(root, pemutil.WithFilename(rootCertFile)); err != nil { + t.Fatalf("Failed serializing root certificate: %v", err) + } + intermediateCertFile := filepath.Join(dir, "intermediate.pem") + if _, err = pemutil.Serialize(firstIntermediate, pemutil.WithFilename(intermediateCertFile)); err != nil { + t.Fatalf("Failed serializing intermediate certificate: %v", err) + } + intermediateKeyFile := filepath.Join(dir, "intermediate.key") + if _, err = pemutil.Serialize(firstIntermediateSigner, pemutil.WithFilename(intermediateKeyFile)); err != nil { + t.Fatalf("Failed serializing intermediate key: %v", err) + } + + var intermediateChainContents []byte + intermediateChain := []*x509.Certificate{secondIntermediate, firstIntermediate} + for _, cert := range intermediateChain { + b, err := pemutil.Serialize(cert) + if err != nil { + t.Fatalf("Failed serializing intermediate certificate: %v", err) + } + intermediateChainContents = append(intermediateChainContents, pem.EncodeToMemory(b)...) + } + intermediateChainFile := filepath.Join(dir, "intermediates.pem") + if err := os.WriteFile(intermediateChainFile, intermediateChainContents, 0644); err != nil { + t.Fatalf("Failed writing intermediate chain: %v", err) + } + intermediateChainKeyFile := filepath.Join(dir, "intermediates.key") + if _, err = pemutil.Serialize(secondIntermediateSigner, pemutil.WithFilename(intermediateChainKeyFile)); err != nil { + t.Fatalf("Failed serializing intermediate key: %v", err) + } + + signer, err := keyutil.GenerateDefaultSigner() + if err != nil { + t.Fatalf("Failed creating signer: %v", err) + } + + csrBytes, err := x509.CreateCertificateRequest(rand.Reader, &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: "test"}, + }, signer) + if err != nil { + t.Fatalf("Failed creating CSR: %v", err) + } + + csr, err := x509.ParseCertificateRequest(csrBytes) + if err != nil { + t.Fatalf("Failed parsing CSR: %v", err) + } + + t.Run("generated-with-defaults", func(t *testing.T) { + caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: t.Context()}) + t.Cleanup(cancel) + logger := zap.NewNop() + + ca := &caddypki.CA{ + StorageRaw: []byte(fmt.Sprintf(`{"module": "file_system", "root": %q}`, storageDir)), + } + if err := ca.Provision(caddyCtx, "local-test-generated", logger); err != nil { + t.Fatalf("Failed provisioning CA: %v", err) + } + + iss := InternalIssuer{ + SignWithRoot: false, + ca: ca, + logger: logger, + } + + c, err := iss.Issue(t.Context(), csr) + if err != nil { + t.Fatalf("Failed issuing certificate: %v", err) + } + + chain, err := pemutil.ParseCertificateBundle(c.Certificate) + if err != nil { + t.Errorf("Failed issuing certificate: %v", err) + } + if len(chain) != 2 { + t.Errorf("Expected 2 certificates in chain; got %d", len(chain)) + } + }) + + t.Run("single-intermediate-from-disk", func(t *testing.T) { + caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: t.Context()}) + t.Cleanup(cancel) + logger := zap.NewNop() + + ca := &caddypki.CA{ + Root: &caddypki.KeyPair{ + Certificate: rootCertFile, + }, + Intermediate: &caddypki.KeyPair{ + Certificate: intermediateCertFile, + PrivateKey: intermediateKeyFile, + }, + StorageRaw: []byte(fmt.Sprintf(`{"module": "file_system", "root": %q}`, storageDir)), + } + + if err := ca.Provision(caddyCtx, "local-test-single-intermediate", logger); err != nil { + t.Fatalf("Failed provisioning CA: %v", err) + } + + iss := InternalIssuer{ + ca: ca, + SignWithRoot: false, + logger: logger, + } + + c, err := iss.Issue(t.Context(), csr) + if err != nil { + t.Fatalf("Failed issuing certificate: %v", err) + } + + chain, err := pemutil.ParseCertificateBundle(c.Certificate) + if err != nil { + t.Errorf("Failed issuing certificate: %v", err) + } + if len(chain) != 2 { + t.Errorf("Expected 2 certificates in chain; got %d", len(chain)) + } + }) + + t.Run("multiple-intermediates-from-disk", func(t *testing.T) { + caddyCtx, cancel := caddy.NewContext(caddy.Context{Context: t.Context()}) + t.Cleanup(cancel) + logger := zap.NewNop() + + ca := &caddypki.CA{ + Root: &caddypki.KeyPair{ + Certificate: rootCertFile, + }, + Intermediate: &caddypki.KeyPair{ + Certificate: intermediateChainFile, + PrivateKey: intermediateChainKeyFile, + }, + StorageRaw: []byte(fmt.Sprintf(`{"module": "file_system", "root": %q}`, storageDir)), + } + + if err := ca.Provision(caddyCtx, "local-test", zap.NewNop()); err != nil { + t.Fatalf("Failed provisioning CA: %v", err) + } + + iss := InternalIssuer{ + ca: ca, + SignWithRoot: false, + logger: logger, + } + + c, err := iss.Issue(t.Context(), csr) + if err != nil { + t.Fatalf("Failed issuing certificate: %v", err) + } + + chain, err := pemutil.ParseCertificateBundle(c.Certificate) + if err != nil { + t.Errorf("Failed issuing certificate: %v", err) + } + if len(chain) != 3 { + t.Errorf("Expected 3 certificates in chain; got %d", len(chain)) + } + }) +}