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

ExtendedProtectionPolicy constructor throws unnecessarily on WASM #77405

Closed
mconnew opened this issue Oct 24, 2022 · 19 comments · Fixed by #88871
Closed

ExtendedProtectionPolicy constructor throws unnecessarily on WASM #77405

mconnew opened this issue Oct 24, 2022 · 19 comments · Fixed by #88871
Labels
arch-wasm WebAssembly architecture area-System.Net.Security blocking Marks issues that we want to fast track in order to unblock other important work
Milestone

Comments

@mconnew
Copy link
Member

mconnew commented Oct 24, 2022

Description

This was an issue discovered by a WCF customer. WCF tracking issue is dotnet/wcf#4942.

It's not possible to construct an instance of ExtendedProtectionPolicy on WASM. This makes it impossible to have a public API expose this type unless you return null when running in a browser. ExtendedProtectionPolicy has the ability to have a policy enforcement that it is never used, which is appropriate for platforms where extended protection isn't supported.

In addition to this problem, the property ExtendedProtectionPolicy.OSSupportsExtendedProtection always returns true with no regard to platform.

WCF exposes a property of type ExtendedProtectionPolicy to enable clients to optionally turn on the use of Extended Protection. The default value for this property is a policy where enforcement is disabled. This is constructed using this line of code:

        private static readonly ExtendedProtectionPolicy s_disabledPolicy = new ExtendedProtectionPolicy(PolicyEnforcement.Never);

In our property, when a new value is being set, we check if the new value has an enforcement policy set to Always, and if it is we check the value of ExtendedProtectionPolicy.OSSupportsExtendedProtection and throw if trying to set a policy which can't be supported.

                if (value.PolicyEnforcement == PolicyEnforcement.Always &&
                    !System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy.OSSupportsExtendedProtection)
                {
                    throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(
                        new PlatformNotSupportedException(SR.ExtendedProtectionNotSupported));
                }

If OSSupportsExtendedProtection were implemented to reflect whether extended protection, then this should be sufficient for apps to guard against trying to use extended protection where it's not supported.

Throwing an exception on construction of a policy instance regardless of enforcement policy means all public properties need to return null where previously they didn't. This changes a not null value to a sometimes null value. This also requires adding logic to interpret null to mean the same as PolicyEnforcement.Never and duplicating that logic change everywhere the instance propagates through the code base.

Reproduction Steps

In a Blazor app, create a new instance of BasicHttpBinding using the default constructor. It throws with a PlatformNotSupportedException stating "System.Net.Security is not supported on this platform."

This was customer reported, but based on my understanding of how the throwing of an exception is generated, I suspect getting ExtendedProtectionPolicy.OSSupportsExtendedProtection will throw a PlatformNotSupportedException too instead of returning false.

Expected behavior

When constructing an instance with a PolicyEnforcement value of Never or WhenSupported will not throw. These classes are just OM around describing extended protection so I would expect an exception to only be thrown when a class which accepts an ExtendedProtectionPolicy is passed one with PolicyEnforcement.Always, as WCF does. In this case, the class would never throw.

This isn't a new expectation; this is what happened on OS's earlier than Win7 where extended protection isn't supported.

Actual behavior

Constructing an ExtendedProtectionPolicy with any enforcement policy throws a PlatformNotSupportedException.

Regression?

Regression from .NET Framework and .NET Core outside of a browser. On .NET Framework, OS's prior to Windows 7 didn't support extended protection, and ExtendedProtectionPolicy.OSSupportsExtendedProtection would return false so that libraries and applications can avoid an exception being thrown by trying to use extended protection when it's not supported. The same model should work for .NET too.

Known Workarounds

Add lots of ugly logic through the WCF code base treating null as PolicyEnforcement.Never. This could also cause consuming code to throw a NullReferenceException if expecting the property to return a non-null value as it does on every other platform.

Configuration

No response

Other information

No response

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 24, 2022
@ghost
Copy link

