Skip to content

Commit 15db395

Browse files
Merge pull request #61105 from nextcloud/backport/60735/stable34
[stable34] fix(security): don't propagate ValueError from Crypto::decrypt() fallback
2 parents 495f308 + c0b57cd commit 15db395

2 files changed

Lines changed: 28 additions & 1 deletion

File tree

lib/private/Security/Crypto.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,13 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''):
102102
} catch (Exception $e) {
103103
if ($password === '') {
104104
// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
105-
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
105+
try {
106+
return $this->decryptWithoutSecret($authenticatedCiphertext, '');
107+
} catch (\Throwable) {
108+
// Fallback failed (e.g. v3 ciphertext requires a non-empty key for hash_hkdf),
109+
// rethrow the original exception
110+
throw $e;
111+
}
106112
}
107113
throw $e;
108114
}

tests/lib/Security/CryptoTest.php

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,27 @@ public function testOcVersion2CiphertextDecryptsToCorrectPlaintext(): void {
102102
);
103103
}
104104

105+
public function testDecryptWithWrongSecretThrowsHmacExceptionNotValueError(): void {
106+
// Encrypt with 'secret-A'
107+
$configA = $this->createMock(IConfig::class);
108+
$configA->method('getSystemValue')->with('secret')->willReturn('secret-A');
109+
$configA->method('getSystemValueString')->with('secret')->willReturn('secret-A');
110+
$cryptoA = new Crypto($configA);
111+
$ciphertext = $cryptoA->encrypt('plaintext');
112+
113+
// Decrypt with 'secret-B': first attempt fails (HMAC mismatch), fallback to empty
114+
// string must not propagate a ValueError for v3 ciphertexts — it must rethrow the
115+
// original HMAC exception instead.
116+
$configB = $this->createMock(IConfig::class);
117+
$configB->method('getSystemValue')->with('secret')->willReturn('secret-B');
118+
$configB->method('getSystemValueString')->with('secret')->willReturn('secret-B');
119+
$cryptoB = new Crypto($configB);
120+
121+
$this->expectException(\Exception::class);
122+
$this->expectExceptionMessage('HMAC does not match.');
123+
$cryptoB->decrypt($ciphertext);
124+
}
125+
105126
public function testVersion3CiphertextDecryptsToCorrectPlaintext(): void {
106127
$this->assertSame(
107128
'Another plaintext value that will be encrypted with version 3. It addresses the related key issue. Old ciphertexts should be decrypted properly, but only use the better version for encryption.',

0 commit comments

Comments
 (0)