Fix broken storage key encryption; S3 adapter fixes

README.md

@@ -85,7 +85,8 @@ etc) into the one of the encryption adapters. Here's an example
 ```php
 $adapter = new \ObjectStorage\Adapter\EncryptedStorageAdapter(
     new \ObjectStorage\Adapter\PdoAdapter($pdo),
-    \ParagonIE\Halite\KeyFactory::loadEncryptionKey($pathToKeyfile)
+    \ParagonIE\Halite\KeyFactory::loadAuthenticationKey($pathToSigningKey),
+    \ParagonIE\Halite\KeyFactory::loadEncryptionKey($pathToEncryptionKey)
 );
 // You can use $adapter as before and both the storage keys and objects will be
 // encrypted (use PlaintextKeyEncryptedStorageAdapter if you don't want the
@@ -94,10 +95,12 @@ $adapter = new \ObjectStorage\Adapter\EncryptedStorageAdapter(
 
 The encryption routines are provided by [ParagonIE/Halite][] and libsodium.
 
-Use the following command to generate an encryption key and save it to a file :-
+Use the following commands to generate and save a signing key and an encryption
+key as needed in the previous example:-
 
 ```sh
-./bin/objectstorage genkey /path/to/a/file
+./bin/objectstorage genkey --signing /path/to/a/file
+./bin/objectstorage genkey /path/to/another/file
 ```
 
 You can also use the included encrypt + decrypt commands.  In the following

composer.json

@@ -19,6 +19,7 @@
         "symfony/console": "^4",
         "aws/aws-sdk-php": "^3",
         "friendsofphp/php-cs-fixer": "^2.15",
+        "phpstan/phpstan": "^0.11.8",
         "phpunit/phpunit": "^8.1"
     },
     "suggest": {
@@ -29,5 +30,10 @@
             "ObjectStorage\\": "src/"
         } 
     },
+    "autoload-dev": {
+        "psr-4": {
+            "ObjectStorage\\Test\\": "tests/"
+        }
+    },
     "license": "MIT"
 }

src/Adapter/AbstractEncryptedStorageAdapter.php

@@ -0,0 +1,30 @@
+<?php
+
+namespace ObjectStorage\Adapter;
+
+use ParagonIE\Halite\Symmetric\EncryptionKey;
+
+/**
+ * Base class for encrypted storage adapters.
+ */
+abstract class AbstractEncryptedStorageAdapter
+{
+    const CFG_AUTHENTICATION_KEY = 'authentication_key';
+    const CFG_AUTHENTICATION_KEY_PATH = 'authentication_key_path';
+    const CFG_ENCRYPTION_KEY = 'encryption_key';
+    const CFG_ENCRYPTION_KEY_PATH = 'encryption_key_path';
+    const CFG_STORAGE_ADAPTER = 'storage_adapter';
+
+    protected $encryptionKey;
+    protected $storageAdapter;
+
+    public function setAdapter(StorageAdapterInterface $storageAdapter)
+    {
+        $this->storageAdapter = $storageAdapter;
+    }
+
+    public function setEncryptionKey(EncryptionKey $encryptionKey)
+    {
+        $this->encryptionKey = $encryptionKey;
+    }
+}

src/Adapter/EncryptedStorageAdapter.php

@@ -5,6 +5,7 @@
 use ParagonIE\Halite\Alerts\CannotPerformOperation;
 use ParagonIE\Halite\Halite;
 use ParagonIE\Halite\KeyFactory;
+use ParagonIE\Halite\Symmetric\AuthenticationKey;
 use ParagonIE\Halite\Symmetric\Crypto;
 use ParagonIE\Halite\Symmetric\EncryptionKey;
 use ParagonIE\HiddenString\HiddenString;
@@ -13,19 +14,16 @@
  * Decorates a storage adapter to encrypt and decrypt object data and the keys
  * by which the data are stored.
  */
-class EncryptedStorageAdapter implements StorageAdapterInterface
+class EncryptedStorageAdapter extends AbstractEncryptedStorageAdapter implements StorageAdapterInterface
 {
-    const CFG_ENCRYPTION_KEY = 'encryption_key';
-    const CFG_ENCRYPTION_KEY_PATH = 'encryption_key_path';
-    const CFG_STORAGE_ADAPTER = 'storage_adapter';
-
+    protected $authenticationKey;
     protected $encryptionKey;
     protected $storageAdapter;
 
     public static function build(array $config)
     {
-        if (!isset($config[self::CFG_ENCRYPT_STORAGE_ADAPTER])
-            || !$config[self::CFG_ENCRYPT_STORAGE_ADAPTER] instanceof StorageAdapterInterface
+        if (!isset($config[self::CFG_STORAGE_ADAPTER])
+            || !$config[self::CFG_STORAGE_ADAPTER] instanceof StorageAdapterInterface
         ) {
             throw new \InvalidArgumentException(
                 'The build configuration for this storage adapter is missing an instance of StorageAdapterInterface, keyed as "'
@@ -34,6 +32,31 @@ public static function build(array $config)
             );
         }
 
+        if (isset($config[self::CFG_AUTHENTICATION_KEY])) {
+            if (!$config[self::CFG_AUTHENTICATION_KEY] instanceof AuthenticationKey) {
+                throw new \InvalidArgumentException(
+                    '"' . self::CFG_AUTHENTICATION_KEY . '"  must be an instance of AuthenticationKey.'
+                );
+            }
+            $authenticationKey = $config[self::CFG_AUTHENTICATION_KEY];
+        } elseif (isset($config[self::CFG_AUTHENTICATION_KEY_PATH])) {
+            try {
+                $authenticationKey = KeyFactory::loadAuthenticationKey($config[self::CFG_AUTHENTICATION_KEY_PATH]);
+            } catch (CannotPerformOperation $e) {
+                throw new \InvalidArgumentException(
+                    '"' . self::CFG_AUTHENTICATION_KEY_PATH . '"  must be a readable file.'
+                );
+            }
+        } else {
+            throw new \InvalidArgumentException(
+                'The build configuration for this storage adapter is missing an authentication key ("'
+                    . self::CFG_AUTHENTICATION_KEY
+                    . '" or "'
+                    . self::CFG_AUTHENTICATION_KEY_PATH
+                    . '").'
+            );
+        }
+
         if (isset($config[self::CFG_ENCRYPTION_KEY])) {
             if (!$config[self::CFG_ENCRYPTION_KEY] instanceof EncryptionKey) {
                 throw new \InvalidArgumentException(
@@ -60,89 +83,97 @@ public static function build(array $config)
         }
 
         return new self(
-            $config[self::CFG_ENCRYPT_STORAGE_ADAPTER],
+            $config[self::CFG_STORAGE_ADAPTER],
+            $authenticationKey,
             $encryptionKey
         );
     }
 
     public function __construct(
         StorageAdapterInterface $storageAdapter,
+        AuthenticationKey $authenticationKey,
         EncryptionKey $encryptionKey
     ) {
         $this->storageAdapter = $storageAdapter;
+        $this->authenticationKey = $authenticationKey;
         $this->encryptionKey = $encryptionKey;
     }
 
-    public function setAdapter(StorageAdapterInterface $storageAdapter)
+    public function setAuthenticationKey(AuthenticationKey $authenticationKey)
     {
-        $this->storageAdapter = $storageAdapter;
-    }
-
-    public function setEncryptionKey(EncryptionKey $encryptionKey)
-    {
-        $this->encryptionKey = $encryptionKey;
+        $this->authenticationKey = $authenticationKey;
     }
 
     public function setData($key, $data)
     {
         try {
-            $encryptedStorageKey = Crypto::encrypt(
-                new HiddenString($key),
-                $this->encryptionKey,
+            $authenticStorageKey = Crypto::authenticate(
+                $key,
+                $this->authenticationKey,
                 Halite::ENCODE_BASE64URLSAFE
             );
         } catch (CannotPerformOperation $e) {
-            throw new EncryptionFailureException('Failed to encrypt the storage key.', null, $e);
+            throw new EncryptionFailureException('Failed to hash the storage key.', null, $e);
         }
 
         try {
-            $encryptedData = Crypto::encrypt(
-                new HiddenString($data),
+            $encryptedBlob = Crypto::encrypt(
+                new HiddenString($key . $data),
                 $this->encryptionKey,
                 Halite::ENCODE_BASE64URLSAFE
             );
         } catch (CannotPerformOperation $e) {
-            throw new EncryptionFailureException('Failed to encrypt the object data.', null, $e);
+            throw new EncryptionFailureException('Failed to encrypt the storage key and object data.', null, $e);
         }
 
-        return $this->storageAdapter->setData($encryptedStorageKey, $encryptedData);
+        return $this->storageAdapter->setData($authenticStorageKey, $encryptedBlob);
     }
 
     public function getData($key)
     {
         try {
-            $encryptedStorageKey = Crypto::encrypt(
-                new HiddenString($key),
-                $this->encryptionKey,
+            $authenticStorageKey = Crypto::authenticate(
+                $key,
+                $this->authenticationKey,
                 Halite::ENCODE_BASE64URLSAFE
             );
         } catch (CannotPerformOperation $e) {
-            throw new EncryptionFailureException('Failed to encrypt the storage key.', null, $e);
+            throw new EncryptionFailureException('Failed to hash the storage key.', null, $e);
         }
 
-        $encryptedData = $this->storageAdapter->getData($encryptedStorageKey);
+        $encryptedBlob = $this->storageAdapter->getData($authenticStorageKey);
 
         try {
-            $plaintextData = (string) Crypto::decrypt($encryptedData, $this->encryptionKey);
+            $plaintextBlob = (string) Crypto::decrypt($encryptedBlob, $this->encryptionKey);
         } catch (CannotPerformOperation $e) {
-            throw new EncryptionFailureException('Failed to decrypt the object data.', null, $e);
+            throw new EncryptionFailureException('Failed to decrypt the storage key and object data.', null, $e);
         }
 
-        return $plaintextData;
+        $storageKeyLength = \strlen($key);
+
+        if (false === \hash_equals($key, \substr($plaintextBlob, 0, $storageKeyLength))) {
+            // The $plaintextBlob was definitely encrypted with our encryption key,
+            // but the storage key is not the one in that blob.
+            throw new EncryptionFailureException(
+                'The object data is not the expected one for the supplied storage key. The store has been corrupted or tampered with.'
+            );
+        }
+
+        return \substr($plaintextBlob, $storageKeyLength);
     }
 
     public function deleteData($key)
     {
         try {
-            $encryptedStorageKey = Crypto::encrypt(
-                new HiddenString($key),
-                $this->encryptionKey,
+            $authenticStorageKey = Crypto::authenticate(
+                $key,
+                $this->authenticationKey,
                 Halite::ENCODE_BASE64URLSAFE
             );
         } catch (CannotPerformOperation $e) {
-            throw new EncryptionFailureException('Failed to encrypt the storage key.', null, $e);
+            throw new EncryptionFailureException('Failed to hash the storage key.', null, $e);
         }
 
-        return $this->storageAdapter->deleteData($encryptedStorageKey);
+        return $this->storageAdapter->deleteData($authenticStorageKey);
     }
 }

src/Adapter/PlaintextStorageKeyEncryptedStorageAdapter.php

@@ -4,16 +4,69 @@
 
 use ParagonIE\Halite\Alerts\CannotPerformOperation;
 use ParagonIE\Halite\Halite;
+use ParagonIE\Halite\KeyFactory;
 use ParagonIE\Halite\Symmetric\Crypto;
+use ParagonIE\Halite\Symmetric\EncryptionKey;
 use ParagonIE\HiddenString\HiddenString;
 
 /**
  * Decorates a storage adapter to encrypt and decrypt the object data.
  *
  * Does not encrypt the keys by which data are stored.
  */
-class PlaintextStorageKeyEncryptedStorageAdapter extends EncryptedStorageAdapter
+class PlaintextStorageKeyEncryptedStorageAdapter extends AbstractEncryptedStorageAdapter implements StorageAdapterInterface
 {
+    public static function build(array $config)
+    {
+        if (!isset($config[self::CFG_STORAGE_ADAPTER])
+            || !$config[self::CFG_STORAGE_ADAPTER] instanceof StorageAdapterInterface
+        ) {
+            throw new \InvalidArgumentException(
+                'The build configuration for this storage adapter is missing an instance of StorageAdapterInterface, keyed as "'
+                    . self::CFG_STORAGE_ADAPTER
+                    . '"."'
+            );
+        }
+
+        if (isset($config[self::CFG_ENCRYPTION_KEY])) {
+            if (!$config[self::CFG_ENCRYPTION_KEY] instanceof EncryptionKey) {
+                throw new \InvalidArgumentException(
+                    '"' . self::CFG_ENCRYPTION_KEY . '"  must be an instance of EncryptionKey.'
+                );
+            }
+            $encryptionKey = $config[self::CFG_ENCRYPTION_KEY];
+        } elseif (isset($config[self::CFG_ENCRYPTION_KEY_PATH])) {
+            try {
+                $encryptionKey = KeyFactory::loadEncryptionKey($config[self::CFG_ENCRYPTION_KEY_PATH]);
+            } catch (CannotPerformOperation $e) {
+                throw new \InvalidArgumentException(
+                    '"' . self::CFG_ENCRYPTION_KEY_PATH . '"  must be a readable file.'
+                );
+            }
+        } else {
+            throw new \InvalidArgumentException(
+                'The build configuration for this storage adapter is missing an encryption key ("'
+                    . self::CFG_ENCRYPTION_KEY
+                    . '" or "'
+                    . self::CFG_ENCRYPTION_KEY_PATH
+                    . '").'
+            );
+        }
+
+        return new self(
+            $config[self::CFG_STORAGE_ADAPTER],
+            $encryptionKey
+        );
+    }
+
+    public function __construct(
+        StorageAdapterInterface $storageAdapter,
+        EncryptionKey $encryptionKey
+    ) {
+        $this->storageAdapter = $storageAdapter;
+        $this->encryptionKey = $encryptionKey;
+    }
+
     public function setData($key, $data)
     {
         try {