Skip to content

Limit available DigestMethods and SigningMethods #421

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

Open
naggie opened this issue Jun 2, 2017 · 6 comments
Open

Limit available DigestMethods and SigningMethods #421

naggie opened this issue Jun 2, 2017 · 6 comments

Comments

@naggie
Copy link

naggie commented Jun 2, 2017

Looking at my generated metadata, it seems support is advertised for many different SigningMethods.

<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#md5"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#ripemd160"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#sha224"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#sha384"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha512"/>
<ns1:SigningMethod Algorithm="http,//www.w3.org/2000/09/xmldsig#dsa-sha1"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2009/xmldsig11#dsa-sha256"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-md5"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-ripemd160"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha224"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha384"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha512"/>

It seems this list is generated by checking if each is supported by the xmlsec1 binary.
https://github.com/rohe/pysaml2/blob/master/src/saml2/algsupport.py#L36

My concern is that a few of those signing methods here are insecure; for instance MD5. It's conceivable an assertion could be forged by intercepting a signed assertion and replacing it's signature with a weaker, cracked one.

Assuming my reasoning isn't flawed, I think an option to whitelist algorithms would be a good fix here. I also assume it would be more involved than updating SIGNING_METHODS and DIGEST_METHODS via config, as something needs to prevent xmlsec1 from attempting to verify a signature by raising an exception beforehand.

I believe this is a different issue to #396 which (as far as I can tel) affects outgoing assertions only. Is this correct?

I'm not sure if #382 has similar intentions.

Thanks for you time and pysaml2


I wonder if the best solution is to simply compile xmlsec1 with a subset of algorithm support: -DXMLSEC_NO_MD5=1"

@naggie
Copy link
Author

naggie commented Jun 5, 2017

For anyone with the same concern, I was successful in limiting the available algorithms:

sudo apt-get purge xmlsec1
mkdir xmlsec1 && cd xmlsec1
apt-get source xmlsec1
cd xmlsec1-1*
# edit debian/rules to apply below patch
dpkg-buildpackage -us -uc
cd ..
sudo dpkg -i *.deb
sudo apt-mark hold `*xmlsec1*`

Rules patch:

24c24
< config.status: configure
---
> config.status:
37c37,39
<               --disable-apps-crypto-dl
---
>               --disable-apps-crypto-dl \
>               --enable-md5=no \
>               --enable-ripemd160=no

Result:

<ns0:Extensions>
<ns1:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#sha224"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#sha384"/>
<ns1:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha512"/>
<ns1:SigningMethod Algorithm="http,//www.w3.org/2000/09/xmldsig#dsa-sha1"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2009/xmldsig11#dsa-sha256"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha224"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha384"/>
<ns1:SigningMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha512"/>
</ns0:Extensions>

@rohe do you think this is the best way, or do you think patching pysaml is the answer?

@knaperek
Copy link
Contributor

In my opinion, it is nice if pysaml takes its base list of supported algorithms from xmlsec1 - I have nothing against that.
But, that doesn't mean it should automatically allow and list all of them, period. I think it is a valid request for pysaml to accept a custom defined list of algorithms that further filter the default set reported by xmlsec1.

@c00kiemon5ter
Copy link
Member

Hi,

this is something that should be handled by the app (pysaml), not the system (xmlsec might be used by other apps on the same host).

IMO, a configurable blacklist is the correct way to go with this, as one can blacklist old/insecure algs, but automatically be able to use newly supported ones by xmlsec. The opposite would mean that for every update of xmlsec supported algs, a new entry would need to be manually added to the whitelist.

I don't think this is hard to implement. However there is bigger issue here as discussed in #278 - we shouldn't be using the xmlsec binary directly, but through bindings. Maybe this issue is good initiative to make that happen too.

@peppelinux
Copy link
Member

This would be a good choice
https://pythonhosted.org/xmlsec/modules/xmlsec.html
any alternatives?

@peppelinux
Copy link
Member

Here some important RP related to this topic:
#382 (comment)

@livenson
Copy link

Hello, are there any news on this task?

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

No branches or pull requests

6 participants