Issue when using both RetryStrategy and EnableEncryption, IFailed<message> handlers not called

[jgageelmresourcescom]

We recently updated to Rebus 8.1.0 from a much earlier version and are now seeing CryptographicExceptions where we did not see them before. I believe this error affects anyone that is using both RetryStrategy and EnableEncryption. The result is the IFailed handlers are not getting called so functionally second level retry isn't working.

The error we're seeing is: System.Security.Cryptography.CryptographicException: Padding is invalid and cannot be removed. at System.Security.Cryptography.SymmetricPadding.GetPaddingLength(ReadOnlySpan block, PaddingMode paddingMode, Int32 blockSize) at System.Security.Cryptography.UniversalCryptoDecryptor.UncheckedTransformFinalBlock(ReadOnlySpan inputBuffer, Span outputBuffer) at System.Security.Cryptography.UniversalCryptoDecryptor.UncheckedTransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount) at System.Security.Cryptography.UniversalCryptoTransform.TransformFinalBlock(Byte[] inputBuffer, Int32 inputOffset, Int32 inputCount) at System.Security.Cryptography.CryptoStream.FlushFinalBlockAsync(Boolean useAsync, CancellationToken cancellationToken) at Rebus.Encryption.RijndaelEncryptor.Decrypt(EncryptedData encryptedData) at Rebus.Encryption.DecryptMessagesIncomingStep.Process(IncomingStepContext context, Func next) at Rebus.DataBus.ClaimCheck.HydrateIncomingMessageStep.Process(IncomingStepContext context, Func next) at Rebus.Pipeline.Receive.HandleDeferredMessagesStep.Process(IncomingStepContext context, Func next) at Rebus.Retry.Simple.DefaultRetryStep.DispatchSecondLevelRetry(ITransactionContext transactionContext, StepContext context, Func next) at Rebus.Retry.Simple.DefaultRetryStep.HandleException(Exception exception, ITransactionContext transactionContext, String messageId, IncomingStepContext context, Func next)

Based on this stack, I suspected that the messages coming through the pipeline via the SecondLevelRetry path were not encrypted but DecryptMessagesIncomingStep was trying to decrypt them anyways. I added a custom step called CheckDecryptionNeededStep in the pipeline just before DecryptMessagesIncomingStep where I could confirm my suspicions.

This is the present setup:

services.AddRebus(configure => configure
    .Logging(l => l.NLog())
    .Serialization(s => s.UseNewtonsoftJson(JsonInteroperabilityMode.PureJson))
    .Transport(t => t.UseRabbitMq(<redacted>, queueName)...
        .Ssl(GenerateSslSettings(<redacted>))
        .Prefetch(prefetchCount))
    .Options(o => o.SetNumberOfWorkers(busSettings.NumberOfWorkers))
    .Options(o => o.SetMaxParallelism(busSettings.MaxParallelism))
    .Options(o => o.RetryStrategy(errorQueueName: <redacted>))
    .Options(o => o.EnableEncryption(<redacted>))
    .Options(o => o.Decorate<IPipeline>(c =>
        {
            var pipeline = c.Get<IPipeline>();
            var sanitizeDecryptionStep = new SanitizeDecryptionErrorStep();
            var checkDecryptionNeededStep = new CheckDecryptionNeededStep();
            return new PipelineStepInjector(pipeline)
                .OnReceive(sanitizeDecryptionStep, PipelineRelativePosition.Before, typeof(DeserializeIncomingMessageStep))
                .OnReceive(checkDecryptionNeededStep, PipelineRelativePosition.Before, typeof(DecryptMessagesIncomingStep));
        }))
    );

This custom step (see below) provides a work-around for us but I believe it points to a bug in the pipeline since decrypted messages are coming into the DecryptMessagesIncomingStep with all their encryption headers intact.

public class CheckDecryptionNeededStep : IIncomingStep
{
    public CheckDecryptionNeededStep() { }

    public async Task Process(IncomingStepContext context, Func<Task> next)
    {
        try
        {
            await next();
            return;
        }
        catch (CryptographicException)
        {
            // Content for failed messages is not encrypted at this step, so this exception is expected here.
            // Otherwise throw
            if (!context.Load<bool>(DefaultRetryStep.DispatchAsFailedMessageKey))
            {
                throw;
            }
        }
        // Failed message that is not encrypted, adjust header to bypass decryption in next step
        var transportMessage = context.Load<TransportMessage>();
        var headers = transportMessage.Headers.Clone();
        headers.Remove(EncryptionHeaders.ContentEncryption); // This is the key used to determine whether or not to decrypt
        context.Save(new TransportMessage(headers, transportMessage.Body));
        await next();
    }
}

[mookid8000]

Ooh good catch! 😅 Tuened out that the encryption-related headers were not removed from the transport message when it was decrypted, which caused it to be attempted decrypted again when the incoming pipeline was invoked for the 2nd time.

It's fixed in Rebus 8.4.1, which is on NuGet.org now 🙂

Thanks for reporting it!

[jgageelmresourcescom]

Got our applications updated and verified that this is fixed in the latest (8.4.2).