diff --git a/.github/changelog/fix-signature-bare-host b/.github/changelog/fix-signature-bare-host new file mode 100644 index 0000000000..3bf4afa0ec --- /dev/null +++ b/.github/changelog/fix-signature-bare-host @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Signed requests from other servers are now checked more strictly, and requests that are not meant for your site are rejected before the sender's key is fetched. diff --git a/includes/functions-url.php b/includes/functions-url.php index 4f926fd4cf..693bb809eb 100644 --- a/includes/functions-url.php +++ b/includes/functions-url.php @@ -167,9 +167,14 @@ function fold_authority( $authority ) { * @return bool True if the authority names one of this site's hosts, false otherwise. */ function is_signature_host( $authority ) { + // The raw value enters the signature base, so it has to be a bare host and port, written exactly so. + if ( get_authority( \sprintf( '//%s', $authority ) ) !== (string) $authority ) { + return false; + } + $authority = fold_authority( $authority ); - // An unparseable value folds to nothing and must not match an unparseable entry. + // A host of only dots or brackets folds to nothing and must not match an unparseable entry. if ( '' === $authority ) { return false; } diff --git a/includes/signature/class-http-signature-draft.php b/includes/signature/class-http-signature-draft.php index c69640e27c..a6656b496b 100644 --- a/includes/signature/class-http-signature-draft.php +++ b/includes/signature/class-http-signature-draft.php @@ -108,16 +108,17 @@ public function verify( array $headers, $body = null ) { return new \WP_Error( 'activitypub_signature', 'No Key ID present.' ); } - $public_key = Remote_Actors::get_public_key( $parsed['keyId'] ); - if ( \is_wp_error( $public_key ) ) { - return $public_key; - } - + // Read before the key is fetched, so a request made out to another host or out of its time window fails without a lookup. $signed_data = $this->get_signed_data( $parsed['headers'], $parsed, $headers ); if ( ! $signed_data ) { return new \WP_Error( 'invalid_signed_data', 'Signed data is invalid or expired.' ); } + $public_key = Remote_Actors::get_public_key( $parsed['keyId'] ); + if ( \is_wp_error( $public_key ) ) { + return $public_key; + } + $algorithm = $this->get_signature_algorithm( $parsed, $public_key ); if ( \is_wp_error( $algorithm ) ) { return $algorithm; diff --git a/tests/phpunit/tests/includes/class-test-functions-url.php b/tests/phpunit/tests/includes/class-test-functions-url.php index 55bad41647..e8c609c968 100644 --- a/tests/phpunit/tests/includes/class-test-functions-url.php +++ b/tests/phpunit/tests/includes/class-test-functions-url.php @@ -210,6 +210,14 @@ public function signature_host_provider() { 'home host as a subdomain' => array( 'evil.%s', false ), 'empty' => array( '', false ), 'a port without a host' => array( ':8080', false ), + 'home host, trailing dot' => array( '%s.', true ), + 'userinfo before home host' => array( 'evil.example@%s', false ), + 'home host with a path' => array( '%s/inbox', false ), + 'home host with a query' => array( '%s?x=1', false ), + 'home host with a fragment' => array( '%s#x', false ), + 'leading whitespace' => array( ' %s', false ), + 'empty port' => array( '%s:', false ), + 'port with a leading zero' => array( '%s:08080', false ), ); } diff --git a/tests/phpunit/tests/includes/class-test-signature.php b/tests/phpunit/tests/includes/class-test-signature.php index 7cb5e72db6..f469abf5d2 100644 --- a/tests/phpunit/tests/includes/class-test-signature.php +++ b/tests/phpunit/tests/includes/class-test-signature.php @@ -2040,14 +2040,24 @@ public function test_verify_http_signature_rejects_a_foreign_host() { \update_option( 'activitypub_rfc9421_signature', '0' ); $mock = $this->mock_remote_key(); + $lookups = 0; + $count = function ( $pre ) use ( &$lookups ) { + ++$lookups; + + return $pre; + }; + \add_filter( 'activitypub_pre_http_get_remote_object', $count, 5 ); + $request = $this->build_replayed_request( 'evil.example', 'evil.example' ); $result = Signature::verify_http_signature( $request ); + \remove_filter( 'activitypub_pre_http_get_remote_object', $count, 5 ); \remove_filter( 'activitypub_pre_http_get_remote_object', $mock ); \delete_option( 'activitypub_rfc9421_signature' ); $this->assertWPError( $result, 'A signature made out to another host must not verify, whatever Host claims.' ); $this->assertSame( 'invalid_signed_data', $result->get_error_code() ); + $this->assertSame( 0, $lookups, 'The key must not be looked up for a request made out to another host.' ); } /**