From 8ca5c1cba9a2b63137910d7b0f9ed2ec6d9c0c78 Mon Sep 17 00:00:00 2001
From: Manuel Alejandro de Brito Fontes <aledbf@gmail.com>
Date: Tue, 25 Jun 2019 07:49:00 -0400
Subject: [PATCH 1/2] Do not send empty certificates to nginx

---
 internal/ingress/controller/nginx.go | 11 +++++++++--
 internal/ingress/sslcert.go          |  2 +-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/internal/ingress/controller/nginx.go b/internal/ingress/controller/nginx.go
index 4d32cc73f..8892cf7e8 100644
--- a/internal/ingress/controller/nginx.go
+++ b/internal/ingress/controller/nginx.go
@@ -994,6 +994,10 @@ func configureCertificates(pcfg *ingress.Configuration) error {
 	var servers []*ingress.Server
 
 	for _, server := range pcfg.Servers {
+		if server.SSLCert.PemCertKey == "" {
+			continue
+		}
+
 		servers = append(servers, &ingress.Server{
 			Hostname: server.Hostname,
 			SSLCert: ingress.SSLCert{
@@ -1001,8 +1005,7 @@ func configureCertificates(pcfg *ingress.Configuration) error {
 			},
 		})
 
-		if server.Alias != "" && server.SSLCert.PemCertKey != "" &&
-			ssl.IsValidHostname(server.Alias, server.SSLCert.CN) {
+		if server.Alias != "" && ssl.IsValidHostname(server.Alias, server.SSLCert.CN) {
 			servers = append(servers, &ingress.Server{
 				Hostname: server.Alias,
 				SSLCert: ingress.SSLCert{
@@ -1014,6 +1017,10 @@ func configureCertificates(pcfg *ingress.Configuration) error {
 
 	redirects := buildRedirects(pcfg.Servers)
 	for _, redirect := range redirects {
+		if redirect.SSLCert.PemCertKey == "" {
+			continue
+		}
+
 		servers = append(servers, &ingress.Server{
 			Hostname: redirect.From,
 			SSLCert: ingress.SSLCert{
diff --git a/internal/ingress/sslcert.go b/internal/ingress/sslcert.go
index 4b585a583..03f139393 100644
--- a/internal/ingress/sslcert.go
+++ b/internal/ingress/sslcert.go
@@ -43,7 +43,7 @@ type SSLCert struct {
 	// ExpiresTime contains the expiration of this SSL certificate in timestamp format
 	ExpireTime time.Time `json:"expires"`
 	// Pem encoded certificate and key concatenated
-	PemCertKey string `json:"pemCertKey"`
+	PemCertKey string `json:"pemCertKey,omitempty"`
 }
 
 // GetObjectKind implements the ObjectKind interface as a noop

From 225f881ed0b56a32b98d05f5572a0f31dbc8f439 Mon Sep 17 00:00:00 2001
From: Manuel Alejandro de Brito Fontes <aledbf@gmail.com>
Date: Tue, 25 Jun 2019 09:28:52 -0400
Subject: [PATCH 2/2] Add e2e test for invalid secrets

---
 test/e2e/ssl/secret_update.go | 44 +++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/test/e2e/ssl/secret_update.go b/test/e2e/ssl/secret_update.go
index ed77c6b6c..628d45869 100644
--- a/test/e2e/ssl/secret_update.go
+++ b/test/e2e/ssl/secret_update.go
@@ -17,12 +17,15 @@ limitations under the License.
 package ssl
 
 import (
+	"crypto/tls"
 	"fmt"
+	"net/http"
 	"strings"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/parnurzeal/gorequest"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -77,4 +80,45 @@ var _ = framework.IngressNginxDescribe("SSL", func() {
 		Expect(log).ToNot(ContainSubstring(fmt.Sprintf("starting syncing of secret %v/dummy", f.Namespace)))
 		Expect(log).ToNot(ContainSubstring(fmt.Sprintf("error obtaining PEM from secret %v/dummy", f.Namespace)))
 	})
+
+	It("should return the fake SSL certificate if the secret is invalid", func() {
+		host := "invalid-ssl"
+
+		// create a secret without cert or key
+		f.EnsureSecret(&v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      host,
+				Namespace: f.Namespace,
+			},
+		})
+
+		f.EnsureIngress(framework.NewSingleIngressWithTLS(host, "/", host, []string{host}, f.Namespace, "http-svc", 80, nil))
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return strings.Contains(server, "server_name invalid-ssl") &&
+					strings.Contains(server, "listen 443")
+			})
+
+		req := gorequest.New()
+		resp, _, errs := req.
+			Get(f.GetURL(framework.HTTPS)).
+			TLSClientConfig(&tls.Config{ServerName: host, InsecureSkipVerify: true}).
+			Set("Host", host).
+			End()
+		Expect(errs).Should(BeEmpty())
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+
+		// check the returned secret is the fake one
+		cert := resp.TLS.PeerCertificates[0]
+		Expect(cert.DNSNames[0]).Should(Equal("ingress.local"))
+		Expect(cert.Subject.Organization[0]).Should(Equal("Acme Co"))
+		Expect(cert.Subject.CommonName).Should(Equal("Kubernetes Ingress Controller Fake Certificate"))
+
+		// verify the log contains a warning about invalid certificate
+		log, err := f.NginxLogs()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(log).ToNot(BeEmpty())
+		Expect(log).To(ContainSubstring(fmt.Sprintf("%v/invalid-ssl\" contains no keypair or CA certificate", f.Namespace)))
+	})
 })