Signature: Avoid PHP warning when algorithm is not set

[obenland]

Fixes PHP warning when there is no algorithm key.

Warning: Undefined array key "algorithm" in /wp-content/plugins/activitypub/includes/class-signature.php on line 357

Proposed changes:

• Checks whether the algorithm key exists before accessing it.
• Disables specific phpcs rules.

Other information:

• Have you written new tests for your changes, if applicable?

Testing instructions:

• Go to '..'

Changelog entry

• Automatically create a changelog entry from the details below.

Changelog Entry Details

Significance

• Patch
• Minor
• Major

Type

• Added - for new features
• Changed - for changes in existing functionality
• Deprecated - for soon-to-be removed features
• Removed - for now removed features
• Fixed - for any bug fixes
• Security - in case of vulnerabilities

Message

Inbox requests that are missing an algorithm parameter in their signature no longer create a PHP warning.

[Copilot]

Pull Request Overview

This PR prevents a PHP warning when the algorithm key is missing in the signature block and refines the PHPCS ignore comment to target specific rules.

• Wraps the algorithm access in isset() to avoid undefined array key warnings
• Specifies exact PHPCS rules to ignore on the digest comparison line

Comments suppressed due to low confidence (1)

includes/class-signature.php:357

• There isn’t a test case covering when the algorithm key is missing or empty. Please add unit tests to verify that missing or invalid algorithms neither trigger warnings nor cause unexpected return values.

if ( isset( $signature_block['algorithm'] ) ) {

[obenland]

@mediaformat While looking at this function I opened the spec that's linked to in the rsa-sha-512 case.
Any example there mentioning SHA512 has hs2019 set as its algorithm, not rsa-sha-512:

The authorization header and signature would be generated as:

Authorization: Signature keyId="rsa-key-1",algorithm="hs2019",
headers="(request-target) (created) host digest content-length",
signature="Base64(RSA-SHA512(signing string))"

—RSA Example

The error that gets returned when there is no algorithm says "only rsa-sha256 and hs2019 are supported":

wordpress-activitypub/includes/class-signature.php

Lines 286 to 288 in 499bcad

	if ( ! $algorithm ) {
	return new WP_Error( 'activitypub_signature', __( 'Unsupported signature algorithm (only rsa-sha256 and hs2019 are supported)', 'activitypub' ), array( 'status' => 401 ) );
	}

Should the case statement test for hs2019 instead of rsa-sha-512?

[mediaformat]

@obenland I think the error message is correct, in that the declared algorithm should be either rsa-256 or hs2019, because under hs2019 the key is meant to be derived from keyId.

Implementers SHOULD derive the
digital signature algorithm used by an implementation from the key
metadata identified by the keyId rather than from this field. If
algorithm is provided and differs from the key metadata identified
by the keyId, for example rsa-sha256 but an EdDSA key is
identified via keyId, then an implementation MUST produce an error.
Implementers should note that previous versions of the algorithm
parameter did not use the key information to derive the digital
signature type and thus could be utilized by attackers to expose
security vulnerabilities.
— algorithm

The implementation actually derives the key from the digest header rather than the keyId.

wordpress-activitypub/includes/class-signature.php

Lines 294 to 298 in 499bcad

	$algorithm = 'sha256';
	$digest = explode( '=', $headers['digest'], 2 );
	if ( 'SHA-512' === $digest[0] ) {
	$algorithm = 'sha512';
	}

[obenland]

If algorithm is provided

@mediaformat Based on the doc you shared, I guess we shouldn't return an error if algorithm is not set?

[pfefferle]

What about checking if empty? @mediaformat is it a best practice to always default to sha256 if the algorithm is set but not supported? If not, I would be more strict in the checks!?!