ghost commented Oct 24, 2022

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

Issue Details

Description

This was an issue discovered by a WCF customer. WCF tracking issue is dotnet/wcf#4942.

It's not possible to construct an instance of ExtendedProtectionPolicy on WASM. This makes it impossible to have a public API expose this type unless you return null when running in a browser. ExtendedProtectionPolicy has the ability to have a policy enforcement that it is never used, which is appropriate for platforms where extended protection isn't supported.

In addition to this problem, the property ExtendedProtectionPolicy.OSSupportsExtendedProtection always returns true with no regard to platform.

WCF exposes a property of type ExtendedProtectionPolicy to enable clients to optionally turn on the use of Extended Protection. The default value for this property is a policy where enforcement is disabled. This is constructed using this line of code:

        private static readonly ExtendedProtectionPolicy s_disabledPolicy = new ExtendedProtectionPolicy(PolicyEnforcement.Never);

In our property, when a new value is being set, we check if the new value has an enforcement policy set to Always, and if it is we check the value of ExtendedProtectionPolicy.OSSupportsExtendedProtection and throw if trying to set a policy which can't be supported.

                if (value.PolicyEnforcement == PolicyEnforcement.Always &&
                    !System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy.OSSupportsExtendedProtection)
                {
                    throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(
                        new PlatformNotSupportedException(SR.ExtendedProtectionNotSupported));
                }

If OSSupportsExtendedProtection were implemented to reflect whether extended protection, then this should be sufficient for apps to guard against trying to use extended protection where it's not supported.

Throwing an exception on construction of a policy instance regardless of enforcement policy means all public properties need to return null where previously they didn't. This changes a not null value to a sometimes null value. This also requires adding logic to interpret null to mean the same as PolicyEnforcement.Never and duplicating that logic change everywhere the instance propagates through the code base.

Reproduction Steps

In a Blazor app, create a new instance of BasicHttpBinding using the default constructor. It throws with a PlatformNotSupportedException stating "System.Net.Security is not supported on this platform."

This was customer reported, but based on my understanding of how the throwing of an exception is generated, I suspect getting ExtendedProtectionPolicy.OSSupportsExtendedProtection will throw a PlatformNotSupportedException too instead of returning false.

Expected behavior

When constructing an instance with a PolicyEnforcement value of Never or WhenSupported will not throw. These classes are just OM around describing extended protection so I would expect an exception to only be thrown when a class which accepts an ExtendedProtectionPolicy is passed one with PolicyEnforcement.Always, as WCF does. In this case, the class would never throw.

This isn't a new expectation; this is what happened on OS's earlier than Win7 where extended protection isn't supported.

Actual behavior

Constructing an ExtendedProtectionPolicy with any enforcement policy throws a PlatformNotSupportedException.

Regression?

Regression from .NET Framework and .NET Core outside of a browser. On .NET Framework, OS's prior to Windows 7 didn't support extended protection, and ExtendedProtectionPolicy.OSSupportsExtendedProtection would return false so that libraries and applications can avoid an exception being thrown by trying to use extended protection when it's not supported. The same model should work for .NET too.

Known Workarounds

Add lots of ugly logic through the WCF code base treating null as PolicyEnforcement.Never. This could also cause consuming code to throw a NullReferenceException if expecting the property to return a non-null value as it does on every other platform.

Configuration

No response

Other information

No response

Author: mconnew
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@ghost
Copy link

ghost commented Nov 8, 2022

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

Issue Details

Description

This was an issue discovered by a WCF customer. WCF tracking issue is dotnet/wcf#4942.

It's not possible to construct an instance of ExtendedProtectionPolicy on WASM. This makes it impossible to have a public API expose this type unless you return null when running in a browser. ExtendedProtectionPolicy has the ability to have a policy enforcement that it is never used, which is appropriate for platforms where extended protection isn't supported.

