makes optional_no_ca bypass proxy side validations

v0.8 refactor changed the `auth-tls-verify-client == "optional_no_ca"`
behavior, it used to ignore proxy side validations of the client crt.
Such validation is reimplemented now.

This should be merged only up to v0.14 due to potentially make
misconfigured clusters insecure.

CHANGELOG/CHANGELOG-v0.14.md

@@ -43,6 +43,7 @@ Highlights of this version
 Breaking backward compatibility from v0.13:
 
 * Default `auth-tls-strict` configuration key value changed from `false` to `true`. This update will change the behavior of misconfigured client auth configurations: when `false` misconfigured mTLS send requests to the backend without any authentication, when `true` misconfigured mTLS will always fail the request. See also the [auth TLS documentation](https://haproxy-ingress.github.io/v0.14/docs/configuration/keys/#auth-tls).
+* `auth-tls-verify-client`, when configured as `optional_no_ca`, used to validate client certificates against the configured CA bundle. This happens on controller versions from v0.8 to v0.13. Since v0.14 `optional_no_ca` will bypass certificate validation. Change `auth-tls-verify-client` to `optional` in order to preserve the old behavior.
 * Default `--watch-gateway` command-line option changed from `false` to `true`. On v0.13 this option can only be enabled if the Gateway API CRDs are installed, otherwise the controller would refuse to start. Since v0.14 the controller will always check if the CRDs are installed. This will change the behavior on clusters that has Gateway API resources and doesn't declare the command-line option: v0.13 would ignore the resources and v0.14 would find and apply them. See also the [watch gateway documentation](https://haproxy-ingress.github.io/v0.14/docs/configuration/command-line/#watch-gateway).
 * All the response payload managed by the controller using Lua script was rewritten in a backward compatible behavior, however deployments that overrides the `services.lua` script might break. See the [HTTP Responses](https://haproxy-ingress.github.io/v0.14/docs/configuration/keys/#http-response) documentation on how to customize HTTP responses using controller's configuration keys.
 * Two frontends changed their names, which can break deployments that uses the frontend name on metrics, logging, or in the `config-proxy` global configuration key. Frontends changed are: `_front_https`, changed its name to `_front_https__local` if at least one ssl-passthrough is configured, and `_front__auth`, changed its default value to `_front__auth__local`. These changes were made to make the metric's dashboard consistent despite the ssl-passthrough configuration. See the new [metrics example page](https://haproxy-ingress.github.io/v0.14/docs/examples/metrics/) and update your dashboard if using HAProxy Ingress' one.

docs/content/en/docs/configuration/keys.md

@@ -891,7 +891,7 @@ The following keys are supported:
 * `auth-tls-error-page`: Optional URL of the page to redirect the user if he doesn't provide a certificate or the certificate is invalid.
 * `auth-tls-secret`: Mandatory secret name with `ca.crt` key providing all certificate authority bundles used to validate client certificates. Since v0.9, an optional `ca.crl` key can also provide a CRL in PEM format for the server to verify against. A filename prefixed with `file://` can be used containing the CA bundle in PEM format, and optionally followed by a comma and the filename with the crl, eg `file:///dir/ca.pem` or `file:///dir/ca.pem,/dir/crl.pem`.
 * `auth-tls-strict`: Defines if a wrong or incomplete configuration, eg missing secret with `ca.crt`, should forbid connection attempts. If `false`, a wrong or incomplete configuration will ignore the authentication config, allowing anonymous connection. If `true`, a strict configuration is used: all requests will be rejected with HTTP 495 or 496, or redirected to the error page if configured, until a proper `ca.crt` is provided. Strict configuration will only be used if `auth-tls-secret` has a secret name and `auth-tls-verify-client` is missing or is not configured as `off`. This options used to have `false` as the default value up to v0.13, changing its default to `true` since v0.14 to improve security.
-* `auth-tls-verify-client`: Optional configuration of Client Verification behavior. Supported values are `off`, `on`, `optional` and `optional_no_ca`. The default value is `on` if a valid secret is provided, `off` otherwise.
+* `auth-tls-verify-client`: Optional configuration of Client Verification behavior. Supported values are `off`, `on`, `optional` and `optional_no_ca`. The default value is `on` if a valid secret is provided, `off` otherwise. `optional` makes the certificate optional but validates it when provided by the client. From v0.8 to v0.13 controller versions, `optional_no_ca` used to validate the certificate as well, since v0.14 it makes the proxy bypass any validation.
 * `ssl-fingerprint-lower`: Defines if the certificate fingerprint should be in lowercase hexadecimal digits. The default value is `false`, which uses uppercase digits.
 * `ssl-fingerprint-sha2-bits`: Defines the number of bits of the SHA-2 fingerprint of the client certificate. Valid values are `224`, `256`, `384` or `512`. The header `X-SSL-Client-SHA2` will only be added if this option is declared.
 * `ssl-headers-prefix`: Configures which prefix should be used on HTTP headers. Since [RFC 6648](https://tools.ietf.org/html/rfc6648) `X-` prefix on unstandardized headers changed from a convention to deprecation. This configuration allows to select which pattern should be used on header names.

pkg/converters/ingress/annotations/host.go

@@ -51,7 +51,18 @@ func (c *updater) setAuthTLSConfig(mapper *Mapper, target *types.TLSConfig, host
 		tls.CAFilename = c.fakeCA.Filename
 		tls.CAHash = c.fakeCA.SHA1Hash
 	}
-	tls.CAVerifyOptional = verify.Value == "optional" || verify.Value == "optional_no_ca"
+	switch verify.Value {
+	case "optional_no_ca":
+		tls.CAVerify = types.CAVerifySkipCheck
+	case "optional":
+		tls.CAVerify = types.CAVerifyOptional
+	case "on":
+		tls.CAVerify = types.CAVerifyAlways
+	default:
+		if tls.CAFilename != "" {
+			tls.CAVerify = types.CAVerifyAlways
+		}
+	}
 	return true
 }
 

pkg/converters/ingress/annotations/host_test.go

@@ -147,6 +147,7 @@ func TestTLSConfig(t *testing.T) {
 				TLSConfig: hatypes.TLSConfig{
 					CAFilename: fakeCAFilename,
 					CAHash:     fakeCAHash,
+					CAVerify:   hatypes.CAVerifyAlways,
 				},
 			},
 			logging: "ERROR error building TLS auth config on ingress 'system/ing1': secret not found: 'system/caerr'",
@@ -160,6 +161,7 @@ func TestTLSConfig(t *testing.T) {
 				TLSConfig: hatypes.TLSConfig{
 					CAFilename: "/path/ca.crt",
 					CAHash:     "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
+					CAVerify:   hatypes.CAVerifyAlways,
 				},
 			},
 		},
@@ -185,9 +187,9 @@ func TestTLSConfig(t *testing.T) {
 			},
 			expected: hatypes.HostTLSConfig{
 				TLSConfig: hatypes.TLSConfig{
-					CAFilename:       fakeCAFilename,
-					CAHash:           fakeCAHash,
-					CAVerifyOptional: true,
+					CAFilename: fakeCAFilename,
+					CAHash:     fakeCAHash,
+					CAVerify:   hatypes.CAVerifyOptional,
 				}},
 			logging: "ERROR error building TLS auth config on ingress 'system/ing1': secret not found: 'system/caerr'",
 		},
