Merge pull request #60735 from nextcloud/fix/noid/crypto-decrypt-fallback-valueerror

fix(security): don't propagate ValueError from Crypto::decrypt() fallback

Author: miaulalala

lib/private/Security/Crypto.php

@@ -103,7 +103,13 @@ public function decrypt(string $authenticatedCiphertext, string $password = ''):
 		} catch (Exception $e) {
 			if ($password === '') {
 				// Retry with empty secret as a fallback for instances where the secret might not have been set by accident
-				return $this->decryptWithoutSecret($authenticatedCiphertext, '');
+				try {
+					return $this->decryptWithoutSecret($authenticatedCiphertext, '');
+				} catch (\Throwable) {
+					// Fallback failed (e.g. v3 ciphertext requires a non-empty key for hash_hkdf),
+					// rethrow the original exception
+					throw $e;
+				}
 			}
 			throw $e;
 		}

tests/lib/Security/CryptoTest.php

@@ -99,6 +99,27 @@ public function testOcVersion2CiphertextDecryptsToCorrectPlaintext(): void {
 		);
 	}
 
+	public function testDecryptWithWrongSecretThrowsHmacExceptionNotValueError(): void {
+		// Encrypt with 'secret-A'
+		$configA = $this->createMock(IConfig::class);
+		$configA->method('getSystemValue')->with('secret')->willReturn('secret-A');
+		$configA->method('getSystemValueString')->with('secret')->willReturn('secret-A');
+		$cryptoA = new Crypto($configA);
+		$ciphertext = $cryptoA->encrypt('plaintext');
+
+		// Decrypt with 'secret-B': first attempt fails (HMAC mismatch), fallback to empty
+		// string must not propagate a ValueError for v3 ciphertexts — it must rethrow the
+		// original HMAC exception instead.
+		$configB = $this->createMock(IConfig::class);
+		$configB->method('getSystemValue')->with('secret')->willReturn('secret-B');
+		$configB->method('getSystemValueString')->with('secret')->willReturn('secret-B');
+		$cryptoB = new Crypto($configB);
+
+		$this->expectException(\Exception::class);
+		$this->expectExceptionMessage('HMAC does not match.');
+		$cryptoB->decrypt($ciphertext);
+	}
+
 	public function testVersion3CiphertextDecryptsToCorrectPlaintext(): void {
 		$this->assertSame(
 			'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.',