26 ms·

Coordinated disclosure of XML roundtrip vulnerabilities in Go’s standard library

223 points by jupenur 6y ago

random5634 6y ago
XML I'm amazed people can get it as right as they do half the time? I do think Go will get fixed eventually. It's just too weird if they couldn't fix the core issue? But I've never used XML if I can help it, so I'm absolutely no expert on what would make it impossible to fix something like this.
- jerf 6y ago
  They can fix the core issue... they can not do so while maintaining the 1.0 backwards compatibility promise. The data structures in encoding/xml in Go 1.0 are fundamentally incorrect for this use case.
  - random5634 6y ago
    Is you sense they will maintain the promise? That is commitment (and I wouldn't be surprised if true). Could they add a flag / toggle to the existing API you could toggle to change behavior or do they need an entire new API?
jarym 6y ago
Glad this got found. I remember when XML was being widely adopted that there'd be frequent vulnerabilities found in Java-based parsers. A large part of this stems from how complicated XML can get - if it were only elements and attributes it might have been fine. Namespaces made it a bit more complicated. Processing Instructions made it hideous.
- ben509 6y ago
  It's really nuts trying to implement something like SAML in XML. The standard is a security minefield.
  - tptacek 6y ago
    What makes it worse is that XMLDSIG is exponentially more complicated. Most of the ecosystem literally shells out to libxmlsec1 and assumes it does the right thing. DSIG is a batshit standard that attempts to support arbitrary combinations of signed and unsigned parts in a single document, tied together with a DOM-like scheme, passed through a canonicalizing transformation that has itself broken SAML before. It's a fractal of bad security design.
    - llimllib 6y ago
      Also it's my experience that nobody follows the standard - producing valid SAML isn't enough, you need to produce the exact SAML your consumer expects or receivers will reject it. (The context here was passing users off from healthcare.gov to issuers)
      - tptacek 6y ago
        What makes it worse is that there are practical reasons to implement that way; I've done so for clients, because of bugs found in other SAML parsers that we couldn't leave people susceptible to. One of the material things you can do to lock down a SAML implementation is to accept only the pattern of XML tokens you expect from mainstream IdPs, and then wait for people to complain.
        llimllib 6y ago
        I'm so happy I no longer need to work with it. I wrote a manifesto on how it (doesn't) work for the person that replaced me on that project, and it was long, detailed, and angry
    - bawolff 6y ago
      Not to mention that libxmlsec1 has some insane insecure defaults that are effectively undocumented. (I'd go into more details, but i literally just sent a security report yesterday to a saml library for using it wrong, so i guess i shouldn't post publicly about it until they fix)
- userbinator 6y ago
  XML and many other things related to it (including Java, SOAP, CORBA, etc.) are an example of what could be called "Enterprise mindset" taken to an extreme. Insanely high levels of abstraction and indirection, absurd amounts of needless flexibility and generality, and essentially zero thought given to efficiency or simplicity. It's as if the people responsible for these spent all their time thinking "what's the most complicated way to do something?"
warp 6y ago
> Despite significant efforts by the Go security team, it has not been possible to patch the vulnerabilities discussed in this blog post. Well, that is not something you want to see in a public disclosure.
- Thaxll 6y ago
  Because the standard lib was not designed to handle that use case and so they can't change it right now to not break compatibility, which is why they're going to add new API in Go 1.16 that will release in February.
  - Groxx 6y ago
    Which they also point out may be insufficient: >By Mattermost’s estimates this new API will not be a reasonable solution for most use cases currently affected by the vulnerabilities. Parsing and resolving namespaces is an essential requirement for correctly implementing SAML, and even considering only a limited set of real-world SAML messages without strict namespacing requirements would be unlikely to allow for a secure implementation.
    - tptacek 6y ago
      Yes: people shouldn't be using encoding/xml to implement SAML, at all. The library was already functionally problematic for SAML, because it doesn't fully implement namespaces. Nor does it implement `xml-exc-c14n`. For the IdP I wrote last year, I just wrote my own XML; it's not that big a deal. Software security people have understood for a long time that XMLDSIG is sketchy, and that implementations often need to be "bug-compatible" to interoperate safely. SAML is an XMLDSIG protocol. I feel bad for putting it this way, but I think that reasonably skilled security engineers should be alarmed if their platform's standard XML library easily allows you to implement something that claims to be DSIG.
      - jka 6y ago
        Meta-question: are software standards generally becoming more security-friendly over time?
