Merge pull request #7413 from pixelfed/staging

Fix deletes, again
pull/7414/head^2
dansup 2 days ago committed by GitHub
commit a02fc2e19d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -191,7 +191,7 @@ class FederationController extends Controller
if (isset($obj['object']) && isset($obj['object']['type']) && isset($obj['object']['id'])) {
if ($obj['object']['type'] === 'Person') {
if (Profile::whereRemoteUrl($obj['object']['id'])->exists()) {
dispatch(new DeleteWorker($headers, $payload))->onQueue('inbox');
dispatch(new DeleteWorker($headers, $payload, $request->getPathInfo()))->onQueue('inbox');
return;
}
@ -199,14 +199,14 @@ class FederationController extends Controller
if ($obj['object']['type'] === 'Tombstone') {
if ($this->isKnownTombstone($obj['object']['id'])) {
dispatch(new DeleteWorker($headers, $payload))->onQueue('delete');
dispatch(new DeleteWorker($headers, $payload, $request->getPathInfo()))->onQueue('delete');
return;
}
}
if ($obj['object']['type'] === 'Story') {
dispatch(new DeleteWorker($headers, $payload))->onQueue('story');
dispatch(new DeleteWorker($headers, $payload, $request->getPathInfo()))->onQueue('story');
return;
}
@ -269,7 +269,7 @@ class FederationController extends Controller
}
if ($obj['object']['type'] === 'Tombstone') {
if (Status::whereObjectUrl($obj['object']['id'])->exists()) {
if ($this->isKnownTombstone($obj['object']['id'])) {
dispatch(new DeleteWorker($headers, $payload))->onQueue('delete');
return;

@ -21,6 +21,8 @@ class DeleteWorker implements ShouldQueue
protected $payload;
protected $inboxPath = '/f/inbox';
public $timeout = 300;
public $tries = 1;
@ -32,10 +34,13 @@ class DeleteWorker implements ShouldQueue
*
* @return void
*/
public function __construct($headers, $payload)
public function __construct($headers, $payload, ?string $inboxPath = null)
{
$this->headers = $headers;
$this->payload = $payload;
if ($inboxPath) {
$this->inboxPath = $inboxPath;
}
}
/**
@ -77,14 +82,15 @@ class DeleteWorker implements ShouldQueue
return;
}
if ($payload['type'] === 'Delete' &&
if (
$payload['type'] === 'Delete' &&
((is_string($payload['object']) &&
$payload['object'] === $payload['actor']) ||
(is_array($payload['object']) &&
isset($payload['object']['id'], $payload['object']['type']) &&
$payload['object']['type'] === 'Person' &&
$payload['actor'] === $payload['object']['id']
))
(is_array($payload['object']) &&
isset($payload['object']['id'], $payload['object']['type']) &&
$payload['object']['type'] === 'Person' &&
$payload['actor'] === $payload['object']['id']
))
) {
$actor = $payload['actor'];
if ($this->verifySignature($headers, $payload) == true) {
@ -132,8 +138,9 @@ class DeleteWorker implements ShouldQueue
if (! $date) {
return false;
}
if (! now()->parse($date)->gt(now()->subDays(1)) ||
! now()->parse($date)->lt(now()->addDays(1))
if (
! now()->parse($date)->gt(now()->subDays(1)) ||
! now()->parse($date)->lt(now()->addDays(1))
) {
return false;
}
@ -148,7 +155,8 @@ class DeleteWorker implements ShouldQueue
$keyDomain = parse_url($keyId, PHP_URL_HOST);
$idDomain = parse_url($id, PHP_URL_HOST);
$actorDomain = parse_url($bodyDecoded['actor'] ?? '', PHP_URL_HOST);
if (isset($bodyDecoded['object'])
if (
isset($bodyDecoded['object'])
&& is_array($bodyDecoded['object'])
&& isset($bodyDecoded['object']['attributedTo'])
) {
@ -189,8 +197,7 @@ class DeleteWorker implements ShouldQueue
if (! $pkey) {
return false;
}
$inboxPath = '/f/inbox';
[$verified, $headers] = HttpSignature::verify($pkey, $signatureData, $headers, $inboxPath, $body);
[$verified, $headers] = HttpSignature::verify($pkey, $signatureData, $headers, $this->inboxPath, $body);
if ($verified == 1) {
return true;
}

@ -1,5 +1,6 @@
<?php
use App\Federation\Handlers\DirectMessageHandler;
use App\Jobs\Federation\DeliverDirectMessageActivity;
use App\Models\DirectMessage;
use App\Models\DmConversation;
@ -25,6 +26,9 @@ beforeEach(function () {
Redis::spy();
Queue::fake();
Http::fake();
// Ids minted in the same millisecond only sort by creation order when the
// worker bits are fixed. Left unset they are random for every id.
config(['snowflake.datacenter_id' => 1, 'snowflake.worker_id' => 1]);
});
@ -146,6 +150,35 @@ describe('dm:backfill-conversations', function () {
});
});
describe('remote deletes of converted messages', function () {
it('hands a converted message back to the status path so the legacy status goes too', function () {
$bob = dmProfile(dmLocalUser());
$carol = dmRemoteProfile('carol');
dmLegacy($carol, $bob, 'old one', [], ['uri' => $carol->remote_url.'/statuses/9']);
$this->artisan('dm:backfill-conversations', ['--force' => true])->assertSuccessful();
$handler = app(DirectMessageHandler::class);
expect($handler->handleDelete($carol, $carol->remote_url.'/statuses/9'))->toBeFalse()
->and(DmMessage::count())->toBe(0);
});
it('fully handles a message that never was a status', function () {
$bob = dmProfile(dmLocalUser());
$carol = dmRemoteProfile('carol');
$service = app(DirectMessageService::class);
$service->storeMessage($service->findOrCreateDm($carol, $bob), $carol, [
'body' => 'new one',
'ap_object_uri' => $carol->remote_url.'/statuses/10',
]);
expect(app(DirectMessageHandler::class)->handleDelete($carol, $carol->remote_url.'/statuses/10'))->toBeTrue()
->and(DmMessage::count())->toBe(0);
});
});
describe('cleanup', function () {
it('removes the message when the status behind it is deleted', function () {
$alice = dmProfile(dmLocalUser());

@ -2,6 +2,7 @@
use App\Federation\Handlers\DirectMessageHandler;
use App\Federation\Validators\DirectMessageValidator;
use App\Jobs\InboxPipeline\DeleteWorker;
use App\Jobs\MediaPipeline\MediaDeletePipeline;
use App\Models\DmConversation;
use App\Models\DmConversationParticipant;
@ -40,11 +41,13 @@ beforeEach(function () {
Queue::fake();
Http::fake();
// Ids minted in the same millisecond only sort by creation order when the
// worker bits are fixed. Left unset they are random for every id.
config(['snowflake.datacenter_id' => 1, 'snowflake.worker_id' => 1]);
config([
'instance.enable_cc' => false,
'federation.activitypub.enabled' => true,
'snowflake.datacenter_id' => 1,
'snowflake.worker_id' => 1,
]);
});
@ -467,10 +470,16 @@ describe('groups', function () {
describe('conversation identity', function () {
it('hashes the same people to the same conversation whatever order they come in', function () {
// Snowflake ids minted in the same millisecond differ only in their
// last bits, which a float comparison cannot see
$a = 1007571621538590723;
$b = $a + 1;
$c = $a + 2;
expect(DmConversation::dmHash($a, $b))->toBe(DmConversation::dmHash($b, $a))->and(DmConversation::participantsHash([$a, $b, $c]))->toBe(DmConversation::participantsHash([$c, $a, $b]))->and(DmConversation::participantsHash([$a, $b, $c]))->toBe(DmConversation::participantsHash([(string) $b, $c, $a, $a]))->and(DmConversation::dmHash($a, $b))->not->toBe(DmConversation::dmHash($a, $c));
expect(DmConversation::dmHash($a, $b))->toBe(DmConversation::dmHash($b, $a))
->and(DmConversation::participantsHash([$a, $b, $c]))->toBe(DmConversation::participantsHash([$c, $a, $b]))
->and(DmConversation::participantsHash([$a, $b, $c]))->toBe(DmConversation::participantsHash([(string) $b, $c, $a, $a]))
->and(DmConversation::dmHash($a, $b))->not->toBe(DmConversation::dmHash($a, $c));
});
it('keeps one conversation when two participants have neighbouring ids', function () {
@ -621,6 +630,43 @@ describe('deletes', function () {
Queue::assertPushed(MediaDeletePipeline::class, 1);
});
it('lets a delete for a direct message through the inbox endpoints', function (string $endpoint) {
$bobUser = dmLocalUser();
$bob = dmProfile($bobUser);
$alice = dmRemoteProfile();
dmSeedHosts();
dmDeliver($alice, dmNote($alice, '1', [$bob]));
// What Loops and Mastodon send
$delete = [
'@context' => 'https://www.w3.org/ns/activitystreams',
'id' => $alice->remote_url.'/statuses/1#delete',
'type' => 'Delete',
'actor' => $alice->remote_url,
'to' => [$bob->permalink()],
'object' => ['id' => $alice->remote_url.'/statuses/1', 'type' => 'Tombstone'],
];
$this->postJson(str_replace('{username}', $bobUser->username, $endpoint), $delete)->assertOk();
Queue::assertPushed(DeleteWorker::class, 1);
})->with(['/f/inbox', '/users/{username}/inbox']);
it('still drops a delete for something this server has never seen', function () {
$alice = dmRemoteProfile();
dmSeedHosts();
$this->postJson('/f/inbox', [
'id' => $alice->remote_url.'/statuses/404#delete',
'type' => 'Delete',
'actor' => $alice->remote_url,
'object' => ['id' => $alice->remote_url.'/statuses/404', 'type' => 'Tombstone'],
])->assertOk();
Queue::assertNotPushed(DeleteWorker::class);
});
it('ignores a delete from someone who did not write the message', function () {
$bob = dmProfile(dmLocalUser());
$alice = dmRemoteProfile();

@ -0,0 +1,145 @@
<?php
use App\Jobs\InboxPipeline\ActivityHandler;
use App\Jobs\InboxPipeline\DeleteWorker;
use App\Models\DmMessage;
use App\Models\Profile;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Queue;
use Illuminate\Support\Facades\Redis;
require_once __DIR__.'/helpers.php';
uses(LazilyRefreshDatabase::class);
/*
|--------------------------------------------------------------------------
| A signed Delete, from the HTTP request to the message being gone
|--------------------------------------------------------------------------
|
| The other inbound tests hand an activity straight to the Inbox. A Delete
| has two more places to die before it gets there: the inbox endpoint only
| queues deletes for things this server has, and the DeleteWorker verifies
| the HTTP signature, which covers the path the request was sent to. Servers
| deliver direct messages to the recipient's own inbox (Loops always does),
| so that is the path that has to verify, not only the shared inbox.
|
*/
beforeEach(function () {
Redis::spy();
Queue::fake();
Http::fake();
config([
'snowflake.datacenter_id' => 1,
'snowflake.worker_id' => 1,
'instance.enable_cc' => false,
'federation.activitypub.enabled' => true,
]);
});
function dmSignedPost($test, Profile $actor, $privateKey, string $path, array $activity)
{
$body = json_encode($activity, JSON_UNESCAPED_SLASHES);
$host = parse_url(config('app.url'), PHP_URL_HOST);
$date = now()->toRfc7231String();
$digest = 'SHA-256='.base64_encode(hash('sha256', $body, true));
$signingString = implode("\n", [
'(request-target): post '.$path,
'host: '.$host,
'date: '.$date,
'digest: '.$digest,
]);
openssl_sign($signingString, $signature, $privateKey, OPENSSL_ALGO_SHA256);
return $test->call('POST', $path, [], [], [], [
'HTTP_HOST' => $host,
'HTTP_DATE' => $date,
'HTTP_DIGEST' => $digest,
'HTTP_SIGNATURE' => 'keyId="'.$actor->key_id.'",algorithm="rsa-sha256",headers="(request-target) host date digest",signature="'.base64_encode($signature).'"',
'CONTENT_TYPE' => 'application/activity+json',
'HTTP_ACCEPT' => 'application/activity+json',
], $body);
}
it('deletes a direct message from a signed Delete', function (string $endpoint) {
$key = openssl_pkey_new(['private_key_bits' => 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA]);
$bobUser = dmLocalUser();
$bob = dmProfile($bobUser);
$alice = dmRemoteProfile();
$alice->public_key = openssl_pkey_get_details($key)['key'];
$alice->save();
dmSeedHosts();
dmDeliver($alice, dmNote($alice, '1', [$bob]));
expect(DmMessage::count())->toBe(1);
$response = dmSignedPost($this, $alice, $key, str_replace('{username}', $bobUser->username, $endpoint), [
'@context' => 'https://www.w3.org/ns/activitystreams',
'id' => $alice->remote_url.'/statuses/1#delete',
'type' => 'Delete',
'actor' => $alice->remote_url,
'to' => [$bob->permalink()],
'object' => ['id' => $alice->remote_url.'/statuses/1', 'type' => 'Tombstone'],
]);
$response->assertOk();
// The endpoint queued it
$worker = Queue::pushed(DeleteWorker::class)->first();
expect($worker)->not->toBeNull();
// The signature verified against the path it was delivered to
$worker->handle();
$handler = Queue::pushed(ActivityHandler::class)->first();
expect($handler)->not->toBeNull();
// And the inbox removed the message
$handler->handle();
expect(DmMessage::count())->toBe(0);
})->with(['/f/inbox', '/users/{username}/inbox']);
it('rejects a Delete whose signature was made for a different inbox', function () {
$key = openssl_pkey_new(['private_key_bits' => 2048, 'private_key_type' => OPENSSL_KEYTYPE_RSA]);
$bobUser = dmLocalUser();
$bob = dmProfile($bobUser);
$alice = dmRemoteProfile();
$alice->public_key = openssl_pkey_get_details($key)['key'];
$alice->save();
dmSeedHosts();
dmDeliver($alice, dmNote($alice, '1', [$bob]));
$activity = [
'id' => $alice->remote_url.'/statuses/1#delete',
'type' => 'Delete',
'actor' => $alice->remote_url,
'object' => ['id' => $alice->remote_url.'/statuses/1', 'type' => 'Tombstone'],
];
// Signed for the shared inbox, replayed at a personal one
$body = json_encode($activity, JSON_UNESCAPED_SLASHES);
$host = parse_url(config('app.url'), PHP_URL_HOST);
$date = now()->toRfc7231String();
$digest = 'SHA-256='.base64_encode(hash('sha256', $body, true));
openssl_sign("(request-target): post /f/inbox\nhost: {$host}\ndate: {$date}\ndigest: {$digest}", $signature, $key, OPENSSL_ALGO_SHA256);
$this->call('POST', '/users/'.$bobUser->username.'/inbox', [], [], [], [
'HTTP_HOST' => $host,
'HTTP_DATE' => $date,
'HTTP_DIGEST' => $digest,
'HTTP_SIGNATURE' => 'keyId="'.$alice->key_id.'",algorithm="rsa-sha256",headers="(request-target) host date digest",signature="'.base64_encode($signature).'"',
'CONTENT_TYPE' => 'application/activity+json',
], $body)->assertOk();
Queue::pushed(DeleteWorker::class)->first()->handle();
Queue::assertNotPushed(ActivityHandler::class);
expect(DmMessage::count())->toBe(1);
});

@ -43,18 +43,6 @@ if (! function_exists('dmLocalUser')) {
]);
}
/**
* Give $b the id right after $a, the way two profiles created in the same
* millisecond end up. Ids that close are equal once compared as floats,
* so anything that orders or matches ids has to cope with it.
*/
function dmNeighbour(Profile $a, Profile $b): Profile
{
DB::table('profiles')->where('id', $b->id)->update(['id' => $a->id + 1]);
return Profile::findOrFail($a->id + 1);
}
/**
* Seed the DNS and banned-domain caches so URL validation passes without
* a network lookup. Call after factories, the lazy refresh can flush the
@ -71,6 +59,18 @@ if (! function_exists('dmLocalUser')) {
Cache::put('instances:banned:domains', [], 1209600);
}
/**
* Give $b the id right after $a, the way two profiles created in the same
* millisecond end up. Ids that close are equal once compared as floats,
* so anything that orders or matches ids has to cope with it.
*/
function dmNeighbour(Profile $a, Profile $b): Profile
{
DB::table('profiles')->where('id', $b->id)->update(['id' => $a->id + 1]);
return Profile::findOrFail($a->id + 1);
}
function dmFollow(Profile $follower, Profile $target): void
{
Follower::create([

Loading…
Cancel
Save