From 02219bdfb8f69a3acd30f17b1dd0f711bc385c66 Mon Sep 17 00:00:00 2001
From: Antoine Cotten <hello@acotten.com>
Date: Sun, 24 Jun 2018 23:34:25 +0200
Subject: [PATCH] Improve best-cert guessing with empty tls.hosts

---
 internal/ingress/controller/controller.go     | 11 +--
 .../ingress/controller/controller_test.go     | 70 +++++++++++++++----
 2 files changed, 62 insertions(+), 19 deletions(-)

diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go
index 92786af9a2..367d628163 100644
--- a/internal/ingress/controller/controller.go
+++ b/internal/ingress/controller/controller.go
@@ -1063,7 +1063,7 @@ func extractTLSSecretName(host string, ing *extensions.Ingress,
 		}
 	}
 
-	// no TLS host matching host name, try each TLS host for matching CN
+	// no TLS host matching host name, try each TLS host for matching SAN or CN
 	for _, tls := range ing.Spec.TLS {
 		key := fmt.Sprintf("%v/%v", ing.Namespace, tls.SecretName)
 		cert, err := getLocalSSLCert(key)
@@ -1072,13 +1072,16 @@ func extractTLSSecretName(host string, ing *extensions.Ingress,
 			continue
 		}
 
-		if cert == nil {
+		if cert == nil { // for tests
 			continue
 		}
 
-		if sets.NewString(cert.CN...).Has(host) {
-			return tls.SecretName
+		err = cert.Certificate.VerifyHostname(host)
+		if err != nil {
+			continue
 		}
+		glog.V(3).Infof("Found SSL certificate matching host %q: %q", host, key)
+		return tls.SecretName
 	}
 
 	return ""
diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go
index e658884597..12c89619aa 100644
--- a/internal/ingress/controller/controller_test.go
+++ b/internal/ingress/controller/controller_test.go
@@ -17,6 +17,9 @@ limitations under the License.
 package controller
 
 import (
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/asn1"
 	"testing"
 
 	extensions "k8s.io/api/extensions/v1beta1"
@@ -25,13 +28,13 @@ import (
 )
 
 func TestExtractTLSSecretName(t *testing.T) {
-	tests := []struct {
+	testCases := map[string]struct {
 		host    string
 		ingress *extensions.Ingress
 		fn      func(string) (*ingress.SSLCert, error)
 		expName string
 	}{
-		{
+		"nil ingress": {
 			"foo.bar",
 			nil,
 			func(string) (*ingress.SSLCert, error) {
@@ -39,7 +42,7 @@ func TestExtractTLSSecretName(t *testing.T) {
 			},
 			"",
 		},
-		{
+		"empty ingress": {
 			"foo.bar",
 			&extensions.Ingress{},
 			func(string) (*ingress.SSLCert, error) {
@@ -47,7 +50,7 @@ func TestExtractTLSSecretName(t *testing.T) {
 			},
 			"",
 		},
-		{
+		"ingress tls, nil secret": {
 			"foo.bar",
 			&extensions.Ingress{
 				ObjectMeta: metav1.ObjectMeta{
@@ -69,7 +72,7 @@ func TestExtractTLSSecretName(t *testing.T) {
 			},
 			"",
 		},
-		{
+		"ingress tls, no host, matching cert cn": {
 			"foo.bar",
 			&extensions.Ingress{
 				ObjectMeta: metav1.ObjectMeta{
@@ -88,12 +91,12 @@ func TestExtractTLSSecretName(t *testing.T) {
 			},
 			func(string) (*ingress.SSLCert, error) {
 				return &ingress.SSLCert{
-					CN: []string{"foo.bar", "example.com"},
+					Certificate: fakeX509Cert([]string{"foo.bar", "example.com"}),
 				}, nil
 			},
 			"demo",
 		},
-		{
+		"ingress tls, no host, wildcard cert with matching cn": {
 			"foo.bar",
 			&extensions.Ingress{
 				ObjectMeta: metav1.ObjectMeta{
@@ -102,30 +105,67 @@ func TestExtractTLSSecretName(t *testing.T) {
 				Spec: extensions.IngressSpec{
 					TLS: []extensions.IngressTLS{
 						{
-							Hosts:      []string{"foo.bar", "example.com"},
 							SecretName: "demo",
 						},
 					},
 					Rules: []extensions.IngressRule{
 						{
-							Host: "foo.bar",
+							Host: "test.foo.bar",
 						},
 					},
 				},
 			},
 			func(string) (*ingress.SSLCert, error) {
 				return &ingress.SSLCert{
-					CN: []string{"foo.bar", "example.com"},
+					Certificate: fakeX509Cert([]string{"*.foo.bar", "foo.bar"}),
 				}, nil
 			},
 			"demo",
 		},
+		"ingress tls, hosts, matching cert cn": {
+			"foo.bar",
+			&extensions.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test",
+				},
+				Spec: extensions.IngressSpec{
+					TLS: []extensions.IngressTLS{
+						{
+							Hosts:      []string{"foo.bar", "example.com"},
+							SecretName: "demo",
+						},
+					},
+					Rules: []extensions.IngressRule{
+						{
+							Host: "foo.bar",
+						},
+					},
+				},
+			},
+			func(string) (*ingress.SSLCert, error) {
+				return nil, nil
+			},
+			"demo",
+		},
 	}
 
-	for _, testCase := range tests {
-		name := extractTLSSecretName(testCase.host, testCase.ingress, testCase.fn)
-		if name != testCase.expName {
-			t.Errorf("expected %v as the name of the secret but got %v", testCase.expName, name)
-		}
+	for title, tc := range testCases {
+		t.Run(title, func(t *testing.T) {
+			name := extractTLSSecretName(tc.host, tc.ingress, tc.fn)
+			if name != tc.expName {
+				t.Errorf("Expected Secret name %q (got %q)", tc.expName, name)
+			}
+		})
+	}
+}
+
+var oidExtensionSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17}
+
+func fakeX509Cert(dnsNames []string) *x509.Certificate {
+	return &x509.Certificate{
+		DNSNames: dnsNames,
+		Extensions: []pkix.Extension{
+			{Id: oidExtensionSubjectAltName},
+		},
 	}
 }