Files
cacti/tests/Unit/Security/Csrf/CsrfSecretNotTrackedTest.php
Thomas VincentandTheWitness cb49f31ff9 Replace csrf-magic with maintained CSRF protection (#7744)
* build: add symfony/security-csrf

Backs the CSRF token core that replaces the vendored csrf-magic fork.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: fix security-csrf package path assertion

symfony/security-csrf ships CsrfTokenManager.php at the package root, not under src/.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor: register the composer autoloader before the CSRF include

CSRF validation ran before the autoloader existed, which blocks any class-based guard.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): add CactiCsrfGuard over symfony/security-csrf

Issues and validates session-scoped tokens; not yet wired into the request path.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): accept pre-upgrade csrf-magic tokens during the grace window

Legacy tokens carry their own expiry, so the fallback closes itself without a config flag.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): port the output buffer rewriter to CactiCsrfGuard

Keeps the hidden field name and JS globals that third-party plugins rely on.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): use json_encode for the JS script context in rewriteBuffer

htmlspecialchars() protects the HTML attribute context but not the
<script> string literal, which relied on Symfony's token generator
never emitting a quote rather than being safe by construction.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(csrf): move the XHR shim to include/js as Cacti-owned code

It has been maintained by the Cacti Group since 2020 and is not an upstream package.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* refactor(csrf): retire the vendored csrf-magic fork

The procedural csrf_* surface stays as a deprecated shim so plugins keep working.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* feat(csrf): validate requests through CactiCsrfGuard

csrf-magic self-invoked from its own file; the guard is called explicitly.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(install): stop gating the permission step on the csrf secret

Retiring csrf-magic removed the directory its default path lives in, and setStep() clamps to the first errored step, so no fresh install could advance.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): key the legacy fallback on the raw secret bytes

refresh_csrf.php writes PHP source with a trailing newline, and trimming that yields a different HMAC key than csrf-magic used.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): log when the guard degrades to disabled in a web context

The installer is the only web path that can reach it, and failing open in silence is not something csrf-magic could do.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: changelog for the CSRF library replacement

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): mint the token before the session write callback closes it

csrf_startup() minted lazily from the output buffer callback, which runs after session_write_close()'s shutdown-function registration, so a token minted there on a legacy-fallback POST was silently dropped.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): make refresh_csrf.php a no-op

Its default secret path no longer ships and live tokens are session-bound, not HMACs over the secret, so rotation neither worked nor did anything meaningful; running it during the upgrade grace window used to delete the legacy secret and kill validation for pre-upgrade tokens outright.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): fall back to the old default secret path when reading it

csrf-magic tried its own directory as a second candidate regardless of $path_csrf_secret; the new reader only tried the configured path, so a site pointing $path_csrf_secret at a file that was never created lost the legacy-token grace window entirely.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs(csrf): point the JS header at its BSD license sidecar

The header reduces the original BSD terms to prose and the actual text sits in an unreferenced csrf.js.LICENSE, so add a pointer to it.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(csrf): restore csrf_check()'s bool return and $fatal contract

The shim ignored $fatal and always returned void, so audit/audit.php's `!csrf_check(false)` evaluated `!null` (always true) and denied every request instead of getting a real pass/fail result back.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: reference the pull request number in the changelog entry

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: resolve includes through Cacti path constants

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): enforce full coverage

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* Harden CSRF fallback handling

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): assert the component stays out of the index

Committing it would leave a second copy to drift against the lock.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: align CSRF checks with PHP 8.2

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* docs: normalize the CSRF changelog entry

* Fix CHANGELOG entry ordering

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(csrf): support constrained test environments

* fix(csrf): narrow nullable guard state

* test: place CSRF coverage in security domain

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* security(test): match disabled exec exactly

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* ci: run CSRF coverage on PHP 8.3

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
2026-09-18 00:16:39 -04:00

101 lines
4.4 KiB
PHP

<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
| |
| This program is distributed in the hope that it will be useful, |
| but WITHOUT ANY WARRANTY; without even the implied warranty of |
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
| GNU General Public License for more details. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* include/vendor/csrf/csrf-secret.php holds the legacy secret pre-upgrade CSRF
* tokens are derived from. csrf_get_secret() only generates one when it is missing
* or empty, so a secret committed to the repository is never replaced: every
* install built from that tree shares it, and anyone who reads the repository
* can mint tokens for all of them.
*
* .gitignore has always listed the path, but the file was committed anyway and
* an ignore rule does not apply to something already in the index. These pin
* both halves so it cannot drift back.
*/
/**
* Runs a git command in the repository, or skips the test where there is no
* checkout to ask (a release tarball, for instance).
*
* @param string $arguments The git arguments to run.
*
* @return array{output: string, status: int} The trimmed output and exit status.
*/
function csrf_secret_git(string $arguments) : array {
$root = dirname(__DIR__, 4);
/* In a worktree or a submodule .git is a file pointing at the real gitdir,
so testing for a directory would skip where tracking is inspectable. */
if (!file_exists($root . '/.git')) {
test()->markTestSkipped('not a git checkout, so tracking cannot be inspected');
}
$output = [];
$status = 0;
exec('git -C ' . escapeshellarg($root) . ' ' . $arguments . ' 2>/dev/null', $output, $status);
return ['output' => trim(implode("\n", $output)), 'status' => $status];
}
test('the csrf secret is not tracked in the repository', function () {
$tracked = csrf_secret_git('ls-files --error-unmatch include/vendor/csrf/csrf-secret.php');
// a zero status means git knows the file, which is the bug
expect($tracked['status'])->not->toBe(0)
->and($tracked['output'])->toBe('');
});
test('no file under the csrf vendor directory holds a bare secret', function () {
$tracked = csrf_secret_git('ls-files include/vendor/csrf/');
/* The vendored csrf-magic fork was removed, so nothing under this path is
tracked any more. That is a stronger guarantee than scanning what is
there, and it also stops the directory returning with a secret in it. */
expect($tracked['output'])->toBe('');
});
test('the secret path is still ignored so it cannot be committed again', function () {
$gitignore = file_get_contents(CACTI_PATH_BASE . '/.gitignore');
expect($gitignore)->not->toBeFalse('.gitignore must be readable');
expect($gitignore)->toContain('include/vendor/csrf/csrf-secret.php');
});
/**
* Removing the file is only safe because csrf_get_secret() writes a fresh
* secret the first time it finds none, so this pins the generation path rather
* than assuming it. The function moved from the vendored fork into
* include/csrf.php when csrf-magic was retired; the assertions are unchanged.
*/
test('the secret is still generated and stored when none exists', function () {
$source = file_get_contents(CACTI_PATH_INCLUDE . '/csrf.php');
expect($source)->not->toBeFalse('include/csrf.php must be readable');
$start = strpos($source, 'function csrf_get_secret(');
expect($start)->not->toBeFalse();
$end = strpos($source, "\nfunction ", $start + 1);
$body = substr($source, $start, ($end === false ? strlen($source) : $end) - $start);
expect($body)->toContain('csrf_generate_secret()')
->and($body)->toContain('csrf_writable(')
->and($body)->toContain('fwrite(');
});