Skip to content

Commit f561796

Browse files
committed
fix slices everywhere + rollback managedauth to a proper impl
1 parent 5a1ea5c commit f561796

File tree

2 files changed

+49
-31
lines changed

2 files changed

+49
-31
lines changed

src/DataProtection/DataProtection/src/Managed/ManagedAuthenticatedEncryptor.cs

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -474,23 +474,6 @@ public byte[] Decrypt(ArraySegment<byte> protectedPayload, ArraySegment<byte> ad
474474
protectedPayload.Validate();
475475
additionalAuthenticatedData.Validate();
476476

477-
#if NET10_0_OR_GREATER
478-
var size = GetDecryptedSize(protectedPayload.Count);
479-
var plaintext = new byte[size];
480-
var destination = plaintext.AsSpan();
481-
482-
if (!TryDecrypt(
483-
cipherText: protectedPayload,
484-
additionalAuthenticatedData: additionalAuthenticatedData,
485-
destination: destination,
486-
out var bytesWritten))
487-
{
488-
throw Error.CryptCommon_GenericError(new ArgumentException("Not enough space in destination array"));
489-
}
490-
491-
CryptoUtil.Assert(bytesWritten == size, "bytesWritten == size");
492-
return plaintext;
493-
#else
494477
// Argument checking - input must at the absolute minimum contain a key modifier, IV, and MAC
495478
if (protectedPayload.Count < checked(KEY_MODIFIER_SIZE_IN_BYTES + _symmetricAlgorithmBlockSizeInBytes + _validationAlgorithmDigestLengthInBytes))
496479
{
@@ -516,14 +499,27 @@ public byte[] Decrypt(ArraySegment<byte> protectedPayload, ArraySegment<byte> ad
516499
ReadOnlySpan<byte> keyModifier = protectedPayload.Array!.AsSpan(keyModifierOffset, ivOffset - keyModifierOffset);
517500

518501
// Step 2: Decrypt the KDK and use it to restore the original encryption and MAC keys.
502+
#if NET10_0_OR_GREATER
503+
Span<byte> decryptedKdk = _keyDerivationKey.Length <= 256
504+
? stackalloc byte[256].Slice(0, _keyDerivationKey.Length)
505+
: new byte[_keyDerivationKey.Length];
506+
#else
519507
var decryptedKdk = new byte[_keyDerivationKey.Length];
508+
#endif
520509

521510
byte[]? validationSubkeyArray = null;
522511
var validationSubkey = _validationAlgorithmSubkeyLengthInBytes <= 128
523512
? stackalloc byte[128].Slice(0, _validationAlgorithmSubkeyLengthInBytes)
524513
: (validationSubkeyArray = new byte[_validationAlgorithmSubkeyLengthInBytes]);
525514

515+
#if NET10_0_OR_GREATER
516+
Span<byte> decryptionSubkey =
517+
_symmetricAlgorithmSubkeyLengthInBytes <= 128
518+
? stackalloc byte[128].Slice(0, _symmetricAlgorithmSubkeyLengthInBytes)
519+
: new byte[_symmetricAlgorithmBlockSizeInBytes];
520+
#else
526521
byte[] decryptionSubkey = new byte[_symmetricAlgorithmSubkeyLengthInBytes];
522+
#endif
527523

528524
// calling "fixed" is basically pinning the array, meaning the GC won't move it around. (Also for safety concerns)
529525
// note: it is safe to call `fixed` on null - it is just a no-op
@@ -551,9 +547,29 @@ public byte[] Decrypt(ArraySegment<byte> protectedPayload, ArraySegment<byte> ad
551547
}
552548

553549
// Step 4: Validate the MAC provided as part of the payload.
550+
#if NET10_0_OR_GREATER
551+
var ivAndCiphertextSpan = protectedPayload.Slice(ivOffset, macOffset - ivOffset);
552+
var providedMac = protectedPayload.Slice(macOffset, _validationAlgorithmDigestLengthInBytes);
553+
if (!ValidateMac(ivAndCiphertextSpan, providedMac, validationSubkey, validationSubkeyArray))
554+
{
555+
throw Error.CryptCommon_PayloadInvalid();
556+
}
557+
#else
554558
CalculateAndValidateMac(protectedPayload.Array!, ivOffset, macOffset, eofOffset, validationSubkey, validationSubkeyArray);
559+
#endif
555560

556561
// Step 5: Decipher the ciphertext and return it to the caller.
562+
#if NET10_0_OR_GREATER
563+
using var symmetricAlgorithm = CreateSymmetricAlgorithm();
564+
symmetricAlgorithm.SetKey(decryptionSubkey); // setKey is a single-shot usage of symmetricAlgorithm. Not allocatey
565+
566+
// note: here protectedPayload.Array is taken without an offset (can't use AsSpan() on ArraySegment)
567+
var ciphertext = protectedPayload.Array.AsSpan(ciphertextOffset, macOffset - ciphertextOffset);
568+
var iv = protectedPayload.Array.AsSpan(ivOffset, _symmetricAlgorithmBlockSizeInBytes);
569+
570+
// symmetricAlgorithm is created with CBC mode (see CreateSymmetricAlgorithm())
571+
return symmetricAlgorithm.DecryptCbc(ciphertext, iv);
572+
#else
557573
var iv = protectedPayload.Array!.AsSpan(ivOffset, _symmetricAlgorithmBlockSizeInBytes).ToArray();
558574

559575
using (var symmetricAlgorithm = CreateSymmetricAlgorithm())
@@ -569,13 +585,19 @@ public byte[] Decrypt(ArraySegment<byte> protectedPayload, ArraySegment<byte> ad
569585
return outputStream.ToArray();
570586
}
571587
}
588+
#endif
572589
}
573590
finally
574591
{
575592
// delete since these contain secret material
576593
validationSubkey.Clear();
577594

595+
#if NET10_0_OR_GREATER
596+
decryptedKdk.Clear();
597+
decryptionSubkey.Clear();
598+
#else
578599
Array.Clear(decryptedKdk, 0, decryptedKdk.Length);
600+
#endif
579601
}
580602
}
581603
}
@@ -584,7 +606,6 @@ public byte[] Decrypt(ArraySegment<byte> protectedPayload, ArraySegment<byte> ad
584606
// Homogenize all exceptions to CryptographicException.
585607
throw Error.CryptCommon_GenericError(ex);
586608
}
587-
#endif
588609
}
589610

