Verify signatures when downloading packages #247

costdev · 2025-09-09T06:18:59Z

This PR introduces signature verification for packages. The PR still needs initial testing.

Approach

The 'upgrader_pre_download' hook is used to:

Download the package
Set the trusted keys
Perform signature verification

The callback includes guards and clean up surrounding the steps above.

Using this particular hook means minimal duplication of Core's code while returning back into Core's process as soon as possible.

New dependency

This PR introduces yocLib - Multibase as a dependency. This is used to convert signing keys from Base58BTC to Base64. While we could use another implementation, this is used elsewhere in the project and I erred on the side of getting something testable out quicker.

Pre-merge note

If we continue with this dependency, or find another external dependency, the bundler script will need to be checked to ensure it includes/excludes files as its supposed to. This should be done before this PR is merged.

Testing

Ensure you have enabled signature verification. You can drop this into a file in wp-content/mu-plugins:

<?php
/**
 * Plugin Name: FAIR: Verify Package Signatures
 * Description: Enables FAIR package signature verification
 * Author:      FAIR Contributors
 * License:     GPLv2 or later
 * Version:     1.0.0
 */
add_filter( 'fair.packages.updater.verify_signatures', '__return_true' );

kasparsd

Thanks for working on this!

Please see the comments inline for feedback. I believe we might want to adjust the return value of the upgrader_pre_download filter to ensure it stops WP core from downloading the package if the verification fails.

composer.json

inc/updater/class-updater.php

kasparsd · 2025-09-09T09:48:38Z

I'm thinking about the alternative workflows where this could apply. Will this apply to the did: installer too? Or if a user goes through the manual install workflow to upload an update to an existing package?

costdev · 2025-09-09T09:58:50Z

Will this apply to the did: installer too?

Yeah this should work with the did: direct installer right away.

Or if a user goes through the manual install workflow to upload an update to an existing package?

Yes I believe so. The FAIR plugin sets up the Updater for each installed package on init. The Updater::run() method calls Updater::load_hooks(), which is where this PR hooks to upgrader_pre_download for that package.

I'm not sure whether this is the case when uploading a new package though. I expect that is different.

kasparsd

Looks good now!

inc/updater/class-updater.php

kasparsd · 2025-09-09T10:12:00Z

I'm not sure whether this is the case when uploading a new package though. I expect that is different.

Yeah, there is no reliable way to determine the package vendor if we only have the ZIP. For a secure installation users should use the did: installer.

We could attempt to extract the vendor did from the uploaded package meta header and then compare the downloaded file hash/checksum + signature with any of the available releases. But that doesn't really help because a malicious actor could obviously remove the did info from the meta.

I love the did installer! Such a simple UI for a super-secure installer.

costdev · 2025-09-09T13:39:27Z

FYI: After chatting with @rmccue, I'm going to rework some of this to use a subclass of the WP_Upgrader class so we have some more control and it'll read better.

inc/updater/class-base58btc.php

Signed-off-by: costdev <[email protected]>

verify_file_signature() is documented to return bool|WP_Error, with false meaning verification was not attempted. However, the function's code doesn't actually return false. Signed-off-by: costdev <[email protected]>

Signed-off-by: costdev <[email protected]>

The plugin will include distribution dependencies, whereas workflows install development dependencies too, so this check will always fail. Signed-off-by: costdev <[email protected]>

Signed-off-by: costdev <[email protected]>

This was previously sending the whole key object, resulting in a Fatal Error. Signed-off-by: costdev <[email protected]>

Signed-off-by: costdev <[email protected]>

This reverts commit e430753. Signed-off-by: costdev <[email protected]>

This reverts commit ca4c5ea. Signed-off-by: costdev <[email protected]>

This reverts commit be67b25. Signed-off-by: costdev <[email protected]>

Signed-off-by: costdev <[email protected]>

