From: Willy Tarreau Date: Fri, 10 Jul 2026 09:27:37 +0000 (+0200) Subject: BUG/MEDIUM: stats: subject "stats admin" accesses to "stats scope" filtering X-Git-Url: http://git.kaiwu.me/web/doc/static/gitweb.js?a=commitdiff_plain;h=f34517a1aac4b753a25e982f9aaeb2e1d7e2eba9;p=haproxy.git BUG/MEDIUM: stats: subject "stats admin" accesses to "stats scope" filtering It was reported by Red Hat in partnership with AISLE Research that proxies updated via a stats page in "stats admin" mode were not subject to the filtering of "stats scope". It is totally true and was never meant to be when "stats scope" was added in 1.2 in 2006 while "stats admin" arrived 5 years later in 1.4, nor was it even implied by the doc, but it is easy to see how someone who would combine the two could expect them to work together: - "stats scope" only limits which proxies are shown, the purpose being to avoid polluting the page with a long list of auxiliary proxies that take ages to render (e.g. those dedictated to stick-tables etc). A common use case is to only show known frontends so that numerous backends are not listed, it's most often used on public access. - "stats admin" is used to permit to change a proxy or a server's state and is protected using HTTP Basic auth, cleartext passwords by default, and is mostly meant to be used from trusted sources, hence private access. The two are almost never seen together (no single occurrence found in github issues for 92 occurrences of "stats admin" and 5 of "stats scope"), but admittedly, if someone were to combine the two, seeing buttons on the listed proxies only, they would surely expect to only be able to act on those. So in order to clear this void, this commit does the following upon a POST: - when a scope is defined, the requested proxy must exist and match the list of permitted ones, otherwise a DENY is returned. This ensures that the POST cannot be abused either to infer the existence of a given proxy name. - when no scope is defined, a non-existing proxy continues to return an "unknown proxy" error. The doc was updated to explain this new behavior. This commit should be backported to stable versions, along with the previous patch it depends on: MINOR: stats: factor the proxy vs scope check into its own function The stats code was split between 2.9 and 3.0, so the changes must all be performed in stats.c before 3.0. Also, ctx->http_px does not exist earlier, but http_px is simply the calling stream's backend (s->be at some places). Thanks to Red Hat and AISLE Research for reporting this. --- diff --git a/doc/configuration.txt b/doc/configuration.txt index 2acf2763f..b76c7541f 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -12758,7 +12758,9 @@ stats admin { if | unless } matched. The admin level allows to enable/disable servers from the web interface. By - default, statistics page is read-only for security reasons. + default, statistics page is read-only for security reasons. If "stats scope" + directives are set in the section, then only proxies designated by these + directives will accept state changes; access to other ones will be denied. Currently, the POST request is limited to the buffer size minus the reserved buffer space, which means that if the list of servers is too long, the @@ -12793,8 +12795,8 @@ stats admin { if | unless } stats http-request auth unless AUTH stats admin if AUTH_ADMIN - See also : "stats enable", "stats auth", "stats http-request", section 12.2 - about userlists and section 7 about ACL usage. + See also : "stats enable", "stats auth", "stats http-request", "stats scope", + section 12.2 about userlists and section 7 about ACL usage. ssl-f-use [ ...]* Assignate a certificate to the current frontend. @@ -13122,11 +13124,11 @@ stats scope { | "." } section in which the statement appears. When this statement is specified, only the sections enumerated with this - statement will appear in the report. All other ones will be hidden. This - statement may appear as many times as needed if multiple sections need to be - reported. Please note that the name checking is performed as simple string - comparisons, and that it is never checked that a give section name really - exists. + statement will appear in the report. All other ones will be hidden, and + attempts to change their state in admin mode will be rejected. This statement + may appear as many times as needed if multiple sections need to be reported. + Please note that the name checking is performed as simple string comparisons, + and that it is never checked that a give section name really exists. Though this statement alone is enough to enable statistics reporting, it is recommended to set all other settings in order to avoid relying on default @@ -13150,7 +13152,8 @@ stats scope { | "." } stats uri /admin?stats stats refresh 5s - See also : "stats auth", "stats enable", "stats realm", "stats uri" + See also : "stats auth", "stats enable", "stats realm", "stats uri" and + "stats admin" stats show-desc [ ] diff --git a/src/stats-html.c b/src/stats-html.c index 68790fbb9..469ca6ac1 100644 --- a/src/stats-html.c +++ b/src/stats-html.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -1734,7 +1735,29 @@ static int stats_process_http_post(struct stconn *sc) /* Now we can check the key to see what to do */ if (!px && (strcmp(key, "b") == 0)) { - if ((px = proxy_be_by_name(value)) == NULL) { + /* we need to lookup the proxy because its name + * may be a real name ("pub") or numeric ("#4"). + */ + px = proxy_be_by_name(value); + + /* now we have two possibilities: + * - if the are "stats scope" rules, a non-existing + * proxy or a forbidden one are the same, we return + * a deny because a non-existing name cannot match + * a scope rule and we don't want to disclose the + * existence of a given name. + * - without "stats scope" rules, we just return + * not found. + */ + if (ctx->http_px->uri_auth && ctx->http_px->uri_auth->scope) { + if (!px || !stats_proxy_in_scope(px, ctx->http_px->uri_auth, ctx->http_px)) { + /* "stats scope" rules do not permit to access this proxy name */ + ctx->st_code = STAT_STATUS_DENY; + goto out; + } + } + + if (!px) { /* the backend name is unknown or ambiguous (duplicate names) */ ctx->st_code = STAT_STATUS_ERRP; goto out;