Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove default remote certificate callback #3329

Merged
merged 4 commits into from
May 5, 2023
Merged

Remove default remote certificate callback #3329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b0d4c5a
Remove default remote certificate callback
timtay-microsoft May 5, 2023
c40179d
asdf
timtay-microsoft May 5, 2023
cc0be98
http as well
timtay-microsoft May 5, 2023
df46f23
Revert "asdf"
timtay-microsoft May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions e2e/Tests/provisioning/ProvisioningE2ETests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -901,14 +901,7 @@ public static ProvisioningClientOptions CreateProvisioningClientOptionsFromName(

IotHubClientMqttSettings => transportSettings.Protocol == IotHubClientTransportProtocol.Tcp
? new ProvisioningClientOptions(new ProvisioningClientMqttSettings(ProvisioningClientTransportProtocol.Tcp))
: new ProvisioningClientOptions(new ProvisioningClientMqttSettings(ProvisioningClientTransportProtocol.WebSocket)
{
#if NET472
// MQTTNET doesn't support setting this callback on older .NET framework versions
// https://github.com/dotnet/MQTTnet/blob/99f4f46062611cd08e18b82515962b69e8c619e4/Source/MQTTnet/Implementations/MqttWebSocketChannel.cs#L222
RemoteCertificateValidationCallback = null,
#endif
}),
: new ProvisioningClientOptions(new ProvisioningClientMqttSettings(ProvisioningClientTransportProtocol.WebSocket)),

_ => throw new NotSupportedException($"Unknown transport: '{transportSettings}'.")
};
Expand Down
12 changes: 8 additions & 4 deletions iothub/service/src/Http/HttpClientFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,17 @@ internal static HttpClient Create(string hostName, IotHubServiceClientOptions op
{
SslProtocols = options.SslProtocols,
CheckCertificateRevocationList = options.CertificateRevocationCheck,
ServerCertificateCustomValidationCallback = (httpRequest, certificate, chain, policyErrors) =>
{
return options.RemoteCertificateValidationCallback.Invoke(httpRequest, certificate, chain, policyErrors);
},
};
#pragma warning restore CA2000 // Dispose objects before losing scope

if (options.RemoteCertificateValidationCallback != null)
{
httpMessageHandler.ServerCertificateCustomValidationCallback = (httpRequest, certificate, chain, policyErrors) =>
{
return options.RemoteCertificateValidationCallback.Invoke(httpRequest, certificate, chain, policyErrors);
};
}

if (options.Proxy != null)
{
httpMessageHandler.UseProxy = true;
Expand Down
13 changes: 1 addition & 12 deletions iothub/service/src/IotHubServiceClientOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public sealed class IotHubServiceClientOptions
/// does not support this feature.
/// </para>
/// </remarks>
public RemoteCertificateValidationCallback RemoteCertificateValidationCallback { get; set; } = DefaultRemoteCertificateValidation;
public RemoteCertificateValidationCallback RemoteCertificateValidationCallback { get; set; }

/// <summary>
/// Sets the retry policy used in the operation retries.
Expand All @@ -126,17 +126,6 @@ public sealed class IotHubServiceClientOptions
/// </remarks>
public IIotHubServiceRetryPolicy RetryPolicy { get; set; } = new IotHubServiceExponentialBackoffRetryPolicy(0, TimeSpan.FromHours(12), true);

// The default remote certificate validation callback. It returns false if any SSL level exceptions occurred
// during the handshake.
private static bool DefaultRemoteCertificateValidation(
object sender,
X509Certificate certificate,
X509Chain chain,
SslPolicyErrors sslPolicyErrors)
{
return sslPolicyErrors == SslPolicyErrors.None;
}

internal IotHubServiceClientOptions Clone()
{
return new IotHubServiceClientOptions
Expand Down
13 changes: 1 addition & 12 deletions provisioning/device/src/TransportSettings/ProvisioningClientTransportSettings.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,7 @@ public abstract class ProvisioningClientTransportSettings
        /// </para>
        /// </remarks>
        /// <seealso href="https://learn.microsoft.com/dotnet/api/system.net.websockets.clientwebsocketoptions.remotecertificatevalidationcallback"/>
public RemoteCertificateValidationCallback RemoteCertificateValidationCallback { get; set; } = DefaultRemoteCertificateValidation;

// The default remote certificate validation callback. It returns false if any SSL level exceptions occurred
// during the handshake.
internal static bool DefaultRemoteCertificateValidation(
object sender,
X509Certificate certificate,
X509Chain chain,
SslPolicyErrors sslPolicyErrors)
{
return sslPolicyErrors == SslPolicyErrors.None;
}
public RemoteCertificateValidationCallback RemoteCertificateValidationCallback { get; set; }

/// <inheritdoc/>
[EditorBrowsable(EditorBrowsableState.Never)]
Expand Down
14 changes: 0 additions & 14 deletions provisioning/device/tests/amqp/ProvisioningClientAmqpSettingsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,5 @@ public void ProvisioningClientAmqpSettings_Clone()
amqpSettings.CertificateRevocationCheck = false;
amqpSettings.Should().NotBeEquivalentTo(clone);
}

[TestMethod]
[DataRow(SslPolicyErrors.None)]
[DataRow(SslPolicyErrors.RemoteCertificateNotAvailable)]
[DataRow(SslPolicyErrors.RemoteCertificateNameMismatch)]
[DataRow(SslPolicyErrors.RemoteCertificateChainErrors)]
public void DefaultRemoteCertificateValidation_DifferentSslPolicyErrors(SslPolicyErrors sslPolicyErrors)
{
// arrange - act
var validation = ProvisioningClientTransportSettings.DefaultRemoteCertificateValidation("sender", null, null, sslPolicyErrors);

// assert
validation.Should().Be(sslPolicyErrors == SslPolicyErrors.None);
}
}
}
14 changes: 0 additions & 14 deletions provisioning/device/tests/mqtt/ProvisioningClientMqttSettingsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,5 @@ public void ProvisioningClientMqttSettings_Clone()
mqttSettings.CertificateRevocationCheck = false;
mqttSettings.Should().NotBeEquivalentTo(clone);
}

[TestMethod]
[DataRow(SslPolicyErrors.None)]
[DataRow(SslPolicyErrors.RemoteCertificateNotAvailable)]
[DataRow(SslPolicyErrors.RemoteCertificateNameMismatch)]
[DataRow(SslPolicyErrors.RemoteCertificateChainErrors)]
public void DefaultRemoteCertificateValidation_DifferentSslPolicyErrors(SslPolicyErrors sslPolicyErrors)
{
// arrange - act
var validation = ProvisioningClientTransportSettings.DefaultRemoteCertificateValidation("sender", null, null, sslPolicyErrors);

// assert
validation.Should().Be(sslPolicyErrors == SslPolicyErrors.None);
}
}
}