From 464847634d6a63c4b9d6ac91fa5cd5617718e0e1 Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Fri, 27 Sep 2019 18:15:20 -0400 Subject: [PATCH 1/2] Only query Chronicle during RemoteFetch. --- composer.json | 2 + src/Fetch.php | 90 ++++++++++++++++++++++++++++++++++------ src/RemoteFetch.php | 1 + test/RemoteFetchTest.php | 8 ++++ 4 files changed, 88 insertions(+), 13 deletions(-) diff --git a/composer.json b/composer.json index bf7d571..7cb3e22 100644 --- a/composer.json +++ b/composer.json @@ -25,6 +25,8 @@ ], "require": { "php": "^5.5|^7", + "ext-curl": "*", + "ext-json": "*", "guzzlehttp/guzzle": "^6", "paragonie/constant_time_encoding": "^1|^2", "paragonie/sodium_compat": "^1.11" diff --git a/src/Fetch.php b/src/Fetch.php index 8ee7eea..09f4630 100644 --- a/src/Fetch.php +++ b/src/Fetch.php @@ -35,6 +35,12 @@ class Fetch */ protected $chroniclePublicKey = ''; + /** + * List of bundles that have just been downloaded (e.g. RemoteFetch) + * @var array $unverified + */ + protected $unverified = []; + /** * Fetch constructor. * @@ -70,10 +76,13 @@ public function getLatestBundle($checkEd25519Signature = null, $checkChronicle = if (\is_null($checkEd25519Signature)) { $checkEd25519Signature = (bool) (static::CHECK_SIGNATURE_BY_DEFAULT && $sodiumCompatIsntSlow); } - if (\is_null($checkChronicle)) { + $conditionalChronicle = \is_null($checkChronicle); + if ($conditionalChronicle) { $checkChronicle = (bool) (static::CHECK_CHRONICLE_BY_DEFAULT && $sodiumCompatIsntSlow); } + /** @var int $bundleIndex */ + $bundleIndex = 0; /** @var Bundle $bundle */ foreach ($this->listBundles('', $this->trustChannel) as $bundle) { if ($bundle->hasCustom()) { @@ -88,14 +97,33 @@ public function getLatestBundle($checkEd25519Signature = null, $checkChronicle = $valid = true; if ($checkEd25519Signature) { $valid = $valid && $validator->checkEd25519Signature($bundle); + if (!$valid) { + $this->markBundleAsBad($bundleIndex, 'Ed25519 signature mismatch'); + } } - if ($checkChronicle) { + if ($conditionalChronicle && $checkChronicle) { + // Conditional Chronicle check (only on first brush): + $index = array_search($bundle->getFilePath(), $this->unverified, true); + if ($index !== false) { + $validChronicle = $validator->checkChronicleHash($bundle); + $valid = $valid && $validChronicle; + if ($validChronicle) { + unset($this->unverified[$index]); + } else { + $this->markBundleAsBad($bundleIndex, 'Chronicle'); + } + } + } elseif ($checkChronicle) { + // Always check Chronicle: $valid = $valid && $validator->checkChronicleHash($bundle); } if ($valid) { return $bundle; } + } else { + $this->markBundleAsBad($bundleIndex, 'SHA256 mismatch'); } + ++$bundleIndex; } throw new BundleException('No valid bundles were found in the data directory.'); } @@ -133,18 +161,29 @@ public function setChronicle($url, $publicKey) } /** - * List bundles - * - * @param string $customValidator Fully-qualified class name for Validator - * @param string $trustChannel - * @return array - * - * @throws CertaintyException + * @param int $index + * @param string $reason + * @throws EncodingException + * @throws FilesystemException */ - protected function listBundles( - $customValidator = '', - $trustChannel = Certainty::TRUST_DEFAULT - ) { + protected function markBundleAsBad($index = 0, $reason = '') + { + $data = $this->loadCaCertsFile(); + $now = (new \DateTime())->format(\DateTime::ATOM); + $data[$index]['bad-bundle'] = 'Marked bad on ' . $now . ' for reason: ' . $reason; + \file_put_contents( + $this->dataDirectory . '/ca-certs.json', + json_encode($data, JSON_PRETTY_PRINT) + ); + } + + /** + * @return array + * @throws EncodingException + * @throws FilesystemException + */ + protected function loadCaCertsFile() + { if (!\file_exists($this->dataDirectory . '/ca-certs.json')) { throw new FilesystemException('ca-certs.json not found in data directory.'); } @@ -160,6 +199,23 @@ protected function listBundles( if (!\is_array($data)) { throw new EncodingException('ca-certs.json is not a valid JSON file.'); } + return (array) $data; + } + + /** + * List bundles + * + * @param string $customValidator Fully-qualified class name for Validator + * @param string $trustChannel + * @return array + * + * @throws CertaintyException + */ + protected function listBundles( + $customValidator = '', + $trustChannel = Certainty::TRUST_DEFAULT + ) { + $data = $this->loadCaCertsFile(); $bundles = []; /** @var array $row */ foreach ($data as $row) { @@ -167,6 +223,14 @@ protected function listBundles( // The necessary keys are not defined. continue; } + if (!file_exists($this->dataDirectory . '/' . $row['file'])) { + // Skip nonexistent files + continue; + } + if (!empty($row['bad-bundle'])) { + // Bundle marked as "bad" + continue; + } if ($row['trust-channel'] !== $trustChannel) { // Only include these. continue; diff --git a/src/RemoteFetch.php b/src/RemoteFetch.php index 9cb9715..ac74c9d 100644 --- a/src/RemoteFetch.php +++ b/src/RemoteFetch.php @@ -185,6 +185,7 @@ protected function remoteFetchBundles() /** @var string $body */ $body = (string) $request->getBody(); \file_put_contents($this->dataDirectory . '/' . $filename, $body); + $this->unverified []= $this->dataDirectory . '/' . $item['file']; } } diff --git a/test/RemoteFetchTest.php b/test/RemoteFetchTest.php index afec2f6..a745720 100644 --- a/test/RemoteFetchTest.php +++ b/test/RemoteFetchTest.php @@ -58,5 +58,13 @@ public function testRemoteFetch() ); $fetch->setCacheTimeout(new \DateInterval('PT01M')); $this->assertTrue($fetch->cacheExpired()); + + + $latest = $fetch->getLatestBundle(); + file_put_contents($latest->getFilePath(), ' corrupt', FILE_APPEND); + (new RemoteFetch($this->dir))->getLatestBundle(); + + $cacerts = json_decode(file_get_contents($this->dir . '/ca-certs.json'), true); + $this->assertTrue(!empty($cacerts[0]['bad-bundle'])); } } From c0813ee5d46dce998647393be23f6e40b06e2bd3 Mon Sep 17 00:00:00 2001 From: Paragon Initiative Enterprises Date: Fri, 27 Sep 2019 18:18:32 -0400 Subject: [PATCH 2/2] Fix Psalm nits --- src/Fetch.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Fetch.php b/src/Fetch.php index 09f4630..bf5f692 100644 --- a/src/Fetch.php +++ b/src/Fetch.php @@ -163,11 +163,13 @@ public function setChronicle($url, $publicKey) /** * @param int $index * @param string $reason + * @return void * @throws EncodingException * @throws FilesystemException */ protected function markBundleAsBad($index = 0, $reason = '') { + /** @var array> $data */ $data = $this->loadCaCertsFile(); $now = (new \DateTime())->format(\DateTime::ATOM); $data[$index]['bad-bundle'] = 'Marked bad on ' . $now . ' for reason: ' . $reason;