@@ -200,9 +202,9 @@ func TestTLSConfig(t *testing.T) {
 			},
 			expected: hatypes.HostTLSConfig{
 				TLSConfig: hatypes.TLSConfig{
-					CAFilename:       "/path/ca.crt",
-					CAHash:           "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
-					CAVerifyOptional: true,
+					CAFilename: "/path/ca.crt",
+					CAHash:     "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
+					CAVerify:   hatypes.CAVerifyOptional,
 				}},
 		},
 		// 9
@@ -213,19 +215,32 @@ func TestTLSConfig(t *testing.T) {
 			},
 			expected: hatypes.HostTLSConfig{
 				TLSConfig: hatypes.TLSConfig{
-					CAFilename:       "/path/ca.crt",
-					CAHash:           "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
-					CAVerifyOptional: true,
+					CAFilename: "/path/ca.crt",
+					CAHash:     "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
+					CAVerify:   hatypes.CAVerifyOptional,
 				}},
 		},
 		// 10
+		{
+			ann: map[string]string{
+				ingtypes.HostAuthTLSSecret:       "cafile",
+				ingtypes.HostAuthTLSVerifyClient: "optional_no_ca",
+			},
+			expected: hatypes.HostTLSConfig{
+				TLSConfig: hatypes.TLSConfig{
+					CAFilename: "/path/ca.crt",
+					CAHash:     "c0e1bf73caf75d7353cf3ecdd20ceb2f6fa1cab1",
+					CAVerify:   hatypes.CAVerifySkipCheck,
+				}},
+		},
+		// 11
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLCiphers: "some-cipher-1:some-cipher-2",
 			},
 			expected: hatypes.HostTLSConfig{},
 		},
