Skip to content

Commit 347b16a

Browse files
bordingtimbussmann
andauthored
Remote certificate validation can't be disabled when accessing the management API (#1600)
* Apply certificate validation setting to management client * Add delay to test to prevent failure --------- Co-authored-by: Tim Bussmann <[email protected]>
1 parent 07c00a9 commit 347b16a

File tree

6 files changed

+20
-7
lines changed

6 files changed

+20
-7
lines changed

src/NServiceBus.Transport.RabbitMQ.CommandLine/BrokerConnectionBinder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ protected override BrokerConnection GetBoundValue(BindingContext bindingContext)
2525
var connectionConfiguration = ConnectionConfiguration.Create(connectionString);
2626
var managementApiConfiguration = ManagementApiConfiguration.Create(managementApiUrl, managementApiUserName, managementApiPassword);
2727

28-
var managementClient = new ManagementClient(connectionConfiguration, managementApiConfiguration);
28+
var managementClient = new ManagementClient(connectionConfiguration, managementApiConfiguration, disableCertificateValidation);
2929
var brokerVerifier = new BrokerVerifier(managementClient, true);
3030

3131
var certificateCollection = new X509Certificate2Collection();

src/NServiceBus.Transport.RabbitMQ.CommandLine/BrokerVerifierBinder.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
using System.CommandLine.Binding;
55
using NServiceBus.Transport.RabbitMQ.ManagementApi;
66

