From d64f9fff468af081ad594a4e1be07a28b89c6e0a Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Thu, 18 Oct 2018 13:22:39 -0400 Subject: [PATCH] Boyscouting. Update easydb to 2.7 to eliminate boolean workaround. --- composer.json | 2 +- src/Chronicle/Chronicle.php | 41 +++++++++---------- src/Chronicle/Handlers/Register.php | 13 ++++-- src/Chronicle/Handlers/Revoke.php | 14 +++++-- .../Middleware/CheckAdminSignature.php | 13 +----- .../Middleware/CheckClientSignature.php | 26 ++++++++++-- src/Chronicle/Process/Attest.php | 17 +++++++- src/Chronicle/Process/CrossSign.php | 3 ++ src/Chronicle/Process/Replicate.php | 26 ++++++++++-- src/Chronicle/Scheduled.php | 5 +++ 10 files changed, 110 insertions(+), 50 deletions(-) diff --git a/composer.json b/composer.json index 2e2947b..9c5220d 100644 --- a/composer.json +++ b/composer.json @@ -18,7 +18,7 @@ "ext-pdo": "*", "guzzlehttp/guzzle": "^6", "paragonie/blakechain": "^1", - "paragonie/easydb": "^2", + "paragonie/easydb": "^2.7", "paragonie/sapient": "^1", "paragonie/slim-sapient": "^1", "paragonie/sodium_compat": "^1.7", diff --git a/src/Chronicle/Chronicle.php b/src/Chronicle/Chronicle.php index f92f8a6..7d9f05e 100644 --- a/src/Chronicle/Chronicle.php +++ b/src/Chronicle/Chronicle.php @@ -69,7 +69,10 @@ public static function extendBlakechain( $db->beginTransaction(); /** @var array $lasthash */ $lasthash = $db->row( - 'SELECT currhash, hashstate FROM chronicle_chain ORDER BY id DESC LIMIT 1' + 'SELECT currhash, hashstate + FROM chronicle_chain + ORDER BY id DESC + LIMIT 1' ); // Instantiate the Blakechain. @@ -152,36 +155,30 @@ public static function errorResponse( ); } - /** - * If we're using SQLite, we need a 1 or a 0. - * Otherwise, TRUE/FALSE is fine. - * - * @param bool $value - * @return bool|int - */ - public static function getDatabaseBoolean(bool $value) - { - if (self::$easyDb->getDriver() === 'sqlite') { - return $value ? 1 : 0; - } - return !empty($value); - } - /** * Given a clients Public ID, retrieve their Ed25519 public key. * * @param string $clientId + * @param bool $adminOnly * @return SigningPublicKey * * @throws ClientNotFound */ - public static function getClientsPublicKey(string $clientId): SigningPublicKey + public static function getClientsPublicKey(string $clientId, bool $adminOnly = false): SigningPublicKey { - /** @var array $sqlResult */ - $sqlResult = static::$easyDb->row( - "SELECT * FROM chronicle_clients WHERE publicid = ?", - $clientId - ); + if ($adminOnly) { + /** @var array $sqlResult */ + $sqlResult = static::$easyDb->row( + "SELECT * FROM chronicle_clients WHERE publicid = ? AND isAdmin", + $clientId + ); + } else { + /** @var array $sqlResult */ + $sqlResult = static::$easyDb->row( + "SELECT * FROM chronicle_clients WHERE publicid = ?", + $clientId + ); + } if (empty($sqlResult)) { throw new ClientNotFound('Client not found'); } diff --git a/src/Chronicle/Handlers/Register.php b/src/Chronicle/Handlers/Register.php index c195ff2..ad0d4b3 100644 --- a/src/Chronicle/Handlers/Register.php +++ b/src/Chronicle/Handlers/Register.php @@ -39,8 +39,9 @@ class Register implements HandlerInterface * @throws FilesystemException * @throws GuzzleException * @throws InvalidMessageException - * @throws TargetNotFound + * @throws SecurityViolation * @throws \SodiumException + * @throws TargetNotFound */ public function __invoke( RequestInterface $request, @@ -161,14 +162,18 @@ public function __invoke( * @return string * * @throws \PDOException - * @throws \Exception + * @throws SecurityViolation */ protected function createClient(array $post): string { $db = Chronicle::getDatabase(); $now = (new \DateTime())->format(\DateTime::ATOM); do { - $clientId = Base64UrlSafe::encode(\random_bytes(24)); + try { + $clientId = Base64UrlSafe::encode(\random_bytes(24)); + } catch (\Throwable $ex) { + throw new SecurityViolation('CSPRNG is broken'); + } } while ($db->exists('SELECT count(id) FROM chronicle_clients WHERE publicid = ?', $clientId)); $db->beginTransaction(); @@ -178,7 +183,7 @@ protected function createClient(array $post): string 'publicid' => $clientId, 'publickey' => $post['publickey'], 'comment' => $post['comment'] ?? '', - 'isAdmin' => Chronicle::getDatabaseBoolean(false), + 'isAdmin' => false, 'created' => $now, 'modified' => $now ] diff --git a/src/Chronicle/Handlers/Revoke.php b/src/Chronicle/Handlers/Revoke.php index 0b5ac76..7e707aa 100644 --- a/src/Chronicle/Handlers/Revoke.php +++ b/src/Chronicle/Handlers/Revoke.php @@ -91,7 +91,11 @@ public function __invoke( $post['publickey'] ); if (!$found) { - return Chronicle::errorResponse($response, 'Error: Client not found. It may have already been deleted.', 404); + return Chronicle::errorResponse( + $response, + 'Error: Client not found. It may have already been deleted.', + 404 + ); } /** @var bool $isAdmin */ $isAdmin = $db->cell( @@ -100,7 +104,11 @@ public function __invoke( $post['publickey'] ); if ($isAdmin) { - return Chronicle::errorResponse($response, 'You cannot delete administrators from this API.', 403); + return Chronicle::errorResponse( + $response, + 'You cannot delete administrators from this API.', + 403 + ); } $db->delete( @@ -108,7 +116,7 @@ public function __invoke( [ 'publicid' => $post['clientid'], 'publickey' => $post['publickey'], - 'isAdmin' => Chronicle::getDatabaseBoolean(false) + 'isAdmin' => false ] ); if ($db->commit()) { diff --git a/src/Chronicle/Middleware/CheckAdminSignature.php b/src/Chronicle/Middleware/CheckAdminSignature.php index cd2b498..4fd3e8a 100644 --- a/src/Chronicle/Middleware/CheckAdminSignature.php +++ b/src/Chronicle/Middleware/CheckAdminSignature.php @@ -4,7 +4,6 @@ use ParagonIE\Chronicle\Chronicle; use ParagonIE\Chronicle\Exception\ClientNotFound; -use ParagonIE\ConstantTime\Base64UrlSafe; use ParagonIE\Sapient\CryptographyKeys\SigningPublicKey; /** @@ -25,16 +24,6 @@ class CheckAdminSignature extends CheckClientSignature */ public function getPublicKey(string $clientId): SigningPublicKey { - /** @var array $sqlResult */ - $sqlResult = Chronicle::getDatabase()->row( - "SELECT * FROM chronicle_clients WHERE publicid = ? AND isAdmin", - $clientId - ); - if (empty($sqlResult)) { - throw new ClientNotFound('Client not found or is not an administrator.'); - } - return new SigningPublicKey( - Base64UrlSafe::decode($sqlResult['publickey']) - ); + return Chronicle::getClientsPublicKey($clientId, true); } } diff --git a/src/Chronicle/Middleware/CheckClientSignature.php b/src/Chronicle/Middleware/CheckClientSignature.php index f095682..6fc50ba 100644 --- a/src/Chronicle/Middleware/CheckClientSignature.php +++ b/src/Chronicle/Middleware/CheckClientSignature.php @@ -46,6 +46,20 @@ public function getClientId(RequestInterface $request): string return (string) \array_shift($header); } + /** + * Only selects a valid result if the client has isAdmin set to TRUE. + * + * @param string $clientId + * @return SigningPublicKey + * + * @throws ClientNotFound + */ + public function getPublicKey(string $clientId): SigningPublicKey + { + // The second parameter gets overridden in CheckAdminSignature to TRUE: + return Chronicle::getClientsPublicKey($clientId, false); + } + /** * @param RequestInterface $request * @param ResponseInterface $response @@ -69,15 +83,19 @@ public function __invoke( try { /** @var SigningPublicKey $publicKey */ - $publicKey = Chronicle::getClientsPublicKey($clientId); + $publicKey = $this->getPublicKey($clientId); } catch (ClientNotFound $ex) { - return Chronicle::errorResponse($response, 'Invalid client', 403); + return Chronicle::errorResponse($response, $ex->getMessage(), 403); } try { - $request = Chronicle::getSapient()->verifySignedRequest($request, $publicKey); + $request = Chronicle::getSapient() + ->verifySignedRequest($request, $publicKey); + if ($request instanceof Request) { - $serverPublicKey = Chronicle::getSigningKey()->getPublicKey()->getString(); + $serverPublicKey = Chronicle::getSigningKey() + ->getPublicKey() + ->getString(); if (\hash_equals($serverPublicKey, $publicKey->getString())) { return Chronicle::errorResponse( $response, diff --git a/src/Chronicle/Process/Attest.php b/src/Chronicle/Process/Attest.php index 66f83f8..5c50217 100644 --- a/src/Chronicle/Process/Attest.php +++ b/src/Chronicle/Process/Attest.php @@ -11,6 +11,11 @@ /** * Class Attest + * + * This process publishes the latest hash of each replicated Chronicle + * onto the local instance, to create an immutable record of the replicated + * Chronicles and provide greater resilience against malicious tampering. + * * @package ParagonIE\Chronicle\Process */ class Attest @@ -31,6 +36,8 @@ public function __construct(array $settings = []) } /** + * Do we need to run the attestation process? + * * @return bool * * @throws FilesystemException @@ -97,7 +104,15 @@ public function attestAll(): array foreach (Chronicle::getDatabase()->run('SELECT id, uniqueid FROM chronicle_replication_sources') as $row) { /** @var array $latest */ $latest = Chronicle::getDatabase()->row( - "SELECT currhash, summaryhash FROM chronicle_replication_chain WHERE source = ? ORDER BY id DESC LIMIT 1", + "SELECT + currhash, + summaryhash + FROM + chronicle_replication_chain + WHERE + source = ? + ORDER BY id DESC + LIMIT 1", $row['id'] ); $latest['source'] = $row['uniqueid']; diff --git a/src/Chronicle/Process/CrossSign.php b/src/Chronicle/Process/CrossSign.php index 18108db..e8b5b74 100644 --- a/src/Chronicle/Process/CrossSign.php +++ b/src/Chronicle/Process/CrossSign.php @@ -20,6 +20,9 @@ /** * Class CrossSign + * + * Publish the latest hash onto another remote Chronicle instance. + * * @package ParagonIE\Chronicle\Process */ class CrossSign diff --git a/src/Chronicle/Process/Replicate.php b/src/Chronicle/Process/Replicate.php index b699ca4..66af05a 100644 --- a/src/Chronicle/Process/Replicate.php +++ b/src/Chronicle/Process/Replicate.php @@ -19,6 +19,11 @@ /** * Class Replicate + * + * Maintain a replica (mirror) of another Chronicle instance. + * Unless Attestation is enabled, this doesn't affect the main + * Chronicle; mirroring is separate. + * * @package ParagonIE\Chronicle\Process */ class Replicate @@ -131,7 +136,15 @@ protected function appendToChain(array $entry): bool $db->beginTransaction(); /** @var array $lasthash */ $lasthash = $db->row( - 'SELECT currhash, hashstate FROM chronicle_replication_chain WHERE source = ? ORDER BY id DESC LIMIT 1', + 'SELECT + currhash, + hashstate + FROM + chronicle_replication_chain + WHERE + source = ? + ORDER BY id DESC + LIMIT 1', $this->id ); @@ -157,7 +170,7 @@ protected function appendToChain(array $entry): bool ); if (!$sigMatches) { $db->rollBack(); - throw new SecurityViolation('Invalid Ed25519 signature'); + throw new SecurityViolation('Invalid Ed25519 signature provided by source Chronicle.'); } /* Update the Blakechain */ @@ -202,7 +215,14 @@ protected function getLatestSummaryHash(): string { /** @var string $last */ $last = Chronicle::getDatabase()->cell( - "SELECT summaryhash FROM chronicle_replication_chain WHERE source = ? ORDER BY id DESC LIMIT 1", + "SELECT + summaryhash + FROM + chronicle_replication_chain + WHERE + source = ? + ORDER BY id DESC + LIMIT 1", $this->id ); if (empty($last)) { diff --git a/src/Chronicle/Scheduled.php b/src/Chronicle/Scheduled.php index 3f65eee..e3cb4be 100644 --- a/src/Chronicle/Scheduled.php +++ b/src/Chronicle/Scheduled.php @@ -37,6 +37,7 @@ public function __construct(array $settings = []) * * @return self * + * @throws Exception\ChainAppendException * @throws Exception\FilesystemException * @throws Exception\ReplicationSourceNotFound * @throws Exception\SecurityViolation @@ -96,9 +97,13 @@ public function doReplication(): self } /** + * Run the Attestation process (if it's scheduled). + * * @return self * + * @throws Exception\ChainAppendException * @throws Exception\FilesystemException + * @throws \SodiumException */ public function doAttestation(): self {