…ater\Updater`. Signed-off-by: costdev <[email protected]>

Signed-off-by: costdev <[email protected]>

costdev · 2025-09-18T21:12:46Z

The force-push was a rebase on main due to a merge conflict.

Every FAIR package hooks to verify signature on download. Simply using `md5( $package )` to protect against an infinite loop is overkill, because if some other FAIR package checks for a matching URL first, the actual matching FAIR package will bail out. Example: - User clicks to install FAIR Plugin B. - FAIR Plugin A's filter runs first (alphabetical order). `$has_run[ md5( $package ) ]` is set. - FAIR Plugin B's filter runs. `$has_run[ md5( $package ) ]` has already been set, so it bails. - FAIR Plugin B's package doesn't get signature verification. To resolve this, each FAIR package's filter now uses `md5( $this->did . '_' . $package )`, which is specific to _that package_ for _that URL_. Signed-off-by: costdev <[email protected]>

rmccue · 2025-09-21T14:43:44Z

In fairpm/fair-beacon#52 we switched the signature algorithm to sha384, which I think was the last blocker here. @costdev is this now ready for review/merge?

costdev · 2025-09-21T15:02:14Z

@rmccue Yes, ready for review, testing and merge. At the moment, you can test by enabling signature verification and installing Git Updater via Direct Install - did:plc:afjf7gsjzsqmgc7dlhb553mv.

MU Plugin to enable signature verification:

<?php
/**
 * Plugin Name: FAIR: Verify Package Signatures
 * Description: Enables FAIR package signature verification
 * Author:      FAIR Contributors
 * License:     GPLv2 or later
 * Version:     1.0.0
 */
add_filter( 'fair.packages.updater.verify_signatures', '__return_true' );

Signed-off-by: costdev <[email protected]>

costdev · 2025-09-21T17:40:44Z

I've added an error_log() so testers can see when it's successfully verified the package. Just ensure WP_DEBUG and WP_DEBUG_LOG are set to true.

**DO NOT MERGE WITH THIS error_log() in the PR - I'll remove it once the PR has 1-2 approvals **

afragen · 2025-09-21T17:51:58Z

I now see this issue

afragen · 2025-09-21T17:55:09Z

If the package is not yet installed then Updater\get_packages() returns null

costdev · 2025-09-21T17:55:25Z

That's related to another PR. I'll get that fixed shortly.

afragen · 2025-09-21T18:20:47Z

afragen · 2025-09-21T18:23:38Z

Cache issue. All working/verifying.

Signed-off-by: costdev <[email protected]>

rmccue · 2025-09-22T22:11:55Z

inc/packages/admin/namespace.php

 		return $result;
 	}

+	( new Updater\Updater( $did ) )->run();


For the future: we should rename this method to be clearer, and move some of the global state checks out of it.

inc/updater/class-updater.php

Signed-off-by: costdev <[email protected]>

costdev force-pushed the verify_signature_on_download branch from 8c48893 to a1b0f1e Compare September 9, 2025 06:21

costdev requested review from afragen and rmccue September 9, 2025 06:43

kasparsd reviewed Sep 9, 2025

View reviewed changes

composer.json Outdated Show resolved Hide resolved

inc/updater/class-updater.php Show resolved Hide resolved

inc/updater/class-updater.php Outdated Show resolved Hide resolved

kasparsd approved these changes Sep 9, 2025

View reviewed changes

inc/updater/class-updater.php Outdated Show resolved Hide resolved

costdev force-pushed the verify_signature_on_download branch from 7d438d7 to 9c8ee6e Compare September 12, 2025 04:44

afragen reviewed Sep 12, 2025

View reviewed changes

inc/updater/class-base58btc.php Show resolved Hide resolved

costdev force-pushed the verify_signature_on_download branch from bd2b3af to 488bd27 Compare September 16, 2025 00:21

costdev added 18 commits September 18, 2025 22:10

Verify signatures on download.

7efba75

Signed-off-by: costdev <[email protected]>

Remove unreachable error condition.

fc7c141

verify_file_signature() is documented to return bool|WP_Error, with false meaning verification was not attempted. However, the function's code doesn't actually return false. Signed-off-by: costdev <[email protected]>

Add yocLib - Multibase as a dependency.

4564f01

Signed-off-by: costdev <[email protected]>

Convert signing keys from base58btc to base64.

30edfe8

Signed-off-by: costdev <[email protected]>

Remove unnecessary use WP_Error statement.

fb60c7e

Signed-off-by: costdev <[email protected]>

Only include composer files for distribution.

da9acce

Signed-off-by: costdev <[email protected]>

Remove verson-controlled files check from workflows.

5ba0343

The plugin will include distribution dependencies, whereas workflows install development dependencies too, so this check will always fail. Signed-off-by: costdev <[email protected]>

Return the path if verification succeeds.

62bcf59

Signed-off-by: costdev <[email protected]>

Handle a potential false verification result.

6e1b897

Signed-off-by: costdev <[email protected]>

Pass the publicKeyMultibase value for decoding.

163dce0

This was previously sending the whole key object, resulting in a Fatal Error. Signed-off-by: costdev <[email protected]>

Have PHPCS ignore a non-snake_case property name.

f10d0ae

Signed-off-by: costdev <[email protected]>

Revert "Remove verson-controlled files check from workflows."

df83dab

This reverts commit e430753. Signed-off-by: costdev <[email protected]>

Revert "Only include composer files for distribution."

eb1c057

This reverts commit ca4c5ea. Signed-off-by: costdev <[email protected]>

Revert "Add yocLib - Multibase as a dependency."

97c3005

This reverts commit be67b25. Signed-off-by: costdev <[email protected]>

Port yocLib Multibase Base58 (BTC) over.

7b7408e

Signed-off-by: costdev <[email protected]>

Exclude class-base58btc from PHPCS.

92e6ed3

Signed-off-by: costdev <[email protected]>

Recode signatures from base64url to base64.

4245b23

Signed-off-by: costdev <[email protected]>

Strip the Ed25519 prefix before recoding keys to base64.

1f40d4b

Signed-off-by: costdev <[email protected]>

costdev added 4 commits September 18, 2025 22:10

Have PHPCS ignore a non-snake_case property name.

45be4d7

Signed-off-by: costdev <[email protected]>

Abstract the key material for clarity.

7a0e11b

Signed-off-by: costdev <[email protected]>

Make the $filepath and $local_version properties optional in `Upd…

