Skip to content

Commit f080cbd

Browse files
pfefferleobenland
pfefferle
and
obenland
authored
Use a more explicit signature verification (#1078)
* Use a more explicit signature verification This is a proposal to use the `permission_callback`, instead of a general hook, to verify signatures. The advantage is, that it is easier to enable/disable verification for specific endpoints this way. See #1077 * phpcs fix * fix test * ignore this for now * changelog * keep the old error and change the function name to be more desciptive props @jeherve * add some phpdoc props @jeherve * verify actor endpoints * no need to mention `activitypub` here * rename functions * Update includes/rest/class-server.php Co-authored-by: Konstantin Obenland <[email protected]> * Update includes/rest/class-server.php Co-authored-by: Konstantin Obenland <[email protected]> * messed up search/replace * one last change * add some checks to prevent PHP warnings * add integration test * fix phpcs --------- Co-authored-by: Konstantin Obenland <[email protected]>
1 parent cda1bf4 commit f080cbd

File tree

10 files changed

+171
-42
lines changed

10 files changed

+171
-42
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2020
* Improve Interactions moderation
2121
* Compatibility with Akismet
2222
* Comment type mapping for `Like` and `Announce`
23+
* Signature verification for API endpoints
2324
* Changed priority of Attachments, to favor `Image` over `Audio` and `Video`
2425

2526
### Fixed

‎includes/class-signature.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,6 @@ public static function verify_http_signature( $request ) {
321321
}
322322

323323
$verified = \openssl_verify( $signed_data, $signature_block['signature'], $public_key, $algorithm ) > 0;
324-
325324
if ( ! $verified ) {
326325
return new WP_Error( 'activitypub_signature', __( 'Invalid signature', 'activitypub' ), array( 'status' => 401 ) );
327326
}
@@ -403,7 +402,11 @@ public static function parse_signature_header( $signature ) {
403402
$parsed_header['signature'] = \base64_decode( preg_replace( '/\s+/', '', trim( $matches[1] ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_decode
404403
}
405404

406-
if ( ( $parsed_header['signature'] ) && ( $parsed_header['algorithm'] ) && ( ! $parsed_header['headers'] ) ) {
405+
if (
406+
! empty( $parsed_header['signature'] ) &&
407+
! empty( $parsed_header['algorithm'] ) &&
408+
empty( $parsed_header['headers'] )
409+
) {
407410
$parsed_header['headers'] = array( 'date' );
408411
}
409412

@@ -461,6 +464,10 @@ public static function get_signed_data( $signed_headers, $signature_block, $head
461464
}
462465
}
463466
if ( 'date' === $header ) {
467+
if ( empty( $headers[ $header ][0] ) ) {
468+
continue;
469+
}
470+
464471
// Allow a bit of leeway for misconfigured clocks.
465472
$d = new DateTime( $headers[ $header ][0] );
466473
$d->setTimeZone( new DateTimeZone( 'UTC' ) );
@@ -474,7 +481,10 @@ public static function get_signed_data( $signed_headers, $signature_block, $head
474481
return false;
475482
}
476483
}
477-
$signed_data .= $header . ': ' . $headers[ $header ][0] . "\n";
484+
485+
if ( ! empty( $headers[ $header ][0] ) ) {
486+
$signed_data .= $header . ': ' . $headers[ $header ][0] . "\n";
487+
}
478488
}
479489
return \rtrim( $signed_data, "\n" );
480490
}

‎includes/rest/class-actors.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public static function register_routes() {
4141
array(
4242
'methods' => WP_REST_Server::READABLE,
4343
'callback' => array( self::class, 'get' ),
44-
'permission_callback' => '__return_true',
44+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
4545
),
4646
)
4747
);

‎includes/rest/class-followers.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public static function register_routes() {
4343
'methods' => WP_REST_Server::READABLE,
4444
'callback' => array( self::class, 'get' ),
4545
'args' => self::request_parameters(),
46-
'permission_callback' => '__return_true',
46+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
4747
),
4848
)
4949
);

‎includes/rest/class-following.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public static function register_routes() {
4343
'methods' => \WP_REST_Server::READABLE,
4444
'callback' => array( self::class, 'get' ),
4545
'args' => self::request_parameters(),
46-
'permission_callback' => '__return_true',
46+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
4747
),
4848
)
4949
);

‎includes/rest/class-inbox.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public static function register_routes() {
4545
'methods' => WP_REST_Server::CREATABLE,
4646
'callback' => array( self::class, 'shared_inbox_post' ),
4747
'args' => self::shared_inbox_post_parameters(),
48-
'permission_callback' => '__return_true',
48+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
4949
),
5050
)
5151
);
@@ -58,13 +58,13 @@ public static function register_routes() {
5858
'methods' => WP_REST_Server::CREATABLE,
5959
'callback' => array( self::class, 'user_inbox_post' ),
6060
'args' => self::user_inbox_post_parameters(),
61-
'permission_callback' => '__return_true',
61+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
6262
),
6363
array(
6464
'methods' => WP_REST_Server::READABLE,
6565
'callback' => array( self::class, 'user_inbox_get' ),
6666
'args' => self::user_inbox_get_parameters(),
67-
'permission_callback' => '__return_true',
67+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
6868
),
6969
)
7070
);

