pysaml2 version in global requirements must be updated to 4.5.0

[Divya K Konoor]

As per security vulnerability CVE-2016-10149, XML External Entity (XXE) vulnerability in PySAML2 4.4.0 and earlier allows remote attackers to read arbitrary files via a crafted SAML XML request or response and it has a CVSS v3 Base Score of 7.5.

The above vulnerability has been fixed in version 4.5.0 as per https://github.com/rohe/pysaml2/issues/366. The latest version of pysaml2 (https://pypi.python.org/pypi/pysaml2/4.5.0) has this fix. However, the global requirements has the version set to < 4.0.3

https://github.com/openstack/requirements/blob/master/global-requirements.txt#L230
pysaml2>=4.0.2,<4.0.3

https://github.com/openstack/requirements/blob/master/upper-constraints.txt#L347
pysaml2===4.0.2

The version of pysaml2 supported for OpenStack should be updated such that OpenStack deployments are not vulnerable to the above mentioned CVE.

pysaml2 is used by OpenStack Keystone for identity Federation. This bug in itself is not a security vulnerability but not fixing this bug causes OpenStack deployments to be vulnerable.

[Jeremy Stanley (fungi)]

So... you're saying that Keystone allows untrusted users to pass crafted SAML XML requests or responses through routines in PySAML2?

[Divya K Konoor (dikonoor)]

Fungi, that's what the CVE leads us to believe.

[Matthew Thode (prometheanfire)]

https://github.com/gentoo/gentoo/blob/master/dev-python/pysaml2/files/xxe-4.0.2.patch and https://github.com/gentoo/gentoo/blob/master/dev-python/pysaml2/files/pysaml-4.0.2_CVE-2017-1000433.patch are the backported pysaml2 patches for 4.0.2 that we use. Though ya, it would be better to move to 4.5.0 even if only to remove the cap on it (from a requirements perspective).

[Jeremy Stanley (fungi)]

Divya: my question was more about how Keystone is using PySAML2. Does it pass arbitrary user-supplied SAML XML through the library, or does it create the XML itself in ways not directly under the control of an untrusted party?

[Divya K Konoor (dikonoor)]

Fungi, Colleen or Lance from Keystone might be in a better position to answer this.

[Colleen Murphy (krinkle)]

Keystone does not accept any user-supplied XML. Keystone uses pysaml2 to generate XML metadata based on config options and SAML assertions based on JSON API requests.

[Lance Bragstad (lbragstad)]

The usage of the PySAML2 library in keystone is isolated to a single module dedicated to identity provider functionality [0], which would make sense if we're dealing with SAML assertions. From what I can tell after briefly refreshing myself with the code, is that we use the library to generate SAML assertions based on a user's token. Instead of authenticating for a token, a user authenticates *with* a token for a SAML assertion they can give to a service provider (e.g. keystone-to-keystone federation.

From what I can tell, and consulting with other keystone developers who are more familiar with this area of the code, it is a POST call used for authentication that only requires the ID of a token [1].

Regardless, it doesn't sound like upgrading the requirement would hurt?

[0] https://github.com/openstack/keystone/blob/8948050c03252853d406ddea157633550cb639e4/keystone/federation/idp.py
[1] https://developer.openstack.org/api-ref/identity/v3-ext/index.html#generate-a-saml-assertion

[Morgan Fainberg (mdrnstm)]

This is an update we should do for the sake of being most correct forward looking. IT wont impact keystone since as Colleen said, we don't accept user-supplied XML.

This is a low-impact bug that should for best practices sake be addressed going forward (R-release and later) as long as there are no reasons we cannot include it.

[Jeremy Stanley (fungi)]

Thanks Keystoners! And yes, the reasons I was asking was so we could determine whether this use exposed the vulnerability in the library (in which case we'd want to update stable branch code/global requirements/upper constraints and issue an OSSN) or whether it can just be updated normally in the course of ongoing development as a measure of hygiene.

[Divya K Konoor (dikonoor)]

Lance / Colleen / fungi /Morgan, thanks for your quick responses. If I understood it right, the plan of action here is :
1. Update upper constraints to 4.5.0 for pysaml2 for Rocky and above.
2. Deployers of Queens and backward can go and apply patches on top of 4.0.2 (comment 3)

OpenStack/Keystone is NOT impacted by this vulnerability and the above is merely for good hygiene.

[Matthew Thode (prometheanfire)]

keystone, you can test by depending on this patch https://review.openstack.org/557434

[Lance Bragstad (lbragstad)]

Given the comments from the keystone team, I'm going to mark this as Low for the time being. The patch Matt linked in comment #11 also passed when depending on a bump of pysaml2 to version 4.5.0.

From a keystone perspective, we should be able to close this once the OpenStack Proposal Bot proposes an update of pysaml2 to keystone's requirements file.

[Lance Bragstad (lbragstad)]

Relevant context from IRC:

http://eavesdrop.openstack.org/irclogs/%23openstack-requirements/%23openstack-requirements.2018-03-29.log.html#t2018-03-29T20:58:08

[Lance Bragstad (lbragstad)]

We should be able to close this for keystone once Matt's patch to handle the pysaml2 bump merges [0].

[0] https://review.openstack.org/#/c/558217/