jerf 6y ago
Anyone have examples of XML that can be mutated? My guess is that it wouldn't take much. I expect that a similar problem will be found in many other libraries, if the XML was publicized. XML namespaces made a critical... "mistake" is probably too strong, but "design choice that deviated too far from people's mental model" is about right... that has prevented them from being anywhere near as useful or safe as they could be. In an XML document using XML namespaces, "ns1:tagname" may not equal "ns1:tagname", and "ns1:tagname" can be equal to "ns2:tagname". This breaks people's mental models of how XML works, and correspondingly, breaks people's code that manipulates XML. (I actually used the Go XML library as an SVG validator in the ~1.8 timeframe and had to fork it to fix namespaces well enough to serve in that role. I didn't know about how to exploit it in a specific XML protocol but I've know about the issues for a while. "Why didn't you upstream it then?" Well, as this security bulletin implies, the data structures in encoding/xml are fundamentally wrong for namespaced XML to be round-tripped and there is no backwards-compatible solution to the problem, so it was obvious to me without even trying that it would be rejected. This has also been discussed on a number of tickets subsequently over the years, so that XML namespace handling is weak in the standard library is not news to the Go developers. Note also that it's "round-tripping" that is the problem; if you parse & consume you can write correct code, it's the sending it back out that can be problematic.) Namespaces fundamentally rewrite the nature of XML tag and attribute names. No longer are they just strings; now they are tuples of the form (namespace URL, tag name)... and namespace URL is NOT the prefix that shows up before the colon! The prefix is an abbreviation of an earlier tag declaration. So in the XML <tag xmlns="https://sample.com/1" xmlns:example1="https://blah.org/1"> <example1:tag xmlns:example2="https://blah.org/2"> <example2:tag xmlns:example1="https://anewsite.com/xmlns"> <example1:tag /> </example2:tag> </example1:tag> </tag> not a SINGLE ONE of those "tag"s is the same! They are, respectively, actually (https://sample.com/1 https://sample.com/1, tag), (https://blah.org/1 https://blah.org/1, tag), (https://blah.org/2 https://blah.org/2, tag), and (https://anewsite.com/xmlns https://anewsite.com/xmlns, tag). There's a ton of code, and indeed, even quite a few standards, that will get that wrong. (Note the redefinition of 'example1' in there; that is perfectly legal.) Even more excitingly, <tag xmlns="https://sample.com/1" xmlns:example1="https://sample.com/1"> <example1:tag/> <example2:tag xmlns:example2="https://sample.com/1" /> </tag> ARE all the exact tag and should be treated as such, despite the different "tag names" appearing. Reserializing these can be exciting, because A: Your XML library, in principle, ought to be presenting you the (XMLNS, tagname) tuple with the abbreviation stripped away, to discourage you from paying too much attention to the abbreviation but B: humans in general and a lot of code expect the namespace abbreviations to stay the same in a round trip, and may even standardize on what the abbreviations should be. There's a LOT of code out there in the world looking for "'p' or 'xhtml:p'" as the tag name and not ("http://www.w3.org/1999/xhtml http://www.w3.org/1999/xhtml", "p"). In general, to maintain roundtrip equality, you have to either A: maintain a table of the abbreviations you see, when they were introduced, and also which was used or B: just use the (XMLNS, tagname) and ensure that while outputing that the relevant namespaces have always been declared. Generally for me I go for option B as it's generally easier to get correct and I pair it with a table of the most common namespaces for what I'm working in, so that, for example, XHTML gets a hard-coded "xhtml:" prefix. It is very easy if you try to implement A to screw it up in a way that can corrupt the namespaces on some input. (Option B has its own pathologies. Consider: <tag xmlns:sample="https://example.com/1"> <sample:tag1 /> <sample:tag2 /> </tag> It's really easy to write code that will drop the xmlns specification on all of the children of "tag", since it didn't use it there, and if your code throws away where the XMLNS was declared and just looks to whether the NS is currently declared, it'll see a new declaration of the "sample" namespace on every usage. Technically correct if the downstream code handles namespaces correctly (big if!), but visually unappealing.) Not defending Go here, except inasmuch as it's such a common error to make that I have a hard time naming libraries and standards that get namespaces completely correct, for as simple as they are in principle. (I think SVG and XHTML have it right. XMPP is very, very close, but still has a few places where the "stream" tag is placed in different namespaces and you're just supposed to know to handle it the same in all the namespaces it appears it... which most people do only because it doesn't occur to them that technically these are separate tags, so it all kinda works out in the end.... libxml2 is correct but I've seen a lot of things that build on top of it and they almost all screw up namespaces.)
tedunangst 6y ago
I've never liked (nor understood the popularity) of signature schemes that require parsing before verification. This has also led to problems with X.509. And DKIM. And plists. And package managers. And more. It's much simpler to sign the entire message, unparsed, and it's immune to these issues. We went through a decade of debate before deciding that "encrypt then mac" is the only right way to do things. That knowledge hasn't trickled down to other domains.
- dboreham 6y ago
  I believe the problem being addressed is where the payload may be transcoded in flight or otherwise not delivered in exactly the same form. Put another way: the signature validates the payload, however it may end up being represented to the validator on delivery. It isn't simply a transport integrity measure.
  - hnarn 6y ago
    I might be missing the point here, but isn't the whole idea of signing a message that it should not be possible to "transcode the message in flight"? If you even allow the message to be "not delivered in exactly the same form" in the first place, you're introducing an attack vector completely without reason, because what you instead could do is let the payload be strongly signed and unchanged, and then have differing parsing rules at the end.
    - whatshisface 6y ago
      You sign your letter and seal it in an envelope. I put your envelope into a cardboard box and give it to your friend. Your friend refuses to open your letter because you did not sign my box.
      - jeltz 6y ago
        No, this is more like your friend refusing to trust the contents of the letter after the mailman cut the letter into small pieces and glued them back together.
