Merge pull request #8637 from kubevirt-bot/cherry-pick-8147-to-releas…

…e-0.58

[release-0.58] Return certificate chain for export cert

pkg/certificates/triple/cert/cert.go

@@ -36,10 +36,11 @@ const (
 
 // Config contains the basic fields required for creating a certificate
 type Config struct {
-	CommonName   string
-	Organization []string
-	AltNames     AltNames
-	Usages       []x509.ExtKeyUsage
+	CommonName          string
+	Organization        []string
+	AltNames            AltNames
+	Usages              []x509.ExtKeyUsage
+	NotBefore, NotAfter *time.Time
 }
 
 // AltNames contains the domain names and IP addresses that will be added
@@ -56,12 +57,7 @@ func NewPrivateKey() (*rsa.PrivateKey, error) {
 }
 
 // NewSelfSignedCACert creates a CA certificate
-func NewSelfSignedCACert(cfg Config, key crypto.Signer, duration time.Duration, altNames ...string) (*x509.Certificate, error) {
-	return NewSelfSignedCACertWithAltNames(cfg, key, duration)
-}
-
-// NewSelfSignedCACertWithAltNames creates a CA certificate that allows alternative names
-func NewSelfSignedCACertWithAltNames(cfg Config, key crypto.Signer, duration time.Duration, altNames ...string) (*x509.Certificate, error) {
+func NewSelfSignedCACert(cfg Config, key crypto.Signer, duration time.Duration) (*x509.Certificate, error) {
 	now := time.Now()
 	tmpl := x509.Certificate{
 		SerialNumber: new(big.Int).SetInt64(randomSerialNumber()),
@@ -74,7 +70,13 @@ func NewSelfSignedCACertWithAltNames(cfg Config, key crypto.Signer, duration tim
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
 		BasicConstraintsValid: true,
 		IsCA:                  true,
-		DNSNames:              altNames,
+		DNSNames:              cfg.AltNames.DNSNames,
+	}
+	if cfg.NotBefore != nil {
+		tmpl.NotBefore = *cfg.NotBefore
+	}
+	if cfg.NotAfter != nil {
+		tmpl.NotAfter = *cfg.NotAfter
 	}
 
 	certDERBytes, err := x509.CreateCertificate(cryptorand.Reader, &tmpl, &tmpl, key.Public(), key)
@@ -110,6 +112,12 @@ func NewSignedCert(cfg Config, key crypto.Signer, caCert *x509.Certificate, caKe
 		KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
 		ExtKeyUsage:  cfg.Usages,
 	}
+	if cfg.NotBefore != nil {
+		certTmpl.NotBefore = *cfg.NotBefore
+	}
+	if cfg.NotAfter != nil {
+		certTmpl.NotAfter = *cfg.NotAfter
+	}
 	certDERBytes, err := x509.CreateCertificate(cryptorand.Reader, &certTmpl, caCert, key.Public(), caKey)
 	if err != nil {
 		return nil, err

pkg/storage/export/export/export_test.go

@@ -20,6 +20,7 @@ package export
 
 import (
 	"crypto/tls"
+	"crypto/x509"
 	"encoding/pem"
 	"fmt"
 	"io/ioutil"
@@ -255,23 +256,67 @@ var _ = Describe("Export controller", func() {
 		Expect(os.RemoveAll(certDir)).To(Succeed())
 	})
 
-	generateExpectedCert := func() string {
-		key, err := certutil.NewPrivateKey()
-		Expect(err).ToNot(HaveOccurred())
-
+	generateCertFromTime := func(cn string, before, after *time.Time) string {
+		defer GinkgoRecover()
 		config := certutil.Config{
-			CommonName: "blah blah",
+			CommonName: cn,
+			NotBefore:  before,
+			NotAfter:   after,
 		}
+		defer GinkgoRecover()
+		caKeyPair, _ := triple.NewCA("kubevirt.io", time.Hour*24*7)
+
+		intermediateKey, err := certutil.NewPrivateKey()
+		Expect(err).ToNot(HaveOccurred())
+		intermediateConfig := certutil.Config{
+			CommonName: fmt.Sprintf("%s@%d", "intermediate", time.Now().Unix()),
+			NotBefore:  before,
+			NotAfter:   after,
+		}
+		intermediateConfig.Usages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
+		intermediateCert, err := certutil.NewSignedCert(intermediateConfig, intermediateKey, caKeyPair.Cert, caKeyPair.Key, time.Hour)
+		Expect(err).ToNot(HaveOccurred())
 
-		cert, err := certutil.NewSelfSignedCACertWithAltNames(config, key, time.Hour, "hahaha.wwoo", "*.apps-crc.testing", "fgdgd.dfsgdf")
+		key, err := certutil.NewPrivateKey()
+		Expect(err).ToNot(HaveOccurred())
+
+		config.AltNames.DNSNames = []string{"hahaha.wwoo", "*.apps-crc.testing", "fgdgd.dfsgdf"}
+		config.Usages = []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}
+		cert, err := certutil.NewSignedCert(config, key, intermediateCert, intermediateKey, time.Hour)
 		Expect(err).ToNot(HaveOccurred())
 		pemOut := strings.Builder{}
-		Expect(pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})).To(Succeed())
+		pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw})
+		pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: intermediateCert.Raw})
+		pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: caKeyPair.Cert.Raw})
 		return strings.TrimSpace(pemOut.String())
 	}
 