-		// 11
+		// 12
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLCiphers: "some-cipher-1:some-cipher-2",
@@ -238,14 +253,14 @@ func TestTLSConfig(t *testing.T) {
 					Ciphers: "some-cipher-2:some-cipher-3",
 				}},
 		},
-		// 12
+		// 13
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLCipherSuites: "some-cipher-1:some-cipher-2",
 			},
 			expected: hatypes.HostTLSConfig{},
 		},
-		// 13
+		// 14
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLCipherSuites: "some-cipher-suite-1:some-cipher-suite-2",
@@ -258,14 +273,14 @@ func TestTLSConfig(t *testing.T) {
 					CipherSuites: "some-cipher-suite-2:some-cipher-suite-3",
 				}},
 		},
-		// 14
+		// 15
 		{
 			annDefault: map[string]string{
 				ingtypes.HostTLSALPN: "h2,http/1.1",
 			},
 			expected: hatypes.HostTLSConfig{},
 		},
-		// 15
+		// 16
 		{
 			annDefault: map[string]string{
 				ingtypes.HostTLSALPN: "h2,http/1.1",
@@ -278,7 +293,7 @@ func TestTLSConfig(t *testing.T) {
 					ALPN: "h2",
 				}},
 		},
-		// 16
+		// 17
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLOptionsHost: "ssl-min-ver TLSv1.2",
@@ -288,7 +303,7 @@ func TestTLSConfig(t *testing.T) {
 					Options: "ssl-min-ver TLSv1.2",
 				}},
 		},
