Feature/3902 rabbitmq mutual tls

[darrenschwarz]

Add mutual TLS (mTLS) support to RabbitMQ messaging gateways

Fixes #3902

Summary

Adds mutual TLS (mTLS) authentication support for RabbitMQ connections in both the Sync (RMQ v6) and Async (RMQ v7) messaging gateways.

Changes

Core Implementation

Added client certificate configuration to RmqMessagingGatewayConnection:

• ClientCertificate - X509Certificate2 object (for programmatic configuration)
• ClientCertificatePath - File path to .pfx/PKCS#12 certificate (for file-based configuration)
• ClientCertificatePassword - Optional password for certificate files
• TrustServerSelfSignedCertificate - Opt-in flag for test/dev environments (follows SQL Server naming convention)

New TLS Configuration Helpers:

• Added RmqTlsConfigurator classes (Async and Sync variants) to centralize TLS/SSL configuration logic
• Extracts certificate loading and SslOption configuration into reusable internal static classes
• Supports both X509Certificate2 objects and file paths with optional passwords
• Includes conditional compilation to support both .NET 8.0 (X509Certificate2) and .NET 9.0+ (X509CertificateLoader)
• Integrated with RmqMessageGateway via RmqTlsConfigurator.ConfigureIfEnabled() call

Fixed GetSanitizedUri() to handle mTLS URIs that don't contain username/password credentials (adds early return for URIs without @ symbol; existing username/password sanitization logic unchanged and backward compatible)

Cross-Gateway Parity

Both Sync and Async gateways have identical mTLS implementations, maintaining the established pattern of feature parity between RMQ v6 and v7 clients.

Bug Fixes

Pre-existing RabbitMQ Bugs (Discovered During Testing)

While testing the mTLS implementation, CI revealed pre-existing bugs in the RabbitMQ message requeuing logic:

1. Missing NOT operator in AddUserDefinedHeaders (SYNC version)

• Issue: The SYNC version of AddUserDefinedHeaders was missing the ! negation operator that exists in the ASYNC version
• Impact: Only system headers were being copied during requeue instead of user-defined headers, causing message body loss
• Fix: Changed if (_headersToReset.Contains(header.Key)) to if (!_headersToReset.Contains(header.Key))
• Commit: 15acf14

These fixes bring the SYNC implementation in line with the ASYNC version.

mTLS Implementation Bugs (Fixed in This PR)

1. Certificate Validation Hostname Mismatch

• Issue: Server certificate is issued for hostname "rabbitmq-mtls" but tests connect to "localhost", causing SSL name mismatch errors
• Impact: All 18 Docker-dependent mTLS tests failed with AuthenticationException: The remote certificate was rejected by the provided RemoteCertificateValidationCallback
• Fix: Updated RmqTlsConfigurator (both Async and Sync) to accept both RemoteCertificateChainErrors AND RemoteCertificateNameMismatch when TrustServerSelfSignedCertificate is enabled
• Files: src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqTlsConfigurator.cs:60-61, src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqTlsConfigurator.cs:60-61

2. Race Condition in Sync Consumer

• Issue: RmqMessageConsumer.CreateConsumer() was manually calling HandleBasicConsumeOk() after BasicConsume(), but RabbitMQ client already calls this method automatically as a callback
• Impact: Intermittent InvalidOperationException: Operations that change non-concurrent collections must have exclusive access due to concurrent modifications to non-thread-safe HashSet in DefaultBasicConsumer
• Fix: Removed the redundant manual call to HandleBasicConsumeOk() and added explanatory comment
• Files: src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageConsumer.cs:380-381

Testing

All 32 mTLS tests (16 async + 16 sync) pass consistently.

Quick Test Commands (3 minutes total)

Run all 32 mTLS tests (most efficient):

dotnet test tests/Paramore.Brighter.RMQ.Async.Tests/Paramore.Brighter.RMQ.Async.Tests.csproj --filter "Category=MutualTLS" --framework net10.0 && \
dotnet test tests/Paramore.Brighter.RMQ.Sync.Tests/Paramore.Brighter.RMQ.Sync.Tests.csproj --filter "Category=MutualTLS" --framework net10.0

OR run separately by category:

# 1. Unit/configuration tests only (14 tests, no Docker required, ~5 seconds)
dotnet test tests/Paramore.Brighter.RMQ.Async.Tests/Paramore.Brighter.RMQ.Async.Tests.csproj \
  --filter "Category=MutualTLS&Requires!=Docker-mTLS" --framework net10.0

dotnet test tests/Paramore.Brighter.RMQ.Sync.Tests/Paramore.Brighter.RMQ.Sync.Tests.csproj \
  --filter "Category=MutualTLS&Requires!=Docker-mTLS" --framework net10.0

# 2. Acceptance/observability tests (18 tests, requires Docker, ~3 minutes)
# First time setup:
cd tests
./generate-test-certs.sh
docker-compose -f docker-compose.rabbitmq-mtls.yml up -d
cd ..

# Run Docker-dependent tests:
dotnet test tests/Paramore.Brighter.RMQ.Async.Tests/Paramore.Brighter.RMQ.Async.Tests.csproj \
  --filter "Requires=Docker-mTLS" --framework net10.0

