From a44496142fa6efda21cf324200ada223c2d81be5 Mon Sep 17 00:00:00 2001 From: samunro Date: Fri, 5 Apr 2019 18:14:37 +0200 Subject: [PATCH 1/6] Made ServiceProvider.SigningCertificate Optional The HttpRedirectBindingBuilder.SigningKey setter allows for null values. // Check if the key is of a supported type. [SAMLBind] sect. 3.4.4.1 specifies this. if (!(value is RSACryptoServiceProvider || value is DSA || value == null)) { throw new ArgumentException("Signing key must be an instance of either RSACryptoServiceProvider or DSA."); } This expression which is used to assign to that property fails if the ServiceProvider does not have a SigningCertificate. SigningKey = config.ServiceProvider.SigningCertificate.PrivateKey, Added the Elvis operator to implement this. SigningKey = config.ServiceProvider.SigningCertificate?.PrivateKey, --- src/Owin.Security.Saml/SamlMessage.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Owin.Security.Saml/SamlMessage.cs b/src/Owin.Security.Saml/SamlMessage.cs index af07393..bf8a4a0 100644 --- a/src/Owin.Security.Saml/SamlMessage.cs +++ b/src/Owin.Security.Saml/SamlMessage.cs @@ -150,7 +150,7 @@ private string AuthnRequestForIdp(IdentityProvider identityProvider, Saml20Authn var redirectBuilder = new HttpRedirectBindingBuilder { - SigningKey = config.ServiceProvider.SigningCertificate.PrivateKey, + SigningKey = config.ServiceProvider.SigningCertificate?.PrivateKey, Request = request.GetXml().OuterXml }; if (context.Authentication != null && @@ -229,4 +229,4 @@ public override string BuildFormPost() return base.BuildFormPost(); // See Saml20SignonHandler.cs, line 591 (post binding) } } -} \ No newline at end of file +} From d6afeb5c1ad208d77a295f482e84fa008730fc5a Mon Sep 17 00:00:00 2001 From: samunro Date: Fri, 5 Apr 2019 18:19:43 +0200 Subject: [PATCH 2/6] Removed blank line at the end of file. From fffb8da0ffc83dbb416015ff01a879dd8fd71bfe Mon Sep 17 00:00:00 2001 From: Scott Munro Date: Tue, 9 Apr 2019 17:52:15 +0200 Subject: [PATCH 3/6] Default to RedirectAfterLogin in SamlAuthenticationHandler.ApplyResponseChallengeAsync The current default is currentUri which does not seem like the best choice if a value for RedirectAfterLogin is available. I was expecting the browser to be redirected to RedirectAfterLogin post authentication but I actually saw an endless loop of authentications because it was redirecting to currentUri. --- src/Owin.Security.Saml/SamlAuthenticationHandler.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Owin.Security.Saml/SamlAuthenticationHandler.cs b/src/Owin.Security.Saml/SamlAuthenticationHandler.cs index 159ae07..30b8592 100644 --- a/src/Owin.Security.Saml/SamlAuthenticationHandler.cs +++ b/src/Owin.Security.Saml/SamlAuthenticationHandler.cs @@ -109,7 +109,8 @@ protected override async Task ApplyResponseChallengeAsync() AuthenticationProperties properties = challenge.Properties; if (string.IsNullOrEmpty(properties.RedirectUri)) { - properties.RedirectUri = currentUri; + properties.RedirectUri = !string.IsNullOrWhiteSpace(Options.RedirectAfterLogin) ? Options.RedirectAfterLogin : currentUri; + if (_logger.IsEnabled(TraceEventType.Verbose)) { _logger.WriteVerbose(string.Format("Setting the RedirectUri to {0}.", properties.RedirectUri)); @@ -228,4 +229,4 @@ private static AuthenticationTicket GetHandledResponseTicket() } } -} \ No newline at end of file +} From befc1ac18b2307d487f0b3a976b4ec81f35c4c14 Mon Sep 17 00:00:00 2001 From: Scott Munro Date: Tue, 31 Aug 2021 13:35:49 +0200 Subject: [PATCH 4/6] There were errors with Azure AD. The metadata included RoleDescriptors of type SecurityTokenServiceType and ApplicationServiceType which were not expected. There was a workaround which involved removing those but that also meant that the signature had to be removed. I added types which allows the metadata to be deserialized - even if there is no special handling for them. The certificates in the SAML response were being passed in a way that the existing code did not expect. They are now parsed successfully. --- OwinSecuritySamlNupkg.ps1 | 5 +-- Saml2CoreNupkg.ps1 | 3 +- .../Owin.Security.Saml.nuspec | 8 ++--- src/SAML2.Core/Protocol/Utility.cs | 35 +++++++++++++------ src/SAML2.Core/SAML2.Core.csproj | 2 ++ src/SAML2.Core/SAML2.Core.nuspec | 6 ++-- src/SAML2.Core/Saml20Constants.cs | 2 ++ .../Schema/Metadata/ApplicationServiceType.cs | 7 ++++ .../Schema/Metadata/RoleDescriptor.cs | 2 ++ .../Metadata/SecurityTokenServiceType.cs | 8 +++++ src/SAML2.Tests/SAML2.Tests.csproj | 6 ---- .../SelfHostOwinSPExample.csproj | 3 -- 12 files changed, 58 insertions(+), 29 deletions(-) create mode 100644 src/SAML2.Core/Schema/Metadata/ApplicationServiceType.cs create mode 100644 src/SAML2.Core/Schema/Metadata/SecurityTokenServiceType.cs diff --git a/OwinSecuritySamlNupkg.ps1 b/OwinSecuritySamlNupkg.ps1 index 965f032..3ac32db 100644 --- a/OwinSecuritySamlNupkg.ps1 +++ b/OwinSecuritySamlNupkg.ps1 @@ -7,7 +7,8 @@ param( # Tool locations $nuget = ".\tools\NuGet.exe" -$msbuild = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\MSBuild.exe" +#$msbuild = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\MSBuild.exe" +$msbuild = "& 'C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\Bin\MSBuild.exe'"; # RegEx strings $assemblyVersionPattern = '^\[assembly: AssemblyVersion\("[0-9]+(\.([0-9]+|\*)){1,3}"\)\]$' @@ -57,7 +58,7 @@ Invoke-Expression "$msbuild $solution /p:Configuration=Debug /p:Platform=`"Any C Invoke-Expression "$msbuild $solution /p:Configuration=Release /p:Platform=`"Any CPU`" /t:Clean" # Optional: Build -# Invoke-Expression "$msbuild $solution /p:Configuration=Release /p:Platform=`"Any CPU`" /t:Build" +Invoke-Expression "$msbuild $solution /p:Configuration=Release /p:Platform=`"Any CPU`" /t:Build" # Optional: Run unit tests # Invoke-Expression ".\src\packages\NUnit.Runners\tools\nunit.exe" diff --git a/Saml2CoreNupkg.ps1 b/Saml2CoreNupkg.ps1 index 9ec1a82..a00304d 100644 --- a/Saml2CoreNupkg.ps1 +++ b/Saml2CoreNupkg.ps1 @@ -7,7 +7,8 @@ param( # Tool locations $nuget = ".\tools\NuGet.exe" -$msbuild = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\MSBuild.exe" +#$msbuild = "C:\Windows\Microsoft.NET\Framework64\v4.0.30319\MSBuild.exe" +$msbuild = "& 'C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\MSBuild\Current\Bin\MSBuild.exe'"; # RegEx strings $assemblyVersionPattern = '^\[assembly: AssemblyVersion\("[0-9]+(\.([0-9]+|\*)){1,3}"\)\]$' diff --git a/src/Owin.Security.Saml/Owin.Security.Saml.nuspec b/src/Owin.Security.Saml/Owin.Security.Saml.nuspec index 7af6ab1..e472480 100644 --- a/src/Owin.Security.Saml/Owin.Security.Saml.nuspec +++ b/src/Owin.Security.Saml/Owin.Security.Saml.nuspec @@ -1,19 +1,19 @@  - $id$ + scott.munro.$id$ $version$ $title$ $author$ $author$ - https://github.com/elerch/SAML2/blob/master/LICENSE - https://github.com/elerch/SAML2 + https://github.com/samunro/SAML2/blob/master/LICENSE + https://github.com/samunro/SAML2 $description$ Working SP-Initiated requests against testshib.org (redirect binding) Copyright 2015 SAML2 Owin library - + diff --git a/src/SAML2.Core/Protocol/Utility.cs b/src/SAML2.Core/Protocol/Utility.cs index f863c46..04bd78e 100644 --- a/src/SAML2.Core/Protocol/Utility.cs +++ b/src/SAML2.Core/Protocol/Utility.cs @@ -4,6 +4,7 @@ using SAML2.Schema.Core; using SAML2.Schema.Metadata; using SAML2.Schema.Protocol; +using SAML2.Schema.XmlDSig; using SAML2.Specification; using SAML2.Utils; using System; @@ -44,20 +45,34 @@ public static IEnumerable GetTrustedSigners(ICollection k.KeyInfo.Items.AsEnumerable().Cast())) { - // Check certificate specifications - if (clause is KeyInfoX509Data) { - var cert = XmlSignatureUtils.GetCertificateFromKeyInfo((KeyInfoX509Data)clause); - if (!CertificateSatisfiesSpecifications(identityProvider, cert)) { - continue; - } - } + foreach (var clause in keys.SelectMany(k => k.KeyInfo.Items.AsEnumerable().OfType())) { + var trustedSigner = GetTrustedSigner(clause, identityProvider); + + if(trustedSigner != null) yield return trustedSigner; + } + + foreach (var x509Data in keys.SelectMany(k => k.KeyInfo.Items.AsEnumerable().OfType())) { + var clause = new KeyInfoX509Data((byte[])x509Data.Items[0]); + + var trustedSigner = GetTrustedSigner(clause, identityProvider); - var key = XmlSignatureUtils.ExtractKey(clause); - yield return key; + if(trustedSigner != null) yield return trustedSigner; } } + private static AsymmetricAlgorithm GetTrustedSigner(KeyInfoClause clause, IdentityProvider identityProvider) + { + // Check certificate specifications + if (clause is KeyInfoX509Data) + { + var cert = XmlSignatureUtils.GetCertificateFromKeyInfo((KeyInfoX509Data)clause); + + if (!CertificateSatisfiesSpecifications(identityProvider, cert)) return null; + } + + return XmlSignatureUtils.ExtractKey(clause); + } + /// /// Determines whether the certificate is satisfied by all specifications. /// diff --git a/src/SAML2.Core/SAML2.Core.csproj b/src/SAML2.Core/SAML2.Core.csproj index 92e6661..dacac48 100644 --- a/src/SAML2.Core/SAML2.Core.csproj +++ b/src/SAML2.Core/SAML2.Core.csproj @@ -170,7 +170,9 @@ + + diff --git a/src/SAML2.Core/SAML2.Core.nuspec b/src/SAML2.Core/SAML2.Core.nuspec index 4953ade..5e67b6c 100644 --- a/src/SAML2.Core/SAML2.Core.nuspec +++ b/src/SAML2.Core/SAML2.Core.nuspec @@ -1,13 +1,13 @@  - $id$ + scott.munro.$id$ $version$ $title$ $author$ $author$ - https://github.com/elerch/SAML2/blob/master/LICENSE - https://github.com/elerch/SAML2 + https://github.com/samunro/SAML2/blob/master/LICENSE + https://github.com/samunro/SAML2 $description$ Working SP-Initiated requests against testshib.org (redirect binding) Copyright 2015 diff --git a/src/SAML2.Core/Saml20Constants.cs b/src/SAML2.Core/Saml20Constants.cs index 22e98c9..60e5867 100644 --- a/src/SAML2.Core/Saml20Constants.cs +++ b/src/SAML2.Core/Saml20Constants.cs @@ -25,6 +25,8 @@ public class Saml20Constants /// public const string Metadata = "urn:oasis:names:tc:SAML:2.0:metadata"; + public const string WsFederationNamespace = "http://docs.oasis-open.org/wsfed/federation/200706"; + /// /// The XML namespace of XmlDSig /// diff --git a/src/SAML2.Core/Schema/Metadata/ApplicationServiceType.cs b/src/SAML2.Core/Schema/Metadata/ApplicationServiceType.cs new file mode 100644 index 0000000..3dfca50 --- /dev/null +++ b/src/SAML2.Core/Schema/Metadata/ApplicationServiceType.cs @@ -0,0 +1,7 @@ +using System.Xml.Serialization; + +namespace SAML2.Schema.Metadata +{ + [XmlType(Namespace = Saml20Constants.WsFederationNamespace)] + public class ApplicationServiceType: RoleDescriptor {} +} diff --git a/src/SAML2.Core/Schema/Metadata/RoleDescriptor.cs b/src/SAML2.Core/Schema/Metadata/RoleDescriptor.cs index fc00371..563f3e4 100644 --- a/src/SAML2.Core/Schema/Metadata/RoleDescriptor.cs +++ b/src/SAML2.Core/Schema/Metadata/RoleDescriptor.cs @@ -17,6 +17,8 @@ namespace SAML2.Schema.Metadata [XmlInclude(typeof(SsoDescriptor))] [XmlInclude(typeof(SpSsoDescriptor))] [XmlInclude(typeof(IdpSsoDescriptor))] + [XmlInclude(typeof(SecurityTokenServiceType))] + [XmlInclude(typeof(ApplicationServiceType))] [Serializable] [XmlType(Namespace = Saml20Constants.Metadata)] [XmlRoot(ElementName, Namespace = Saml20Constants.Metadata, IsNullable = false)] diff --git a/src/SAML2.Core/Schema/Metadata/SecurityTokenServiceType.cs b/src/SAML2.Core/Schema/Metadata/SecurityTokenServiceType.cs new file mode 100644 index 0000000..4307a26 --- /dev/null +++ b/src/SAML2.Core/Schema/Metadata/SecurityTokenServiceType.cs @@ -0,0 +1,8 @@ +using System.Xml.Serialization; + +namespace SAML2.Schema.Metadata +{ + [XmlType(Namespace = Saml20Constants.WsFederationNamespace)] + public class SecurityTokenServiceType: RoleDescriptor{} +} + diff --git a/src/SAML2.Tests/SAML2.Tests.csproj b/src/SAML2.Tests/SAML2.Tests.csproj index 893d6f3..07d63d6 100644 --- a/src/SAML2.Tests/SAML2.Tests.csproj +++ b/src/SAML2.Tests/SAML2.Tests.csproj @@ -132,12 +132,6 @@ Always - - Always - - - Always - diff --git a/src/SelfHostOwinSPExample/SelfHostOwinSPExample.csproj b/src/SelfHostOwinSPExample/SelfHostOwinSPExample.csproj index 4bcb46e..30b7275 100644 --- a/src/SelfHostOwinSPExample/SelfHostOwinSPExample.csproj +++ b/src/SelfHostOwinSPExample/SelfHostOwinSPExample.csproj @@ -67,9 +67,6 @@ - - Always - From fedab88445709922a63747cef2128ccde435ecda Mon Sep 17 00:00:00 2001 From: Scott Munro Date: Tue, 31 Aug 2021 14:00:08 +0200 Subject: [PATCH 5/6] There were problems with checking the signature on an assertion because it was a hash of the whole document that was being used instead of one based on just the assertion. --- src/SAML2.Core/Saml20Assertion.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/SAML2.Core/Saml20Assertion.cs b/src/SAML2.Core/Saml20Assertion.cs index b23a04c..0b07bdd 100644 --- a/src/SAML2.Core/Saml20Assertion.cs +++ b/src/SAML2.Core/Saml20Assertion.cs @@ -611,7 +611,14 @@ private void InsertAttributes() /// The trusted signers. private void LoadXml(XmlElement element, IEnumerable trustedSigners, Saml2Configuration config) { - XmlAssertion = element; + //If the assertion element belongs to a larger document then it would be a hash of that document that would be used when checking the signature. + //Create a new, smaller document that contains only the assertion so that it is a hash of the assertion that is used. + var xmlDocument = new XmlDocument() { PreserveWhitespace = true }; + + xmlDocument.LoadXml(element.OuterXml); + + XmlAssertion = xmlDocument.DocumentElement; + if (trustedSigners != null) { if (!CheckSignature(trustedSigners)) From c13ec7132c0d80b0eea17d0edee9a65147e4c5ad Mon Sep 17 00:00:00 2001 From: Scott Munro Date: Tue, 31 Aug 2021 16:05:21 +0200 Subject: [PATCH 6/6] Changed version numbers so that I could create new NuGet package versions. --- src/Owin.Security.Saml/Owin.Security.Saml.nuspec | 2 +- src/Owin.Security.Saml/Properties/AssemblyInfo.cs | 4 ++-- src/SAML2.Core/Properties/AssemblyInfo.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Owin.Security.Saml/Owin.Security.Saml.nuspec b/src/Owin.Security.Saml/Owin.Security.Saml.nuspec index e472480..9694939 100644 --- a/src/Owin.Security.Saml/Owin.Security.Saml.nuspec +++ b/src/Owin.Security.Saml/Owin.Security.Saml.nuspec @@ -13,7 +13,7 @@ Copyright 2015 SAML2 Owin library - + diff --git a/src/Owin.Security.Saml/Properties/AssemblyInfo.cs b/src/Owin.Security.Saml/Properties/AssemblyInfo.cs index bc9fd3a..3b7f9b7 100644 --- a/src/Owin.Security.Saml/Properties/AssemblyInfo.cs +++ b/src/Owin.Security.Saml/Properties/AssemblyInfo.cs @@ -32,5 +32,5 @@ // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion("1.0.0.0")] -[assembly: AssemblyFileVersion("1.0.0.0")] +[assembly: AssemblyVersion("1.2.0.0")] +[assembly: AssemblyFileVersion("1.2.0.0")] diff --git a/src/SAML2.Core/Properties/AssemblyInfo.cs b/src/SAML2.Core/Properties/AssemblyInfo.cs index 7917134..5cf6c33 100644 --- a/src/SAML2.Core/Properties/AssemblyInfo.cs +++ b/src/SAML2.Core/Properties/AssemblyInfo.cs @@ -32,5 +32,5 @@ // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion("2.0.0.0")] -[assembly: AssemblyFileVersion("2.0.0.0")] +[assembly: AssemblyVersion("1.2.0.0")] +[assembly: AssemblyFileVersion("1.2.0.0")]