From c07a48a3eccbd7b23026f72136d3392bbc6f795a Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 4 Jul 2023 13:48:04 +0200 Subject: [PATCH] security: fix CVE-2023-36456 Signed-off-by: Jens Langhammer # Conflicts: # website/sidebars.js --- authentik/lib/default.yml | 11 +++++- authentik/lib/utils/http.py | 6 ++- internal/config/struct.go | 15 ++++---- internal/utils/web/http_forwarded.go | 44 ++++++++++++++++++++++ internal/web/web.go | 2 +- website/docs/installation/configuration.md | 5 +++ website/docs/security/CVE-2023-36456.md | 21 +++++++++++ website/sidebars.js | 3 +- 8 files changed, 94 insertions(+), 13 deletions(-) create mode 100644 internal/utils/web/http_forwarded.go create mode 100644 website/docs/security/CVE-2023-36456.md diff --git a/authentik/lib/default.yml b/authentik/lib/default.yml index 1617f445d..f128234e7 100644 --- a/authentik/lib/default.yml +++ b/authentik/lib/default.yml @@ -5,18 +5,25 @@ postgresql: name: authentik user: authentik port: 5432 - password: 'env://POSTGRES_PASSWORD' + password: "env://POSTGRES_PASSWORD" use_pgbouncer: false listen: listen_http: 0.0.0.0:9000 listen_https: 0.0.0.0:9443 listen_metrics: 0.0.0.0:9300 + trusted_proxy_cidrs: + - 127.0.0.0/8 + - 10.0.0.0/8 + - 172.16.0.0/12 + - 192.168.0.0/16 + - fe80::/10 + - ::1/128 redis: host: localhost port: 6379 - password: '' + password: "" tls: false tls_reqs: "none" db: 0 diff --git a/authentik/lib/utils/http.py b/authentik/lib/utils/http.py index d20ad29eb..bce4fdbaa 100644 --- a/authentik/lib/utils/http.py +++ b/authentik/lib/utils/http.py @@ -16,10 +16,12 @@ LOGGER = get_logger() def _get_client_ip_from_meta(meta: dict[str, Any]) -> str: """Attempt to get the client's IP by checking common HTTP Headers. - Returns none if no IP Could be found""" + Returns none if no IP Could be found + + No additional validation is done here as requests are expected to only arrive here + via the go proxy, which deals with validating these headers for us""" headers = ( "HTTP_X_FORWARDED_FOR", - "HTTP_X_REAL_IP", "REMOTE_ADDR", ) for _header in headers: diff --git a/internal/config/struct.go b/internal/config/struct.go index b237a425a..40a784b06 100644 --- a/internal/config/struct.go +++ b/internal/config/struct.go @@ -38,13 +38,14 @@ type RedisConfig struct { } type ListenConfig struct { - HTTP string `yaml:"listen_http" env:"AUTHENTIK_LISTEN__HTTP"` - HTTPS string `yaml:"listen_https" env:"AUTHENTIK_LISTEN__HTTPS"` - LDAP string `yaml:"listen_ldap" env:"AUTHENTIK_LISTEN__LDAP"` - LDAPS string `yaml:"listen_ldaps" env:"AUTHENTIK_LISTEN__LDAPS"` - Radius string `yaml:"listen_radius" env:"AUTHENTIK_LISTEN__RADIUS"` - Metrics string `yaml:"listen_metrics" env:"AUTHENTIK_LISTEN__METRICS"` - Debug string `yaml:"listen_debug" env:"AUTHENTIK_LISTEN__DEBUG"` + HTTP string `yaml:"listen_http" env:"AUTHENTIK_LISTEN__HTTP"` + HTTPS string `yaml:"listen_https" env:"AUTHENTIK_LISTEN__HTTPS"` + LDAP string `yaml:"listen_ldap" env:"AUTHENTIK_LISTEN__LDAP"` + LDAPS string `yaml:"listen_ldaps" env:"AUTHENTIK_LISTEN__LDAPS"` + Radius string `yaml:"listen_radius" env:"AUTHENTIK_LISTEN__RADIUS"` + Metrics string `yaml:"listen_metrics" env:"AUTHENTIK_LISTEN__METRICS"` + Debug string `yaml:"listen_debug" env:"AUTHENTIK_LISTEN__DEBUG"` + TrustedProxyCIDRs []string `yaml:"trusted_proxy_cidrs" env:"AUTHENTIK_LISTEN__TRUSTED_PROXY_CIDRS"` } type PathsConfig struct { diff --git a/internal/utils/web/http_forwarded.go b/internal/utils/web/http_forwarded.go new file mode 100644 index 000000000..becf32e94 --- /dev/null +++ b/internal/utils/web/http_forwarded.go @@ -0,0 +1,44 @@ +package web + +import ( + "net" + "net/http" + + "github.com/gorilla/handlers" + log "github.com/sirupsen/logrus" + "goauthentik.io/internal/config" +) + +// ProxyHeaders Set proxy headers like X-Forwarded-For and such, but only if the direct connection +// comes from a client that's in a list of trusted CIDRs +func ProxyHeaders() func(http.Handler) http.Handler { + nets := []*net.IPNet{} + for _, rn := range config.Get().Listen.TrustedProxyCIDRs { + _, cidr, err := net.ParseCIDR(rn) + if err != nil { + continue + } + nets = append(nets, cidr) + } + ph := handlers.ProxyHeaders + return func(h http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + host, _, err := net.SplitHostPort(r.RemoteAddr) + if err == nil { + // remoteAddr will be nil if the IP cannot be parsed + remoteAddr := net.ParseIP(host) + for _, allowedCidr := range nets { + if remoteAddr != nil && allowedCidr.Contains(remoteAddr) { + log.WithField("remoteAddr", remoteAddr).WithField("cidr", allowedCidr.String()).Trace("Setting proxy headers") + ph(h).ServeHTTP(w, r) + return + } + } + } + // Request is not directly coming from a CIDR we "trust" + // so set XFF to the direct host IP + r.Header.Set("X-Forwarded-For", host) + h.ServeHTTP(w, r) + }) + } +} diff --git a/internal/web/web.go b/internal/web/web.go index 83a89dbfa..97a1eb068 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -35,7 +35,7 @@ type WebServer struct { func NewWebServer(g *gounicorn.GoUnicorn) *WebServer { l := log.WithField("logger", "authentik.router") mainHandler := mux.NewRouter() - mainHandler.Use(handlers.ProxyHeaders) + mainHandler.Use(web.ProxyHeaders()) mainHandler.Use(handlers.CompressHandler) loggingHandler := mainHandler.NewRoute().Subrouter() loggingHandler.Use(web.NewLoggingHandler(l, nil)) diff --git a/website/docs/installation/configuration.md b/website/docs/installation/configuration.md index 53410b19e..4ef659fb3 100644 --- a/website/docs/installation/configuration.md +++ b/website/docs/installation/configuration.md @@ -57,6 +57,11 @@ kubectl exec -it deployment/authentik-worker -c authentik -- ak dump_config - `AUTHENTIK_LISTEN__LDAPS`: Listening address:port (e.g. `0.0.0.0:6636`) for LDAPS (LDAP outpost) - `AUTHENTIK_LISTEN__METRICS`: Listening address:port (e.g. `0.0.0.0:9300`) for Prometheus metrics (All) - `AUTHENTIK_LISTEN__DEBUG`: Listening address:port (e.g. `0.0.0.0:9900`) for Go Debugging metrics (All) +- `AUTHENTIK_LISTEN__TRUSTED_PROXY_CIDRS`: List of CIDRs that proxy headers should be accepted from (Server) + + Defaults to `127.0.0.0/8`, `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16`, `fe80::/10`, `::1/128`. + + Requests directly coming from one an address within a CIDR specified here are able to set proxy headers, such as `X-Forwarded-For`. Requests coming from other addresses will not be able to set these headers. ## authentik Settings diff --git a/website/docs/security/CVE-2023-36456.md b/website/docs/security/CVE-2023-36456.md new file mode 100644 index 000000000..babdaabf4 --- /dev/null +++ b/website/docs/security/CVE-2023-36456.md @@ -0,0 +1,21 @@ +# CVE-2023-36456 + +_Reported by [@thijsa](https://github.com/thijsa)_ + +## Lack of Proxy IP headers validation + +### Summary + +authentik does not verify the source of the X-Forwarded-For and X-Real-IP headers, both in the Python code and the go code. + +### Impact + +Only authentik setups that are directly accessible by users without a reverse proxy are susceptible to this. Possible spoofing of IP addresses in logs, downstream applications proxied by (built in) outpost, IP bypassing in custom flows if used. + +### Details + +This poses a possible security risk when you have flows or policies that check the user's IP address, e.g. when you want to ignore the user's 2 factor authentication when the user is connected to the company network. + +Another security risk is that the IP addresses in the logfiles and user sessions is not reliable anymore, anybody can spoof this address and you cannot verify that the user has logged in from the IP address that is in their account's log. + +And the third risk is that this header is passed on to the proxied application behind an outpost. The application may do any kind of verification, logging, blocking or rate limiting based on the IP address, and this IP address can be overridden by anybody that want to. diff --git a/website/sidebars.js b/website/sidebars.js index 9385a4099..b45691ab9 100644 --- a/website/sidebars.js +++ b/website/sidebars.js @@ -322,10 +322,11 @@ module.exports = { }, items: [ "security/policy", + "security/CVE-2023-36456", + "security/CVE-2023-26481", "security/CVE-2022-23555", "security/CVE-2022-46145", "security/CVE-2022-46172", - "security/CVE-2023-26481", ], }, ],