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.

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

[release/7.0] Use platform runtime check for ProtectedData #80196

Merged

Conversation

vcsjones
Copy link
Member

@vcsjones vcsjones commented Jan 4, 2023

Backport of #80158 to release/7.0

Description

Reported by a customer in #78875. The System.Security.Cryptography.ProtectedData package erroneously throws a PlatformNotSupportedException on Windows for non .NETCoreApp targets, like UWP, which use the .NETStandard targets.

This changes the package from using target platform identifiers to using runtime checks to determine if the platform is Windows. Using target platform identifiers with .NET Standard was removed in .NET 7, so the checks are now performed at runtime.

Customer Impact

Customers are unable to use version 7.0.0 of this package on Windows in a UWP project, or other .NET platforms that are not a .NETCoreApp target.

Regression

This is a regression from the 6.0.x version of the package, which works correctly on Windows with .NET Standard.

Testing

Manually verified the package contents and that it can be used from a UWP project.

Risk

Medium-low. The tests verify the code is behaving as-expected both on Windows and non-Windows systems, but the change also simplifies the target framework of the package, which may have unexpected consequences in build-over-build validation tools.

Package authoring signed off?

Required. This services System.Security.Cryptography.ProtectedData.

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

@ghost
Copy link

ghost commented Jan 4, 2023

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #80158 to release/7.0

Description

Reported by a customer in #78875. The System.Security.Cryptography.ProtectedData package erroneously throws a PlatformNotSupportedException on Windows for non .NETCoreApp targets, like UWP, which use the .NETStandard targets.

This changes the package from using target platform identifiers to using runtime checks to determine if the platform is Windows. Using target platform identifiers with .NET Standard was removed in .NET 7, so the checks are now performed at runtime.

Customer Impact

Customers are unable to use version 7.0.0 of this package on Windows in a UWP project, or other .NET platforms that are not a .NETCoreApp target.

Regression

This is a regression from the 6.0.x version of the package, which works correctly on Windows with .NET Standard.

Testing

Manually verified the package contents and that it can be used from a UWP project.

Risk

Uncertain. This adjusts the package's TargetFrameworks. On the other hand, the changes are not complex and overall simplify the structure of the package. This also adds a dependency on System.Memory for .NETStandard.

I will defer to @bartonjs and/or @ViktorHofer to provide input here.

Package authoring signed off?

Required. This services System.Security.Cryptography.ProtectedData.

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author: vcsjones
Assignees: -
Labels:

area-System.Security

Milestone: -

@carlossanlop
Copy link
Member

@bartonjs when/if this is ready, can you please add the servicing-consider label, then send an email to Tactics requesting approval?

@bartonjs bartonjs added the Servicing-consider Issue for next servicing release review label Jan 4, 2023
@bartonjs bartonjs added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 5, 2023
@rbhanda rbhanda added this to the 7.0.3 milestone Jan 5, 2023
@carlossanlop
Copy link
Member

Approved by Tactics (7.0.3).
Signed off by area owners.
CI failure is known/unrelated/fixed: #79097
Required OOB package change looks good.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 05202cf into dotnet:release/7.0 Jan 5, 2023
@vcsjones vcsjones deleted the backport-80158-to-release-7.0 branch January 5, 2023 23:00
@ghost ghost locked as resolved and limited conversation to collaborators Feb 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants