diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ef8071df..d306d863f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Improve Interactions moderation * Compatibility with Akismet * Comment type mapping for `Like` and `Announce` +* Signature verification for API endpoints * Changed priority of Attachments, to favor `Image` over `Audio` and `Video` ### Fixed diff --git a/includes/class-signature.php b/includes/class-signature.php index 5adc6094e..89aa52114 100644 --- a/includes/class-signature.php +++ b/includes/class-signature.php @@ -321,7 +321,6 @@ public static function verify_http_signature( $request ) { } $verified = \openssl_verify( $signed_data, $signature_block['signature'], $public_key, $algorithm ) > 0; - if ( ! $verified ) { return new WP_Error( 'activitypub_signature', __( 'Invalid signature', 'activitypub' ), array( 'status' => 401 ) ); } @@ -403,7 +402,11 @@ public static function parse_signature_header( $signature ) { $parsed_header['signature'] = \base64_decode( preg_replace( '/\s+/', '', trim( $matches[1] ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode } - if ( ( $parsed_header['signature'] ) && ( $parsed_header['algorithm'] ) && ( ! $parsed_header['headers'] ) ) { + if ( + ! empty( $parsed_header['signature'] ) && + ! empty( $parsed_header['algorithm'] ) && + empty( $parsed_header['headers'] ) + ) { $parsed_header['headers'] = array( 'date' ); } @@ -461,6 +464,10 @@ public static function get_signed_data( $signed_headers, $signature_block, $head } } if ( 'date' === $header ) { + if ( empty( $headers[ $header ][0] ) ) { + continue; + } + // Allow a bit of leeway for misconfigured clocks. $d = new DateTime( $headers[ $header ][0] ); $d->setTimeZone( new DateTimeZone( 'UTC' ) ); @@ -474,7 +481,10 @@ public static function get_signed_data( $signed_headers, $signature_block, $head return false; } } - $signed_data .= $header . ': ' . $headers[ $header ][0] . "\n"; + + if ( ! empty( $headers[ $header ][0] ) ) { + $signed_data .= $header . ': ' . $headers[ $header ][0] . "\n"; + } } return \rtrim( $signed_data, "\n" ); } diff --git a/includes/rest/class-actors.php b/includes/rest/class-actors.php index 3e146ff7c..00de9bf10 100644 --- a/includes/rest/class-actors.php +++ b/includes/rest/class-actors.php @@ -41,7 +41,7 @@ public static function register_routes() { array( 'methods' => WP_REST_Server::READABLE, 'callback' => array( self::class, 'get' ), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); diff --git a/includes/rest/class-followers.php b/includes/rest/class-followers.php index a8705a088..ee25ecc98 100644 --- a/includes/rest/class-followers.php +++ b/includes/rest/class-followers.php @@ -43,7 +43,7 @@ public static function register_routes() { 'methods' => WP_REST_Server::READABLE, 'callback' => array( self::class, 'get' ), 'args' => self::request_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); diff --git a/includes/rest/class-following.php b/includes/rest/class-following.php index aca50a43c..79cba650f 100644 --- a/includes/rest/class-following.php +++ b/includes/rest/class-following.php @@ -43,7 +43,7 @@ public static function register_routes() { 'methods' => \WP_REST_Server::READABLE, 'callback' => array( self::class, 'get' ), 'args' => self::request_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); diff --git a/includes/rest/class-inbox.php b/includes/rest/class-inbox.php index bedf602e4..9d0b44c47 100644 --- a/includes/rest/class-inbox.php +++ b/includes/rest/class-inbox.php @@ -45,7 +45,7 @@ public static function register_routes() { 'methods' => WP_REST_Server::CREATABLE, 'callback' => array( self::class, 'shared_inbox_post' ), 'args' => self::shared_inbox_post_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); @@ -58,13 +58,13 @@ public static function register_routes() { 'methods' => WP_REST_Server::CREATABLE, 'callback' => array( self::class, 'user_inbox_post' ), 'args' => self::user_inbox_post_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), array( 'methods' => WP_REST_Server::READABLE, 'callback' => array( self::class, 'user_inbox_get' ), 'args' => self::user_inbox_get_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); diff --git a/includes/rest/class-outbox.php b/includes/rest/class-outbox.php index 7f363824c..22183bd29 100644 --- a/includes/rest/class-outbox.php +++ b/includes/rest/class-outbox.php @@ -45,7 +45,7 @@ public static function register_routes() { 'methods' => WP_REST_Server::READABLE, 'callback' => array( self::class, 'user_outbox_get' ), 'args' => self::request_parameters(), - 'permission_callback' => '__return_true', + 'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ), ), ) ); diff --git a/includes/rest/class-server.php b/includes/rest/class-server.php index 93bb35356..cf36a3633 100644 --- a/includes/rest/class-server.php +++ b/includes/rest/class-server.php @@ -35,8 +35,7 @@ public static function init() { * Add sever hooks. */ public static function add_hooks() { - \add_filter( 'rest_request_before_callbacks', array( self::class, 'validate_activitypub_requests' ), 9, 3 ); - \add_filter( 'rest_request_before_callbacks', array( self::class, 'authorize_activitypub_requests' ), 10, 3 ); + \add_filter( 'rest_request_before_callbacks', array( self::class, 'validate_requests' ), 9, 3 ); \add_filter( 'rest_request_parameter_order', array( self::class, 'request_parameter_order' ), 10, 2 ); } @@ -74,39 +73,24 @@ public static function application_actor() { } /** - * Callback function to authorize each api requests + * Callback function to authorize an api request. * - * @see WP_REST_Request + * The function is meant to be used as part of permission callbacks for rest api endpoints. + * + * It verifies the signature of POST, PUT, PATCH, and DELETE requests, as well as GET requests in secure mode. + * You can use the filter 'activitypub_defer_signature_verification' to defer the signature verification. + * HEAD requests are always bypassed. * * @see https://www.w3.org/wiki/SocialCG/ActivityPub/Primer/Authentication_Authorization#Authorized_fetch * @see https://swicg.github.io/activitypub-http-signature/#authorized-fetch * - * @param WP_REST_Response|\WP_HTTP_Response|WP_Error|mixed $response Result to send to the client. - * Usually a WP_REST_Response or WP_Error. - * @param array $handler Route handler used for the request. - * @param \WP_REST_Request $request Request used to generate the response. + * @param \WP_REST_Request $request The request object. * - * @return mixed|WP_Error The response, error, or modified response. + * @return bool|\WP_Error True if the request is authorized, WP_Error if not. */ - public static function authorize_activitypub_requests( $response, $handler, $request ) { + public static function verify_signature( $request ) { if ( 'HEAD' === $request->get_method() ) { - return $response; - } - - if ( \is_wp_error( $response ) ) { - return $response; - } - - $route = $request->get_route(); - - // Check if it is an activitypub request and exclude webfinger and nodeinfo endpoints. - if ( - ! \str_starts_with( $route, '/' . ACTIVITYPUB_REST_NAMESPACE ) || - \str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'webfinger' ) || - \str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'nodeinfo' ) || - \str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'application' ) - ) { - return $response; + return true; } /** @@ -123,11 +107,11 @@ public static function authorize_activitypub_requests( $response, $handler, $req $defer = \apply_filters( 'activitypub_defer_signature_verification', false, $request ); if ( $defer ) { - return $response; + return true; } if ( - // POST-Requests are always signed. + // POST-Requests always have to be signed. 'GET' !== $request->get_method() || // GET-Requests only require a signature in secure mode. ( 'GET' === $request->get_method() && use_authorized_fetch() ) @@ -142,7 +126,7 @@ public static function authorize_activitypub_requests( $response, $handler, $req } } - return $response; + return true; } /** @@ -155,7 +139,7 @@ public static function authorize_activitypub_requests( $response, $handler, $req * * @return mixed|WP_Error The response, error, or modified response. */ - public static function validate_activitypub_requests( $response, $handler, $request ) { + public static function validate_requests( $response, $handler, $request ) { if ( 'HEAD' === $request->get_method() ) { return $response; } diff --git a/readme.txt b/readme.txt index 658d9e882..0d952a4ed 100644 --- a/readme.txt +++ b/readme.txt @@ -142,6 +142,7 @@ For reasons of data protection, it is not possible to see the followers of other * Improved: Interactions moderation * Improved: Compatibility with Akismet * Improved: Comment type mapping for `Like` and `Announce` +* Improved: Signature verification for API endpoints * Improved: Changed priority of Attachments, to favor `Image` over `Audio` and `Video` * Fixed: Empty `url` attributes in the Reply block no longer cause PHP warnings diff --git a/tests/includes/rest/class-test-inbox.php b/tests/includes/rest/class-test-inbox.php index 9af64b8a1..849b48784 100644 --- a/tests/includes/rest/class-test-inbox.php +++ b/tests/includes/rest/class-test-inbox.php @@ -13,6 +13,48 @@ * @coversDefaultClass \Activitypub\Rest\Inbox */ class Test_Inbox extends \WP_UnitTestCase { + /** + * Test user ID. + * + * @var int + */ + protected static $user_id; + + /** + * Post ID. + * + * @var int + */ + protected static $post_id; + + /** + * Create fake data before tests run. + * + * @param WP_UnitTest_Factory $factory Helper that creates fake data. + */ + public static function wpSetUpBeforeClass( $factory ) { + self::$user_id = $factory->user->create( + array( + 'role' => 'author', + ) + ); + + self::$post_id = $factory->post->create( + array( + 'post_author' => self::$user_id, + 'post_title' => 'Test Post', + 'post_content' => 'Test Content', + 'post_status' => 'publish', + ) + ); + } + + /** + * Clean up after tests. + */ + public static function wpTearDownAfterClass() { + wp_delete_user( self::$user_id ); + } /** * Set up the test. @@ -28,7 +70,6 @@ public function set_up() { */ public function tear_down() { \delete_option( 'permalink_structure' ); - \add_filter( 'activitypub_defer_signature_verification', '__return_false' ); } /** @@ -75,6 +116,8 @@ public function test_missing_attribute() { $this->assertEquals( 400, $response->get_status() ); $this->assertEquals( 'rest_missing_callback_param', $response->get_data()['code'] ); $this->assertEquals( 'object', $response->get_data()['data']['params'][0] ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -97,6 +140,8 @@ public function test_follow_request() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -119,6 +164,8 @@ public function test_follow_request_global_inbox() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -159,6 +206,8 @@ public function test_create_request() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -199,6 +248,8 @@ public function test_create_request_global_inbox() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -227,6 +278,8 @@ public function test_update_request() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -249,6 +302,8 @@ public function test_like_request() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -271,6 +326,8 @@ public function test_announce_request() { // Dispatch the request. $response = \rest_do_request( $request ); $this->assertEquals( 202, $response->get_status() ); + + \remove_filter( 'activitypub_defer_signature_verification', '__return_true' ); } /** @@ -354,4 +411,80 @@ public function the_data_provider() { ), ); } + + /** + * Test user_inbox_post verification. + * + * @covers ::user_inbox_post + */ + public function test_user_inbox_post_verification() { + add_filter( + 'pre_get_remote_metadata_by_actor', + function ( $json, $actor ) { + $user = \Activitypub\Collection\Actors::get_by_id( self::$user_id ); + $public_key = \Activitypub\Signature::get_public_key_for( $user->get__id() ); + + // Return ActivityPub Profile with signature. + return array( + 'id' => $actor, + 'type' => 'Person', + 'publicKey' => array( + 'id' => $actor . '#main-key', + 'owner' => $actor, + 'publicKeyPem' => $public_key, + ), + ); + }, + 10, + 2 + ); + + // Get the post object. + $post = get_post( self::$post_id ); + + // Test valid request. + $actor = \Activitypub\Collection\Actors::get_by_id( self::$user_id ); + $object = \Activitypub\Transformer\Post::transform( $post )->to_object(); + $activity = new \Activitypub\Activity\Activity( 'Like' ); + $activity->from_array( + array( + 'id' => 'https://example.com/activity/1', + 'type' => 'Like', + 'actor' => 'https://example.com/actor', + 'object' => $object->get_id(), + ) + ); + + // Mock remote actor URL. + $activity->add_cc( $actor->get_id() ); + $activity = $activity->to_json(); + + // Generate_digest & generate_signature. + $digest = \Activitypub\Signature::generate_digest( $activity ); + $date = gmdate( 'D, d M Y H:i:s T' ); + $signature = \Activitypub\Signature::generate_signature( self::$user_id, 'POST', $actor->get_inbox(), $date, $digest ); + + $this->assertMatchesRegularExpression( + '/keyId="' . preg_quote( $actor->get_id(), '/' ) . '#main-key",algorithm="rsa-sha256",headers="\(request-target\) host date digest",signature="[^"]*"/', + $signature + ); + + // Signed headers. + $url_parts = wp_parse_url( $actor->get_inbox() ); + $route = $url_parts['path']; + $host = $url_parts['host']; + + $request = new \WP_REST_Request( 'POST', str_replace( '/wp-json', '', $route ) ); + $request->set_header( 'content-type', 'application/activity+json' ); + $request->set_header( 'digest', $digest ); + $request->set_header( 'signature', $signature ); + $request->set_header( 'date', $date ); + $request->set_header( 'host', $host ); + $request->set_body( $activity ); + + $response = \rest_do_request( $request ); + $this->assertEquals( 202, $response->get_status() ); + + remove_filter( 'pre_get_remote_metadata_by_actor', '__return_true' ); + } }