Skip to content
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

sign_alg/digest_alg policy config is broken #212

Closed
rhoerbe opened this issue Apr 10, 2019 · 5 comments
Closed

sign_alg/digest_alg policy config is broken #212

rhoerbe opened this issue Apr 10, 2019 · 5 comments

Comments

@rhoerbe
Copy link
Contributor

rhoerbe commented Apr 10, 2019

specifying the algorithm causes an Exception:
module 'saml2.xmldsig' has no attribute 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'

Code Version

Satosa 3.4.8 pysaml 4.7.0

Expected Behavior

Should work as documented in the REAME

Possible Solution

Workaround: do not specify, or use the internal keys of pysaml2 (e.g. saml2.xmldsig.SIG_RSA_SHA256) like this:

    policy:
      default:
        sign_alg: 'SIG_RSA_SHA256'

Offending code

https://github.com/IdentityPython/SATOSA/blob/master/src/satosa/frontends/saml2.py#L349

The string from yaml should be converted back to the xmldsig name.

Is there an equivalent function in the SAMl backend?

@peppelinux
Copy link
Member

peppelinux commented Apr 11, 2019

Hi @rhoerbe,

this means that we should just have to put
SIG_RSA_SHA384
instead of
saml2.xmldsig.SIG_RSA_SHA384
in .yaml configuration file.

Regarding my tests, because at this moment I'm on this, I get the followings.
Using in saml2_frontend

        policy:
          default:
            sign_alg: 'SIG_RSA_SHA384'
            digest_alg: 'DIGEST_SHA384'

I get what I desidered, as show here:

<ns0:Response xmlns:ns0="urn:oasis:names:tc:SAML:2.0:protocol"
              xmlns:ns1="urn:oasis:names:tc:SAML:2.0:assertion"
              xmlns:ns2="http://www.w3.org/2000/09/xmldsig#"
              xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
              Destination="http://sp1.testunical.it:8000/saml2/acs/"
              ID="id-I613tDQkRZbgt0vWY"
              InResponseTo="id-K2uYhN6GztvgTNnaF"
              IssueInstant="2019-04-11T09:33:42Z"
              Version="2.0"
              > <ns1:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://satosa.testunical.it:10000/Saml2IDP/proxy</ns1:Issuer>

[....]

 <ns2:SignedInfo> <ns2:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /> <ns2:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha384" /> <ns2:Reference URI="#id-I613tDQkRZbgt0vWY"> <ns2:Transforms> <ns2:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /> <ns2:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /> </ns2:Transforms> <ns2:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#sha384" /> <ns2:DigestValue>Tmg+es1zg3TSMneLmY25aAaKkteycBHhtbAFTj3f2T9tonY4kWpe2F2D8WVFA/+X</ns2:DigestValue> </ns2:Reference> </ns2:SignedInfo>

But in backend I didn't get the same result, using:

service:
  sp:
    sign_alg: 'SIG_RSA_SHA256'
    digest_alg: 'DIGEST_SHA256'

I see

<ns0:AuthnRequest xmlns:ns0="urn:oasis:names:tc:SAML:2.0:protocol"
                  xmlns:ns1="urn:oasis:names:tc:SAML:2.0:assertion"
                  xmlns:ns2="http://www.w3.org/2000/09/xmldsig#"
                  AssertionConsumerServiceURL="https://satosa.testunical.it:10000/Saml2/acs/post"
                  Destination="http://idp1.testunical.it:9000/idp/sso/redirect"
                  ID="id-CyjO7hx0Lx514MzFg"
                  IssueInstant="2019-04-11T09:40:47Z"
                  ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                  Version="2.0"
                  >

[....]

<ns2:SignedInfo> <ns2:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /> <ns2:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" /> <ns2:Reference URI="#id-c2IsFlD53eos1FxkS"> <ns2:Transforms> <ns2:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /> <ns2:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /> </ns2:Transforms> <ns2:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" /> <ns2:DigestValue>KvCrDYZ7v8YFgVNfI0dDwXfpL5A=</ns2:DigestValue> </ns2:Reference> </ns2:SignedInfo>

I'm going through this...

@rhoerbe
Copy link
Contributor Author

rhoerbe commented Apr 11, 2019

Yes, the algorithm needs to be a string (and saml2.xmldsig.SIG_RSA_SHA384 would only be available in a .py config, not in .yaml.).

I am not sure if this is more than a temporary solution. Either doc doc needs to be fixed, or the code.

@peppelinux
Copy link
Member

peppelinux commented Apr 12, 2019

I got it,
In official doc we have

sign_alg: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"
digest_alg: "http://www.w3.org/2001/04/xmlenc#sha256"

This bug should be easy to patch, we have these options:

a) Change documentation to use saml2.xmldsig's elements like "SIG_RSA_SHA384" and other available in it;
b) Leave the documentation as is, add support for the following notations as examples:
- "saml2.xmldsig.SIG_RSA_SHA384"
- "SIG_RSA_SHA1"
- "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"
c) Leave documentation as is and support only
- "SIG_RSA_SHA256"
- "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"

I think that c) would be formally correct, any ideas?

@peppelinux
Copy link
Member

Really patched now, hope to see it merged soon.
PR here: #216

@c00kiemon5ter
Copy link
Member

  • The sign_alg and digest_alg option will be removed.
  • signing_algorithm and digest_algorithm should be used instead; under service/<entity-type> (not under policy/default)
  • The algorithm-string itself should be used as the value, not the internal symbols.

I will update the readme to remove those options under policy.

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

Successfully merging a pull request may close this issue.

3 participants