Skip to content

fix(nc35): verify App Store certificates with phpseclib 3 on Nextcloud 35 - #521

Merged
WilcoLouwerse merged 4 commits into
ConductionNL:developmentfrom
WilcoLouwerse:fix/nc35-phpseclib3-x509
Oct 12, 2026
Merged

WilcoLouwerse merged 4 commits into
ConductionNL:developmentfrom
WilcoLouwerse:fix/nc35-phpseclib3-x509

Conversation

@WilcoLouwerse

Copy link
Copy Markdown
Contributor

What

On Nextcloud 35 every App Store install through versioniq crashed before downloading anything:

Error: Class "phpseclib\File\X509" not found in lib/Service/SelectedReleaseInstallerService.php:144

Nextcloud 32–34 bundle phpseclib 2 (phpseclib\File\X509), Nextcloud 35 bundles phpseclib 3 (phpseclib3\File\X509); core's own Installer moved to phpseclib3 in 35. The certificate check now creates the X509 class the server ships. Both versions offer the calls the check makes (loadCA, loadX509, loadCRL, validateSignature, getRevoked), the same calls core makes. tests/stubs/server-internals.php gets a phpseclib3 stub next to the existing one.

#509 made versioniq declare NC 35; this is what it still needed to install anything there.

Verified

  • Clean nextcloud:35-apache (35.0.1), versioniq 1.6.8-unstable.20261008145948 from the release tarball:
    • without the fix: occ versioniq:install opencatalogi 2.1.18-beta.20261009035514 → the error above, for every app;
    • with the fix: versioniq:install + app:enable of openregister, integriq, opencatalogi, stackiq, portaliq and thematiq (their current betas) all succeed.
  • Clean nextcloud:34-apache (34.0.4) with the fix: versioniq:install openregister 2.1.37-beta.20261009010913 + app:enable succeed (phpseclib 2 path unchanged).
  • phpunit -c tests/phpunit-unit-only.xml: OK (755 tests).
  • cs:check: no findings in the changed files (it flags lib/Repair/RemoveRetiredCronJobs.php and its test, both untouched and already on development).
  • Psalm did not run locally (Psalm 5.26 crashes on this machine's PHP 8.4 before analysing); CI runs it.

Not in this PR

The App Store still offers versioniq only for NC 32–34: the newest stable there is 1.6.6 (max-version 34), and a beta is published as a nightly, which a normal NC 35 does not show. So installing versioniq from the App Store on NC 35 needs a stable release that includes #509 and this fix (development → beta → main; the standing promotion PR is #423).

Found while writing install instructions for Gemeente Rotterdam (Jira WOO-589), which install the Conduction betas through occ versioniq:install.

🤖 Generated with Claude Code

…d 35

Nextcloud 35 bundles phpseclib 3 (namespace phpseclib3) instead of
phpseclib 2, so every App Store install crashed with
'Class "phpseclib\File\X509" not found' before downloading anything.
The certificate check now uses the X509 class the server ships; both
offer the calls it makes, the same calls core's Installer makes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@remko48 remko48 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE (Standard)

The fix is minimal and matches core. Nextcloud's lib/private/Installer.php on master (NC 35) switched use phpseclibFileX509 → use phpseclib3FileX509 and otherwise makes exactly the same calls, in the same order (loadCA → loadX509 → loadCRL → validateSignature → getRevoked($loadedCertificate['tbsCertificate']['serialNumber']->toString()) → validateSignature). verifyCertificate() follows that sequence, so selecting the class is the only change needed. nextcloud/3rdparty confirms the split: stable32/33/34 require phpseclib/phpseclib ^2.0.55 and master requires ^3.0.55. Probing v3 first is the right order: it is the version that continues forward, and the use phpseclibFileX509 as X509V2 import does not trigger autoload, so it is harmless on 35 where v2 is gone.

🟢 Minor — @spec on newX509() points at the spec file, not the requirement it implements (inline comment)

What was verified

  • Read the full diff and the whole verifyCertificate() at head 6fa8174.
  • Compared it with core lib/private/Installer.php on stable34 (phpseclib 2) and master (phpseclib 3): same call sequence, only the namespace differs.
  • Checked the bundled phpseclib major in nextcloud/3rdparty composer.json for stable32/33/34/master.
  • info.xml declares min-version="32" max-version="35", so both branches of newX509() are reachable.
  • The new phpseclib3FileX509 stub matches the existing v2 stub's signatures, and Psalm preloads it via psalm.xml.
  • CI on the head commit: PHPUnit is green on stable32–35 × PHP 8.3/8.4. psalm, phpstan, phpcs, phpmd and Hydra Gates are green.
  • The three red checks don't come from this diff (reasoned from their logs, not reproduced). openapi dies on Api#updates @return (array-key-indexed JSON object), which the PR does not touch. Features Check reports docs/features.json is out of date. Quality Report fails because skipped test jobs left it with no verdict.

What was NOT run

  • No install on a real NC 34 or NC 35 instance; I relied on the author's verification in the description.
  • No local PHPUnit, Psalm or Hydra run.
  • I didn't check whether features.json / openapi also fail on development (its Quality Report was still queued).

Comment thread lib/Service/SelectedReleaseInstallerService.php Outdated
WilcoLouwerse and others added 3 commits October 12, 2026 12:05
…dpoint

The openapi extractor refused `array<array-key, mixed>` as a JSON object
key type on Api#updates; the snapshot is keyed by app id, so it is now
`array<string, mixed>` like Api#advisories. openapi.json gains the
/api/updates path and docs/features.json the admin-api and
pending-updates features, both of which development had not
regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ment

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
AvailabilityResultStore::read() returned array<array-key, mixed>, so
Psalm refused Api#updates' array<string, mixed> return type. Narrow it
once in the store, as AdvisoryResultStore::read() already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@WilcoLouwerse
WilcoLouwerse merged commit ab57692 into ConductionNL:development Oct 12, 2026
48 checks passed
@WilcoLouwerse
WilcoLouwerse deleted the fix/nc35-phpseclib3-x509 branch October 12, 2026 13:43
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.

3 participants