Merge pull request #4229 from aledbf/4106

Do not send empty certificates to nginx

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 != "" &&
+		if server.Alias != "" && ssl.IsValidHostname(server.Alias, server.SSLCert.CN) {
-			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{

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

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)))
+	})
 })