Support ssl_reject_handshake in Ingress and VS

To handle missing or invalid TLS Secrets in Ingress and VirtualServer resources, previously the Ingress Controller would generate the following configuration: ssl_certificate /etc/nginx/secrets/default; ssl_certificate_key /etc/nginx/secrets/default; ssl_ciphers NULL; The configuration will break any attempts for clients to establish TLS connections for the affected server in NGINX. The configuration has the following limitations: - It requires a TLS cert and key (we used the default server Secret as it was always present on the file system) - It doesn't work if clients and NGINX use TLS v1.3: NGINX will terminate TLS connection. This commit introduces the new ssl_reject_handshake directive, which configures NGINX to break any attempt to establish a TLS connection: ssl_reject_handshake on; The directive addresses the mentioned limitations: it doesn't require a TLS cert and key and works with TLS v1.3.
nginx · Apr 2, 2021 · 2ae6e26 · 2ae6e26
1 parent 86cbd89
commit 2ae6e26
Show file tree

Hide file tree

Showing 15 changed files with 51 additions and 67 deletions.
diff --git a/docs-web/configuration/ingress-resources/basic-configuration.md b/docs-web/configuration/ingress-resources/basic-configuration.md
@@ -28,7 +28,7 @@ spec:
 Here is a breakdown of what this Ingress resource definition means:
 * The `metadata.name` field defines the name of the resource `cafe‑ingress`.
 * In the `spec.tls` field we set up SSL/TLS termination:
