From 6b5df6b42bd5ee3a0efb360cdc6d419c13956dca Mon Sep 17 00:00:00 2001 From: Ashley Davis Date: Mon, 19 Jul 2021 16:57:01 +0100 Subject: [PATCH] remove unused ValidateIssuedCertificate function this function doesn't appear to be used anywhere, which makes it a little confusing when trying to work out how the checks are done in the e2e tests. given that we encourage people not to import cert-manager as a module and that anyone who does is likely not to use functions in the test directory, it seems safe enough to remove this rather than deprecating it. Signed-off-by: Ashley Davis --- test/e2e/framework/helper/certificates.go | 176 ---------------------- 1 file changed, 176 deletions(-) diff --git a/test/e2e/framework/helper/certificates.go b/test/e2e/framework/helper/certificates.go index 29da2e426..940c24302 100644 --- a/test/e2e/framework/helper/certificates.go +++ b/test/e2e/framework/helper/certificates.go @@ -18,23 +18,16 @@ package helper import ( "context" - "crypto/ecdsa" - "crypto/ed25519" - "crypto/rsa" "crypto/x509" "fmt" "sort" "time" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apiutil "github.com/jetstack/cert-manager/pkg/api/util" cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" - "github.com/jetstack/cert-manager/pkg/util" - "github.com/jetstack/cert-manager/pkg/util/pki" "github.com/jetstack/cert-manager/test/e2e/framework/log" e2eutil "github.com/jetstack/cert-manager/test/e2e/util" ) @@ -82,175 +75,6 @@ func (h *Helper) WaitForCertificateNotReadyUpdate(cert *cmapi.Certificate, timeo return h.handleResult(cert.Namespace, cert.Name, result, err) } -// ValidateIssuedCertificate will ensure that the given Certificate has a -// certificate issued for it, and that the details on the x509 certificate are -// correct as defined by the Certificate's spec. -func (h *Helper) ValidateIssuedCertificate(certificate *cmapi.Certificate, rootCAPEM []byte) (*x509.Certificate, error) { - log.Logf("Getting the TLS certificate Secret resource") - secret, err := h.KubeClient.CoreV1().Secrets(certificate.Namespace).Get(context.TODO(), certificate.Spec.SecretName, metav1.GetOptions{}) - if err != nil { - return nil, err - } - if !(len(secret.Data) == 2 || len(secret.Data) == 3) { - return nil, fmt.Errorf("Expected 2 keys in certificate secret, but there was %d", len(secret.Data)) - } - - keyBytes, ok := secret.Data[corev1.TLSPrivateKeyKey] - if !ok { - return nil, fmt.Errorf("No private key data found for Certificate %q (secret %q)", certificate.Name, certificate.Spec.SecretName) - } - key, err := pki.DecodePrivateKeyBytes(keyBytes) - if err != nil { - return nil, err - } - - // validate private key is of the correct type (rsa, ed25519 or ecdsa) - privateKey := certificate.Spec.PrivateKey - if privateKey == nil { - privateKey = &cmapi.CertificatePrivateKey{} - } - switch privateKey.Algorithm { - case cmapi.PrivateKeyAlgorithm(""), - cmapi.RSAKeyAlgorithm: - _, ok := key.(*rsa.PrivateKey) - if !ok { - return nil, fmt.Errorf("Expected private key of type RSA, but it was: %T", key) - } - case cmapi.ECDSAKeyAlgorithm: - _, ok := key.(*ecdsa.PrivateKey) - if !ok { - return nil, fmt.Errorf("Expected private key of type ECDSA, but it was: %T", key) - } - case cmapi.Ed25519KeyAlgorithm: - _, ok := key.(ed25519.PrivateKey) - if !ok { - return nil, fmt.Errorf("Expected private key of type Ed25519, but it was: %T", key) - } - default: - return nil, fmt.Errorf("unrecognised requested private key algorithm %q", certificate.Spec.PrivateKey.Algorithm) - } - - // TODO: validate private key KeySize - - // check the provided certificate is valid - expectedOrganization := pki.OrganizationForCertificate(certificate) - expectedDNSNames := certificate.Spec.DNSNames - expectedIPAddresses := certificate.Spec.IPAddresses - uris, err := pki.URIsForCertificate(certificate) - if err != nil { - return nil, fmt.Errorf("failed to parse URIs: %s", err) - } - - expectedURIs := pki.URLsToString(uris) - - certBytes, ok := secret.Data[corev1.TLSCertKey] - if !ok { - return nil, fmt.Errorf("No certificate data found for Certificate %q (secret %q)", certificate.Name, certificate.Spec.SecretName) - } - - cert, err := pki.DecodeX509CertificateBytes(certBytes) - if err != nil { - return nil, err - } - - commonNameCorrect := true - expectedCN := certificate.Spec.CommonName - if len(expectedCN) == 0 && len(cert.Subject.CommonName) > 0 { - // issuers might set an IP or DNSName as CN - if !util.Contains(cert.DNSNames, cert.Subject.CommonName) && !util.Contains(pki.IPAddressesToString(cert.IPAddresses), cert.Subject.CommonName) { - commonNameCorrect = false - } - } else if expectedCN != cert.Subject.CommonName { - commonNameCorrect = false - } - - if !commonNameCorrect || !util.Subset(cert.DNSNames, expectedDNSNames) || !util.EqualUnsorted(pki.URLsToString(cert.URIs), expectedURIs) || - !util.Subset(pki.IPAddressesToString(cert.IPAddresses), expectedIPAddresses) || - !(len(cert.Subject.Organization) == 0 || util.EqualUnsorted(cert.Subject.Organization, expectedOrganization)) { - return nil, fmt.Errorf("Expected certificate valid for CN %q, O %v, dnsNames %v, uriSANs %v,but got a certificate valid for CN %q, O %v, dnsNames %v, uriSANs %v, ipAddresses %v", - expectedCN, expectedOrganization, expectedDNSNames, expectedURIs, cert.Subject.CommonName, cert.Subject.Organization, cert.DNSNames, cert.URIs, cert.IPAddresses) - } - - if certificate.Status.NotAfter == nil { - return nil, fmt.Errorf("No certificate expiration found for Certificate %q", certificate.Name) - } - if !cert.NotAfter.Equal(certificate.Status.NotAfter.Time) { - return nil, fmt.Errorf("Expected certificate expiry date to be %v, but got %v", certificate.Status.NotAfter, cert.NotAfter) - } - - label, ok := secret.Annotations[cmapi.CertificateNameKey] - if !ok { - return nil, fmt.Errorf("Expected secret to have certificate-name label, but had none") - } - - if label != certificate.Name { - return nil, fmt.Errorf("Expected secret to have certificate-name label with a value of %q, but got %q", certificate.Name, label) - } - - certificateKeyUsages, certificateExtKeyUsages, err := pki.BuildKeyUsages(certificate.Spec.Usages, certificate.Spec.IsCA) - if err != nil { - return nil, fmt.Errorf("failed to build key usages from certificate: %s", err) - } - - defaultCertKeyUsages, defaultCertExtKeyUsages, err := h.defaultKeyUsagesToAdd(certificate.Namespace, &certificate.Spec.IssuerRef) - if err != nil { - return nil, err - } - - certificateKeyUsages |= defaultCertKeyUsages - certificateExtKeyUsages = append(certificateExtKeyUsages, defaultCertExtKeyUsages...) - - // If using ECDSA then ignore key encipherment - if certificate.Spec.PrivateKey != nil && certificate.Spec.PrivateKey.Algorithm == cmapi.ECDSAKeyAlgorithm { - certificateKeyUsages &^= x509.KeyUsageKeyEncipherment - cert.KeyUsage &^= x509.KeyUsageKeyEncipherment - } - - certificateExtKeyUsages = h.deduplicateExtKeyUsages(certificateExtKeyUsages) - - if !h.keyUsagesMatch(cert.KeyUsage, cert.ExtKeyUsage, - certificateKeyUsages, certificateExtKeyUsages) { - return nil, fmt.Errorf("key usages and extended key usages do not match: exp=%s got=%s exp=%s got=%s", - apiutil.KeyUsageStrings(certificateKeyUsages), apiutil.KeyUsageStrings(cert.KeyUsage), - apiutil.ExtKeyUsageStrings(certificateExtKeyUsages), apiutil.ExtKeyUsageStrings(cert.ExtKeyUsage)) - } - - if !util.EqualUnsorted(cert.EmailAddresses, certificate.Spec.EmailAddresses) { - return nil, fmt.Errorf("certificate doesn't contain Email SANs: exp=%v got=%v", certificate.Spec.EmailAddresses, cert.EmailAddresses) - } - - var dnsName string - if len(expectedDNSNames) > 0 { - dnsName = expectedDNSNames[0] - } - - // TODO: move this verification step out of this function - if rootCAPEM != nil { - rootCertPool := x509.NewCertPool() - rootCertPool.AppendCertsFromPEM(rootCAPEM) - intermediateCertPool := x509.NewCertPool() - intermediateCertPool.AppendCertsFromPEM(certBytes) - opts := x509.VerifyOptions{ - DNSName: dnsName, - Intermediates: intermediateCertPool, - Roots: rootCertPool, - } - - if _, err := cert.Verify(opts); err != nil { - return nil, err - } - } - - readyCond := apiutil.GetCertificateCondition(certificate, cmapi.CertificateConditionReady) - if readyCond.Status != cmmeta.ConditionTrue || readyCond.ObservedGeneration != certificate.Generation { - return nil, fmt.Errorf( - "expected Certificate to have ready condition true, with an observedGeneration matching the Certificate generation, got=%+v", - readyCond) - } - - return cert, nil -} - func (h *Helper) deduplicateExtKeyUsages(us []x509.ExtKeyUsage) []x509.ExtKeyUsage { extKeyUsagesMap := make(map[x509.ExtKeyUsage]bool) for _, e := range us {