4 ms·

Microsoft's SignedXml implementation in the .Net framework fixed this 8 years ago so long as you are correctly using the GetIdElement which makes sure there are

Microsoft's SignedXml implementation in the .Net framework fixed this 8 years ago so long as you are correctly using the GetIdElement which makes sure there are no duplicates. https://coding.abel.nu/2016/03/vulnerability-in-net-signedxml/ https://coding.abel.nu/2016/03/vulnerability-in-net-signedxm...

jrpelkonen 2y ago
I know next to nothing about .net, but this seems like the classic “you’re holding it wrong” excuse to me. If there’s a way to call an api the wrong way and the right way, and both appear to work, large number of developers will implement the insecure api. Why can’t the incorrect api be removed? I understand there’s pressure to support old client code but vulnerabilities should trump backwards compatibility.
- SigmundA 2y ago
  The incorrect api would be using GetXml and looking at the raw XmlElement and using select nodes or something vs using the GetIdElement on the SignedXML object itself, its not going to prevent you from looking at the xml document directly and do something incorrect but it gives you a correct helper method right next to CheckSignature to do the right thing. I mean at some point you do have to understand the difference between xml and a specific schema of it and how its used in SAML, its not like xml elements are required to have a unique id attribute. This isn't something you would call directly anyway unless you were writing your own SAML client, which isn't that hard but there are existing ones, here is a simple one that works well: https://github.com/jitbit/AspNetSaml https://github.com/jitbit/AspNetSaml
silon42 2y ago
And they "forgot" to tell people that if doing this properly, they need to use a schema/DTD where Id is defined as ID and then guaranteed unique by XML parser. I've seen invalid schemas/signatures where Id was just defined as string in the schema (fails when verifying using libxml/xmlsec for example)
- SigmundA 2y ago
  GetElementId makes sure its a NCName and makes sure its unique regardless, you can view the source here: https://github.com/dotnet/runtime/blob/c4d7f7c6f2e2f34f07e64c6caa3bf9b2ce915cc1/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs#L557 https://github.com/dotnet/runtime/blob/c4d7f7c6f2e2f34f07e64...

jrpelkonen 2y ago

I know next to nothing about .net, but this seems like the classic “you’re holding it wrong” excuse to me. If there’s a way to call an api the wrong way and the right way, and both appear to work, large number of developers will implement the insecure api. Why can’t the incorrect api be removed? I understand there’s pressure to support old client code but vulnerabilities should trump backwards compatibility.

SigmundA 2y ago
The incorrect api would be using GetXml and looking at the raw XmlElement and using select nodes or something vs using the GetIdElement on the SignedXML object itself, its not going to prevent you from looking at the xml document directly and do something incorrect but it gives you a correct helper method right next to CheckSignature to do the right thing. I mean at some point you do have to understand the difference between xml and a specific schema of it and how its used in SAML, its not like xml elements are required to have a unique id attribute. This isn't something you would call directly anyway unless you were writing your own SAML client, which isn't that hard but there are existing ones, here is a simple one that works well: https://github.com/jitbit/AspNetSaml https://github.com/jitbit/AspNetSaml

silon42 2y ago

And they "forgot" to tell people that if doing this properly, they need to use a schema/DTD where Id is defined as ID and then guaranteed unique by XML parser. I've seen invalid schemas/signatures where Id was just defined as string in the schema (fails when verifying using libxml/xmlsec for example)

SigmundA 2y ago
GetElementId makes sure its a NCName and makes sure its unique regardless, you can view the source here: https://github.com/dotnet/runtime/blob/c4d7f7c6f2e2f34f07e64c6caa3bf9b2ce915cc1/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/SignedXml.cs#L557 https://github.com/dotnet/runtime/blob/c4d7f7c6f2e2f34f07e64...