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

update SSL tests to deal better with disabled protocols #65120

Merged
merged 15 commits into from
Feb 17, 2022
Merged

update SSL tests to deal better with disabled protocols #65120

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9f2b1a1
update SSL tests to deal better with disabled protocols
wfurt Feb 10, 2022
318518c
Improve detection of Null encryption on Windows
wfurt Feb 10, 2022
6dddb96
update expectation for Mismatched protocols
wfurt Feb 10, 2022
36b4b36
update detection
wfurt Feb 11, 2022
bd6ee9c
Merge branch 'sslTests_65098' of https://github.com/dotnet/runtime in…
wfurt Feb 11, 2022
429d7da
wrap win32 exception
wfurt Feb 14, 2022
d1ad211
Merge branch 'main' of https://github.com/dotnet/runtime into sslTest…
wfurt Feb 14, 2022
212fa5b
update ProtocolMismatchData sets
wfurt Feb 15, 2022
76a485a
remove debug print
wfurt Feb 15, 2022
c4add0d
final cleanup
wfurt Feb 15, 2022
371f08b
Merge branch 'main' of https://github.com/dotnet/runtime into sslTest…
wfurt Feb 15, 2022
bc8c739
Merge branch 'main' into sslTests_65098
wfurt Feb 15, 2022
34cfde3
generate mismatch data
wfurt Feb 15, 2022
c5eb617
Merge branch 'main' of https://github.com/dotnet/runtime into sslTest…
wfurt Feb 15, 2022
63e9638
avoid SslProtocols.Default
wfurt Feb 16, 2022
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
23 changes: 17 additions & 6 deletions src/libraries/System.Net.Security/tests/FunctionalTests/ClientAsyncAuthenticateTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,23 @@ public static IEnumerable<object[]> ProtocolMismatchData()
yield return new object[] { SslProtocols.Ssl2, SslProtocols.Tls12, typeof(Exception) };
yield return new object[] { SslProtocols.Ssl3, SslProtocols.Tls12, typeof(Exception) };
#pragma warning restore 0618
yield return new object[] { SslProtocols.Tls, SslProtocols.Tls11, typeof(AuthenticationException) };
yield return new object[] { SslProtocols.Tls, SslProtocols.Tls12, typeof(AuthenticationException) };
yield return new object[] { SslProtocols.Tls11, SslProtocols.Tls, typeof(AuthenticationException) };
yield return new object[] { SslProtocols.Tls12, SslProtocols.Tls, typeof(AuthenticationException) };
yield return new object[] { SslProtocols.Tls12, SslProtocols.Tls11, typeof(AuthenticationException) };
yield return new object[] { SslProtocols.Tls11, SslProtocols.Tls12, typeof(AuthenticationException) };
if (PlatformDetection.SupportsTls10)
{
yield return new object[] { SslProtocols.Tls, SslProtocols.Tls11, PlatformDetection.SupportsTls11 ? typeof(AuthenticationException) : typeof(IOException) };
yield return new object[] { SslProtocols.Tls, SslProtocols.Tls12, PlatformDetection.SupportsTls12 ? typeof(AuthenticationException) : typeof(IOException) };
}

if (PlatformDetection.SupportsTls11)
{
yield return new object[] { SslProtocols.Tls11, SslProtocols.Tls, PlatformDetection.SupportsTls10 ? typeof(AuthenticationException) : typeof(IOException) };
yield return new object[] { SslProtocols.Tls11, SslProtocols.Tls12, PlatformDetection.SupportsTls12 ? typeof(AuthenticationException) : typeof(IOException) };
}

if (PlatformDetection.SupportsTls12)
{
yield return new object[] { SslProtocols.Tls12, SslProtocols.Tls11, PlatformDetection.SupportsTls11 ? typeof(AuthenticationException) : typeof(IOException) };
yield return new object[] { SslProtocols.Tls12, SslProtocols.Tls, PlatformDetection.SupportsTls10 ? typeof(AuthenticationException) : typeof(IOException) };
wfurt marked this conversation as resolved.
Show resolved Hide resolved
}
}

