fix(#3517): metrics are not optional, and the endpoint is denied by default
Two review findings, folded together. mara: a swarm integrated auto deployed forge always has metrics, so the toggle is gone. The endpoint follows behindGateway instead, which is the swarm-integrated shape and the condition the protected location lives under. Serving it without that location would put it on a listener openFirewall can expose with nothing in front. argus: /metrics matched no access_control rule, so default_policy one_factor governed it. That is any authenticated subject, which today means any operator and tomorrow any agent. My audience argument covered the Bearer path only; the same endpoint also accepts CookieSession, and a cookie carries no audience at all, so the audience was never what stood between a browser session and this data. The rule is deny rather than a client-scoped allow because the collector's client does not exist yet. Authelia refuses a subject naming an unregistered client, and does so in a preStart validator rather than at build time, so naming one early yields a green nixos-rebuild and dead swarm SSO on the next restart. Denying until the client is registered makes publishing the endpoint safe on its own; registering it is a one-line change from deny to that allow. Rule order is load-bearing: authelia takes the first match.
This commit is contained in:
parent
4e17deada5
commit
b2ee674415
2 changed files with 52 additions and 53 deletions
|
|
@ -305,36 +305,6 @@ in
|
|||
'';
|
||||
};
|
||||
|
||||
metricsEnable = lib.mkOption {
|
||||
type = lib.types.bool;
|
||||
default = false;
|
||||
example = true;
|
||||
description = ''
|
||||
Serve forgejo's prometheus metrics and publish them, behind
|
||||
authelia, at `https://<domain>/metrics`.
|
||||
|
||||
Off by default. Forgejo's metrics carry repository, user and
|
||||
request counts for the whole hive, so turning them on is a
|
||||
disclosure decision an operator should make deliberately rather
|
||||
than inherit from a collector appearing elsewhere in the swarm.
|
||||
|
||||
::: {.warning}
|
||||
This option is deliberately **one** switch for two changes that
|
||||
must never be made separately. Forgejo serves `/metrics` on its
|
||||
normal HTTP listener, and the gateway vhost proxies `/` to that
|
||||
listener — so enabling the endpoint *without* the protected
|
||||
location would publish it through the existing catch-all, to
|
||||
anyone. The `= /metrics` location added alongside is an exact
|
||||
match and therefore wins over the `/` prefix.
|
||||
:::
|
||||
|
||||
Authentication is the gateway's, not forgejo's own `[metrics]
|
||||
TOKEN`: a scraper presents an audience-scoped authelia token and
|
||||
nginx checks it via `auth_request`, so the swarm keeps one
|
||||
identity system instead of a static bearer per service.
|
||||
'';
|
||||
};
|
||||
|
||||
mirrors = lib.mkOption {
|
||||
type = lib.types.listOf (
|
||||
lib.types.submodule {
|
||||
|
|
@ -480,10 +450,6 @@ in
|
|||
"${cfg.domain}" = (gatewayCfg.lib.tlsFor cfg.domain) // {
|
||||
listen = gatewayCfg.lib.listen;
|
||||
extraConfig = gatewayCfg.lib.securityHeaders;
|
||||
# ⚠️ The metrics locations merge into THIS attrset, not into the
|
||||
# vhost above it. `//` is shallow: merging at the vhost level
|
||||
# would replace `locations` wholesale and silently drop the `/`
|
||||
# proxy — a config that still renders and still starts.
|
||||
locations = {
|
||||
"/" = {
|
||||
proxyPass = "http://127.0.0.1:${toString cfg.httpPort}/";
|
||||
|
|
@ -495,14 +461,13 @@ in
|
|||
proxy_send_timeout 1h;
|
||||
'';
|
||||
};
|
||||
}
|
||||
// lib.optionalAttrs cfg.metricsEnable {
|
||||
|
||||
# ⚠️ EXACT match, and that is what makes this safe. Forgejo
|
||||
# serves `/metrics` on the same listener the `/` prefix above
|
||||
# already proxies, so without a more specific location the
|
||||
# endpoint would be public the moment it is enabled.
|
||||
# `= /metrics` outranks the `/` prefix in nginx, so this
|
||||
# location — and its auth — is the one that runs.
|
||||
# endpoint would ride that catch-all to anyone. `= /metrics`
|
||||
# outranks the `/` prefix in nginx, so this location — and its
|
||||
# auth — is the one that runs.
|
||||
"= /metrics" = {
|
||||
proxyPass = "http://127.0.0.1:${toString cfg.httpPort}/metrics";
|
||||
extraConfig = ''
|
||||
|
|
@ -748,17 +713,20 @@ in
|
|||
DEFAULT_BRANCH = "main";
|
||||
DEFAULT_PRIVATE = "private";
|
||||
};
|
||||
# Not an option: a swarm-integrated, auto-deployed forge
|
||||
# always has metrics. Tied to `behindGateway` because that
|
||||
# IS the swarm-integrated shape — it is the condition under
|
||||
# which the protected `= /metrics` location below exists.
|
||||
# Serving the endpoint without that location would put it on
|
||||
# a listener `openFirewall` can expose, with nothing in
|
||||
# front of it.
|
||||
#
|
||||
# No `TOKEN` here on purpose. Forgejo can guard this itself
|
||||
# with a static bearer, but the swarm authenticates the
|
||||
# scraper at the gateway with an audience-scoped authelia
|
||||
# token, so a second credential system per service would buy
|
||||
# nothing and would be the one that stops getting rotated.
|
||||
#
|
||||
# Reachability is not decided here: forgejo serves this on
|
||||
# its normal listener, which is loopback-only. What publishes
|
||||
# it is the `= /metrics` vhost location, and that location is
|
||||
# what authenticates it.
|
||||
metrics.ENABLED = cfg.metricsEnable;
|
||||
# scraper at the gateway, so a second credential system per
|
||||
# service would buy nothing and would be the one that stops
|
||||
# getting rotated.
|
||||
metrics.ENABLED = cfg.behindGateway;
|
||||
# Repo migrations / pull-mirrors fetch from the source
|
||||
# URL *inside* Forgejo. hyperhive code is synced from
|
||||
# `localhost` (and the host LAN), which Forgejo's
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ let
|
|||
hyperhiveDomain = hyperhiveCfg.domain;
|
||||
swarmDomain = hyperhiveCfg.swarm.domain;
|
||||
uiCfg = hyperhiveCfg.swarm.ui;
|
||||
forgeCfg = hyperhiveCfg.swarm.forge;
|
||||
|
||||
# Group an account must hold to reach operator-only surfaces. Named
|
||||
# here because this module writes the rule that enforces it and
|
||||
|
|
@ -1134,11 +1135,41 @@ in
|
|||
# the account to disagree silently.
|
||||
access_control = {
|
||||
default_policy = "one_factor";
|
||||
rules = lib.optional uiCfg.enable {
|
||||
domain = uiCfg.domain;
|
||||
subject = [ "group:${operatorGroup}" ];
|
||||
policy = "one_factor";
|
||||
};
|
||||
# ⚠️ ORDER MATTERS — authelia takes the FIRST matching rule.
|
||||
# The metrics rule is listed first so it cannot be shadowed
|
||||
# by a broader domain rule added later.
|
||||
rules =
|
||||
# The forge's metrics endpoint. `deny` is deliberate and
|
||||
# is the whole protection right now: the endpoint is
|
||||
# always served (a swarm-integrated forge always has
|
||||
# metrics), and `default_policy` is `one_factor`, which
|
||||
# means *any* authenticated subject — every operator
|
||||
# today, every agent once they hold authelia accounts.
|
||||
#
|
||||
# Being reachable by a Bearer token is not sufficient on
|
||||
# its own: `authn_strategies` on this endpoint also
|
||||
# accepts `CookieSession`, and a cookie carries no
|
||||
# audience, so the audience is not what stands between a
|
||||
# browser session and this data.
|
||||
#
|
||||
# The collector gets in by REPLACING this with a
|
||||
# client-scoped allow (`subject = ["oauth2:client:<id>"]`)
|
||||
# once such a client is registered. Denying until then is
|
||||
# what makes publishing the endpoint safe on its own —
|
||||
# authelia refuses a subject naming a client that is not
|
||||
# registered, and it does so in a `preStart` validator,
|
||||
# so naming one early takes the whole SSO service down on
|
||||
# the next restart rather than failing the build.
|
||||
lib.optional forgeCfg.behindGateway {
|
||||
domain = forgeCfg.domain;
|
||||
resources = [ "^/metrics$" ];
|
||||
policy = "deny";
|
||||
}
|
||||
++ lib.optional uiCfg.enable {
|
||||
domain = uiCfg.domain;
|
||||
subject = [ "group:${operatorGroup}" ];
|
||||
policy = "one_factor";
|
||||
};
|
||||
};
|
||||
|
||||
# The cookie domain is the SWARM's domain, NOT authelia's
|
||||
|
|
|
|||
Loading…
Reference in a new issue