forrestthewoods 6y ago
Can we just consider XML itself an unsolvable vulnerability and call it day?
- lyxsus 6y ago
  What do you suggest to use instead? It's a bit too early to use RDF, people haven't catch up yet.
  - tptacek 6y ago
    There is no good reason that the thing that 98% of people use SAML to do should involve XML.
chrsig 6y ago
It's worth noting that the go1 compatibility promise[1] allows for breaking compatibility in the name of security issues: >Security. A security issue in the specification or implementation may come to light whose resolution requires breaking compatibility. We reserve the right to address such security issues. If the go team decides that this issue is worth a breaking change is another question entirely. [1]https://golang.org/doc/go1compat https://golang.org/doc/go1compat
tannhaeuser 6y ago
XML namespaces were controversial when introduced, and their implementation as privileged "xmlns:..." attributes with complex scoping, layering, and defaulting rules have been criticized many times; see [1] for a reflection from 2010 by an insider admitting to the fact that "every step on the process that led to the current situation with XML Namespaces seems reasonable". When in 1996-98 W3C/The SGML Extended Review Board subset XML from SGML to define a generic markup convention for use with the expected wealth of upcoming vocabularies on the web, the issue of name collisions between elements (and attributes) from different vocabularies was deemed significant. Of course, in hindsight, with only SVG and MathML (and rarely HTML 5 in XHTML serialization) left on the web and having been incorporated as foreign elements directly into HTML, this seems overkill (even though there are actually collisions between eg. the title element in SVG vs HTML). There's an alternative (and saner IMHO) approach for dealing with XML namespaces in ISO/IEC 19757-9 [2] by just presenting a canonical (ie. always the same) namespace prefix as part of an element name by a parser API to an app, guided by processing instructions for binding canonical namespace prefixes to namespace URLs, which might also help enterprise-y XML with lots of XML Schema use. Of course, this doesn't help with roundtripping xmlns-bindings (eg. with their exact ordering, possible redundancy, temporary/insignificant namespace prefixes, re-binding in document fragments etc.) through DOM representations, which seems the problem here. [1]: https://blog.jclark.com/2010/01/xml-namespaces.html https://blog.jclark.com/2010/01/xml-namespaces.html [2]: https://www.iso.org/obp/ui/#iso:std:iso-iec:19757:-9:ed-1:v1:en https://www.iso.org/obp/ui/#iso:std:iso-iec:19757:-9:ed-1:v1...
nimish 6y ago
`encoding/xml` has had broken handling of namespaces for a long time. It’s possible to hack it on but the only reasonable choice is to use a libxml2 binding which also gets you canonicalization, another can of worms. Unsurprised it can cause security issues, especially in XML-DSig which is a nightmare to handle correctly.
- blablabla123 6y ago
  Yup, I think it becomes very quickly obvious when using `encoding/xml` with XMLs that have multiple namespaces that the handling is incomplete. Hard to believe such an xml could even survive one roundtrip. It's also documented that the implementation is incomplete: Mapping between XML elements and data structures is inherently flawed ... See package json for a textual representation more suitable to data structures.