61fe4c2

…ater\Updater`. Signed-off-by: costdev <[email protected]>

Add updater support during AJAX installation.

3ac5dac

Signed-off-by: costdev <[email protected]>

costdev force-pushed the verify_signature_on_download branch from 696e20a to 3ac5dac Compare September 18, 2025 21:12

costdev marked this pull request as ready for review September 21, 2025 15:01

Add debug code for testers.

dd0da16

Signed-off-by: costdev <[email protected]>

Merge branch 'main' into verify_signature_on_download

bd0ae20

afragen approved these changes Sep 21, 2025

View reviewed changes

afragen self-requested a review September 21, 2025 17:51

Merge branch 'main' into verify_signature_on_download

05bd63d

afragen approved these changes Sep 21, 2025

View reviewed changes

Enable signature verification by default.

02d80e8

Signed-off-by: costdev <[email protected]>

rmccue reviewed Sep 22, 2025

View reviewed changes

Use sha1() instead of md5().

8b849db

Signed-off-by: costdev <[email protected]>

rmccue approved these changes Sep 22, 2025

View reviewed changes

rmccue merged commit d59d2d6 into fairpm:main Sep 22, 2025
46 checks passed

costdev mentioned this pull request Sep 23, 2025

Remove debug message upon successful signature verification. #259

Merged

Verify signatures when downloading packages #247

Verify signatures when downloading packages #247

Uh oh!

Conversation

costdev commented Sep 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approach

New dependency

Pre-merge note

Testing

Uh oh!

kasparsd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kasparsd commented Sep 9, 2025

Uh oh!

costdev commented Sep 9, 2025

Uh oh!

kasparsd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kasparsd commented Sep 9, 2025

Uh oh!

costdev commented Sep 9, 2025

Uh oh!

Uh oh!

costdev commented Sep 18, 2025

Uh oh!

rmccue commented Sep 21, 2025

Uh oh!

costdev commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

costdev commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afragen commented Sep 21, 2025

Uh oh!

afragen commented Sep 21, 2025

Uh oh!

costdev commented Sep 21, 2025

Uh oh!

afragen commented Sep 21, 2025

Uh oh!

afragen commented Sep 21, 2025

Uh oh!

rmccue Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

costdev commented Sep 9, 2025 •

edited

Loading

costdev commented Sep 21, 2025 •

edited

Loading

costdev commented Sep 21, 2025 •

edited

Loading