-		// 17
+		// 18
 		{
 			annDefault: map[string]string{
 				ingtypes.HostSSLOptionsHost: "ssl-min-ver TLSv1.2",

pkg/haproxy/config.go

@@ -257,14 +257,16 @@ func (c *config) WriteFrontendMaps() error {
 			fmaps.RedirFromMap.AddHostnameMappingRegex(host.Redirect.RedirectHostRegex, host.Hostname)
 		}
 		if host.HasTLSAuth() {
-			fmaps.TLSAuthList.AddHostnameMapping(host.Hostname, "")
-			if !host.TLS.CAVerifyOptional {
+			if host.TLS.CAVerify != hatypes.CAVerifySkipCheck {
+				fmaps.TLSAuthList.AddHostnameMapping(host.Hostname, "")
+			}
+			if !host.TLS.CAVerifyOptional() {
 				fmaps.TLSNeedCrtList.AddHostnameMapping(host.Hostname, "")
 			}
 			page := host.TLS.CAErrorPage
 			if page != "" {
 				fmaps.TLSInvalidCrtPagesMap.AddHostnameMapping(host.Hostname, page)
-				if !host.TLS.CAVerifyOptional {
+				if !host.TLS.CAVerifyOptional() {
 					fmaps.TLSMissingCrtPagesMap.AddHostnameMapping(host.Hostname, page)
 				}
 			}

pkg/haproxy/instance_test.go

@@ -2057,9 +2057,9 @@ func TestInstanceTCPServices(t *testing.T) {
 			port:    7008,
 			backend: b.BackendID(),
 			tls: hatypes.TLSConfig{
-				TLSFilename:      "/ssl/7008.pem",
-				CAFilename:       "/ssl/ca-7008.pem",
-				CAVerifyOptional: true,
+				TLSFilename: "/ssl/7008.pem",
+				CAFilename:  "/ssl/ca-7008.pem",
+				CAVerify:    hatypes.CAVerifySkipCheck,
 			},
 		},
 		{
@@ -2178,7 +2178,7 @@ frontend _front_tcp_7007
     mode tcp
     default_backend d1_app_8080
 frontend _front_tcp_7008
-    bind :7008 ssl crt /ssl/7008.pem ca-file /ssl/ca-7008.pem verify optional
+    bind :7008 ssl crt /ssl/7008.pem ca-file /ssl/ca-7008.pem verify optional ca-ignore-err all crt-ignore-err all
     mode tcp
     default_backend d1_app_8080
 frontend _front_tcp_7009
@@ -5079,7 +5079,7 @@ func TestInstanceWildcardHostname(t *testing.T) {
 	h.AddPath(b, "/", hatypes.MatchBegin)
 	h.TLS.CAFilename = "/var/haproxy/ssl/ca/d1.local.pem"
 	h.TLS.CAHash = "1"
-	h.TLS.CAVerifyOptional = true
+	h.TLS.CAVerify = hatypes.CAVerifyOptional
 	h.TLS.CAErrorPage = "http://sub.d1.local/error.html"
 	for _, path := range b.Paths {
 		path.SSLRedirect = true

pkg/haproxy/types/host.go

@@ -445,6 +445,11 @@ func (h *Host) String() string {
 	return fmt.Sprintf("%+v", *h)
 }
 
+// CAVerifyOptional ...
+func (tls *TLSConfig) CAVerifyOptional() bool {
+	return tls.CAVerify == CAVerifyOptional || tls.CAVerify == CAVerifySkipCheck
+}
+
 // HasTLS ...
 func (h *HostTLSConfig) HasTLS() bool {
 	return h.TLSFilename != ""

pkg/haproxy/types/types.go

@@ -290,21 +290,31 @@ type TCPServiceHost struct {
 	Backend  BackendID
 }
 
+// CAVerify ...
+type CAVerify string
+
+// ...
+const (
+	CAVerifySkipCheck CAVerify = "skip-check"
+	CAVerifyOptional  CAVerify = "optional"
+	CAVerifyAlways    CAVerify = "always"
+)
+
 // TLSConfig ...
 type TLSConfig struct {
-	ALPN             string
-	CAFilename       string
-	CAHash           string
-	CAVerifyOptional bool
-	Ciphers          string
-	CipherSuites     string
-	CRLFilename      string
-	CRLHash          string
-	Options          string
-	TLSCommonName    string
-	TLSFilename      string
-	TLSHash          string
-	TLSNotAfter      time.Time
+	ALPN          string
+	CAFilename    string
+	CAHash        string
+	CAVerify      CAVerify
+	Ciphers       string
+	CipherSuites  string
+	CRLFilename   string
+	CRLHash       string
+	Options       string
+	TLSCommonName string
+	TLSFilename   string
+	TLSHash       string
+	TLSNotAfter   time.Time
 }
 
 // TCPBackends ...

rootfs/etc/templates/haproxy/haproxy.tmpl

@@ -1010,6 +1010,7 @@ frontend {{ $proxy_name }}
             {{- if $tls.ALPN }} alpn {{ $tls.ALPN }}{{ end }}
             {{- if $tls.CAFilename }}
                 {{- "" }} ca-file {{ $tls.CAFilename }} verify {{ if $tls.CAVerifyOptional}}optional{{ else }}required{{ end }}
+                {{- if (eq $tls.CAVerify "skip-check") }} ca-ignore-err all crt-ignore-err all{{ end }}
                 {{- if $tls.CRLFilename }} crl-file {{ $tls.CRLFilename }}{{ end }}
             {{- end }}
             {{- if $tls.Ciphers }} ciphers {{ $tls.Ciphers }}{{ end }}