1

I'm having an issue where an IDP that is using Artifact Binding is signing their ArtifactResponse at the Assertion element level instead of signing the ArtifactResponse/document as a whole. This seems to be supported using other methods, but not artifact binding.

Specifically the GetAssertionElement() function is not overridden in the Saml2ArtifactResponse class like it is in the Saml2AuthnResponse class, and therefore defaults to returning NULL. Then the ValidateXmlSignature() method sees this null assertion and only checks the signature at the document level.

If I add the following to the Saml2ArtifactResponse class (that I copied from Saml2AuthnResponse) I believe it would work as expected, where it is able to then validate that the Assertion is signed and pass the signature validation.

XmlElement assertionElementCache = null;
        protected override XmlElement GetAssertionElement()
        {
            if (assertionElementCache == null)
            {
#if NETFULL || NETSTANDARD || NETCORE || NET50 || NET60
                assertionElementCache = GetAssertionElementReference().ToXmlDocument().DocumentElement;
#else
                assertionElementCache = GetAssertionElementReference();
#endif
            }
            return assertionElementCache;
        }

        private XmlElement GetAssertionElementReference()
        {
            var assertionElements = XmlDocument.DocumentElement.SelectNodes($"//*[local-name()='{Schemas.Saml2Constants.Message.Assertion}']");
            if (assertionElements.Count != 1)
            {
                throw new Saml2RequestException("There is not exactly one Assertion element. Maybe the response is encrypted (set the Saml2Configuration.DecryptionCertificate).");
            }
            return assertionElements[0] as XmlElement;
        }

I'm not sure if this make sense to do and/or is a valid use case for Artifact binding. If so it would be great for this to be implemented in the package.

Joshua K.
  • 21
  • 3

1 Answers1

0

It is okay to only sign on assertion level.

Please let me know if the code in this commit works for you https://github.com/ITfoxtec/ITfoxtec.Identity.Saml2/commit/f70b17b2f517f8caea55e2ce208f552148395a86

Code changes in Saml2ArtifactResponse.cs

string innerArtifactResponseXmlCache = null;
private string GetInnerArtifactResponseXml(string innerElementName, bool returnNull = false)
{
    if (innerArtifactResponseXmlCache == null)
    {
        var innerElements = XmlDocument.DocumentElement.SelectNodes(string.Format("//*[local-name()='{0}']", innerElementName));
        if (innerElements?.Count == 1)
        {
            innerArtifactResponseXmlCache = innerElements[0].OuterXml;
        }
        else
        {
            if (!returnNull && innerElements?.Count != 1)
            {
                throw new Saml2RequestException("There is not exactly one inner artifact response element.");
            }
        }
    }
    return innerArtifactResponseXmlCache;
}

XmlElement assertionElementCache = null;
protected override XmlElement GetAssertionElement()
{
    if (assertionElementCache == null)
    {
#if NETFULL || NETSTANDARD || NETCORE || NET50 || NET60
            assertionElementCache = GetAssertionElementReference().ToXmlDocument().DocumentElement;
#else
            assertionElementCache = GetAssertionElementReference();
#endif
    }
    return assertionElementCache;
}

private XmlElement GetAssertionElementReference()
{
    if(InnerRequest is Saml2AuthnResponse)
    {
        var innerArtifactResponseXml = GetInnerArtifactResponseXml(InnerRequest.ElementName, returnNull: true);
        if (innerArtifactResponseXml != null)
        {
            InnerRequest.Read(innerArtifactResponseXml, false, false);

            var assertionElements = InnerRequest.XmlDocument.DocumentElement.SelectNodes($"//*[local-name()='{Schemas.Saml2Constants.Message.Assertion}']");
            if (assertionElements.Count != 1)
            {
                throw new Saml2RequestException("There is not exactly one Assertion element. Maybe the response is encrypted (set the Saml2Configuration.DecryptionCertificate).");
            }
            return assertionElements[0] as XmlElement;
        }
    }
    return null;
}
Anders Revsgaard
  • 3,636
  • 1
  • 9
  • 25