In addition to this problem, the property ExtendedProtectionPolicy.OSSupportsExtendedProtection always returns true with no regard to platform.

WCF exposes a property of type ExtendedProtectionPolicy to enable clients to optionally turn on the use of Extended Protection. The default value for this property is a policy where enforcement is disabled. This is constructed using this line of code:

        private static readonly ExtendedProtectionPolicy s_disabledPolicy = new ExtendedProtectionPolicy(PolicyEnforcement.Never);

In our property, when a new value is being set, we check if the new value has an enforcement policy set to Always, and if it is we check the value of ExtendedProtectionPolicy.OSSupportsExtendedProtection and throw if trying to set a policy which can't be supported.

                if (value.PolicyEnforcement == PolicyEnforcement.Always &&
                    !System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy.OSSupportsExtendedProtection)
                {
                    throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(
                        new PlatformNotSupportedException(SR.ExtendedProtectionNotSupported));
                }

If OSSupportsExtendedProtection were implemented to reflect whether extended protection, then this should be sufficient for apps to guard against trying to use extended protection where it's not supported.

Throwing an exception on construction of a policy instance regardless of enforcement policy means all public properties need to return null where previously they didn't. This changes a not null value to a sometimes null value. This also requires adding logic to interpret null to mean the same as PolicyEnforcement.Never and duplicating that logic change everywhere the instance propagates through the code base.

Reproduction Steps

In a Blazor app, create a new instance of BasicHttpBinding using the default constructor. It throws with a PlatformNotSupportedException stating "System.Net.Security is not supported on this platform."

This was customer reported, but based on my understanding of how the throwing of an exception is generated, I suspect getting ExtendedProtectionPolicy.OSSupportsExtendedProtection will throw a PlatformNotSupportedException too instead of returning false.

Expected behavior

When constructing an instance with a PolicyEnforcement value of Never or WhenSupported will not throw. These classes are just OM around describing extended protection so I would expect an exception to only be thrown when a class which accepts an ExtendedProtectionPolicy is passed one with PolicyEnforcement.Always, as WCF does. In this case, the class would never throw.

This isn't a new expectation; this is what happened on OS's earlier than Win7 where extended protection isn't supported.

Actual behavior

Constructing an ExtendedProtectionPolicy with any enforcement policy throws a PlatformNotSupportedException.

Regression?

Regression from .NET Framework and .NET Core outside of a browser. On .NET Framework, OS's prior to Windows 7 didn't support extended protection, and ExtendedProtectionPolicy.OSSupportsExtendedProtection would return false so that libraries and applications can avoid an exception being thrown by trying to use extended protection when it's not supported. The same model should work for .NET too.

Known Workarounds

Add lots of ugly logic through the WCF code base treating null as PolicyEnforcement.Never. This could also cause consuming code to throw a NullReferenceException if expecting the property to return a non-null value as it does on every other platform.

Configuration

No response

Other information

No response

Author: mconnew
Assignees: -
Labels:

area-System.Net.Security, untriaged

Milestone: -

@bartonjs
Copy link
Member

bartonjs commented Nov 8, 2022

It looks like ExtendedProtectionPolicy is part of System.Net.Security instead of System.Security.

I'm guessing it's using an auto-generated PNSE on wasm. @mconnew 's suggestion of making the static supported property return false and the ctor not fail for Never/WhenSupported sounds reasonable to me.

@rzikm rzikm added the arch-wasm WebAssembly architecture label Nov 11, 2022
@ghost
Copy link

ghost commented Nov 11, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

This was an issue discovered by a WCF customer. WCF tracking issue is dotnet/wcf#4942.

It's not possible to construct an instance of ExtendedProtectionPolicy on WASM. This makes it impossible to have a public API expose this type unless you return null when running in a browser. ExtendedProtectionPolicy has the ability to have a policy enforcement that it is never used, which is appropriate for platforms where extended protection isn't supported.