7-
class BrokerVerifierBinder(Option<string> connectionStringOption, Option<string> connectionStringEnvOption, Option<string> managementApiUrlOption, Option<string> managementApiUserNameOption, Option<string> managementApiPasswordOption) : BinderBase<BrokerVerifier>
7+
class BrokerVerifierBinder(Option<string> connectionStringOption, Option<string> connectionStringEnvOption, Option<string> managementApiUrlOption, Option<string> managementApiUserNameOption, Option<string> managementApiPasswordOption, Option<bool> disableCertificateValidationOption) : BinderBase<BrokerVerifier>
88
{
99
protected override BrokerVerifier GetBoundValue(BindingContext bindingContext)
1010
{
@@ -13,13 +13,14 @@ protected override BrokerVerifier GetBoundValue(BindingContext bindingContext)
1313
var managementApiUrl = bindingContext.ParseResult.GetValueForOption(managementApiUrlOption);
1414
var managementApiUserName = bindingContext.ParseResult.GetValueForOption(managementApiUserNameOption);
1515
var managementApiPassword = bindingContext.ParseResult.GetValueForOption(managementApiPasswordOption);
16+
var disableCertificateValidation = bindingContext.ParseResult.GetValueForOption(disableCertificateValidationOption);
1617

1718
var connectionString = GetConnectionString(connectionStringOptionValue, connectionStringEnvOptionValue);
1819

1920
var connectionConfiguration = ConnectionConfiguration.Create(connectionString);
2021
var managementApiConfiguration = ManagementApiConfiguration.Create(managementApiUrl, managementApiUserName, managementApiPassword);
2122

22-
var managementClient = new ManagementClient(connectionConfiguration, managementApiConfiguration);
23+
var managementClient = new ManagementClient(connectionConfiguration, managementApiConfiguration, disableCertificateValidation);
2324
var brokerVerifier = new BrokerVerifier(managementClient, true);
2425

2526
return brokerVerifier;

src/NServiceBus.Transport.RabbitMQ.CommandLine/SharedOptions.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,16 @@ public static BrokerVerifierBinder CreateBrokerVerifierBinderWithOptions(Command
160160
var managementApiUrlOption = CreateManagementApiUrlOption();
161161
var managementApiUserNameOption = CreateManagementApiUserNameOption();
162162
var managementApiPasswordOption = CreateManagementApiPasswordOption();
163+
var disableCertValidationOption = CreateDisableCertValidationOption();
163164

164165
command.AddOption(connectionStringOption);
165166
command.AddOption(connectionStringEnvOption);
166167
command.AddOption(managementApiUrlOption);
167168
command.AddOption(managementApiUserNameOption);
168169
command.AddOption(managementApiPasswordOption);
170+
command.AddOption(disableCertValidationOption);
169171

170-
return new BrokerVerifierBinder(connectionStringOption, connectionStringEnvOption, managementApiUrlOption, managementApiUserNameOption, managementApiPasswordOption);
172+
return new BrokerVerifierBinder(connectionStringOption, connectionStringEnvOption, managementApiUrlOption, managementApiUserNameOption, managementApiPasswordOption, disableCertValidationOption);
171173
}
172174

173175
public static RoutingTopologyBinder CreateRoutingTopologyBinderWithOptions(Command command)

src/NServiceBus.Transport.RabbitMQ.Tests/BrokerVerifierTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@ public async Task ValidateDeliveryLimit_Should_Throw_When_A_Policy_On_Queue_Has_
124124

125125
await managementClient.CreatePolicy(policyName, policy).ConfigureAwait(false);
126126

127-
// If this test appears flaky, a delay should be added here to give the broker some time to apply the policy before calling ValidateDeliveryLimit
127+
// If this test appears flaky, the delay should be increased to give the broker more time to apply the policy before calling ValidateDeliveryLimit
128+
await Task.Delay(TimeSpan.FromSeconds(30));
128129

129130
var exception = Assert.ThrowsAsync<InvalidOperationException>(async () => await brokerVerifier.ValidateDeliveryLimit(queueName));
130131
Assert.That(exception.Message, Does.Contain($"The delivery limit for '{queueName}' is set to the non-default value of '{deliveryLimit}'. Remove any delivery limit settings from queue arguments, user policies or operator policies to correct this."));

src/NServiceBus.Transport.RabbitMQ/Administration/ManagementApi/ManagementClient.cs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace NServiceBus.Transport.RabbitMQ.ManagementApi;
77
using System.Net;
88
using System.Net.Http;
99
using System.Net.Http.Json;
10+
using System.Net.Security;
1011
using System.Threading;
1112
using System.Threading.Tasks;
1213

@@ -20,7 +21,7 @@ class ManagementClient : IDisposable
2021

2122
bool disposed;
2223

23-
public ManagementClient(ConnectionConfiguration connectionConfiguration, ManagementApiConfiguration? managementApiConfiguration = null)
24+
public ManagementClient(ConnectionConfiguration connectionConfiguration, ManagementApiConfiguration? managementApiConfiguration = null, bool disableRemoteCertificateValidation = false)
2425
{
2526
ArgumentNullException.ThrowIfNull(connectionConfiguration);
2627

@@ -60,6 +61,14 @@ public ManagementClient(ConnectionConfiguration connectionConfiguration, Managem
6061
PreAuthenticate = true
6162
};
6263

64+
if (disableRemoteCertificateValidation)
65+
{
66+
handler.SslOptions = new SslClientAuthenticationOptions
67+
{
68+
RemoteCertificateValidationCallback = (_, _, _, _) => true
69+
};
70+
}
71+
6372
httpClient = new HttpClient(handler) { BaseAddress = uriBuilder.Uri };
6473
}
6574

src/NServiceBus.Transport.RabbitMQ/RabbitMQTransport.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ public override async Task<TransportInfrastructure> Initialize(HostSettings host
227227
additionalClusterNodes
228228
);
229229

230-
ManagementClient = new ManagementClient(ConnectionConfiguration, ManagementApiConfiguration);
230+
ManagementClient = new ManagementClient(ConnectionConfiguration, ManagementApiConfiguration, !ValidateRemoteCertificate);
231231

232232
var brokerVerifier = new BrokerVerifier(ManagementClient, ValidateDeliveryLimits);
233233
await brokerVerifier.Initialize(cancellationToken).ConfigureAwait(false);

0 commit comments

Comments
 (0)