[stable32] feat: crl revocation checker#7122
Merged
vitormattos merged 29 commits intostable32from Mar 5, 2026
Merged
Conversation
Replaces ad-hoc 'valid'/'revoked'/... string literals with a typed PHP 8.1 backed enum. Makes invalid states impossible to represent and enables exhaustive pattern-matching across the CRL pipeline. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Thin interface that wraps the ldap_* extension functions (connect, configure, bind, read, listEntries, search, getEntries, unbind). Enables injection of test doubles without the php-ldap extension. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Delegates every ILdapConnection method to the corresponding ldap_* function. Throws RuntimeException at construction time when the ldap extension is absent, so callers fail fast before reaching network I/O. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Resolves ldap:// or ldaps:// CRL URLs, negotiates LDAPv3 anonymous bind, reads the certificateRevocationList attribute and returns the raw DER bytes. Results are cached for 24 h via ICacheFactory. A finally block guarantees ldap_unbind even when getEntries throws. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Checks whether a certificate serial number appears in any CRL listed in its CDP extension. Supports HTTP(S) and LDAP(S) download URLs; HTTP responses are cached 24 h. The local-CA pattern is memoized to avoid rebuilding the regex on every call. openssl crl invocation is extracted to a protected method for subclass-level test overriding. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Removes the manual 'new CrlRevocationChecker(...)' construction from the handler body and promotes it to a constructor-injected parameter. This satisfies the Dependency Inversion Principle and allows the container or tests to supply alternative implementations. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Adds CrlRevocationChecker as the last constructor parameter and forwards it to parent::__construct, closing the DI chain introduced in AEngineHandler. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Same as CfsslHandler: adds the parameter and delegates to the parent constructor so the DI container can wire CrlRevocationChecker automatically. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Replaces the old string comparison with a CrlValidationStatus enum switch. VALID and DISABLED pass through; REVOKED raises the existing 'Certificate has been revoked' error; every other status (urls_inaccessible, validation_failed, …) raises 'Certificate revocation status could not be verified' – fail-closed to prevent signing with an unverifiable certificate. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Provides two new initial-state values: - crl_external_validation_enabled: current app-config boolean - ldap_extension_available: runtime detection via function_exists Both are consumed by the CrlValidation Vue component. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
New Vue component that lets administrators enable or disable external CRL validation and shows a warning when the php-ldap extension is absent, since LDAP distribution points will be silently skipped in that case. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Imports the CrlValidation component and adds it to the settings template between the Validation and DocMDP sections. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Covers cache hit/miss, local vs remote CRL paths, REVOKED/VALID/NO_URLS outcomes, openssl exit codes, serial-matching, and revocation-date extraction. Uses an inner testable subclass to override execOpenSslCrl without spawning a real process. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
24 tests covering: cache hit returns early, all scope keywords (base/ one/onelevel/sub/subtree), connect failure, empty result set, binary attribute stripping, finally-block unbind on exception, and cache storage after a successful download. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The CrlRevocationChecker is now injected, so the reflection workaround that forced private fields is no longer necessary. Related assertions are covered by CrlRevocationCheckerTest. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Replaces the raw string literals 'valid' and 'revoked' in test fixtures with the typed CrlValidationStatus enum cases. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Migrates 'valid'/'revoked' string literals to CrlValidationStatus enum and adds test cases for the new DISABLED and urls_inaccessible/ validation_failed statuses introduced by the fail-closed policy. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The three UndefinedConstant suppressions for LDAP_OPT_* that were recorded against CrlRevocationChecker are now moot: those constants were moved to PhpLdapConnection, which suppresses them directly with @psalm-suppress UndefinedConstant. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Throwing in the constructor of PhpLdapConnection crashed the DI container on every request on servers that do not have the php-ldap extension installed, even when no LDAP CRL URL was being checked. Moving the guard to connect() keeps instantiation safe and lets the existing RuntimeException catch-block in fetchFromLdap log a warning and return null gracefully instead. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…dHelper The FilenameValidator check calls \OCP\Server::get() which has no DI container in unit-test context and throws an unexpected exception. Because that check came before the size check, the \OCP\Util::uploadLimit() branch was never reached, @Unlink was never called, and the temp file survived – breaking testValidateUploadedFileTooBig. Moving the size check earlier also makes semantic sense: rejecting oversized files is cheap and should happen before any filename-policy evaluation. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
OpenSslHandler now expects 13 constructor arguments after CrlRevocationChecker was injected via the DI chain (AEngineHandler ← CfsslHandler/OpenSslHandler). The test was still passing only 12, causing ArgumentCountError. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
IFilenameValidator has no isForbidden() method. The correct API is isFilenameValid(string $filename): bool which returns true when the name is acceptable – so the guard must negate it. Also updated FileUploadHelperTest: - testValidateUploadedFileSuccess: sets isFilenameValid→true explicitly - testValidateUploadedFileWithForbiddenName: mocks isFilenameValid→false (PHPUnit will not let a per-test stub override a setUp stub, so the default was removed from setUp and added only to the success test) Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…rTest The mock returned null by default, causing AEngineHandler to access $crlDetails['status'] on a null value and trigger a PHP warning. Stubbing validate() to return ['status' => CrlValidationStatus::VALID] fulfils the contract and silences the warning. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…rTest Same root cause as AEngineHandlerTest: the CrlRevocationChecker mock returned null, causing 7 tests to trigger "Undefined array key 'status'" warnings in AEngineHandler::addCrlValidationInfo(). Stubbing validate() to return ['status' => CrlValidationStatus::VALID] satisfies the return-type contract and eliminates the warnings. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
CI environments with the php-ldap extension installed do not trigger UndefinedConstant for LDAP_OPT_* constants, so the baseline entry became a stale UnusedBaselineEntry. Local environments without php-ldap still need the suppression, which is now handled via an inline @psalm-suppress on the configure() method instead of the shared baseline. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
The three UndefinedConstant entries for LDAP_OPT_* are now suppressed via @psalm-suppress directly in PhpLdapConnection::configure(), making these baseline entries unused and causing UnusedBaselineEntry errors in CI. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> [skip ci]
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos
force-pushed
the
backport/7081/stable32
branch
from
d1acb92 to
d00b4f8
Compare
github-project-automation
bot
moved this from 0. Needs triage
to 4. to release
in Roadmap
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #7081
Warning, This backport's changes differ from the original and might be incomplete⚠️
Todo
Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.