Skip to content

Commit 3800323

Browse files
KrisellKrisell
committed
Remove unnecessary double MAC for AEAD ciphers
1 parent 1a2ecc9 commit 3800323

File tree

2 files changed

+71
-41
lines changed

2 files changed

+71
-41
lines changed

‎src/Illuminate/Encryption/Encrypter.php

Lines changed: 47 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@
1010

1111
class Encrypter implements EncrypterContract, StringEncrypter
1212
{
13+
/**
14+
* The supported cipher algorithms and their properties.
15+
*
16+
* @var array
17+
*/
18+
private static $supportedCiphers = [
19+
'AES-128-CBC' => ['size' => 16, 'aead' => false],
20+
'AES-256-CBC' => ['size' => 32, 'aead' => false],
21+
'AES-128-GCM' => ['size' => 16, 'aead' => true],
22+
'AES-256-GCM' => ['size' => 32, 'aead' => true],
23+
];
24+
1325
/**
1426
* The encryption key.
1527
*
@@ -37,12 +49,13 @@ public function __construct($key, $cipher = 'AES-128-CBC')
3749
{
3850
$key = (string) $key;
3951

40-
if (static::supported($key, $cipher)) {
41-
$this->key = $key;
42-
$this->cipher = $cipher;
43-
} else {
44-
throw new RuntimeException('The only supported ciphers are AES-128-CBC, AES-256-CBC, AES-128-GCM, and AES-256-GCM with the correct key lengths.');
52+
if (! static::supported($key, $cipher)) {
53+
$ciphers = implode(', ', array_keys(self::$supportedCiphers));
54+
throw new RuntimeException("Unsupported cipher or incorrect key length. Supported ciphers are: $ciphers.");
4555
}
56+
57+
$this->key = $key;
58+
$this->cipher = $cipher;
4659
}
4760

4861
/**
@@ -54,12 +67,11 @@ public function __construct($key, $cipher = 'AES-128-CBC')
5467
*/
5568
public static function supported($key, $cipher)
5669
{
57-
$length = mb_strlen($key, '8bit');
70+
if (! isset(self::$supportedCiphers[$cipher])) {
71+
return false;
72+
}
5873

59-
return ($cipher === 'AES-128-CBC' && $length === 16) ||
60-
($cipher === 'AES-256-CBC' && $length === 32) ||
61-
($cipher === 'AES-128-GCM' && $length === 16) ||
62-
($cipher === 'AES-256-GCM' && $length === 32);
74+
return mb_strlen($key, '8bit') === self::$supportedCiphers[$cipher]['size'];
6375
}
6476

6577
/**
@@ -70,7 +82,7 @@ public static function supported($key, $cipher)
7082
*/
7183
public static function generateKey($cipher)
7284
{
73-
return random_bytes($cipher === 'AES-128-CBC' ? 16 : 32);
85+
return random_bytes(self::$supportedCiphers[$cipher]['size']);
7486
}
7587

7688
/**
@@ -86,33 +98,31 @@ public function encrypt($value, $serialize = true)
8698
{
8799
$iv = random_bytes(openssl_cipher_iv_length($this->cipher));
88100

89-
$tag = in_array($this->cipher, ['AES-128-GCM', 'AES-256-GCM']) ? '' : null;
90-
91-
// First we will encrypt the value using OpenSSL. After this is encrypted we
92-
// will proceed to calculating a MAC for the encrypted value so that this
93-
// value can be verified later as not having been changed by the users.
94-
$value =
95-
in_array($this->cipher, ['AES-128-GCM', 'AES-256-GCM']) ?
96-
\openssl_encrypt(
97-
$serialize ? serialize($value) : $value,
98-
$this->cipher, $this->key, 0, $iv, $tag
99-
) :
100-
\openssl_encrypt(
101-
$serialize ? serialize($value) : $value,
102-
$this->cipher, $this->key, 0, $iv
103-
);
101+
// A tag (mac) is returned by openssl_encrypt for AEAD ciphers.
102+
// Including $tag in the call for non-AEAD ciphers results in a warning before PHP 8.1.
103+
$tag = '';
104+
$value = self::$supportedCiphers[$this->cipher]['aead']
105+
? \openssl_encrypt(
106+
$serialize ? serialize($value) : $value,
107+
$this->cipher, $this->key, 0, $iv, $tag
108+
)
109+
: \openssl_encrypt(
110+
$serialize ? serialize($value) : $value,
111+
$this->cipher, $this->key, 0, $iv
112+
);
104113

105114
if ($value === false) {
106115
throw new EncryptException('Could not encrypt the data.');
107116
}
108117

109-
// Once we get the encrypted value we'll go ahead and base64_encode the input
110-
// vector and create the MAC for the encrypted value so we can then verify
111-
// its authenticity. Then, we'll JSON the data into the "payload" array.
112-
$mac = $this->hash(
113-
$iv = base64_encode($iv), $value, $tag = $tag ? base64_encode($tag) : ''
114-
);
118+
$iv = base64_encode($iv);
119+
$tag = base64_encode($tag);
120+
121+
$mac = self::$supportedCiphers[$this->cipher]['aead']
122+
? '' // For AEAD-algoritms, the tag/mac is returned by openssl_encrypt
123+
: $this->hash($iv, $value);
115124

125+
// Both tag and mac are included for compatibility reasons. A breaking update could use the same name for these.
116126
$json = json_encode(compact('iv', 'value', 'mac', 'tag'), JSON_UNESCAPED_SLASHES);
117127

118128
if (json_last_error() !== JSON_ERROR_NONE) {
@@ -184,12 +194,11 @@ public function decryptString($payload)
184194
*
185195
* @param string $iv
186196
* @param mixed $value
187-
* @param string $tag
188197
* @return string
189198
*/
190-
protected function hash($iv, $value, $tag = '')
199+
protected function hash($iv, $value)
191200
{
192-
return hash_hmac('sha256', $tag.$iv.$value, $this->key);
201+
return hash_hmac('sha256', $iv.$value, $this->key);
193202
}
194203

195204
/**
@@ -211,7 +220,8 @@ protected function getJsonPayload($payload)
211220
throw new DecryptException('The payload is invalid.');
212221
}
213222

214-
if (! $this->validMac($payload)) {
223+
// We only need to check for the valid MAC if a non-AEAD algorithm is used
224+
if (! self::$supportedCiphers[$this->cipher]['aead'] && ! $this->validMac($payload)) {
215225
throw new DecryptException('The MAC is invalid.');
216226
}
217227

@@ -239,7 +249,7 @@ protected function validPayload($payload)
239249
protected function validMac(array $payload)
240250
{
241251
return hash_equals(
242-
$this->hash($payload['iv'], $payload['value'], $payload['tag'] ?? ''), $payload['mac']
252+
$this->hash($payload['iv'], $payload['value']), $payload['mac']
243253
);
244254
}
245255

‎tests/Encryption/EncrypterTest.php

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,34 +56,54 @@ public function testWithCustomCipher()
5656
$this->assertSame('foo', $e->decrypt($encrypted));
5757
}
5858

59+
public function testThatAnAeadCipherIncludesTag()
60+
{
61+
$e = new Encrypter(str_repeat('b', 32), 'AES-256-GCM');
62+
$encrypted = $e->encrypt('foo');
63+
$data = json_decode(base64_decode($encrypted));
64+
65+
$this->assertEmpty($data->mac);
66+
$this->assertNotEmpty($data->tag);
67+
}
68+
69+
public function testThatANonAeadCipherIncludesMac()
70+
{
71+
$e = new Encrypter(str_repeat('b', 32), 'AES-256-CBC');
72+
$encrypted = $e->encrypt('foo');
73+
$data = json_decode(base64_decode($encrypted));
74+
75+
$this->assertEmpty($data->tag);
76+
$this->assertNotEmpty($data->mac);
77+
}
78+
5979
public function testDoNoAllowLongerKey()
6080
{
6181
$this->expectException(RuntimeException::class);
62-
$this->expectExceptionMessage('The only supported ciphers are AES-128-CBC, AES-256-CBC, AES-128-GCM, and AES-256-GCM with the correct key lengths.');
82+
$this->expectExceptionMessage('Unsupported cipher or incorrect key length. Supported ciphers are: AES-128-CBC, AES-256-CBC, AES-128-GCM, AES-256-GCM.');
6383

6484
new Encrypter(str_repeat('z', 32));
6585
}
6686

6787
public function testWithBadKeyLength()
6888
{
6989
$this->expectException(RuntimeException::class);
70-
$this->expectExceptionMessage('The only supported ciphers are AES-128-CBC, AES-256-CBC, AES-128-GCM, and AES-256-GCM with the correct key lengths.');
90+
$this->expectExceptionMessage('Unsupported cipher or incorrect key length. Supported ciphers are: AES-128-CBC, AES-256-CBC, AES-128-GCM, AES-256-GCM.');
7191

7292
new Encrypter(str_repeat('a', 5));
7393
}
7494

7595
public function testWithBadKeyLengthAlternativeCipher()
7696
{
7797
$this->expectException(RuntimeException::class);
78-
$this->expectExceptionMessage('The only supported ciphers are AES-128-CBC, AES-256-CBC, AES-128-GCM, and AES-256-GCM with the correct key lengths.');
98+
$this->expectExceptionMessage('Unsupported cipher or incorrect key length. Supported ciphers are: AES-128-CBC, AES-256-CBC, AES-128-GCM, AES-256-GCM.');
7999

80100
new Encrypter(str_repeat('a', 16), 'AES-256-CFB8');
81101
}
82102

83103
public function testWithUnsupportedCipher()
84104
{
85105
$this->expectException(RuntimeException::class);
86-
$this->expectExceptionMessage('The only supported ciphers are AES-128-CBC, AES-256-CBC, AES-128-GCM, and AES-256-GCM with the correct key lengths.');
106+
$this->expectExceptionMessage('Unsupported cipher or incorrect key length. Supported ciphers are: AES-128-CBC, AES-256-CBC, AES-128-GCM, AES-256-GCM.');
87107

88108
new Encrypter(str_repeat('c', 16), 'AES-256-CFB8');
89109
}

0 commit comments

Comments
 (0)