From dfabece6eb3e7664cbcfecb0d295436fcfe60f13 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Mon, 25 Feb 2019 20:42:44 +0000 Subject: [PATCH] Use a one-use CA to sign temporary certificates Signed-off-by: James Munnelly --- pkg/controller/certificates/controller.go | 2 +- pkg/controller/certificates/sync.go | 50 ++++++++++++++++++++++- pkg/issuer/ca/issue.go | 2 +- pkg/issuer/ca/issue_test.go | 3 +- pkg/issuer/selfsigned/issue.go | 2 +- pkg/util/pki/csr.go | 2 +- 6 files changed, 53 insertions(+), 8 deletions(-) diff --git a/pkg/controller/certificates/controller.go b/pkg/controller/certificates/controller.go index 609d29326..255dd6674 100644 --- a/pkg/controller/certificates/controller.go +++ b/pkg/controller/certificates/controller.go @@ -111,7 +111,7 @@ func New(ctx *controllerpkg.Context) *Controller { ctrl.helper = issuer.NewHelper(ctrl.issuerLister, ctrl.clusterIssuerLister) ctrl.issuerFactory = issuer.NewIssuerFactory(ctx) ctrl.clock = clock.RealClock{} - ctrl.localTemporarySigner = ctrl.generateTemporaryCertificate + ctrl.localTemporarySigner = generateLocallySignedTemporaryCertificate return ctrl } diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index b47fcda9a..409084d59 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -449,8 +449,8 @@ func isTemporaryCertificate(cert *x509.Certificate) bool { return cert.SerialNumber.Int64() == staticTemporarySerialNumber } -func (c *Controller) generateTemporaryCertificate(crt *v1alpha1.Certificate, pk []byte) ([]byte, error) { - template, err := pki.GenerateTemplate(nil, crt) +func generateSelfSignedTemporaryCertificate(crt *v1alpha1.Certificate, pk []byte) ([]byte, error) { + template, err := pki.GenerateTemplate(crt) template.SerialNumber = big.NewInt(staticTemporarySerialNumber) signer, err := pki.DecodePrivateKeyBytes(pk) @@ -466,6 +466,52 @@ func (c *Controller) generateTemporaryCertificate(crt *v1alpha1.Certificate, pk return b, nil } +// generateLocallySignedTemporaryCertificate signs a temporary certificate for +// the given certificate resource using a one-use temporary CA that is then +// discarded afterwards. +// This is to mitigate a potential attack against x509 certificates that use a +// predictable serial number and weak MD5 hashing algorithms. +// In practice, this shouldn't really be a concern anyway. +func generateLocallySignedTemporaryCertificate(crt *v1alpha1.Certificate, pk []byte) ([]byte, error) { + // generate a throwaway self-signed root CA + caPk, err := pki.GenerateECPrivateKey(pki.ECCurve521) + if err != nil { + return nil, err + } + caCertTemplate, err := pki.GenerateTemplate(&v1alpha1.Certificate{ + Spec: v1alpha1.CertificateSpec{ + CommonName: "cert-manager.local", + IsCA: true, + }, + }) + if err != nil { + return nil, err + } + _, caCert, err := pki.SignCertificate(caCertTemplate, caCertTemplate, caPk.Public(), caPk) + if err != nil { + return nil, err + } + + // sign a temporary certificate using the root CA + template, err := pki.GenerateTemplate(crt) + if err != nil { + return nil, err + } + template.SerialNumber = big.NewInt(staticTemporarySerialNumber) + + signeeKey, err := pki.DecodePrivateKeyBytes(pk) + if err != nil { + return nil, err + } + + b, _, err := pki.SignCertificate(template, caCert, signeeKey.Public(), caPk) + if err != nil { + return nil, err + } + + return b, nil +} + func (c *Controller) updateCertificateStatus(old, new *v1alpha1.Certificate) (*v1alpha1.Certificate, error) { if reflect.DeepEqual(old.Status, new.Status) { return nil, nil diff --git a/pkg/issuer/ca/issue.go b/pkg/issuer/ca/issue.go index 5ae54c54e..68de6077a 100644 --- a/pkg/issuer/ca/issue.go +++ b/pkg/issuer/ca/issue.go @@ -77,7 +77,7 @@ func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*issuer.Issu } // generate a x509 certificate template for this Certificate - template, err := pki.GenerateTemplate(c.issuer, crt) + template, err := pki.GenerateTemplate(crt) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) return nil, err diff --git a/pkg/issuer/ca/issue_test.go b/pkg/issuer/ca/issue_test.go index 4e0c2498b..a3f1ec78b 100644 --- a/pkg/issuer/ca/issue_test.go +++ b/pkg/issuer/ca/issue_test.go @@ -58,8 +58,7 @@ func generateECDSAPrivateKey(t *testing.T) *ecdsa.PrivateKey { } func generateSelfSignedCert(t *testing.T, crt *v1alpha1.Certificate, key crypto.Signer, duration time.Duration) (derBytes, pemBytes []byte) { - selfSignedIssuer := gen.Issuer("test", gen.SetIssuerSelfSigned(v1alpha1.SelfSignedIssuer{})) - template, err := pki.GenerateTemplate(selfSignedIssuer, crt) + template, err := pki.GenerateTemplate(crt) if err != nil { t.Errorf("error generating template: %v", err) } diff --git a/pkg/issuer/selfsigned/issue.go b/pkg/issuer/selfsigned/issue.go index a9e12b0c2..545922620 100644 --- a/pkg/issuer/selfsigned/issue.go +++ b/pkg/issuer/selfsigned/issue.go @@ -57,7 +57,7 @@ func (c *SelfSigned) Issue(ctx context.Context, crt *v1alpha1.Certificate) (*iss } // generate a x509 certificate template for this Certificate - template, err := pki.GenerateTemplate(c.issuer, crt) + template, err := pki.GenerateTemplate(crt) if err != nil { c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) return nil, err diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 3e494a4fe..3a410fa89 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -145,7 +145,7 @@ func GenerateCSR(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x50 // This should create a Certificate template that is equivalent to the CertificateRequest // generated by GenerateCSR. // The PublicKey field must be populated by the caller. -func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) (*x509.Certificate, error) { +func GenerateTemplate(crt *v1alpha1.Certificate) (*x509.Certificate, error) { commonName := CommonNameForCertificate(crt) dnsNames := DNSNamesForCertificate(crt) ipAddresses := IPAddressesForCertificate(crt)