russell_h 6y ago
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.
- zwass 6y ago
  As a service provider, are there viable alternatives to support SSO in an application?
  - toupeira 6y ago
    OpenID Connect seems to be pretty well established: https://openid.net/certification/ https://openid.net/certification/ It defines an authentication protocol on top of OAuth2, and is a different beast from the older OpenID standards.
GauntletWizard 6y ago
I'd like to ask everyone here who's familiar with SAML to take a look at SPIFFE[1], which underlies Istio. I'm biased in this regard, but I view SPIFFE's inclusion of JWT Tokens as an authentication method as fundamentally flawed - By allowing bearer tokens, you are no longer verifying identity, but passing identity around. JWT has also been susceptible in the past[2] to the same kinds of attacks here - Poorly defined verification semantics. I suspect that buried in the semantics around SPIFFE's SPIRE Server and Agent are a number of vulnerabilities or other ways that trust doesn't mean quite what you think it means. I'd love for someone with interest to take a look. Besides the obvious downsides fundamental to Isitio's MITM Proxy architecture, I think there's more lurking on that edge. [1] https://spiffe.io/ https://spiffe.io/ [2] https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/ https://auth0.com/blog/critical-vulnerabilities-in-json-web-...

223 points by jupenur 6y ago

XML I'm amazed people can get it as right as they do half the time? I do think Go will get fixed eventually. It's just too weird if they couldn't fix the core issue? But I've never used XML if I can help it, so I'm absolutely no expert on what would make it impossible to fix something like this.

jerf 6y ago
They can fix the core issue... they can not do so while maintaining the 1.0 backwards compatibility promise. The data structures in encoding/xml in Go 1.0 are fundamentally incorrect for this use case.
- random5634 6y ago
  Is you sense they will maintain the promise? That is commitment (and I wouldn't be surprised if true). Could they add a flag / toggle to the existing API you could toggle to change behavior or do they need an entire new API?

jarym 6y ago

Glad this got found. I remember when XML was being widely adopted that there'd be frequent vulnerabilities found in Java-based parsers. A large part of this stems from how complicated XML can get - if it were only elements and attributes it might have been fine. Namespaces made it a bit more complicated. Processing Instructions made it hideous.

ben509 6y ago
It's really nuts trying to implement something like SAML in XML. The standard is a security minefield.
- tptacek 6y ago
  What makes it worse is that XMLDSIG is exponentially more complicated. Most of the ecosystem literally shells out to libxmlsec1 and assumes it does the right thing. DSIG is a batshit standard that attempts to support arbitrary combinations of signed and unsigned parts in a single document, tied together with a DOM-like scheme, passed through a canonicalizing transformation that has itself broken SAML before. It's a fractal of bad security design.
  - llimllib 6y ago
    Also it's my experience that nobody follows the standard - producing valid SAML isn't enough, you need to produce the exact SAML your consumer expects or receivers will reject it. (The context here was passing users off from healthcare.gov to issuers)
    - tptacek 6y ago
      What makes it worse is that there are practical reasons to implement that way; I've done so for clients, because of bugs found in other SAML parsers that we couldn't leave people susceptible to. One of the material things you can do to lock down a SAML implementation is to accept only the pattern of XML tokens you expect from mainstream IdPs, and then wait for people to complain.
      - llimllib 6y ago
        I'm so happy I no longer need to work with it. I wrote a manifesto on how it (doesn't) work for the person that replaced me on that project, and it was long, detailed, and angry
  - bawolff 6y ago
    Not to mention that libxmlsec1 has some insane insecure defaults that are effectively undocumented. (I'd go into more details, but i literally just sent a security report yesterday to a saml library for using it wrong, so i guess i shouldn't post publicly about it until they fix)
