diff --git a/internal/apis/certmanager/validation/certificaterequest_test.go b/internal/apis/certmanager/validation/certificaterequest_test.go index 79829b0de..b7ef87c20 100644 --- a/internal/apis/certmanager/validation/certificaterequest_test.go +++ b/internal/apis/certmanager/validation/certificaterequest_test.go @@ -17,11 +17,9 @@ limitations under the License. package validation import ( - "bytes" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" - "encoding/pem" "reflect" "testing" @@ -32,7 +30,6 @@ import ( cminternal "github.com/cert-manager/cert-manager/internal/apis/certmanager" cminternalmeta "github.com/cert-manager/cert-manager/internal/apis/meta" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" - "github.com/cert-manager/cert-manager/pkg/util/pki" utilpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/cert-manager/test/unit/gen" ) @@ -572,10 +569,12 @@ func TestValidateCertificateRequest(t *testing.T) { cr: &cminternal.CertificateRequest{ Spec: cminternal.CertificateRequestSpec{ // mustGenerateCSR will set the default usages for us - Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) { + Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) error { // manually remove extensions that encode default usages cr.Extensions = nil cr.ExtraExtensions = nil + + return nil }), IssuerRef: validIssuerRef, Usages: []cminternal.KeyUsage{cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature}, @@ -588,12 +587,12 @@ func TestValidateCertificateRequest(t *testing.T) { cr: &cminternal.CertificateRequest{ Spec: cminternal.CertificateRequestSpec{ // mustGenerateCSR will set the default usages for us - Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) { + Request: mustGenerateCSR(t, gen.Certificate("test", gen.SetCertificateDNSNames("example.com")), func(cr *x509.CertificateRequest) error { // manually remove extensions that encode default usages cr.Extensions = nil cr.ExtraExtensions = []pkix.Extension{ { - Id: pki.OIDExtensionKeyUsage, + Id: utilpki.OIDExtensionKeyUsage, Critical: false, Value: func(t *testing.T) []byte { asn1KeyUsage, err := asn1.Marshal(asn1.BitString{Bytes: []byte{}, BitLength: 0}) @@ -605,6 +604,8 @@ func TestValidateCertificateRequest(t *testing.T) { }(t), }, } + + return nil }), IssuerRef: validIssuerRef, Usages: []cminternal.KeyUsage{cminternal.UsageKeyEncipherment, cminternal.UsageDigitalSignature}, @@ -877,30 +878,10 @@ func TestValidateCertificateRequest(t *testing.T) { } } -func mustGenerateCSR(t *testing.T, crt *cmapi.Certificate, modifiers ...func(*x509.CertificateRequest)) []byte { - // Create a new private key - pk, err := utilpki.GenerateRSAPrivateKey(2048) +func mustGenerateCSR(t *testing.T, crt *cmapi.Certificate, modifiers ...gen.CSRModifier) []byte { + csrPEM, _, err := gen.CSRForCertificate(crt, modifiers...) if err != nil { t.Fatal(err) } - - x509CSR, err := utilpki.GenerateCSR(crt) - if err != nil { - t.Fatal(err) - } - for _, modifier := range modifiers { - modifier(x509CSR) - } - csrDER, err := utilpki.EncodeCSR(x509CSR, pk) - if err != nil { - t.Fatal(err) - } - - csrPEM := bytes.NewBuffer([]byte{}) - err = pem.Encode(csrPEM, &pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrDER}) - if err != nil { - t.Fatal(err) - } - - return csrPEM.Bytes() + return csrPEM } diff --git a/pkg/controller/certificates/requestmanager/util_test.go b/pkg/controller/certificates/requestmanager/util_test.go index 2c2fdfadd..475e4703b 100644 --- a/pkg/controller/certificates/requestmanager/util_test.go +++ b/pkg/controller/certificates/requestmanager/util_test.go @@ -19,7 +19,6 @@ package requestmanager import ( "crypto" "crypto/x509" - "encoding/pem" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -77,7 +76,7 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error) return nil, err } - privateKey, err := pki.GeneratePrivateKeyForCertificate(crt) + csrPEM, privateKey, err := gen.CSRForCertificate(crt) if err != nil { return nil, err } @@ -87,11 +86,6 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error) return nil, err } - csrPEM, err := generateCSRImpl(crt, privateKeyBytes) - if err != nil { - return nil, err - } - csr, err := pki.DecodeX509CertificateRequestBytes(csrPEM) if err != nil { return nil, err @@ -173,26 +167,3 @@ func createCryptoBundle(originalCert *cmapi.Certificate) (*cryptoBundle, error) certBytes: certBytes, }, nil } - -func generateCSRImpl(crt *cmapi.Certificate, pk []byte) ([]byte, error) { - csr, err := pki.GenerateCSR(crt) - if err != nil { - return nil, err - } - - signer, err := pki.DecodePrivateKeyBytes(pk) - if err != nil { - return nil, err - } - - csrDER, err := pki.EncodeCSR(csr, signer) - if err != nil { - return nil, err - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - - return csrPEM, nil -} diff --git a/pkg/util/pki/match_test.go b/pkg/util/pki/match_test.go index ea8e11bcf..119bb8408 100644 --- a/pkg/util/pki/match_test.go +++ b/pkg/util/pki/match_test.go @@ -14,14 +14,12 @@ See the License for the specific language governing permissions and limitations under the License. */ -package pki +package pki_test import ( - "bytes" "crypto" "crypto/x509" "encoding/asn1" - "encoding/pem" "reflect" "testing" @@ -29,10 +27,12 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" + "github.com/cert-manager/cert-manager/pkg/util/pki" + "github.com/cert-manager/cert-manager/test/unit/gen" ) func mustGenerateRSA(t *testing.T, keySize int) crypto.PrivateKey { - pk, err := GenerateRSAPrivateKey(keySize) + pk, err := pki.GenerateRSAPrivateKey(keySize) if err != nil { t.Fatal(err) } @@ -40,7 +40,7 @@ func mustGenerateRSA(t *testing.T, keySize int) crypto.PrivateKey { } func mustGenerateECDSA(t *testing.T, keySize int) crypto.PrivateKey { - pk, err := GenerateECPrivateKey(keySize) + pk, err := pki.GenerateECPrivateKey(keySize) if err != nil { t.Fatal(err) } @@ -48,7 +48,7 @@ func mustGenerateECDSA(t *testing.T, keySize int) crypto.PrivateKey { } func mustGenerateEd25519(t *testing.T) crypto.PrivateKey { - pk, err := GenerateEd25519PrivateKey() + pk, err := pki.GenerateEd25519PrivateKey() if err != nil { t.Fatal(err) } @@ -75,18 +75,18 @@ func TestPrivateKeyMatchesSpec(t *testing.T) { violations: []string{"spec.privateKey.size"}, }, "should match if keySize and algorithm are correct (ECDSA)": { - key: mustGenerateECDSA(t, ECCurve256), + key: mustGenerateECDSA(t, pki.ECCurve256), expectedAlgo: cmapi.ECDSAKeyAlgorithm, expectedSize: 256, }, "should not match if ECDSA keySize is incorrect": { - key: mustGenerateECDSA(t, ECCurve256), + key: mustGenerateECDSA(t, pki.ECCurve256), expectedAlgo: cmapi.ECDSAKeyAlgorithm, - expectedSize: ECCurve521, + expectedSize: pki.ECCurve521, violations: []string{"spec.privateKey.size"}, }, "should not match if keyAlgorithm is incorrect": { - key: mustGenerateECDSA(t, ECCurve256), + key: mustGenerateECDSA(t, pki.ECCurve256), expectedAlgo: cmapi.RSAKeyAlgorithm, expectedSize: 2048, violations: []string{"spec.privateKey.algorithm"}, @@ -98,7 +98,7 @@ func TestPrivateKeyMatchesSpec(t *testing.T) { } for name, test := range tests { t.Run(name, func(t *testing.T) { - violations, err := PrivateKeyMatchesSpec( + violations, err := pki.PrivateKeyMatchesSpec( test.key, cmapi.CertificateSpec{ PrivateKey: &cmapi.CertificatePrivateKey{ @@ -132,7 +132,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { violations []string }{ "should not report any violation if Certificate otherName(s) match the CertificateRequest's": { - crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{ + crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ { @@ -140,7 +140,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { UTF8Value: "upn@testdomain.local", }, }, - }}, t), + }}), certSpec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ @@ -153,7 +153,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { err: "", }, "should report violation if Certificate otherName(s) mismatch the CertificateRequest's": { - crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{ + crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ { @@ -161,7 +161,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { UTF8Value: "upn@testdomain.local", }, }, - }}, t), + }}), certSpec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ @@ -177,7 +177,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { }, }, "should not report violation if Certificate otherName(s) match the CertificateRequest's (with different order)": { - crSpec: MustBuildCertificateRequest(&cmapi.Certificate{Spec: cmapi.CertificateSpec{ + crSpec: mustBuildCertificateRequest(t, &cmapi.Certificate{Spec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ { @@ -189,7 +189,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { UTF8Value: "upn@testdomain.local", }, }, - }}, t), + }}), certSpec: cmapi.CertificateSpec{ CommonName: "cn", OtherNames: []cmapi.OtherName{ @@ -208,7 +208,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { } for name, test := range tests { t.Run(name, func(t *testing.T) { - violations, err := RequestMatchesSpec(test.crSpec, test.certSpec) + violations, err := pki.RequestMatchesSpec(test.crSpec, test.certSpec) if err != nil { if test.err == "" { t.Errorf("Unexpected error: %s", err.Error()) @@ -226,12 +226,7 @@ func TestCertificateRequestOtherNamesMatchSpec(t *testing.T) { func TestRequestMatchesSpecSubject(t *testing.T) { createCSRBlob := func(literalSubject string) []byte { - pk, err := GenerateRSAPrivateKey(2048) - if err != nil { - t.Fatal(err) - } - - seq, err := UnmarshalSubjectStringToRDNSequence(literalSubject) + seq, err := pki.UnmarshalSubjectStringToRDNSequence(literalSubject) if err != nil { t.Fatal(err) } @@ -241,16 +236,15 @@ func TestRequestMatchesSpecSubject(t *testing.T) { t.Fatal(err) } - csr := &x509.CertificateRequest{ - RawSubject: asn1Seq, - } - - csrBytes, err := x509.CreateCertificateRequest(bytes.NewBuffer(nil), csr, pk) + pemBytes, _, err := gen.CSR(x509.Ed25519, func(cr *x509.CertificateRequest) error { + cr.RawSubject = asn1Seq + return nil + }) if err != nil { t.Fatal(err) } - return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csrBytes}) + return pemBytes } tests := []struct { @@ -282,7 +276,7 @@ func TestRequestMatchesSpecSubject(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - violations, err := RequestMatchesSpec( + violations, err := pki.RequestMatchesSpec( &cmapi.CertificateRequest{ Spec: cmapi.CertificateRequestSpec{ Request: test.x509CSR, @@ -442,7 +436,7 @@ func TestSecretDataAltNamesMatchSpec(t *testing.T) { } for name, test := range tests { t.Run(name, func(t *testing.T) { - violations, err := SecretDataAltNamesMatchSpec(&corev1.Secret{Data: map[string][]byte{corev1.TLSCertKey: test.data}}, test.spec) + violations, err := pki.SecretDataAltNamesMatchSpec(&corev1.Secret{Data: map[string][]byte{corev1.TLSCertKey: test.data}}, test.spec) switch { case err != nil: if test.err != err.Error() { @@ -461,17 +455,17 @@ func TestSecretDataAltNamesMatchSpec(t *testing.T) { } func selfSignCertificate(t *testing.T, spec cmapi.CertificateSpec) []byte { - pk, err := GenerateRSAPrivateKey(2048) + pk, err := pki.GenerateRSAPrivateKey(2048) if err != nil { t.Fatal(err) } - template, err := CertificateTemplateFromCertificate(&cmapi.Certificate{Spec: spec}) + template, err := pki.CertificateTemplateFromCertificate(&cmapi.Certificate{Spec: spec}) if err != nil { t.Fatal(err) } - pemData, _, err := SignCertificate(template, template, pk.Public(), pk) + pemData, _, err := pki.SignCertificate(template, template, pk.Public(), pk) if err != nil { t.Fatal(err) } @@ -479,23 +473,12 @@ func selfSignCertificate(t *testing.T, spec cmapi.CertificateSpec) []byte { return pemData } -func MustBuildCertificateRequest(crt *cmapi.Certificate, t *testing.T) *cmapi.CertificateRequest { - pk, err := GenerateRSAPrivateKey(2048) +func mustBuildCertificateRequest(t *testing.T, crt *cmapi.Certificate) *cmapi.CertificateRequest { + pemData, _, err := gen.CSRForCertificate(crt) if err != nil { t.Fatal(err) } - csrTemplate, err := GenerateCSR(crt, WithOtherNames(true)) - if err != nil { - t.Fatal(err) - } - - var buffer bytes.Buffer - csr, err := x509.CreateCertificateRequest(&buffer, csrTemplate, pk) - if err != nil { - t.Fatal(err) - } - pemData := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: csr}) cr := &cmapi.CertificateRequest{ ObjectMeta: metav1.ObjectMeta{ Name: t.Name(), diff --git a/test/e2e/suite/issuers/acme/certificaterequest/dns01.go b/test/e2e/suite/issuers/acme/certificaterequest/dns01.go index 363b7b761..961e2099c 100644 --- a/test/e2e/suite/issuers/acme/certificaterequest/dns01.go +++ b/test/e2e/suite/issuers/acme/certificaterequest/dns01.go @@ -116,7 +116,7 @@ func testRFC2136DNSProvider() bool { crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{dnsDomain}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) @@ -129,7 +129,7 @@ func testRFC2136DNSProvider() bool { It("should obtain a signed certificate for a wildcard domain", func() { By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{"*." + dnsDomain}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) @@ -142,7 +142,7 @@ func testRFC2136DNSProvider() bool { It("should obtain a signed certificate for a wildcard and apex domain", func() { By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{"*." + dnsDomain, dnsDomain}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) diff --git a/test/e2e/suite/issuers/acme/certificaterequest/http01.go b/test/e2e/suite/issuers/acme/certificaterequest/http01.go index e8bc7af28..a9cb1c8f4 100644 --- a/test/e2e/suite/issuers/acme/certificaterequest/http01.go +++ b/test/e2e/suite/issuers/acme/certificaterequest/http01.go @@ -125,7 +125,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{acmeIngressDomain}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) @@ -141,7 +141,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{acmeIngressDomain}, nil, nil, x509.ECDSA) Expect(err).NotTo(HaveOccurred()) @@ -158,7 +158,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() // the maximum length of a single segment of the domain being requested const maxLengthOfDomainSegment = 63 By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{ acmeIngressDomain, e2eutil.RandomSubdomainLength(acmeIngressDomain, maxLengthOfDomainSegment), @@ -176,7 +176,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{e2eutil.RandomSubdomain(acmeIngressDomain)}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) @@ -191,7 +191,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() It("should fail to obtain a certificate for an invalid ACME dns name", func() { // create test fixture By("Creating a CertificateRequest") - cr, _, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, _, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{"google.com"}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) @@ -210,7 +210,7 @@ var _ = framework.CertManagerDescribe("ACME CertificateRequest (HTTP01)", func() crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, nil, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, nil, []string{acmeIngressDomain}, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) diff --git a/test/e2e/suite/issuers/ca/certificaterequest.go b/test/e2e/suite/issuers/ca/certificaterequest.go index 1967be860..1f723f642 100644 --- a/test/e2e/suite/issuers/ca/certificaterequest.go +++ b/test/e2e/suite/issuers/ca/certificaterequest.go @@ -92,7 +92,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() { certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, &metav1.Duration{ Duration: time.Hour * 24 * 90, }, @@ -109,7 +109,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() { certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, &metav1.Duration{ Duration: time.Hour * 24 * 90, }, @@ -126,7 +126,7 @@ var _ = framework.CertManagerDescribe("CA CertificateRequest", func() { certRequestClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuerName, v1.IssuerKind, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuerName, v1.IssuerKind, &metav1.Duration{ Duration: time.Hour * 24 * 90, }, diff --git a/test/e2e/suite/issuers/vault/certificaterequest/approle.go b/test/e2e/suite/issuers/vault/certificaterequest/approle.go index f9e415907..4d718df55 100644 --- a/test/e2e/suite/issuers/vault/certificaterequest/approle.go +++ b/test/e2e/suite/issuers/vault/certificaterequest/approle.go @@ -153,7 +153,7 @@ func runVaultAppRoleTests(issuerKind string) { Expect(err).NotTo(HaveOccurred()) By("Creating a CertificateRequest") - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, vaultIssuerName, issuerKind, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, vaultIssuerName, issuerKind, &metav1.Duration{ Duration: time.Hour * 24 * 90, }, @@ -245,7 +245,7 @@ func runVaultAppRoleTests(issuerKind string) { By("Creating a CertificateRequest") crClient := f.CertManagerClientSet.CertmanagerV1().CertificateRequests(f.Namespace.Name) - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, vaultIssuerName, + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, vaultIssuerName, issuerKind, v.inputDuration, crDNSNames, crIPAddresses, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) _, err = crClient.Create(ctx, cr, metav1.CreateOptions{}) diff --git a/test/e2e/suite/issuers/venafi/tpp/certificaterequest.go b/test/e2e/suite/issuers/venafi/tpp/certificaterequest.go index ecde72da4..ed4a8baeb 100644 --- a/test/e2e/suite/issuers/venafi/tpp/certificaterequest.go +++ b/test/e2e/suite/issuers/venafi/tpp/certificaterequest.go @@ -83,7 +83,7 @@ var _ = TPPDescribe("CertificateRequest with a properly configured Issuer", func dnsNames := []string{rand.String(10) + ".venafi-e2e.example"} - cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, issuer.Name, cmapi.IssuerKind, nil, dnsNames, nil, nil, x509.RSA) + cr, key, err := util.NewCertManagerBasicCertificateRequest(certificateRequestName, f.Namespace.Name, issuer.Name, cmapi.IssuerKind, nil, dnsNames, nil, nil, x509.RSA) Expect(err).NotTo(HaveOccurred()) By("Creating a CertificateRequest") diff --git a/test/e2e/util/util.go b/test/e2e/util/util.go index f745b659a..a236dcc79 100644 --- a/test/e2e/util/util.go +++ b/test/e2e/util/util.go @@ -22,8 +22,6 @@ import ( "context" "crypto" "crypto/x509" - "crypto/x509/pkix" - "encoding/pem" "fmt" "net" "net/url" @@ -46,7 +44,7 @@ import ( cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" clientset "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/typed/certmanager/v1" "github.com/cert-manager/cert-manager/pkg/util" - "github.com/cert-manager/cert-manager/pkg/util/pki" + "github.com/cert-manager/cert-manager/test/unit/gen" ) func CertificateOnlyValidForDomains(cert *x509.Certificate, commonName string, dnsNames ...string) bool { @@ -163,8 +161,13 @@ func WaitForCRDToNotExist(ctx context.Context, client apiextensionsv1.CustomReso } // Deprecated: use test/unit/gen/CertificateRequest in future -func NewCertManagerBasicCertificateRequest(name, issuerName string, issuerKind string, duration *metav1.Duration, - dnsNames []string, ips []net.IP, uris []string, keyAlgorithm x509.PublicKeyAlgorithm) (*v1.CertificateRequest, crypto.Signer, error) { +func NewCertManagerBasicCertificateRequest( + name, namespace string, + issuerName, issuerKind string, + duration *metav1.Duration, + dnsNames []string, ips []net.IP, uris []string, + keyAlgorithm x509.PublicKeyAlgorithm, +) (*v1.CertificateRequest, crypto.Signer, error) { cn := "test.domain.com" if len(dnsNames) > 0 { cn = dnsNames[0] @@ -179,68 +182,25 @@ func NewCertManagerBasicCertificateRequest(name, issuerName string, issuerKind s parsedURIs = append(parsedURIs, parsed) } - var sk crypto.Signer - var signatureAlgorithm x509.SignatureAlgorithm - var err error - - switch keyAlgorithm { - case x509.RSA: - sk, err = pki.GenerateRSAPrivateKey(2048) - if err != nil { - return nil, nil, err - } - signatureAlgorithm = x509.SHA256WithRSA - case x509.ECDSA: - sk, err = pki.GenerateECPrivateKey(pki.ECCurve256) - if err != nil { - return nil, nil, err - } - signatureAlgorithm = x509.ECDSAWithSHA256 - case x509.Ed25519: - sk, err = pki.GenerateEd25519PrivateKey() - if err != nil { - return nil, nil, err - } - signatureAlgorithm = x509.PureEd25519 - default: - return nil, nil, fmt.Errorf("unrecognised key algorithm: %s", err) - } - - csr := &x509.CertificateRequest{ - Version: 0, - SignatureAlgorithm: signatureAlgorithm, - PublicKeyAlgorithm: keyAlgorithm, - PublicKey: sk.Public(), - Subject: pkix.Name{ - CommonName: cn, - }, - DNSNames: dnsNames, - IPAddresses: ips, - URIs: parsedURIs, - } - - csrBytes, err := pki.EncodeCSR(csr, sk) + csrPEM, sk, err := gen.CSR(keyAlgorithm, + gen.SetCSRCommonName(cn), + gen.SetCSRDNSNames(dnsNames...), + gen.SetCSRIPAddresses(ips...), + gen.SetCSRURIs(parsedURIs...), + ) if err != nil { return nil, nil, err } - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrBytes, - }) - - return &v1.CertificateRequest{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - Spec: v1.CertificateRequestSpec{ - Duration: duration, - Request: csrPEM, - IssuerRef: cmmeta.ObjectReference{ - Name: issuerName, - Kind: issuerKind, - }, - }, - }, sk, nil + return gen.CertificateRequest(name, + gen.SetCertificateRequestNamespace(namespace), + gen.SetCertificateRequestDuration(duration), + gen.SetCertificateRequestCSR(csrPEM), + gen.SetCertificateRequestIssuer(cmmeta.ObjectReference{ + Name: issuerName, + Kind: issuerKind, + }), + ), sk, nil } func NewCertManagerVaultCertificate(name, secretName, issuerName string, issuerKind string, duration, renewBefore *metav1.Duration) *v1.Certificate { diff --git a/test/integration/certificates/issuing_controller_test.go b/test/integration/certificates/issuing_controller_test.go index 9a1b7b8ff..d0664de21 100644 --- a/test/integration/certificates/issuing_controller_test.go +++ b/test/integration/certificates/issuing_controller_test.go @@ -148,22 +148,11 @@ func TestIssuingController(t *testing.T) { t.Fatal(err) } - // Create x509 CSR from Certificate - csr, err := utilpki.GenerateCSR(crt) + csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk) if err != nil { t.Fatal(err) } - // Encode CSR - csrDER, err := utilpki.EncodeCSR(csr, sk) - if err != nil { - t.Fatal(err) - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - // Sign Certificate certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt) if err != nil { @@ -371,22 +360,11 @@ func TestIssuingController_PKCS8_PrivateKey(t *testing.T) { t.Fatal(err) } - // Create x509 CSR from Certificate - csr, err := utilpki.GenerateCSR(crt) + csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk) if err != nil { t.Fatal(err) } - // Encode CSR - csrDER, err := utilpki.EncodeCSR(csr, sk) - if err != nil { - t.Fatal(err) - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - // Sign Certificate certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt) if err != nil { @@ -589,22 +567,11 @@ func Test_IssuingController_SecretTemplate(t *testing.T) { t.Fatal(err) } - // Create x509 CSR from Certificate - csr, err := utilpki.GenerateCSR(crt) + csrPEM, err := gen.CSRWithSignerForCertificate(crt, sk) if err != nil { t.Fatal(err) } - // Encode CSR - csrDER, err := utilpki.EncodeCSR(csr, sk) - if err != nil { - t.Fatal(err) - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - // Sign Certificate certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt) if err != nil { @@ -836,22 +803,11 @@ func Test_IssuingController_AdditionalOutputFormats(t *testing.T) { t.Fatal(err) } - // Create x509 CSR from Certificate - csr, err := utilpki.GenerateCSR(crt) + csrPEM, err := gen.CSRWithSignerForCertificate(crt, pk) if err != nil { t.Fatal(err) } - // Encode CSR - csrDER, err := utilpki.EncodeCSR(csr, pk) - if err != nil { - t.Fatal(err) - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - // Sign Certificate certTemplate, err := utilpki.CertificateTemplateFromCertificate(crt) if err != nil { diff --git a/test/integration/certificates/revisionmanager_controller_test.go b/test/integration/certificates/revisionmanager_controller_test.go index a561f5986..9c43ad319 100644 --- a/test/integration/certificates/revisionmanager_controller_test.go +++ b/test/integration/certificates/revisionmanager_controller_test.go @@ -18,7 +18,6 @@ package certificates import ( "context" - "encoding/pem" "strconv" "testing" "time" @@ -36,7 +35,6 @@ import ( "github.com/cert-manager/cert-manager/pkg/controller/certificates/revisionmanager" logf "github.com/cert-manager/cert-manager/pkg/logs" "github.com/cert-manager/cert-manager/pkg/metrics" - utilpki "github.com/cert-manager/cert-manager/pkg/util/pki" "github.com/cert-manager/cert-manager/test/unit/gen" ) @@ -107,27 +105,11 @@ func TestRevisionManagerController(t *testing.T) { t.Fatal(err) } - // Create a new private key - sk, err := utilpki.GenerateRSAPrivateKey(2048) + csrPEM, _, err := gen.CSRForCertificate(crt) if err != nil { t.Fatal(err) } - csr, err := utilpki.GenerateCSR(crt) - if err != nil { - t.Fatal(err) - } - - // Encode CSR - csrDER, err := utilpki.EncodeCSR(csr, sk) - if err != nil { - t.Fatal(err) - } - - csrPEM := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrDER, - }) - // Create 6 CertificateRequests which are owned by this Certificate for i := 0; i < 6; i++ { _, err = cmCl.CertmanagerV1().CertificateRequests(namespace).Create(ctx, &cmapi.CertificateRequest{ diff --git a/test/integration/validation/certificaterequest_test.go b/test/integration/validation/certificaterequest_test.go index f0124e66d..728262c61 100644 --- a/test/integration/validation/certificaterequest_test.go +++ b/test/integration/validation/certificaterequest_test.go @@ -18,7 +18,6 @@ package validation import ( "context" - "encoding/pem" "strings" "testing" "time" @@ -33,7 +32,7 @@ import ( "github.com/cert-manager/cert-manager/pkg/api" cmapi "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" - "github.com/cert-manager/cert-manager/pkg/util/pki" + "github.com/cert-manager/cert-manager/test/unit/gen" ) var certGVK = schema.GroupVersionKind{ @@ -200,22 +199,9 @@ func TestValidationCertificateRequests(t *testing.T) { } func mustGenerateCSR(t *testing.T, cert *cmapi.Certificate) []byte { - request, err := pki.GenerateCSR(cert) + csr, _, err := gen.CSRForCertificate(cert) if err != nil { t.Fatal(err) } - - sk, err := pki.GenerateRSAPrivateKey(2048) - if err != nil { - t.Fatal(err) - } - csrBytes, err := pki.EncodeCSR(request, sk) - if err != nil { - t.Fatal(err) - } - csr := pem.EncodeToMemory(&pem.Block{ - Type: "CERTIFICATE REQUEST", Bytes: csrBytes, - }) - return csr } diff --git a/test/unit/gen/csr.go b/test/unit/gen/csr.go index 0e6ff0868..a135c11ce 100644 --- a/test/unit/gen/csr.go +++ b/test/unit/gen/csr.go @@ -28,11 +28,57 @@ import ( "net" "net/url" + v1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" "github.com/cert-manager/cert-manager/pkg/util/pki" ) type CSRModifier func(*x509.CertificateRequest) error +var defaultGenerateCSROptions = []pki.GenerateCSROption{ + pki.WithEncodeBasicConstraintsInRequest(true), + pki.WithNameConstraints(true), + pki.WithOtherNames(true), + pki.WithUseLiteralSubject(true), +} + +func CSRForCertificate(crt *v1.Certificate, mods ...CSRModifier) (csr []byte, sk crypto.Signer, err error) { + cr, err := pki.GenerateCSR(crt, defaultGenerateCSROptions...) + if err != nil { + return nil, nil, err + } + + modifiers := []CSRModifier{} + modifiers = append(modifiers, func(c *x509.CertificateRequest) error { + *c = *cr + return nil + }) + modifiers = append(modifiers, mods...) + + return CSR(cr.PublicKeyAlgorithm, modifiers...) +} + +func CSRWithSignerForCertificate(crt *v1.Certificate, sk crypto.Signer, mods ...CSRModifier) (csr []byte, err error) { + cr, err := pki.GenerateCSR(crt, defaultGenerateCSROptions...) + if err != nil { + return nil, err + } + + modifiers := []CSRModifier{} + modifiers = append(modifiers, func(c *x509.CertificateRequest) error { + if c.PublicKeyAlgorithm != cr.PublicKeyAlgorithm { + return fmt.Errorf("public key algorithm mismatch: %s != %s", c.PublicKeyAlgorithm, cr.PublicKeyAlgorithm) + } + if c.SignatureAlgorithm != cr.SignatureAlgorithm { + return fmt.Errorf("signature algorithm mismatch: %s != %s", c.SignatureAlgorithm, cr.SignatureAlgorithm) + } + *c = *cr + return nil + }) + modifiers = append(modifiers, mods...) + + return CSRWithSigner(sk, modifiers...) +} + func CSR(keyAlgorithm x509.PublicKeyAlgorithm, mods ...CSRModifier) (csr []byte, sk crypto.Signer, err error) { switch keyAlgorithm { case x509.RSA: