* Harden RRDtool proxy request handling
* Address PR review feedback: encrypt oversized-request errors, fix env inheritance, spacing, and symlink test
Addresses review comments on PR #17:
- lib/client.php: encrypt the 'Request too large' error frame once the
client is authenticated, matching the existing encrypted-error pattern
- lib/client.php: build the removespikes child-process environment from
getenv() instead of $_ENV so the full current environment (e.g. TEMP on
Windows) is inherited, with RRDP_RRDTOOL_PATH merged on top
- lib/client.php: add the missing space in the RRDTOOL_PIPE_UNAVAILABLE
error frame so it matches the other 'ERROR: ...' responses
- tests/run.php: check the symlink() fixture return value and skip the
symlink-escape assertion (instead of producing a false positive) when
symlink() is unavailable
* Upgrade phpseclib 3.x to phpseclib/phpseclib ^4.0
- composer.json/composer.lock: require phpseclib/phpseclib ^4.0 (was ^3.0)
- Update phpseclib3\\... namespace usages to phpseclib4\\... across
rrdtool-proxy.php, lib/client.php, lib/functions.php, lib/wizard.php,
and tests/run.php
- lib/functions.php: phpseclib4 removed the Random class, so
Random::string(32) is replaced with random_bytes(32)
- Fix lib/__master__.php and lib/replicator.php, which PR #17 never
migrated off the old pre-Composer phpseclib API (new \\phpseclib\\Crypt\\RSA(),
->loadKey(), ->getPublicKeyFingerprint() -- none of which exist in
phpseclib 3.x or 4.x, so these MSR/replicator peer-authentication code
paths would fatal at runtime). Both now use
RSA::loadPublicKey()->getFingerprint('md5') wrapped in try/catch,
matching the pattern already used in lib/client.php, plus a
constant-time hash_equals() fingerprint comparison
- tests/run.php: relabel the phpseclib round-trip check from 'phpseclib 3'
to 'phpseclib 4'
* Bump minimum PHP version to 8.2 and add CHANGELOG.md
- composer.json: require php ^8.2 (was ^8.1)
- include/global.php: bump RRDP_PHP_VERSION_REQUIRED to 80200 to match
- README.md: update minimum/recommended PHP version to 8.2
- composer.lock: refreshed for the new php platform constraint
- Add CHANGELOG.md tracking changes prefixed security:/issue:/feature:
* Add PHPDoc blocks to all functions
Adds a PHPDoc block above every function in rrdtool-proxy.php,
lib/functions.php, lib/client.php, lib/__master__.php, lib/replicator.php,
lib/wizard.php, cli/removespikes.php, tests/run.php, and
.phpstan-bootstrap.php, each with a brief description and inferred
@param/@return types. Function signatures themselves are unchanged; PHPStan
will flag any inferred types that turn out to be inaccurate.
* Add .github/copilot-instructions.md
Generated from README.md and this session's learnings: project architecture
(daemon entrypoint, shared lib helpers, client/master/replicator roles,
wizard, removespikes CLI), build/lint/test commands (composer scripts,
php tests/run.php, Windows platform-req caveats), and conventions (tabs,
GPL header block, PHPDoc-only typing, phpseclib4 gotchas, CHANGELOG entry
prefixes).
Also fix .gitignore: the '.git*' pattern was unintentionally also matching
and ignoring the .github/ directory; added a '!.github/' exception so files
under .github/ (like this one) are tracked.
* Add .gitattributes enforcing LF line endings; normalize existing CRLF files
- Add .gitattributes with '* text=auto eol=lf' so Git normalizes line
endings to LF on checkout/commit regardless of local core.autocrlf
settings.
- .gitignore: add '!.gitattributes' alongside the existing '!.github/'
exception, since '.git*' was also unintentionally ignoring .gitattributes
itself.
- Normalize the two tracked files that had CRLF line endings
(.github/copilot-instructions.md, CHANGELOG.md) to LF; no other tracked
files were affected.
* Manual PSR coding-style adjustments
Manual formatting/coding-style cleanup in cli/removespikes.php,
lib/__master__.php, lib/client.php, and lib/replicator.php.
* Address automated review feedback: symlink confinement, key-pair validation, and misc fixes
Addresses 10 unresolved copilot-pull-request-reviewer comments on PR #20:
- lib/functions.php: rewrite rrdp_resolve_path_within() to walk the path one
segment at a time (instead of only checking the nearest existing ancestor
via file_exists()), so a symlink anywhere along a not-yet-fully-existing
path is resolved and confined even if the symlink itself is dangling.
- lib/functions.php: add rrdp_resolve_command_paths(), which resolves every
RRD file path operand referenced by an RRDtool command (leading bare file
arguments and DEF:/SDEF: clauses) against the RRA root and rewrites the
command to use the canonical resolved paths, closing the symlink-escape
gap left by the lexical-only rrdp_command_has_unsafe_path() check.
- lib/client.php: wire rrdp_resolve_command_paths() into the RRDtool command
dispatch path, rejecting/rewriting unsafe operands before they reach the
RRDtool pipe.
- lib/client.php: enforce the request-size limit on each completed
transaction and the trailing unfinished remainder, instead of on the
aggregate receive buffer before splitting on _EOT_, so multiple legitimate
transactions in one read no longer trip a false 'Request too large'.
- rrdtool-proxy.php: validate at startup that the loaded public/private key
files actually form a matching pair (comparing derived fingerprints)
instead of only deriving a fingerprint from the public key, failing
startup instead of accepting requests that will all fail to decrypt.
- lib/__master__.php, lib/replicator.php: resolve peer-supplied FULLSCAN
folder/file names (and RRDLAST/RRDDUMP request filenames) through
rrdp_resolve_rra_path() before using them in mkdir()/touch()/
file_put_contents()/RRDtool restore-dump-last-flushcached operations,
instead of concatenating raw peer-supplied text; reject and skip entries
that fail confinement.
- lib/replicator.php, lib/__master__.php: guard rrdtool_pipe_init() failures
in the main replication loop the same way lib/client.php already does,
instead of indexing into a false return value and never retrying.
- cli/removespikes.php: make createRRDFileFromXML() return whether the
RRDtool restore actually succeeded, and exit non-zero when it fails
instead of silently exiting 0.
- .phpstan.neon: restore level 6 (was lowered to 2, contradicting
.github/copilot-instructions.md) and add .phpstan-baseline.neon to
capture the 42 pre-existing findings surfaced at level 6 without hiding
future regressions.
- tests/run.php: add the standard Cacti Group GPL header block required for
every PHP file.
---------
Co-authored-by: Thomas Vincent <thomasvincent@gmail.com>
Some more QA fixes from discussions that were made and attempts to
reduce repeating code.
- Created functions to determine if RRDtool Proxy Server or
RRDcached are running
- Created several functions for code readability
- wizard_handle_title() to handle the top blue title bar
- wizard_prompt_wordwrap() to provide a colored prompt and
indent any text that is wrapped to show it is part of said
prompt
- wizard_handle_settings() to handle display and input of
settings in a common fashion. There are one or two values
that require custom functionality such as the enable password
and IP version which can be found inside there
- Corrected issues with ANSI codes causing wordwrap() to wrap sooner
than expected resulting in strange layouts rather than properly
justified
- Make enable password secure by using the built in PHP functions
password_hash() and password_verify()
Fixes the following items:
- Update README.md to provide more information
- Add password to enabled mode
- Add 'help' as another alias to 'list'
- Make Cacti RRDProxy logo have color!
- Move ANSi color codes to defines for code review
- Minor reformatting of various messages