+	generateFutureCert := func() string {
+		before := time.Now().AddDate(0, 1, 0)
+		after := before.AddDate(0, 0, 7)
+		return generateCertFromTime("future cert", &before, &after)
+	}
+
+	generateExpiredCert := func() string {
+		before := time.Now().AddDate(0, -1, 0)
+		after := before.AddDate(0, 0, 7)
+		return generateCertFromTime("expired cert", &before, &after)
+	}
+
+	generateExpectedCert := func() string {
+		before := time.Now()
+		after := before.AddDate(0, 0, 7)
+		return generateCertFromTime("working cert", &before, &after)
+	}
+
 	var expectedPem = generateExpectedCert()
 
+	generateOverlappingCert := func() string {
+		before := time.Now().AddDate(0, 0, -3)
+		after := before.AddDate(0, 0, 7)
+		return generateCertFromTime("overlapping cert", &before, &after) + "\n" + expectedPem
+	}
+
 	generateRouteCert := func() string {
 		key, err := certutil.NewPrivateKey()
 		Expect(err).ToNot(HaveOccurred())
@@ -287,18 +332,34 @@ var _ = Describe("Export controller", func() {
 		return strings.TrimSpace(pemOut.String()) + "\n" + expectedPem
 	}
 
-	createRouteConfigMap := func() *k8sv1.ConfigMap {
+	createRouteConfigMapFromFunc := func(certFunc func() string) *k8sv1.ConfigMap {
 		return &k8sv1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      routeCAConfigMapName,
 				Namespace: controller.KubevirtNamespace,
 			},
 			Data: map[string]string{
-				routeCaKey: generateRouteCert(),
+				routeCaKey: certFunc(),
 			},
 		}
 	}
 
+	createFutureRouteConfigMap := func() *k8sv1.ConfigMap {
+		return createRouteConfigMapFromFunc(generateFutureCert)
+	}
+
+	createExpiredRouteConfigMap := func() *k8sv1.ConfigMap {
+		return createRouteConfigMapFromFunc(generateExpiredCert)
+	}
+
+	createOverlappingRouteConfigMap := func() *k8sv1.ConfigMap {
+		return createRouteConfigMapFromFunc(generateOverlappingCert)
+	}
+
+	createRouteConfigMap := func() *k8sv1.ConfigMap {
+		return createRouteConfigMapFromFunc(generateRouteCert)
+	}
+
 	validIngressDefaultBackend := func(serviceName string) *networkingv1.Ingress {
 		return &networkingv1.Ingress{
 			Spec: networkingv1.IngressSpec{
@@ -866,16 +927,19 @@ var _ = Describe("Export controller", func() {
 		Entry("ingress with rules no backend service", ingressRulesNoBackend(), ""),
 	)
 
-	DescribeTable("should find host when route is defined", func(route *routev1.Route, hostname, expectedCert string) {
-		Expect(controller.RouteCache.Add(route)).To(Succeed())
-		Expect(controller.RouteConfigMapInformer.GetStore().Add(createRouteConfigMap())).To(Succeed())
+	DescribeTable("should find host when route is defined", func(createCMFunc func() *k8sv1.ConfigMap, route *routev1.Route, hostname, expectedCert string) {
+		controller.RouteCache.Add(route)
+		controller.RouteConfigMapInformer.GetStore().Add(createCMFunc())
 		host, cert := controller.getExternalLinkHostAndCert()
-		Expect(hostname).To(Equal(host))
-		Expect(expectedCert).To(Equal(cert))
+		Expect(host).To(Equal(hostname))
+		Expect(cert).To(Equal(expectedCert))
 	},
-		Entry("route with service and host", routeToHostAndService(components.VirtExportProxyServiceName), "virt-exportproxy-kubevirt.apps-crc.testing", expectedPem),
-		Entry("route with different service and host", routeToHostAndService("other-service"), "", ""),
-		Entry("route with service and no ingress", routeToHostAndNoIngress(), "", ""),
+		Entry("route with service and host", createRouteConfigMap, routeToHostAndService(components.VirtExportProxyServiceName), "virt-exportproxy-kubevirt.apps-crc.testing", expectedPem),
+		Entry("route with different service and host", createRouteConfigMap, routeToHostAndService("other-service"), "", ""),
+		Entry("route with service and no ingress", createRouteConfigMap, routeToHostAndNoIngress(), "", ""),
+		Entry("should not find route cert if in future", createFutureRouteConfigMap, routeToHostAndService(components.VirtExportProxyServiceName), "virt-exportproxy-kubevirt.apps-crc.testing", ""),
+		Entry("should not find route cert if expired", createExpiredRouteConfigMap, routeToHostAndService(components.VirtExportProxyServiceName), "virt-exportproxy-kubevirt.apps-crc.testing", ""),
+		Entry("should find correct route cert if overlapping exists", createOverlappingRouteConfigMap, routeToHostAndService(components.VirtExportProxyServiceName), "virt-exportproxy-kubevirt.apps-crc.testing", expectedPem),
 	)
 
 	It("should pick ingress over route if both exist", func() {

pkg/storage/export/export/links.go

@@ -25,6 +25,7 @@ import (
 	"fmt"
 	"path"
 	"strings"
+	"time"
 	"unicode"
 
 	routev1 "github.com/openshift/api/route/v1"
@@ -196,9 +197,13 @@ func (ctrl *VMExportController) getRouteCert(hostName string) (string, error) {
 }
 
 func (ctrl *VMExportController) findCertByHostName(hostName string, certs []*x509.Certificate) (string, error) {
+	now := time.Now()
+	var latestCert *x509.Certificate
 	for _, cert := range certs {
 		if ctrl.matchesOrWildCard(hostName, cert.Subject.CommonName) {
-			return buildPemFromCert(cert)
+			if latestCert == nil || (cert.NotAfter.After(latestCert.NotAfter) && cert.NotBefore.Before(time.Now())) {
+				latestCert = cert
+			}
 		}
 		for _, extension := range cert.Extensions {
 			if extension.Id.String() == subjectAltNameId {
@@ -211,19 +216,32 @@ func (ctrl *VMExportController) findCertByHostName(hostName string, certs []*x50
 				names := strings.Split(value, " ")
 				for _, name := range names {
 					if ctrl.matchesOrWildCard(hostName, name) {
-						return buildPemFromCert(cert)
+						if latestCert == nil || (cert.NotAfter.After(latestCert.NotAfter) && cert.NotBefore.Before(time.Now())) {
+							latestCert = cert
+						}
 					}
 				}
 			}
 		}
 	}
+	if latestCert != nil && latestCert.NotAfter.After(now) && latestCert.NotBefore.Before(now) {
+		return ctrl.buildPemFromCert(latestCert, certs)
+	}
 	return "", nil
 }
 
-func buildPemFromCert(cert *x509.Certificate) (string, error) {
+func (ctrl *VMExportController) buildPemFromCert(matchingCert *x509.Certificate, allCerts []*x509.Certificate) (string, error) {
 	pemOut := strings.Builder{}
-	if err := pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}); err != nil {
-		return "", err
+	pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: matchingCert.Raw})
+	if matchingCert.Issuer.CommonName != matchingCert.Subject.CommonName && !matchingCert.IsCA {
+		//lookup issuer recursively, if not found a blank is returned.
+		chain, err := ctrl.findCertByHostName(matchingCert.Issuer.CommonName, allCerts)
+		if err != nil {
+			return "", err
+		}
+		if _, err := pemOut.WriteString(chain); err != nil {
+			return "", err
+		}
 	}
 	return strings.TrimSpace(pemOut.String()), nil
 }

tests/storage/export.go

@@ -938,12 +938,10 @@ var _ = SIGDescribe("Export", func() {
 			config := certutil.Config{
 				CommonName: "blah blah",
 			}
+			config.AltNames.DNSNames = []string{"hahaha.wwoo", hostName, "fgdgd.dfsgdf"}
 
-			cert, err := certutil.NewSelfSignedCACertWithAltNames(config, key, time.Hour, "hahaha.wwoo", hostName, "fgdgd.dfsgdf")
-			if err != nil {
-				return "", err
-			}
-
+			cert, err := certutil.NewSelfSignedCACert(config, key, time.Hour)
+			Expect(err).ToNot(HaveOccurred())
 			pemOut := strings.Builder{}
 			if err := pem.Encode(&pemOut, &pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}); err != nil {
 				return "", err