Boyscouting. Update easydb to 2.7 to eliminate boolean workaround.

paragonie-security · paragonie-security · commit d64f9fff468a · 2018-10-18T13:22:39.000-04:00
diff --git 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
@@ -69,7 +69,10 @@ public static function extendBlakechain(
         $db->beginTransaction();
         /** @var array<string, string> $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<string, string> $sqlResult */
-        $sqlResult = static::$easyDb->row(
-            "SELECT * FROM chronicle_clients WHERE publicid = ?",
-            $clientId
-        );
+        if ($adminOnly) {
+            /** @var array<string, string> $sqlResult */
+            $sqlResult = static::$easyDb->row(
+                "SELECT * FROM chronicle_clients WHERE publicid = ? AND isAdmin",
+                $clientId
+            );
+        } else {
+            /** @var array<string, string> $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
@@ -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
@@ -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,15 +104,19 @@ 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(
             'chronicle_clients',
             [
                 '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
@@ -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<string, string> $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
@@ -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
@@ -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<string, string> $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
@@ -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
@@ -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<string, string> $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
@@ -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
     {

Original file line number	Diff line number	Diff line change
`@@ -37,6 +37,7 @@ public function __construct(array $settings = [])`
`37`	`37`	`*`
`38`	`38`	`* @return self`
`39`	`39`	`*`
	`40`	`+ * @throws Exception\ChainAppendException`
`40`	`41`	`* @throws Exception\FilesystemException`
`41`	`42`	`* @throws Exception\ReplicationSourceNotFound`
`42`	`43`	`* @throws Exception\SecurityViolation`
`@@ -96,9 +97,13 @@ public function doReplication(): self`
`96`	`97`	`}`
`97`	`98`
`98`	`99`	`/**`
	`100`	`+ * Run the Attestation process (if it's scheduled).`
	`101`	`+ *`
`99`	`102`	`* @return self`
`100`	`103`	`*`
	`104`	`+ * @throws Exception\ChainAppendException`
`101`	`105`	`* @throws Exception\FilesystemException`
	`106`	`+ * @throws \SodiumException`
`102`	`107`	`*/`
`103`	`108`	`public function doAttestation(): self`
`104`	`109`	`{`