-    * In the `secretName`, we reference a secret resource by its name, `cafe‑secret`. The secret must belong to the same namespace as the Ingress, it must be of the type ``kubernetes.io/tls`` and contain keys named ``tls.crt`` and ``tls.key`` that hold the certificate and private key as described [here](https://kubernetes.io/docs/concepts/services-networking/ingress/#tls>). If the secret doesn't exist, NGINX will break any attempt to establish a TLS connection to the hosts to which the secret is applied.
+    * In the `secretName`, we reference a secret resource by its name, `cafe‑secret`. The secret must belong to the same namespace as the Ingress, it must be of the type ``kubernetes.io/tls`` and contain keys named ``tls.crt`` and ``tls.key`` that hold the certificate and private key as described [here](https://kubernetes.io/docs/concepts/services-networking/ingress/#tls>). If the secret doesn't exist or is invalid, NGINX will break any attempt to establish a TLS connection to the hosts to which the secret is applied.
     * In the `hosts` field, we apply the certificate and key to our `cafe.example.com` host.
 * In the `spec.rules` field, we define a host with domain name `cafe.example.com`.
 * In the `paths` field, we define two path‑based rules:

diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md
@@ -133,7 +133,7 @@ redirect:
      - Type
      - Required
    * - ``secret``
-     - The name of a secret with a TLS certificate and key. The secret must belong to the same namespace as the VirtualServer. The secret must be of the type ``kubernetes.io/tls`` and contain keys named ``tls.crt`` and ``tls.key`` that contain the certificate and private key as described `here <https://kubernetes.io/docs/concepts/services-networking/ingress/#tls>`_. If the secret doesn't exist, NGINX will break any attempt to establish a TLS connection to the host of the VirtualServer.
+     - The name of a secret with a TLS certificate and key. The secret must belong to the same namespace as the VirtualServer. The secret must be of the type ``kubernetes.io/tls`` and contain keys named ``tls.crt`` and ``tls.key`` that contain the certificate and private key as described `here <https://kubernetes.io/docs/concepts/services-networking/ingress/#tls>`_. If the secret doesn't exist or is invalid, NGINX will break any attempt to establish a TLS connection to the host of the VirtualServer.
      - ``string``
      - No
    * - ``redirect``

diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go
@@ -32,7 +32,6 @@ import (
 )
 
 const (
-	pemFileNameForMissingTLSSecret  = "/etc/nginx/secrets/default"
 	pemFileNameForWildcardTLSSecret = "/etc/nginx/secrets/wildcard"
 	appProtectPolicyFolder          = "/etc/nginx/waf/nac-policies/"
 	appProtectLogConfFolder         = "/etc/nginx/waf/nac-logconfs/"

diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go
@@ -313,6 +313,7 @@ func addSSLConfig(server *version1.Server, owner runtime.Object, host string, in
 	}
 
 	var pemFile string
+	var rejectHandshake bool
 
 	if tlsSecret != "" {
 		secretRef := secretRefs[tlsSecret]
@@ -321,27 +322,25 @@ func addSSLConfig(server *version1.Server, owner runtime.Object, host string, in
 			secretType = secretRef.Secret.Type
 		}
 		if secretType != "" && secretType != api_v1.SecretTypeTLS {
-			pemFile = pemFileNameForMissingTLSSecret
+			rejectHandshake = true
 			warnings.AddWarningf(owner, "TLS secret %s is of a wrong type '%s', must be '%s'", tlsSecret, secretType, api_v1.SecretTypeTLS)
 		} else if secretRef.Error != nil {
-			pemFile = pemFileNameForMissingTLSSecret
+			rejectHandshake = true
 			warnings.AddWarningf(owner, "TLS secret %s is invalid: %v", tlsSecret, secretRef.Error)
 		} else {
 			pemFile = secretRef.Path
 		}
 	} else if isWildcardEnabled {
 		pemFile = pemFileNameForWildcardTLSSecret
 	} else {
-		pemFile = pemFileNameForMissingTLSSecret
+		rejectHandshake = true
 		warnings.AddWarningf(owner, "TLS termination for host '%s' requires specifying a TLS secret or configuring a global wildcard TLS secret", host)
 	}
 
 	server.SSL = true
 	server.SSLCertificate = pemFile
 	server.SSLCertificateKey = pemFile
-	if pemFile == pemFileNameForMissingTLSSecret {
-		server.SSLCiphers = "NULL"
-	}
+	server.SSLRejectHandshake = rejectHandshake
 
 	return warnings
 }

diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go
@@ -84,16 +84,16 @@ func TestGenerateNginxCfgWithMissingTLSSecret(t *testing.T) {
 	apRes := make(map[string]string)
 	result, resultWarnings := generateNginxCfg(&cafeIngressEx, apRes, false, configParams, false, false, &StaticConfigParams{}, false)
 
-	expectedCiphers := "NULL"
+	expectedSSLRejectHandshake := true
 	expectedWarnings := Warnings{
 		cafeIngressEx.Ingress: {
 			"TLS secret cafe-secret is invalid: secret doesn't exist",
 		},
 	}
 
-	resultCiphers := result.Servers[0].SSLCiphers
-	if !reflect.DeepEqual(resultCiphers, expectedCiphers) {
-		t.Errorf("generateNginxCfg returned SSLCiphers %v,  but expected %v", resultCiphers, expectedCiphers)
+	resultSSLRejectHandshake := result.Servers[0].SSLRejectHandshake
+	if !reflect.DeepEqual(resultSSLRejectHandshake, expectedSSLRejectHandshake) {
+		t.Errorf("generateNginxCfg returned SSLRejectHandshake %v,  but expected %v", resultSSLRejectHandshake, expectedSSLRejectHandshake)
 	}
 	if diff := cmp.Diff(expectedWarnings, resultWarnings); diff != "" {
 		t.Errorf("generateNginxCfg returned unexpected result (-want +got):\n%s", diff)
@@ -965,10 +965,8 @@ func TestAddSSLConfig(t *testing.T) {
 			},
 			isWildcardEnabled: false,
 			expectedServer: version1.Server{
-				SSL:               true,
-				SSLCertificate:    pemFileNameForMissingTLSSecret,
-				SSLCertificateKey: pemFileNameForMissingTLSSecret,
-				SSLCiphers:        "NULL",
+				SSL:                true,
+				SSLRejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: {
@@ -995,10 +993,8 @@ func TestAddSSLConfig(t *testing.T) {
 			},
 			isWildcardEnabled: false,
 			expectedServer: version1.Server{
-				SSL:               true,
-				SSLCertificate:    pemFileNameForMissingTLSSecret,
-				SSLCertificateKey: pemFileNameForMissingTLSSecret,
-				SSLCiphers:        "NULL",
+				SSL:                true,
+				SSLRejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: {
@@ -1026,10 +1022,8 @@ func TestAddSSLConfig(t *testing.T) {
 			},
 			isWildcardEnabled: false,
 			expectedServer: version1.Server{
-				SSL:               true,
-				SSLCertificate:    pemFileNameForMissingTLSSecret,
-				SSLCertificateKey: pemFileNameForMissingTLSSecret,
-				SSLCiphers:        "NULL",
+				SSL:                true,
+				SSLRejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: {
@@ -1065,10 +1059,8 @@ func TestAddSSLConfig(t *testing.T) {
 			},
 			isWildcardEnabled: false,
 			expectedServer: version1.Server{
-				SSL:               true,
-				SSLCertificate:    pemFileNameForMissingTLSSecret,
-				SSLCertificateKey: pemFileNameForMissingTLSSecret,
-				SSLCiphers:        "NULL",
+				SSL:                true,
+				SSLRejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: {

diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go
@@ -69,7 +69,7 @@ type Server struct {
 	SSL                   bool
 	SSLCertificate        string
 	SSLCertificateKey     string
-	SSLCiphers            string
+	SSLRejectHandshake    bool
 	TLSPassthrough        bool
 	GRPCOnly              bool
 	StatusZone            string

diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl
@@ -41,10 +41,11 @@ server {
 	listen {{$port}} ssl{{if $server.HTTP2}} http2{{end}}{{if $server.ProxyProtocol}} proxy_protocol{{end}};
 	{{- end}}
 	{{end}}
+	{{if $server.SSLRejectHandshake}}
+	ssl_reject_handshake on;
+	{{else}}
 	ssl_certificate {{$server.SSLCertificate}};
 	ssl_certificate_key {{$server.SSLCertificateKey}};
-	{{if $server.SSLCiphers}}
-	ssl_ciphers {{$server.SSLCiphers}};
 	{{end}}
 	{{end}}
 	{{end}}

diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl
@@ -26,10 +26,11 @@ server {
 	listen {{$port}} ssl{{if $server.HTTP2}} http2{{end}}{{if $server.ProxyProtocol}} proxy_protocol{{end}};
 	{{- end}}
 	{{end}}
+	{{if $server.SSLRejectHandshake}}
+	ssl_reject_handshake on;
+	{{else}}
 	ssl_certificate {{$server.SSLCertificate}};
 	ssl_certificate_key {{$server.SSLCertificateKey}};
-	{{if $server.SSLCiphers}}
-	ssl_ciphers {{$server.SSLCiphers}};
 	{{end}}
 	{{end}}
 

diff --git a/internal/configs/version1/templates_test.go b/internal/configs/version1/templates_test.go
@@ -51,7 +51,6 @@ var ingCfg = IngressNginxConfig{
 			SSL:               true,
 			SSLCertificate:    "secret.pem",
 			SSLCertificateKey: "secret.pem",
-			SSLCiphers:        "NULL",
 			SSLPorts:          []int{443},
 			SSLRedirect:       true,
 			Locations: []Location{

diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go
@@ -78,10 +78,10 @@ type Server struct {
 
 // SSL defines SSL configuration for a server.
 type SSL struct {
-	HTTP2          bool
-	Certificate    string
-	CertificateKey string
-	Ciphers        string
+	HTTP2           bool
+	Certificate     string
+	CertificateKey  string
+	RejectHandshake bool
 }
 
 // IngressMTLS defines TLS configuration for a server. This is a subset of TLS specifically for clients auth.

diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl
@@ -89,11 +89,11 @@ server {
     listen 443 ssl{{ if $ssl.HTTP2 }} http2{{ end }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};
         {{ end }}
 
+        {{ if $ssl.RejectHandshake }}
+    ssl_reject_handshake on;
+        {{ else }}
     ssl_certificate {{ $ssl.Certificate }};
     ssl_certificate_key {{ $ssl.CertificateKey }};
-
-        {{ if $ssl.Ciphers }}
-    ssl_ciphers {{ $ssl.Ciphers }};
         {{ end }}
     {{ end }}
 

diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl
@@ -58,11 +58,11 @@ server {
     listen 443 ssl{{ if $ssl.HTTP2 }} http2{{ end }}{{ if $s.ProxyProtocol }} proxy_protocol{{ end }};
         {{ end }}
 
+        {{ if $ssl.RejectHandshake }}
+    ssl_reject_handshake on;
+        {{ else }}
     ssl_certificate {{ $ssl.Certificate }};
     ssl_certificate_key {{ $ssl.CertificateKey }};
-
-        {{ if $ssl.Ciphers }}
-    ssl_ciphers {{ $ssl.Ciphers }};
         {{ end }}
     {{ end }}
 

diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go
@@ -113,7 +113,6 @@ var virtualServerCfg = VirtualServerConfig{
 			HTTP2:          true,
 			Certificate:    "cafe-secret.pem",
 			CertificateKey: "cafe-secret.pem",
-			Ciphers:        "NULL",
 		},
 		TLSRedirect: &TLSRedirect{
 			BasedOn: "$scheme",

diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go
@@ -1996,24 +1996,22 @@ func (vsc *virtualServerConfigurator) generateSSLConfig(owner runtime.Object, tl
 		secretType = secretRef.Secret.Type
 	}
 	var name string
-	var ciphers string
+	var rejectHandshake bool
 	if secretType != "" && secretType != api_v1.SecretTypeTLS {
-		name = pemFileNameForMissingTLSSecret
-		ciphers = "NULL"
+		rejectHandshake = true
 		vsc.addWarningf(owner, "TLS secret %s is of a wrong type '%s', must be '%s'", tls.Secret, secretType, api_v1.SecretTypeTLS)
 	} else if secretRef.Error != nil {
-		name = pemFileNameForMissingTLSSecret
-		ciphers = "NULL"
+		rejectHandshake = true
 		vsc.addWarningf(owner, "TLS secret %s is invalid: %v", tls.Secret, secretRef.Error)
 	} else {
 		name = secretRef.Path
 	}
 
 	ssl := version2.SSL{
-		HTTP2:          cfgParams.HTTP2,
-		Certificate:    name,
-		CertificateKey: name,
-		Ciphers:        ciphers,
+		HTTP2:           cfgParams.HTTP2,
+		Certificate:     name,
+		CertificateKey:  name,
+		RejectHandshake: rejectHandshake,
 	}
 
 	return &ssl

diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go
@@ -4010,10 +4010,8 @@ func TestGenerateSSLConfig(t *testing.T) {
 				},
 			},
 			expectedSSL: &version2.SSL{
-				HTTP2:          false,
-				Certificate:    pemFileNameForMissingTLSSecret,
-				CertificateKey: pemFileNameForMissingTLSSecret,
-				Ciphers:        "NULL",
+				HTTP2:           false,
+				RejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: []string{"TLS secret secret is invalid: secret doesn't exist"},
@@ -4033,10 +4031,8 @@ func TestGenerateSSLConfig(t *testing.T) {
 				},
 			},
 			expectedSSL: &version2.SSL{
-				HTTP2:          false,
-				Certificate:    pemFileNameForMissingTLSSecret,
-				CertificateKey: pemFileNameForMissingTLSSecret,
-				Ciphers:        "NULL",
+				HTTP2:           false,
+				RejectHandshake: true,
 			},
 			expectedWarnings: Warnings{
 				nil: []string{"TLS secret secret is of a wrong type 'nginx.org/ca', must be 'kubernetes.io/tls'"},
@@ -4057,10 +4053,10 @@ func TestGenerateSSLConfig(t *testing.T) {
 			},
 			inputCfgParams: &ConfigParams{},
 			expectedSSL: &version2.SSL{
-				HTTP2:          false,
-				Certificate:    "secret.pem",
-				CertificateKey: "secret.pem",
-				Ciphers:        "",
+				HTTP2:           false,
+				Certificate:     "secret.pem",
+				CertificateKey:  "secret.pem",
+				RejectHandshake: false,
 			},
 			expectedWarnings: Warnings{},
 			msg:              "normal case with HTTPS",