mirror of
https://github.com/Cacti/cacti.git
synced 2026-10-29 08:06:58 +09:00
fix/graph-wrapper-date-range-develop
16
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 TheWitness
|
7b551eea72 |
chore: normalize CRLF line endings to LF across Cacti-owned files (#8156)
Excludes vendored packages, third-party JS, patches, and the Win32 probe fixture. Pure EOL change; guards pass and php -l is clean. |
||
|
TheWitnessandci
|
9facd9b500 |
security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 confirmations) (#8149)
* security: batch 7 develop - consolidate remaining develop-only GHSA advisories (12 fixes + 8 regression confirmations) Fixes: f7jw (automation preview XSS), rp5g (db_column_exists LIKE), vx2m (automation pivot CASE WHEN), h5wg (plugin DDL identifiers), 929h (aggregate LIKE filter), mfw8 (add_graphs REGEXP), m49v (user_admin sort_column allowlist), mpfm (rrd error-image theme allowlist), m3fh (SNMPv3 control chars), cg45 (remember-me row revoke), xq26 (remember-me retention window), m67r (import xml_path containment). Confirmed already fixed on develop, pinned with regression tests: 4gph, 977w, mgcq, 5p79, cfhh, wfcf, pjq5, wp33. Excluded: 227p (1.2.x-only), mx5c (maintainer private branch). * security: address batch-7 review feedback (lexical-first xml_path containment, session-wide sort allowlist, connection-aware db_column_exists quoting, reject-not-normalize plugin DDL identifiers, behavior-based test names) * style: apply php-cs-fixer single-quote fix to api_automation pivot builders; add WSL dev-environment note to copilot instructions * fix phpstan string typing * test: refresh SQL interpolation baseline for db_column_exists db_qstr change (database.php 9->8) --------- Co-authored-by: ci <ci@local> |
||
|
TheWitness
|
5e870a91df |
security: batch 6 develop - path_php_binary escape (GHSA-4p7f), boost-log .log guard (GHSA-m6wx), tree lock/unlock/get_node authz (GHSA-73qf), rrd create strip (GHSA-967c) (#8148)
* security: batch 6 develop - escape path_php_binary (GHSA-4p7f), boost-log .log guard (GHSA-m6wx), tree lock/unlock/get_node authz (GHSA-73qf), confirm rrd create control-char strip (GHSA-967c) * security: cast path_php_binary before escaping in resource_cache_out (GHSA-4p7f) |
||
|
TheWitness
|
9d628b4959 |
security: batch 5 develop - fail closed on No-Auth admin bypass (GHSA-69vw), tag GHSA-3jj2, add 1.2.32 CHANGELOG section (#8146)
* security(GHSA-69vw): fail closed on auth_method None instead of auto-granting an admin session - check_reset_no_authentication() no longer wipes the admin password, flips auth_method, or hands an unauthenticated request an admin session; it fails closed and requires a manual auth_method reset (GHSA-69vw-qxxv-p8w2). - Tag GHSA-3jj2-v5ch-wmq5 in the CHANGELOG: the domains_login_process() realm bypass is already closed by the Login Providers rewrite (realm < 1000 fails closed). - CHANGELOG: add a 1.2.32 section below 1.3.0-dev with the GHSAs also fixed in 1.2.x, and de-duplicate the security entries the #8145 merge doubled. * docs(CHANGELOG): file host_id SQLi fix under 1.2.31 as GHSA-r7hc-hrpv-52gf; drop plain issue#7693 entry |
||
|
TheWitness
|
41e6d9e033 |
security: consolidate verified GHSA fixes for develop (batch 4) (#8145)
* security: consolidate verified GHSA fixes for develop (batch 4) Develop counterparts of the 1.2.x batch-4 fixes in #8144, re-verified against current develop HEAD. - GHSA-4rmr-wvjq-qxc2: include/auth.php Basic Auth and guest branches fall through to the per-page realm check instead of returning early. - GHSA-j67j-wpm4-9g3x: the pre-dispatch update_policy branch in user_admin.php and user_group_admin.php requires a CSRF-token POST. The denylist -> allowlist redesign is tracked for 1.3 in #8143. - GHSA-2p3x-f5jm-rg6p: the View-Graphs -> poller RCE chain is broken by the realm check (4rmr), the update_policy guard (j67j) and shell-free webhits (x5pw). - GHSA-x5pw-cg7j-mj4r: scripts/webhits.pl runs wc through a shell-free list-form open. - GHSA-9737-rqh3-h7fh: names interpolated into raise_message() now use __esc() across the UI; URL- and HTML-bearing messages are excluded. - GHSA-9x9h-2577-9w86: the colordropdown input is built as a node and the label is no longer round-tripped through parseHTML (the list item already used createTextNode on develop). - GHSA-h96j-8xhc-2p3w: remote_agent.php authorizes on $_SERVER['REMOTE_ADDR'], not get_client_addr(), which honours client-settable proxy headers. Regression guards under tests/Unit/Security/{Auth,Csrf,Xss,ShellInjection}. php -l, check-changelog.py and the sink/architectural baselines all pass. * chore: reference PR #8145 on the batch-4 1.3.0-dev CHANGELOG entries * security: resolve develop merge - keep id-only xmlerror unescaped (matches #8141 h745 test) * security(GHSA-j67j): use REQUEST_METHOD POST guard instead of raw \ (hardening-pattern-guard) * security(GHSA-9737): fix review findings - drop double-encoding in __esc, escape missed name sinks, correct plugins format-arg count |
||
|
TheWitness
|
0055dd00cc |
security: consolidate verified GHSA fixes for develop (batch 3) + CHANGELOG CVE reconciliation (#8141)
* security: consolidate verified GHSA fixes for develop (batch 3) Develop counterparts of the 1.2.x fixes in #8138, each re-verified against develop HEAD. - GHSA-77pr-g949-4v5f: spikekill.php also requires is_graph_allowed() on the target graph, not just the Spike Kill realm. - GHSA-w3qw-762w-6g9v: the include/global.php GET action guard fails closed on an array-form action[] before the scalar denylist comparison. - GHSA-xhpr-w454-cc9w: update_poller_cache() casts the stored output_type to int before interpolation. - GHSA-j3px-vw6r-g25x: graphs_new.php escapes stored snmp_index via db_qstr() in the data-query IN() pivot. - GHSA-h745-3r76-g253: graphs_new.php escapes the data-query and field names in the XML parse error messages (__ -> __esc). GHSA-297h-95ff-r767 does not apply on develop: the clog log-file selector renders through the drop_array filter framework, which escapes. tests/Unit/Security/ConsolidatedBatch3Test.php adds source-scanning regression guards. CHANGELOG (1.3.0-dev) updated; php -l, the security baselines, and check-changelog.py all pass. * chore: lead 1.3.0-dev security CHANGELOG entries with the GHSA id Normalize the 1.3.0-dev -security entries to the approved "-security: GHSA-xxxx [CVE-xxxx] Description" form, moving the advisory id to the front. CVE tokens are omitted because none of these advisories have a CVE issued yet (verified via the repository security-advisories API). Covers the five batch-3 entries plus the pre-existing entries that still used the trailing (GHSA-xxxx) form. * chore: use the -security#GHSA-xxxx: form for the 1.3.0-dev CHANGELOG entries Correct the batch-3 and reconciled 1.3.0-dev -security entries to the approved "-security#GHSA-xxxx: CVE-yyyy Description" form (GHSA id in the prefix). No CVE tokens are added because none of these advisories have a CVE issued yet (verified via the repository security-advisories API). * test: split batch-3 security guards into vulnerability-class files; reconcile CHANGELOG - Move the ConsolidatedBatch3Test.php guards into the matching Auth, Csrf, SqlInjection and Xss folders with behavior-based filenames, and strengthen them per review: the GET-guard test asserts the array branch actually returns 405 and exits; the snmp_index test rejects both raw concatenation branches; the parse-error test asserts every name-bearing message uses __esc(). - Append this pull request number to the five new 1.3.0-dev security entries. - Add issued CVE identifiers to the older -security#GHSA CHANGELOG entries (verified via the repository security-advisories API). Entries with no CVE yet, entries that already carried a CVE, and non-GHSA lines are unchanged. |
||
|
TheWitness
|
bb92c46e78 |
feat(auth): automatic User Group assignment from Login Provider groups (#8129)
* feat(auth): automatic User Group assignment from Login Provider groups Add per-Login-Provider "Auto Assignment" rules to the User Group editor so a user is added to (or removed from) a Cacti User Group at every login based on the groups their Login Provider reports. User Group editor (user_group_admin.php): - Add General / Auto Assignment / Rights spacers to the General tab and rename "General Group Options" to "Group Options". - Render one text field per eligible Login Provider under Auto Assignment (label = provider name). The admin enters a plain Group Name (e.g. "Cacti Admins"), never a DN. - Store the rules as JSON in the new user_auth_group.auto_assignments column, keyed by login_providers.id. Group resolution: - SAML2 and OpenID match the configured name against the group attribute / group claim values. - LDAP/AD resolve membership live against the directory using a raw group name plus the Search Base, trying AD (memberOf + nested LDAP_MATCHING_RULE_IN_CHAIN), FreeIPA / 389 DS (groupOfNames/groupOfUniqueNames) and RFC2307 posixGroup shapes so one plain name works everywhere. This requires a directory-read bind, so the field is only offered for Anonymous Searching or Specific Searching with a stored service account. - A directory that cannot be reached returns "undetermined" so existing memberships are left untouched rather than revoked on a transient outage. Lifecycle: - auto_assignments is reconciled on every login (lib/auth.php credential flow and login_sso.php redirect flow). - Deleting a Login Provider strips its key from every group's auto_assignments. Schema: add user_auth_group.auto_assignments (cacti.sql + 1_3_0 upgrade). * ci: document free-text group-name validateInput for constraint ratchet * fix(auth): address PR review on automatic User Group assignment - LDAP: add an Active Directory bare-name query (objectClass=group with the member:1.2.840.113556.1.4.1941 nested-membership rule) so a plain group name resolves on AD, which neither the posixGroup nor groupOfNames probe could match. - LDAP: make single-group membership tri-state and propagate an "undetermined" result, so a search that fails after binding (timeout/dropped connection) no longer looks like confirmed non-membership and revokes assignments. - SAML2/OpenID: supportsAutoAssignment() now requires a configured group claim, and applyAutoAssignments() skips any provider that is not currently capable, so a blank claim can never strip users from their groups. - Only reset the permission cache when a membership row actually changed (accumulate db_affected_rows) rather than on every login that has a rule. - Add unit tests for the reconciliation boundaries (add / remove / no-op / incapable / undetermined) and for the LDAP group-query construction (AD plain + nested, posixGroup, groupOfNames, DN mode). - Regenerate locales/po/cacti.pot for the new UI strings. * fix(phpstan): resolve level-8 errors from auto group assignment - Guard supportsAutoAssignment() behind an instanceof AbstractLoginProvider check so it is not called on the base LoginProviderInterface. - Assign the injected field list to a variable (inject_form_variables takes it by reference) and pass an is_array-guarded group row. - Bump the lib/ldap.php ldap_get_entries baseline count 3->6 for the three new group-membership query helpers. |
||
|
TheWitness
|
f030e5cc60 |
feat(login_providers): form field hints, cert/private-key display, and encrypted storage (#8074)
* feat(login_providers): add form hints, cert/private-key display, and Weblate strings
- Adds example.com-style placeholders across the LDAP/SAML2/OpenID Login Provider fields (server, DN/group DN/search filter, entity IDs, SSO/SLO/discovery URLs, group claims), so the expected syntax is visible without opening the docs.
- Adds a 'textcert' form method (form_cert_box()) for pasted PEM certificates: styled like filepath/dirpath, it parses the certificate and shows its expiration date, using the deviceUp/deviceDown status classes ('Good Till: YYYY-MM-DD' / 'Expired: YYYY-MM-DD'), or flags invalid input. Applied to SAML2's SP/IDP certificate fields.
- Adds a 'privkey' form method (form_privkey_box()) for the SAML2 SP Private Key: never redisplays the stored value (leaving it blank on save keeps the existing key); the status area shows '[stored]' or '[not set]' instead.
- Adds cacti_encrypt_secret()/cacti_decrypt_secret() (lib/auth.php), an AES-256-CBC envelope with a per-installation key persisted in \settings\ on first use, modeled on plugin_servcheck's servcheck_encrypt_credential()/servcheck_decrypt_credential(). The SP Private Key is now encrypted at rest; AbstractLoginProvider::existingParameter()/encryptOrKeepExisting() preserve the existing encrypted value when the privkey field is left blank.
- Regenerates locales/po/cacti.pot so Weblate picks up the new strings.
- Adds tests/Unit/Security/Auth/LoginProviderSecretFieldsTest.php (10 tests) covering the encrypt/decrypt round-trip, IV uniqueness, failure modes, and both new form widgets' rendering.
Verified: PHPStan clean, full Pest Security suite passes (835 tests including the 10 new ones), php-cs-fixer clean.
* fix(login_providers): never redisplay stored cert/key, preserve on blank save
form_cert_box() previously echoed the stored PEM certificate back into the textarea, and form_save() overwrote sp_x509cert/idp_x509cert with an empty string whenever the field was submitted blank - unlike sp_private_key, which already had blank-preserves-existing handling.
- form_cert_box() now behaves like form_privkey_box(): the textarea is always rendered blank (only repopulating from a failed same-submission's SESS_FIELD_VALUES, never the stored certificate); the expiration/status indicator is still computed from the stored value.
- Added AbstractLoginProvider::keepExistingIfBlank(), the non-secret counterpart to encryptOrKeepExisting(): a blank submission keeps the existing stored value instead of clearing it. Wired into SamlLoginProvider::collectParameters() for both sp_x509cert and idp_x509cert.
- Extended tests/Unit/Security/Auth/LoginProviderSecretFieldsTest.php: form_cert_box() never leaks the stored certificate into the rendered HTML, and keepExistingIfBlank()/encryptOrKeepExisting() resolve correctly for both a fresh submission and a blank field with no existing row (no database access needed for either case).
Verified: PHPStan clean, full Pest Security suite passes (838 tests), php-cs-fixer clean.
* fix(login_providers): address Copilot review round 1 + CI formatting
- CI: fixed a php-cs-fixer docblock indentation mismatch in lib/auth.php.
- Upgraded to actual authenticated encryption: the secret envelope is now AES-256-GCM (IV + 16-byte tag + ciphertext) instead of plain AES-256-CBC, which had no MAC/tag - a tampered database value now fails to decrypt instead of silently producing modified plaintext.
- cacti_secret_key() now fails closed on a corrupt persisted key (not valid base64, or not exactly 32 bytes) by throwing, instead of silently substituting an unpersisted random key that would desync from every existing/future ciphertext. cacti_decrypt_secret() catches this and degrades to false (logged); cacti_encrypt_secret() lets it propagate so a broken key never results in a secret being silently stored insecurely or lost.
- Added a backward-compatible read path for the SP Private Key: a provider whose parameters still hold the raw PEM from before this feature existed (cacti_decrypt_secret() returns false because it isn't a valid envelope) now falls back to using that value directly instead of losing the key. AbstractLoginProvider::encryptOrKeepExisting() opportunistically re-encrypts such a legacy value the next time the form is saved (even with the privkey field left blank), rather than requiring the admin to re-paste it.
- form_cert_box()/form_privkey_box() no longer repopulate from SESS_FIELD_VALUES: form_input_validate() writes that session value on every submission (including a successful save), and the save's redirect runs before bottom_footer() clears it - the immediate next page load was echoing the just-submitted plaintext private key/certificate back into the textarea. Both fields are now unconditionally blank.
Verified: PHPStan clean, full Pest Security suite passes (841 tests, 6 new), php-cs-fixer clean (fresh cache, whole-repo dry-run).
|
||
|
TheWitness
|
a52368125e |
feat(auth): rename User Domains to Login Providers, add SAML2 and OpenID Connect (#8066)
* feat(auth): rename User Domains to Login Providers Collapses user_domains/user_domains_ldap into a single login_providers table with a JSON parameters column, migrated in place during the 1.3.0 upgrade. Renames the admin page, menu, breadcrumbs, and help links to match. * feat(auth): rework login providers as Cacti\Auth classes Introduces LoginProviderInterface with credential (LDAP/AD) and redirect (SAML2/OpenID) flavors, replacing the procedural domains_login_process()/domains_ldap_*() functions. Adds blank-field group membership gating shared by every provider type. * feat(auth): add SAML2 and OpenID Connect login providers Adds onelogin/php-saml and firebase/php-jwt, a new cacti_http() SSRF-hardened outbound request helper for every OpenID discovery/token/JWKS call, and login_sso.php to handle the redirect-based SSO flow. Login page now shows a button per enabled SAML2/OpenID provider, and per-provider auth cookie opt-out is enforced on both the password and SSO login paths. * chore(i18n): regenerate cacti.pot * feat(auth): add Local and Basic auth as Cacti\Auth providers LocalAuthLoginProvider (CredentialLoginProviderInterface) and BasicAuthLoginProvider (new PreAuthenticatedLoginProviderInterface, since Basic Auth has no password for Cacti to check) are thin adapters over the existing local_auth_login_process()/basic_auth_login_process() functions, which are left untouched to avoid disturbing their regression-tested lockout/timing/rehash behavior. * feat(auth): move local password verification into LocalAuthLoginProvider secpass_login_process() and local_auth_login_process() are retired; their lockout, constant-time verification, forced-password-change, and rehash-on-login logic now live end to end in the class (verifyCredential/enforcePasswordPolicy/rehashIfNeeded). secpass_check_pass() stays a shared lib/auth.php function since password change/reset/user_admin also call it outside any login flow. Local auth still has no login_providers row: isEnabled() is always true and it cannot be disabled or deleted. * feat(auth): move Login Provider CRUD into Cacti\Auth classes AbstractLoginProvider gains static deleteById()/enableById()/disableById()/makeDefaultById()/persist(); each concrete provider owns its own collectParameters(), dispatched via LoginProviderFactory. login_providers.php's form_save()/form_actions() call these instead of doing raw SQL, and the retired provider_remove()/provider_disable()/provider_enable()/provider_default()/provider_collect_parameters() procedural functions are gone. Local/Basic auth are unaffected: they never extend AbstractLoginProvider, so they structurally cannot expose delete/disable. * fix(auth): address Copilot review findings on SSO/SAML/OIDC security Fixes callback/ACS/SLS/metadata URLs that used the raw provider id instead of the login realm (broke every SSO login), adds InResponseTo correlation for SAML and sub/azp validation for OIDC, requires HTTPS for OIDC discovery/endpoints, pins cacti_http() connections against DNS rebinding via CURLOPT_RESOLVE, exempts login_sso.php from the authenticated-page redirect guard and gates it on auth_method, wraps provider dispatch in a exception boundary, adds a dedicated SAML SLO path instead of routing it through login completion, makes remember-me an explicit opt-in for SSO, fixes disabled/deleted providers not revoking existing auth cookies, restricts the default-realm action to LDAP/AD, and fixes the 1.3.0 migration silently enabling group enforcement for installs that had it off. Also updates the E2E seed script for the login_providers schema. * chore(auth): refresh CI security baselines and document no-op validation Regenerate the sink inventory baseline for the new/renamed lib/Auth/* and login_providers.php files, and add '// no-validation:' comments on the intentionally-unconstrained admin/free-text/secret fields (LDAP DNs/filters, SAML certs/keys, OIDC client credentials, provider display fields) flagged by the validator constraint ratchet. Refreshes the validator baseline to drop the deleted user_domains.php entries. * refactor(ldap): remove dead DB-lookup path from Ldap::__construct The domain_id>0 branch duplicated the JSON-parameter-to-property mapping already owned by LdapLoginProvider::buildLdap(), and nothing constructs Ldap with a real id anymore (all call sites pass 0). Drop the parameter and the branch so Ldap stays a pure connection/protocol class with no DB coupling. Also fixes a latent bug: group_require is a typed bool property, but LdapLoginProvider::authenticate() assigned it an int (1/0) from a strict_types=1 file, which throws a TypeError on every LDAP/AD login attempt. Assign a real bool instead. * fix(auth): fix LDAP login, upgrade ordering, and hardening guard failures LdapLoginProvider compared LdapError::GetErrorDetails()'s int error_num against the string '0' with strict equality, so it never matched and every LDAP/AD login was rejected regardless of valid credentials. Compare against int 0 instead. install/upgrades/1_3_0.php called login_providers_convert_1_3_0() (which reads and then drops user_domains/user_domains_ldap) before the pre-existing legacy ALTER TABLE statements that add the debug/network_timeout/bind_timeout columns those reads depend on, so upgrades failed once the tables were already gone. Moved the call to run after those ALTERs. OpenIdLoginProvider::complete() read state/code from raw \ instead of gnrv(), tripping the hardening pattern guard; switched to the request helper. * style(auth): apply php-cs-fixer formatting and normalize line endings to LF Runs php-cs-fixer across the new lib/Auth/* classes, login_providers.php, login_sso.php, and the other touched files, fixing indentation/alignment/docblock style to match project convention. Also normalizes several files that had picked up CRLF line endings back to the LF the project's .php-cs-fixer.php config (setLineEnding) and the rest of the tracked tree actually use. * fix(auth): address second Copilot review round (9 high/medium findings) - Make login_providers_convert_1_3_0() resumable: CREATE TABLE IF NOT EXISTS and INSERT ... ON DUPLICATE KEY UPDATE so an interrupted upgrade can be re-run instead of leaving a half-migrated, unrecoverable table. - SamlLoginProvider::complete() now fails closed when no saved AuthnRequest ID exists instead of passing null to processResponse(), which skipped InResponseTo validation and accepted unsolicited IdP-initiated assertions. - LoginResult gains an isCredentialFailure flag; only a genuine wrong-password LDAP bind (LdapError::Failure) counts toward the lockout counter now, not connection errors or group-membership denials. - Fixed button_label/debug being read from the JSON parameters column in OpenIdLoginProvider/SamlLoginProvider/LdapLoginProvider when they are actually top-level login_providers columns; AbstractLoginProvider now hydrates and exposes both. - Removed SamlLoginProvider's wantMessagesSigned, which silently required the outer SAML message (not just the assertion) to be signed, rejecting IdPs that only sign the assertion as the UI promises. - Added getTemplateUserId() to CredentialLoginProviderInterface and implemented it on LocalAuthLoginProvider so the contract (not just incidental AbstractLoginProvider inheritance) guarantees the method lib/auth.php calls. - cacti_http()'s CURLOPT_RESOLVE entries now bracket IPv6 literals correctly, and safe-IP validation strips parse_url()'s brackets before checking, fixing IPv6 target pinning. - login_providers.php's type dropdown now swaps the Full Name attribute default (cn/displayName) on an actual user-driven type change, not on page load, so a new Active Directory provider does not silently save the LDAP default. - Extracted auth_apply_group_login_opts() and call it from both auth_login.php and login_sso.php, so SSO logins honor a user's group-derived login_opts the same way password logins already do. * fix(auth): fix PHPStan level 8 errors and pest harness stub drift Test harness fix: the LoginProvidersLoginRealmValidationTest stub LoginResult class was missing the isCredentialFailure property added to the real class, causing 'Undefined property' warnings that broke the harness's JSON output. Added the property and set it on the simulated bad-password failure. PHPStan (48+ errors, all newly introduced by this branch's rewrite, never actually verified until now): narrowed db_fetch_row_prepared()/db_fetch_assoc()'s array|bool (including bare true, which survives a '?: []' short-circuit - fixed with explicit is_array() checks instead) at every call site across auth_login.php, login_sso.php, lib/auth.php, lib/Auth/LocalAuthLoginProvider.php, lib/Auth/LoginProviderFactory.php, and install/upgrades/1_3_0.php. Fixed OpenIdLoginProvider::discover()'s return shape to match its declared docblock instead of returning the raw decoded JSON. Fixed lib/functions.php's cacti_http() to narrow curl_exec()'s string|bool result with is_string() (not ===false, which still leaves 'true' as a residual type) and guard / as non-empty for curl_setopt_array()'s stubs. Fixed a real bug in SamlLoginProvider::processLogout(): processSLO(true) was passing 'true' as \ (the first parameter) instead of \ (the fifth), so it always redirected/exited internally with the default \=false, making Cacti's own logout.php never regain control. Now calls processSLO(keepLocalSession: true, stay: true) and is typed ': never' to match; login_sso.php's now-genuinely-unreachable break statements after processLogout()/initiate() are removed. Fixed login_providers.php passing a expression (not a variable) to inject_form_variables()'s by-reference parameter. Verified: PHPStan analyse clean (0 errors), full Pest Unit/Security suite passes (128 tests), hardening guard/sink inventory/validator ratchet all still clean. * fix(auth): fix ambiguous column and array+bool crash in user_log filter The user filter dropdown query selected a bare 'id' column after joining user_auth and login_providers, which is ambiguous now that login_providers also has an id column (unlike the old user_domains.domain_id) - MySQL rejected the query outright. Qualified as ua.id. Also narrow db_fetch_assoc()'s array|bool result before the \ + \ + \ merge, which was throwing a TypeError whenever the query failed instead of surfacing the real SQL error. * fix(auth): address third Copilot review round (5 findings + tests) - AbstractLoginProvider::persist() now disables the template account for every provider type; SAML2/OpenID also copy it via user_copy() in login_sso_complete() and were left directly usable as a local login. - cacti_http_resolve_safe_ips() now detects IPv4-mapped IPv6 literals (::ffff:0:0/96) via inet_pton() and validates the embedded IPv4 address, closing an SSRF bypass via e.g. ::ffff:169.254.169.254. - login_providers_convert_1_3_0() now tracks db_install_execute()'s status for every row and only drops the legacy user_domains/user_domains_ldap tables once every row (and the final row count) is confirmed migrated. - Ldap::Getcn() now looks up ldap_get_entries()'s always-lowercased attribute keys case-insensitively while still storing the result under the originally-requested key, fixing AD's displayName (and any other mixed-case configured attribute) always resolving to a blank full name. - SamlLoginProvider::processLogout() now redirects to the LogoutResponse URL processSLO() returns for a LogoutRequest (stay:true), instead of discarding it and always redirecting to Cacti's own logout page - federated logout was never actually acknowledged to the IdP. - Added tests/Unit/Security/Auth/OpenIdLoginProviderTest.php (13 tests) covering verifyIdToken() signature/issuer/audience/azp/nonce validation, discover()'s HTTPS enforcement, and complete()'s callback-state and UserInfo-subject-matching checks, against a locally-generated RSA keypair/JWKS. - Regenerated the sink inventory baseline for the new header_redirect call in SamlLoginProvider::processLogout(). Verified: PHPStan clean, full Pest Security/Auth suite passes (830 tests including the 13 new ones), php-cs-fixer/hardening-guard/sink-inventory/validator-ratchet all clean. |
||
|
|
1e5329dd89 |
security: consolidate verified GHSA-rjvj win32 cmd.exe hardening for develop (#8056)
* fix(win32): harden the cmd.exe shell sinks on develop (GHSA-rjvj-r52f-8v5q) Forward-port of the 1.2.x win32 command-injection remediation. On Windows the SNMP/ping/data-query/trap commands run through cmd.exe, which ignores the \" escape and toggles quoting on every ", so cacti_escapeshellarg cannot stop & | ^ < > ( ). - add cacti_escapeshellarg_cmd(), stripping those operators on win32 before quoting (a Unix pass-through) - snmp_escape_string(): strip the operators in the win32 branch - route the SNMP binary hostname, the data-query device values (get_script_query_path), the data-input arg builders, the Windows ping hostname, and the SNMP trap notification arguments through the strip get_script_query_path is device-reachable (a rogue monitored device supplies an SNMP index/system field that executes on the Windows poller); the rest are post-auth. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(win32): strip %VAR% from the hostname on Windows (GHSA-rjvj-r52f-8v5q) cmd.exe expands %VAR% even inside quotes, so a device hostname of %USERNAME% would expand into the SNMP/ping command line. Add an opt-in $strip_env flag to cacti_escapeshellarg_cmd() and set it for the SNMP binary and ping hostnames, which never legitimately contain a percent. Community and credential values are left untouched because they can legitimately contain %. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * docs(changelog): note win32 cmd.exe hardening for develop * security: require Package Management permission to trust new package signers package_accept_key() (reachable via the repository-based package import flow) inserted new keys into package_public_keys, the global trust store consulted by import_validate_signature() for every future import by every user, without checking for the Package Management (realm 29) permission that gates package_keys.php/package.php/package_repos.php for the same table. An Import Templates (realm 17) user could therefore add themselves as a trusted signer for the whole install. Also add the 'accept' action to the CSRF-protected bad_actions list so the acceptance step requires POST plus a valid token like every other state-changing action. GHSA-gxgm-8rhv-r7jv * docs(changelog): note package-signer trust authorization fix * security: close tab-based open redirect bypass in CSRF timeout handler sanitize_uri()'s guard ( |
||
|
Thomas VincentandTheWitness
|
055c5ef5fa |
security: reject LDAP logins that skip a domain bind (#8009)
* hardening: validate the login realm and escape the LDAP username per context domains_login_process() now rejects a realm get_auth_realms() does not list and fails closed on one that is listed but has no domain behind it, and the Ldap class escapes the username for the DN and the search filter separately rather than relying on a filter-only blocklist. - an unlisted realm sent with a non-empty password used to match no branch at all and returned an empty user with no error, which auth_login.php reads as an authenticated user and provisions from the template account Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security: reject LDAP logins that skip a domain bind Posted realms outside user_domains no longer fall through to template or guest login. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test(ldap): align escaping contract with username lookup Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * test: cover LDAP domain login through OpenLDAP The e2e stack now binds a real directory user and checks that a forged realm cannot skip that bind. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(ldap): address Copilot review feedback Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix: substitute LDAP filter placeholders in one pass LDAP_ESCAPE_FILTER leaves angle brackets intact, so a looped str_replace can rescan one value as another key. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * Change realm retrieval function from gnrv to gfrv * tests: add gfrv() stub to harnesses that exercise gfrv()-calling code domains_login_process() and api_aggregate_convert_template() call gfrv() for realm/aggregate_template_id, but their test harnesses only stubbed gnrv(), so Pest failed with 'Call to undefined function gfrv()'. --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net> |
||
|
Thomas Vincent
|
5727cebebd | security: forward-port 1.2.x hardening (dsstats, guest, permissions, cookie) (#7844) | ||
|
Thomas Vincent
|
831d645fbf | hardening: fail closed on OAuth callback state (#7807) | ||
|
Thomas Vincent
|
db2ac2adf5 |
hardening: make password reset links account-wide single use (#7816)
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Thomas Vincent
|
49f5212181 |
fix(tests): restore paths and isolate test doubles (#7764)
* fix(tests): correct path depths after the domain reorganisation The category move left nine files resolving paths relative to their old depth, so the bootstrap could not read tests/Helpers/UnitStubs.php and no test ran at all. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix tests without replacing production functions * remove unrelated failing integration tests * remove unrelated broken test workflows --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> |
||
|
Sean Mancini
|
96033267cb |
refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders - Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class - Group all 186 test files into domain folders: DataCollection/, Ui/, Core/, Database/, Installer/, Scripts/, Security/, Plugin/ - Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions - Fix all __DIR__ relative paths to match new folder nesting depth - Update CI workflow path triggers and phpunit XML configs for new file locations - Update .github/copilot-instructions.md with unit test organization guidelines - Add tests/vendor/ to .gitignore * add gaurds * fix up guard for unit tests * fix redeclaration of cacti_log in tests * fix updates to unit tests * fix test patjhs * fix: update test __DIR__ paths for reorganized test structure * fix: conditionally define read_user_setting to prevent redeclaration in tests * fix: conditionally define functions to prevent redeclaration in tests - Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix with function_exists() checks to prevent conflicts when test stubs are loaded before lib/functions.php in the same PHP process * fix: add unit test bootstrap to prevent function redeclaration errors - Create tests/bootstrap-unit.php to load core libraries before tests - Ensures lib/functions.php loads before test stubs define stub functions - Test stubs use function_exists() checks, so they detect pre-loaded functions - Update phpunit.xml to use bootstrap-unit.php for Unit testsuite - Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process * fix: load test stubs before global.php in unit test bootstrap - Reorder bootstrap to load CactiStubs.php and UnitStubs.php first - Ensures stub functions like __() are available when global.php loads - global.php calls get_cacti_version_text() which needs __() for translations - Fixes 'Call to undefined function __()' error during bootstrap * fix: remove function conflicts from test stubs - Remove stub definitions for functions already in lib/functions.php: cacti_sizeof, cacti_log, cacti_strtolower, read_config_option, read_user_setting, cacti_count - Keep translation function stubs (__(), __esc()) needed by global.php - Update bootstrap to load lib/functions.php after stubs - This prevents function redeclaration errors while maintaining test isolation * fix: prevent constant and variable redeclaration in bootstrap - Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap (CactiStubs.php now handles this with guards) - Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING - Only set CACTI_TEST_BOOTSTRAP env vars if not already set - Fixes PHP warnings about 'Constant X already defined' * fix: remove redundant lib/functions.php load from bootstrap include/global.php already loads lib/functions.php at lines 287-288. The bootstrap was trying to load it again after global.php, but global.php needs format_cacti_version() to be available on lines 293-294 to define CACTI_VERSION and CACTI_VERSION_FULL constants. Solution: Let global.php handle loading lib/functions.php itself, and don't load it separately in the bootstrap. * fix: remove more conflicting function stubs from test helpers - Remove stubs for functions also defined in lib/functions.php: raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix - Remove POLLER_ID constant stub (defined unconditionally by global.php) - Keep only translation functions and test-specific helpers - This prevents function/constant redeclaration errors * fix: minimize test stubs to only translation functions Remove ALL helper functions and constants from UnitStubs.php that are: - Defined in lib/functions.php (raise_message, clean_up_lines, etc.) - Defined in lib/html_utility.php (srv, set_request_var) - Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*) - Defined in global.php (POLLER_ID) Keep ONLY translation functions (__(), __esc()) that are: - Called by global.php during initialization - Not defined elsewhere - Required for proper bootstrap sequence This eliminates all constant and function redeclaration errors. * fix: remove __esc stub - already defined in global_languages.php Keep only __() translation function stub, which is needed by global.php and not defined elsewhere in Cacti core. * fix: handle no-arg case in __() translation stub The __() function may be called with just a string and no format arguments. Previously, vsprintf() would fail with 'arguments array must contain 2 items'. Now, if no args are provided, return the text as-is. |