Skip to content

Commit

Permalink
security: fix CVE 2024 52307 (#12115)
Browse files Browse the repository at this point in the history
* security: fix CVE-2024-52307

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* add docs

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

* fix tests

Signed-off-by: Jens Langhammer <jens@goauthentik.io>

---------

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
  • Loading branch information
BeryJu committed Nov 21, 2024
1 parent e10c47d commit 11b013d
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 20 deletions.
21 changes: 11 additions & 10 deletions authentik/root/monitoring.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
"""Metrics view"""

from base64 import b64encode
from hmac import compare_digest
from pathlib import Path
from tempfile import gettempdir

from django.conf import settings
from django.db import connections
Expand All @@ -16,22 +18,21 @@


class MetricsView(View):
"""Wrapper around ExportToDjangoView, using http-basic auth"""
"""Wrapper around ExportToDjangoView with authentication, accessed by the authentik router"""

def __init__(self, **kwargs):
_tmp = Path(gettempdir())
with open(_tmp / "authentik-core-metrics.key") as _f:
self.monitoring_key = _f.read()

def get(self, request: HttpRequest) -> HttpResponse:
"""Check for HTTP-Basic auth"""
auth_header = request.META.get("HTTP_AUTHORIZATION", "")
auth_type, _, given_credentials = auth_header.partition(" ")
credentials = f"monitor:{settings.SECRET_KEY}"
expected = b64encode(str.encode(credentials)).decode()
authed = auth_type == "Basic" and given_credentials == expected
authed = auth_type == "Bearer" and compare_digest(given_credentials, self.monitoring_key)
if not authed and not settings.DEBUG:
response = HttpResponse(status=401)
response["WWW-Authenticate"] = 'Basic realm="authentik-monitoring"'
return response

return HttpResponse(status=401)
monitoring_set.send_robust(self)

return ExportToDjangoView(request)


Expand Down
18 changes: 14 additions & 4 deletions authentik/root/tests.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,34 @@
"""root tests"""

from base64 import b64encode
from pathlib import Path
from secrets import token_urlsafe
from tempfile import gettempdir

from django.conf import settings
from django.test import TestCase
from django.urls import reverse


class TestRoot(TestCase):
"""Test root application"""

def setUp(self):
_tmp = Path(gettempdir())
self.token = token_urlsafe(32)
with open(_tmp / "authentik-core-metrics.key", "w") as _f:
_f.write(self.token)

def tearDown(self):
_tmp = Path(gettempdir())
(_tmp / "authentik-core-metrics.key").unlink()

def test_monitoring_error(self):
"""Test monitoring without any credentials"""
response = self.client.get(reverse("metrics"))
self.assertEqual(response.status_code, 401)

def test_monitoring_ok(self):
"""Test monitoring with credentials"""
creds = "Basic " + b64encode(f"monitor:{settings.SECRET_KEY}".encode()).decode("utf-8")
auth_headers = {"HTTP_AUTHORIZATION": creds}
auth_headers = {"HTTP_AUTHORIZATION": f"Bearer {self.token}"}
response = self.client.get(reverse("metrics"), **auth_headers)
self.assertEqual(response.status_code, 200)

Expand Down
25 changes: 22 additions & 3 deletions internal/web/metrics.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package web

import (
"encoding/base64"
"fmt"
"io"
"net/http"
"os"
"path"

"github.com/gorilla/mux"
"github.com/gorilla/securecookie"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/client_golang/prometheus/promhttp"
Expand All @@ -14,14 +18,25 @@ import (
"goauthentik.io/internal/utils/sentry"
)

const MetricsKeyFile = "authentik-core-metrics.key"

var Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{
Name: "authentik_main_request_duration_seconds",
Help: "API request latencies in seconds",
}, []string{"dest"})

func (ws *WebServer) runMetricsServer() {
m := mux.NewRouter()
l := log.WithField("logger", "authentik.router.metrics")
tmp := os.TempDir()
key := base64.StdEncoding.EncodeToString(securecookie.GenerateRandomKey(64))
keyPath := path.Join(tmp, MetricsKeyFile)
err := os.WriteFile(keyPath, []byte(key), 0o600)
if err != nil {
l.WithError(err).Warning("failed to save metrics key")
return
}

m := mux.NewRouter()
m.Use(sentry.SentryNoSampleMiddleware)
m.Path("/metrics").HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
promhttp.InstrumentMetricHandler(
Expand All @@ -36,7 +51,7 @@ func (ws *WebServer) runMetricsServer() {
l.WithError(err).Warning("failed to get upstream metrics")
return
}
re.SetBasicAuth("monitor", config.Get().SecretKey)
re.Header.Set("Authorization", fmt.Sprintf("Bearer %s", key))
res, err := ws.upstreamHttpClient().Do(re)
if err != nil {
l.WithError(err).Warning("failed to get upstream metrics")
Expand All @@ -49,9 +64,13 @@ func (ws *WebServer) runMetricsServer() {
}
})
l.WithField("listen", config.Get().Listen.Metrics).Info("Starting Metrics server")
err := http.ListenAndServe(config.Get().Listen.Metrics, m)
err = http.ListenAndServe(config.Get().Listen.Metrics, m)
if err != nil {
l.WithError(err).Warning("Failed to start metrics server")
}
l.WithField("listen", config.Get().Listen.Metrics).Info("Stopping Metrics server")
err = os.Remove(keyPath)
if err != nil {
l.WithError(err).Warning("failed to remove metrics key file")
}
}
2 changes: 1 addition & 1 deletion internal/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func NewWebServer() *WebServer {
loggingHandler.Use(web.NewLoggingHandler(l, nil))

tmp := os.TempDir()
socketPath := path.Join(tmp, "authentik-core.sock")
socketPath := path.Join(tmp, UnixSocketName)

// create http client to talk to backend, normal client if we're in debug more
// and a client that connects to our socket when in non debug mode
Expand Down
4 changes: 2 additions & 2 deletions website/docs/developer-docs/releases/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Short summary of the issue
### Patches
authentik x, y and z fix this issue, for other versions the workaround can be used.
authentik x, y and z fix this issue, for other versions the workaround below can be used.
### Impact
Expand All @@ -96,7 +96,7 @@ Describe a workaround if possible
If you have any questions or comments about this advisory:
- Email us at [security@goauthentik.io](mailto:security@goauthentik.io)
- Email us at [security@goauthentik.io](mailto:security@goauthentik.io).
```

</details>
Expand Down
36 changes: 36 additions & 0 deletions website/docs/security/cves/CVE-2024-52307.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# CVE-2024-52307

_Reported by [@mgerstner](https://github.com/mgerstner)_

## Timing attack due to a lack of constant time comparison for metrics view

### Summary

Due to the usage of a non-constant time comparison for the `/-/metrics/` endpoint it was possible to brute-force the `SECRET_KEY`, which is used to authenticate the endpoint. The `/-/metrics/` endpoint returns Prometheus metrics and is not intended to be accessed directly, as the Go proxy running in the authentik server container fetches data from this endpoint and serves it on a separate port (9300 by default), which can be scraped by Prometheus without being exposed publicly.

### Patches

authentik 2024.8.5 and 2024.10.3 fix this issue, for other versions the workaround below can be used.

### Impact

With enough attempts the `SECRET_KEY` of the authentik installation can be brute-forced, which can be used to sign new or modify existing cookies.

### Workarounds

Since the `/-/metrics/` endpoint is not intended to be accessed publicly, requests to the endpoint can be blocked by the reverse proxy/load balancer used in conjunction with authentik.

For example for nginx:

```
location /-/metrics/ {
deny all;
return 404;
}
```

### For more information

If you have any questions or comments about this advisory:

- Email us at [security@goauthentik.io](mailto:security@goauthentik.io).
1 change: 1 addition & 0 deletions website/sidebars.js
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ export default {
type: "category",
label: "2024",
items: [
"security/cves/CVE-2024-52307",
"security/cves/CVE-2024-52287",
"security/cves/CVE-2024-47077",
"security/cves/CVE-2024-47070",
Expand Down

0 comments on commit 11b013d

Please sign in to comment.