From 9c4ea6bf789ac86be7d438a15bb2e80ac5b9a681 Mon Sep 17 00:00:00 2001 From: Samuel Lucas <63159663+samuel-lucas6@users.noreply.github.com> Date: Sat, 14 Dec 2024 12:38:27 +0000 Subject: [PATCH] src: Check for disposed in all incremental classes. This stuff isn't thread safe, but official .NET IDisposable APIs don't seem to be either. I might change that in the future. The other thing to note is that it's unnecessary for the tests to have DynamicData. The same is true for InvalidOperation tests. It just inflates the number of tests, so I might change this. --- src/Geralt.Tests/BLAKE2bTests.cs | 22 ++++++ src/Geralt.Tests/Ed25519Tests.cs | 20 +++++ src/Geralt.Tests/Poly1305Tests.cs | 19 +++++ src/Geralt.Tests/XChaCha20Poly1305Tests.cs | 77 ++++++++++++------- src/Geralt/Crypto/GuardedHeapAllocation.cs | 1 - src/Geralt/Crypto/IncrementalBLAKE2b.cs | 9 +++ src/Geralt/Crypto/IncrementalEd25519ph.cs | 7 ++ src/Geralt/Crypto/IncrementalPoly1305.cs | 7 ++ .../Crypto/IncrementalXChaCha20Poly1305.cs | 7 ++ 9 files changed, 139 insertions(+), 30 deletions(-) diff --git a/src/Geralt.Tests/BLAKE2bTests.cs b/src/Geralt.Tests/BLAKE2bTests.cs index 80ffcb6..6b748c9 100644 --- a/src/Geralt.Tests/BLAKE2bTests.cs +++ b/src/Geralt.Tests/BLAKE2bTests.cs @@ -505,4 +505,26 @@ public void Incremental_Verify_InvalidOperation(string hash, string message, str Assert.ThrowsException(() => blake2b.CacheState()); Assert.ThrowsException(() => blake2b.RestoreCachedState()); } + + [TestMethod] + [DynamicData(nameof(UnkeyedTestVectors), DynamicDataSourceType.Method)] + [DynamicData(nameof(KeyedTestVectors), DynamicDataSourceType.Method)] + public void Incremental_Disposed(string hash, string message, string? key = null) + { + var h = new byte[hash.Length / 2]; + var m = Convert.FromHexString(message); + var k = key != null ? Convert.FromHexString(key) : Array.Empty(); + + var blake2b = new IncrementalBLAKE2b(h.Length, k); + + blake2b.Dispose(); + + Assert.ThrowsException(() => blake2b.Reinitialize(h.Length, k)); + Assert.ThrowsException(() => blake2b.Update(m)); + Assert.ThrowsException(() => blake2b.Finalize(h)); + Assert.ThrowsException(() => blake2b.FinalizeAndVerify(h)); + Assert.ThrowsException(() => blake2b.CacheState()); + Assert.ThrowsException(() => blake2b.RestoreCachedState()); + Assert.ThrowsException(() => blake2b.Dispose()); + } } diff --git a/src/Geralt.Tests/Ed25519Tests.cs b/src/Geralt.Tests/Ed25519Tests.cs index 005a28e..07ced6c 100644 --- a/src/Geralt.Tests/Ed25519Tests.cs +++ b/src/Geralt.Tests/Ed25519Tests.cs @@ -458,4 +458,24 @@ public void Incremental_Verify_InvalidOperation(string signature, string message Assert.ThrowsException(() => ed25519ph.Finalize(s, sk)); Assert.ThrowsException(() => ed25519ph.FinalizeAndVerify(s, pk)); } + + [TestMethod] + [DynamicData(nameof(Rfc8032Ed25519phTestVectors), DynamicDataSourceType.Method)] + public void Incremental_Disposed(string signature, string message, string privateKey) + { + var s = Convert.FromHexString(signature); + var m = Convert.FromHexString(message); + var sk = Convert.FromHexString(privateKey); + var pk = sk[^Ed25519.PublicKeySize..]; + + var ed25519ph = new IncrementalEd25519ph(); + + ed25519ph.Dispose(); + + Assert.ThrowsException(() => ed25519ph.Reinitialize()); + Assert.ThrowsException(() => ed25519ph.Update(m)); + Assert.ThrowsException(() => ed25519ph.Finalize(s, sk)); + Assert.ThrowsException(() => ed25519ph.FinalizeAndVerify(s, pk)); + Assert.ThrowsException(() => ed25519ph.Dispose()); + } } diff --git a/src/Geralt.Tests/Poly1305Tests.cs b/src/Geralt.Tests/Poly1305Tests.cs index ebb74c2..1f03aa5 100644 --- a/src/Geralt.Tests/Poly1305Tests.cs +++ b/src/Geralt.Tests/Poly1305Tests.cs @@ -257,4 +257,23 @@ public void Incremental_Verify_InvalidOperation(string tag, string message, stri Assert.ThrowsException(() => poly1305.Finalize(t)); Assert.ThrowsException(() => poly1305.FinalizeAndVerify(t)); } + + [TestMethod] + [DynamicData(nameof(Rfc8439TestVectors), DynamicDataSourceType.Method)] + public void Incremental_Disposed(string tag, string message, string oneTimeKey) + { + var t = Convert.FromHexString(tag); + var m = Convert.FromHexString(message); + var k = Convert.FromHexString(oneTimeKey); + + var poly1305 = new IncrementalPoly1305(k); + + poly1305.Dispose(); + + Assert.ThrowsException(() => poly1305.Reinitialize(k)); + Assert.ThrowsException(() => poly1305.Update(m)); + Assert.ThrowsException(() => poly1305.Finalize(t)); + Assert.ThrowsException(() => poly1305.FinalizeAndVerify(t)); + Assert.ThrowsException(() => poly1305.Dispose()); + } } diff --git a/src/Geralt.Tests/XChaCha20Poly1305Tests.cs b/src/Geralt.Tests/XChaCha20Poly1305Tests.cs index c284b09..746140c 100644 --- a/src/Geralt.Tests/XChaCha20Poly1305Tests.cs +++ b/src/Geralt.Tests/XChaCha20Poly1305Tests.cs @@ -252,6 +252,42 @@ public void Incremental_Reinitialize_Valid(string key, string plaintext, string Assert.AreEqual(plaintext, Convert.ToHexString(p).ToLower()); } + [TestMethod] + [DynamicData(nameof(IncrementalDecryptTestVectors), DynamicDataSourceType.Method)] + public void Incremental_Tampered(string header, string key, string plaintext, string ciphertext, string associatedData) + { + var p = new byte[plaintext.Length / 2]; + var parameters = new List + { + Convert.FromHexString(header), + Convert.FromHexString(key), + Convert.FromHexString(ciphertext), + Convert.FromHexString(associatedData) + }; + + foreach (var param in parameters) { + param[0]++; + using var decryptor = new IncrementalXChaCha20Poly1305(decryption: true, parameters[0], parameters[1]); + Assert.ThrowsException(() => decryptor.Pull(p, parameters[2], parameters[3])); + param[0]--; + } + Assert.IsTrue(p.SequenceEqual(new byte[p.Length])); + } + + [TestMethod] + [DataRow("3677e196fb57f611fe71cf25cbd892481f7a7179c2827102", "808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9f", "4c616469657320616e642047656e746c656d656e206f662074686520636c617373206f66202739393a204966204920636f756c64206f6666657220796f75206f6e6c79206f6e652074697020666f7220746865206675747572652c2073756e73637265656e20776f756c642062652069742e", "cc55781c4cb5443c19b856b695a2a6fc801935832c9c8278da67de429c3760231c846838d5a58a89815a99d572bfcf6ec14ed725931c6ac1fb7445eb25cad39773502f9550670fd918638c89d83ddfda7297b59281b6012f780c0b3f0cb6309345573c7967fbcfb8843ce04db7912754fe861f5963c83dc6ad066c0d04b3235ade74ca", "")] + public void Incremental_MissingRekey(string header, string key, string plaintext, string ciphertext, string associatedData) + { + var h = Convert.FromHexString(header); + var k = Convert.FromHexString(key); + var p = new byte[plaintext.Length / 2]; + var c = Convert.FromHexString(ciphertext); + + using var decryptor = new IncrementalXChaCha20Poly1305(decryption: true, h, k); + // Should rekey here + Assert.ThrowsException(() => decryptor.Pull(p, c)); + } + [TestMethod] [DynamicData(nameof(IncrementalInvalidParameterSizes), DynamicDataSourceType.Method)] public void Incremental_Invalid(int headerSize, int keySize, int ciphertextSize, int plaintextSize) @@ -294,38 +330,21 @@ public void Incremental_InvalidOperation() } [TestMethod] - [DynamicData(nameof(IncrementalDecryptTestVectors), DynamicDataSourceType.Method)] - public void Incremental_Tampered(string header, string key, string plaintext, string ciphertext, string associatedData) + public void Incremental_Disposed() { - var p = new byte[plaintext.Length / 2]; - var parameters = new List - { - Convert.FromHexString(header), - Convert.FromHexString(key), - Convert.FromHexString(ciphertext), - Convert.FromHexString(associatedData) - }; + var h = new byte[IncrementalXChaCha20Poly1305.HeaderSize]; + var k = new byte[IncrementalXChaCha20Poly1305.KeySize]; + var p = new byte[h.Length]; + var c = new byte[p.Length + IncrementalXChaCha20Poly1305.TagSize]; - foreach (var param in parameters) { - param[0]++; - using var decryptor = new IncrementalXChaCha20Poly1305(decryption: true, parameters[0], parameters[1]); - Assert.ThrowsException(() => decryptor.Pull(p, parameters[2], parameters[3])); - param[0]--; - } - Assert.IsTrue(p.SequenceEqual(new byte[p.Length])); - } + var encryptor = new IncrementalXChaCha20Poly1305(decryption: false, h, k); - [TestMethod] - [DataRow("3677e196fb57f611fe71cf25cbd892481f7a7179c2827102", "808182838485868788898a8b8c8d8e8f909192939495969798999a9b9c9d9e9f", "4c616469657320616e642047656e746c656d656e206f662074686520636c617373206f66202739393a204966204920636f756c64206f6666657220796f75206f6e6c79206f6e652074697020666f7220746865206675747572652c2073756e73637265656e20776f756c642062652069742e", "cc55781c4cb5443c19b856b695a2a6fc801935832c9c8278da67de429c3760231c846838d5a58a89815a99d572bfcf6ec14ed725931c6ac1fb7445eb25cad39773502f9550670fd918638c89d83ddfda7297b59281b6012f780c0b3f0cb6309345573c7967fbcfb8843ce04db7912754fe861f5963c83dc6ad066c0d04b3235ade74ca", "")] - public void Incremental_MissingRekey(string header, string key, string plaintext, string ciphertext, string associatedData) - { - var h = Convert.FromHexString(header); - var k = Convert.FromHexString(key); - var p = new byte[plaintext.Length / 2]; - var c = Convert.FromHexString(ciphertext); + encryptor.Dispose(); - using var decryptor = new IncrementalXChaCha20Poly1305(decryption: true, h, k); - // Should rekey here - Assert.ThrowsException(() => decryptor.Pull(p, c)); + Assert.ThrowsException(() => encryptor.Reinitialize(decryption: false, h, k)); + Assert.ThrowsException(() => encryptor.Push(c, p, IncrementalXChaCha20Poly1305.ChunkFlag.Final)); + Assert.ThrowsException(() => encryptor.Pull(p, c)); + Assert.ThrowsException(() => encryptor.Rekey()); + Assert.ThrowsException(() => encryptor.Dispose()); } } diff --git a/src/Geralt/Crypto/GuardedHeapAllocation.cs b/src/Geralt/Crypto/GuardedHeapAllocation.cs index b148288..278ee9a 100644 --- a/src/Geralt/Crypto/GuardedHeapAllocation.cs +++ b/src/Geralt/Crypto/GuardedHeapAllocation.cs @@ -17,7 +17,6 @@ public GuardedHeapAllocation(int size) _pointer = sodium_malloc((nuint)size); if (_pointer == IntPtr.Zero) { throw new OutOfMemoryException("Unable to allocate memory."); } _size = size; - _disposed = false; } public unsafe Span AsSpan() diff --git a/src/Geralt/Crypto/IncrementalBLAKE2b.cs b/src/Geralt/Crypto/IncrementalBLAKE2b.cs index b462343..4e08bb7 100644 --- a/src/Geralt/Crypto/IncrementalBLAKE2b.cs +++ b/src/Geralt/Crypto/IncrementalBLAKE2b.cs @@ -21,6 +21,7 @@ public sealed class IncrementalBLAKE2b : IDisposable private int _hashSize; private bool _finalized; private bool _cached; + private bool _disposed; public IncrementalBLAKE2b(int hashSize, ReadOnlySpan key = default) { @@ -30,6 +31,7 @@ public IncrementalBLAKE2b(int hashSize, ReadOnlySpan key = default) public void Reinitialize(int hashSize, ReadOnlySpan key = default) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } Validation.SizeBetween(nameof(hashSize), hashSize, MinHashSize, MaxHashSize); if (key.Length != 0) { Validation.SizeBetween(nameof(key), key.Length, MinKeySize, MaxKeySize); } _hashSize = hashSize; @@ -40,6 +42,7 @@ public void Reinitialize(int hashSize, ReadOnlySpan key = default) public void Update(ReadOnlySpan message) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } if (_finalized) { throw new InvalidOperationException("Cannot update after finalizing without reinitializing or restoring a cached state."); } int ret = crypto_generichash_update(ref _state, message, (ulong)message.Length); if (ret != 0) { throw new CryptographicException("Error updating hash function state."); } @@ -47,6 +50,7 @@ public void Update(ReadOnlySpan message) public void Finalize(Span hash) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing or restoring a cached state."); } Validation.EqualToSize(nameof(hash), hash.Length, _hashSize); _finalized = true; @@ -56,6 +60,7 @@ public void Finalize(Span hash) public bool FinalizeAndVerify(ReadOnlySpan hash) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing or restoring a cached state."); } Validation.EqualToSize(nameof(hash), hash.Length, _hashSize); Span computedHash = stackalloc byte[_hashSize]; @@ -67,6 +72,7 @@ public bool FinalizeAndVerify(ReadOnlySpan hash) public void CacheState() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } if (_finalized) { throw new InvalidOperationException("Cannot cache the state after finalizing without reinitializing."); } _cachedState = _state; _cached = true; @@ -74,6 +80,7 @@ public void CacheState() public void RestoreCachedState() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } if (!_cached) { throw new InvalidOperationException("Cannot restore the state when it has not been cached."); } _state = _cachedState; _finalized = false; @@ -82,7 +89,9 @@ public void RestoreCachedState() [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] public void Dispose() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalBLAKE2b)); } _state = default; _cachedState = default; + _disposed = true; } } diff --git a/src/Geralt/Crypto/IncrementalEd25519ph.cs b/src/Geralt/Crypto/IncrementalEd25519ph.cs index 30f3a61..9c5aa1f 100644 --- a/src/Geralt/Crypto/IncrementalEd25519ph.cs +++ b/src/Geralt/Crypto/IncrementalEd25519ph.cs @@ -12,6 +12,7 @@ public sealed class IncrementalEd25519ph : IDisposable private crypto_sign_state _state; private bool _finalized; + private bool _disposed; public IncrementalEd25519ph() { @@ -21,6 +22,7 @@ public IncrementalEd25519ph() public void Reinitialize() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalEd25519ph)); } _finalized = false; int ret = crypto_sign_init(ref _state); if (ret != 0) { throw new CryptographicException("Error initializing signature scheme state."); } @@ -28,6 +30,7 @@ public void Reinitialize() public void Update(ReadOnlySpan message) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalEd25519ph)); } if (_finalized) { throw new InvalidOperationException("Cannot update after finalizing without reinitializing."); } int ret = crypto_sign_update(ref _state, message, (ulong)message.Length); if (ret != 0) { throw new CryptographicException("Error updating signature scheme state."); } @@ -35,6 +38,7 @@ public void Update(ReadOnlySpan message) public void Finalize(Span signature, ReadOnlySpan privateKey) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalEd25519ph)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing."); } Validation.EqualToSize(nameof(signature), signature.Length, SignatureSize); Validation.EqualToSize(nameof(privateKey), privateKey.Length, PrivateKeySize); @@ -45,6 +49,7 @@ public void Finalize(Span signature, ReadOnlySpan privateKey) public bool FinalizeAndVerify(ReadOnlySpan signature, ReadOnlySpan publicKey) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalEd25519ph)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing."); } Validation.EqualToSize(nameof(signature), signature.Length, SignatureSize); Validation.EqualToSize(nameof(publicKey), publicKey.Length, PublicKeySize); @@ -55,6 +60,8 @@ public bool FinalizeAndVerify(ReadOnlySpan signature, ReadOnlySpan p [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] public void Dispose() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalEd25519ph)); } _state = default; + _disposed = true; } } diff --git a/src/Geralt/Crypto/IncrementalPoly1305.cs b/src/Geralt/Crypto/IncrementalPoly1305.cs index 66e687d..b449295 100644 --- a/src/Geralt/Crypto/IncrementalPoly1305.cs +++ b/src/Geralt/Crypto/IncrementalPoly1305.cs @@ -11,6 +11,7 @@ public sealed class IncrementalPoly1305 : IDisposable private crypto_onetimeauth_state _state; private bool _finalized; + private bool _disposed; public IncrementalPoly1305(ReadOnlySpan oneTimeKey) { @@ -20,6 +21,7 @@ public IncrementalPoly1305(ReadOnlySpan oneTimeKey) public void Reinitialize(ReadOnlySpan oneTimeKey) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalPoly1305)); } Validation.EqualToSize(nameof(oneTimeKey), oneTimeKey.Length, KeySize); _finalized = false; int ret = crypto_onetimeauth_init(ref _state, oneTimeKey); @@ -28,6 +30,7 @@ public void Reinitialize(ReadOnlySpan oneTimeKey) public void Update(ReadOnlySpan message) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalPoly1305)); } if (_finalized) { throw new InvalidOperationException("Cannot update after finalizing without reinitializing."); } int ret = crypto_onetimeauth_update(ref _state, message, (ulong)message.Length); if (ret != 0) { throw new CryptographicException("Error updating message authentication code state."); } @@ -35,6 +38,7 @@ public void Update(ReadOnlySpan message) public void Finalize(Span tag) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalPoly1305)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing."); } Validation.EqualToSize(nameof(tag), tag.Length, TagSize); _finalized = true; @@ -44,6 +48,7 @@ public void Finalize(Span tag) public bool FinalizeAndVerify(ReadOnlySpan tag) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalPoly1305)); } if (_finalized) { throw new InvalidOperationException("Cannot finalize twice without reinitializing."); } Validation.EqualToSize(nameof(tag), tag.Length, TagSize); Span computedTag = stackalloc byte[TagSize]; @@ -56,6 +61,8 @@ public bool FinalizeAndVerify(ReadOnlySpan tag) [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] public void Dispose() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalPoly1305)); } _state = default; + _disposed = true; } } diff --git a/src/Geralt/Crypto/IncrementalXChaCha20Poly1305.cs b/src/Geralt/Crypto/IncrementalXChaCha20Poly1305.cs index 75170cf..ec5805d 100644 --- a/src/Geralt/Crypto/IncrementalXChaCha20Poly1305.cs +++ b/src/Geralt/Crypto/IncrementalXChaCha20Poly1305.cs @@ -13,6 +13,7 @@ public sealed class IncrementalXChaCha20Poly1305 : IDisposable private crypto_secretstream_xchacha20poly1305_state _state; private bool _decryption; private bool _finalized; + private bool _disposed; public enum ChunkFlag { @@ -30,6 +31,7 @@ public IncrementalXChaCha20Poly1305(bool decryption, Span header, ReadOnly public void Reinitialize(bool decryption, Span header, ReadOnlySpan key) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalXChaCha20Poly1305)); } Validation.EqualToSize(nameof(header), header.Length, HeaderSize); Validation.EqualToSize(nameof(key), key.Length, KeySize); _decryption = decryption; @@ -47,6 +49,7 @@ public void Push(Span ciphertextChunk, ReadOnlySpan plaintextChunk, public void Push(Span ciphertextChunk, ReadOnlySpan plaintextChunk, ReadOnlySpan associatedData, ChunkFlag chunkFlag = ChunkFlag.Message) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalXChaCha20Poly1305)); } if (_decryption) { throw new InvalidOperationException("Cannot push into a decryption stream."); } if (_finalized) { throw new InvalidOperationException("Cannot push after the final chunk without reinitializing."); } Validation.EqualToSize(nameof(ciphertextChunk), ciphertextChunk.Length, plaintextChunk.Length + TagSize); @@ -57,6 +60,7 @@ public void Push(Span ciphertextChunk, ReadOnlySpan plaintextChunk, public ChunkFlag Pull(Span plaintextChunk, ReadOnlySpan ciphertextChunk, ReadOnlySpan associatedData = default) { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalXChaCha20Poly1305)); } if (!_decryption) { throw new InvalidOperationException("Cannot pull from an encryption stream."); } if (_finalized) { throw new InvalidOperationException("Cannot pull after the final chunk without reinitializing."); } Validation.NotLessThanMin(nameof(ciphertextChunk), ciphertextChunk.Length, TagSize); @@ -69,6 +73,7 @@ public ChunkFlag Pull(Span plaintextChunk, ReadOnlySpan ciphertextCh public void Rekey() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalXChaCha20Poly1305)); } if (_finalized) { throw new InvalidOperationException("Cannot rekey after the final chunk without reinitializing."); } crypto_secretstream_xchacha20poly1305_rekey(ref _state); } @@ -76,6 +81,8 @@ public void Rekey() [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization)] public void Dispose() { + if (_disposed) { throw new ObjectDisposedException(nameof(IncrementalXChaCha20Poly1305)); } _state = default; + _disposed = true; } }