userbinator 6y ago
XML and many other things related to it (including Java, SOAP, CORBA, etc.) are an example of what could be called "Enterprise mindset" taken to an extreme. Insanely high levels of abstraction and indirection, absurd amounts of needless flexibility and generality, and essentially zero thought given to efficiency or simplicity. It's as if the people responsible for these spent all their time thinking "what's the most complicated way to do something?"

warp 6y ago

> Despite significant efforts by the Go security team, it has not been possible to patch the vulnerabilities discussed in this blog post. Well, that is not something you want to see in a public disclosure.

Thaxll 6y ago
Because the standard lib was not designed to handle that use case and so they can't change it right now to not break compatibility, which is why they're going to add new API in Go 1.16 that will release in February.
- Groxx 6y ago
  Which they also point out may be insufficient: >By Mattermost’s estimates this new API will not be a reasonable solution for most use cases currently affected by the vulnerabilities. Parsing and resolving namespaces is an essential requirement for correctly implementing SAML, and even considering only a limited set of real-world SAML messages without strict namespacing requirements would be unlikely to allow for a secure implementation.
  - tptacek 6y ago
    Yes: people shouldn't be using encoding/xml to implement SAML, at all. The library was already functionally problematic for SAML, because it doesn't fully implement namespaces. Nor does it implement `xml-exc-c14n`. For the IdP I wrote last year, I just wrote my own XML; it's not that big a deal. Software security people have understood for a long time that XMLDSIG is sketchy, and that implementations often need to be "bug-compatible" to interoperate safely. SAML is an XMLDSIG protocol. I feel bad for putting it this way, but I think that reasonably skilled security engineers should be alarmed if their platform's standard XML library easily allows you to implement something that claims to be DSIG.
    - jka 6y ago
      Meta-question: are software standards generally becoming more security-friendly over time?

jerf 6y ago

