Skip to content

Enable ExtendedProtectionPolicy in generic System.Net.Security build #88871

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

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Enable ExtendedProtectionPolicy in generic System.Net.Security build #88871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f38673
Enable ExtendedProtectionPolicy in generic System.Net.Security build …
filipnavara Jul 13, 2023
fc49dec
Update tests
filipnavara Jul 17, 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
2 changes: 2 additions & 0 deletions src/libraries/System.Net.Security/src/ExcludeApiList.PNSE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy
System.Security.Authentication.ExtendedProtection.ServiceNameCollection
15 changes: 9 additions & 6 deletions src/libraries/System.Net.Security/src/System.Net.Security.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<PropertyGroup>
<TargetPlatformIdentifier>$([MSBuild]::GetTargetPlatformIdentifier('$(TargetFramework)'))</TargetPlatformIdentifier>
<GeneratePlatformNotSupportedAssemblyMessage Condition="'$(TargetPlatformIdentifier)' == ''">SR.SystemNetSecurity_PlatformNotSupported</GeneratePlatformNotSupportedAssemblyMessage>
<ApiExclusionListPath Condition="'$(TargetPlatformIdentifier)' == ''">ExcludeApiList.PNSE.txt</ApiExclusionListPath>
<DefineConstants Condition="'$(TargetPlatformIdentifier)' == 'windows'">$(DefineConstants);TARGET_WINDOWS</DefineConstants>
<DefineConstants Condition="'$(TargetPlatformIdentifier)' == 'android'">$(DefineConstants);TARGET_ANDROID</DefineConstants>
<UseAndroidCrypto Condition="'$(TargetPlatformIdentifier)' == 'android'">true</UseAndroidCrypto>
Expand All @@ -18,6 +19,14 @@
<GenAPIExcludeApiList>ReferenceAssemblyExclusions.txt</GenAPIExcludeApiList>
</PropertyGroup>
<Import Project="$(CommonPath)System\Security\Cryptography\Asn1Reader\System.Security.Cryptography.Asn1Reader.Shared.projitems" Condition="'$(UseManagedNtlm)' == 'true'" />
<ItemGroup>
<Compile Include="System\Security\Authentication\ExtendedProtection\ExtendedProtectionPolicy.cs" />
<Compile Include="System\Security\Authentication\ExtendedProtection\ServiceNameCollection.cs" />
<Compile Include="$(CommonPath)System\Obsoletions.cs"
Link="Common\System\Obsoletions.cs" />
<Compile Include="$(CommonPath)System\Net\UriScheme.cs"
Link="Common\System\Net\UriScheme.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetPlatformIdentifier)' != ''">
<Compile Include="System\Net\CertificateValidationPal.cs" />
<Compile Include="System\Net\SslStreamContext.cs" />
Expand Down Expand Up @@ -53,10 +62,8 @@
<Compile Include="System\Net\NTAuthentication.cs" />
<Compile Include="System\Net\StreamFramer.cs" />
<Compile Include="System\Net\Security\NegotiateStream.cs" />
<Compile Include="System\Security\Authentication\ExtendedProtection\ExtendedProtectionPolicy.cs" />
<Compile Include="System\Security\Authentication\ExtendedProtection\PolicyEnforcement.cs" />
<Compile Include="System\Security\Authentication\ExtendedProtection\ProtectionScenario.cs" />
<Compile Include="System\Security\Authentication\ExtendedProtection\ServiceNameCollection.cs" />
<!-- IP parser -->
<Compile Include="$(CommonPath)System\Net\IPv4AddressHelper.Common.cs"
Link="System\Net\IPv4AddressHelper.Common.cs" />
Expand Down Expand Up @@ -87,8 +94,6 @@
Link="Common\System\Net\ExceptionCheck.cs" />
<Compile Include="$(CommonPath)System\Net\SecurityProtocol.cs"
Link="Common\System\Net\SecurityProtocol.cs" />
<Compile Include="$(CommonPath)System\Net\UriScheme.cs"
Link="Common\System\Net\UriScheme.cs" />
<!-- Common -->
<Compile Include="$(CommonPath)System\NotImplemented.cs"
Link="Common\System\NotImplemented.cs" />
Expand All @@ -106,8 +111,6 @@
Condition="'$(UseManagedNtlm)' != 'true'" />
<Compile Include="$(CommonPath)System\HexConverter.cs"
Link="Common\System\HexConverter.cs" />
<Compile Include="$(CommonPath)System\Obsoletions.cs"
Link="Common\System\Obsoletions.cs" />
<Compile Include="$(CommonPath)Interop\Windows\SChannel\Interop.SECURITY_STATUS.cs"
Link="Common\Interop\Windows\SChannel\Interop.SECURITY_STATUS.cs" />
</ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion ...ecurity/src/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicy.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public static bool OSSupportsExtendedProtection
get
{
// .NET Core is supported only on Win7+ where ExtendedProtection is supported.
return true;
return OperatingSystem.IsWindows();
Copy link
Member Author

@filipnavara filipnavara Jul 13, 2023

Choose a reason for hiding this comment

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

This is controversial. I can change it to !OperatingSystem.IsBrowser() to be on the safe side and behave like older .NET versions did. However, the reality is that it's not really implemented on Linux/macOS either. Some bits and pieces may have effect (eg. SPN validation for NTLM Negotiation) but others are blatantly ignored (https://github.com/dotnet/runtime/pull/87930/files#r1263108882).

Copy link
Member

@mconnew mconnew Jul 15, 2023

Choose a reason for hiding this comment

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

It's better to say it's not supported when there's known gaps than to say it's supported and have the consumer believe ExtendedProtection is being used when it's actually not. It's off by default, so if you explicitly turn it on, it should actually do the expected thing.

Copy link
Member

@wfurt wfurt Jul 16, 2023

Choose a reason for hiding this comment

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

I think this is OK. I feel it is better to claim false if the functionality is not clear or complete.

}
}
}
Expand Down
10 changes: 9 additions & 1 deletion ...itTests/System/Security/Authentication/ExtendedProtection/ExtendedProtectionPolicyTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,19 @@ public void Constructor_PolicyEnforcement_MembersAreSet()
}

[Fact]
public void ExtendedProtectionPolicy_OSSupportsExtendedProtection()
[PlatformSpecific(TestPlatforms.Windows)]
public void ExtendedProtectionPolicy_OSSupportsExtendedProtection_Windows()
{
Assert.True(ExtendedProtectionPolicy.OSSupportsExtendedProtection);
}

[Fact]
[PlatformSpecific(TestPlatforms.AnyUnix)]
public void ExtendedProtectionPolicy_OSSupportsExtendedProtection_NonWindows()
{
Assert.False(ExtendedProtectionPolicy.OSSupportsExtendedProtection);
}

[Fact]
public void ExtendedProtectionPolicy_Properties()
{
Expand Down