keystoneauth auth plugins should not use etree XML parsing

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

So this affect the saml2 authentication plugin and the exploit needs to first impersonate an identity provider in order to answer with malicious xml content ?

It can either be the Service Provider response or the Identity Provider response. These values are both parsed using lxml.etree.XML.

keystoneauth has this same code.

Adding keystone-coresec for further investigation.

So in order to impersonate Service Provider response or the Identity Provider response, these endpoints need to be in http right ? If so, this is likely a C1 type of bug (according to https://security.openstack.org/vmt-process.html#incident-report-taxonomy ).

So does this mean that (say in a federated auth model) any malicious actor at a configured IdP can crash services calling into keystoneauth?

Jeremy - my understanding is that, yes, if you control the SP or the IdP then you can crash any application using keystoneauth / keystoneclient when the system is configured for SAML authentication (federation) by crafting a response to exploit the expansion function in lxml.etree. It's possible the SP/IdP might even be able to read a file off the system via external entity expansion... I'll have to look at the code some more... the client takes the value sent to it and then modifies it for reuse without getting rid of stuff that would be invalid or ignored as far as I can tell.

If somebody controls the SP and/or IdP then there's probably all sorts of other badness that they can get up to that's worse than crashing the applications. Keystone does try to protect the deployment from some of these things (for example, the mapping needs to be valid and will therefore limit what the SP and IdP can get up to... a sane configuration would prevent the SP/IdP from becoming admin through the mapping).

So, keystone will protect against some bad actions by SP/IdP. The deployer might think that this makes them "safe" and trust their external IdP. The SP/IdP admin could exploit this to crash their (or their customers') systems (or read files off their systems?) by crafting bad responses.

Tristan - I wouldn't object to a C1 classification. I don't know of a real exploit here, this is all speculative.

Found an interesting reference: http://blog.sendsafely.com/web-based-single-sign-on-and-the-dangers-of-saml-xml-parsing

So, this likely can't crash keystone but could cause issues with the client. Depends on how apache_saml2 processing handles this problem.

This is not likely to hit anything outside of client(s). It doesn't change the risk of ugly things.

This also likely can be made public, but I'm happy to let brant think that over a bit more.

Ah you did say it was only consumers of ksa not keystoneserver. Sorry.

I've removed the privacy settings and put the OSSA task as Won't Fix based on above comments. This can be put back to incomplete if the situation changes.

Marking this as "wont fix" against KSC as it shouldn't be needed long term (Session, AuthPlugins, and CLI are all being removed soon/are deprecated)

Unfortunately defusedxml is not part of global-requirements and that's not a battle i care to have. However we are also wanting to move away from lxml to remove the C dependencies that lxml bring. So alternative options would be appreciated.

For a shorter term fix, googling the issue brings up [1], suggesting we can do:

    from lxml import etree
    parser = etree.XMLParser(resolve_entities=False)

I think (i haven't looked closely) that this should be safe for SAML. Can we update bandit to check for this specifically?

[1] http://mikeknoop.com/lxml-xxe-exploit/

Fix proposed to branch: master
Review: https://review.openstack.org/536761

Unassigning due to inactivity

Change abandoned by "Gage Hugo <email address hidden>" on branch: master
Review: https://review.opendev.org/c/openstack/keystoneauth/+/536761
Reason: Abandoning since there hasn't been any recent activity, if anyone wants to continue this work, please feel free to restore this or create a new change.