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

[P2] Support System.ServiceModel.Channels.SecurityBindingElement #3860

Closed
StephenBonikowsky opened this issue Aug 30, 2019 · 17 comments
Closed
Labels
WSFederation Issues related to adding support for WSFederation.
Milestone

Comments

@StephenBonikowsky
Copy link
Member

StephenBonikowsky commented Aug 30, 2019

APIs Port/Implement Public Contract Testing
property SecurityAlgorithmSuite DefaultAlgorithmSuite
  1. Merged
  1. Added
  1. Unit
  2. Scenario
property SecurityKeyEntropyMode KeyEntropyMode
  • Merged
  • Added
  • Unit
  • Scenario

API Notes


Implementation Notes
Already ported.


Public Contract Notes

  • Public Contract: System.ServiceModel.Security

Testing Notes


PR Links
DefaultAlgorithmSuite was added to PC via: #3875
KeyEntropyMode was added to PC via: #3900
Scenario tests tracked by: #4022

@krispenner
Copy link

Hi @StephenBonikowsky,

Will this also add support for the derived class AsymmetricSecurityBindingElement?

For example, below is effectively the one and only code block that is preventing us from targeting .NET Standard and running on .NET Core. I would be forever grateful if this would be supported with your upcoming release in some way.

var messageSecurity = new AsymmetricSecurityBindingElement()
{
    MessageSecurityVersion = MessageSecurityVersion.WSSecurity11WSTrustFebruary2005WSSecureConversationFebruary2005WSSecurityPolicy11,
    InitiatorTokenParameters = new X509SecurityTokenParameters { InclusionMode = SecurityTokenInclusionMode.Never },
    RecipientTokenParameters = new X509SecurityTokenParameters { InclusionMode = SecurityTokenInclusionMode.Never },
    MessageProtectionOrder = MessageProtectionOrder.SignBeforeEncrypt,
    EnableUnsecuredResponse = true,
    IncludeTimestamp = false,
    DefaultAlgorithmSuite = SecurityAlgorithmSuite.Basic128Rsa15,
};
messageSecurity.SetKeyDerivation(false);
messageSecurity.EndpointSupportingTokenParameters.Signed.Add(new X509SecurityTokenParameters());

@mconnew
Copy link
Member

mconnew commented Oct 23, 2019

