security: fix CVE 2024 52307 (#12115)
* 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>
This commit is contained in:
@ -1,6 +1,8 @@
|
|||||||
"""Metrics view"""
|
"""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.conf import settings
|
||||||
from django.db import connections
|
from django.db import connections
|
||||||
@ -16,22 +18,21 @@ monitoring_set = Signal()
|
|||||||
|
|
||||||
|
|
||||||
class MetricsView(View):
|
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:
|
def get(self, request: HttpRequest) -> HttpResponse:
|
||||||
"""Check for HTTP-Basic auth"""
|
"""Check for HTTP-Basic auth"""
|
||||||
auth_header = request.META.get("HTTP_AUTHORIZATION", "")
|
auth_header = request.META.get("HTTP_AUTHORIZATION", "")
|
||||||
auth_type, _, given_credentials = auth_header.partition(" ")
|
auth_type, _, given_credentials = auth_header.partition(" ")
|
||||||
credentials = f"monitor:{settings.SECRET_KEY}"
|
authed = auth_type == "Bearer" and compare_digest(given_credentials, self.monitoring_key)
|
||||||
expected = b64encode(str.encode(credentials)).decode()
|
|
||||||
authed = auth_type == "Basic" and given_credentials == expected
|
|
||||||
if not authed and not settings.DEBUG:
|
if not authed and not settings.DEBUG:
|
||||||
response = HttpResponse(status=401)
|
return HttpResponse(status=401)
|
||||||
response["WWW-Authenticate"] = 'Basic realm="authentik-monitoring"'
|
|
||||||
return response
|
|
||||||
|
|
||||||
monitoring_set.send_robust(self)
|
monitoring_set.send_robust(self)
|
||||||
|
|
||||||
return ExportToDjangoView(request)
|
return ExportToDjangoView(request)
|
||||||
|
|
||||||
|
|
||||||
|
@ -1,8 +1,9 @@
|
|||||||
"""root tests"""
|
"""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.test import TestCase
|
||||||
from django.urls import reverse
|
from django.urls import reverse
|
||||||
|
|
||||||
@ -10,6 +11,16 @@ from django.urls import reverse
|
|||||||
class TestRoot(TestCase):
|
class TestRoot(TestCase):
|
||||||
"""Test root application"""
|
"""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):
|
def test_monitoring_error(self):
|
||||||
"""Test monitoring without any credentials"""
|
"""Test monitoring without any credentials"""
|
||||||
response = self.client.get(reverse("metrics"))
|
response = self.client.get(reverse("metrics"))
|
||||||
@ -17,8 +28,7 @@ class TestRoot(TestCase):
|
|||||||
|
|
||||||
def test_monitoring_ok(self):
|
def test_monitoring_ok(self):
|
||||||
"""Test monitoring with credentials"""
|
"""Test monitoring with credentials"""
|
||||||
creds = "Basic " + b64encode(f"monitor:{settings.SECRET_KEY}".encode()).decode("utf-8")
|
auth_headers = {"HTTP_AUTHORIZATION": f"Bearer {self.token}"}
|
||||||
auth_headers = {"HTTP_AUTHORIZATION": creds}
|
|
||||||
response = self.client.get(reverse("metrics"), **auth_headers)
|
response = self.client.get(reverse("metrics"), **auth_headers)
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 200)
|
||||||
|
|
||||||
|
@ -1,11 +1,15 @@
|
|||||||
package web
|
package web
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/base64"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"os"
|
||||||
|
"path"
|
||||||
|
|
||||||
"github.com/gorilla/mux"
|
"github.com/gorilla/mux"
|
||||||
|
"github.com/gorilla/securecookie"
|
||||||
"github.com/prometheus/client_golang/prometheus"
|
"github.com/prometheus/client_golang/prometheus"
|
||||||
"github.com/prometheus/client_golang/prometheus/promauto"
|
"github.com/prometheus/client_golang/prometheus/promauto"
|
||||||
"github.com/prometheus/client_golang/prometheus/promhttp"
|
"github.com/prometheus/client_golang/prometheus/promhttp"
|
||||||
@ -14,14 +18,25 @@ import (
|
|||||||
"goauthentik.io/internal/utils/sentry"
|
"goauthentik.io/internal/utils/sentry"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const MetricsKeyFile = "authentik-core-metrics.key"
|
||||||
|
|
||||||
var Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{
|
var Requests = promauto.NewHistogramVec(prometheus.HistogramOpts{
|
||||||
Name: "authentik_main_request_duration_seconds",
|
Name: "authentik_main_request_duration_seconds",
|
||||||
Help: "API request latencies in seconds",
|
Help: "API request latencies in seconds",
|
||||||
}, []string{"dest"})
|
}, []string{"dest"})
|
||||||
|
|
||||||
func (ws *WebServer) runMetricsServer() {
|
func (ws *WebServer) runMetricsServer() {
|
||||||
m := mux.NewRouter()
|
|
||||||
l := log.WithField("logger", "authentik.router.metrics")
|
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.Use(sentry.SentryNoSampleMiddleware)
|
||||||
m.Path("/metrics").HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
m.Path("/metrics").HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||||
promhttp.InstrumentMetricHandler(
|
promhttp.InstrumentMetricHandler(
|
||||||
@ -36,7 +51,7 @@ func (ws *WebServer) runMetricsServer() {
|
|||||||
l.WithError(err).Warning("failed to get upstream metrics")
|
l.WithError(err).Warning("failed to get upstream metrics")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
re.SetBasicAuth("monitor", config.Get().SecretKey)
|
re.Header.Set("Authorization", fmt.Sprintf("Bearer %s", key))
|
||||||
res, err := ws.upstreamHttpClient().Do(re)
|
res, err := ws.upstreamHttpClient().Do(re)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
l.WithError(err).Warning("failed to get upstream metrics")
|
l.WithError(err).Warning("failed to get upstream metrics")
|
||||||
@ -49,9 +64,13 @@ func (ws *WebServer) runMetricsServer() {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
l.WithField("listen", config.Get().Listen.Metrics).Info("Starting Metrics server")
|
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 {
|
if err != nil {
|
||||||
l.WithError(err).Warning("Failed to start metrics server")
|
l.WithError(err).Warning("Failed to start metrics server")
|
||||||
}
|
}
|
||||||
l.WithField("listen", config.Get().Listen.Metrics).Info("Stopping 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")
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -53,7 +53,7 @@ func NewWebServer() *WebServer {
|
|||||||
loggingHandler.Use(web.NewLoggingHandler(l, nil))
|
loggingHandler.Use(web.NewLoggingHandler(l, nil))
|
||||||
|
|
||||||
tmp := os.TempDir()
|
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
|
// 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
|
// and a client that connects to our socket when in non debug mode
|
||||||
|
@ -78,7 +78,7 @@ Short summary of the issue
|
|||||||
|
|
||||||
### Patches
|
### 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
|
### Impact
|
||||||
|
|
||||||
@ -96,7 +96,7 @@ Describe a workaround if possible
|
|||||||
|
|
||||||
If you have any questions or comments about this advisory:
|
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>
|
</details>
|
||||||
|
36
website/docs/security/cves/CVE-2024-52307.md
Normal file
36
website/docs/security/cves/CVE-2024-52307.md
Normal file
@ -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).
|
@ -656,6 +656,7 @@ export default {
|
|||||||
type: "category",
|
type: "category",
|
||||||
label: "2024",
|
label: "2024",
|
||||||
items: [
|
items: [
|
||||||
|
"security/cves/CVE-2024-52307",
|
||||||
"security/cves/CVE-2024-52287",
|
"security/cves/CVE-2024-52287",
|
||||||
"security/cves/CVE-2024-47077",
|
"security/cves/CVE-2024-47077",
|
||||||
"security/cves/CVE-2024-47070",
|
"security/cves/CVE-2024-47070",
|
||||||
|
Reference in New Issue
Block a user