diff --git a/.github/changelog/fix-signing-key-owner b/.github/changelog/fix-signing-key-owner new file mode 100644 index 0000000000..7da24527ad --- /dev/null +++ b/.github/changelog/fix-signing-key-owner @@ -0,0 +1,4 @@ +Significance: patch +Type: security + +Incoming activities are now only accepted when they are signed with the sending account's own key. diff --git a/includes/class-signature.php b/includes/class-signature.php index 0d9b58e377..531cf6e176 100644 --- a/includes/class-signature.php +++ b/includes/class-signature.php @@ -129,15 +129,17 @@ public static function sign_request( $args, $url ) { /** * Verifies the http signatures * - * On success the verified keyId is returned (a truthy string), so callers can bind it to - * the activity actor without re-parsing headers, which cannot tell which signature label - * actually validated. Pass/fail callers should branch on {@see is_wp_error()} as before. + * On success the id of the actor that owns the verified key is returned (a truthy string), so + * callers can bind the signer to the activity actor without re-parsing headers, which cannot + * tell which signature label actually validated. Pass/fail callers should branch on + * {@see is_wp_error()} as before. * * @since 9.0.0 Returns the verified keyId on success instead of `true`. + * @since unreleased Returns the id of the actor that owns the verified key instead of the keyId. * * @param \WP_REST_Request|array $request The request object or $_SERVER array. * - * @return string|\WP_Error The verified keyId on success, WP_Error on failure. + * @return string|\WP_Error The id of the actor that owns the verified key on success, WP_Error on failure. */ public static function verify_http_signature( $request ) { if ( \is_object( $request ) ) { // REST Request object. diff --git a/includes/collection/class-remote-actors.php b/includes/collection/class-remote-actors.php index 8e482c0167..28c238c3e2 100644 --- a/includes/collection/class-remote-actors.php +++ b/includes/collection/class-remote-actors.php @@ -809,11 +809,14 @@ public static function normalize_identifier( $actor ) { /** * Get public key from key_id. * - * @param string $key_id The URL to the public key. + * @since unreleased Added the `$owner` parameter. + * + * @param string $key_id The URL to the public key. + * @param string|null $owner Optional. Set to the id of the actor the key was taken from. * * @return resource|\WP_Error The public key resource or WP_Error. */ - public static function get_public_key( $key_id ) { + public static function get_public_key( $key_id, &$owner = null ) { $no_profile_error = new \WP_Error( 'activitypub_no_remote_profile_found', 'No Profile found or Profile not accessible', array( 'status' => 401 ) ); $no_key_error = new \WP_Error( 'activitypub_no_remote_key_found', 'No Public-Key found', array( 'status' => 401 ) ); @@ -862,6 +865,9 @@ public static function get_public_key( $key_id ) { return $no_key_error; } + // The actor this key belongs to, so the caller can bind it to the activity's actor. + $owner = $actor['id']; + return $key_resource; } diff --git a/includes/functions-request.php b/includes/functions-request.php index ba82738b23..4e782a6dcd 100644 --- a/includes/functions-request.php +++ b/includes/functions-request.php @@ -185,6 +185,16 @@ function maybe_set_no_store( $response ) { /** * Requests the Meta-Data from the Actors profile. * + * Avoid this function in new code and use {@see \Activitypub\Proxy::get()} instead. It returns + * the stored remote actor first (the `ap_actor` post, via {@see Remote_Actors::fetch_by_various()}, + * which resolves an `acct:` handle through WebFinger first) and only fetches when none is stored, + * so it can be stale, and `$cached` is ignored. + * + * Every writer of that record stores what `Proxy::get()` fetched from the actor's own host, except + * an incoming Update, which stores the actor it embeds. Two checks keep that the actor's own data: + * the inbox requires the signing key's owner to be the actor, and `Update::update_actor()` requires + * the object's id to be the actor. Neither may be relaxed. + * * @param array|string $actor The Actor array or URL. * @param bool $cached Optional. Whether the result should be cached. Default true. * diff --git a/includes/rest/trait-verification.php b/includes/rest/trait-verification.php index 32a0b770d1..b891a3640e 100644 --- a/includes/rest/trait-verification.php +++ b/includes/rest/trait-verification.php @@ -92,17 +92,17 @@ public function verify_signature( $request, $force_signature = false ) { * URL hands out, so the two must be answered alike. */ if ( ! \in_array( $request->get_method(), array( 'GET', 'HEAD' ), true ) || use_authorized_fetch() || $force_signature ) { - $verified_key_id = Signature::verify_http_signature( $request ); - if ( \is_wp_error( $verified_key_id ) ) { + $signer = Signature::verify_http_signature( $request ); + if ( \is_wp_error( $signer ) ) { return new \WP_Error( 'activitypub_signature_verification', - $verified_key_id->get_error_message(), + $signer->get_error_message(), array( 'status' => 401 ) ); } - // Verify the signing key's host matches the activity actor's host. - $key_id_check = $this->verify_key_id( $request, $verified_key_id ); + // Verify the signing key belongs to the activity actor. + $key_id_check = $this->verify_key_id( $request, $signer ); if ( \is_wp_error( $key_id_check ) ) { return $key_id_check; } @@ -161,27 +161,28 @@ private function verify_activity_id( $request ) { } /** - * Check that the signature keyId and activity actor share the same host. + * Check that the verified signing key belongs to the activity actor. * - * Binds against the keyId that {@see Signature::verify_http_signature()} actually - * verified, passed in by the caller. Re-parsing the headers here would be unsafe: a - * request can present several signature labels (or a draft and an RFC 9421 header) with - * different keyIds, and only the verifier knows which one validated. + * Binds against the key owner that {@see Signature::verify_http_signature()} returns for the + * signature that actually verified. Re-parsing the headers here would be unsafe: a request can + * present several signature labels (or a draft and an RFC 9421 header) with different keyIds, + * and only the verifier knows which one validated. * - * Fails closed when either side has no parsable host. A non-URL keyId such as - * `acct:mallory@attacker.example` resolves to a real key through WebFinger yet yields no host, - * so treating "no host" as "nothing to check" would drop the only tie between the signing key - * and the claimed actor. Both hosts must be present and equal. + * The owner is the actor {@see \Activitypub\Collection\Remote_Actors::get_public_key()} took the key from, the stored + * actor or the fetched one, and that lookup already requires it on the keyId's host. A keyId + * with no host (an `acct:` keyId) gets no key there. So comparing the owner with the actor + * also binds the hosts. * * @since 8.1.0 * @since 9.0.0 Added the `$key_id` parameter; binds against the verified keyId. * @since 9.2.1 Reject a keyId or actor with no parsable host instead of allowing it. + * @since unreleased The second parameter is the verified key's owner; it has to be the actor. * * @param \WP_REST_Request $request The request object. - * @param string|null $key_id The keyId that verified the signature. + * @param string|null $owner The id of the actor that owns the verified key. * @return true|\WP_Error True if valid, WP_Error on mismatch. */ - private function verify_key_id( $request, $key_id ) { + private function verify_key_id( $request, $owner ) { $json = $request->get_json_params(); $actor = isset( $json['actor'] ) ? object_to_uri( $json['actor'] ) : null; @@ -190,12 +191,17 @@ private function verify_key_id( $request, $key_id ) { return true; } - // The keyId and the actor must resolve to the same host; an identifier with no parsable - // host (e.g. an `acct:` keyId) is unbindable and therefore not an authorized one. - if ( ! is_same_host( $key_id, $actor ) ) { + /* + * The same host is not enough: on a server where users hold their own keys, one user's + * key would pass for another user on that server, so the key's owner has to be the actor. + * The stored actor the owner may come from can be trusted because this check is what keeps + * an Update from writing another actor's key into it. The host check fails closed on an + * identifier without a host, which could otherwise match itself. + */ + if ( ! is_same_host( $owner, $actor ) || $owner !== $actor ) { return new \WP_Error( 'activitypub_key_actor_mismatch', - \__( 'Signing key and activity actor must be on the same host.', 'activitypub' ), + \__( 'Signing key does not belong to the activity actor.', 'activitypub' ), array( 'status' => 403 ) ); } diff --git a/includes/signature/class-http-message-signature.php b/includes/signature/class-http-message-signature.php index b8b8032c5b..aa2a6394ab 100644 --- a/includes/signature/class-http-message-signature.php +++ b/includes/signature/class-http-message-signature.php @@ -137,10 +137,11 @@ public function sign( $args, $url ) { * Verify the HTTP Signature against a request. * * @since 9.0.0 Returns the verified keyId on success instead of `true`. + * @since unreleased Returns the id of the actor that owns the verified key instead of the keyId. * * @param array $headers The HTTP headers. * @param string|null $body The request body, if applicable. - * @return string|\WP_Error The verified keyId on success, WP_Error on failure. + * @return string|\WP_Error The id of the actor that owns the verified key on success, WP_Error on failure. */ public function verify( array $headers, $body = null ) { $parsed = $this->parse_signature_labels( $headers ); @@ -151,17 +152,15 @@ public function verify( array $headers, $body = null ) { $errors = new \WP_Error(); foreach ( $parsed as $data ) { $result = $this->verify_signature_label( $data, $headers, $body ); - if ( true === $result ) { + if ( ! \is_wp_error( $result ) ) { /* - * Return the keyId of the label that actually verified, so the caller binds + * Return the key owner of the label that actually verified, so the caller binds * against the real signer instead of guessing among several labels. */ - return $data['params']['keyid']; + return $result; } - if ( \is_wp_error( $result ) ) { - $errors->add( $result->get_error_code(), $result->get_error_message() ); - } + $errors->add( $result->get_error_code(), $result->get_error_message() ); } // No valid signature found. @@ -225,7 +224,7 @@ private function parse_signature_labels( array $headers ) { * @param array $data Parsed signature data. * @param array $headers HTTP headers. * @param string|null $body Request body, if applicable. - * @return bool|\WP_Error True, if the signature is valid, WP_Error on failure. + * @return string|\WP_Error The id of the actor that owns the label's key, if the signature is valid, WP_Error on failure. */ private function verify_signature_label( $data, $headers, $body ) { $params = $data['params']; @@ -299,11 +298,17 @@ static function ( $component ) { return new \WP_Error( 'missing_keyid', 'Missing keyId in signature parameters.' ); } - $public_key = Remote_Actors::get_public_key( $params['keyid'] ); + $owner = null; + $public_key = Remote_Actors::get_public_key( $params['keyid'], $owner ); if ( \is_wp_error( $public_key ) ) { return $public_key; } + // The owner is what callers bind to the actor, so a key without one does not verify anything. + if ( ! \is_string( $owner ) || '' === $owner ) { + return new \WP_Error( 'activitypub_no_key_owner', 'The signing key has no owner.' ); + } + // Algorithm verification. $algorithm = $this->verify_algorithm( $params['alg'] ?? '', $public_key ); if ( \is_wp_error( $algorithm ) ) { @@ -323,7 +328,7 @@ static function ( $component ) { return new \WP_Error( 'activitypub_signature', 'Invalid signature' ); } - return true; + return $owner; } /** diff --git a/includes/signature/class-http-signature-draft.php b/includes/signature/class-http-signature-draft.php index 3bdb62a804..2e9be8d841 100644 --- a/includes/signature/class-http-signature-draft.php +++ b/includes/signature/class-http-signature-draft.php @@ -91,10 +91,11 @@ public function sign( $args, $url ) { * Verify the HTTP Signature against a request. * * @since 9.0.0 Returns the verified keyId on success instead of `true`. + * @since unreleased Returns the id of the actor that owns the verified key instead of the keyId. * * @param array $headers The HTTP headers. * @param string|null $body The request body, if applicable. - * @return string|\WP_Error The verified keyId on success, WP_Error on failure. + * @return string|\WP_Error The id of the actor that owns the verified key on success, WP_Error on failure. */ public function verify( array $headers, $body = null ) { if ( ! isset( $headers['signature'] ) && ! isset( $headers['authorization'] ) ) { @@ -127,11 +128,17 @@ public function verify( array $headers, $body = null ) { return new \WP_Error( 'invalid_signed_data', 'Signed data is invalid or expired.' ); } - $public_key = Remote_Actors::get_public_key( $parsed['keyId'] ); + $owner = null; + $public_key = Remote_Actors::get_public_key( $parsed['keyId'], $owner ); if ( \is_wp_error( $public_key ) ) { return $public_key; } + // The owner is what callers bind to the actor, so a key without one does not verify anything. + if ( ! \is_string( $owner ) || '' === $owner ) { + return new \WP_Error( 'activitypub_no_key_owner', 'The signing key has no owner.', array( 'status' => 401 ) ); + } + $algorithm = $this->get_signature_algorithm( $parsed, $public_key ); if ( \is_wp_error( $algorithm ) ) { return $algorithm; @@ -148,8 +155,8 @@ public function verify( array $headers, $body = null ) { return new \WP_Error( 'activitypub_signature', 'Invalid signature', array( 'status' => 401 ) ); } - // Return the verified keyId so the caller can bind it to the activity actor. - return $parsed['keyId']; + // Return the key's owner so the caller can bind it to the activity actor. + return $owner; } /** diff --git a/includes/signature/interface-http-signature.php b/includes/signature/interface-http-signature.php index 760ff2e69b..7dcf3f9483 100644 --- a/includes/signature/interface-http-signature.php +++ b/includes/signature/interface-http-signature.php @@ -32,7 +32,7 @@ public function sign( $args, $url ); * * @param array $headers The HTTP headers. * @param string|null $body The request body, if applicable. - * @return string|\WP_Error The verified keyId on success, WP_Error on failure. + * @return string|\WP_Error The id of the actor that owns the verified key on success, WP_Error on failure. */ public function verify( array $headers, $body = null ); diff --git a/tests/phpunit/tests/includes/class-test-signature.php b/tests/phpunit/tests/includes/class-test-signature.php index fe10cafd1c..eab4c675f4 100644 --- a/tests/phpunit/tests/includes/class-test-signature.php +++ b/tests/phpunit/tests/includes/class-test-signature.php @@ -1157,9 +1157,9 @@ public function test_verify_http_signature_rfc9421_returns_verifying_label_key_i $result = Signature::verify_http_signature( $request ); $this->assertSame( - 'https://example.org/author/admin#main-key', + 'https://example.org/author/admin', $result, - 'The keyId of the label that actually verified must be returned, not the first label.' + 'The key owner of the label that actually verified must be returned, not the first label.' ); \remove_filter( 'activitypub_pre_http_get_remote_object', $mock_remote_key_retrieval ); @@ -1823,7 +1823,7 @@ public function test_verify_http_signature_returns_verified_key_id() { $result = Signature::verify_http_signature( $request ); - $this->assertSame( 'https://example.org/author/admin#main-key', $result, 'The verified keyId is returned on success.' ); + $this->assertSame( 'https://example.org/author/admin', $result, 'The verified key\'s owner is returned on success.' ); \remove_filter( 'activitypub_pre_http_get_remote_object', $mock_remote_key_retrieval ); } diff --git a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php index c7cb1504ba..64bdead366 100644 --- a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php +++ b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php @@ -120,6 +120,85 @@ public function test_get_public_key_redirected_reference( $owner_redirect, $same } } + /** + * The key's owner is the actor the key was taken from: stored, fetched, or a standalone key's owner. + * + * @covers ::get_public_key + */ + public function test_get_public_key_sets_owner() { + $stored = 'https://example.com/users/stored'; + Remote_Actors::upsert( + array( + 'id' => $stored, + 'type' => 'Person', + 'inbox' => $stored . '/inbox', + 'preferredUsername' => 'stored', + 'publicKey' => array( + 'id' => $stored . '#main-key', + 'owner' => $stored, + 'publicKeyPem' => $this->x509_key, + ), + ) + ); + + $this->stub_remote_requests(); + $fetched = 'https://example.com/users/fetched'; + $this->responses[ $fetched ] = array( + 'id' => $fetched, + 'type' => 'Person', + 'publicKey' => array( + 'id' => $fetched . '#main-key', + 'owner' => $fetched, + 'publicKeyPem' => $this->x509_key, + ), + ); + $this->responses['https://example.com/keys/1'] = array( + 'id' => 'https://example.com/keys/1', + 'owner' => $fetched, + 'publicKeyPem' => $this->x509_key, + ); + + $owners = array(); + foreach ( array( $stored . '#main-key', $fetched . '#main-key', 'https://example.com/keys/1' ) as $key_id ) { + $owner = null; + $this->assertNotWPError( Remote_Actors::get_public_key( $key_id, $owner ) ); + $owners[] = $owner; + } + $this->unstub_remote_requests(); + + $this->assertSame( array( $stored, $fetched, $fetched ), $owners ); + } + + /** + * A keyId without a host cannot name the actor it belongs to, so it gets no key and no owner. + * + * @covers ::get_public_key + */ + public function test_get_public_key_rejects_a_key_id_without_host() { + $actor = array( + 'id' => 'https://attacker.example/users/mallory', + 'type' => 'Person', + 'publicKey' => array( + 'id' => 'https://attacker.example/users/mallory#main-key', + 'owner' => 'https://attacker.example/users/mallory', + 'publicKeyPem' => $this->x509_key, + ), + ); + $mock = static function () use ( $actor ) { + return $actor; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $mock ); + + $owner = null; + $result = Remote_Actors::get_public_key( 'acct:mallory@attacker.example', $owner ); + + \remove_filter( 'activitypub_pre_http_get_remote_object', $mock ); + + $this->assertWPError( $result ); + $this->assertSame( 'activitypub_no_remote_key_found', $result->get_error_code() ); + $this->assertNull( $owner ); + } + /** * Key and owner reference redirects. * diff --git a/tests/phpunit/tests/includes/rest/class-test-actors-inbox-controller.php b/tests/phpunit/tests/includes/rest/class-test-actors-inbox-controller.php index eff15927a6..8794f13bcf 100644 --- a/tests/phpunit/tests/includes/rest/class-test-actors-inbox-controller.php +++ b/tests/phpunit/tests/includes/rest/class-test-actors-inbox-controller.php @@ -494,6 +494,9 @@ public function test_user_inbox_post_verification() { $remote_object_filter = function ( $json, $actor ) { $public_key = Actors::get_public_key( self::$user_id ); + // The key is fetched by its keyId; like a real server, answer with the actor's id without the fragment. + $actor = \strip_fragment_from_url( $actor ); + // Return ActivityPub Profile with signature. return array( 'id' => $actor, diff --git a/tests/phpunit/tests/includes/rest/class-test-trait-verification.php b/tests/phpunit/tests/includes/rest/class-test-trait-verification.php index 835a8fcc90..1f0b96f08d 100644 --- a/tests/phpunit/tests/includes/rest/class-test-trait-verification.php +++ b/tests/phpunit/tests/includes/rest/class-test-trait-verification.php @@ -228,6 +228,98 @@ public function redirected_signing_key_provider() { ); } + /** + * A key signs only for its own actor, not for another actor on the same host. + * + * @dataProvider signing_key_owner_provider + * @covers ::verify_signature + * + * @param string $format Signature format. + * @param string $actor The actor the activity claims, on the signer's host. + * @param bool $passes Whether the signature has to be accepted. + */ + public function test_verify_signature_binds_the_key_to_its_actor( $format, $actor, $passes ) { + $this->stub_remote_requests(); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents + $fixtures = \json_decode( \file_get_contents( AP_TESTS_DIR . '/data/fixtures/http-signature-keys.json' ), true )['rsa']; + $keys = $fixtures['2048']; + $signer = 'https://example.com/users/mallory'; + $this->responses[ $signer ] = array( + 'id' => $signer, + 'type' => 'Person', + 'publicKey' => array( + 'id' => $signer . '#main-key', + 'owner' => $signer, + 'publicKeyPem' => $keys['public_key'], + ), + ); + $this->responses['https://example.com/users/alice'] = array( + 'id' => 'https://example.com/users/alice', + 'type' => 'Person', + 'publicKey' => array( + 'id' => 'https://example.com/users/alice#main-key', + 'owner' => 'https://example.com/users/alice', + 'publicKeyPem' => $fixtures['4096']['public_key'], + ), + ); + + $route = '/' . ACTIVITYPUB_REST_NAMESPACE . '/inbox'; + $url = \set_url_scheme( 'http://' . \wp_parse_url( \home_url(), PHP_URL_HOST ) . \wp_parse_url( \home_url( '/' . \rest_get_url_prefix() . $route ), PHP_URL_PATH ) ); + $method = 'draft' === $format ? new Http_Signature_Draft() : new Http_Message_Signature(); + $args = $method->sign( + array( + 'method' => 'POST', + 'body' => \wp_json_encode( + array( + 'id' => 'https://example.com/activities/1', + 'type' => 'Update', + 'actor' => $actor, + 'object' => array( 'id' => $actor ), + ) + ), + 'headers' => array( + 'Host' => \wp_parse_url( $url, PHP_URL_HOST ), + 'Date' => \gmdate( 'D, d M Y H:i:s T' ), + 'Content-Type' => 'application/activity+json', + ), + 'key_id' => $signer . '#main-key', + 'private_key' => \openssl_pkey_get_private( $keys['private_key'] ), + ), + $url + ); + + $_SERVER['REQUEST_URI'] = \wp_parse_url( $url, PHP_URL_PATH ); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_SERVER['HTTP_HOST'] = \wp_parse_url( $url, PHP_URL_HOST ); + $request = new \WP_REST_Request( 'POST', $route ); + $request->set_headers( $args['headers'] ); + $request->set_body( $args['body'] ); + + $result = $this->instance->verify_signature( $request ); + + if ( $passes ) { + $this->assertTrue( $result ); + } else { + $this->assertWPError( $result ); + $this->assertSame( 'activitypub_key_actor_mismatch', $result->get_error_code() ); + $this->assertSame( 403, $result->get_error_data()['status'] ); + } + } + + /** + * Signer and claimed actor on the same host, in both signature formats. + * + * @return array Test cases. + */ + public function signing_key_owner_provider() { + return array( + 'draft own actor' => array( 'draft', 'https://example.com/users/mallory', true ), + 'draft other actor' => array( 'draft', 'https://example.com/users/alice', false ), + 'rfc9421 own actor' => array( 'rfc9421', 'https://example.com/users/mallory', true ), + 'rfc9421 other actor' => array( 'rfc9421', 'https://example.com/users/alice', false ), + ); + } + /** * Test defer filter bypasses signature verification. * @@ -659,85 +751,77 @@ public function test_verify_owner_blog_actor_filter_revokes_admin_access() { /** * Data provider for verify_key_id tests. * - * @return array[] Test cases: [ verified_key_id, actor, expected_pass ]. + * @return array[] Test cases: [ verified key owner, actor, expected_pass ]. */ public function data_verify_key_id() { return array( - 'matching hosts' => array( - 'https://remote.example/users/alice#main-key', + 'own key' => array( + 'https://remote.example/users/alice', 'https://remote.example/users/alice', true, ), - 'mismatched hosts' => array( - 'https://evil.example/users/alice#main-key', + // A user on the same server who holds their own key must not sign for another user. + 'key of another actor' => array( + 'https://remote.example/users/mallory', + 'https://remote.example/users/alice', + false, + ), + 'key on another host' => array( + 'https://evil.example/users/alice', 'https://remote.example/users/alice', false, ), 'no actor in body' => array( - 'https://remote.example/users/alice#main-key', + 'https://remote.example/users/alice', null, true, ), - // A missing keyId cannot be bound to the actor, so it must fail closed. - 'no verified key id' => array( + // No owner cannot be bound to the actor, so it must fail closed. + 'no key owner' => array( null, 'https://remote.example/users/alice', false, ), - // A non-URL `acct:` keyId resolves to a real key but yields no host: the bypass this guards. - 'acct key id cannot bind' => array( - 'acct:mallory@attacker.example', - 'https://victim.example/users/alice', + 'empty key owner' => array( + '', + 'https://remote.example/users/alice', false, ), - // An `acct:` actor likewise has no host and cannot be bound. 'acct actor cannot bind' => array( - 'https://remote.example/users/alice#main-key', + 'https://remote.example/users/alice', 'acct:alice@remote.example', false, ), - // The FeatureRequest fill sets actor = strip_fragment( keyId ); a hostless `acct:` - // keyId makes both sides hostless, which must still fail closed rather than self-match. - 'acct key id and actor' => array( + // A hostless owner must not bind by matching a hostless actor, as the FeatureRequest fill could produce. + 'acct owner and actor' => array( 'acct:mallory@attacker.example', 'acct:mallory@attacker.example', false, ), - // An empty keyId string has no host and cannot be bound. - 'empty key id' => array( - '', - 'https://remote.example/users/alice', - false, - ), 'actor as object with id' => array( - 'https://remote.example/users/alice#main-key', + 'https://remote.example/users/alice', array( 'id' => 'https://remote.example/users/alice' ), true, ), 'actor object mismatch' => array( - 'https://evil.example/users/alice#main-key', + 'https://remote.example/users/mallory', array( 'id' => 'https://remote.example/users/alice' ), false, ), - 'case-insensitive hosts' => array( - 'https://Remote.Example/users/alice#main-key', - 'https://remote.example/users/alice', - true, - ), ); } /** - * Test that verify_key_id checks the verified keyId's host against the actor host. + * Test that verify_key_id requires the verified key's owner to be the actor. * * @dataProvider data_verify_key_id * @covers ::verify_key_id * - * @param string|null $key_id The keyId that verified the signature. + * @param string|null $owner The id of the actor that owns the verified key. * @param string|array|null $actor The actor value in the JSON body. * @param bool $should_pass Whether the check should pass. */ - public function test_verify_key_id( $key_id, $actor, $should_pass ) { + public function test_verify_key_id( $owner, $actor, $should_pass ) { $request = new \WP_REST_Request( 'POST', '/activitypub/1.0/inbox' ); $body = array( @@ -758,7 +842,7 @@ public function test_verify_key_id( $key_id, $actor, $should_pass ) { if ( \PHP_VERSION_ID < 80100 ) { $method->setAccessible( true ); } - $result = $method->invoke( $this->instance, $request, $key_id ); + $result = $method->invoke( $this->instance, $request, $owner ); if ( $should_pass ) { $this->assertTrue( $result );