‎includes/rest/class-outbox.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ public static function register_routes() {
4545
'methods' => WP_REST_Server::READABLE,
4646
'callback' => array( self::class, 'user_outbox_get' ),
4747
'args' => self::request_parameters(),
48-
'permission_callback' => '__return_true',
48+
'permission_callback' => array( 'Activitypub\Rest\Server', 'verify_signature' ),
4949
),
5050
)
5151
);

‎includes/rest/class-server.php‎

Lines changed: 15 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ public static function init() {
3535
* Add sever hooks.
3636
*/
3737
public static function add_hooks() {
38-
\add_filter( 'rest_request_before_callbacks', array( self::class, 'validate_activitypub_requests' ), 9, 3 );
39-
\add_filter( 'rest_request_before_callbacks', array( self::class, 'authorize_activitypub_requests' ), 10, 3 );
38+
\add_filter( 'rest_request_before_callbacks', array( self::class, 'validate_requests' ), 9, 3 );
4039
\add_filter( 'rest_request_parameter_order', array( self::class, 'request_parameter_order' ), 10, 2 );
4140
}
4241

@@ -74,39 +73,24 @@ public static function application_actor() {
7473
}
7574

7675
/**
77-
* Callback function to authorize each api requests
76+
* Callback function to authorize an api request.
7877
*
79-
* @see WP_REST_Request
78+
* The function is meant to be used as part of permission callbacks for rest api endpoints.
79+
*
80+
* It verifies the signature of POST, PUT, PATCH, and DELETE requests, as well as GET requests in secure mode.
81+
* You can use the filter 'activitypub_defer_signature_verification' to defer the signature verification.
82+
* HEAD requests are always bypassed.
8083
*
8184
* @see https://www.w3.org/wiki/SocialCG/ActivityPub/Primer/Authentication_Authorization#Authorized_fetch
8285
* @see https://swicg.github.io/activitypub-http-signature/#authorized-fetch
8386
*
84-
* @param WP_REST_Response|\WP_HTTP_Response|WP_Error|mixed $response Result to send to the client.
85-
* Usually a WP_REST_Response or WP_Error.
86-
* @param array $handler Route handler used for the request.
87-
* @param \WP_REST_Request $request Request used to generate the response.
87+
* @param \WP_REST_Request $request The request object.
8888
*
89-
* @return mixed|WP_Error The response, error, or modified response.
89+
* @return bool|\WP_Error True if the request is authorized, WP_Error if not.
9090
*/
91-
public static function authorize_activitypub_requests( $response, $handler, $request ) {
91+
public static function verify_signature( $request ) {
9292
if ( 'HEAD' === $request->get_method() ) {
93-
return $response;
94-
}
95-
96-
if ( \is_wp_error( $response ) ) {
97-
return $response;
98-
}
99-
100-
$route = $request->get_route();
101-
102-
// Check if it is an activitypub request and exclude webfinger and nodeinfo endpoints.
103-
if (
104-
! \str_starts_with( $route, '/' . ACTIVITYPUB_REST_NAMESPACE ) ||
105-
\str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'webfinger' ) ||
106-
\str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'nodeinfo' ) ||
107-
\str_starts_with( $route, '/' . \trailingslashit( ACTIVITYPUB_REST_NAMESPACE ) . 'application' )
108-
) {
109-
return $response;
93+
return true;
11094
}
11195

11296
/**
@@ -123,11 +107,11 @@ public static function authorize_activitypub_requests( $response, $handler, $req
123107
$defer = \apply_filters( 'activitypub_defer_signature_verification', false, $request );
124108

125109
if ( $defer ) {
126-
return $response;
110+
return true;
127111
}
128112

129113
if (
130-
// POST-Requests are always signed.
114+
// POST-Requests always have to be signed.
131115
'GET' !== $request->get_method() ||
132116
// GET-Requests only require a signature in secure mode.
133117
( 'GET' === $request->get_method() && use_authorized_fetch() )
@@ -142,7 +126,7 @@ public static function authorize_activitypub_requests( $response, $handler, $req
142126
}
143127
}
144128

145-
return $response;
129+
return true;
146130
}
147131

148132
/**
@@ -155,7 +139,7 @@ public static function authorize_activitypub_requests( $response, $handler, $req
155139
*
156140
* @return mixed|WP_Error The response, error, or modified response.
157141
*/
158-
public static function validate_activitypub_requests( $response, $handler, $request ) {
142+
public static function validate_requests( $response, $handler, $request ) {
159143
if ( 'HEAD' === $request->get_method() ) {
160144
return $response;
161145
}

‎readme.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ For reasons of data protection, it is not possible to see the followers of other
142142
* Improved: Interactions moderation
143143
* Improved: Compatibility with Akismet
144144
* Improved: Comment type mapping for `Like` and `Announce`
145+
* Improved: Signature verification for API endpoints
145146
* Improved: Changed priority of Attachments, to favor `Image` over `Audio` and `Video`
146147
* Fixed: Empty `url` attributes in the Reply block no longer cause PHP warnings
147148

0 commit comments

Comments
 (0)