From af552a8fe0b7300a3270265bf66c01efb40e6f7a Mon Sep 17 00:00:00 2001 From: Mogens Heller Grabe Date: Thu, 4 Apr 2024 13:44:20 +0200 Subject: [PATCH] fix encryption + 2nd level retries bug --- ...ySecondLevelRetriesAndEncryptionAreCool.cs | 49 +++++++++++++++++++ ...PipelineAndTransactionContextCallbacks.cs} | 2 +- .../Encryption/DecryptMessagesIncomingStep.cs | 20 +++++--- .../EncryptionConfigurationExtensions.cs | 2 +- 4 files changed, 65 insertions(+), 8 deletions(-) create mode 100644 Rebus.Tests/Bugs/VerifySecondLevelRetriesAndEncryptionAreCool.cs rename Rebus.Tests/Examples/{ChangeCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs => ModifyCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs} (98%) diff --git a/Rebus.Tests/Bugs/VerifySecondLevelRetriesAndEncryptionAreCool.cs b/Rebus.Tests/Bugs/VerifySecondLevelRetriesAndEncryptionAreCool.cs new file mode 100644 index 000000000..22f98e094 --- /dev/null +++ b/Rebus.Tests/Bugs/VerifySecondLevelRetriesAndEncryptionAreCool.cs @@ -0,0 +1,49 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using Rebus.Activation; +using Rebus.Config; +using Rebus.Encryption; +using Rebus.Retry.Simple; +using Rebus.Tests.Contracts; +using Rebus.Tests.Contracts.Extensions; +using Rebus.Transport.InMem; +// ReSharper disable AccessToDisposedClosure +#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously + +namespace Rebus.Tests.Bugs; + +[TestFixture] +[Description("Exposed a bug that was caused by not removing the encryption headers from the transport message after descryption. This caused decryption to happen twice, but on un-encrypted contents the 2nd time around")] +public class VerifySecondLevelRetriesAndEncryptionAreCool : FixtureBase +{ + static readonly string SecretKey = FixedRijndaelEncryptionKeyProvider.GenerateNewKey(); + + [Test] + public async Task ItWorks() + { + using var activator = new BuiltinHandlerActivator(); + using var gotTheFailedMessage = new ManualResetEvent(initialState: false); + + activator.Handle(async _ => throw new IndexOutOfRangeException("it's out of range buddy")); + + activator.Handle>(async failed => + { + gotTheFailedMessage.Set(); + }); + + var bus = Configure.With(activator) + .Transport(t => t.UseInMemoryTransport(new(), "whatever")) + .Options(o => + { + o.RetryStrategy(secondLevelRetriesEnabled: true); + o.EnableEncryption(SecretKey); + }) + .Start(); + + await bus.SendLocal("HEJ"); + + gotTheFailedMessage.WaitOrDie(TimeSpan.FromSeconds(3)); + } +} \ No newline at end of file diff --git a/Rebus.Tests/Examples/ChangeCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs b/Rebus.Tests/Examples/ModifyCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs similarity index 98% rename from Rebus.Tests/Examples/ChangeCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs rename to Rebus.Tests/Examples/ModifyCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs index ce17012e8..4eaa1d697 100644 --- a/Rebus.Tests/Examples/ChangeCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs +++ b/Rebus.Tests/Examples/ModifyCurrentPrincipalInPipelineAndTransactionContextCallbacks.cs @@ -17,7 +17,7 @@ namespace Rebus.Tests.Examples; [TestFixture] [Description("Demonstrates in full how the current principal can be changed in the pipeline and in some relevant transaction context callbacks")] -public class ChangeCurrentPrincipalInPipelineAndTransactionContextCallbacks : FixtureBase +public class ModifyCurrentPrincipalInPipelineAndTransactionContextCallbacks : FixtureBase { [Test] public async Task CheckHowItWorks() diff --git a/Rebus/Encryption/DecryptMessagesIncomingStep.cs b/Rebus/Encryption/DecryptMessagesIncomingStep.cs index 16e44f402..1b4dff206 100644 --- a/Rebus/Encryption/DecryptMessagesIncomingStep.cs +++ b/Rebus/Encryption/DecryptMessagesIncomingStep.cs @@ -31,16 +31,24 @@ public DecryptMessagesIncomingStep(IAsyncEncryptor encryptor) public async Task Process(IncomingStepContext context, Func next) { var transportMessage = context.Load(); + var originalHeaders = transportMessage.Headers; - if (transportMessage.Headers.TryGetValue(EncryptionHeaders.ContentEncryption, out var contentEncryptionValue) + if (originalHeaders.TryGetValue(EncryptionHeaders.ContentEncryption, out var contentEncryptionValue) && contentEncryptionValue == _encryptor.ContentEncryptionValue) { - var headers = transportMessage.Headers.Clone(); - var encryptedBodyBytes = transportMessage.Body; + var headers = originalHeaders.Clone(); + + // remove these to prevent subsequent invocations of the pipeline to decrypt the transport message again + headers.Remove(EncryptionHeaders.ContentEncryption); + headers.Remove(EncryptionHeaders.ContentInitializationVector); + headers.Remove(EncryptionHeaders.KeyId); - headers.TryGetValue(EncryptionHeaders.KeyId, out var keyId); - - var iv = GetIv(headers); + // must look for key ID and IV in original headers :) + originalHeaders.TryGetValue(EncryptionHeaders.KeyId, out var keyId); + + var iv = GetIv(originalHeaders); + + var encryptedBodyBytes = transportMessage.Body; var bodyBytes = await _encryptor.Decrypt(new EncryptedData(encryptedBodyBytes, iv, keyId)); context.Save(new TransportMessage(headers, bodyBytes)); diff --git a/Rebus/Encryption/EncryptionConfigurationExtensions.cs b/Rebus/Encryption/EncryptionConfigurationExtensions.cs index b4b7e5e77..8eaad6d89 100644 --- a/Rebus/Encryption/EncryptionConfigurationExtensions.cs +++ b/Rebus/Encryption/EncryptionConfigurationExtensions.cs @@ -83,7 +83,7 @@ IAsyncEncryptor GetAsyncEncryptor() catch (Exception exception) { throw new RebusConfigurationException(exception, - @"Could not get IAsyncEncryptor to use when enabling encryption on the data bus storage. Please either enable encryption on the transport (via .Options(o => o.EnableEncryption(...))) to have an IAsyncEncryptor registered."); + "Could not get IAsyncEncryptor to use when enabling encryption on the data bus storage. Please either enable encryption on the transport (via .Options(o => o.EnableEncryption(...))) to have an IAsyncEncryptor registered."); } }