I'm the maintainer of one of the affected SAML libraries.
People need to stop using SAML. This needs to be a priority. A little background, for those who haven't had the displeasure of working with it:
When a user wants to log into an application (the "Service Provider"), and is required to SSO against an "Identity Provider", the Identity Provider basically generates an XML document with information about the user, then signs that document using a thing known as an XML Digital Signature, or XMLDSIG.
When you think of "signing" a document, normally you would serialize that document out to bytes, apply your signature scheme over the bytes, then send along both the bytes and the signature. But for reasons which are irrelevant to modern implementations, XMLDSIG prefers to stuff the signature metadata back inside the XML document that was just signed. Obviously this invalidates the signature, so you also inject some metadata instructing receivers on how to put the document back how it was. There are several algorithms available for this. Then you ship around that XML document. Basically means that when the Identity Provider receives one of these documents it needs to:
1. Parse the XML document (which cannot yet be trusted)
2. Find the signature inside the document
3. Find the metadata about what algorithm(s) to use to restore the document
4. Run the document through whatever transforms are described in that metadata (keep in mind that up to this point the document might well have been supplied by an attacker)
5. Serialize the transformed document back out to bytes, being careful not to touch any whitespace, etc
6. Verify the signature over the re-serialized document
If all of this succeeds and was implemented perfectly, you can trust the output of step 5. Ideally you should re-parse it. A common failure mode is trusting the original input instead, so be careful about that.
Obviously this is a crazy approach to one of the most security-critical parts of an application on the internet, and it breaks all the time.
Unfortunately people persist in using this fundamentally broken protocol, so huge thank you to the team at Mattermost for their research in this area.
I share your opinion of SAML, but I have to ask, as someone who has also implemented it in Golang: what gave you any confidence in an implementation backed by encoding/xml? It was to me immediately pretty obvious that DSIG and encoding/xml aren't a fit, if only because of encoding/xml's poor namespace support. There are other DSIG Golang libraries that use an etree-style interface for what I presume is the same reason.
Blog author here; Russell's implementation is backed by github.com/beevik/etree, but like you said, it's just an interface. The tokenizer is still encoding/xml.
Adding better support for namespaces and providing APIs compatible with dsig doesn't remove the underlying vulnerabilities.
Ugh. That's disappointing. I loathe SAML, but also think the right thing to do here is to make sure nobody uses encoding/xml as part of their SAML stack.
I don't know about that. libxml certainly doesn't round-trip XML documents in general (though I don't think it breaks namespaces at least), whether that breaks SAML or not I have no idea.
Anyway from tptacek's other comments it looks like general-purpose XML libraries should not be assumed suitable for SAML, instead they should have purpose-built implementation for the SAML bit, then once the document has been properly validated and the SAML bits stripped off I guess that can be passed onto a general-purpose library:
> SAML libraries should include purpose-built, locked-down, SAML-only XMLDSIGs, and those XMLDSIGs should include purpose-built, stripped-down XMLs.
I would go out of my way to avoid libxmlsec1 and libxml. I honestly don't understand why it's so hard for a SAML implementation to just bring its own hardened stripped-down XML.
If I had to hazard a guess, bespoke implementation is usually recommended against, especially for complex formats. That it would be the best practice for saml does sound counter-intuitive.
This is like saying that variable name scoping is a semantic convention on top of the C language grammar and that a lexer can't really implement it. In the case of C, it turns out that the lexer must implement it. In the case of XML, processing name spaces directives during lexing is the right thing to do in nearly all cases. But it's not what these SAML libraries needed.
OpenID Connect, like others mention, but there's more in life, than only SSO? Organizations also want to automatically deactivate of user accounts?
There's something called SCIM, "System for Cross-domain Identity Management", that does this, and which you can use together with OpenID Connect (OIDC).
SCIM can automatically deactivate a user account, if the person leaves the organization or moves to a different department. And can auto add and remove him/her to/from various user groups.
But with SAML, managers / admins still need to micro manage the user accounts, e.g. place the user in the correct group, if s/he gets a new job role. SAML only syncs user accounts upon login, from what I've understood. (So if the user stays logged in, then, with SAML, his/her account permissions can get out-of-date?)
Depending on who your customers are, you might get away with only supporting OIDC. But not supporting SAML is going to be a problem as you move into the big enterprises.
Many big companies run on SAML, and expect to auth with vendors over SAML. That's why russell_h's comment is probably futile; it's the enterprises with the big SaaS budgets that keep SAML relevant, and they don't care if HN doesn't like it.
Maybe in about a decade SAML will be less important to enterprises? SAML 2.0 is only about 15 years old.
SCIM is a pleasure to implement compared to SAML, no doubt. You might be able to get away with only supporting SCIM, the main thing you'd be missing is "just-in-time" user provisioning.
But given that you'll probably need SCIM at some point anyway, probably a good idea to start with SCIM, and then add SAML only when you need to! It'll also inform what subset of SAML you actually need to implement.
> good idea to start with SCIM, and then add SAML only when you need to
Sounds like a good approach yes. (It seems you've added SCIM to some software? About how long did it take? Was there any "gotchas")
> the main thing you'd be missing is "just-in-time" user provisioning
Hmm could that depend on the organization using the software I'm developing? — Possibly they'll synchronize user accounts and groups, upon installation of the software, and whenever anything changes — and then all user accounts will be ready already, when someone wants to log in.
But if they syncronize only, say, once a day, then, with SAML, one could still log in, and the account would get created and added to the correct groups, also if the sync that would create one's account, hadn't happened yet? (OIDC could help a bit, but it doesn't understand user groups and permissions, only SAML and SCIM does, right.)
It looks intimidating, but it seems to me that most of what happens are standard XML operations. E.g. 1: read XML, 2: use XPath, 3: read element names, structure, and attributes, 4, some parts of: apply XPath or XSLT, if specified, 5: serialize XML (which implies whitespace treatment). Given that the authors of the standard considered the standard XML operations to be readily available, the cryptographic additions do not seem to be that complex.
I just wrapped up a Go based SAML integration and I was initially using your package. I encounter a bunch of issues with namespaces, and eventually ended up using xmlsec1 directly against a formed up XML template. I agree with you, SAML should go away. This was my first experience with it, and the nuanced complexities were so exhausting. I’m glad the integration is done...
I wonder how hard would it be to adopt some SAML 2.0 (or maybe just 1.x) with this, and maybe a few other problematic bits updated, but otherwise unchanged? Do you think the rest is worth keeping?
E.g. we did not stop using TLS when TLS 1.0 proved to have problems; we updated the cryptography and kept using the logic.
But the problem here isn't he encryption. Well, for all I know, the encryption could be completely broken, I'm not a crypto-expert.
But the problem described in the post wasn't the encryption. It was the logic. Specifically the order that things are done in. Parsing something before verifying it can be dangerous.
Indeed! Let's scratch the XMLDSIG entirely and replace it with a sane scheme.
Does SAML have enough salvageable parts to try fixing that, instead of going with something completely different? SAML is so pervasive that migrating off it can't be cheap or easy.
> But for reasons which are irrelevant to modern implementations, XMLDSIG prefers to stuff the signature metadata back inside the XML document that was just signed.
People need to stop using SAML. This needs to be a priority. A little background, for those who haven't had the displeasure of working with it:
When a user wants to log into an application (the "Service Provider"), and is required to SSO against an "Identity Provider", the Identity Provider basically generates an XML document with information about the user, then signs that document using a thing known as an XML Digital Signature, or XMLDSIG.
When you think of "signing" a document, normally you would serialize that document out to bytes, apply your signature scheme over the bytes, then send along both the bytes and the signature. But for reasons which are irrelevant to modern implementations, XMLDSIG prefers to stuff the signature metadata back inside the XML document that was just signed. Obviously this invalidates the signature, so you also inject some metadata instructing receivers on how to put the document back how it was. There are several algorithms available for this. Then you ship around that XML document. Basically means that when the Identity Provider receives one of these documents it needs to:
If all of this succeeds and was implemented perfectly, you can trust the output of step 5. Ideally you should re-parse it. A common failure mode is trusting the original input instead, so be careful about that.Obviously this is a crazy approach to one of the most security-critical parts of an application on the internet, and it breaks all the time.
Unfortunately people persist in using this fundamentally broken protocol, so huge thank you to the team at Mattermost for their research in this area.