590611
private byte[] CreateContextHeader()

src/DataProtection/DataProtection/test/Microsoft.AspNetCore.DataProtection.Tests/Internal/RoundtripEncryptionHelpers.cs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,7 @@ namespace Microsoft.AspNetCore.DataProtection.Tests.Internal;
1313
internal static class RoundtripEncryptionHelpers
1414
{
1515
/// <summary>
16-
/// <see cref="IAuthenticatedEncryptor.TryEncrypt"/> and <see cref="IAuthenticatedEncryptor.TryDecrypt"/> APIs should do the same steps
17-
/// as <see cref="IAuthenticatedEncryptor.Encrypt"/> and <see cref="IAuthenticatedEncryptor.Decrypt"/> APIs.
18-
/// <br/>
19-
/// Method ensures that the two APIs are equivalent in terms of their behavior by performing a roundtrip encrypt-decrypt test.
20-
/// </summary>
21-
public static void AssertTryEncryptTryDecryptParity(IAuthenticatedEncryptor encryptor, ReadOnlySpan<byte> plaintext, ReadOnlySpan<byte> aad)
22-
=> AssertTryEncryptTryDecryptParity(encryptor, plaintext, aad);
23-
24-
/// <summary>
25-
/// <see cref="IAuthenticatedEncryptor.TryEncrypt"/> and <see cref="IAuthenticatedEncryptor.TryDecrypt"/> APIs should do the same steps
16+
/// <see cref="ISpanAuthenticatedEncryptor.TryEncrypt"/> and <see cref="ISpanAuthenticatedEncryptor.TryDecrypt"/> APIs should do the same steps
2617
/// as <see cref="IAuthenticatedEncryptor.Encrypt"/> and <see cref="IAuthenticatedEncryptor.Decrypt"/> APIs.
2718
/// <br/>
2819
/// Method ensures that the two APIs are equivalent in terms of their behavior by performing a roundtrip encrypt-decrypt test.
@@ -41,7 +32,10 @@ public static void AssertTryEncryptTryDecryptParity(IAuthenticatedEncryptor encr
4132
var expectedEncryptedSize = spanAuthenticatedEncryptor.GetEncryptedSize(plaintext.Count);
4233
Assert.Equal(expectedEncryptedSize, ciphertext.Length);
4334
var expectedDecryptedSize = spanAuthenticatedEncryptor.GetDecryptedSize(ciphertext.Length);
44-
Assert.Equal(expectedDecryptedSize, decipheredtext.Length);
35+
36+
// note: for decryption we cant know for sure how many bytes will be written.
37+
// so we cant assert equality, but we can check if expected decrypted size is greater or equal than original deciphered text
38+
Assert.True(expectedDecryptedSize >= decipheredtext.Length);
4539

4640
// perform TryEncrypt and Decrypt roundtrip - ensures cross operation compatibility
4741
var cipherTextPooled = ArrayPool<byte>.Shared.Rent(expectedEncryptedSize);
@@ -65,12 +59,15 @@ public static void AssertTryEncryptTryDecryptParity(IAuthenticatedEncryptor encr
6559
{
6660
var encrypted = spanAuthenticatedEncryptor.Encrypt(plaintext, aad);
6761
var decipheredTryDecrypt = spanAuthenticatedEncryptor.TryDecrypt(encrypted, aad, plainTextPooled, out var bytesWritten);
68-
Assert.Equal(plaintext.AsSpan(), plainTextPooled.AsSpan());
62+
Assert.Equal(plaintext.AsSpan(), plainTextPooled.AsSpan(0, bytesWritten));
6963
Assert.True(decipheredTryDecrypt);
64+
65+
// now we should know that bytesWritten is STRICTLY equal to the deciphered text
66+
Assert.Equal(decipheredtext.Length, bytesWritten);
7067
}
7168
finally
7269
{
73-
ArrayPool<byte>.Shared.Return(cipherTextPooled);
70+
ArrayPool<byte>.Shared.Return(plainTextPooled);
7471
}
7572
}
7673
}

0 commit comments

Comments
 (0)