Merge 0bd35dc388 into de1a4c463c

2025-02-17 09:50:28 -08:00 · 2025-02-17 09:50:28 -08:00 · 276c08130e
commit 276c08130e
parent de1a4c463c 0bd35dc388
13 changed files with 308 additions and 2 deletions
--- a/docs/user-guide/nginx-configuration/annotations-risk.md
+++ b/docs/user-guide/nginx-configuration/annotations-risk.md
@ -108,9 +108,11 @@
 | Redirect | temporal-redirect | Medium | location |
 | Redirect | temporal-redirect-code | Low | location |
 | Rewrite | app-root | Medium | location |
+| Rewrite | force-ssl-forbid-http | Medium | location |
 | Rewrite | force-ssl-redirect | Medium | location |
 | Rewrite | preserve-trailing-slash | Medium | location |
 | Rewrite | rewrite-target | Medium | ingress |
+| Rewrite | ssl-forbid-http | Low | location |
 | Rewrite | ssl-redirect | Low | location |
 | Rewrite | use-regex | Low | location |
 | SSLCipher | ssl-ciphers | Low | ingress |
--- a/docs/user-guide/nginx-configuration/annotations.md
+++ b/docs/user-guide/nginx-configuration/annotations.md
@ -59,6 +59,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/cors-expose-headers](#enable-cors)|string|
 |[nginx.ingress.kubernetes.io/cors-allow-credentials](#enable-cors)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/cors-max-age](#enable-cors)|number|
+|[nginx.ingress.kubernetes.io/force-ssl-forbid-http](#server-side-https-enforcement-through-forbidden-errors)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/force-ssl-redirect](#server-side-https-enforcement-through-redirect)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/from-to-www-redirect](#redirect-fromto-www)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/http2-push-preload](#http2-push-preload)|"true" or "false"|
@ -104,6 +105,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/session-cookie-path](#cookie-affinity)|string|
 |[nginx.ingress.kubernetes.io/session-cookie-samesite](#cookie-affinity)|string|"None", "Lax" or "Strict"|
 |[nginx.ingress.kubernetes.io/session-cookie-secure](#cookie-affinity)|string|
+|[nginx.ingress.kubernetes.io/ssl-forbid-http](#server-side-https-enforcement-through-forbidden-errors)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/ssl-redirect](#server-side-https-enforcement-through-redirect)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/ssl-passthrough](#ssl-passthrough)|"true" or "false"|
 |[nginx.ingress.kubernetes.io/stream-snippet](#stream-snippet)|string|
@ -627,6 +629,21 @@ This can be achieved by using the `nginx.ingress.kubernetes.io/force-ssl-redirec

 To preserve the trailing slash in the URI with `ssl-redirect`, set `nginx.ingress.kubernetes.io/preserve-trailing-slash: "true"` annotation for that particular resource.

+### Server-side HTTPS enforcement through forbidden errors
+
+In certain scenarios, you might prefer to return a 403 Forbidden error response instead of redirecting traffic to the HTTPS port.
+This approach helps prevent misconfigured clients from inadvertently leaking sensitive data over unencrypted connections.
+
+This can be enabled globally using `ssl-forbid-http: "true"` in the [ConfigMap][./configmap.md#ssl-forbid-http].
+
+To configure this feature for specific Ingress resources, you can use the `nginx.ingress.kubernetes.io/ssl-forbid-http: "true"`
+annotation in the particular resource.
+
+When using SSL off-loading outside of the cluster (e.g. AWS ELB), it may be useful to enforce 403 Forbidden errors to HTTP requests
+even when there is no TLS certificate available.
+
+This can be achieved by using the `nginx.ingress.kubernetes.io/force-ssl-forbid-http: "true"` annotation in the particular resource.
+
 ### Redirect from/to www

 In some scenarios, it is required to redirect from `www.domain.com` to `domain.com` or vice versa, which way the redirect is performed depends on the configured `host` value in the Ingress object.
--- a/docs/user-guide/nginx-configuration/configmap.md
+++ b/docs/user-guide/nginx-configuration/configmap.md
@ -189,6 +189,8 @@ The following table shows a configuration option's name, type, and the default v
 | [proxy-request-buffering](#proxy-request-buffering)                             | string       | "on"                                                                                                                                                                                                                                                                                                                                                         |                                                                                     |
 | [ssl-redirect](#ssl-redirect)                                                   | bool         | "true"                                                                                                                                                                                                                                                                                                                                                       |                                                                                     |
 | [force-ssl-redirect](#force-ssl-redirect)                                       | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
+| [ssl-forbid-http](#ssl-forbid-http)                                             | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
+| [force-ssl-forbid-http](#force-ssl-forbid-http)                                 | bool         | "false"                                                                                                                                                                                                                                                                                                                                                      |                                                                                     |
 | [denylist-source-range](#denylist-source-range)                                 | []string     | []string{}                                                                                                                                                                                                                                                                                                                                                   |                                                                                     |
 | [whitelist-source-range](#whitelist-source-range)                               | []string     | []string{}                                                                                                                                                                                                                                                                                                                                                   |                                                                                     |
 | [skip-access-log-urls](#skip-access-log-urls)                                   | []string     | []string{}                                                                                                                                                                                                                                                                                                                                                   |                                                                                     |
@ -1154,6 +1156,18 @@ _**default:**_ "true"
 Sets the global value of redirects (308) to HTTPS if the server has a default TLS certificate (defined in extra-args).
 _**default:**_ "false"

+## ssl-forbid-http
+
+Sets the global value of 403 Forbidden errors to HTTP if the server has a TLS certificate (defined in an Ingress rule).
+
+_**default:**_ "false"
+
+## force-ssl-forbid-http
+
+Sets the global value of 403 Forbidden errors to HTTP if the server has a default TLS certificate (defined in extra-args).
+
+_**default:**_ "false"
+
 ## denylist-source-range

 Sets the default denylisted IPs for each `server` block. This can be overwritten by an annotation on an Ingress rule.
--- a/docs/user-guide/tls.md
+++ b/docs/user-guide/tls.md
@ -78,21 +78,31 @@ HSTS is enabled by default.

 To disable this behavior use `hsts: "false"` in the configuration [ConfigMap][ConfigMap].

-## Server-side HTTPS enforcement through redirect
+## Server-side HTTPS enforcement

 By default the controller redirects HTTP clients to the HTTPS port
 443 using a 308 Permanent Redirect response if TLS is enabled for that Ingress.

-This can be disabled globally using `ssl-redirect: "false"` in the NGINX [config map][ConfigMap],
+This can be disabled globally using `ssl-redirect: "false"` in the [config map][ConfigMap],
 or per-Ingress with the `nginx.ingress.kubernetes.io/ssl-redirect: "false"`
 annotation in the particular resource.

+In certain scenarios, you might prefer to return a 403 Forbidden error response instead of redirecting traffic to the HTTPS port.
+This approach helps prevent misconfigured clients from inadvertently leaking sensitive data over unencrypted connections.
+
+This can be enabled globally using `ssl-forbid-http: "true"` in the [config map][ConfigMap],
+or per-Ingress with the `nginx.ingress.kubernetes.io/ssl-forbid-http: "true"` annotation in the particular resource.
+
 !!! tip
    When using SSL offloading outside of cluster (e.g. AWS ELB) it may be useful to enforce a
    redirect to HTTPS even when there is no TLS certificate available.
    This can be achieved by using the `nginx.ingress.kubernetes.io/force-ssl-redirect: "true"`
    annotation in the particular resource.

+    Similarly, you can enforce 403 Forbidden errors to HTTP requests using the
+    `nginx.ingress.kubernetes.io/force-ssl-forbid-http: "true"` annotation in the particular
+    resource.
+
 ## Automated Certificate Management with cert-manager

 [cert-manager] automatically requests missing or expired certificates from a range of 
--- a/internal/ingress/annotations/rewrite/main.go
+++ b/internal/ingress/annotations/rewrite/main.go
@ -32,6 +32,8 @@ const (
 	sslRedirectAnnotation           = "ssl-redirect"
 	preserveTrailingSlashAnnotation = "preserve-trailing-slash"
 	forceSSLRedirectAnnotation      = "force-ssl-redirect"
+	sslForbidHTTPAnnotation         = "ssl-forbid-http"
+	forceSSLForbidHTTPAnnotation    = "force-ssl-forbid-http"
 	useRegexAnnotation              = "use-regex"
 	appRootAnnotation               = "app-root"
 )
@ -64,6 +66,18 @@ var rewriteAnnotations = parser.Annotation{
 			Risk:          parser.AnnotationRiskMedium,
 			Documentation: `This annotation forces the redirection to HTTPS even if the Ingress is not TLS Enabled`,
 		},
+		sslForbidHTTPAnnotation: {
+			Validator:     parser.ValidateBool,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskLow,
+			Documentation: `This annotation defines if the location section should forbid HTTP requests`,
+		},
+		forceSSLForbidHTTPAnnotation: {
+			Validator:     parser.ValidateBool,
+			Scope:         parser.AnnotationScopeLocation,
+			Risk:          parser.AnnotationRiskMedium,
+			Documentation: `This annotation forces the forbidden error to HTTP even if the Ingress is not TLS Enabled`,
+		},
 		useRegexAnnotation: {
 			Validator: parser.ValidateBool,
 			Scope:     parser.AnnotationScopeLocation,
@ -88,6 +102,10 @@ type Config struct {
 	SSLRedirect bool `json:"sslRedirect"`
 	// ForceSSLRedirect indicates if the location section is accessible SSL only
 	ForceSSLRedirect bool `json:"forceSSLRedirect"`
+	// SSLForbidHTTP indicates if the location section is accessible SSL only
+	SSLForbidHTTP bool `json:"sslForbidHTTP"`
+	// ForceSSLForbidHTTP indicates if the location section is accessible SSL only
+	ForceSSLForbidHTTP bool `json:"forceSSLForbidHTTP"`
 	// PreserveTrailingSlash indicates if the trailing slash should be kept during a tls redirect
 	PreserveTrailingSlash bool `json:"preserveTrailingSlash"`
 	// AppRoot defines the Application Root that the Controller must redirect if it's in '/' context
@ -113,6 +131,12 @@ func (r1 *Config) Equal(r2 *Config) bool {
 	if r1.ForceSSLRedirect != r2.ForceSSLRedirect {
 		return false
 	}
+	if r1.SSLForbidHTTP != r2.SSLForbidHTTP {
+		return false
+	}
+	if r1.ForceSSLForbidHTTP != r2.ForceSSLForbidHTTP {
+		return false
+	}
 	if r1.AppRoot != r2.AppRoot {
 		return false
 	}
@ -172,6 +196,22 @@ func (a rewrite) Parse(ing *networking.Ingress) (interface{}, error) {
 		config.ForceSSLRedirect = a.r.GetDefaultBackend().ForceSSLRedirect
 	}

+	config.SSLForbidHTTP, err = parser.GetBoolAnnotation(sslForbidHTTPAnnotation, ing, a.annotationConfig.Annotations)
+	if err != nil {
+		if errors.IsValidationError(err) {
+			klog.Warningf("%s is invalid, defaulting to '%t'", sslForbidHTTPAnnotation, a.r.GetDefaultBackend().SSLForbidHTTP)
+		}
+		config.SSLForbidHTTP = a.r.GetDefaultBackend().SSLForbidHTTP
+	}
+
+	config.ForceSSLForbidHTTP, err = parser.GetBoolAnnotation(forceSSLForbidHTTPAnnotation, ing, a.annotationConfig.Annotations)
+	if err != nil {
+		if errors.IsValidationError(err) {
+			klog.Warningf("%s is invalid, defaulting to '%t'", forceSSLForbidHTTPAnnotation, a.r.GetDefaultBackend().ForceSSLForbidHTTP)
+		}
+		config.ForceSSLForbidHTTP = a.r.GetDefaultBackend().ForceSSLForbidHTTP
+	}
+
 	config.UseRegex, err = parser.GetBoolAnnotation(useRegexAnnotation, ing, a.annotationConfig.Annotations)
 	if err != nil {
 		if errors.IsValidationError(err) {
--- a/internal/ingress/annotations/rewrite/main_test.go
+++ b/internal/ingress/annotations/rewrite/main_test.go
@ -213,6 +213,70 @@ func TestForceSSLRedirect(t *testing.T) {
 	}
 }

+func TestSSLForbidHTTP(t *testing.T) {
+	ing := buildIngress()
+
+	i, err := NewParser(mockBackend{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	forbid, ok := i.(*Config)
+	if !ok {
+		t.Errorf("expected a Forbid type")
+	}
+	if forbid.SSLForbidHTTP {
+		t.Errorf("Expected false but returned true")
+	}
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("ssl-forbid-http")] = "true"
+	ing.SetAnnotations(data)
+
+	i, err = NewParser(mockBackend{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	forbid, ok = i.(*Config)
+	if !ok {
+		t.Errorf("expected a Forbid type")
+	}
+	if !forbid.SSLForbidHTTP {
+		t.Errorf("Expected true but returned false")
+	}
+}
+
+func TestForceSSLForbidHTTP(t *testing.T) {
+	ing := buildIngress()
+
+	i, err := NewParser(mockBackend{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	forbid, ok := i.(*Config)
+	if !ok {
+		t.Errorf("expected a Forbid type")
+	}
+	if forbid.ForceSSLForbidHTTP {
+		t.Errorf("Expected false but returned true")
+	}
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("force-ssl-forbid-http")] = "true"
+	ing.SetAnnotations(data)
+
+	i, err = NewParser(mockBackend{}).Parse(ing)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	forbid, ok = i.(*Config)
+	if !ok {
+		t.Errorf("expected a Forbid type")
+	}
+	if !forbid.ForceSSLForbidHTTP {
+		t.Errorf("Expected true but returned false")
+	}
+}
+
 func TestAppRoot(t *testing.T) {
 	ap := NewParser(mockBackend{redirect: true})

--- a/internal/ingress/controller/template/template.go
+++ b/internal/ingress/controller/template/template.go
@ -435,6 +435,8 @@ func locationConfigForLua(l, a interface{}) string {
 		force_ssl_redirect = string_to_bool(ngx.var.force_ssl_redirect),
 	    ssl_redirect = string_to_bool(ngx.var.ssl_redirect),
 	    force_no_ssl_redirect = string_to_bool(ngx.var.force_no_ssl_redirect),
+	    force_ssl_forbid_http = string_to_bool(ngx.var.force_ssl_forbid_http),
+	    ssl_forbid_http = string_to_bool(ngx.var.ssl_forbid_http),
 	    preserve_trailing_slash = string_to_bool(ngx.var.preserve_trailing_slash),
 	    use_port_in_redirects = string_to_bool(ngx.var.use_port_in_redirects),
 	*/
@ -443,12 +445,16 @@ func locationConfigForLua(l, a interface{}) string {
 	    set $force_ssl_redirect "%t";
 	    set $ssl_redirect "%t";
 	    set $force_no_ssl_redirect "%t";
+	    set $force_ssl_forbid_http "%t";
+	    set $ssl_forbid_http "%t";
 	    set $preserve_trailing_slash "%t";
 	    set $use_port_in_redirects "%t";
 	`,
 		location.Rewrite.ForceSSLRedirect,
 		location.Rewrite.SSLRedirect,
 		isLocationInLocationList(l, all.Cfg.NoTLSRedirectLocations),
+		location.Rewrite.ForceSSLForbidHTTP,
+		location.Rewrite.SSLForbidHTTP,
 		location.Rewrite.PreserveTrailingSlash,
 		location.UsePortInRedirects,
 	)
--- a/internal/ingress/defaults/main.go
+++ b/internal/ingress/defaults/main.go
@ -126,6 +126,13 @@ type Backend struct {
 	// This is useful if doing SSL offloading outside of cluster eg AWS ELB
 	ForceSSLRedirect bool `json:"force-ssl-redirect"`

+	// Enables or disables forbidden errors (403) to HTTP
+	SSLForbidHTTP bool `json:"ssl-forbid-http"`
+
+	// Enables or disables forbidden errors (403) to HTTP even without TLS cert
+	// This is useful if doing SSL offloading outside of cluster eg AWS ELB
+	ForceSSLForbidHTTP bool `json:"force-ssl-forbid-http"`
+
 	// Enables or disables the specification of port in redirects
 	// Default: false
 	UsePortInRedirects bool `json:"use-port-in-redirects"`
--- a/rootfs/etc/nginx/lua/lua_ingress.lua
+++ b/rootfs/etc/nginx/lua/lua_ingress.lua
@ -62,6 +62,18 @@ local function randomseed()
  math.randomseed(seed)
 end

+local function forbid_http(location_config)
+  if location_config.force_ssl_forbid_http and ngx.var.pass_access_scheme == "http" then
+    return true
+  end
+
+  if ngx.var.pass_access_scheme ~= "http" then
+    return false
+  end
+
+  return location_config.ssl_forbid_http and certificate_configured_for_current_request()
+end
+
 local function redirect_to_https(location_config)
  if location_config.force_no_ssl_redirect then
    return false
@ -115,6 +127,8 @@ function _M.rewrite()
    force_ssl_redirect = string_to_bool(ngx.var.force_ssl_redirect),
    ssl_redirect = string_to_bool(ngx.var.ssl_redirect),
    force_no_ssl_redirect = string_to_bool(ngx.var.force_no_ssl_redirect),
+    force_ssl_forbid_http = string_to_bool(ngx.var.force_ssl_forbid_http),
+    ssl_forbid_http = string_to_bool(ngx.var.ssl_forbid_http),
    preserve_trailing_slash = string_to_bool(ngx.var.preserve_trailing_slash),
    use_port_in_redirects = string_to_bool(ngx.var.use_port_in_redirects),
  }
@ -154,6 +168,10 @@ function _M.rewrite()
    ngx.var.pass_port = 443
  end

+  if forbid_http(location_config) then
+    ngx.exit(ngx.HTTP_FORBIDDEN)
+  end
+
  if redirect_to_https(location_config) then
    local request_uri = ngx.var.request_uri
    -- do not append a trailing slash on redirects unless enabled by annotations
--- a/test/data/cleanConf.expected.conf
+++ b/test/data/cleanConf.expected.conf
@ -115,6 +115,8 @@ http {
 					force_ssl_redirect = false,
 					ssl_redirect = false,
 					force_no_ssl_redirect = false,
+					force_ssl_forbid_http = false,
+					ssl_forbid_http = false,
 					use_port_in_redirects = false,
 				})
 				balancer.rewrite()
--- a/test/data/cleanConf.src.conf
+++ b/test/data/cleanConf.src.conf
@ -158,6 +158,8 @@ lua_shared_dict ocsp_response_cache 5M;
 		force_ssl_redirect = false,
 		ssl_redirect = false,
 		force_no_ssl_redirect = false,
+		force_ssl_forbid_http = false,
+		ssl_forbid_http = false,
 		use_port_in_redirects = false,
 	})
                balancer.rewrite()
--- a/test/e2e/annotations/forcesslforbidhttp.go
+++ b/test/e2e/annotations/forcesslforbidhttp.go
@ -0,0 +1,51 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package annotations
+
+import (
+	"net/http"
+
+	"github.com/onsi/ginkgo/v2"
+
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.DescribeAnnotation("force-ssl-forbid-http", func() {
+	f := framework.NewDefaultFramework("forcesslforbidhttp")
+
+	ginkgo.BeforeEach(func() {
+		f.NewEchoDeployment()
+	})
+
+	ginkgo.It("should send forbidden errors for http", func() {
+		host := "forcesslforbid.bar.com"
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/force-ssl-forbid-http": "true",
+			"nginx.ingress.kubernetes.io/force-ssl-redirect":    "true",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			Expect().
+			Status(http.StatusForbidden)
+	})
+})
--- a/test/e2e/annotations/sslforbidhttp.go
+++ b/test/e2e/annotations/sslforbidhttp.go
@ -0,0 +1,73 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package annotations
+
+import (
+	"fmt"
+	"net/http"
+
+	"github.com/onsi/ginkgo/v2"
+
+	"k8s.io/ingress-nginx/test/e2e/framework"
+)
+
+var _ = framework.DescribeAnnotation("ssl-forbid-http", func() {
+	f := framework.NewDefaultFramework("sslforbidhttp")
+
+	ginkgo.BeforeEach(func() {
+		f.NewEchoDeployment()
+	})
+
+	ginkgo.It("should send forbidden errors for http when tls is present", func() {
+		host := "sslforbid.bar.com"
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/ssl-forbid-http": "true",
+			"nginx.ingress.kubernetes.io/ssl-redirect":    "true",
+		}
+
+		ing := framework.NewSingleIngressWithTLS(host, "/", host, []string{host}, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			Expect().
+			Status(http.StatusForbidden)
+	})
+
+	ginkgo.It("should pass through for http when tls is absent", func() {
+		host := "sslforbidnotls.bar.com"
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/ssl-forbid-http": "true",
+			"nginx.ingress.kubernetes.io/ssl-redirect":    "true",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, annotations)
+		f.EnsureIngress(ing)
+
+		expectBodyRequestURI := fmt.Sprintf("request_uri=http://%v:80", host)
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", host).
+			Expect().
+			Status(http.StatusOK).
+			Body().Contains(expectBodyRequestURI)
+	})
+})