Validate MAC per key (#51063)

* Ensure mac is validated against keys individually

* lint

Author: timacdonald

src/Illuminate/Encryption/Encrypter.php

@@ -163,10 +163,19 @@ public function decrypt($payload, $unserialize = true)
             $tag = empty($payload['tag']) ? null : base64_decode($payload['tag'])
         );
 
+        $foundValidMac = false;
+
         // Here we will decrypt the value. If we are able to successfully decrypt it
         // we will then unserialize it and return it out to the caller. If we are
         // unable to decrypt this value we will throw out an exception message.
         foreach ($this->getAllKeys() as $key) {
+            if (
+                $this->shouldValidateMac() &&
+                ! ($foundValidMac = $foundValidMac || $this->validMacForKey($payload, $key))
+            ) {
+                continue;
+            }
+
             $decrypted = \openssl_decrypt(
                 $payload['value'], strtolower($this->cipher), $key, 0, $iv, $tag ?? ''
             );
@@ -176,7 +185,11 @@ public function decrypt($payload, $unserialize = true)
             }
         }
 
-        if ($decrypted === false) {
+        if ($this->shouldValidateMac() && ! $foundValidMac) {
+            throw new DecryptException('The MAC is invalid.');
+        }
+
+        if (($decrypted ?? false) === false) {
             throw new DecryptException('Could not decrypt the data.');
         }
 
@@ -232,10 +245,6 @@ protected function getJsonPayload($payload)
             throw new DecryptException('The payload is invalid.');
         }
 
-        if (! self::$supportedCiphers[strtolower($this->cipher)]['aead'] && ! $this->validMac($payload)) {
-            throw new DecryptException('The MAC is invalid.');
-        }
-
         return $payload;
     }
 
@@ -265,24 +274,28 @@ protected function validPayload($payload)
     }
 
     /**
-     * Determine if the MAC for the given payload is valid.
+     * Determine if the MAC for the given payload is valid for the primary key.
      *
      * @param  array  $payload
      * @return bool
      */
     protected function validMac(array $payload)
     {
-        foreach ($this->getAllKeys() as $key) {
-            $valid = hash_equals(
-                $this->hash($payload['iv'], $payload['value'], $key), $payload['mac']
-            );
-
-            if ($valid === true) {
-                return true;
-            }
-        }
+        return $this->validMacForKey($payload, $this->key);
+    }
 
-        return false;
+    /**
+     * Determine if the MAC is valid for the given payload and key.
+     *
+     * @param  array  $payload
+     * @param  string  $key
+     * @return bool
+     */
+    protected function validMacForKey($payload, $key)
+    {
+        return hash_equals(
+            $this->hash($payload['iv'], $payload['value'], $key), $payload['mac']
+        );
     }
 
     /**
@@ -302,6 +315,16 @@ protected function ensureTagIsValid($tag)
         }
     }
 
+    /**
+     * Determine if we should validate the MAC while decrypting.
+     *
+     * @return bool
+     */
+    protected function shouldValidateMac()
+    {
+        return ! self::$supportedCiphers[strtolower($this->cipher)]['aead'];
+    }
+
     /**
      * Get the encryption key that the encrypter is currently using.
      *

tests/Encryption/EncrypterTest.php

@@ -38,6 +38,18 @@ public function testRawStringEncryptionWithPreviousKeys()
         $this->assertSame('foo', $decrypted);
     }
 
+    public function testItValidatesMacOnPerKeyBasis()
+    {
+        // Payload created with (key: str_repeat('b', 16)) but will
+        // "successfully" decrypt with (key: str_repeat('a', 16)), however it
+        // outputs a random binary string as it is not the correct key.
+        $encrypted = 'eyJpdiI6Ilg0dFM5TVRibEFqZW54c3lQdWJoVVE9PSIsInZhbHVlIjoiRGJpa2p2ZHI3eUs0dUtRakJneUhUUT09IiwibWFjIjoiMjBjZWYxODdhNThhOTk4MTk1NTc0YTE1MDgzODU1OWE0ZmQ4MDc5ZjMxYThkOGM1ZmM1MzlmYzBkYTBjMWI1ZiIsInRhZyI6IiJ9';
+
+        $new = new Encrypter(str_repeat('a', 16));
+        $new->previousKeys([str_repeat('b', 16)]);
+        $this->assertSame('foo', $new->decryptString($encrypted));
+    }
+
     public function testEncryptionUsingBase64EncodedKey()
     {
         $e = new Encrypter(random_bytes(16));