In addition to this problem, the property ExtendedProtectionPolicy.OSSupportsExtendedProtection always returns true with no regard to platform.

WCF exposes a property of type ExtendedProtectionPolicy to enable clients to optionally turn on the use of Extended Protection. The default value for this property is a policy where enforcement is disabled. This is constructed using this line of code:

        private static readonly ExtendedProtectionPolicy s_disabledPolicy = new ExtendedProtectionPolicy(PolicyEnforcement.Never);

In our property, when a new value is being set, we check if the new value has an enforcement policy set to Always, and if it is we check the value of ExtendedProtectionPolicy.OSSupportsExtendedProtection and throw if trying to set a policy which can't be supported.

                if (value.PolicyEnforcement == PolicyEnforcement.Always &&
                    !System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy.OSSupportsExtendedProtection)
                {
                    throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(
                        new PlatformNotSupportedException(SR.ExtendedProtectionNotSupported));
                }

If OSSupportsExtendedProtection were implemented to reflect whether extended protection, then this should be sufficient for apps to guard against trying to use extended protection where it's not supported.

Throwing an exception on construction of a policy instance regardless of enforcement policy means all public properties need to return null where previously they didn't. This changes a not null value to a sometimes null value. This also requires adding logic to interpret null to mean the same as PolicyEnforcement.Never and duplicating that logic change everywhere the instance propagates through the code base.

Reproduction Steps

In a Blazor app, create a new instance of BasicHttpBinding using the default constructor. It throws with a PlatformNotSupportedException stating "System.Net.Security is not supported on this platform."

This was customer reported, but based on my understanding of how the throwing of an exception is generated, I suspect getting ExtendedProtectionPolicy.OSSupportsExtendedProtection will throw a PlatformNotSupportedException too instead of returning false.

Expected behavior

When constructing an instance with a PolicyEnforcement value of Never or WhenSupported will not throw. These classes are just OM around describing extended protection so I would expect an exception to only be thrown when a class which accepts an ExtendedProtectionPolicy is passed one with PolicyEnforcement.Always, as WCF does. In this case, the class would never throw.

This isn't a new expectation; this is what happened on OS's earlier than Win7 where extended protection isn't supported.

Actual behavior

Constructing an ExtendedProtectionPolicy with any enforcement policy throws a PlatformNotSupportedException.

Regression?

Regression from .NET Framework and .NET Core outside of a browser. On .NET Framework, OS's prior to Windows 7 didn't support extended protection, and ExtendedProtectionPolicy.OSSupportsExtendedProtection would return false so that libraries and applications can avoid an exception being thrown by trying to use extended protection when it's not supported. The same model should work for .NET too.

Known Workarounds

Add lots of ugly logic through the WCF code base treating null as PolicyEnforcement.Never. This could also cause consuming code to throw a NullReferenceException if expecting the property to return a non-null value as it does on every other platform.

Configuration

No response

Other information

No response

Author: mconnew
Assignees: -
Labels:

arch-wasm, area-System.Net.Security, untriaged

Milestone: -

@mconnew mconnew added the blocking Marks issues that we want to fast track in order to unblock other important work label May 25, 2023
@mconnew
Copy link
Member Author

mconnew commented May 25, 2023

Just realized I should have applied the blocking label as this is blocking the WCF client being used on WASM and there are developers wanting to do this.

@lewing lewing added this to the 8.0.0 milestone May 25, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 25, 2023
@ghost
Copy link

ghost commented May 25, 2023

Tagging subscribers to this area: @dotnet/area-microsoft-win32
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

This was an issue discovered by a WCF customer. WCF tracking issue is dotnet/wcf#4942.

It's not possible to construct an instance of ExtendedProtectionPolicy on WASM. This makes it impossible to have a public API expose this type unless you return null when running in a browser. ExtendedProtectionPolicy has the ability to have a policy enforcement that it is never used, which is appropriate for platforms where extended protection isn't supported.