dotnet test tests/Paramore.Brighter.RMQ.Sync.Tests/Paramore.Brighter.RMQ.Sync.Tests.csproj \
  --filter "Requires=Docker-mTLS" --framework net10.0

Why These Commands Are Fast

• Targets specific test projects - Only builds RMQ-related projects (not the entire solution)
• Single framework - Uses --framework net10.0 instead of testing on net8.0, net9.0, net10.0
• Precise filtering - Only runs the 32 mTLS tests, not all RMQ tests
• 3 minutes vs 3-5 hours - Avoids building superfluous projects

Test Coverage

Added comprehensive test coverage across four categories:

Configuration Tests (14): Verify SSL plumbing, certificate loading edge cases, and validation logic

• Certificate object configuration
• File path configuration with passwords
• Invalid certificate handling
• Missing file error handling
• SSL option configuration

Acceptance Tests (4): Verify actual mTLS connections against Docker RabbitMQ

• Basic publish with client certificate
• Round-trip publish and consume

Observability Tests (10): Verify W3C Trace Context (TraceParent, TraceState, Baggage) and CloudEvents trace context survive mTLS connections

• TraceParent header preservation
• TraceState and Baggage preservation
• BrighterTracer.WriteProducerEvent integration
• CloudEvents trace context serialization

Quorum Queue Tests (4): Verify mTLS works with RabbitMQ quorum queues

• Trace context with quorum queues
• Baggage with quorum queues

All tests include both async and sync variants to ensure cross-gateway uniformity.

Test Stability

• Tests use [Collection("RabbitMQ mTLS", DisableParallelization = true)] to prevent TLS handshake race conditions
• Docker-dependent tests are marked with [Trait("Requires", "Docker-mTLS")] for selective execution
• All tests pass consistently on .NET 8.0, 9.0, and 10.0

Infrastructure

• Added Docker Compose configuration for RabbitMQ with mTLS enabled (tests/docker-compose.rabbitmq-mtls.yml)
• Added RabbitMQ mTLS configuration file (tests/rabbitmq-mtls.conf)
• Added certificate generation script (tests/generate-test-certs.sh) for creating test CA, server, and client certificates
• Updated .gitignore to exclude generated test certificates

Test Harness

Added a simple Web API sample in samples/WebAPI/WebAPI_mTLS_TestHarness/ that demonstrates:

• Publishing and consuming messages over RabbitMQ with mTLS
• Both producer and consumer in a single application
• REST API with Swagger UI for easy testing

This provides a working example of the mTLS configuration and verifies end-to-end functionality.

Usage Example

// Option 1: Provide certificate object directly
var cert = new X509Certificate2("client-cert.pfx", "password");
var connection = new RmqMessagingGatewayConnection
{
    AmpqUri = new AmqpUriSpecification(new Uri("amqps://rabbitmq.example.com:5671")),
    Exchange = new Exchange("my.exchange"),
    ClientCertificate = cert,
    TrustServerSelfSignedCertificate = true // For dev/test only
};

// Option 2: Load from file path
var connection = new RmqMessagingGatewayConnection
{
    AmpqUri = new AmqpUriSpecification(new Uri("amqps://rabbitmq.example.com:5671")),
    Exchange = new Exchange("my.exchange"),
    ClientCertificatePath = "/path/to/client-cert.pfx",
    ClientCertificatePassword = "password",
    TrustServerSelfSignedCertificate = false // Production: validate properly
};

// Use with producer or consumer as normal
var producer = new RmqMessageProducer(connection);
await producer.SendAsync(message);

Files Changed

Added

• src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqTlsConfigurator.cs
• src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqTlsConfigurator.cs
• tests/docker-compose.rabbitmq-mtls.yml
• tests/rabbitmq-mtls.conf
• tests/generate-test-certs.sh
• 6 test files with 32 comprehensive test cases
• samples/WebAPI/WebAPI_mTLS_TestHarness/ (manual test harness)

Modified

• src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessageGateway.cs
• src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessagingGatewayConnection.cs
• src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageGateway.cs
• src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessagingGatewayConnection.cs
• src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessageConsumer.cs

Notes

• Certificate configuration is fully backward compatible - existing code continues to work unchanged
• TrustServerSelfSignedCertificate defaults to false for production safety
• Certificate object takes precedence over file path if both are configured
• Tests use trait-based filtering rather than environment variables per feedback on ADR

Security Considerations

• TrustServerSelfSignedCertificate = true should only be used in test/development environments
• In production, use properly signed certificates and validate the full certificate chain (TrustServerSelfSignedCertificate = false)
• Certificate passwords should be stored securely (e.g., Azure Key Vault, environment variables, not hardcoded)
• Certificates are loaded securely using the appropriate .NET APIs for the target framework

Breaking Changes

None. This is a purely additive feature. Existing code without mTLS configuration continues to work as before.

Known Issue

There is an intermittent test failure on .NET 10.0 only in CI:

