Skip to content

Added support for selectable SIGN and DIGEST algs in saml2_backend #214

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

Conversation

peppelinux
Copy link
Member

@peppelinux peppelinux commented Apr 11, 2019

With this PR we can specify desidered sign and digest algs in saml2_backend.yaml as follow

module: satosa.backends.saml2.SAMLBackend
name: Saml2
config:
  sp_config:
    service:
      sp:
        sign_alg: 'SIG_RSA_SHA256'
        digest_alg: 'DIGEST_SHA256'

        [...]

With this we'll have backend's authnRequest to saml2 idp with the desidered sign and enc algorithm, as the following example:

<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-UpdUvPPCmSubNkZZQ"
                  IssueInstant="2019-04-11T10:10:14Z"
                  ProtocolBinding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST"
                  Version="2.0"
                  > <ns1:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">https://satosa.testunical.it:10000/Saml2/proxy_saml2_backend</ns1:Issuer> <ns2:Signature Id="Signature1"> <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-sha256" /> <ns2:Reference URI="#id-UpdUvPPCmSubNkZZQ"> <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/xmlenc#sha256" /> <ns2:DigestValue>5rJFCIv6atCQPQz5CUmGR0OP2mnVbtUJwOlRBlQt3N4=</ns2:DigestValue> </ns2:Reference> </ns2:SignedInfo>

Hope to see this merged soon. Thank you

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

@rhoerbe
Copy link
Contributor

rhoerbe commented Apr 11, 2019

If the supported algorithms identifiers are not those from the W3C XMLDsig spec, the documentation should enumerate the possible values, e.g. by pointing to the relevant code section:

https://github.com/IdentityPython/pysaml2/blob/master/src/saml2/xmldsig/__init__.py#L17

Also, do I understand correctly, that there are different yaml paths for the key words in the SP and IDP config?
config.idp_config.service.idp.policy.default.sig_alg vs. config.sp_config.service.sdp..sig_alg

if this is the case, it should be explained in the documentation.

@peppelinux
Copy link
Member Author

peppelinux commented Apr 11, 2019

Yes, paths are different for sp and idp.

In backend I followed this path to get the value:
kwargs['sign_alg'] = self.config['sp_config']['service']['sp']['sign_alg']
kwargs['digest_alg'] = self.config['sp_config']['service']['sp']['digest_alg']

This could be also intelligibly understood because of its yaml structure

config:
  sp_config:
    service:
      sp:

I think that we could document and develop a better approach to push custom options in every kwarg in a more generalized way as well. I just did this and probably this approach could be used to merge any other options.

I'd prefer to avoid all those try/except found in pySAML2, because I think that it's better to raise the exception if an invalid option is used, instead of hide the problem. This could also remove the constant doubt in pysaml2 about config or code problems.

I agree about a better documentation, we could do more. SaToSa need it, it's a great piece of software!

A similar experience, about sign and digest algs, was already done in pySAML2 here:
IdentityPython/pysaml2#597

@peppelinux
Copy link
Member Author

I'd also say regarding available algorithms:

import saml2.xmldsig
saml2.xmldsig.SIG_ALLOWED_ALG                                                                                                                                                                                                          
(('SIG_RSA_SHA1', 'http://www.w3.org/2000/09/xmldsig#rsa-sha1'),
 ('SIG_RSA_SHA224', 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha224'),
 ('SIG_RSA_SHA256', 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha256'),
 ('SIG_RSA_SHA384', 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha384'),
 ('SIG_RSA_SHA512', 'http://www.w3.org/2001/04/xmldsig-more#rsa-sha512'))

saml2.xmldsig.DIGEST_ALLOWED_ALG 
(('DIGEST_SHA1', 'http://www.w3.org/2000/09/xmldsig#sha1'),
 ('DIGEST_SHA224', 'http://www.w3.org/2001/04/xmldsig-more#sha224'),
 ('DIGEST_SHA256', 'http://www.w3.org/2001/04/xmlenc#sha256'),
 ('DIGEST_SHA384', 'http://www.w3.org/2001/04/xmldsig-more#sha384'),
 ('DIGEST_SHA512', 'http://www.w3.org/2001/04/xmlenc#sha512'),
 ('DIGEST_RIPEMD160', 'http://www.w3.org/2001/04/xmlenc#ripemd160'))

This should be the reference, an easy way to document how to get them all without any static constant in the documentation.

...I'd like also to push an option in pySAML2 config to disallow some of those algorithms, but this would be another thread!

@peppelinux
Copy link
Member Author

Hi @c00kiemon5ter, probably it's the moment to merge this PR, IdentityPython/pysaml2#744 have been succesfully merged in pysaml2 as well, so it should work as it is

@c00kiemon5ter
Copy link
Member

c00kiemon5ter commented Dec 14, 2020

We do not need this anymore. We have introduced the signing_algorithm and digest_algorithm config options in pysaml2. This should be good enough to use here - we do not need different options. The same holds for the frontend.

an example of the configuration would be

module: satosa.backends.saml2.SAMLBackend
name: Saml2
config:
  sp_config:
    service:
      sp:
        signing_algorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512"
        digest_algorithm: "http://www.w3.org/2001/04/xmlenc#sha512"

YAML and satosa are not responsible to resolve symbols. A symbol like SIG_RSA_SHA256 should be resolved within the configuration definition. This can only be done if the configuration is specified as a python file.
All such symbols should be removed from pysaml2 and nobody should depend on them.

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 this pull request may close these issues.

3 participants