diff --git a/.github/changelog/fix-refresh-key-on-failure b/.github/changelog/fix-refresh-key-on-failure new file mode 100644 index 0000000000..3c3bb6be7a --- /dev/null +++ b/.github/changelog/fix-refresh-key-on-failure @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Accounts on other servers that change their signing key are no longer rejected until their profile is refreshed. diff --git a/includes/class-signature.php b/includes/class-signature.php index 531cf6e176..aef9122a89 100644 --- a/includes/class-signature.php +++ b/includes/class-signature.php @@ -7,6 +7,7 @@ namespace Activitypub; +use Activitypub\Collection\Remote_Actors; use Activitypub\Signature\Http_Message_Signature; use Activitypub\Signature\Http_Signature_Draft; @@ -136,6 +137,7 @@ public static function sign_request( $args, $url ) { * * @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. + * @since unreleased Refreshes a stored actor once when its key does not verify. * * @param \WP_REST_Request|array $request The request object or $_SERVER array. * @@ -152,8 +154,22 @@ public static function verify_http_signature( $request ) { } $signature = isset( $headers['signature_input'] ) ? new Http_Message_Signature() : new Http_Signature_Draft(); + $result = $signature->verify( $headers, $body ?? null ); - return $signature->verify( $headers, $body ?? null ); + /* + * A known key that does not verify, or whose type no longer matches the algorithm, may have + * been rotated. Refresh the stored actor from its own host once and verify again, as + * Mastodon does. + */ + if ( \is_wp_error( $result ) && \array_intersect( array( 'activitypub_signature', 'alg_key_mismatch' ), $result->get_error_codes() ) && \is_object( $request ) ) { + $key_id = self::get_key_id( $request ); + + if ( $key_id && Remote_Actors::refresh_public_key( $key_id ) ) { + $result = $signature->verify( $headers, $body ?? null ); + } + } + + return $result; } /** diff --git a/includes/collection/class-remote-actors.php b/includes/collection/class-remote-actors.php index 28c238c3e2..e887374a5d 100644 --- a/includes/collection/class-remote-actors.php +++ b/includes/collection/class-remote-actors.php @@ -871,6 +871,58 @@ public static function get_public_key( $key_id, &$owner = null ) { return $key_resource; } + /** + * Refresh a stored actor from its own host after its key failed to verify. + * + * The key of a stored actor is read from the stored record first, so an actor that rotates its + * key would stay locked out until something refreshes that record. A key taken from the proxy + * is at most as old as the proxy's cache, so only stored actors are refreshed. + * + * Each actor is refreshed at most once every few minutes, so failing signatures cannot cause a + * fetch every time. The throttle is a transient, not the record's modified date, so the + * scheduler still refreshes, counts errors for and cleans up this actor as before. + * + * @since unreleased + * + * @param string $key_id The keyId whose key did not verify. + * + * @return bool True if the stored actor changed, false otherwise. + */ + public static function refresh_public_key( $key_id ) { + $post = self::get_by_uri( \strip_fragment_from_url( $key_id ) ); + if ( \is_wp_error( $post ) ) { + return false; + } + + // The stored document's id, not the post's guid, which core may rewrite on save. + $stored = \json_decode( $post->post_content, true ); + $id = $stored['id'] ?? null; + if ( ! \is_string( $id ) ) { + return false; + } + + $throttle = 'activitypub_key_refresh_' . \md5( $id ); + if ( \get_transient( $throttle ) ) { + return false; + } + \set_transient( $throttle, 1, 5 * MINUTE_IN_SECONDS ); + + // Proxy::get() only answers with the document its own host serves under this id. + $actor = Proxy::get( $id, array( 'cached' => false ) ); + if ( ! \is_array( $actor ) || ( $actor['id'] ?? null ) !== $id ) { + return false; + } + + $fresh = Actor::init_from_array( $actor ); + + // Only a changed document is worth a write, compared the way it would be stored. + if ( ! $fresh instanceof Actor || \json_decode( self::serialize_for_storage( $fresh )[0], true ) === $stored ) { + return false; + } + + return ! \is_wp_error( self::update( $post, $fresh ) ); + } + /** * Extract public key PEM from a fetched object. * diff --git a/tests/phpunit/tests/includes/class-test-signature.php b/tests/phpunit/tests/includes/class-test-signature.php index eab4c675f4..4806dafeaa 100644 --- a/tests/phpunit/tests/includes/class-test-signature.php +++ b/tests/phpunit/tests/includes/class-test-signature.php @@ -10,6 +10,7 @@ namespace Activitypub\Tests; use Activitypub\Collection\Actors; +use Activitypub\Collection\Remote_Actors; use Activitypub\Http; use Activitypub\Signature; @@ -1089,6 +1090,95 @@ public function test_verify_http_signature_rfc9421() { \delete_option( 'activitypub_rfc9421_signature' ); } + /** + * A stored key that does not verify is refreshed from the actor's host once, so a rotated key heals. + * + * @covers ::verify_http_signature + */ + public function test_verify_http_signature_refreshes_a_rotated_key() { + \update_option( 'activitypub_rfc9421_signature', '0' ); + $old = self::$test_keys['rsa']['4096']; + $new = self::$test_keys['rsa']['2048']; + $id = 'https://example.org/users/rotated'; + + $document = static function ( $public_key ) use ( $id ) { + return array( + 'id' => $id, + 'type' => 'Person', + 'inbox' => $id . '/inbox', + 'preferredUsername' => 'rotated', + 'publicKey' => array( + 'id' => $id . '#main-key', + 'owner' => $id, + 'publicKeyPem' => $public_key, + ), + ); + }; + Remote_Actors::upsert( $document( $old['public_key'] ) ); + + // Let the next refresh through, as if the throttle had expired. + $expire = static function () use ( $id ) { + \delete_transient( 'activitypub_key_refresh_' . \md5( $id ) ); + }; + + $served = $document( $new['public_key'] ); + $fetches = 0; + $mock = static function ( $pre, $url ) use ( &$fetches, &$served, $id ) { + if ( $url !== $id ) { + return $pre; + } + ++$fetches; + return $served; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $mock, 10, 2 ); + + $request = static function () use ( $id, $new ) { + $args = \apply_filters( + 'http_request_args', + array( + 'method' => 'POST', + 'body' => \wp_json_encode( + array( + 'type' => 'Update', + 'actor' => $id, + ) + ), + 'key_id' => $id . '#main-key', + 'private_key' => \openssl_pkey_get_private( $new['private_key'] ), + 'user_id' => 1, + 'headers' => array( + 'Content-Type' => 'application/activity+json', + 'Date' => \gmdate( 'D, d M Y H:i:s T' ), + 'Host' => 'example.org', + ), + ), + 'https://example.org/wp-json/activitypub/1.0/inbox' + ); + $request = new \WP_REST_Request( 'POST', ACTIVITYPUB_REST_NAMESPACE . '/inbox' ); + $request->set_body( $args['body'] ); + $request->set_headers( $args['headers'] ); + + return $request; + }; + + // The actor rotated to the new key; its host serves it, the stored record still has the old one. + $this->assertSame( $id, Signature::verify_http_signature( $request() ), 'A rotated key verifies after one refresh.' ); + $this->assertSame( 1, $fetches ); + + // A key its host does not serve either still fails, and is not fetched again at once. + $served = $document( $old['public_key'] ); + Remote_Actors::upsert( $served ); + $expire(); + $this->assertWPError( Signature::verify_http_signature( $request() ) ); + $this->assertSame( 2, $fetches ); + $this->assertWPError( Signature::verify_http_signature( $request() ) ); + $this->assertSame( 2, $fetches, 'An actor refreshed a moment ago is not fetched again.' ); + + \remove_filter( 'activitypub_pre_http_get_remote_object', $mock, 10 ); + \delete_option( 'activitypub_rfc9421_signature' ); + $expire(); + } + /** * RFC 9421: the keyId returned is the label that actually verified, not merely the first. * 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 64bdead366..66fed587fe 100644 --- a/tests/phpunit/tests/includes/collection/class-test-remote-actors.php +++ b/tests/phpunit/tests/includes/collection/class-test-remote-actors.php @@ -199,6 +199,85 @@ public function test_get_public_key_rejects_a_key_id_without_host() { $this->assertNull( $owner ); } + /** + * A stored actor whose key failed is refreshed from its host, at most once every few minutes. + * + * @covers ::refresh_public_key + */ + public function test_refresh_public_key() { + global $wpdb; + + $id = 'https://example.com/users/rotated'; + $actor = array( + 'id' => $id, + 'type' => 'Person', + 'inbox' => $id . '/inbox', + 'preferredUsername' => 'rotated', + 'publicKey' => array( + 'id' => $id . '#main-key', + 'owner' => $id, + 'publicKeyPem' => $this->x509_key, + ), + ); + $post_id = Remote_Actors::upsert( $actor ); + + $fetches = 0; + $served = $actor; + + $served['publicKey']['publicKeyPem'] = $this->pkcs1_key; + + $mock = static function ( $pre, $url ) use ( &$fetches, &$served, $id ) { + if ( $url !== $id ) { + return $pre; + } + ++$fetches; + return $served; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $mock, 10, 2 ); + + // Let the next refresh through, as if the throttle had expired. + $expire = static function () use ( $id ) { + \delete_transient( 'activitypub_key_refresh_' . \md5( $id ) ); + }; + + // A new key is stored, and a second failing signature right after does not fetch again. + $this->assertTrue( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertFalse( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertSame( 1, $fetches, 'A refreshed actor is not fetched again at once.' ); + $this->assertSame( $this->pkcs1_key, \json_decode( \get_post( $post_id )->post_content, true )['publicKey']['publicKeyPem'] ); + + // The same document again is no change, and leaves the record's modified date to the scheduler. + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching + $wpdb->update( $wpdb->posts, array( 'post_modified_gmt' => '2020-01-01 00:00:00' ), array( 'ID' => $post_id ) ); + \clean_post_cache( $post_id ); + $expire(); + $this->assertFalse( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertSame( 2, $fetches ); + $this->assertSame( '2020-01-01 00:00:00', \get_post( $post_id )->post_modified_gmt, 'An unchanged document is not written.' ); + + // A changed profile with the same key is stored. + $served['name'] = 'Renamed'; + $expire(); + $this->assertTrue( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertSame( 3, $fetches ); + $this->assertSame( 'Renamed', \json_decode( \get_post( $post_id )->post_content, true )['name'] ); + + // A failed fetch changes nothing and is not retried at once. + $served = new \WP_Error( 'activitypub_not_found', 'Not found' ); + $expire(); + $this->assertFalse( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertFalse( Remote_Actors::refresh_public_key( $id . '#main-key' ) ); + $this->assertSame( 4, $fetches, 'A failed fetch is not retried at once.' ); + $this->assertSame( $this->pkcs1_key, \json_decode( \get_post( $post_id )->post_content, true )['publicKey']['publicKeyPem'] ); + + // An actor that is not stored is left to the proxy cache. + $this->assertFalse( Remote_Actors::refresh_public_key( 'https://example.com/users/unknown#main-key' ) ); + $this->assertSame( 4, $fetches ); + + \remove_filter( 'activitypub_pre_http_get_remote_object', $mock, 10 ); + $expire(); + } + /** * Key and owner reference redirects. *