@krispenner, I have a concern about whether that will enable you because you are setting the MessageProtectionOrder. That implies you are using security mode Message, but we can only support TransportWithMessageCredential (which doesn't sign or encrypt the message). Can you please provide you complete binding configuration so I can validate that there's nothing else missing.
We also don't have tests for the various MessageSecurityVersion's that are possible so we might need to fill out some functionality to support your exact use case, which means we need to write a test for your scenario. What kind of credentials are you using?

@krispenner
Copy link

@mconnew, thank you for you reply. We are using certificate based mutual authentication. We were given the code below from the party that is hosting the service in order for us to communicate with them, they offer no other binding options. We then had to move our .NET Core hosted app on Linux App Services to .NET Framework on Windows App Services because we could not implement the below when targeting .NET Standard.

var endpoint = new EndpointAddress(options.ServiceUri);

var binding = new CustomBinding()
{
    OpenTimeout = options.OpenTimeout,
    CloseTimeout = options.CloseTimeout,
    SendTimeout = options.SendTimeout,
    ReceiveTimeout = options.ReceiveTimeout,
};
                
var messageEncoding = new TextMessageEncodingBindingElement()
{
    WriteEncoding = Encoding.UTF8,
    MessageVersion = MessageVersion.Soap11,
};
binding.Elements.Add(messageEncoding);

// !! not supported in .NET Standard
var messageSecurity = new AsymmetricSecurityBindingElement()
{
    MessageSecurityVersion = MessageSecurityVersion.WSSecurity11WSTrustFebruary2005WSSecureConversationFebruary2005WSSecurityPolicy11,
    InitiatorTokenParameters = new X509SecurityTokenParameters { InclusionMode = SecurityTokenInclusionMode.Never },
    RecipientTokenParameters = new X509SecurityTokenParameters { InclusionMode = SecurityTokenInclusionMode.Never },
    MessageProtectionOrder = MessageProtectionOrder.SignBeforeEncrypt,
    EnableUnsecuredResponse = true,
    IncludeTimestamp = false,
    DefaultAlgorithmSuite = SecurityAlgorithmSuite.Basic128Rsa15,
};
messageSecurity.SetKeyDerivation(false);
messageSecurity.EndpointSupportingTokenParameters.Signed.Add(new X509SecurityTokenParameters());
binding.Elements.Add(messageSecurity);
                
var transport = new HttpsTransportBindingElement()
{
    RequireClientCertificate = true,
    MaxReceivedMessageSize = int.MaxValue,
    MaxBufferSize = int.MaxValue,
};
binding.Elements.Add(transport);

var client = new XyzClient(binding, endpoint);

client.ClientCredentials.ClientCertificate.Certificate = options.ClientCertificate;
client.ClientCredentials.ServiceCertificate.DefaultCertificate = options.ServiceCertificate;

// !! not supported in .NET Standard
client.Endpoint.Contract.ProtectionLevel = ProtectionLevel.None;
      
return client;

Note there is also a second line of code near the bottom that is also not supported in .NET Standard but I would assume as we are setting it to None it probably wouldn't make a difference. Maybe you can also comment on that piece to confirm.

// !! not supported in .NET Standard
client.Endpoint.Contract.ProtectionLevel = ProtectionLevel.None;

@mconnew
Copy link
Member

mconnew commented Oct 24, 2019

@krispenner, unfortunately I have some bad news for you. The class AsymmetricSecurityBindingElement is only used when doing full message security. I was able to get TransportWithMessageCredentials working by rewriting WCF's usage of the System.IdentityModel version of SignedXml to use the System.Cryptography.Xml version instead. Unfortunately that version doesn't support all the transforms needed to support full message security. There's potential to use a SignedXml implementation from another team, but that needs some work to support WCF's usage scenario and I haven't even begun to have a serious conversation with that team about it yet.
I wouldn't expect to be able to support your scenario until at the earliest .NET 5 release, and that's if we can get agreement and schedule to get the work done by the other team.

@krispenner
Copy link

@mconnew
Ah rats! Well thank you for letting me know, we will stick with .NET Fx then for the time being then.
Fingers crossed for .NET 5 support. It would be nice to support all WCF client features in .NET 5 so everyone could move forward.
If this topic ever comes up in your work and you remember me, please tag me in any issues that include discussions on this. If there is any voting mechanism for WCF features I would also be interested in voicing my support for this there. If there is any development I could help with please also let me know. Thanks for your time!

@StephenBonikowsky
Copy link
Member Author

Tracking Message security work with #3424.

@StephenBonikowsky StephenBonikowsky modified the milestones: 3.1, 3.1 servicing Nov 4, 2019
@StephenBonikowsky StephenBonikowsky modified the milestones: 3.1 servicing, 3.1 Dec 9, 2019
@StephenBonikowsky StephenBonikowsky removed this from the 3.1 milestone Mar 2, 2020
@krispenner
Copy link

krispenner commented Mar 4, 2020

Hi @StephenBonikowsky, it looks like all this is completed now?!! :) Can you please confirm? Could you also let me know how I can track the status of the release that will contain this feature 3.1.x?? Or provided a targeted date when this will be released? Thank you so much!

@StephenBonikowsky
Copy link
Member Author

@krispenner We are getting close. The scenario is not complete end-to-end yet, I closed a bunch of the issues tracking specific APIs that we have added to the public contract already in 3.1.
The final pieces are in the last stages of development and will be released in one of the 3.1 monthly servicing releases.
.NET Core does not release updates in April, but we should make it into one of the subsequent monthly updates.

#3871 will likely be the final Issue to be closed when we are code complete, so you can track that.

@krispenner
Copy link

Sounds great, very excited! 👍

@mconnew
Copy link
Member

mconnew commented Mar 5, 2020

@krispenner, from my memory of when I investigated your scenario, I don't expect the specific scenario you listed as working as a result of #3871. I believe usage of AsymmetricSecurityBindingElement implies full message security which we still don't have any concrete plans to be able to support.

@krispenner
Copy link

@mconnew thanks for letting me know. Are there any high-level expectations for this to be available in .NET 5 at the latest or even that target is unlikely at this point?

@mconnew
Copy link
Member

mconnew commented Mar 8, 2020

@krispenner, unfortunately our hands are tied. We depend on code which isn't in .NET Core to achieve message security. There is a potential path to bring the feature, but I'll be honest it will be a while before we can consider thinking about trying to get everything lined up for that. We have a couple of other big features we're lining up right now which will have a wider impact, but I can't talk about all of them right now. I can tell you we're bringing reliable sessions next.

@krispenner
Copy link

@mconnew and @StephenBonikowsky are there any new updates to share on this work?

Specifically, as per my request for full message security implementation.

@mconnew wrote:

The class AsymmetricSecurityBindingElement is only used when doing full message security. I was able to get TransportWithMessageCredentials working by rewriting WCF's usage of the System.IdentityModel version of SignedXml to use the System.Cryptography.Xml version instead. Unfortunately that version doesn't support all the transforms needed to support full message security. There's potential to use a SignedXml implementation from another team, but that needs some work to support WCF's usage scenario and I haven't even begun to have a serious conversation with that team about it yet.
I wouldn't expect to be able to support your scenario until at the earliest .NET 5 release, and that's if we can get agreement and schedule to get the work done by the other team.

If this is still not supported nor expected to be supported, then based on my sample code above can you recommend the least destructive changes to this code that would allow it to run in .NET 5 or even .NET Core 3.1? If it's just removing some extra security piece that could potentially be replaced by a similar feature but be supported I could request the vendor to make those code changes to their service. Because if they don't upgrade their service, then it will eventually be at risk when .NET Framework 4.8 stops being supported by MS. So I may have a case for them to invest in making some small code changes for that reason.

@mconnew
Copy link
Member

mconnew commented Oct 29, 2020

@krispenner, you are using MessageSecurity with certificate authentication. Both the client and server are validating the other end using a certificate and then using those certificates to encrypt the communication. This is odd as you are also using an HttpsTransportBindingElement which will also encrypt the connection meaning you have redundant encryption. You are sending an encrypted message over an encrypted connection. You are also using the February 2005 versions of the protocols. A binding which will give you almost the same features is WSHttpBinding.
The reason to use AsymmetricSecurityBindingElement is when you don't want the server where you send your messages to be able to read the message but another server that gets forwarded the message can read it. If that's the scenario, then you shouldn't switch to a different binding without serious consideration of your security model. If the server you are sending the message to is the destination consumer of the message, then switching to another binding won't change your security model.

There are 2 different ways to use WSHttpBinding. If you use SecurityMode.Transport, then it will use simply use certificates to secure the HTTPS connection and the actual messages are sent over HTTPS which will provide over the wire encryption. The client validates the server based on the server's certificate used for HTTPS and the server requires a client certificate provided as part of the HTTPS connection and validates the client based on that certificate. This is the option with the best performance.
The second option is to use SecurityMode.TransportWithMessageCredential. What this does is establish a secure connection to the server using HTTPS, but the client doesn't provide a client certificate. The client validates the server's identity via the HTTPS connection, but the server doesn't validate the client this way. Instead the client certificate is provided as part of a security header provided to the server. This is also done in the asymmetric security binding scenario you have above. The difference is the certificate isn't used to sign the message or encrypt it, it's only used for identification purposes. The HTTPS secure connection provides the encryption and tamper detection (TLS has a hash verification component to ensure the data hasn't been modified on it's way).
One thing to be aware of is that when using TransportWithMessageCredential, WSHttpBinding establishes a persistent session using WS-SecureConversation. If the service is running on multiple hosts behind a load balancer and the load balancer doesn't have clients be sticky to a single backend host based on the client ip, then subsequent requests can hop between different hosts which breaks the secure conversation session. If this is the case, then you will want to disable the use of secure conversation. You do this by setting WSHttpBinding.Security.Message.EstablishSecurityContext to false. This isn't a problem with with SecurityMode.Transport.

@krispenner
Copy link

Thank you very much @mconnew, this is a great explanation. I'm going to take this back to them and see if they will change anything on their end based on your comments.

@mconnew
Copy link
Member

mconnew commented Oct 30, 2020

If the server is a WCF server they should just be able to add another endpoint besides their existing one on a new url so existing clients will continue to work.

@krispenner
Copy link

Yes for sure, that is something I was also going to include in my suggestion. Thanks again, this is amazing help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WSFederation Issues related to adding support for WSFederation.
Projects
None yet
Development

No branches or pull requests

5 participants