IdentityPython / pysaml2

Python implementation of SAML2
Apache License 2.0
561 stars 423 forks source link

Limit available DigestMethods and SigningMethods #421

Open naggie opened 7 years ago

naggie commented 7 years ago

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 https://github.com/rohe/pysaml2/pull/396 which (as far as I can tel) affects outgoing assertions only. Is this correct?

I'm not sure if https://github.com/rohe/pysaml2/issues/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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 5 years ago

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

peppelinux commented 4 years ago

Here some important RP related to this topic: https://github.com/IdentityPython/pysaml2/issues/382#issuecomment-687677749

livenson commented 1 year ago

Hello, are there any news on this task?