In addition to this problem, the property ExtendedProtectionPolicy.OSSupportsExtendedProtection always returns true with no regard to platform.

WCF exposes a property of type ExtendedProtectionPolicy to enable clients to optionally turn on the use of Extended Protection. The default value for this property is a policy where enforcement is disabled. This is constructed using this line of code:

        private static readonly ExtendedProtectionPolicy s_disabledPolicy = new ExtendedProtectionPolicy(PolicyEnforcement.Never);

In our property, when a new value is being set, we check if the new value has an enforcement policy set to Always, and if it is we check the value of ExtendedProtectionPolicy.OSSupportsExtendedProtection and throw if trying to set a policy which can't be supported.

                if (value.PolicyEnforcement == PolicyEnforcement.Always &&
                    !System.Security.Authentication.ExtendedProtection.ExtendedProtectionPolicy.OSSupportsExtendedProtection)
                {
                    throw DiagnosticUtility.ExceptionUtility.ThrowHelperError(
                        new PlatformNotSupportedException(SR.ExtendedProtectionNotSupported));
                }

If OSSupportsExtendedProtection were implemented to reflect whether extended protection, then this should be sufficient for apps to guard against trying to use extended protection where it's not supported.

Throwing an exception on construction of a policy instance regardless of enforcement policy means all public properties need to return null where previously they didn't. This changes a not null value to a sometimes null value. This also requires adding logic to interpret null to mean the same as PolicyEnforcement.Never and duplicating that logic change everywhere the instance propagates through the code base.

Reproduction Steps

In a Blazor app, create a new instance of BasicHttpBinding using the default constructor. It throws with a PlatformNotSupportedException stating "System.Net.Security is not supported on this platform."

This was customer reported, but based on my understanding of how the throwing of an exception is generated, I suspect getting ExtendedProtectionPolicy.OSSupportsExtendedProtection will throw a PlatformNotSupportedException too instead of returning false.

Expected behavior

When constructing an instance with a PolicyEnforcement value of Never or WhenSupported will not throw. These classes are just OM around describing extended protection so I would expect an exception to only be thrown when a class which accepts an ExtendedProtectionPolicy is passed one with PolicyEnforcement.Always, as WCF does. In this case, the class would never throw.

This isn't a new expectation; this is what happened on OS's earlier than Win7 where extended protection isn't supported.

Actual behavior

Constructing an ExtendedProtectionPolicy with any enforcement policy throws a PlatformNotSupportedException.

Regression?

Regression from .NET Framework and .NET Core outside of a browser. On .NET Framework, OS's prior to Windows 7 didn't support extended protection, and ExtendedProtectionPolicy.OSSupportsExtendedProtection would return false so that libraries and applications can avoid an exception being thrown by trying to use extended protection when it's not supported. The same model should work for .NET too.

Known Workarounds

Add lots of ugly logic through the WCF code base treating null as PolicyEnforcement.Never. This could also cause consuming code to throw a NullReferenceException if expecting the property to return a non-null value as it does on every other platform.

Configuration

No response

Other information

No response

Author: mconnew
Assignees: radical
Labels:

arch-wasm, area-Microsoft.Win32, area-System.Net.Security, blocking

Milestone: 8.0.0

@lewing
Copy link
Member

lewing commented Jul 11, 2023

ping @radical

@radical
Copy link
Member

radical commented Jul 11, 2023

IIUC, for:

public static bool OSSupportsExtendedProtection
{
get
{
// .NET Core is supported only on Win7+ where ExtendedProtection is supported.
return true;
}
}

