Skip to content

Harden RRDtool proxy request handling - #17

Draft
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:fix/harden-rrdproxy-security
Draft

Harden RRDtool proxy request handling#17
somethingwithproof wants to merge 1 commit into
Cacti:developfrom
somethingwithproof:fix/harden-rrdproxy-security

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

Summary

  • migrate the client protocol implementation to phpseclib 3 while preserving Cacti wire compatibility
  • bound encrypted and compressed request frames and fail closed on malformed input
  • confine RRDtool and filesystem operations to configured roots
  • remove shell interpolation from RRDtool and removespikes execution
  • make RSA key writes atomic, validate matching key pairs, and enforce private-key permissions
  • fix partial socket writes, RRDtool startup failures, admin disconnect cleanup, and IPv6 admin binding
  • add focused security and regression coverage and repair the active PHPStan configuration

Why

The proxy mixed phpseclib 1 calls with a phpseclib 3 dependency, accepted unbounded frames, passed client-controlled removespikes arguments through a shell, and allowed several filesystem operations without canonical root confinement. Socket and key-management paths also had reliability and permission issues.

Impact

Requests containing traversal, absolute paths, embedded command framing, malformed encryption frames, or oversized payloads now fail closed. Existing Cacti clients retain the current RSA/Rijndael-CBC wire format.

The compatibility protocol still lacks signed proof of private-key possession and authenticated message integrity. The README now documents that limitation and recommends a trusted, firewalled management network until a versioned client-and-server protocol upgrade is available.

Validation

  • composer test — 27 checks passed
  • composer run phpstan — no errors at the configured baseline
  • composer run lint — 11 files passed
  • composer run phpcsfixer — clean
  • composer validate --strict — clean
  • composer audit — no vulnerability advisories
  • git diff --check — clean
  • removespikes help and proxy version smoke tests passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant