Releasedate: 2013-09-11
Releasenote
Changed the serialization part of the protocolSupportEnumeration elements to support the .Net4.5 runtime environment. Microsoft changed the serialization of string array and anyUri datatypes in .Net4.5 runtime and a workaround has been implemented in the OIOSAML component.
Hi,
I ran in to some issues when trying to use OIOSAML.Net in a service provider for the Dutch DigiD identity provider. There are some fixes and workarounds that I would like to share, what would be a practical way of doing that?
In the end I got login working, currently working on logout.
1. General weakness: .ToString() on possible null values
In many places, the construction context.Session[<key>].ToString() is used. This is not allowed becausecontext.Session[<key>] can be null and the null object does not have a ToString()method.
You must use either (string)context.Session[<key>] or context.Session[<key>] as string and always do a null check. Alternatively, an utility function that returns empty string for a null value might come in handy.
To find this weakness, simply search for "].ToString()" in all source code. Everywhere where .Session is seen on the same line is an instance of the bug.
2. Bug in HttpSOAPBindingBuilder.GetResponse causes artifact signature validation to fail
DigiD uses Artifact protocol binding, that you do not officially support (although I was glad to see that code was mostly present). This binding requires proper interpretation of SOAP responses.
Near the end of HttpSOAPBindingBuilder.GetResponse, the response body is read into an XmlDocument, but PreserveWhitespace is not set. The property should be set to true before the document is loaded to allow any signature checking:
xDoc.PreserveWhitespace = true; xDoc.Load(response.GetReaderAtBodyContents());
Even with whitespace, the signature of the Assertion cannot be verified because the enclosing ArtifactResponse added whitespace after the Assertion was signed. Actually, the ArtifactResponse is also signed and its signature *can* be validated. To accomplish this, simply validate the first signature in the XmlDocument. To implement this check, I have added code in theparser.IsArtifactResponse()-block ofHandleSOAPsimilar to the existing code in theparser.IsArtifactResolve()block and setomitAssertionSignatureCheck="true"in web.config to stop the failing Assertion signature check.
3. Internal error in SignedXmlWithIdResolvement when parsing artifact
Because of the way the HttpSOAPBindingBuilder.GetResponse stream is handled in the HttpArtifactBindingBuilder, XmlSignatureUtils.RetrieveSignature may be called for an XMLElement that does not have an owner document.
This in turn causes the workaround for SignedXml.GetIdElement in SignedXmlWithIdResolvement to fail and raise the runtime error 'Malformed Reference Element' to be raised (exactly the error for which you introduced SignedXMLWithIdResolvement).
My workaround is to introduce this code near the beginning of RetrieveSignature(XmlElement el):
if (el.OwnerDocument.DocumentElement == null) { XmlDocument xd = new XmlDocument(); xd.PreserveWhitespace = true; xd.LoadXml(el.OuterXml); el = xd.DocumentElement; }
4. sha256 encryption not supported in SignedXml signature check
DigiD uses sha256 signatures in its outgoing messages. Older .Net versions do not support this at all, but from .Net 4.0 it is possible to enable it. I have added this code to XmlSignatureUtils.RetrieveSignature, just after the LoadXml call:
if (signedXml.SignatureMethod.Contains("rsa-sha256")) CryptoConfig.AddAlgorithm(typeof(RSAPKCS1SHA256SignatureDescription), signedXml.SignatureMethod);
You also need to define class RSAPKCS1SHA256SignatureDescription, as is shown here:
http://geekswithblogs.net/mkoerner/archive/2013/07/12/saml2-federationmetadata-validation.aspx
5. SignedXml.CheckSignature() without parameters does not work for sha256 signature
Even after fixing [4], the DigiD IdP metadata is still rejected because of a problem in the parameterless version of SignedXml.CheckSignature used in public static bool CheckSignature(XmlDocument doc).
Workaround: retrieve the certificates from the document if sha256 is used and use them to check the signature.
6. HttpSOAPBindingBuilder does not support client certifcate binding
I notice that Basic authentication is optionally supported in OIOSAML SOAP calls, but all Dutch government SOAP services --including DigiD-- use certificate authentication.
To allow to configure this, I have added a Certificate element to the HttpBasicAuthElement class:
/// <summary> /// Set ClientCertificate in web.config to enable client certificate authentication. /// </summary> [XmlElement] public Certificate ClientCertificate;
Now we can configure a SOAP client certificate in web.config, in the same way as the signing certificate is configured:
<IDPEndPoints metadata="c:\saml\idpmetadata"> <add id="https://was-preprod1.digid.nl/saml/idp/metadata" omitAssertionSignatureCheck="true"> <ArtifactResolution> <ClientCertificate findValue="573217998714D9FAFC8DF722800E9D6C18390A0F" storeLocation="LocalMachine" storeName="My" x509FindType="FindByThumbprint" /> </ArtifactResolution> </add> </IDPEndPoints>
(certificate used is same asSigningCertificate)
I have renamed HttpBasicAuthElement globally to HttpAuthElement and bool Enabled to BasicAuthEnabled to clarify that the class now also supports non-basic authentication configuration.
Hi Berend
Wow, it sure sounds like you have put a lot of work into this.
1. Yes - that seems to be multiple bugs. We will register it as such.
2. Yes - that is also a bug (not uncommon, unfortunately). We will register and fix it.
3. That is also a bug. I cannot determine at this time is it also affects SOAP binding. If it does we will definitively fix. If it only affects artifact binding then see *below.
4. We must also support previous versions, so while your change will enable SHA256 support for .NET 4+, we will have to make the change is such a way that it also supports SHA256 for .NET versions below 4. IIRC you will have to explicitly reference a crypto provider.
5. We will investigate.
6. OIOSAML specification does require that client certificate (two-way TLS/SSL) can be used for SOAP invocations. We will look into adopting your changes.
*Re: artifact binding: As you are aware, artifact binding is not officially supported by OIOSAML.NET. Whether we can adopt your changes will depend on the cost/benefit of the changes. Obviously, having access to your source changes could minimize our effort in that regard.
It is obvious that you have put a lot of work into this. We would be very grateful if you could upload the Subversion patch to this group (attach it to a post). We will then try to incorporate as much as possible into the main branch of OIOSAML.NET as we go forward.
At this point we cannot promise that we will perform all of your changes/fixes as we will have to prioritize the changes with other work items in the backlog. But we can promise that we will take a good look at it.
Again, thanks *a lot* for your work. We really appreciate it!
Hello Uffe,
Thanks for your detailed and positive reply.
I will prepare a subversion patch (or set of patches, one per distinct item) based on the official OIOSAMLNet 1.7.7 download. Is that OK, or do I need to use some other branch? (if so, where do I find it). It is also in my own interest to present the changes in a manner that you can easily use them - the more you can incorporate, the less difficult it will be for me to merge a future version of OIOSAML back into my project.
It will take me some time, I expect to be able to make the patch(es) by Thursday.
Regarding [4]: to make the code to support sha256 compile in older .Net versions, we could use reflection, see code below. Supporting sha256 in older versions of SignXml is as far as I know not possible. Microsoft put the necessary code changes in the core assembly mscorlib.dll.
My suggestion is to use this .Net version independent function:
/// /// To support SHA256 for XML signatures, an additional algorithm must be enabled. /// This is not supported in .Net versions older than 4.0. In older versions, /// an exception will be raised if an SHA256 signature method is attempted to be used. /// /// SignedXml object internal static void EnableSignatureMethod(SignedXml signedXml) { if (signedXml.SignatureMethod.Contains("rsa-sha256")) { MethodInfo miAddAlgorithm = typeof(CryptoConfig).GetMethod("AddAlgorithm", BindingFlags.Public | BindingFlags.Static); if (miAddAlgorithm == null) throw new InvalidOperationException("This version of .Net does not support CryptoConfig.AddAlgorithm. Enabling sha256 not psosible."); miAddAlgorithm.Invoke(null, new object[] { typeof(RSAPKCS1SHA256SignatureDescription), new string[] { signedXml.SignatureMethod }}); } }
I have also looked into using the CLR Security add-on (http://clrsecurity.codeplex.com/) or BouncyCastle (http://www.bouncycastle.org/) but both would cause much more change in OIOSAML. The main problem is that neither CLR Security nor BouncyCastle have an equivalent of the SignXml class, making it much more difficult to prepare the signature correctly.
Best regards,
Berend
Individual patches based on 1.7.7 will be perfect!
Great info on SHA256 in .net. I recall wondering how ADFS 2 could support SHA256 then WIF could not. As far as I knew, ADFS2 was based on WIF. So I did some disassembling and found some really fishy code where it took an entirely different path in case of SHA256.
Out of curiosity: There is no reference .NET implementation for Dutch DigiD?
Your work is much appreciated :-)
Thanks,
Uffe Seerup
Hi Uffe,
No, DigiD provides no reference implementation whatsoever. There just is a "pre-production" environment that you can use for testing, a list of requirements and very sketchy documentation. I was very glad to have found the Danish implementation: fully open source, including a decent IdP emulator and with documentation that helped me to get up and running quickly.
Your library has been invaluable for me, so I am glad if I can provide some help to your project in return. Apart from the DigiD support, we have also built our own IdP based on your emulator, that we plan to use to provide unified authentication for various cloud software products.
Decos SAML demo If anyone is curious to see what I did with OIOSAML, there is a prototype online. The Decos burgerberichten.nl software will provide a citizen login using DigiD:https://www.burgerberichten.nl/test (click the DigiD logo to go to DigiD login page) and a maintenance login using our own IdP:https://www.burgerberichten.nl/decos
OIOSAML.Net 1.7.7 patches I finished my patches faster than I expected. The attached zip contains the following patch files:a. bugfixes-1+2.patch : General bugfixes, issues [1] [2]b. workaround-3.patch : Workaround for internal error, issue [3]c. sha256-4+5.patch: sha256 XML signature support, issues [4] [5]d. soapclientcert-6.patch : SOAP binding using client certificate auth, issue [6] e. artifactbinding.patch : Misc. artifact binding fixes
All patches are made on the baseline oiosaml_net_1.7.7\dk.nita.saml20\dk.nita.saml20 sources. Each of the patches can be applied independently, you should be able to ignore any of them that is not to your liking or makes your tests fail. I have also verified that all patches can be successfully applied in the order as shown. To apply a patch, go to the oiosaml_net_1.7.7\dk.nita.saml20\dk.nita.saml20 directory and execute
patch -p1 {patch name}
Wow. Thanks a lot. We will try to adopt as much of this as possible (if not everything).
What happens now is that we will make a preliminary assesment as to the extent of changes needed for each patch.
If the changes are trivial we will just incorporate them in the next release. (This certainly seems to be the case for at least the bug fixes and the client cert). If changes are non-trivial we will need to get an approval from Digitaliseringstyrelsen (the government agency responsible for the reference implementations) before adopting the changes. Something about how we use taxpayers money, you know.
I hope you can appreciate that this being an essential security component of many running applications, the approval, review and testing paths tend to be a little longer than with traditional non-security related components.
Again, thanks a lot for your impressive work. We really appreciate it!
Uffe
I understand the need for your procedure - I only wish that the Dutch govenrment would have a similar professional approach to Open Source software development for their standard platforms.
If code cannot be accepted, I would appreciate to learn why not. This should help me to improve the quality of the code we base on OIOSAML and allow me to learn what kind of enhancements are acceptable.
When my SAML project is finished, I probably have another set of proposed changes. We hope that will be around January 2014.
We will certainly get back to you if there are one or more changes we cannot accept.
However, this is not an all-or-nothing procedure. If, for instance, we feel that further comments or abstractions are needed we may go ahead and make those changes ourselves. We do not expect contributors to adhere to any number of guidelines and/or review processes they cannot reasonably be aware of.
To me it look mostly straightforward. The artifact support could be a problem, but seen from above, the changes look like they are largely in line with changes we could conceivably make for the SOAP parts anyway. And SOAP is where we expect some enhancements going forward.
If you have more change proposals planned we can grant you branch permission on the repository. Make sure that you are a member here: http://digitaliser.dk/group/2501787 and then get back to us and request branch rights.
Thanks
I have accepted the invitation, can you give me branch rights?
For now I will not use them yet, because I am waiting for your Dutch counterpart Logius to resolve some differences in interpretation of the SAML standard that I encountered. Also, I do not want to hijack your development by suggesting a lot of changes that are only useful in my own situation.
The future enhancements that I think might be useful for others are:
1. Security fix for ASP.Net authentication cookies. Dutch safe computing rules require that https cookies are marked as secure and recommend using httpOnly cookies where possible. The asp.net forms authentication implementation of the secure flag is partially incorrect and difficult to configure. In our software we use a reflection trick to correct this for asp.net 2.0 and higher, I have ported that to my version of your library.
2. Mirror the Principal cache in a session cookie, generally reduce the number of server session variables needed. We found that using ASP.Net server session is not always reliable, for instance the SAML session is lost if server times out or application pool is recycled.
3. Allow to configure the software so that SAML session is only dependent on the cookie [2] DigiD requires that the session ends if all browser windows are closed. This is difficult to implement reliably if the session is also cached server side.
4. Enhancements for hosting the SP site as a Microsoft Azure cloud service Optionally read IdP data and client certificate from a virtual directory; some changes in certificate handling to work around deficiencies of the Azure platform.
5. Custom error handler In the prototype I needed to handle DigiD's "cancel login" in a user-friendly way. DigiD raises a Responder\AuthnFailed status when login is canceled, that we redirect to the "before login" landing page in the prototype.
You should already have been granted "admin" on a branch.
Please get back to us if you experience problems.
Best regards