From 19ef9b3db85bc19aed9b6f71f6a3e34d2e0cb9f1 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Sat, 24 Dec 2022 11:28:16 -0300 Subject: [PATCH] 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 | 1 + docs/content/en/docs/configuration/keys.md | 2 +- pkg/converters/ingress/annotations/host.go | 13 ++++- .../ingress/annotations/host_test.go | 47 ++++++++++++------- pkg/haproxy/config.go | 8 ++-- pkg/haproxy/instance_test.go | 10 ++-- pkg/haproxy/types/host.go | 5 ++ pkg/haproxy/types/types.go | 36 +++++++++----- rootfs/etc/templates/haproxy/haproxy.tmpl | 1 + 9 files changed, 84 insertions(+), 39 deletions(-) diff --git a/CHANGELOG/CHANGELOG-v0.14.md b/CHANGELOG/CHANGELOG-v0.14.md index fa756f9dc..55f6f6322 100644 --- a/CHANGELOG/CHANGELOG-v0.14.md +++ b/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. diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md index ce737bad7..418e5ce57 100644 --- a/docs/content/en/docs/configuration/keys.md +++ b/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. diff --git a/pkg/converters/ingress/annotations/host.go b/pkg/converters/ingress/annotations/host.go index 41c032d29..b49488e71 100644 --- a/pkg/converters/ingress/annotations/host.go +++ b/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 } diff --git a/pkg/converters/ingress/annotations/host_test.go b/pkg/converters/ingress/annotations/host_test.go index 81aee14d2..3e3baac48 100644 --- a/pkg/converters/ingress/annotations/host_test.go +++ b/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", diff --git a/pkg/haproxy/config.go b/pkg/haproxy/config.go index 1d0f0dafa..6c5bbc60a 100644 --- a/pkg/haproxy/config.go +++ b/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) } } diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go index 74db6b02e..f2937568e 100644 --- a/pkg/haproxy/instance_test.go +++ b/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 diff --git a/pkg/haproxy/types/host.go b/pkg/haproxy/types/host.go index 19432756f..0babcac6f 100644 --- a/pkg/haproxy/types/host.go +++ b/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 != "" diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go index 65f67ab84..e9dcd6ebb 100644 --- a/pkg/haproxy/types/types.go +++ b/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 ... diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl index e4fa2d6c5..93a356516 100644 --- a/rootfs/etc/templates/haproxy/haproxy.tmpl +++ b/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 }}