#region Helpers
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Net.Security/tests/FunctionalTests/LoggingTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void EventSource_ExistsWithCorrectId()
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
public void EventSource_EventsRaisedAsExpected()
{
if (PlatformDetection.IsWindows10Version22000OrGreater)
if (PlatformDetection.IsWindows10Version20348OrGreater)
{
// [ActiveIssue("https://github.com/dotnet/runtime/issues/58927")]
throw new SkipTestException("Unstable on Windows 11");
Expand Down
12 changes: 9 additions & 3 deletions src/libraries/System.Net.Security/tests/FunctionalTests/ServerAsyncAuthenticateTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;

using Microsoft.DotNet.XUnitExtensions;
using Xunit;
using Xunit.Abstractions;

Expand Down Expand Up @@ -43,13 +43,19 @@ public async Task ServerAsyncAuthenticate_EachSupportedProtocol_Success(SslProto
await ServerAsyncSslHelper(protocol, protocol);
}

[Theory]
[ConditionalTheory]
[MemberData(nameof(ProtocolMismatchData))]
public async Task ServerAsyncAuthenticate_MismatchProtocols_Fails(
SslProtocols serverProtocol,
SslProtocols clientProtocol,
Type expectedException)
{

if ((serverProtocol & SslProtocolSupport.SupportedSslProtocols) == 0)
{
throw new SkipTestException($"None of '{serverProtocol}' requested versions is available");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind the check, but this shouldn't happen should it? Based on the checks in ProtocolMismatchData. I'm just trying to make sure I understand this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two parts. The one side is guarded but the other is not. So we would not start server on unsupported version but the client can be anything.

We could possibly change that as well. We have tests where the client part is set of allowed protocols and the test would work as far as any of them is available. It is also somewhat more complicated as the protocol may not be supported by the SSL stack but on this case the server does support but it is disabled in registry. And when it does, some of the call fail with WIn32Exception. There is also some variations I run into on Linux: The protocols may not be disabled explicitly but all the ciphers suites used by it may - as deemed weak. In that case the API calls succeed but then the negotiation fails with protocol mismatch.

Perhaps we should construct this automatically e.g. create disjoined sets from all supported protocols.

}

Exception e = await Record.ExceptionAsync(
() =>
{
Expand Down Expand Up @@ -236,7 +242,7 @@ public async Task ServerAsyncAuthenticate_ConstructorVerificationDelegate_Succes

(Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams();
var client = new SslStream(clientStream);
var server = new SslStream(serverStream, false, (sender, certificate, chain, sslPolicyErrors) => { validationCallbackCalled = true; return true;});
var server = new SslStream(serverStream, false, (sender, certificate, chain, sslPolicyErrors) => { validationCallbackCalled = true; return true; });

using (client)
using (server)
Expand Down
5 changes: 4 additions & 1 deletion src/libraries/System.Net.Security/tests/FunctionalTests/ServerNoEncryptionTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ public async Task ServerNoEncryption_ClientNoEncryption_ConnectWithNoEncryption(
else
{
var ae = await Assert.ThrowsAsync<AuthenticationException>(() => sslStream.AuthenticateAsClientAsync("localhost", null, SslProtocolSupport.DefaultSslProtocols, false));
Assert.IsType<PlatformNotSupportedException>(ae.InnerException);
if (!OperatingSystem.IsWindows())
{
Assert.IsType<PlatformNotSupportedException>(ae.InnerException);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this correlates anyhow with the new check for IsWindows10Version20348OrGreater in s_supportsNullEncryption?
If so, should it be the same check here as well? Or is this unrelated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessarily. I'm not sure where the expected PlatformNotSupportedException would come from. On windows when the protocol are disabled in registry we get API failure. I mostly disabled this to pass the test so we have time to investigate. My intention is to stabilize the tests but leave #65098 open so we can investigate more.

}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/libraries/System.Net.Security/tests/FunctionalTests/TestConfiguration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public static Task WhenAllOrAnyFailedWithTimeout(params Task[] tasks)
// On Windows, null ciphers (no encryption) are supported.
if (OperatingSystem.IsWindows())
{
return true;
// This may be more complicated but Server 2022 and Windows 11 some with restricted set of default ciphers.
wfurt marked this conversation as resolved.
Show resolved Hide resolved
return !PlatformDetection.IsWindows10Version20348OrGreater;
}

// On macOS and Android, the null cipher (no encryption) is not supported.
Expand Down