From dc97dde2ef3c1b8695cdf5ecb35d17c60ac19207 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 28 Nov 2018 17:00:51 +0000 Subject: [PATCH] Make Certificate Ready condition behaviour consistent between all issuer types Signed-off-by: James Munnelly --- pkg/controller/certificates/sync.go | 142 +++++++++++++++++----------- pkg/issuer/ca/issue.go | 75 +++++---------- pkg/issuer/selfsigned/BUILD.bazel | 2 + pkg/issuer/selfsigned/issue.go | 90 +++++++----------- pkg/issuer/vault/BUILD.bazel | 1 + pkg/issuer/vault/issue.go | 106 ++++++++++----------- pkg/util/kube/pki.go | 27 ++++++ pkg/util/pki/csr.go | 3 +- 8 files changed, 229 insertions(+), 217 deletions(-) diff --git a/pkg/controller/certificates/sync.go b/pkg/controller/certificates/sync.go index 15e14be26..eaba5e993 100644 --- a/pkg/controller/certificates/sync.go +++ b/pkg/controller/certificates/sync.go @@ -24,7 +24,7 @@ import ( "strings" "time" - api "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -81,45 +81,37 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque } }() - el := validation.ValidateCertificate(crtCopy) - if len(el) > 0 { - msg := fmt.Sprintf("Resource validation failed: %v", el.ToAggregate()) - crtCopy.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorConfig, msg, false) - return + // grab existing certificate and validate private key + cert, err := kube.SecretTLSCert(c.secretLister, crtCopy.Namespace, crtCopy.Spec.SecretName) + // if we don't have a certificate, we need to trigger a re-issue immediately + if err != nil && !(k8sErrors.IsNotFound(err) || errors.IsInvalidData(err)) { + return false, err } - for i, c := range crtCopy.Status.Conditions { - if c.Type == v1alpha1.CertificateConditionReady { - if c.Reason == errorConfig && c.Status == v1alpha1.ConditionFalse { - crtCopy.Status.Conditions = append(crtCopy.Status.Conditions[:i], crtCopy.Status.Conditions[i+1:]...) - break - } - } + // update certificate expiry metric + defer c.metrics.UpdateCertificateExpiry(crtCopy, c.secretLister) + c.setCertificateStatus(crtCopy, cert) + + el := validation.ValidateCertificate(crtCopy) + if len(el) > 0 { + c.Recorder.Eventf(crtCopy, corev1.EventTypeWarning, "BadConfig", "Resource validation failed: %v", el.ToAggregate()) + return false, nil } // step zero: check if the referenced issuer exists and is ready issuerObj, err := c.getGenericIssuer(crtCopy) + if k8sErrors.IsNotFound(err) { + c.Recorder.Eventf(crtCopy, corev1.EventTypeWarning, errorIssuerNotFound, err.Error()) + return false, nil + } if err != nil { - s := fmt.Sprintf("Issuer %s does not exist", err.Error()) - glog.Info(s) - c.Recorder.Event(crtCopy, api.EventTypeWarning, errorIssuerNotFound, s) return false, err } el = validation.ValidateCertificateForIssuer(crtCopy, issuerObj) if len(el) > 0 { - msg := fmt.Sprintf("Resource validation failed: %v", el.ToAggregate()) - crtCopy.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorConfig, msg, false) - return - } - - for i, c := range crtCopy.Status.Conditions { - if c.Type == v1alpha1.CertificateConditionReady { - if c.Reason == errorConfig && c.Status == v1alpha1.ConditionFalse { - crtCopy.Status.Conditions = append(crtCopy.Status.Conditions[:i], crtCopy.Status.Conditions[i+1:]...) - break - } - } + c.Recorder.Eventf(crtCopy, corev1.EventTypeWarning, "BadConfig", "Resource validation failed: %v", el.ToAggregate()) + return false, nil } issuerReady := issuerObj.HasCondition(v1alpha1.IssuerCondition{ @@ -127,44 +119,30 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque Status: v1alpha1.ConditionTrue, }) if !issuerReady { - s := fmt.Sprintf("Issuer %s not ready", issuerObj.GetObjectMeta().Name) - glog.Info(s) - c.Recorder.Event(crtCopy, api.EventTypeWarning, errorIssuerNotReady, s) - return false, fmt.Errorf(s) + c.Recorder.Eventf(crtCopy, corev1.EventTypeWarning, errorIssuerNotReady, "Issuer %s not ready", issuerObj.GetObjectMeta().Name) + return false, nil } i, err := c.IssuerFactory().IssuerFor(issuerObj) if err != nil { - s := "Error initializing issuer: " + err.Error() - glog.Info(s) - c.Recorder.Event(crtCopy, api.EventTypeWarning, errorIssuerInit, s) - return false, err + c.Recorder.Eventf(crtCopy, corev1.EventTypeWarning, errorIssuerInit, "Internal error initialising issuer: %v", err) + return false, nil } key, err := kube.SecretTLSKey(c.secretLister, crtCopy.Namespace, crtCopy.Spec.SecretName) // if we don't have a private key, we need to trigger a re-issue immediately if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { + glog.V(4).Infof("Invoking issue function due to certificate private not found or invalid") return c.issue(ctx, i, crtCopy) } if err != nil { return false, err } - // grab existing certificate and validate private key - cert, err := kube.SecretTLSCert(c.secretLister, crtCopy.Namespace, crtCopy.Spec.SecretName) - // if we don't have a certificate, we need to trigger a re-issue immediately - if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { + if cert == nil { + glog.V(4).Infof("Invoking issue function as existing certificate does not exist") return c.issue(ctx, i, crtCopy) } - if err != nil { - return false, err - } - - metaNotAfter := metav1.NewTime(cert.NotAfter) - crtCopy.Status.NotAfter = &metaNotAfter - - // update certificate expiry metric - defer c.metrics.UpdateCertificateExpiry(crt, c.secretLister) // begin checking if the TLS certificate is valid/needs a re-issue or renew @@ -174,24 +152,28 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque return false, err } if !matches { + glog.V(4).Infof("Invoking issue function due to certificate private key not matching certificate") return c.issue(ctx, i, crtCopy) } // validate the common name is correct expectedCN := pki.CommonNameForCertificate(crtCopy) if expectedCN != cert.Subject.CommonName { + glog.V(4).Infof("Invoking issue function due to certificate common name not matching spec") return c.issue(ctx, i, crtCopy) } // validate the dns names are correct expectedDNSNames := pki.DNSNamesForCertificate(crtCopy) if !util.EqualUnsorted(cert.DNSNames, expectedDNSNames) { + glog.V(4).Infof("Invoking issue function due to certificate dns names not matching spec") return c.issue(ctx, i, crtCopy) } // check if the certificate needs renewal needsRenew := c.Context.IssuerOptions.CertificateNeedsRenew(cert, crt.Spec.RenewBefore) if needsRenew { + glog.V(4).Infof("Invoking issue function due to certificate needing renewal") return c.issue(ctx, i, crtCopy) } @@ -208,6 +190,54 @@ func (c *Controller) Sync(ctx context.Context, crt *v1alpha1.Certificate) (reque return false, nil } +// setCertificateStatus will update the status subresource of the certificate. +// It will not actually submit the resource to the apiserver. +func (c *Controller) setCertificateStatus(crt *v1alpha1.Certificate, cert *x509.Certificate) { + if cert == nil { + crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, "NotFound", "Certificate does not exist", false) + return + } + + metaNotAfter := metav1.NewTime(cert.NotAfter) + crt.Status.NotAfter = &metaNotAfter + + // Derive & set 'Ready' condition on Certificate resource + matches, matchErrs := c.certificateMatchesSpec(crt, cert) + reason := "Ready" + if cert.NotAfter.Before(now()) { + reason = "Expired" + matchErrs = append(matchErrs, fmt.Sprintf("Certificate has expired")) + } + if !matches { + reason = "DoesNotMatch" + } + if len(matchErrs) > 0 { + crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, reason, strings.Join(matchErrs, ", "), false) + return + } + + crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, reason, "Certificate is up to date and has not expired", false) + + return +} + +func (c *Controller) certificateMatchesSpec(crt *v1alpha1.Certificate, cert *x509.Certificate) (bool, []string) { + var errs []string + // validate the common name is correct + expectedCN := pki.CommonNameForCertificate(crt) + if expectedCN != cert.Subject.CommonName { + errs = append(errs, fmt.Sprintf("Common name on TLS certificate not up to date: %q", cert.Subject.CommonName)) + } + + // validate the dns names are correct + expectedDNSNames := pki.DNSNamesForCertificate(crt) + if !util.EqualUnsorted(cert.DNSNames, expectedDNSNames) { + errs = append(errs, fmt.Sprintf("DNS names on TLS certificate not up to date: %q", cert.DNSNames)) + } + + return len(errs) == 0, errs +} + // TODO: replace with a call to controllerpkg.Helper.GetGenericIssuer func (c *Controller) getGenericIssuer(crt *v1alpha1.Certificate) (v1alpha1.GenericIssuer, error) { switch crt.Spec.IssuerRef.Kind { @@ -267,23 +297,23 @@ func ownerRef(crt *v1alpha1.Certificate) metav1.OwnerReference { } } -func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, cert, key, ca []byte) (*api.Secret, error) { +func (c *Controller) updateSecret(crt *v1alpha1.Certificate, namespace string, cert, key, ca []byte) (*corev1.Secret, error) { secret, err := c.Client.CoreV1().Secrets(namespace).Get(crt.Spec.SecretName, metav1.GetOptions{}) if err != nil && !k8sErrors.IsNotFound(err) { return nil, err } if k8sErrors.IsNotFound(err) { - secret = &api.Secret{ + secret = &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: crt.Spec.SecretName, Namespace: namespace, }, - Type: api.SecretTypeTLS, + Type: corev1.SecretTypeTLS, Data: map[string][]byte{}, } } - secret.Data[api.TLSCertKey] = cert - secret.Data[api.TLSPrivateKeyKey] = key + secret.Data[corev1.TLSCertKey] = cert + secret.Data[corev1.TLSPrivateKeyKey] = key if ca != nil { secret.Data[TLSCAKey] = ca @@ -343,14 +373,14 @@ func (c *Controller) issue(ctx context.Context, issuer issuer.Interface, crt *v1 if _, err := c.updateSecret(crt, crt.Namespace, resp.Certificate, resp.PrivateKey, resp.CA); err != nil { s := messageErrorSavingCertificate + err.Error() glog.Info(s) - c.Recorder.Event(crt, api.EventTypeWarning, errorSavingCertificate, s) + c.Recorder.Event(crt, corev1.EventTypeWarning, errorSavingCertificate, s) return false, err } if len(resp.Certificate) > 0 { s := messageCertificateIssued glog.Info(s) - c.Recorder.Event(crt, api.EventTypeNormal, successCertificateIssued, s) + c.Recorder.Event(crt, corev1.EventTypeNormal, successCertificateIssued, s) crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertificateIssued, s, true) // as we have just written a certificate, we should schedule it for renewal diff --git a/pkg/issuer/ca/issue.go b/pkg/issuer/ca/issue.go index dc45f71db..c83e6ce09 100644 --- a/pkg/issuer/ca/issue.go +++ b/pkg/issuer/ca/issue.go @@ -18,9 +18,9 @@ package ca import ( "context" - "crypto/x509" - "fmt" + "github.com/golang/glog" + corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" @@ -50,54 +50,57 @@ func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Issue // if one does not already exist, generate a new one signeeKey, err = pki.GeneratePrivateKeyForCertificate(crt) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorPrivateKey, fmt.Sprintf("Error generating private key for certificate: %v", err), false) - return issuer.IssueResponse{}, err + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "PrivateKeyError", "Error generating certificate private key: %v", err) + // don't trigger a retry. An error from this function implies some + // invalid input parameters, and retrying without updating the + // resource will not help. + return issuer.IssueResponse{}, nil } } if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorPrivateKey, fmt.Sprintf("Error getting private key for certificate: %v", err), false) + glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) return issuer.IssueResponse{}, err } // extract the public component of the key - publicKey, err := pki.PublicKeyForPrivateKey(signeeKey) + signeePublicKey, err := pki.PublicKeyForPrivateKey(signeeKey) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorPrivateKey, fmt.Sprintf("Error getting public key from private key: %v", err), false) + glog.Errorf("Error getting public key from private key: %v", err) return issuer.IssueResponse{}, err } - // get a copy of the *CA* certificate named on the Issuer - caCert, err := kube.SecretTLSCert(c.secretsLister, c.resourceNamespace, c.issuer.GetSpec().CA.SecretName) + // get a copy of the CA certificate named on the Issuer + caCert, caKey, err := kube.SecretTLSKeyPair(c.secretsLister, c.resourceNamespace, c.issuer.GetSpec().CA.SecretName) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorCA, fmt.Sprintf("Error getting signing CA: %v", err), false) + glog.Errorf("Error getting signing CA for Issuer: %v", err) return issuer.IssueResponse{}, err } - // actually sign the certificate - certPem, err := c.obtainCertificate(crt, publicKey, caCert) + // generate a x509 certificate template for this Certificate + template, err := pki.GenerateTemplate(c.issuer, crt) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorSigning, fmt.Sprintf("Error signing certificate: %v", err), false) + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) + return issuer.IssueResponse{}, err + } + + // sign and encode the certificate + certPem, _, err := pki.SignCertificate(template, caCert, signeePublicKey, caKey) + if err != nil { + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) return issuer.IssueResponse{}, err } // Encode output private key and CA cert ready for return keyPem, err := pki.EncodePrivateKey(signeeKey) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorPrivateKey, fmt.Sprintf("Error encoding certificate private key: %v", err), false) + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) return issuer.IssueResponse{}, err } // encode the CA certificate to be bundled in the output caPem, err := pki.EncodeX509(caCert) if err != nil { - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, - reasonErrorSigning, fmt.Sprintf("Error encoding certificate: %v", err), false) + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error encoding certificate: %v", err) return issuer.IssueResponse{}, err } @@ -107,31 +110,3 @@ func (c *CA) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Issue CA: caPem, }, nil } - -func (c *CA) obtainCertificate(crt *v1alpha1.Certificate, signeeKey interface{}, signerCert *x509.Certificate) ([]byte, error) { - commonName := crt.Spec.CommonName - altNames := crt.Spec.DNSNames - if len(commonName) == 0 && len(altNames) == 0 { - return nil, fmt.Errorf("no domains specified on certificate") - } - - // get a copy of the CAs private key - signerKey, err := kube.SecretTLSKey(c.secretsLister, c.resourceNamespace, c.issuer.GetSpec().CA.SecretName) - if err != nil { - return nil, fmt.Errorf("error getting issuer private key: %s", err.Error()) - } - - // generate a x509 certificate template for this Certificate - template, err := pki.GenerateTemplate(c.issuer, crt) - if err != nil { - return nil, err - } - - // sign and encode the certificate - crtPem, _, err := pki.SignCertificate(template, signerCert, signeeKey, signerKey) - if err != nil { - return nil, err - } - - return crtPem, nil -} diff --git a/pkg/issuer/selfsigned/BUILD.bazel b/pkg/issuer/selfsigned/BUILD.bazel index b4aed86ca..ffeca283e 100644 --- a/pkg/issuer/selfsigned/BUILD.bazel +++ b/pkg/issuer/selfsigned/BUILD.bazel @@ -16,6 +16,8 @@ go_library( "//pkg/util/errors:go_default_library", "//pkg/util/kube:go_default_library", "//pkg/util/pki:go_default_library", + "//vendor/github.com/golang/glog:go_default_library", + "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], diff --git a/pkg/issuer/selfsigned/issue.go b/pkg/issuer/selfsigned/issue.go index 2ef2d7236..c07ea1794 100644 --- a/pkg/issuer/selfsigned/issue.go +++ b/pkg/issuer/selfsigned/issue.go @@ -18,9 +18,9 @@ package selfsigned import ( "context" - "crypto" - "time" + "github.com/golang/glog" + corev1 "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" @@ -30,76 +30,56 @@ import ( "github.com/jetstack/cert-manager/pkg/util/pki" ) -const ( - errorGetCertKeyPair = "ErrGetKey" - errorIssueCert = "ErrIssueCert" - errorEncodePrivateKey = "ErrEncodePrivateKey" - - successCertIssued = "CertIssueSuccess" - - messageErrorGetCertKeyPair = "Error getting keypair for certificate: " - messageErrorIssueCert = "Error issuing TLS certificate: " - messageErrorEncodePrivateKey = "Error encoding private key: " - - messageCertIssued = "Certificate issued successfully" -) - -const ( - // certificateDuration of 1 year - certificateDuration = time.Hour * 24 * 365 - defaultOrganization = "cert-manager" -) - func (c *SelfSigned) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { - signeeKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName) + // get a copy of the existing/currently issued Certificate's private key + signeePrivateKey, err := kube.SecretTLSKey(c.secretsLister, crt.Namespace, crt.Spec.SecretName) if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { - signeeKey, err = pki.GeneratePrivateKeyForCertificate(crt) + // if one does not already exist, generate a new one + signeePrivateKey, err = pki.GeneratePrivateKeyForCertificate(crt) + if err != nil { + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "PrivateKeyError", "Error generating certificate private key: %v", err) + // don't trigger a retry. An error from this function implies some + // invalid input parameters, and retrying without updating the + // resource will not help. + return issuer.IssueResponse{}, nil + } } - if err != nil { - s := messageErrorGetCertKeyPair + err.Error() - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorGetCertKeyPair, s, false) + glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) return issuer.IssueResponse{}, err } - certPem, err := c.obtainCertificate(crt, signeeKey) + // extract the public component of the key + signeePublicKey, err := pki.PublicKeyForPrivateKey(signeePrivateKey) if err != nil { - s := messageErrorIssueCert + err.Error() - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s, false) + glog.Errorf("Error getting public key from private key: %v", err) return issuer.IssueResponse{}, err } - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued, true) - - keyPem, err := pki.EncodePrivateKey(signeeKey) + // generate a x509 certificate template for this Certificate + template, err := pki.GenerateTemplate(c.issuer, crt) if err != nil { - s := messageErrorEncodePrivateKey + err.Error() - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorEncodePrivateKey, s, false) + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) + return issuer.IssueResponse{}, err + } + + // sign and encode the certificate + certPem, _, err := pki.SignCertificate(template, template, signeePublicKey, signeePrivateKey) + if err != nil { + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Error signing certificate: %v", err) + return issuer.IssueResponse{}, err + } + + // Encode output private key + keyPem, err := pki.EncodePrivateKey(signeePrivateKey) + if err != nil { + c.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) return issuer.IssueResponse{}, err } return issuer.IssueResponse{ - Certificate: certPem, PrivateKey: keyPem, + Certificate: certPem, CA: certPem, }, nil } - -func (c *SelfSigned) obtainCertificate(crt *v1alpha1.Certificate, privateKey crypto.PrivateKey) ([]byte, error) { - publicKey, err := pki.PublicKeyForPrivateKey(privateKey) - if err != nil { - return nil, err - } - - template, err := pki.GenerateTemplate(c.issuer, crt) - if err != nil { - return nil, err - } - - crtPem, _, err := pki.SignCertificate(template, template, publicKey, privateKey) - if err != nil { - return nil, err - } - - return crtPem, nil -} diff --git a/pkg/issuer/vault/BUILD.bazel b/pkg/issuer/vault/BUILD.bazel index 3922554b1..4cc1c033e 100644 --- a/pkg/issuer/vault/BUILD.bazel +++ b/pkg/issuer/vault/BUILD.bazel @@ -19,6 +19,7 @@ go_library( "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/hashicorp/vault/api:go_default_library", "//vendor/github.com/hashicorp/vault/helper/certutil:go_default_library", + "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/client-go/listers/core/v1:go_default_library", ], diff --git a/pkg/issuer/vault/issue.go b/pkg/issuer/vault/issue.go index df41536ff..39f70a4ff 100644 --- a/pkg/issuer/vault/issue.go +++ b/pkg/issuer/vault/issue.go @@ -30,12 +30,14 @@ import ( "github.com/golang/glog" vault "github.com/hashicorp/vault/api" "github.com/hashicorp/vault/helper/certutil" + corev1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" + "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" "github.com/jetstack/cert-manager/pkg/issuer" "github.com/jetstack/cert-manager/pkg/util/errors" "github.com/jetstack/cert-manager/pkg/util/kube" "github.com/jetstack/cert-manager/pkg/util/pki" - k8sErrors "k8s.io/apimachinery/pkg/api/errors" ) const ( @@ -50,14 +52,59 @@ const ( ) func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.IssueResponse, error) { - key, certPem, caPem, err := v.obtainCertificate(ctx, crt) + // get a copy of the existing/currently issued Certificate's private key + signeePrivateKey, err := kube.SecretTLSKey(v.secretsLister, crt.Namespace, crt.Spec.SecretName) + if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { + // if one does not already exist, generate a new one + signeePrivateKey, err = pki.GeneratePrivateKeyForCertificate(crt) + if err != nil { + v.Recorder.Eventf(crt, corev1.EventTypeWarning, "PrivateKeyError", "Error generating certificate private key: %v", err) + // don't trigger a retry. An error from this function implies some + // invalid input parameters, and retrying without updating the + // resource will not help. + return issuer.IssueResponse{}, nil + } + } if err != nil { - s := messageErrorIssueCert + err.Error() - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionFalse, errorIssueCert, s, false) + glog.Errorf("Error getting private key %q for certificate: %v", crt.Spec.SecretName, err) return issuer.IssueResponse{}, err } - crt.UpdateStatusCondition(v1alpha1.CertificateConditionReady, v1alpha1.ConditionTrue, successCertIssued, messageCertIssued, true) + /// BEGIN building CSR + // TODO: we should probably surface some of these errors to users + template, err := pki.GenerateCSR(v.issuer, crt) + if err != nil { + return issuer.IssueResponse{}, err + } + derBytes, err := pki.EncodeCSR(template, signeePrivateKey) + if err != nil { + return issuer.IssueResponse{}, err + } + pemRequestBuf := &bytes.Buffer{} + err = pem.Encode(pemRequestBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: derBytes}) + if err != nil { + return issuer.IssueResponse{}, fmt.Errorf("error encoding certificate request: %s", err.Error()) + } + /// END building CSR + + /// BEGIN requesting certificate + certDuration := v1alpha1.DefaultCertificateDuration + if crt.Spec.Duration != nil { + certDuration = crt.Spec.Duration.Duration + } + + certPem, caPem, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, pemRequestBuf.Bytes()) + if err != nil { + v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorSigning", "Failed to request certificate: %v", err) + return issuer.IssueResponse{}, err + } + /// END requesting certificate + + key, err := pki.EncodePrivateKey(signeePrivateKey) + if err != nil { + v.Recorder.Eventf(crt, corev1.EventTypeWarning, "ErrorPrivateKey", "Error encoding private key: %v", err) + return issuer.IssueResponse{}, err + } return issuer.IssueResponse{ PrivateKey: key, @@ -66,55 +113,6 @@ func (v *Vault) Issue(ctx context.Context, crt *v1alpha1.Certificate) (issuer.Is }, nil } -func (v *Vault) obtainCertificate(ctx context.Context, crt *v1alpha1.Certificate) ([]byte, []byte, []byte, error) { - // get existing certificate private key - signeeKey, err := kube.SecretTLSKey(v.secretsLister, crt.Namespace, crt.Spec.SecretName) - if k8sErrors.IsNotFound(err) || errors.IsInvalidData(err) { - signeeKey, err = pki.GeneratePrivateKeyForCertificate(crt) - if err != nil { - return nil, nil, nil, fmt.Errorf("error generating private key: %s", err.Error()) - } - } - - if err != nil { - return nil, nil, nil, fmt.Errorf("error getting certificate private key: %s", err.Error()) - } - - template, err := pki.GenerateCSR(v.issuer, crt) - if err != nil { - return nil, nil, nil, err - } - - derBytes, err := pki.EncodeCSR(template, signeeKey) - if err != nil { - return nil, nil, nil, err - } - - pemRequestBuf := &bytes.Buffer{} - err = pem.Encode(pemRequestBuf, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: derBytes}) - if err != nil { - return nil, nil, nil, fmt.Errorf("error encoding certificate request: %s", err.Error()) - } - - certDuration := v1alpha1.DefaultCertificateDuration - if crt.Spec.Duration != nil { - certDuration = crt.Spec.Duration.Duration - } - - crtBytes, caBytes, err := v.requestVaultCert(template.Subject.CommonName, certDuration, template.DNSNames, pemRequestBuf.Bytes()) - - if err != nil { - return nil, nil, nil, err - } - - keyBytes, err := pki.EncodePrivateKey(signeeKey) - if err != nil { - return nil, nil, nil, err - } - - return keyBytes, crtBytes, caBytes, nil -} - func (v *Vault) configureCertPool(cfg *vault.Config) error { certs := v.issuer.GetSpec().Vault.CABundle if len(certs) == 0 { diff --git a/pkg/util/kube/pki.go b/pkg/util/kube/pki.go index 5899f6034..2c0810f9d 100644 --- a/pkg/util/kube/pki.go +++ b/pkg/util/kube/pki.go @@ -73,3 +73,30 @@ func SecretTLSCert(secretLister corelisters.SecretLister, namespace, name string return cert, nil } + +func SecretTLSKeyPair(secretLister corelisters.SecretLister, namespace, name string) (*x509.Certificate, crypto.Signer, error) { + secret, err := secretLister.Secrets(namespace).Get(name) + if err != nil { + return nil, nil, err + } + + certBytes, ok := secret.Data[api.TLSCertKey] + if !ok { + return nil, nil, fmt.Errorf("no certificate data for %q in secret '%s/%s'", api.TLSCertKey, namespace, name) + } + cert, err := pki.DecodeX509CertificateBytes(certBytes) + if err != nil { + return nil, nil, errors.NewInvalidData(err.Error()) + } + + keyBytes, ok := secret.Data[api.TLSPrivateKeyKey] + if !ok { + return nil, nil, fmt.Errorf("no private key data for %q in secret '%s/%s'", api.TLSCertKey, namespace, name) + } + key, err := pki.DecodePrivateKeyBytes(keyBytes) + if err != nil { + return nil, key, errors.NewInvalidData(err.Error()) + } + + return cert, key, nil +} diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index 063a15e4e..a2f2bec7c 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -142,7 +142,6 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) } pubKeyAlgo, _, err := SignatureAlgorithm(crt) - if err != nil { return nil, err } @@ -176,7 +175,7 @@ func GenerateTemplate(issuer v1alpha1.GenericIssuer, crt *v1alpha1.Certificate) // key of the signer. // It returns a PEM encoded copy of the Certificate as well as a *x509.Certificate // which can be used for reading the encoded values. -func SignCertificate(template *x509.Certificate, issuerCert *x509.Certificate, publicKey interface{}, signerKey interface{}) ([]byte, *x509.Certificate, error) { +func SignCertificate(template *x509.Certificate, issuerCert *x509.Certificate, publicKey crypto.PublicKey, signerKey interface{}) ([]byte, *x509.Certificate, error) { derBytes, err := x509.CreateCertificate(rand.Reader, template, issuerCert, publicKey, signerKey) if err != nil {