• RmqMessageProducerRequeuingMessageTests.When_posting_a_message_via_the_messaging_gateway
• Passes consistently on .NET 9.0
• ASYNC version passes on both .NET 9.0 and 10.0
• SYNC now has identical logic to ASYNC

This appears to be a .NET 10.0-specific timing/environmental issue that requires further investigation. The core bug fix (missing ! operator) is correct and working.

Bug Fix: Corrected header filtering logic in sync RmqMessagePublisher

This PR includes a critical bug fix for message header handling that was discovered during mTLS testing.

Issue: In commit 76aeced5c (September 2025), a refactoring of the sync RmqMessagePublisher accidentally inverted the header filtering logic in the AddUserDefinedHeaders method:

// BEFORE (correct):
if (!_headersToReset.Any(htr => htr.Equals(header.Key)))
    headers.Add(header.Key, header.Value);

// AFTER (incorrect - missing NOT operator):
if (_headersToReset.Contains(header.Key))
{
    headers[header.Key] = header.Value;
}

Impact: This caused only Brighter's internal system headers (MESSAGE_TYPE, TOPIC, HANDLED_COUNT, DELIVERY_TAG, CORRELATION_ID) to be copied during message publishing and requeuing, while user-defined headers were discarded. This resulted in message body loss and data corruption during message requeuing operations.

Fix: Restored the correct logic by adding back the NOT operator:

// CORRECTED:
if (!_headersToReset.Contains(header.Key))
{
    headers[header.Key] = header.Value;
}

This brings the sync implementation in line with the async version (src/Paramore.Brighter.MessagingGateway.RMQ.Async/RmqMessagePublisher.cs:231), which retained the correct logic throughout the refactoring.

Discovery: This bug was discovered during mTLS acceptance testing when messages were being requeued and custom headers were not preserved, causing test failures.

Files Changed:

• src/Paramore.Brighter.MessagingGateway.RMQ.Sync/RmqMessagePublisher.cs (line 197)

---

Note on Race Condition Fix

The race condition fix in RmqMessageConsumer.CreateConsumer() (removal of manual HandleBasicConsumeOk() call) has been reverted from this PR and will be addressed in a separate issue for better scope management. See RACE_CONDITION_ISSUE.md for details on creating the tracking issue.

---

[iancooper]

Likely back on Brighter stuff tomorrow

[iancooper]

@darrenschwarz I am done commenting on this, for now, if you want to move it on.

[darrenschwarz]

@darrenschwarz I am done commenting on this, for now, if you want to move it on.

All comments have been addressed, ready for review.

[codescene-delta-analysis]

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

[iancooper]

Thanks for adding the samples, we appreciate it when new features come with examples of how to use them

[iancooper]

Is this still reversing, though? The logic is that we only want to reset certain headers from a list. If they are in the list, we overwrite them. If we use the ! operator, this means reset if the list does not contain the header.

[iancooper]

@darrenschwarz Even I am getting confused as to whether the sign reversal was fixed now

[darrenschwarz]

@darrenschwarz Even I am getting confused as to whether the sign reversal was fixed now

So the fix here is correct, unless I misunderstand the intention. This will ensure that we respect any user defined headers, and we are checking against the static system/blocked list of headers (_headersToReset).

I'll run through with you when we catchup. If the fix is good, then I was thinking that we might want to do some minor renaming to make intention more apparent.

[iancooper]

So the fix here is correct, unless I misunderstand the intention. This will ensure that we respect any user defined headers, and we are checking against the static system/blocked list of headers (_headersToReset).

I'll take a look. I need to refresh my memory of why we reset some headers.

Arguably the fix is out-of-scope for a change to mTLS. Now, human authors do this, because sometimes we stumble over a bug, and need to fix it to progress. I guess, when reviewing an agent's code, it's harder to see intent - why did you change this? - why did it help your code work.

[darrenschwarz]

So the fix here is correct, unless I misunderstand the intention. This will ensure that we respect any user defined headers, and we are checking against the static system/blocked list of headers (_headersToReset).

I'll take a look. I need to refresh my memory of why we reset some headers.

Arguably the fix is out-of-scope for a change to mTLS. Now, human authors do this, because sometimes we stumble over a bug, and need to fix it to progress. I guess, when reviewing an agent's code, it's harder to see intent - why did you change this? - why did it help your code work.

This was actually a human in the loop instance, I cam across it because tests were failing. :)

[darrenschwarz]

So the fix here is correct, unless I misunderstand the intention. This will ensure that we respect any user defined headers, and we are checking against the static system/blocked list of headers (_headersToReset).

I'll take a look. I need to refresh my memory of why we reset some headers.
Arguably the fix is out-of-scope for a change to mTLS. Now, human authors do this, because sometimes we stumble over a bug, and need to fix it to progress. I guess, when reviewing an agent's code, it's harder to see intent - why did you change this? - why did it help your code work.

This was actually a human in the loop instance, I cam across it because tests were failing. :)

I'm happy to revert this and create another issue, and submit the fix as an isolated item.

[iancooper]

I'm happy to revert this and create another issue, and submit the fix as an isolated item.

Nah, all good. Only gave the feedback in case it was an agent