.. we want to have this property return false on `wasm.

But System.Net.Security has:

<UnsupportedOSPlatforms>browser</UnsupportedOSPlatforms>

.. which would cause the assembly to have auto-generated PNSEs.

What would be a good way to have this property return false for wasm?

  1. I tried a substitution xml, but that doesn't seem to take affect. The assembly still gets the PNSE.
  2. I tried to include a partial class with just the property for wasm, but that fails with:
    obj/System.Net.Security/Release/net8.0/System.Net.Security.notsupported.cs(710,28): error CS0102: The type 'ExtendedProtectionPolicy' already contains a definition for 'OSSupportsExtendedProtection'

cc @eerhardt

@radical
Copy link
Member

radical commented Jul 11, 2023

cc @akoeplinger @marek-safar

@eerhardt
Copy link
Member

@ericstj @ViktorHofer - is there a way to generate PNSE stubs for all the code in an assembly, but still allow a "partial" implementation for one of the APIs? As above - if we want to return false for this static bool property, and have all the other code thrown PNSE.

@ViktorHofer
Copy link
Member

Yes that's possible. See

<GenAPIExcludeApiList>ReferenceAssemblyExclusions.txt</GenAPIExcludeApiList>
for an example.

@radical
Copy link
Member

radical commented Jul 12, 2023

Yes that's possible. See

<GenAPIExcludeApiList>ReferenceAssemblyExclusions.txt</GenAPIExcludeApiList>

for an example.

Thank you! @akoeplinger also suggested the same thing. I will use this 👍

radical added a commit to radical/runtime that referenced this issue Jul 12, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 12, 2023
@radical
Copy link
Member

radical commented Jul 12, 2023

#88715 - worked locally, let's see what CI has to say about it.
Also, need to figure out the testing for this.

@mconnew
Copy link
Member Author

mconnew commented Jul 12, 2023

The proposed change is still insufficient. WCF has this API exposed on a public property which is guaranteed to be non-null and defaults to new ExtendedProtectionPolicy(PolicyEnforcement.Never);. This means instantiating an instance of our class throws. Even with this change we still won't be able to instantiate ExtendedProtectionPolicy, even when passing PolicyEnforcement.Never.

This type was added to .NET Framework when Windows Vista was still a supported OS. Windows Vista didn't support ExtendedProtection, but Windows 7 did. The original .NET Framework implementation should work fine without any automatic generation of PNSE throwing implementation as you can treat the browser the same way that Vista was.

This class doesn't carry any ExtendedProtection implementation, it's simply an OM to describe what level of protection you desire. It's up to the consumer of ExtendedProtectionPolicy to react to it. E.g. WCF has this functionality matrix:

PolicyEnforcement OSSupportsExtendedProtection Behavior
Never N/A Doesn't use ExtendedProtection features
WhenSupported true Uses ExtendedProtection features
WhenSupported false Doesn't use ExtendedProtection features
Always true Uses ExtendedProtection features
Always false Throws an exception saying it's not supported (currently says OS, we should change that to platform)

Basically the ExtendedProtectionPolicy class shouldn't be trying to enforce platform support, that's down to the individual implementations that use ExtendedProtection to enforce. It's just a description of what your policy is going to be, but has no real functionality. The only platform aware code should be OSSupportsExtendedProtection.

@radical
Copy link
Member

radical commented Jul 12, 2023

I'm not familiar with this at all. #88715 is partial attempt at:

@mconnew 's suggestion of making the static supported property return false and the ctor not fail for Never/WhenSupported sounds reasonable to me.

@bartonjs who would be a good person to implement the changes needed here? And add a test for it.

@bartonjs
Copy link
Member

@bartonjs who would be a good person to implement the changes needed here?

I don't know, since System.Net.Security is outside my area. @wfurt, maybe?

@wfurt
Copy link
Member

wfurt commented Jul 13, 2023

I can possibly tale a look, perhaps after platform complete as bug fix. Unless @filipnavara wants to jump on it.

@filipnavara
Copy link
Member

filipnavara commented Jul 13, 2023

I may look into it, but I still have some PRs in the pipeline that are waiting for review (or in some cases I need to address feedback)...

@radical radical removed their assignment Jul 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 17, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net.Security blocking Marks issues that we want to fast track in order to unblock other important work
Projects
None yet
9 participants