Skip to content

Commit 3287d7f

Browse files
soatoksoatok
authored
Enforce Revoke constraints demanded by the Specification (#66)
1 parent cf6f942 commit 3287d7f

5 files changed

Lines changed: 897 additions & 48 deletions

File tree

‎docs/reference/classes/tables.md‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ Returns `int`
350350

351351
**Throws:** `DependencyException`, `SodiumException`
352352

353-
#### [`insertLeaf`](../../../src/Tables/MerkleState.php#L229-L285)
353+
#### [`insertLeaf`](../../../src/Tables/MerkleState.php#L229-L293)
354354

355355
**API** · Returns `bool`
356356

@@ -364,23 +364,23 @@ Insert leaf with retry logic for deadlocks
364364

365365
**Throws:** `ConcurrentException`, `CryptoException`, `DependencyException`, `NotImplementedException`, `RandomException`, `SodiumException`
366366

367-
#### [`getLeafByRoot`](../../../src/Tables/MerkleState.php#L290-L306)
367+
#### [`getLeafByRoot`](../../../src/Tables/MerkleState.php#L312-L328)
368368

369369
**API** · Returns `?FediE2EE\PKDServer\Tables\Records\MerkleLeaf`
370370

371371
**Parameters:**
372372

373373
- `$root`: `string`
374374

375-
#### [`getLeafByID`](../../../src/Tables/MerkleState.php#L311-L327)
375+
#### [`getLeafByID`](../../../src/Tables/MerkleState.php#L333-L349)
376376

377377
**API** · Returns `?FediE2EE\PKDServer\Tables\Records\MerkleLeaf`
378378

379379
**Parameters:**
380380

381381
- `$primaryKey`: `int`
382382

383-
#### [`getHashesSince`](../../../src/Tables/MerkleState.php#L367-L409)
383+
#### [`getHashesSince`](../../../src/Tables/MerkleState.php#L389-L431)
384384

385385
**API** · Returns `array`
386386

@@ -595,7 +595,7 @@ Returns `bool`
595595

596596
**Throws:** `ConcurrentException`, `CryptoException`, `DependencyException`, `NotImplementedException`, `ProtocolException`, `RandomException`, `SodiumException`, `TableException`
597597

598-
#### [`burnDown`](../../../src/Tables/PublicKeys.php#L817-L826)
598+
#### [`burnDown`](../../../src/Tables/PublicKeys.php#L853-L862)
599599

600600
Returns `bool`
601601

@@ -606,7 +606,7 @@ Returns `bool`
606606

607607
**Throws:** `ConcurrentException`, `CryptoException`, `DependencyException`, `NotImplementedException`, `ProtocolException`, `RandomException`, `SodiumException`, `TableException`
608608

609-
#### [`fireproof`](../../../src/Tables/PublicKeys.php#L916-L924)
609+
#### [`fireproof`](../../../src/Tables/PublicKeys.php#L952-L960)
610610

611611
Returns `bool`
612612

@@ -617,7 +617,7 @@ Returns `bool`
617617

618618
**Throws:** `ConcurrentException`, `CryptoException`, `DependencyException`, `NotImplementedException`, `ProtocolException`, `RandomException`, `SodiumException`, `TableException`
619619

620-
#### [`undoFireproof`](../../../src/Tables/PublicKeys.php#L999-L1007)
620+
#### [`undoFireproof`](../../../src/Tables/PublicKeys.php#L1035-L1043)
621621

622622
Returns `bool`
623623

@@ -628,7 +628,7 @@ Returns `bool`
628628

629629
**Throws:** `ConcurrentException`, `CryptoException`, `DependencyException`, `NotImplementedException`, `ProtocolException`, `RandomException`, `SodiumException`, `TableException`
630630

631-
#### [`checkpoint`](../../../src/Tables/PublicKeys.php#L1082-L1090)
631+
#### [`checkpoint`](../../../src/Tables/PublicKeys.php#L1118-L1126)
632632

633633
Returns `bool`
634634

‎src/Tables/MerkleState.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,14 @@ public function insertLeaf(MerkleLeaf $leaf, callable $inTransaction, int $maxRe
249249
if ($this->db->inTransaction()) {
250250
$this->db->rollBack();
251251
}
252+
253+
// Unique constraint violation = duplicate message replay
254+
if ($this->isUniqueConstraintViolation($e)) {
255+
throw new ProtocolException(
256+
'Message has already been processed'
257+
);
258+
}
259+
252260
$shouldRetry = false;
253261

254262
// Check for deadlock/lock timeout errors by driver
@@ -284,6 +292,20 @@ public function insertLeaf(MerkleLeaf $leaf, callable $inTransaction, int $maxRe
284292
return false;
285293
}
286294

295+
private function isUniqueConstraintViolation(PDOException $e): bool
296+
{
297+
return match ($this->db->getDriver()) {
298+
'pgsql' => $e->getCode() === '23505',
299+
'mysql' => $e->getCode() === '23000'
300+
&& str_contains($e->getMessage(), 'Duplicate entry'),
301+
'sqlite' => str_contains(
302+
$e->getMessage(),
303+
'UNIQUE constraint failed'
304+
),
305+
default => false,
306+
};
307+
}
308+
287309
/**
288310
* @api
289311
*/

‎src/Tables/PublicKeys.php‎

Lines changed: 52 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -641,26 +641,62 @@ protected function revokeKeyCallback(MerkleLeaf $leaf, Payload $payload, string
641641

642642
// Explicit check that the outer actor (from ActivityPub) matches the protocol message
643643
$this->explicitOuterActorCheck($outerActor, $actionData['actor']);
644+
645+
//= https://raw.githubusercontent.com/fedi-e2ee/public-key-directory-specification/refs/heads/main/Specification.md#revokekey
646+
//# Attempting to issue a `RevokeKey` **MUST** fail unless there is another public key associated with this Actor.
647+
$allValidKeys = $this->getPublicKeysFor($actionData['actor']);
648+
if (count($allValidKeys) < 2) {
649+
throw new ProtocolException(
650+
'Cannot revoke the last remaining key'
651+
);
652+
}
653+
654+
// Identify the target key to revoke by matching public key material
655+
$targetPublicKey = PublicKey::fromString($actionData['public-key']);
656+
$targetRow = null;
657+
foreach ($allValidKeys as $row) {
658+
if (hash_equals(
659+
$targetPublicKey->toString(),
660+
$row['public-key']->toString()
661+
)) {
662+
$targetRow = $row;
663+
break;
664+
}
665+
}
666+
if ($targetRow === null) {
667+
throw new ProtocolException(
668+
'Target key not found or already revoked'
669+
);
670+
}
671+
672+
// Verify signature by a DIFFERENT valid key (not the target)
644673
$sm = Bundle::fromJson($rawJson)->toSignedMessage();
645-
$candidatePublicKeys = $this->getPublicKeysFor(
646-
actorName: $actionData['actor'],
647-
keyId: $actionData['key-id'] ?? ''
648-
);
649-
foreach ($candidatePublicKeys as $row) {
674+
$signatureIsValid = false;
675+
foreach ($allValidKeys as $row) {
676+
if ($row['actorpublickeyid'] === $targetRow['actorpublickeyid']) {
677+
continue;
678+
}
650679
if ($sm->verify($row['public-key'])) {
651-
// Valid key found!
652-
$this->db->update(
653-
'pkd_actors_publickeys',
654-
[
655-
'trusted' => false,
656-
'revokeleaf' => $leaf->getPrimaryKey(),
657-
],
658-
['actorpublickeyid' => $row['actorpublickeyid']]
659-
);
660-
return $this->getRecord($row['actorpublickeyid']);
680+
$signatureIsValid = true;
681+
break;
661682
}
662683
}
663-
throw new ProtocolException('Signature is not valid');
684+
if (!$signatureIsValid) {
685+
throw new ProtocolException(
686+
'RevokeKey must be signed by a different valid key'
687+
);
688+
}
689+
690+
// Revoke the target key
691+
$this->db->update(
692+
'pkd_actors_publickeys',
693+
[
694+
'trusted' => false,
695+
'revokeleaf' => $leaf->getPrimaryKey(),
696+
],
697+
['actorpublickeyid' => $targetRow['actorpublickeyid']]
698+
);
699+
return $this->getRecord($targetRow['actorpublickeyid']);
664700
}
665701

666702
//= https://raw.githubusercontent.com/fedi-e2ee/public-key-directory-specification/refs/heads/main/Specification.md#revokekeythirdparty

‎tests/Integration/ActorLifecycleTest.php‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,10 @@
1212
};
1313
use FediE2EE\PKD\Crypto\Protocol\Actions\{
1414
AddKey,
15-
RevokeKey
15+
RevokeKey,
16+
RevokeKeyThirdParty
1617
};
18+
use FediE2EE\PKD\Crypto\Revocation;
1719
use FediE2EE\PKD\Crypto\Exceptions\{
1820
BundleException,
1921
CryptoException,
@@ -373,31 +375,45 @@ public function testRevokeKeyWithPeerRewrapping(): void
373375
]);
374376
$peerId = (int) $db->lastInsertId();
375377

376-
// 2. Setup actor and add key
378+
// 2. Setup actor and add two keys
377379
[, $canonical] = $this->makeDummyActor('example.net');
378-
$keypair = SecretKey::generate();
380+
$keypair1 = SecretKey::generate();
381+
$keypair2 = SecretKey::generate();
379382

380383
$this->clearOldTransaction($config);
381384
$protocol = new Protocol($config);
382-
$protocol->setWebFinger($this->createWebFingerMock($config, $canonical, 2));
385+
$protocol->setWebFinger($this->createWebFingerMock($config, $canonical, 4));
383386

384-
$this->addKeyForActor($canonical, $keypair, $protocol, $config);
387+
// Add key1 (self-signed)
388+
$this->addKeyForActor($canonical, $keypair1, $protocol, $config);
385389
$this->assertNotInTransaction();
386390

387-
// Clear rewrapped keys from addKey to check revokeKey's work
388-
$db->safeQuery("DELETE FROM pkd_merkle_leaf_rewrapped_keys");
389-
390-
// 3. Revoke key
391+
// Add key2 (signed by key1)
391392
/** @var MerkleState $merkleState */
392393
$merkleState = $this->table('MerkleState');
393-
$latestRoot = $merkleState->getLatestRoot();
394394
$serverHpke = $config->getHPKE();
395395
$handler = new Handler();
396-
$revokeKey = new \FediE2EE\PKD\Crypto\Protocol\Actions\RevokeKey($canonical, $keypair->getPublicKey());
396+
397+
$latestRoot = $merkleState->getLatestRoot();
398+
$addKey2 = new AddKey($canonical, $keypair2->getPublicKey());
399+
$akm2 = new AttributeKeyMap()
400+
->addKey('actor', SymmetricKey::generate())
401+
->addKey('public-key', SymmetricKey::generate());
402+
$bundle2 = $handler->handle($addKey2->encrypt($akm2), $keypair1, $akm2, $latestRoot);
403+
$encrypted2 = $handler->hpkeEncrypt($bundle2, $serverHpke->encapsKey, $serverHpke->cs);
404+
$protocol->addKey($encrypted2, $canonical);
405+
$this->assertNotInTransaction();
406+
407+
// Clear rewrapped keys from addKey to check revokeKey's work
408+
$db->safeQuery("DELETE FROM pkd_merkle_leaf_rewrapped_keys");
409+
410+
// 3. Revoke key2 (signed by key1)
411+
$latestRoot = $merkleState->getLatestRoot();
412+
$revokeKey = new RevokeKey($canonical, $keypair2->getPublicKey());
397413
$akm = new AttributeKeyMap();
398414
$akm->addKey('actor', SymmetricKey::generate());
399415
$akm->addKey('public-key', SymmetricKey::generate());
400-
$bundle = $handler->handle($revokeKey->encrypt($akm), $keypair, $akm, $latestRoot);
416+
$bundle = $handler->handle($revokeKey->encrypt($akm), $keypair1, $akm, $latestRoot);
401417
$encrypted = $handler->hpkeEncrypt($bundle, $serverHpke->encapsKey, $serverHpke->cs);
402418
$protocol->revokeKey($encrypted, $canonical);
403419

@@ -663,21 +679,19 @@ public function testAddAndRevoke(): void
663679
$this->assertCount(1, $body['public-keys']);
664680
$this->assertSame($keypair1->getPublicKey()->toString(), $body['public-keys'][0]['public-key']);
665681

666-
// 2. RevokeKey (signed by key 1)
682+
// 2. RevokeKeyThirdParty (uses revocation token to revoke last key)
667683
$latestRoot2 = $merkleState->getLatestRoot();
668-
$revokeKey = new RevokeKey($canonical, $keypair1->getPublicKey());
669-
$akm2 = new AttributeKeyMap()
670-
->addKey('actor', SymmetricKey::generate())
671-
->addKey('public-key', SymmetricKey::generate());
672-
$encryptedMsg2 = $revokeKey->encrypt($akm2);
673-
$bundle2 = $handler->handle($encryptedMsg2, $keypair1, $akm2, $latestRoot2);
674-
$encryptedForServer2 = $handler->hpkeEncrypt(
675-
$bundle2,
676-
$serverHpke->encapsKey,
677-
$serverHpke->cs,
684+
$revocation = new Revocation();
685+
$token = $revocation->revokeThirdParty($keypair1);
686+
$revokeAction = new RevokeKeyThirdParty($token);
687+
$bundle2 = $handler->handle(
688+
$revokeAction,
689+
$keypair1,
690+
new AttributeKeyMap(),
691+
$latestRoot2
678692
);
679693
$this->assertNotInTransaction();
680-
$protocol->revokeKey($encryptedForServer2, $canonical);
694+
$protocol->revokeKeyThirdParty($bundle2->toString());
681695
$this->assertNotInTransaction();
682696

683697
// Verify with HTTP request

0 commit comments

Comments
 (0)