From 5ef6f5b9e3e9963431ec3ba1f4013958790a8a89 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:25:30 -0600 Subject: [PATCH 1/6] fix(tools): validate the host as an IP address or an RFC 1123 name Signed-off-by: Thomas Vincent --- CHANGELOG.md | 2 + includes/functions.php | 49 +++++++++++ tests/Unit/WmiValidHostTest.php | 145 ++++++++++++++++++++++++++++++++ wmi_tools.php | 31 +++---- 4 files changed, 207 insertions(+), 20 deletions(-) create mode 100644 tests/Unit/WmiValidHostTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 621b44d..61437a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue: Validate the Query Tool host as an IPv4 address, an IPv6 address or an RFC 1123 host name; the old check refused IPv6, short names and longer top level labels and was never called * issue: Record the WMI query start time from the database clock so devices are not polled again every cycle when PHP and the database use different time zones * security: Run the WMI Query Tool query action only for POST requests so credentials stay out of URLs, access logs and browser history * dev: Cover the Authentication, Queries and Query Tool pages and the Device Template hook with real-request end-to-end tests and replace the source-text checks @@ -19,6 +20,7 @@ * issue: Index cached WMI rows by every column of a comma separated primary key so multi-row results are no longer overwritten * issue: Purge WMI cache rows the device stopped returning, and keep the existing rows when a query fails * issue: Expire stale wmi_processes lock rows and stop the master poller waiting indefinitely for collectors that died +||||||| parent of 7c3e181 (fix(tools): validate the host as an IP address or an RFC 1123 name) * security: Replace the confirmation pages' inline `onClick='cactiReturnTo()'` Cancel buttons with the CSP-safe `cactiReturnTo` class so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive * security: Move the WMI Accounts and Queries filter rows selects' inline onChange handlers into the ready block so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive * dev: Measure CI coverage with xdebug instead of pcov so the plugin's own sources are instrumented (pcov auto-scopes to the Composer root and skipped cacti/plugins/, leaving the patch-coverage gate with nothing to measure) diff --git a/includes/functions.php b/includes/functions.php index 092cf39..6682b97 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -646,6 +646,55 @@ function wmi_collectors_running($seed, $began, $max_age = 3600) { return (int) db_fetch_cell_prepared('SELECT COUNT(*) FROM wmi_processes WHERE taskid = ?', [$seed]); } +/** + * Validates a host as typed into the Query Tool: an IPv4 address, an IPv6 + * address (optionally in brackets), or a host name under RFC 1123 (labels of + * 1 to 63 letters, digits and hyphens that neither start nor end with a + * hyphen, at most 253 characters, optional trailing dot). IPv6 zone ids + * (fe80::1%eth0) are refused, as is a name whose last label is all digits. + * + * @param string $host The host as entered. + * + * @return string|false The host to use (brackets removed), or false when it is + * not a valid address or name. + */ +function wmi_valid_host($host) { + if ($host === '') { + return false; + } + + if ($host[0] === '[' && substr($host, -1) === ']') { + $inner = substr($host, 1, -1); + + return filter_var($inner, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6) !== false ? $inner : false; + } + + if (filter_var($host, FILTER_VALIDATE_IP) !== false) { + return $host; + } + + $name = substr($host, -1) === '.' ? substr($host, 0, -1) : $host; + + if ($name === '' || strlen($name) > 253) { + return false; + } + + $labels = explode('.', $name); + + foreach ($labels as $label) { + if (!preg_match('/^[A-Za-z0-9]([A-Za-z0-9-]{0,61}[A-Za-z0-9])?$/D', $label)) { + return false; + } + } + + // Not a number: 999.1.1.1 is a malformed address, not a host name. + if (ctype_digit((string) end($labels))) { + return false; + } + + return $host; +} + /** * Returns the current unique hash for a WMI query, or generates a new * one if the stored value doesn't already look like a valid 128-bit hex diff --git a/tests/Unit/WmiValidHostTest.php b/tests/Unit/WmiValidHostTest.php new file mode 100644 index 0000000..9acc814 --- /dev/null +++ b/tests/Unit/WmiValidHostTest.php @@ -0,0 +1,145 @@ +toBe($host); +})->with(['192.168.1.10', '10.0.0.1', '255.255.255.255', '0.0.0.0']); + +it('accepts IPv6 addresses in compressed and expanded form', function (string $host) { + expect(wmi_valid_host($host))->toBe($host); +})->with([ + 'loopback' => '::1', + 'unspecified' => '::', + 'compressed' => '2001:db8::1', + 'expanded' => '2001:0db8:0000:0000:0000:0000:0000:0001', + 'link local' => 'fe80::1', + 'mapped IPv4' => '::ffff:192.0.2.1', +]); + +it('removes the brackets from a bracketed IPv6 address', function () { + expect(wmi_valid_host('[2001:db8::1]'))->toBe('2001:db8::1'); + expect(wmi_valid_host('[::1]'))->toBe('::1'); +}); + +it('accepts short names, dotted names and names with any top level label', function (string $host) { + expect(wmi_valid_host($host))->toBe($host); +})->with([ + 'short' => 'win-server', + 'upper case' => 'WIN-SERVER', + 'single letter' => 'a', + 'with digits' => 'srv01', + 'fqdn' => 'server.example.com', + 'local' => 'server.corp.local', + 'long tld' => 'server.example.international', + 'digits inside' => '3com.example.org', + 'trailing dot' => 'example.com.', +]); + +it('accepts labels and names at the length limits', function () { + $label = str_repeat('a', 63); + $name = $label . '.' . $label . '.' . $label . '.' . str_repeat('d', 61); + + expect(strlen($name))->toBe(253); + expect(wmi_valid_host($label . '.example.com'))->toBe($label . '.example.com'); + expect(wmi_valid_host($name))->toBe($name); + expect(wmi_valid_host($name . '.'))->toBe($name . '.'); +}); + +it('rejects labels and names over the length limits', function () { + $label = str_repeat('a', 63); + + expect(wmi_valid_host(str_repeat('a', 64) . '.example.com'))->toBeFalse(); + expect(wmi_valid_host($label . '.' . $label . '.' . $label . '.' . str_repeat('d', 62)))->toBeFalse(); +}); + +it('rejects empty input and malformed names', function (string $host) { + expect(wmi_valid_host($host))->toBeFalse(); +})->with([ + 'empty' => '', + 'space' => ' ', + 'inner space' => 'host name', + 'leading hyphen' => '-bad.example.com', + 'trailing hyphen' => 'bad-.example.com', + 'underscore' => 'bad_host', + 'empty label' => 'a..b', + 'leading dot' => '.example.com', + 'double trailing dot' => 'example.com..', + 'only a dot' => '.', + 'non ascii' => "\xc3\xbcn\xc3\xaf.example.com", + 'port' => 'example.com:445', + 'slashes' => '//host', + 'cidr' => '1.2.3.4/24', +]); + +it('rejects IPv6 zone ids and malformed brackets', function (string $host) { + expect(wmi_valid_host($host))->toBeFalse(); +})->with([ + 'zone id' => 'fe80::1%eth0', + 'bracketed zone id' => '[fe80::1%eth0]', + 'numeric zone id' => '::1%1', + 'bracketed IPv4' => '[192.168.1.1]', + 'bracketed name' => '[example.com]', + 'unclosed bracket' => '[::1', + 'unopened bracket' => '::1]', + 'text after bracket' => '[::1]x', + 'empty brackets' => '[]', + 'too many groups' => '1:2:3:4:5:6:7:8:9', +]); + +it('rejects malformed addresses that look like numbers', function (string $host) { + expect(wmi_valid_host($host))->toBeFalse(); +})->with(['999.1.1.1', '1.2.3', '1.2.3.4.5', '123', '010.0.0.1.1']); + +it('rejects input that looks like an injection attempt', function (string $host) { + expect(wmi_valid_host($host))->toBeFalse(); +})->with([ + 'semicolon' => 'host; id', + 'and' => 'host && id', + 'substitution' => '$(id)', + 'backticks' => '`id`', + 'option' => '-oProxyCommand=x', + 'newline' => "host\nname", + 'nul' => "ho\0st", + 'pipe' => 'host|id', + 'quote' => "host'", + 'percent' => '%USERNAME%', +]); + +it('keeps a host that starts with a hyphen behind the // prefix in the wmic command', function () { + $w = new Linux_WMI('', new Wmic_Shell_Transport()); + $w->username = 'u'; + $w->password = 'p'; + $w->command = 'SELECT 1'; + $w->hostname = '-oProxy'; + + expect($w->getcommand())->toContain(" //'-oProxy' "); +}); + +it('passes an IPv6 address to wmic as one escaped argument after //', function () { + $w = new Linux_WMI('', new Wmic_Shell_Transport()); + $w->username = 'u'; + $w->password = 'p'; + $w->command = 'SELECT 1'; + $w->hostname = '2001:db8::1'; + + expect($w->getcommand())->toContain(" //'2001:db8::1' "); +}); diff --git a/wmi_tools.php b/wmi_tools.php index 5453f24..977a422 100644 --- a/wmi_tools.php +++ b/wmi_tools.php @@ -24,6 +24,7 @@ chdir('../../'); require('./include/auth.php'); +require_once($config['base_path'] . '/plugins/wmi/includes/functions.php'); require_once('./lib/snmp.php'); require_once('./lib/utility.php'); @@ -593,13 +594,22 @@ function walk_host() { $command = get_nfilter_request_var('command'); } - $host = strtolower($host); + $host = strtolower(trim($host)); if ($username == '' || $password == '' || $host == '') { print __('ERROR: You must provide a host, username, password and query', 'wmi'); exit; } + $valid = wmi_valid_host($host); + + if ($valid === false) { + print 'ERROR: The host must be an IPv4 or IPv6 address or a host name'; + exit; + } + + $host = $valid; + // Linux_WMI self-selects the PowerShell/CIM transport on a Windows // Cacti server and the wmic shell transport otherwise, so the same // client is used here regardless of $config['cacti_server_os']. @@ -680,22 +690,3 @@ function walk_host() { print nl2br(html_escape($wmi->error)); } } - -/** - * Validates that a string looks like a syntactically plausible hostname - * or IPv4 address (format only; does not perform DNS/network - * verification). Currently unused/dead code: not called from anywhere - * else in this file. - * - * @param string $host The candidate hostname or IP address. - * - * @return bool True when $host matches the expected hostname/IPv4 - * pattern. - */ -function is_valid_host($host) { - if (preg_match('/^((([0-9]{1,3}\.){3}[0-9]{1,3})|([0-9a-z-.]{0,61})?\.[a-z]{2,4})$/i', $host)) { - return true; - } - - return false; -} From 724884a8680d4cbac5963ca4bc28c96bfcf80558 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:26:36 -0600 Subject: [PATCH 2/6] fix(wmic): read stderr apart from stdout and stop a wmic that hangs Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + includes/linux_wmi.php | 123 +++++++++++++++++++++++++--- tests/Unit/WmiWmicStderrTest.php | 135 +++++++++++++++++++++++++++++++ 3 files changed, 249 insertions(+), 10 deletions(-) create mode 100644 tests/Unit/WmiWmicStderrTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 61437a2..4e8af32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue: Read wmic's stderr apart from its stdout, add it to the error text only, and stop a wmic that does not finish within two minutes * issue: Validate the Query Tool host as an IPv4 address, an IPv6 address or an RFC 1123 host name; the old check refused IPv6, short names and longer top level labels and was never called * issue: Record the WMI query start time from the database clock so devices are not polled again every cycle when PHP and the database use different time zones * security: Run the WMI Query Tool query action only for POST requests so credentials stay out of URLs, access logs and browser history diff --git a/includes/linux_wmi.php b/includes/linux_wmi.php index e8206e5..9282e6c 100644 --- a/includes/linux_wmi.php +++ b/includes/linux_wmi.php @@ -66,32 +66,50 @@ public function error(): ?string; } /** - * Default transport: the Linux `wmic` client invoked through the shell. + * Default transport: the Linux `wmic` client. + * + * stdout is the result; stderr is kept apart and only added to the error + * text. A wmic that does not finish within the timeout is stopped. */ class Wmic_Shell_Transport implements Wmi_Transport { private ?string $error = null; + /** + * @param int $timeout Seconds to wait for wmic before it is stopped. + */ + public function __construct(private int $timeout = 120) { + } + public function query(Wmi_Request $request): array|false { $this->error = null; - $output = []; - $status = 0; + $result = $this->run((DIRECTORY_SEPARATOR === '/' ? 'exec ' : '') . $this->build_command($request)); - exec($this->build_command($request), $output, $status); + if ($result === null) { + $this->error = 'ERROR: Unable to start ' . $request->binary; - if ($status !== 0) { - $this->error = 'ERROR: ' . implode("\n", $output); + return false; + } + + if ($result['timed_out']) { + $this->error = $this->with_stderr('ERROR: wmic did not finish within ' . $this->timeout . ' seconds', $result['stderr']); return false; } - if (count($output) === 0) { - $this->error = 'ERROR: WMI Returned no Data'; + if ($result['status'] !== 0) { + $this->error = $this->with_stderr('ERROR: ' . implode("\n", $result['lines']), $result['stderr']); return false; } - return $output; + if (count($result['lines']) === 0) { + $this->error = $this->with_stderr('ERROR: WMI Returned no Data', $result['stderr']); + + return false; + } + + return $result['lines']; } public function error(): ?string { @@ -102,7 +120,8 @@ public function error(): ?string { * Build the wmic command line. Every device-influenced value is shell * escaped; the hostname and namespace are additionally stripped of cmd.exe * metacharacters on win32. The delimiter is quoted so its pipe characters - * are not read as a shell pipeline. + * are not read as a shell pipeline. The hostname follows a literal `//`, so + * it can never be read as an option. */ public function build_command(Wmi_Request $request): string { $namespace = $request->namespace !== '' @@ -127,6 +146,90 @@ private function single_backslashes(string $value): string { return preg_replace('/\\\\+/', '\\\\', $value); } + /** + * Start a process with its stdout and stderr as pipes. + * + * @param array $pipes + * + * @return resource|false + */ + protected function start_process(string $command, ?array &$pipes) { + return proc_open($command, [0 => ['file', '/dev/null', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes); + } + + /** + * Run a command with stdout and stderr read separately, stopping it when + * it takes longer than the timeout. + * + * @return array{status: int, lines: array, stderr: string, timed_out: bool}|null + */ + private function run(string $command): ?array { + $process = $this->start_process($command, $pipes); + + if (!is_resource($process)) { + return null; + } + + $open = [1 => $pipes[1], 2 => $pipes[2]]; + $buffer = [1 => '', 2 => '']; + $deadline = microtime(true) + $this->timeout; + $expired = false; + + foreach ($open as $stream) { + stream_set_blocking($stream, false); + } + + while ($open !== []) { + $left = $deadline - microtime(true); + + if ($left <= 0) { + $expired = true; + + break; + } + + $read = $open; + $write = null; + $except = null; + + @stream_select($read, $write, $except, 0, (int) min(200000, $left * 1000000)); + + foreach ($read as $key => $stream) { + $chunk = fread($stream, 8192); + + if ($chunk === false || ($chunk === '' && feof($stream))) { + unset($open[$key]); + } else { + $buffer[$key] .= $chunk; + } + } + } + + if ($expired) { + proc_terminate($process, 9); + } + + fclose($pipes[1]); + fclose($pipes[2]); + + $status = proc_close($process); + + if (substr($buffer[1], -1) === "\n") { + $buffer[1] = substr($buffer[1], 0, -1); + } + + return [ + 'status' => $expired ? -1 : $status, + 'lines' => $buffer[1] === '' ? [] : array_map('rtrim', explode("\n", $buffer[1])), + 'stderr' => substr(trim($buffer[2]), 0, 2000), + 'timed_out' => $expired, + ]; + } + + private function with_stderr(string $message, string $stderr): string { + return $stderr === '' ? $message : ($message === 'ERROR: ' ? 'ERROR: ' : $message . "\n") . $stderr; + } + /** * A hostname or namespace never legitimately contains shell or cmd.exe * metacharacters. cmd.exe ignores \" , toggles quoting on every " , and diff --git a/tests/Unit/WmiWmicStderrTest.php b/tests/Unit/WmiWmicStderrTest.php new file mode 100644 index 0000000..4479cac --- /dev/null +++ b/tests/Unit/WmiWmicStderrTest.php @@ -0,0 +1,135 @@ + $lines stdout lines + */ +function wmi_stderrtest_wmic(array $lines = [], int $status = 0, string $stderr = '', int $sleep = 0): string { + $dir = sys_get_temp_dir() . '/wmi-stderrtest-' . bin2hex(random_bytes(6)); + + mkdir($dir, 0700); + file_put_contents($dir . '/out', $lines === [] ? '' : implode("\n", $lines) . "\n"); + file_put_contents($dir . '/stderr', $stderr); + file_put_contents($dir . '/wmic', "#!/bin/sh\nD=\"\$(dirname \"\$0\")\"\n" . + "if [ -s \"\$D/stderr\" ]; then cat \"\$D/stderr\" >&2; fi\n" . + ($sleep > 0 ? "exec sleep " . $sleep . "\n" : '') . + "cat \"\$D/out\"\nexit " . $status . "\n"); + chmod($dir . '/wmic', 0700); + + return $dir . '/wmic'; +} + +function wmi_stderrtest_request(string $binary): Wmi_Request { + return new Wmi_Request('host.example.com', 'u', 'p', 'root\\CIMV2', 'SELECT 1', '|+|', $binary); +} + +afterAll(function () { + foreach (glob(sys_get_temp_dir() . '/wmi-stderrtest-*') ?: [] as $dir) { + array_map('unlink', glob($dir . '/*') ?: []); + rmdir($dir); + } +}); + +it('keeps stderr out of the result', function () { + $t = new Wmic_Shell_Transport(); + + expect($t->query(wmi_stderrtest_request(wmi_stderrtest_wmic(['CLASS', 'Name', 'A'], 0, "warning: slow\n"))))->toBe(['CLASS', 'Name', 'A']); + expect($t->error())->toBeNull(); +}); + +it('adds stderr to the error text when wmic fails', function () { + $t = new Wmic_Shell_Transport(); + + expect($t->query(wmi_stderrtest_request(wmi_stderrtest_wmic(['NTSTATUS: denied'], 1, "ERROR: Login to remote object.\n"))))->toBeFalse(); + expect($t->error())->toBe("ERROR: NTSTATUS: denied\nERROR: Login to remote object."); +}); + +it('uses stderr as the whole detail when stdout is empty, and keeps the old text without it', function () { + $t = new Wmic_Shell_Transport(); + + $t->query(wmi_stderrtest_request(wmi_stderrtest_wmic([], 1, "ERROR: Login to remote object.\n"))); + + expect($t->error())->toBe('ERROR: ERROR: Login to remote object.'); + + $t->query(wmi_stderrtest_request(wmi_stderrtest_wmic([], 3))); + + expect($t->error())->toBe('ERROR: '); +}); + +it('reports no data, with stderr when there is some', function () { + $t = new Wmic_Shell_Transport(); + + expect($t->query(wmi_stderrtest_request(wmi_stderrtest_wmic([], 0))))->toBeFalse(); + expect($t->error())->toBe('ERROR: WMI Returned no Data'); + + $t->query(wmi_stderrtest_request(wmi_stderrtest_wmic([], 0, "note\n"))); + + expect($t->error())->toBe("ERROR: WMI Returned no Data\nnote"); +}); + +it('caps a long stderr in the error text', function () { + $t = new Wmic_Shell_Transport(); + + $t->query(wmi_stderrtest_request(wmi_stderrtest_wmic(['x'], 1, str_repeat('e', 5000)))); + + expect(strlen($t->error()))->toBe(strlen("ERROR: x\n") + 2000); +}); + +it('stops a wmic that does not finish within the timeout', function () { + $t = new Wmic_Shell_Transport(1); + $began = microtime(true); + + expect($t->query(wmi_stderrtest_request(wmi_stderrtest_wmic([], 0, "still connecting\n", 10))))->toBeFalse(); + expect(microtime(true) - $began)->toBeLessThan(6.0); + expect($t->error())->toBe("ERROR: wmic did not finish within 1 seconds\nstill connecting"); +}); + +it('keeps blank lines inside the output, trims line ends and drops only the final line break', function () { + $t = new Wmic_Shell_Transport(); + + expect($t->query(wmi_stderrtest_request(wmi_stderrtest_wmic(['CLASS', '', 'A ']))))->toBe(['CLASS', '', 'A']); +}); + +it('reports a wmic that cannot be started', function () { + $t = new Wmi_Test_Unstartable_Transport(); + + expect($t->query(wmi_stderrtest_request('/usr/bin/wmic')))->toBeFalse(); + expect($t->error())->toBe('ERROR: Unable to start /usr/bin/wmic'); +}); + +it('fails with the shell message when the executable does not exist', function () { + $t = new Wmic_Shell_Transport(); + + expect($t->query(wmi_stderrtest_request('/nonexistent/wmic-for-tests')))->toBeFalse(); + expect($t->error())->toStartWith('ERROR: '); + expect($t->error())->toContain('wmic-for-tests'); +}); From ec357dba951ba1cd4d7ed59fe6ef9f5446e6afab Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:27:42 -0600 Subject: [PATCH 3/6] fix(wmic): pass credentials in a private authentication file Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + includes/linux_wmi.php | 120 ++++++++++++++- tests/Unit/WmiWmicAuthFileTest.php | 236 +++++++++++++++++++++++++++++ 3 files changed, 349 insertions(+), 8 deletions(-) create mode 100644 tests/Unit/WmiWmicAuthFileTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e8af32..5ad832c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* security: Pass the Windows credentials to wmic in a private authentication file instead of --user and --password arguments, so they no longer show up in the process list * issue: Read wmic's stderr apart from its stdout, add it to the error text only, and stop a wmic that does not finish within two minutes * issue: Validate the Query Tool host as an IPv4 address, an IPv6 address or an RFC 1123 host name; the old check refused IPv6, short names and longer top level labels and was never called * issue: Record the WMI query start time from the database clock so devices are not polled again every cycle when PHP and the database use different time zones diff --git a/includes/linux_wmi.php b/includes/linux_wmi.php index 9282e6c..7ebc2e6 100644 --- a/includes/linux_wmi.php +++ b/includes/linux_wmi.php @@ -68,22 +68,45 @@ public function error(): ?string; /** * Default transport: the Linux `wmic` client. * - * stdout is the result; stderr is kept apart and only added to the error - * text. A wmic that does not finish within the timeout is stopped. + * The credentials go to wmic in a private authentication file (-A) so they + * never appear in the process list. stdout is the result; stderr is kept + * apart and only added to the error text. */ class Wmic_Shell_Transport implements Wmi_Transport { private ?string $error = null; /** - * @param int $timeout Seconds to wait for wmic before it is stopped. + * @param int $timeout Seconds to wait for wmic before it is stopped. + * @param string|null $temp_dir Directory for the authentication file; the + * system temporary directory when null. */ - public function __construct(private int $timeout = 120) { + public function __construct( + private int $timeout = 120, + private ?string $temp_dir = null + ) { } public function query(Wmi_Request $request): array|false { $this->error = null; - $result = $this->run((DIRECTORY_SEPARATOR === '/' ? 'exec ' : '') . $this->build_command($request)); + if (preg_match('/[\r\n\0]/', $request->username . $request->password)) { + $this->error = 'ERROR: WMI credentials must not contain line breaks'; + + return false; + } + + $auth = $this->create_auth_file($request); + + try { + $result = $this->run( + (DIRECTORY_SEPARATOR === '/' ? 'exec ' : '') . $this->build_command($request, $auth['file'] ?? null) + ); + } finally { + if ($auth !== null) { + @unlink($auth['file']); + @rmdir($auth['dir']); + } + } if ($result === null) { $this->error = 'ERROR: Unable to start ' . $request->binary; @@ -122,16 +145,24 @@ public function error(): ?string { * metacharacters on win32. The delimiter is quoted so its pipe characters * are not read as a shell pipeline. The hostname follows a literal `//`, so * it can never be read as an option. + * + * @param string|null $auth_file Credentials file for --authentication-file; + * without it the credentials are passed as + * arguments, which any local user can read. */ - public function build_command(Wmi_Request $request): string { + public function build_command(Wmi_Request $request, ?string $auth_file = null): string { $namespace = $request->namespace !== '' ? ' --namespace=' . cacti_escapeshellarg($this->single_backslashes($this->strip_metachars($request->namespace))) : ''; + $credentials = $auth_file !== null + ? ' --authentication-file=' . cacti_escapeshellarg($auth_file) + : ' --user=' . cacti_escapeshellarg($request->username) . + ' --password=' . cacti_escapeshellarg($request->password); + return cacti_escapeshellarg($request->binary) . ' --delimiter=' . cacti_escapeshellarg($request->separator) . - ' --user=' . cacti_escapeshellarg($request->username) . - ' --password=' . cacti_escapeshellarg($request->password) . + $credentials . $namespace . ' //' . cacti_escapeshellarg($this->strip_metachars(trim($request->hostname))) . ' ' . cacti_escapeshellarg($request->query); @@ -146,6 +177,63 @@ private function single_backslashes(string $value): string { return preg_replace('/\\\\+/', '\\\\', $value); } + /** + * Write the credentials to a file only this user can read, inside a + * directory with a random name. Returns null, after logging a warning, + * when that is not possible; the caller then falls back to arguments. + * + * @return array{dir: string, file: string}|null + */ + private function create_auth_file(Wmi_Request $request): ?array { + $base = $this->temp_dir ?? sys_get_temp_dir(); + $dir = rtrim($base, '/') . '/wmi-' . bin2hex(random_bytes(8)); + + if (!is_dir($base) || !is_writable($base) || !@mkdir($dir, 0700)) { + cacti_log('WARNING: Unable to create a private directory for the wmic credentials; passing them as arguments', false, 'WMI'); + + return null; + } + + $file = $dir . '/' . bin2hex(random_bytes(8)); + $handle = $this->open_new_file($file); + $written = false; + + if ($handle !== false) { + $content = $this->auth_file_content($request); + $written = fwrite($handle, $content) === strlen($content); + + fclose($handle); + } + + if (!$written || !@chmod($file, 0600)) { + if (is_file($file)) { + unlink($file); + } + + rmdir($dir); + + cacti_log('WARNING: Unable to write the wmic credentials file; passing the credentials as arguments', false, 'WMI'); + + return null; + } + + return ['dir' => $dir, 'file' => $file]; + } + + /** + * Create a file that must not exist yet, readable only by this user. + * + * @return resource|false + */ + protected function open_new_file(string $file) { + $old = umask(0077); + $handle = @fopen($file, 'x'); + + umask($old); + + return $handle; + } + /** * Start a process with its stdout and stderr as pipes. * @@ -157,6 +245,22 @@ protected function start_process(string $command, ?array &$pipes) { return proc_open($command, [0 => ['file', '/dev/null', 'r'], 1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes); } + /** + * The samba credentials file format. A DOMAIN\user or DOMAIN/user name is + * split the way wmic splits --user. + */ + private function auth_file_content(Wmi_Request $request): string { + $content = ''; + $user = $request->username; + + if (preg_match('#^([^\\\\/]+)[\\\\/](.+)$#', $user, $match)) { + $content = 'domain=' . $match[1] . "\n"; + $user = $match[2]; + } + + return 'username=' . $user . "\n" . 'password=' . $request->password . "\n" . $content; + } + /** * Run a command with stdout and stderr read separately, stopping it when * it takes longer than the timeout. diff --git a/tests/Unit/WmiWmicAuthFileTest.php b/tests/Unit/WmiWmicAuthFileTest.php new file mode 100644 index 0000000..3e38c29 --- /dev/null +++ b/tests/Unit/WmiWmicAuthFileTest.php @@ -0,0 +1,236 @@ +failure === self::NO_FILE) { + return false; + } + + if ($this->failure === self::SHORT_FILE) { + // A stream that cannot be written to, so the write comes up short. + return fopen('php://memory', 'r'); + } + + return parent::open_new_file($file); + } +} + +/** + * Create a stand-in wmic and the directory it records into. + * + * @param array $lines stdout lines + * + * @return array{binary: string, rec: string, tmp: string} + */ +function wmi_authtest_env(array $lines = [], int $status = 0, string $stderr = '', int $sleep = 0): array { + $root = sys_get_temp_dir() . '/wmi-authtest-' . bin2hex(random_bytes(6)); + $rec = $root . '/rec'; + $tmp = $root . '/tmp'; + + mkdir($root, 0700); + mkdir($rec, 0700); + mkdir($tmp, 0700); + + file_put_contents($rec . '/out', $lines === [] ? '' : implode("\n", $lines) . "\n"); + file_put_contents($rec . '/status', (string) $status); + file_put_contents($rec . '/stderr', $stderr); + file_put_contents($rec . '/sleep', (string) $sleep); + + file_put_contents($root . '/wmic', <<<'SH' +#!/bin/sh +REC="$(dirname "$0")/rec" +AUTH="" +for a in "$@"; do + case "$a" in + --authentication-file=*) AUTH="${a#--authentication-file=}" ;; + esac +done +printf '%s\n' "$@" > "$REC/argv" +if [ -n "$AUTH" ]; then + cp "$AUTH" "$REC/auth_content" + printf '%s' "$AUTH" > "$REC/auth_path" + stat -c '%a' "$AUTH" > "$REC/auth_mode" 2>/dev/null || stat -f '%Lp' "$AUTH" > "$REC/auth_mode" + stat -c '%a' "$(dirname "$AUTH")" > "$REC/dir_mode" 2>/dev/null || stat -f '%Lp' "$(dirname "$AUTH")" > "$REC/dir_mode" +fi +if [ -s "$REC/stderr" ]; then cat "$REC/stderr" >&2; fi +if [ "$(cat "$REC/sleep")" != "0" ]; then exec sleep "$(cat "$REC/sleep")"; fi +cat "$REC/out" +exit "$(cat "$REC/status")" +SH + ); + chmod($root . '/wmic', 0700); + + return ['binary' => $root . '/wmic', 'rec' => $rec, 'tmp' => $tmp]; +} + +function wmi_authtest_request(array $env, string $user = 'u', string $password = 'p'): Wmi_Request { + return new Wmi_Request('host.example.com', $user, $password, 'root\\CIMV2', 'SELECT 1', '|+|', $env['binary']); +} + +function wmi_authtest_recorded(array $env, string $name): string { + return is_file($env['rec'] . '/' . $name) ? (string) file_get_contents($env['rec'] . '/' . $name) : ''; +} + +afterAll(function () { + foreach (glob(sys_get_temp_dir() . '/wmi-authtest-*') ?: [] as $root) { + foreach (['rec', 'tmp'] as $sub) { + foreach (glob($root . '/' . $sub . '/*') ?: [] as $entry) { + if (is_dir($entry)) { + array_map('unlink', glob($entry . '/*') ?: []); + rmdir($entry); + } else { + unlink($entry); + } + } + + @rmdir($root . '/' . $sub); + } + + @unlink($root . '/wmic'); + @rmdir($root); + } +}); + +beforeEach(function () { + $GLOBALS['__test_cacti_log'] = array(); +}); + +it('keeps the credentials out of the arguments and gives wmic a private file', function () { + $env = wmi_authtest_env(['CLASS', 'Name', 'A']); + $t = new Wmic_Shell_Transport(120, $env['tmp']); + + $result = $t->query(wmi_authtest_request($env, 'DOM\\svc', 'S3cret pass')); + $argv = wmi_authtest_recorded($env, 'argv'); + + expect($result)->toBe(['CLASS', 'Name', 'A']); + expect($t->error())->toBeNull(); + expect($argv)->not->toContain('--user'); + expect($argv)->not->toContain('--password'); + expect($argv)->not->toContain('S3cret'); + expect($argv)->toContain('--authentication-file='); + expect($argv)->toContain('//host.example.com'); + expect(wmi_authtest_recorded($env, 'auth_content'))->toBe("username=svc\npassword=S3cret pass\ndomain=DOM\n"); + expect(trim(wmi_authtest_recorded($env, 'auth_mode')))->toBe('600'); + expect(trim(wmi_authtest_recorded($env, 'dir_mode')))->toBe('700'); +}); + +it('removes the credentials file and its directory after a run', function () { + $env = wmi_authtest_env(['CLASS']); + + (new Wmic_Shell_Transport(120, $env['tmp']))->query(wmi_authtest_request($env)); + + expect(file_exists(wmi_authtest_recorded($env, 'auth_path')))->toBeFalse(); + expect(glob($env['tmp'] . '/*'))->toBe(array()); +}); + +it('removes the credentials file when wmic fails or times out', function (array $env, int $timeout) { + $t = new Wmic_Shell_Transport($timeout, $env['tmp']); + + expect($t->query(wmi_authtest_request($env)))->toBeFalse(); + expect(wmi_authtest_recorded($env, 'auth_path'))->not->toBe(''); + expect(glob($env['tmp'] . '/*'))->toBe(array()); +})->with([ + 'non-zero exit' => fn () => [wmi_authtest_env(['NTSTATUS: denied'], 1), 120], + 'timeout' => fn () => [wmi_authtest_env([], 0, '', 10), 1], +]); + +it('writes passwords with special characters unchanged', function (string $password) { + $env = wmi_authtest_env(['CLASS']); + + (new Wmic_Shell_Transport(120, $env['tmp']))->query(wmi_authtest_request($env, 'svc', $password)); + + expect(wmi_authtest_recorded($env, 'auth_content'))->toBe("username=svc\npassword=" . $password . "\n"); +})->with([ + 'equals' => 'a=b=c', + 'quotes' => 'it\'s "quoted"', + 'spaces' => 'two words ', + 'shell' => '$(x) `y` ; | & > <', + 'backslash' => 'a\\b\\\\c', + 'hash' => '#not-a-comment', + 'unicode' => "p\xc3\xa4ss", +]); + +it('splits DOMAIN/user and leaves a UPN user name whole', function (string $user, string $expected) { + $env = wmi_authtest_env(['CLASS']); + + (new Wmic_Shell_Transport(120, $env['tmp']))->query(wmi_authtest_request($env, $user, 'pw')); + + expect(wmi_authtest_recorded($env, 'auth_content'))->toBe($expected); +})->with([ + 'backslash' => ['DOM\\svc', "username=svc\npassword=pw\ndomain=DOM\n"], + 'slash' => ['DOM/svc', "username=svc\npassword=pw\ndomain=DOM\n"], + 'upn' => ['svc@corp.example', "username=svc@corp.example\npassword=pw\n"], + 'plain' => ['svc', "username=svc\npassword=pw\n"], +]); + +it('refuses credentials with line breaks and never starts wmic', function (string $user, string $password) { + $env = wmi_authtest_env(['CLASS']); + $t = new Wmic_Shell_Transport(120, $env['tmp']); + + expect($t->query(wmi_authtest_request($env, $user, $password)))->toBeFalse(); + expect($t->error())->toBe('ERROR: WMI credentials must not contain line breaks'); + expect(wmi_authtest_recorded($env, 'argv'))->toBe(''); + expect(glob($env['tmp'] . '/*'))->toBe(array()); +})->with([ + 'newline in password' => ['u', "a\nb"], + 'return in password' => ['u', "a\rb"], + 'nul in password' => ['u', "a\0b"], + 'newline in user' => ["u\nusername=x", 'p'], +]); + +it('falls back to arguments with a warning when the credentials file cannot be made', function (string $failure) { + $env = wmi_authtest_env(['CLASS']); + $t = new Wmi_Test_Failing_Transport($failure, $env['tmp']); + + expect($t->query(wmi_authtest_request($env, 'svc', 'pw')))->toBe(['CLASS']); + expect(wmi_authtest_recorded($env, 'argv'))->toContain('--user=svc'); + expect(wmi_authtest_recorded($env, 'argv'))->toContain('--password=pw'); + expect($GLOBALS['__test_cacti_log'][0])->toContain('Unable to write the wmic credentials file'); + expect(glob($env['tmp'] . '/*'))->toBe(array()); +})->with([Wmi_Test_Failing_Transport::NO_FILE, Wmi_Test_Failing_Transport::SHORT_FILE]); + +it('falls back to arguments with a warning when no private directory can be made', function () { + $env = wmi_authtest_env(['CLASS']); + $t = new Wmic_Shell_Transport(120, $env['tmp'] . '/missing/deeper'); + + expect($t->query(wmi_authtest_request($env, 'svc', 'pw')))->toBe(['CLASS']); + expect(wmi_authtest_recorded($env, 'argv'))->toContain('--user=svc'); + expect($GLOBALS['__test_cacti_log'][0])->toContain('Unable to create a private directory'); +}); + +it('builds the legacy command line without a credentials file', function () { + $env = wmi_authtest_env(); + $cmd = (new Wmic_Shell_Transport())->build_command(wmi_authtest_request($env, 'svc', 'pw')); + + expect($cmd)->toContain(" --user='svc' --password='pw'"); + expect($cmd)->not->toContain('--authentication-file'); +}); From 308d8ea0f73ef08b4f429781d50e5a683fab947c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:28:48 -0600 Subject: [PATCH 4/6] fix(scripts): point the script-server resource at real files and register path_wmi Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + includes/functions.php | 2 +- locales/po/cacti.pot | 8 + script/wmi-script.php | 8 +- setup.php | 12 ++ .../script_server/exchange_messages.xml | 2 +- tests/Unit/WmiScriptServerTest.php | 142 ++++++++++++++++++ tests/bootstrap-unit.php | 2 +- wmi_script.php | 3 +- 9 files changed, 171 insertions(+), 9 deletions(-) create mode 100644 tests/Unit/WmiScriptServerTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ad832c..a7afbe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue: Point the script-server resource and its scripts at the files that exist, load Cacti from where it is, and register the wmic path as the path_wmi setting (default /usr/bin/wmic) * security: Pass the Windows credentials to wmic in a private authentication file instead of --user and --password arguments, so they no longer show up in the process list * issue: Read wmic's stderr apart from its stdout, add it to the error text only, and stop a wmic that does not finish within two minutes * issue: Validate the Query Tool host as an IPv4 address, an IPv6 address or an RFC 1123 host name; the old check refused IPv6, short names and longer top level labels and was never called diff --git a/includes/functions.php b/includes/functions.php index 6682b97..ac5fae8 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -331,7 +331,7 @@ function plugin_wmi_create_resource_xml($id) { if (isset($wmic['id'])) { $data = "\n"; $data .= ' ' . $wmic['name'] . "\n"; - $data .= " |path_cacti|/scripts/wmi-script.php\n"; + $data .= " |path_cacti|/plugins/wmi/script/wmi-script.php\n"; $data .= " wmi_script\n"; $data .= ' WMI Query for ' . $wmic['name'] . "\n"; $data .= " php\n"; diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index af6a14a..33ced2d 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -566,3 +566,11 @@ msgstr "" #: wmi_tools.php msgid "Showing columns and first one or two rows of data." msgstr "" + +#: setup.php +msgid "WMI Client (wmic) Path" +msgstr "" + +#: setup.php +msgid "The path to the wmic binary used to query Windows devices from a Linux Cacti server." +msgstr "" diff --git a/script/wmi-script.php b/script/wmi-script.php index 91152df..9f5bf87 100644 --- a/script/wmi-script.php +++ b/script/wmi-script.php @@ -23,7 +23,7 @@ */ if (!isset($called_by_script_server)) { - require_once(__DIR__ . '/../include/cli_check.php'); + require_once(__DIR__ . '/../../../include/cli_check.php'); array_shift($_SERVER['argv']); if (isset($_SERVER['argv'][0]) && $_SERVER['argv'][0] == 'wmi_script') { @@ -35,8 +35,8 @@ /** * Script-server/CLI entry point that runs a named saved WMI query against - * a remote host (via the Linux_WMI wrapper around this plugin's bundled - * wmic binary) and either lists its index keys, prints an indexed + * a remote host (via the Linux_WMI wrapper around the wmic client set in + * the path_wmi setting) and either lists its index keys, prints an indexed * key/value pair, or prints a single fetched value, depending on $cmd. * Called by Cacti's script server (or directly from the CLI) when a Data * Input Method configured to use this script is polled. @@ -70,7 +70,7 @@ function wmi_script($hostname, $host_id, $wmiquery, $cmd = '', $arg1 = '', $arg2 $wmi = new Linux_WMI($host_id); $wmi->hostname = $hostname; - $wmi->binary = $config['base_path'] . '/plugins/wmi/wmic'; + $wmi->binary = read_config_option('path_wmi') ?: '/usr/bin/wmic'; // Fetch the info for this WMI query from the database, exit if not found $wmiinfo = db_fetch_row_prepared('SELECT * FROM wmi_wql_queries WHERE name = ?', [$wmiquery], false); diff --git a/setup.php b/setup.php index 3971a9e..eb2554d 100644 --- a/setup.php +++ b/setup.php @@ -528,6 +528,18 @@ function wmi_config_settings() { } else { $settings['misc'] = $temp; } + + if (!isset($settings['path'])) { + $settings['path'] = []; + } + + $settings['path']['path_wmi'] = [ + 'friendly_name' => __('WMI Client (wmic) Path', 'wmi'), + 'description' => __('The path to the wmic binary used to query Windows devices from a Linux Cacti server.', 'wmi'), + 'method' => 'filepath', + 'max_length' => '255', + 'default' => '/usr/bin/wmic' + ]; } /** diff --git a/templates/resource/script_server/exchange_messages.xml b/templates/resource/script_server/exchange_messages.xml index 4e71f21..92d650b 100644 --- a/templates/resource/script_server/exchange_messages.xml +++ b/templates/resource/script_server/exchange_messages.xml @@ -1,6 +1,6 @@ Exchange Messages - |path_cacti|/scripts/wmi-script.php + |path_cacti|/plugins/wmi/script/wmi-script.php wmi_script Queries an Exchange 2003 Server for its Mail Stores and the number of Messages Send and Delivered diff --git a/tests/Unit/WmiScriptServerTest.php b/tests/Unit/WmiScriptServerTest.php new file mode 100644 index 0000000..5b29846 --- /dev/null +++ b/tests/Unit/WmiScriptServerTest.php @@ -0,0 +1,142 @@ +\|path_cacti\|/plugins/wmi/([^<]+)#', $xml, $m))->toBe(1); + expect(is_file(WMI_SS_PLUGIN . '/' . $m[1]))->toBeTrue(); +}); + +it('exports a resource that points at a script that exists', function () { + $GLOBALS['__test_fetch_row_map'] = array('wmi_wql_queries' => array('id' => 3, 'name' => 'svc', 'primary_key' => 'Name')); + + $xml = plugin_wmi_create_resource_xml(3); + + $GLOBALS['__test_fetch_row_map'] = array(); + + expect(preg_match('#\|path_cacti\|/plugins/wmi/([^<]+)#', $xml, $m))->toBe(1); + expect(is_file(WMI_SS_PLUGIN . '/' . $m[1]))->toBeTrue(); + expect($xml)->toContain('wmi_script'); +}); + +it('loads Cacti files the scripts need from where they are', function (string $script) { + $source = (string) file_get_contents(WMI_SS_PLUGIN . '/' . $script); + + expect(preg_match_all("#require(?:_once)?\(__DIR__ \. '([^']+)'\)#", $source, $m))->toBeGreaterThan(0); + + foreach ($m[1] as $relative) { + expect(is_file(dirname(WMI_SS_PLUGIN . '/' . $script) . $relative))->toBeTrue(); + } + + expect($source)->not->toContain('global.php'); + expect($source)->not->toContain('/plugins/wmi/wmic'); +})->with(['wmi_script.php', 'script/wmi-script.php']); + +it('registers the wmic path as a setting with a default', function () { + global $settings, $tabs; + + $settings = array(); + $tabs = array(); + + wmi_config_settings(); + + expect($settings['path']['path_wmi']['default'])->toBe('/usr/bin/wmic'); + expect($settings['path']['path_wmi']['method'])->toBe('filepath'); +}); + +it('keeps the settings that are already on the path tab', function () { + global $settings, $tabs; + + $settings = array('path' => array('path_rrdtool' => array('default' => '/usr/bin/rrdtool'))); + $tabs = array(); + + wmi_config_settings(); + + expect(array_keys($settings['path']))->toBe(array('path_rrdtool', 'path_wmi')); +}); + +/** + * Run the script-server function against a stand-in wmic. + * + * @param array $config + */ +function wmi_ss_run(array $config, string $command, string $arg1, string $arg2): string { + $called_by_script_server = true; + + require_once WMI_SS_PLUGIN . '/script/wmi-script.php'; + + $w = new Linux_WMI(); + + $GLOBALS['__test_config'] = $config; + $GLOBALS['__test_fetch_row_map'] = array( + 'wmi_user_accounts' => array('username' => 'svc', 'password' => $w->encode('pw')), + 'wmi_wql_queries' => array('query' => 'SELECT Name,State FROM Win32_Service', 'primary_key' => 'Name', 'namespace' => 'root\\CIMV2'), + ); + + ob_start(); + + try { + $returned = wmi_script('winsrv', 5, 'svc', $command, $arg1, $arg2); + } finally { + $output = ob_get_clean(); + } + + $GLOBALS['__test_config'] = array(); + $GLOBALS['__test_fetch_row_map'] = array(); + + return $output . (is_string($returned) ? $returned : ''); +} + +function wmi_ss_wmic(): string { + $dir = sys_get_temp_dir() . '/wmi-ss-' . bin2hex(random_bytes(6)); + + mkdir($dir, 0700); + file_put_contents($dir . '/wmic', "#!/bin/sh\nprintf '%s\\n' 'CLASS: Win32_Service' 'Name|+|State' 'Spooler|+|Running' 'Dhcp|+|Stopped'\n"); + chmod($dir . '/wmic', 0700); + + return $dir . '/wmic'; +} + +it('runs the script-server query through the wmic set in the setting', function () { + $wmic = wmi_ss_wmic(); + + expect(wmi_ss_run(array('path_wmi' => $wmic), 'get', 'State', 'Spooler'))->toContain('Running'); + expect(wmi_ss_run(array('path_wmi' => $wmic), 'query', 'index', ''))->toContain('Spooler!Spooler'); + expect(wmi_ss_run(array('path_wmi' => $wmic), 'query', 'State', 'Dhcp'))->toContain("State!Stopped"); + expect(wmi_ss_run(array('path_wmi' => $wmic), 'index', '', ''))->toContain('Spooler'); + + unlink($wmic); + rmdir(dirname($wmic)); +}); + +it('falls back to /usr/bin/wmic when the setting is empty', function () { + if (is_file('/usr/bin/wmic')) { + $this->markTestSkipped('A real wmic is installed.'); + } + + expect(wmi_ss_run(array(), 'get', 'State', 'Spooler'))->toBe(''); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 1ff317f..63f5f1a 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -169,7 +169,7 @@ function api_plugin_db_table_create($plugin, $table, $data) { if (!function_exists('read_config_option')) { function read_config_option($name, $force = false) { - return ''; + return $GLOBALS['__test_config'][$name] ?? ''; } } diff --git a/wmi_script.php b/wmi_script.php index 988da82..02844f2 100644 --- a/wmi_script.php +++ b/wmi_script.php @@ -26,7 +26,6 @@ require_once($config['base_path'] . '/lib/snmp.php'); if (!isset($called_by_script_server)) { - require_once(__DIR__ . '/../include/global.php'); array_shift($_SERVER['argv']); if (isset($_SERVER['argv'][0]) && $_SERVER['argv'][0] == 'wmi_script') { @@ -72,7 +71,7 @@ function wmi_script($hostname, $host_id, $wmiquery, $cmd = '', $arg1 = '', $arg2 $wmi = new Linux_WMI($host_id); $wmi->hostname = $hostname; - $wmi->binary = '/usr/bin/wmic'; + $wmi->binary = read_config_option('path_wmi') ?: '/usr/bin/wmic'; // Fetch the info for this WMI query from the database, exit if not found $wmiinfo = db_fetch_row_prepared('SELECT * From 07364cbcfca7d946745510d471ad6d5b70237f2d Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:29:54 -0600 Subject: [PATCH 5/6] test(wmic): cover removing a credentials file left by a short write Signed-off-by: Thomas Vincent --- tests/Unit/WmiWmicAuthFileTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Unit/WmiWmicAuthFileTest.php b/tests/Unit/WmiWmicAuthFileTest.php index 3e38c29..9da7c17 100644 --- a/tests/Unit/WmiWmicAuthFileTest.php +++ b/tests/Unit/WmiWmicAuthFileTest.php @@ -36,7 +36,9 @@ protected function open_new_file(string $file) { } if ($this->failure === self::SHORT_FILE) { - // A stream that cannot be written to, so the write comes up short. + // The file exists, but the stream cannot be written to, so the write comes up short. + file_put_contents($file, ''); + return fopen('php://memory', 'r'); } From 40f0ad119767305ba532837572bd7eac55397bf1 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 01:50:08 -0600 Subject: [PATCH 6/6] test(e2e): cover the credentials file, host validation and the scripts end to end Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + tests/E2E/CollectorFlowTest.php | 23 +++++++++++++++++++++-- tests/E2E/PagesFlowTest.php | 27 +++++++++++++++++++++++++++ tests/E2E/collector_flow.php | 19 +++++++++++++++++++ tests/E2E/e2e_common.php | 27 ++++++++++++++++++++++++++- tests/E2E/pages_flow.php | 22 ++++++++++++++++++++++ 6 files changed, 116 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7afbe0..5bf1599 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* dev: Extend the end-to-end fake wmic with --authentication-file and a stderr line, and cover host validation, the credentials file and the script-server scripts end to end * issue: Point the script-server resource and its scripts at the files that exist, load Cacti from where it is, and register the wmic path as the path_wmi setting (default /usr/bin/wmic) * security: Pass the Windows credentials to wmic in a private authentication file instead of --user and --password arguments, so they no longer show up in the process list * issue: Read wmic's stderr apart from its stdout, add it to the error text only, and stop a wmic that does not finish within two minutes diff --git a/tests/E2E/CollectorFlowTest.php b/tests/E2E/CollectorFlowTest.php index 970e524..26dd9d3 100644 --- a/tests/E2E/CollectorFlowTest.php +++ b/tests/E2E/CollectorFlowTest.php @@ -47,12 +47,31 @@ function wmi_e2e_collector(): array { it('passes the credentials and query to wmic', function () { $argv = wmi_e2e_collector()['first']['argv']; - expect($argv)->toContain('--user=dom\\user'); - expect($argv)->toContain('--password=pw'); + expect($argv)->not->toContain('--user=dom\\user'); + expect($argv)->not->toContain('--password=pw'); expect($argv)->toContain('//winsrv1.invalid'); expect($argv)->toContain('SELECT Name,State FROM Win32_Service'); }); + it('hands the credentials to wmic in a private file that is gone afterwards', function () { + $o = wmi_e2e_collector(); + + expect(array_filter($o['first']['argv'], fn ($a) => strpos($a, '--authentication-file=') === 0))->toHaveCount(1); + expect($o['first_auth']['content'])->toBe("username=user\npassword=pw\ndomain=dom\n"); + expect($o['first_auth']['mode'])->toBe('600'); + expect($o['first_auth']['exists'])->toBeFalse(); + }); + + it('runs the script-server scripts from the command line against the cache data', function (string $script) { + $o = wmi_e2e_collector()['script_' . $script]; + + expect($o['index'])->toContain('A!A'); + expect($o['index'])->toContain('C!C'); + expect($o['pair'])->toContain('State!Stopped'); + expect($o['list'])->toContain('B'); + expect($o['none'])->toBe(''); + })->with(['dir', 'root']); + it('purges an instance the device stopped returning and updates the rest', function () { $o = wmi_e2e_collector(); diff --git a/tests/E2E/PagesFlowTest.php b/tests/E2E/PagesFlowTest.php index b9ba7b5..555c96a 100644 --- a/tests/E2E/PagesFlowTest.php +++ b/tests/E2E/PagesFlowTest.php @@ -167,6 +167,33 @@ function wmi_e2e_pages(): array { expect(wmi_e2e_pages()['tool_namespaces'][$form])->toBe(['--namespace=root\\CIMV2']); })->with(['empty', 'single', 'double']); + it('passes the tool credentials to wmic in a private file, not as arguments', function () { + $o = wmi_e2e_pages()['tool_run']; + + expect(array_filter($o['argv'], fn ($a) => strpos($a, '--password') === 0 || strpos($a, '--user') === 0))->toBe(array()); + expect($o['auth']['content'])->toStartWith("username=tool\npassword=T00l-secret-"); + expect($o['auth']['content'])->toEndWith("domain=DOM\n"); + expect($o['auth']['mode'])->toBe('600'); + expect($o['auth']['exists'])->toBeFalse(); + }); + + it('refuses hosts that are not an address or a name and runs nothing', function (string $host) { + $o = wmi_e2e_pages()['tool_hosts']['refused'][$host]; + + expect($o['message'])->toBeTrue(); + expect($o['calls'])->toBe(0); + })->with(['bad host', '-oProxy=x', 'a..b', 'fe80::1%eth0', '999.1.1.1', 'host;id']); + + it('accepts short names, IPv4 and IPv6 hosts and passes them after the // prefix', function (string $host, string $expected) { + expect(wmi_e2e_pages()['tool_hosts']['reached'][$host])->toBe($expected); + })->with([ + ['win-server', '//win-server'], + ['[2001:db8::1]', '//2001:db8::1'], + ['10.0.0.5', '//10.0.0.5'], + ['srv.corp.local', '//srv.corp.local'], + ['2001:0db8:0:0:0:0:0:1', '//2001:0db8:0:0:0:0:0:1'], + ]); + it('does not run a tool query without a CSRF token or the WMI realm', function () { $o = wmi_e2e_pages(); diff --git a/tests/E2E/collector_flow.php b/tests/E2E/collector_flow.php index f9fddf1..cf01c07 100644 --- a/tests/E2E/collector_flow.php +++ b/tests/E2E/collector_flow.php @@ -66,6 +66,25 @@ function refresh(int $host = HOST_A, int $query = QUERY_SVC): array { // First refresh caches every row. wmi_e2e_wmic_default(wmi_e2e_result('Win32_Service', $svc_header, [['A', 'Running'], ['B', 'Running'], ['C', 'Stopped']]), 0); $out['first'] = refresh(); +$out['first_auth'] = wmi_e2e_last_auth(); + +// The script-server scripts, run from the command line, against the same data. +$script = function (string $file, array $arguments) use ($root): string { + $lines = []; + + exec(escapeshellarg(PHP_BINARY) . ' ' . escapeshellarg($root . '/plugins/wmi/' . $file) . ' ' . implode(' ', array_map('escapeshellarg', $arguments)) . ' 2>&1', $lines); + + return implode("\n", $lines); +}; + +foreach (['dir' => 'script/wmi-script.php', 'root' => 'wmi_script.php'] as $label => $file) { + $out['script_' . $label] = [ + 'index' => $script($file, ['winsrv1.invalid', (string) HOST_A, 'e2e-svc', 'query', 'index']), + 'pair' => $script($file, ['winsrv1.invalid', (string) HOST_A, 'e2e-svc', 'query', 'State', 'C']), + 'list' => $script($file, ['winsrv1.invalid', (string) HOST_A, 'e2e-svc', 'index']), + 'none' => $script($file, ['winsrv1.invalid', (string) HOST_A, 'no-such-query', 'query', 'index']), + ]; +} // The device drops an instance, which is purged. wmi_e2e_wmic_default(wmi_e2e_result('Win32_Service', $svc_header, [['A', 'Running'], ['B', 'Stopped']]), 0); diff --git a/tests/E2E/e2e_common.php b/tests/E2E/e2e_common.php index b8f7ea1..f713b30 100644 --- a/tests/E2E/e2e_common.php +++ b/tests/E2E/e2e_common.php @@ -122,7 +122,7 @@ function wmi_e2e_finish(): void { db_execute_prepared('DELETE FROM plugin_config WHERE directory = ?', ['wmi']); } - foreach (['wmic', 'out', 'status', 'argv', 'calls', 'rules'] as $name) { + foreach (['wmic', 'out', 'status', 'argv', 'calls', 'rules', 'auth_content', 'auth_path', 'auth_mode'] as $name) { $path = $state['work'] . '/' . $name; if (is_file($path)) { @@ -181,6 +181,17 @@ function wmi_e2e_write_wmic(): void { ALL="$*" printf '%s\n' "$@" > "$W/argv" printf '%s\n' "$ALL" >> "$W/calls" +for a in "$@"; do + case "$a" in + --authentication-file=*) + AUTH="${a#--authentication-file=}" + cp "$AUTH" "$W/auth_content" + printf '%s' "$AUTH" > "$W/auth_path" + stat -c '%a' "$AUTH" > "$W/auth_mode" 2>/dev/null || stat -f '%Lp' "$AUTH" > "$W/auth_mode" + ;; + esac +done +echo "wmic: note on stderr" >&2 OUT="$W/out" ST="$(cat "$W/status")" TAB="$(printf '\t')" @@ -278,6 +289,20 @@ function wmi_e2e_calls(): array { return is_file($path) ? array_values(array_filter(explode("\n", (string) file_get_contents($path)), 'strlen')) : []; } +/** + * What the last wmic call recorded about its credentials file: the content, + * its mode, and whether it still exists. + * + * @return array{content: string, mode: string, path: string, exists: bool} + */ +function wmi_e2e_last_auth(): array { + $work = $GLOBALS['wmi_e2e']['work']; + $read = fn (string $name): string => is_file($work . '/' . $name) ? (string) file_get_contents($work . '/' . $name) : ''; + $path = $read('auth_path'); + + return ['content' => $read('auth_content'), 'mode' => trim($read('auth_mode')), 'path' => $path, 'exists' => $path !== '' && file_exists($path)]; +} + function wmi_e2e_add_account(int $id, string $name, string $username, string $password): void { $wmi = new Linux_WMI(); diff --git a/tests/E2E/pages_flow.php b/tests/E2E/pages_flow.php index fc8bd39..3b67035 100644 --- a/tests/E2E/pages_flow.php +++ b/tests/E2E/pages_flow.php @@ -259,6 +259,7 @@ 'username_in_page' => strpos($page['body'], 'DOM') !== false, 'password_field_empty' => preg_match("/id='password' value=''/", $page['body']) === 1, 'argv' => wmi_e2e_last_argv(), + 'auth' => wmi_e2e_last_auth(), ]; $bad = []; @@ -288,6 +289,27 @@ $out['tool_namespaces'] = $namespaces; +// Hosts the tool refuses run nothing; valid forms reach wmic after the // prefix. +$refused = []; +$reached = []; + +foreach (['bad host', '-oProxy=x', 'a..b', 'fe80::1%eth0', '999.1.1.1', 'host;id'] as $bad) { + $calls = count(wmi_e2e_calls()); + $r = $tool(['host' => $bad] + $tool_fields); + + $refused[$bad] = ['message' => strpos($r['body'], 'The host must be an IPv4 or IPv6 address or a host name') !== false, 'calls' => count(wmi_e2e_calls()) - $calls]; +} + +foreach (['win-server', '[2001:db8::1]', '10.0.0.5', 'srv.corp.local', '2001:0db8:0:0:0:0:0:1'] as $good) { + $tool(['host' => $good] + $tool_fields); + + $target = array_values(array_filter(wmi_e2e_last_argv(), fn ($a) => strpos($a, '//') === 0)); + + $reached[$good] = $target[0] ?? ''; +} + +$out['tool_hosts'] = ['refused' => $refused, 'reached' => $reached]; + $r = $tool($tool_fields, false); $out['tool_no_csrf'] = ['code' => $r['code'], 'ran' => strpos($r['body'], 'Spooler') !== false];