diff --git a/pkg/issuer/ca/issue_test.go b/pkg/issuer/ca/issue_test.go index d1d58beab..bd985c219 100644 --- a/pkg/issuer/ca/issue_test.go +++ b/pkg/issuer/ca/issue_test.go @@ -95,6 +95,11 @@ func noPrivateKeyFieldsSetCheck(expectedCA []byte) func(t *testing.T, s *caFixtu return func(t *testing.T, s *caFixture, args ...interface{}) { resp := args[1].(*issuer.IssueResponse) + if resp == nil { + t.Errorf("no response given, got=%s", resp) + return + } + if len(resp.PrivateKey) > 0 { t.Errorf("expected no new private key to be generated but got: %s", resp.PrivateKey) diff --git a/pkg/issuer/ca/sign.go b/pkg/issuer/ca/sign.go index 1acc0f9e9..61fa1b52e 100644 --- a/pkg/issuer/ca/sign.go +++ b/pkg/issuer/ca/sign.go @@ -36,6 +36,10 @@ func (c *CA) Sign(ctx context.Context, cr *v1alpha1.CertificateRequest) (*issuer if err != nil { log := logf.WithRelatedResourceName(log, c.issuer.GetSpec().CA.SecretName, c.resourceNamespace, "Secret") log.Info("error getting signing CA for Issuer") + + // We're fine to return errors here and later since upon a retry the issuer + // will be marked as 'not ready' - therefore this codepath wont be reached + // again return nil, err } diff --git a/pkg/issuer/ca/sign_test.go b/pkg/issuer/ca/sign_test.go index 06439b6a9..8b5211b46 100644 --- a/pkg/issuer/ca/sign_test.go +++ b/pkg/issuer/ca/sign_test.go @@ -29,6 +29,7 @@ import ( "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1alpha1" testpkg "github.com/jetstack/cert-manager/pkg/controller/test" + "github.com/jetstack/cert-manager/pkg/issuer" "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/test/unit/gen" corev1 "k8s.io/api/core/v1" @@ -108,6 +109,11 @@ func TestSign(t *testing.T) { }, } + rootRSANoCASecret := rootRSACASecret.DeepCopy() + rootRSANoCASecret.Data[corev1.TLSCertKey] = make([]byte, 0) + rootRSANoKeySecret := rootRSACASecret.DeepCopy() + rootRSANoKeySecret.Data[corev1.TLSPrivateKeyKey] = make([]byte, 0) + tests := map[string]caFixture{ "sign a CertificateRequest": { Issuer: gen.Issuer("ca-issuer", @@ -125,6 +131,110 @@ func TestSign(t *testing.T) { CheckFn: noPrivateKeyFieldsSetCheck(rsaPEMCert), Err: false, }, + "fail to find CA tls key pair": { + Issuer: gen.Issuer("ca-issuer", + gen.SetIssuerCA(v1alpha1.CAIssuer{SecretName: "root-ca-secret"}), + ), + CertificateRequest: gen.CertificateRequest("test-cr", + gen.SetCertificateRequestIsCA(true), + gen.SetCertificateRequestCSR(caCSR), + ), + Builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{}, + CertManagerObjects: []runtime.Object{}, + }, + CheckFn: func(t *testing.T, s *caFixture, args ...interface{}) { + err := args[2].(error) + notFoundErr := `secret "root-ca-secret" not found` + if err == nil || err.Error() != notFoundErr { + t.Errorf("unexpected error, exp='%s' got='%+v'", notFoundErr, err) + } + + resp := args[1].(*issuer.IssueResponse) + if resp != nil { + t.Errorf("unexpected response, exp='nil' got='%+v'", resp) + } + }, + Err: true, + }, + "given bad CSR should fail Certificate generation": { + Issuer: gen.Issuer("ca-issuer", + gen.SetIssuerCA(v1alpha1.CAIssuer{SecretName: "root-ca-secret"}), + ), + CertificateRequest: gen.CertificateRequest("test-cr", + gen.SetCertificateRequestIsCA(true), + gen.SetCertificateRequestCSR([]byte("bad-csr")), + ), + Builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rootRSACASecret}, + CertManagerObjects: []runtime.Object{}, + }, + CheckFn: func(t *testing.T, s *caFixture, args ...interface{}) { + err := args[2].(error) + decodeErr := "failed to decode csr from certificate request resource default-unit-test-ns/test-cr" + if err == nil || err.Error() != decodeErr { + t.Errorf("unexpected error, exp='%s' got='%+v'", decodeErr, err) + } + + resp := args[1].(*issuer.IssueResponse) + if resp != nil { + t.Errorf("unexpected response, exp='nil' got='%+v'", resp) + } + }, + Err: true, + }, + "no CA certificate should fail a signing": { + Issuer: gen.Issuer("ca-issuer", + gen.SetIssuerCA(v1alpha1.CAIssuer{SecretName: "root-ca-secret"}), + ), + CertificateRequest: gen.CertificateRequest("test-cr", + gen.SetCertificateRequestIsCA(true), + gen.SetCertificateRequestCSR(caCSR), + ), + Builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rootRSANoCASecret}, + CertManagerObjects: []runtime.Object{}, + }, + CheckFn: func(t *testing.T, s *caFixture, args ...interface{}) { + err := args[2].(error) + noCertError := "error decoding cert PEM block" + if err == nil || err.Error() != noCertError { + t.Errorf("unexpected error, exp='%s' got='%+v'", noCertError, err) + } + + resp := args[1].(*issuer.IssueResponse) + if resp != nil { + t.Errorf("unexpected response, exp='nil' got='%+v'", resp) + } + }, + Err: true, + }, + "no CA key should fail a signing": { + Issuer: gen.Issuer("ca-issuer", + gen.SetIssuerCA(v1alpha1.CAIssuer{SecretName: "root-ca-secret"}), + ), + CertificateRequest: gen.CertificateRequest("test-cr", + gen.SetCertificateRequestIsCA(true), + gen.SetCertificateRequestCSR(caCSR), + ), + Builder: &testpkg.Builder{ + KubeObjects: []runtime.Object{rootRSANoKeySecret}, + CertManagerObjects: []runtime.Object{}, + }, + CheckFn: func(t *testing.T, s *caFixture, args ...interface{}) { + err := args[2].(error) + noKeyError := "error decoding private key PEM block" + if err == nil || err.Error() != noKeyError { + t.Errorf("unexpected error, exp='%s' got='%+v'", noKeyError, err) + } + + resp := args[1].(*issuer.IssueResponse) + if resp != nil { + t.Errorf("unexpected response, exp='nil' got='%+v'", resp) + } + }, + Err: true, + }, } for name, test := range tests { diff --git a/pkg/util/pki/csr.go b/pkg/util/pki/csr.go index c0d1dfb81..08af5e199 100644 --- a/pkg/util/pki/csr.go +++ b/pkg/util/pki/csr.go @@ -23,6 +23,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" + "errors" "fmt" "math/big" "net" @@ -271,7 +272,14 @@ func SignCertificate(template *x509.Certificate, issuerCert *x509.Certificate, p return pemBytes.Bytes(), cert, err } +// SignCSRTemplate signs a certificate template usually based upon a CSR. This +// function expects all fields to be present in the certificate template, +// including it's public key. func SignCSRTemplate(caCerts []*x509.Certificate, caKey crypto.Signer, template *x509.Certificate) (*issuer.IssueResponse, error) { + if len(caCerts) == 0 { + return nil, errors.New("no CA certificates given to sign CSR template") + } + caCert := caCerts[0] certPem, _, err := SignCertificate(template, caCert, template.PublicKey, caKey)