1999e78348
Fixes the consolidated MR !150 Resolves: RHEL-213662 Resolves: RHEL-213686 Resolves: RHEL-213720 Resolves: RHEL-213729
206 lines
6.9 KiB
Diff
206 lines
6.9 KiB
Diff
From ccbe2d6cb72c2f3f8969564e59f864e588b3d54e Mon Sep 17 00:00:00 2001
|
|
From: Nathan Scott <nathans@redhat.com>
|
|
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) <noreply@anthropic.com>
|
|
---
|
|
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,
|
|
};
|
|
|