Skip to content

Signature namespace defined in parent #94

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

Closed
darrenmce opened this issue Jun 11, 2015 · 4 comments
Closed

Signature namespace defined in parent #94

darrenmce opened this issue Jun 11, 2015 · 4 comments

Comments

@darrenmce
Copy link

I'm having an issue where in an Assertion, the namespace of the signature is defined in the Response element.

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" 
xmlns:dsig="http://www.w3.org/2000/09/xmldsig#">
...

    <dsig:Signature>      
...
    </dsig:Signature>

So when the validateSignature method extracts the node and converts it to a string, the namespace is lost (and xml-crypto rejects since it queries based on the namespace-uri)

Is this something passport-saml cares about? or is this a bad use of the Signature element/namespace?

Temporarily, I have put a workaround in the validateSignature function for myself:

Instead of simply var signature = signatures[0].toString(); in line 347

  var signature = signatures[0];

  // If Signature doesn't have it's namespace explicitly defined (was defined in parent)
  // it will be lost in the toString(), in this case, add it
  var prefix = signature.prefix;
  var nsAttr = 'xmlns';
  if (prefix) {
    nsAttr += ':' + prefix;
  }
  if (!signature.hasAttribute(nsAttr)) {
    signature.setAttribute(nsAttr, 'http://www.w3.org/2000/09/xmldsig#');
  }

  signature = signature.toString();
darrenmce pushed a commit to darrenmce/passport-saml that referenced this issue Jun 11, 2015
@ploer
Copy link
Contributor

ploer commented Jun 15, 2015

Hmm, interesting. I'll take a look at this, but might be a while before I get around to it (sounds like your workaround is sufficient for the moment).

@brianhartsock
Copy link
Contributor

@darrenmce Had the same issue. See node-saml/xml-crypto#59 where I'm trying to get a few changes into xml-crypto. Discussion is ongoing.

@darrenmce
Copy link
Author

@brianhartsock ah, great to know its not just me. I took a look at that issue in xml-crypto, and though its not clear to me where the fix belongs its becoming clear to me that a fix is necessary:

response from Oracle:


OAM 11gR2 federation declares the Signature in the ancestor element of the Signature element.

example:

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" 
xmlns:dsig="http://www.w3.org/2000/09/xmldsig#" 
xmlns:enc="http://www.w3.org/2001/04/xmlenc#" 
xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" 
xmlns:x500="urn:oasis:names:tc:SAML:2.0:profiles:attribute:X500" 
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
Destination="http://jungle.uk.oracle.com:7699/fed/sp/authnResponse20" ID="id-9FTHLvhLNQQz5ZnlW-nPu5F9UTe-WemnRyTiv7Gf" IssueInstant="2015-06-22T10:48:27Z" Version="2.0"> 
<saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://darakna.uk.oracle.com:14100/oam/fed</saml:Issuer> 

This might be different to how OIF 11gR1 generates the SAML artifact, but it is complaint with XML standards.

Please see:
Applying Namespaces to Elements and Attributes
6.1 Namespace Scoping
http://www.w3.org/TR/xml-names11/#scoping-defaulting

The processing application should be XML compliant and resolve the dsig Signature namespace from the namespaces declared in the ancestor samlp:Response element. The namespaces declared in the ancestor samlp:Response element should be applied to all elements within that element, up to the samlp:Response/ end tag.


@ploer
Copy link
Contributor

ploer commented Aug 5, 2015

Looks like @brianhartsock 's PR on xmlcrypto got merged; passport-saml PR #104 will take advantage of the change.

@ploer ploer closed this as completed Aug 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants