Ruby-SAML/SimpleSAML w/ Canonix - validation fails when attributes are enabled #5

Open
bdorry opened this Issue Jul 28, 2011 · 7 comments

Comments

Projects
None yet
3 participants
@bdorry

bdorry commented Jul 28, 2011

I'm really not sure if this is an issue with canonix since it also fails with xmlcanonicalizer, but I did eventually find my way here after searching so maybe someone will at least be able to provider a little bit of insight as to what is going.

When I have attributes enabled for a service provider validation fails in Ruby-SAML when validating the SAML response. I have tested using SimpleSAML as an SP to connect to my SimpleSAML IdP and validation is successful, as it is with every other service provider I have enabled. The only one that is failing is the Ruby app using Ruby SAML.

Admittedly I don't know too much about PHP so it is somewhat difficult to track down what is causing the issue. I'm just hoping that perhaps someone else has ran into this same issue and has some insight as to what is going on and may have a suggestion for either a fix or a potential work around.

@brendon

This comment has been minimized.

Show comment Hide comment
@brendon

brendon Jul 28, 2011

Owner

Hi Brian,

My implementation of ruby-saml also brings through extra attributes and seems to validate properly. Are you able to pinpoint the exact point of failure? The ruby-saml validation file does quite a few tests on the incoming message to ensure it's valid. When I had a validation problem (due to an old bug in the original xmlcanonicalizer gem) I just took the code from this file and put it in the controller where I was consuming my SAML response and steped through each step to see where the failure was.

If it turns out to be an incorrect canonicalization of the XML message, please let me know and we might be able to brainstorm reasons for that. Otherwise, it could be a failure in one of the other checks not specific to canonix in which case the ruby-saml github issues area would probably be the better place to ask :D

Hope that helps :D

Brendon

Owner

brendon commented Jul 28, 2011

Hi Brian,

My implementation of ruby-saml also brings through extra attributes and seems to validate properly. Are you able to pinpoint the exact point of failure? The ruby-saml validation file does quite a few tests on the incoming message to ensure it's valid. When I had a validation problem (due to an old bug in the original xmlcanonicalizer gem) I just took the code from this file and put it in the controller where I was consuming my SAML response and steped through each step to see where the failure was.

If it turns out to be an incorrect canonicalization of the XML message, please let me know and we might be able to brainstorm reasons for that. Otherwise, it could be a failure in one of the other checks not specific to canonix in which case the ruby-saml github issues area would probably be the better place to ask :D

Hope that helps :D

Brendon

@bdorry

This comment has been minimized.

Show comment Hide comment
@bdorry

bdorry Jul 28, 2011

It's failing this validation check

check digests

  REXML::XPath.each(sig_element, "//ds:Reference", {"ds"=>"http://www.w3.org/2000/09/xmldsig#"}) do |ref|
    uri                           = ref.attributes.get_attribute("URI").value
    hashed_element                = REXML::XPath.first(self, "//[@ID='#{uri[1,uri.size]}']")
    canoner                       = XML::Util::XmlCanonicalizer.new(false, true)
    canoner.inclusive_namespaces  = inclusive_namespaces if canoner.respond_to?(:inclusive_namespaces) && !inclusive_namespaces.empty?
    canon_hashed_element          = canoner.canonicalize(hashed_element)
    hash                          = Base64.encode64(Digest::SHA1.digest(canon_hashed_element)).chomp
    digest_value                  = REXML::XPath.first(ref, "//ds:DigestValue", {"ds"=>"http://www.w3.org/2000/09/xmldsig#"}).text

    if hash != digest_value
      return soft ? false : (raise Onelogin::Saml::ValidationError.new("Digest mismatch"))
    end
  end

I don't have the project available right now, but I'll dig up some additional details when I can.

I do see that the project has been updated recently so the first thing I am going to try is updating ruby-saml.

bdorry commented Jul 28, 2011

It's failing this validation check

check digests

  REXML::XPath.each(sig_element, "//ds:Reference", {"ds"=>"http://www.w3.org/2000/09/xmldsig#"}) do |ref|
    uri                           = ref.attributes.get_attribute("URI").value
    hashed_element                = REXML::XPath.first(self, "//[@ID='#{uri[1,uri.size]}']")
    canoner                       = XML::Util::XmlCanonicalizer.new(false, true)
    canoner.inclusive_namespaces  = inclusive_namespaces if canoner.respond_to?(:inclusive_namespaces) && !inclusive_namespaces.empty?
    canon_hashed_element          = canoner.canonicalize(hashed_element)
    hash                          = Base64.encode64(Digest::SHA1.digest(canon_hashed_element)).chomp
    digest_value                  = REXML::XPath.first(ref, "//ds:DigestValue", {"ds"=>"http://www.w3.org/2000/09/xmldsig#"}).text

    if hash != digest_value
      return soft ? false : (raise Onelogin::Saml::ValidationError.new("Digest mismatch"))
    end
  end

I don't have the project available right now, but I'll dig up some additional details when I can.

I do see that the project has been updated recently so the first thing I am going to try is updating ruby-saml.

@bdorry bdorry closed this Jul 28, 2011

@bdorry bdorry reopened this Jul 28, 2011

@brendon

This comment has been minimized.

Show comment Hide comment
@brendon

brendon Jul 31, 2011

Owner

Hi Brian, sorry for the slow reply. How did you get on with using the latest version?

I guess if the hashes aren't matching, the best thing to do would be to get the canonicalised version from the PHP side at the point where it's about to be hashed, and compare it to the canonicalised form on this end.

Check out the latest commits to canonix. Someone recently pushed a patch that helped with their namespacing problem (from memory). Perhaps this would help?

Cheers,

Brendon

Owner

brendon commented Jul 31, 2011

Hi Brian, sorry for the slow reply. How did you get on with using the latest version?

I guess if the hashes aren't matching, the best thing to do would be to get the canonicalised version from the PHP side at the point where it's about to be hashed, and compare it to the canonicalised form on this end.

Check out the latest commits to canonix. Someone recently pushed a patch that helped with their namespacing problem (from memory). Perhaps this would help?

Cheers,

Brendon

@dtsato

This comment has been minimized.

Show comment Hide comment
@dtsato

dtsato Sep 28, 2011

Contributor

Hi @bdorry,

I was having problems with the digest check as well, and tracked it down to a bug in the way the namespace nodes are sorted. You can check if my forked project fixes your problem:

https://github.com/dtsato/canonix

If it works, please add a note to my pull request (#6), to help get this released in the main gem.
Cheers,
Danilo

Contributor

dtsato commented Sep 28, 2011

Hi @bdorry,

I was having problems with the digest check as well, and tracked it down to a bug in the way the namespace nodes are sorted. You can check if my forked project fixes your problem:

https://github.com/dtsato/canonix

If it works, please add a note to my pull request (#6), to help get this released in the main gem.
Cheers,
Danilo

@bdorry

This comment has been minimized.

Show comment Hide comment
@bdorry

bdorry Oct 18, 2011

Hi @dtsato

I just got a chance to try this out, works great now, thank you for this. I saw you already got it merged up too.

bdorry commented Oct 18, 2011

Hi @dtsato

I just got a chance to try this out, works great now, thank you for this. I saw you already got it merged up too.

@bdorry bdorry closed this Oct 18, 2011

@bdorry bdorry reopened this Oct 18, 2011

@bdorry

This comment has been minimized.

Show comment Hide comment
@bdorry

bdorry Oct 18, 2011

Actually I made a mistake and had attributes turned off, I'm still receiving this error with the updated gem.

bdorry commented Oct 18, 2011

Actually I made a mistake and had attributes turned off, I'm still receiving this error with the updated gem.

@brendon

This comment has been minimized.

Show comment Hide comment
@brendon

brendon Dec 15, 2011

Owner

Hi sorry for the non-reply, are you still having problems?

Did you step through each stage of the verification process to identify where the failure is happening? It's unfortunate that I don't have much experience with this software to be able to offer any practical advice. Perhaps we can figure it out together? Might help to have an isolated project that exhibits the problem so that we can troubleshoot it more easily.

Owner

brendon commented Dec 15, 2011

Hi sorry for the non-reply, are you still having problems?

Did you step through each stage of the verification process to identify where the failure is happening? It's unfortunate that I don't have much experience with this software to be able to offer any practical advice. Perhaps we can figure it out together? Might help to have an isolated project that exhibits the problem so that we can troubleshoot it more easily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment