From ccbe2d6cb72c2f3f8969564e59f864e588b3d54e Mon Sep 17 00:00:00 2001 From: Nathan Scott Date: Thu, 2 Jul 2026 15:28:34 +1000 Subject: [PATCH] pmproxy: fix missing -Q and -S authentication flags (CWE-306) The pmproxy -Q (require client certificate) and -S (require authenticated clients) flags existed as case blocks in the option parser but were absent from the short_options string and the longopts table, making them permanently unreachable. An unauthenticated HTTP client could access all REST API endpoints including /store and /derive. Fix: - Add Q and S to short_options so pmgetopt_r() delivers them - Add --certreqd and --reqauth entries to the longopts table - Document both flags in the pmproxy(1) man page - Add qa/2100 verifying the flags are accepted and that -S correctly rejects unauthenticated REST API requests with HTTP 403 Note: -S enforcement in the REST API path already exists in http.c and webapi.c. -Q (CERT_REQD) enforcement is only implemented for the legacy PCP wire protocol path, not the REST API; this is a pre-existing limitation to be addressed separately. Reported-by: Francisco Alisson Bezerra, TIM Security Red Team Reported-by: Lucas Gabriel Alves, TIM Security Red Team Reported-by: Massimiliano Brolli, TIM Security Red Team Co-Authored-By: Claude Opus 4.6 (1M context) --- man/man1/pmproxy.1 | 6 +++ qa/2100 | 85 +++++++++++++++++++++++++++++++++++++++ qa/2100.out | 12 ++++++ qa/group | 1 + src/pmproxy/src/pmproxy.c | 4 +- 5 files changed, 107 insertions(+), 1 deletion(-) create mode 100755 qa/2100 create mode 100644 qa/2100.out diff --git a/man/man1/pmproxy.1 b/man/man1/pmproxy.1 index 90ec61be8..30758a114 100644 --- a/man/man1/pmproxy.1 +++ b/man/man1/pmproxy.1 @@ -271,6 +271,9 @@ be unreadable by any user other than the user running the .B pmproxy process). .TP +\fB\-Q\f1, \fB\-\-certreqd\f1 +Require that all client connections provide a trusted client certificate. +.TP \f3\-r\f1 \f2port\f1, \f3\-\-redisport\f1=\f2port\f1 Specify an alternate Redis .I port @@ -285,6 +288,9 @@ The default value is .IR $PCP_RUN_DIR/pmproxy.socket . This option implies \f3pmproxy\f1 is running in \f3timeseries\f1 mode. .TP +\fB\-S\f1, \fB\-\-reqauth\f1 +Require that all client connections be authenticated. +.TP \fB\-t\f1, \fB\-\-timeseries\f1 Operate in automatic archive timeseries discovery mode. This mode of operation will enable the diff --git a/qa/2100 b/qa/2100 new file mode 100755 index 000000000..1de957a2f --- /dev/null +++ b/qa/2100 @@ -0,0 +1,85 @@ +#!/bin/sh +# PCP QA Test No. 2100 +# Verify pmproxy -Q and -S authentication flags are accepted +# and that -S (reqauth) enforces authentication on REST API +# +# Copyright (c) 2026 Red Hat. All Rights Reserved. +# + +seq=`basename $0` +echo "QA output created by $seq" + +# get standard environment, filters and checks +. ./common.product +. ./common.filter +. ./common.check + +which curl >/dev/null 2>&1 || _notrun "no curl executable installed" + +_cleanup() +{ + [ -n "$__pid" ] && kill $__pid 2>/dev/null + wait $__pid 2>/dev/null + cd $here + $sudo rm -rf $tmp $tmp.* +} + +status=0 # success is the default! +__pid="" +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# real QA test starts here + +echo "=== checking -Q and -S appear in usage ===" +pmproxy --help 2>&1 | grep -E '\-[QS]' | sed -e 's/^ *//' + +echo +echo "=== checking -S enforces authentication on REST API ===" +__port=`_find_free_port` +$PCP_BINADM_DIR/pmproxy -S -f -p $__port -l $tmp.log & +__pid=$! +sleep 1 +if kill -0 $__pid 2>/dev/null; then + echo "pmproxy with -S started" + + # unauthenticated request should be rejected + __code=`curl -s -o /dev/null -w '%{http_code}' "http://localhost:$__port/pmapi/context?hostspec=localhost" 2>/dev/null` + if [ "$__code" = "403" ]; then + echo "unauthenticated request correctly rejected (HTTP $__code)" + else + echo "FAIL: expected HTTP 403, got HTTP $__code" + fi + + kill $__pid + wait $__pid 2>/dev/null + __pid="" +else + echo "FAIL: pmproxy with -S did not start" +fi + +echo +echo "=== checking without -S allows unauthenticated access ===" +__port=`_find_free_port` +$PCP_BINADM_DIR/pmproxy -f -p $__port -l $tmp.log2 & +__pid=$! +sleep 1 +if kill -0 $__pid 2>/dev/null; then + echo "pmproxy without -S started" + + # unauthenticated request should succeed + __code=`curl -s -o /dev/null -w '%{http_code}' "http://localhost:$__port/pmapi/context?hostspec=localhost" 2>/dev/null` + if [ "$__code" = "200" ]; then + echo "unauthenticated request correctly allowed (HTTP $__code)" + else + echo "FAIL: expected HTTP 200, got HTTP $__code" + fi + + kill $__pid + wait $__pid 2>/dev/null + __pid="" +else + echo "FAIL: pmproxy without -S did not start" +fi + +# success, all done +exit diff --git a/qa/2100.out b/qa/2100.out new file mode 100644 index 000000000..5302aea77 --- /dev/null +++ b/qa/2100.out @@ -0,0 +1,12 @@ +QA output created by 2100 +=== checking -Q and -S appear in usage === +-Q, --certreqd require client certificate authentication +-S, --reqauth require all client connections to be authenticated + +=== checking -S enforces authentication on REST API === +pmproxy with -S started +unauthenticated request correctly rejected (HTTP 403) + +=== checking without -S allows unauthenticated access === +pmproxy without -S started +unauthenticated request correctly allowed (HTTP 200) diff --git a/qa/group b/qa/group index 742d3b54b..a8b52b68e 100644 --- a/qa/group +++ b/qa/group @@ -1980,5 +1980,6 @@ x11 1984 pmlogconf pmda.redis local 1985 pmfind local valgrind 1986 pmfind local +2100 pmproxy local security 2105 libpcp pmcd local security pmcd.pdu 4751 libpcp threads valgrind local pcp helgrind diff --git a/src/pmproxy/src/pmproxy.c b/src/pmproxy/src/pmproxy.c index d975ef37a..43766c20f 100644 --- a/src/pmproxy/src/pmproxy.c +++ b/src/pmproxy/src/pmproxy.c @@ -86,7 +86,9 @@ static pmLongOptions longopts[] = { PMAPI_OPTIONS_HEADER("Connection options"), { "interface", 1, 'i', "ADDR", "accept connections on this IP address" }, { "port", 1, 'p', "PORT", "accept connections on this port" }, + { "certreqd", 0, 'Q', 0, "require client certificate authentication" }, { "socket", 1, 's', "PATH", "Unix domain socket file [default $PCP_RUN_DIR/pmproxy.socket]" }, + { "reqauth", 0, 'S', 0, "require all client connections to be authenticated" }, { "redisport", 1, 'r', "PORT", "Connect to Redis instance on this TCP/IP port (implies --timeseries)" }, { "redishost", 1, 'h', "HOST", "Connect to Redis instance on this host name (implies --timeseries)" }, PMAPI_OPTIONS_HEADER("Diagnostic options"), @@ -96,7 +98,7 @@ static pmLongOptions longopts[] = { }; static pmOptions opts = { - .short_options = "Ac:C:dD:Ffh:i:l:L:M:p:P:r:s:tU:x:?", + .short_options = "Ac:C:dD:Ffh:i:l:L:M:p:P:Qr:s:StU:x:?", .long_options = longopts, };