Anyone have examples of XML that can be mutated? My guess is that it wouldn't take much. I expect that a similar problem will be found in many other libraries, if the XML was publicized. XML namespaces made a critical... "mistake" is probably too strong, but "design choice that deviated too far from people's mental model" is about right... that has prevented them from being anywhere near as useful or safe as they could be. In an XML document using XML namespaces, "ns1:tagname" may not equal "ns1:tagname", and "ns1:tagname" can be equal to "ns2:tagname". This breaks people's mental models of how XML works, and correspondingly, breaks people's code that manipulates XML. (I actually used the Go XML library as an SVG validator in the ~1.8 timeframe and had to fork it to fix namespaces well enough to serve in that role. I didn't know about how to exploit it in a specific XML protocol but I've know about the issues for a while. "Why didn't you upstream it then?" Well, as this security bulletin implies, the data structures in encoding/xml are fundamentally wrong for namespaced XML to be round-tripped and there is no backwards-compatible solution to the problem, so it was obvious to me without even trying that it would be rejected. This has also been discussed on a number of tickets subsequently over the years, so that XML namespace handling is weak in the standard library is not news to the Go developers. Note also that it's "round-tripping" that is the problem; if you parse & consume you can write correct code, it's the sending it back out that can be problematic.) Namespaces fundamentally rewrite the nature of XML tag and attribute names. No longer are they just strings; now they are tuples of the form (namespace URL, tag name)... and namespace URL is NOT the prefix that shows up before the colon! The prefix is an abbreviation of an earlier tag declaration. So in the XML <tag xmlns="https://sample.com/1" xmlns:example1="https://blah.org/1"> <example1:tag xmlns:example2="https://blah.org/2"> <example2:tag xmlns:example1="https://anewsite.com/xmlns"> <example1:tag /> </example2:tag> </example1:tag> </tag> not a SINGLE ONE of those "tag"s is the same! They are, respectively, actually (https://sample.com/1 https://sample.com/1, tag), (https://blah.org/1 https://blah.org/1, tag), (https://blah.org/2 https://blah.org/2, tag), and (https://anewsite.com/xmlns https://anewsite.com/xmlns, tag). There's a ton of code, and indeed, even quite a few standards, that will get that wrong. (Note the redefinition of 'example1' in there; that is perfectly legal.) Even more excitingly, <tag xmlns="https://sample.com/1" xmlns:example1="https://sample.com/1"> <example1:tag/> <example2:tag xmlns:example2="https://sample.com/1" /> </tag> ARE all the exact tag and should be treated as such, despite the different "tag names" appearing. Reserializing these can be exciting, because A: Your XML library, in principle, ought to be presenting you the (XMLNS, tagname) tuple with the abbreviation stripped away, to discourage you from paying too much attention to the abbreviation but B: humans in general and a lot of code expect the namespace abbreviations to stay the same in a round trip, and may even standardize on what the abbreviations should be. There's a LOT of code out there in the world looking for "'p' or 'xhtml:p'" as the tag name and not ("http://www.w3.org/1999/xhtml http://www.w3.org/1999/xhtml", "p"). In general, to maintain roundtrip equality, you have to either A: maintain a table of the abbreviations you see, when they were introduced, and also which was used or B: just use the (XMLNS, tagname) and ensure that while outputing that the relevant namespaces have always been declared. Generally for me I go for option B as it's generally easier to get correct and I pair it with a table of the most common namespaces for what I'm working in, so that, for example, XHTML gets a hard-coded "xhtml:" prefix. It is very easy if you try to implement A to screw it up in a way that can corrupt the namespaces on some input. (Option B has its own pathologies. Consider: <tag xmlns:sample="https://example.com/1"> <sample:tag1 /> <sample:tag2 /> </tag> It's really easy to write code that will drop the xmlns specification on all of the children of "tag", since it didn't use it there, and if your code throws away where the XMLNS was declared and just looks to whether the NS is currently declared, it'll see a new declaration of the "sample" namespace on every usage. Technically correct if the downstream code handles namespaces correctly (big if!), but visually unappealing.) Not defending Go here, except inasmuch as it's such a common error to make that I have a hard time naming libraries and standards that get namespaces completely correct, for as simple as they are in principle. (I think SVG and XHTML have it right. XMPP is very, very close, but still has a few places where the "stream" tag is placed in different namespaces and you're just supposed to know to handle it the same in all the namespaces it appears it... which most people do only because it doesn't occur to them that technically these are separate tags, so it all kinda works out in the end.... libxml2 is correct but I've seen a lot of things that build on top of it and they almost all screw up namespaces.)

tedunangst 6y ago

I've never liked (nor understood the popularity) of signature schemes that require parsing before verification. This has also led to problems with X.509. And DKIM. And plists. And package managers. And more. It's much simpler to sign the entire message, unparsed, and it's immune to these issues. We went through a decade of debate before deciding that "encrypt then mac" is the only right way to do things. That knowledge hasn't trickled down to other domains.

dboreham 6y ago
I believe the problem being addressed is where the payload may be transcoded in flight or otherwise not delivered in exactly the same form. Put another way: the signature validates the payload, however it may end up being represented to the validator on delivery. It isn't simply a transport integrity measure.
- hnarn 6y ago
  I might be missing the point here, but isn't the whole idea of signing a message that it should not be possible to "transcode the message in flight"? If you even allow the message to be "not delivered in exactly the same form" in the first place, you're introducing an attack vector completely without reason, because what you instead could do is let the payload be strongly signed and unchanged, and then have differing parsing rules at the end.
  - whatshisface 6y ago
    You sign your letter and seal it in an envelope. I put your envelope into a cardboard box and give it to your friend. Your friend refuses to open your letter because you did not sign my box.
    - jeltz 6y ago
      No, this is more like your friend refusing to trust the contents of the letter after the mailman cut the letter into small pieces and glued them back together.

forrestthewoods 6y ago

Can we just consider XML itself an unsolvable vulnerability and call it day?

