Signatures: Improve support for hs2019 algorithm #1848
Conversation
Prefixed is_wp_error with a backslash to ensure the global function is used. Added 'secp256r1' as an alias for 'prime256v1' in the curve name switch statement to improve compatibility.
Introduce a fixture with EC and RSA test keys and expand signature verification tests to cover multiple EC curves and RSA key sizes for the hs2019 algorithm. Add tests for valid and invalid signatures, unsupported curves, and update the signature algorithm provider to expect WP_Error for missing or empty algorithms. Refactor test setup to load keys from a JSON fixture for consistency and maintainability.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the handling of hs2019 signatures by deriving the signature algorithm from the actor’s public key and improving error reporting. Key changes include moving public key retrieval earlier in the signature verification flow, extending the get_signature_algorithm() method to accept a public key and return a WP_Error when appropriate, and adding comprehensive tests for various EC and RSA key scenarios.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/includes/class-test-signature.php | Added test keys storage and expanded test cases for EC and RSA hs2019 signatures. |
| tests/fixtures/http-signature-keys.json | Added fixture file with EC and RSA keys for testing signature verification. |
| includes/class-signature.php | Updated signature verification flow and signature algorithm derivation logic. |
Comments suppressed due to low confidence (2)
includes/class-signature.php:377
- [nitpick] Consider removing or rephrasing the comment about nested switch statements to maintain a professional codebase and avoid potential confusion.
// 3 levels switch statements are fine, right?
includes/class-signature.php:366
- Ensure that KB_IN_BYTES is defined or consider using an explicit literal value to improve clarity and avoid potential undefined constant issues.
if ( $bits >= 4 * KB_IN_BYTES ) {
| } | ||
|
|
||
| return new \WP_Error( 'unsupported_key_type', 'Unsupported key type (only RSA and EC keys are supported).', array( 'status' => 401 ) ); | ||
|
|
There was a problem hiding this comment.
@vpzomtrrfrt I'm curious if you'd have any advice on how this could be improved to switch algorithm="hs2019" support for wordpress-activitypub to "Yes"?
There was a problem hiding this comment.
I haven't followed this space as closely lately, the last I heard was that there was no consensus on how keys should indicate their supported algorithms
|
@mediaformat Curious if you had any thoughts on this change? |
Improves handling of hs2019 signatures by deriving the signature algorithm from the actor's public key.
Proposed changes:
get_signature_algorithm()and inspected.get_signature_algorithm()to accept a public key and return a WP_Error.get_signature_algorithm()to return the generated error rather than a generic one.hs2019case to inspect the public key and derive the algorithm used.Other information:
Testing instructions:
Changelog entry
Changelog Entry Details
Significance
Type
Message
hs2019 signatures for incoming REST API requests now have their algorithm determined based on their public key.