diff --git a/app/Jobs/InboxPipeline/DeleteWorker.php b/app/Jobs/InboxPipeline/DeleteWorker.php index bb9c027a6..830eb1254 100644 --- a/app/Jobs/InboxPipeline/DeleteWorker.php +++ b/app/Jobs/InboxPipeline/DeleteWorker.php @@ -185,6 +185,13 @@ class DeleteWorker implements ShouldQueue if (! $actor) { return false; } + // Rebind: the profile resolved by keyId must belong to the keyId host. + // This rejects a poisoned or stale row whose remote_url host differs + // from the request's keyId host, so a planted key_id -> attacker key + // binding cannot authenticate. + if (parse_url($actor->remote_url, PHP_URL_HOST) !== $keyDomain) { + return false; + } $pkey = openssl_pkey_get_public($actor->public_key); if (! $pkey) { return false; diff --git a/app/Jobs/InboxPipeline/InboxValidator.php b/app/Jobs/InboxPipeline/InboxValidator.php index 4b09fd04f..d77520e1e 100644 --- a/app/Jobs/InboxPipeline/InboxValidator.php +++ b/app/Jobs/InboxPipeline/InboxValidator.php @@ -152,6 +152,13 @@ class InboxValidator implements ShouldQueue if (! $actor) { return false; } + // Rebind: the profile resolved by keyId must belong to the keyId host. + // This rejects a poisoned or stale row whose remote_url host differs + // from the request's keyId host, so a planted key_id -> attacker key + // binding cannot authenticate. + if (parse_url($actor->remote_url, PHP_URL_HOST) !== $keyDomain) { + return false; + } $pkey = openssl_pkey_get_public($actor->public_key); if (! $pkey) { return false; diff --git a/app/Jobs/InboxPipeline/InboxWorker.php b/app/Jobs/InboxPipeline/InboxWorker.php index 48c313916..0c17c1512 100644 --- a/app/Jobs/InboxPipeline/InboxWorker.php +++ b/app/Jobs/InboxPipeline/InboxWorker.php @@ -133,6 +133,13 @@ class InboxWorker implements ShouldQueue if (! $actor) { return false; } + // Rebind: the profile resolved by keyId must belong to the keyId host. + // This rejects a poisoned or stale row whose remote_url host differs + // from the request's keyId host, so a planted key_id -> attacker key + // binding cannot authenticate. + if (parse_url($actor->remote_url, PHP_URL_HOST) !== $keyDomain) { + return false; + } $pkey = openssl_pkey_get_public($actor->public_key); if (! $pkey) { return false; diff --git a/app/Util/ActivityPub/Helpers.php b/app/Util/ActivityPub/Helpers.php index da1101cb7..67cf1ec8a 100644 --- a/app/Util/ActivityPub/Helpers.php +++ b/app/Util/ActivityPub/Helpers.php @@ -1500,7 +1500,29 @@ class Helpers $urlDomain = parse_url($url, PHP_URL_HOST); $domain = parse_url($res['id'], PHP_URL_HOST); - return strtolower($urlDomain) === strtolower($domain); + if (strtolower($urlDomain) !== strtolower($domain)) { + return false; + } + + // The actor's key_id (publicKey.id) must live on the same host as the + // actor id. Without this, a remote actor could advertise a publicKey.id + // pointing at a victim's keyId URI, planting a poisoned + // key_id -> attacker-public-key binding in the unique profiles.key_id + // column. This mirrors the same-host check UpdatePersonValidator already + // enforces on the Update pipeline. + if (isset($res['publicKey']['id'])) { + if (! self::validateUrl($res['publicKey']['id'])) { + return false; + } + + $keyDomain = parse_url($res['publicKey']['id'], PHP_URL_HOST); + + if (strtolower($keyDomain) !== strtolower($domain)) { + return false; + } + } + + return true; } /** diff --git a/tests/Feature/Federation/ProfileKeyIdHostValidationTest.php b/tests/Feature/Federation/ProfileKeyIdHostValidationTest.php new file mode 100644 index 000000000..e2d0d47c1 --- /dev/null +++ b/tests/Feature/Federation/ProfileKeyIdHostValidationTest.php @@ -0,0 +1,82 @@ + attacker key binding in the unique profiles.key_id column and +| silently black-holing the victim's federation. +| +*/ + +/** + * validateUrl() (called internally by isValidProfileData) resolves the host to + * a public IP. Pre-seed the DNS cache so these tests stay deterministic and + * network-free while still exercising the host-comparison logic. + */ +function seedResolvableHost(string $host): void +{ + $key = 'helpers:url:public-ips:'.hash('xxh128', $host); + Cache::put($key, ['203.0.113.10'], 3600); +} + +function keyIdActorDoc(array $overrides = []): array +{ + return array_replace_recursive([ + 'id' => 'https://attacker-host.test/users/attacker', + 'inbox' => 'https://attacker-host.test/users/attacker/inbox', + 'outbox' => 'https://attacker-host.test/users/attacker/outbox', + 'preferredUsername' => 'attacker', + 'publicKey' => [ + 'id' => 'https://attacker-host.test/users/attacker#main-key', + 'owner' => 'https://attacker-host.test/users/attacker', + 'publicKeyPem' => "-----BEGIN PUBLIC KEY-----\nattacker\n-----END PUBLIC KEY-----", + ], + ], $overrides); +} + +beforeEach(function () { + seedResolvableHost('attacker-host.test'); + seedResolvableHost('victim-host.test'); +}); + +it('accepts an actor whose publicKey.id host matches its id host', function () { + $res = keyIdActorDoc(); + + expect(Helpers::isValidProfileData($res, $res['id']))->toBeTrue(); +}); + +it('rejects an actor whose publicKey.id host points at another host', function () { + // The plant attempt: attacker actor advertises the victim's keyId URI. + $res = keyIdActorDoc([ + 'publicKey' => [ + 'id' => 'https://victim-host.test/users/alice#main-key', + ], + ]); + + expect(Helpers::isValidProfileData($res, $res['id']))->toBeFalse(); +}); + +it('rejects an actor whose publicKey.id is not a valid url', function () { + $res = keyIdActorDoc([ + 'publicKey' => [ + 'id' => 'not-a-url', + ], + ]); + + expect(Helpers::isValidProfileData($res, $res['id']))->toBeFalse(); +});