lyxsus 6y ago
What do you suggest to use instead? It's a bit too early to use RDF, people haven't catch up yet.
- tptacek 6y ago
  There is no good reason that the thing that 98% of people use SAML to do should involve XML.

chrsig 6y ago

It's worth noting that the go1 compatibility promise[1] allows for breaking compatibility in the name of security issues: >Security. A security issue in the specification or implementation may come to light whose resolution requires breaking compatibility. We reserve the right to address such security issues. If the go team decides that this issue is worth a breaking change is another question entirely. [1]https://golang.org/doc/go1compat https://golang.org/doc/go1compat

tannhaeuser 6y ago

XML namespaces were controversial when introduced, and their implementation as privileged "xmlns:..." attributes with complex scoping, layering, and defaulting rules have been criticized many times; see [1] for a reflection from 2010 by an insider admitting to the fact that "every step on the process that led to the current situation with XML Namespaces seems reasonable". When in 1996-98 W3C/The SGML Extended Review Board subset XML from SGML to define a generic markup convention for use with the expected wealth of upcoming vocabularies on the web, the issue of name collisions between elements (and attributes) from different vocabularies was deemed significant. Of course, in hindsight, with only SVG and MathML (and rarely HTML 5 in XHTML serialization) left on the web and having been incorporated as foreign elements directly into HTML, this seems overkill (even though there are actually collisions between eg. the title element in SVG vs HTML). There's an alternative (and saner IMHO) approach for dealing with XML namespaces in ISO/IEC 19757-9 [2] by just presenting a canonical (ie. always the same) namespace prefix as part of an element name by a parser API to an app, guided by processing instructions for binding canonical namespace prefixes to namespace URLs, which might also help enterprise-y XML with lots of XML Schema use. Of course, this doesn't help with roundtripping xmlns-bindings (eg. with their exact ordering, possible redundancy, temporary/insignificant namespace prefixes, re-binding in document fragments etc.) through DOM representations, which seems the problem here. [1]: https://blog.jclark.com/2010/01/xml-namespaces.html https://blog.jclark.com/2010/01/xml-namespaces.html [2]: https://www.iso.org/obp/ui/#iso:std:iso-iec:19757:-9:ed-1:v1:en https://www.iso.org/obp/ui/#iso:std:iso-iec:19757:-9:ed-1:v1...

nimish 6y ago

`encoding/xml` has had broken handling of namespaces for a long time. It’s possible to hack it on but the only reasonable choice is to use a libxml2 binding which also gets you canonicalization, another can of worms. Unsurprised it can cause security issues, especially in XML-DSig which is a nightmare to handle correctly.

blablabla123 6y ago
Yup, I think it becomes very quickly obvious when using `encoding/xml` with XMLs that have multiple namespaces that the handling is incomplete. Hard to believe such an xml could even survive one roundtrip. It's also documented that the implementation is incomplete: Mapping between XML elements and data structures is inherently flawed ... See package json for a textual representation more suitable to data structures.

russell_h 6y ago

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.

zwass 6y ago
As a service provider, are there viable alternatives to support SSO in an application?
- toupeira 6y ago
  OpenID Connect seems to be pretty well established: https://openid.net/certification/ https://openid.net/certification/ It defines an authentication protocol on top of OAuth2, and is a different beast from the older OpenID standards.

GauntletWizard 6y ago

I'd like to ask everyone here who's familiar with SAML to take a look at SPIFFE[1], which underlies Istio. I'm biased in this regard, but I view SPIFFE's inclusion of JWT Tokens as an authentication method as fundamentally flawed - By allowing bearer tokens, you are no longer verifying identity, but passing identity around. JWT has also been susceptible in the past[2] to the same kinds of attacks here - Poorly defined verification semantics. I suspect that buried in the semantics around SPIFFE's SPIRE Server and Agent are a number of vulnerabilities or other ways that trust doesn't mean quite what you think it means. I'd love for someone with interest to take a look. Besides the obvious downsides fundamental to Isitio's MITM Proxy architecture, I think there's more lurking on that edge. [1] https://spiffe.io/ https://spiffe.io/ [2] https://auth0.com/blog/critical-vulnerabilities-in-json-web-token-libraries/ https://auth0.com/blog/critical-vulnerabilities-in-json-web-...