.NET’s SignedXML class has had a risky implementation for lookup of XML elements by id in GetIdElement()
when resolving signed xml references. The lookup validated only the first element if there are several with the same id. This opens up for XML Signature Wrapping attacks in any library that is using the default implementation without taking necessary precautions. For SAML2 libraries signature wrapping is a well known attack class with very severe implications.
I reported this privately to Microsoft on December 3rd 2015. They responded (as promised within 24 hours) that they would investigate. The vulnerability was assigned ids CVE-2016-0132 and MS16-035. A fix was released on “patch Tuesday” in March 2016 (and yes, I’m proud to be listed in the acknowledgement section). The fix also contains a number of related breaking changes.
This is an example of a signed XML document with data that might be incorrectly trusted.
<r>
<b Id="q">
<data>Valid data</data>
<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<SignedInfo>
<CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
<Reference URI="#q">
<Transforms>
<Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
</Transforms>
<DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
<DigestValue>Drhn/EC2O7JBZwj9lS/kdS8RYis=</DigestValue>
</Reference>
</SignedInfo>
<SignatureValue>T2hBKCNuonADXznJ2IT/cIH2ZB/8/WvLpywVH3ebWwN9EDKt5T4n4NC7/rhWTFMX3pacGNzS0oDcEe7iYBW05eJou2XGzX+GXD+I8nPE7nXOQzVYZDnN+1tGfn35L1z86iZHyUXZsTwJ1FA9VZk3ph6zCAn5YmBYg495fg2chFI=</SignatureValue>
</Signature>
</b>
<b Id="q">
<data>Some false data</data>
</b>
</r> |
<r>
<b Id="q">
<data>Valid data</data>
<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<SignedInfo>
<CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
<Reference URI="#q">
<Transforms>
<Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
</Transforms>
<DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
<DigestValue>Drhn/EC2O7JBZwj9lS/kdS8RYis=</DigestValue>
</Reference>
</SignedInfo>
<SignatureValue>T2hBKCNuonADXznJ2IT/cIH2ZB/8/WvLpywVH3ebWwN9EDKt5T4n4NC7/rhWTFMX3pacGNzS0oDcEe7iYBW05eJou2XGzX+GXD+I8nPE7nXOQzVYZDnN+1tGfn35L1z86iZHyUXZsTwJ1FA9VZk3ph6zCAn5YmBYg495fg2chFI=</SignatureValue>
</Signature>
</b>
<b Id="q">
<data>Some false data</data>
</b>
</r>
The document demonstrates how two elements have the same id. The unpatched SignedXml.GetIdElement()
method will only find and validate the first occurrence of the id, but code that loops all nodes and checks that the id is present in the signature’s references will trust both <b>
nodes.
Last week I showed a peculiar XML Signature that validates even though the containing document was changed. The reason is that the signature lacks References. Before explaining what’s wrong with the signature – and with the validation code, we’ll have a look at how XML Signatures work.
XML DSig Primer
XML in general is a powerful beast, with so many options available that it quickly gets really complex. The XML Digital Signatures standard is no exception to that. The extra features complexity of XML DSig compared to other signature standard is that one or more different blocks of data can be signed by the same signature block. That data can be the containing XML Document, part of an XML document or some other resource such as a web page. In this post we’ll only look at signing resources in the document containing the signature.
XML Signatures are powerful, but also a bit tricky to get right. Here’s a challenge: I have a signature that will validate, even though the contents of the XML document are altered.
This is the “magic” signature that validates regardless of what XML document it is placed in.
<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<SignedInfo>
<CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
</SignedInfo>
<SignatureValue>ZlSHRu+tG6bVTPxyKvp2PcNqg+lVmBSB2TnzABa2eJZiFcIBtD4/0dhT8Mhr+TbcpzGTbPgMFOx1JmZzfQVzbcMUJfnU674C368VlULAnRa3Avqb+MNJWdAIT8cgdB1yJyZbtuChww13e2CMhikW+xZhaZFzD11KTKqiR+7DRcd+k026w7y6JYy/XgUkS3y1MzuUFO0Uk+tIKI2ik/iLH0XvO0TFO+5uLoz3VaVYj4BIgwFZlYFE/y36EVTeVC4fW7bxPiLdKfZNMin2ZpOusgr/Lyp+J5NQBUOV1FklGsHJbFLN7GJcaUBwnmAib8yReVmAIjsEWBRhB5b6TLojbw==</SignatureValue>
<KeyInfo>
<X509Data>
<X509Certificate>MIIGGDCCBQCgAwIBAgIDDdIRMA0GCSqGSIb3DQEBBQUAMIGMMQswCQYDVQQGEwJJTDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0YWwgQ2VydGlmaWNhdGUgU2lnbmluZzE4MDYGA1UEAxMvU3RhcnRDb20gQ2xhc3MgMSBQcmltYXJ5IEludGVybWVkaWF0ZSBDbGllbnQgQ0EwHhcNMTUwNDI0MTIzMjU4WhcNMTYwNDI0MDQzMDI4WjA4MRcwFQYDVQQDDA5hbmRlcnNAYWJlbC5udTEdMBsGCSqGSIb3DQEJARYOYW5kZXJzQGFiZWwubnUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCeNuAxBAFeueFVtptqq6xY8GetxhZYI3ytgNG3IEHWzN4Pf9Ql58V0dUdYBSDZENOv9WArr4sLkpsB5U9n/C/6pn5auN1Db6GQyo7zTlrs+duuwpcWIodoGrrutqOdabIULrxDbQ+nbCelH64iBCh3YtzsPnGjHUo6JlpfVYau99d7Oh/+8dOKqT1TVs/w1mt1l3AEYSM3SfBx0L8k2xuzi42EcGIySJj2U3eFYqv/kO1sIg/X3Kvuo4CWz4hHGdeRNyZatBZyXtE7SZNdGjvP+D+iuqYH8gWPVTO8TQg9QY+/BpVBfa9JSfSwi4KcAe/a7m/qkyu+gBJ9M21cJXEZAgMBAAGjggLUMIIC0DAJBgNVHRMEAjAAMAsGA1UdDwQEAwIEsDAdBgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwQwHQYDVR0OBBYEFDZrDRuDS0GGF8c3iC9drCI9p63oMB8GA1UdIwQYMBaAFFNy7ZKc4NrLAVx8fpY1TvLUuFGCMBkGA1UdEQQSMBCBDmFuZGVyc0BhYmVsLm51MIIBTAYDVR0gBIIBQzCCAT8wggE7BgsrBgEEAYG1NwECAzCCASowLgYIKwYBBQUHAgEWImh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeS5wZGYwgfcGCCsGAQUFBwICMIHqMCcWIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MAMCAQEagb5UaGlzIGNlcnRpZmljYXRlIHdhcyBpc3N1ZWQgYWNjb3JkaW5nIHRvIHRoZSBDbGFzcyAxIFZhbGlkYXRpb24gcmVxdWlyZW1lbnRzIG9mIHRoZSBTdGFydENvbSBDQSBwb2xpY3ksIHJlbGlhbmNlIG9ubHkgZm9yIHRoZSBpbnRlbmRlZCBwdXJwb3NlIGluIGNvbXBsaWFuY2Ugb2YgdGhlIHJlbHlpbmcgcGFydHkgb2JsaWdhdGlvbnMuMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6Ly9jcmwuc3RhcnRzc2wuY29tL2NydHUxLWNybC5jcmwwgY4GCCsGAQUFBwEBBIGBMH8wOQYIKwYBBQUHMAGGLWh0dHA6Ly9vY3NwLnN0YXJ0c3NsLmNvbS9zdWIvY2xhc3MxL2NsaWVudC9jYTBCBggrBgEFBQcwAoY2aHR0cDovL2FpYS5zdGFydHNzbC5jb20vY2VydHMvc3ViLmNsYXNzMS5jbGllbnQuY2EuY3J0MCMGA1UdEgQcMBqGGGh0dHA6Ly93d3cuc3RhcnRzc2wuY29tLzANBgkqhkiG9w0BAQUFAAOCAQEAPqjFybD6u6vLrRWZNe61WVZK2GMuv3Sm03acHLZIy8+zfoQfM9NhPvsmDmPpAelujJMOHISIkUxBB2qL9y/fDPqMsOg2wXIAJ96in+rmvjmnUd75bdbHJtVKOyx2m2HAjy4kA5bBF96asE0NVVTr8VR4N/NAoO0jPlNDLe8L5M5n1R4rttfjZ8IGQ6++DkGsatD8jh2ZuZUoI8q4gEo+8WKvBmX6bbRhp69ZO7eZoV+KBW66y5DfnBI0LHT0gC93otSO4wClyj8KGmRaAFzIt0f+f7VXl3Zjttc0lUF8mJoqnGDXByg9Q50h0nGp+rxjdURYkWW+WHvnVWo3ejSYiQ==</X509Certificate>
</X509Data>
</KeyInfo>
</Signature> |
<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<SignedInfo>
<CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
</SignedInfo>
<SignatureValue>ZlSHRu+tG6bVTPxyKvp2PcNqg+lVmBSB2TnzABa2eJZiFcIBtD4/0dhT8Mhr+TbcpzGTbPgMFOx1JmZzfQVzbcMUJfnU674C368VlULAnRa3Avqb+MNJWdAIT8cgdB1yJyZbtuChww13e2CMhikW+xZhaZFzD11KTKqiR+7DRcd+k026w7y6JYy/XgUkS3y1MzuUFO0Uk+tIKI2ik/iLH0XvO0TFO+5uLoz3VaVYj4BIgwFZlYFE/y36EVTeVC4fW7bxPiLdKfZNMin2ZpOusgr/Lyp+J5NQBUOV1FklGsHJbFLN7GJcaUBwnmAib8yReVmAIjsEWBRhB5b6TLojbw==</SignatureValue>
<KeyInfo>
<X509Data>
<X509Certificate>MIIGGDCCBQCgAwIBAgIDDdIRMA0GCSqGSIb3DQEBBQUAMIGMMQswCQYDVQQGEwJJTDEWMBQGA1UEChMNU3RhcnRDb20gTHRkLjErMCkGA1UECxMiU2VjdXJlIERpZ2l0YWwgQ2VydGlmaWNhdGUgU2lnbmluZzE4MDYGA1UEAxMvU3RhcnRDb20gQ2xhc3MgMSBQcmltYXJ5IEludGVybWVkaWF0ZSBDbGllbnQgQ0EwHhcNMTUwNDI0MTIzMjU4WhcNMTYwNDI0MDQzMDI4WjA4MRcwFQYDVQQDDA5hbmRlcnNAYWJlbC5udTEdMBsGCSqGSIb3DQEJARYOYW5kZXJzQGFiZWwubnUwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCeNuAxBAFeueFVtptqq6xY8GetxhZYI3ytgNG3IEHWzN4Pf9Ql58V0dUdYBSDZENOv9WArr4sLkpsB5U9n/C/6pn5auN1Db6GQyo7zTlrs+duuwpcWIodoGrrutqOdabIULrxDbQ+nbCelH64iBCh3YtzsPnGjHUo6JlpfVYau99d7Oh/+8dOKqT1TVs/w1mt1l3AEYSM3SfBx0L8k2xuzi42EcGIySJj2U3eFYqv/kO1sIg/X3Kvuo4CWz4hHGdeRNyZatBZyXtE7SZNdGjvP+D+iuqYH8gWPVTO8TQg9QY+/BpVBfa9JSfSwi4KcAe/a7m/qkyu+gBJ9M21cJXEZAgMBAAGjggLUMIIC0DAJBgNVHRMEAjAAMAsGA1UdDwQEAwIEsDAdBgNVHSUEFjAUBggrBgEFBQcDAgYIKwYBBQUHAwQwHQYDVR0OBBYEFDZrDRuDS0GGF8c3iC9drCI9p63oMB8GA1UdIwQYMBaAFFNy7ZKc4NrLAVx8fpY1TvLUuFGCMBkGA1UdEQQSMBCBDmFuZGVyc0BhYmVsLm51MIIBTAYDVR0gBIIBQzCCAT8wggE7BgsrBgEEAYG1NwECAzCCASowLgYIKwYBBQUHAgEWImh0dHA6Ly93d3cuc3RhcnRzc2wuY29tL3BvbGljeS5wZGYwgfcGCCsGAQUFBwICMIHqMCcWIFN0YXJ0Q29tIENlcnRpZmljYXRpb24gQXV0aG9yaXR5MAMCAQEagb5UaGlzIGNlcnRpZmljYXRlIHdhcyBpc3N1ZWQgYWNjb3JkaW5nIHRvIHRoZSBDbGFzcyAxIFZhbGlkYXRpb24gcmVxdWlyZW1lbnRzIG9mIHRoZSBTdGFydENvbSBDQSBwb2xpY3ksIHJlbGlhbmNlIG9ubHkgZm9yIHRoZSBpbnRlbmRlZCBwdXJwb3NlIGluIGNvbXBsaWFuY2Ugb2YgdGhlIHJlbHlpbmcgcGFydHkgb2JsaWdhdGlvbnMuMDYGA1UdHwQvMC0wK6ApoCeGJWh0dHA6Ly9jcmwuc3RhcnRzc2wuY29tL2NydHUxLWNybC5jcmwwgY4GCCsGAQUFBwEBBIGBMH8wOQYIKwYBBQUHMAGGLWh0dHA6Ly9vY3NwLnN0YXJ0c3NsLmNvbS9zdWIvY2xhc3MxL2NsaWVudC9jYTBCBggrBgEFBQcwAoY2aHR0cDovL2FpYS5zdGFydHNzbC5jb20vY2VydHMvc3ViLmNsYXNzMS5jbGllbnQuY2EuY3J0MCMGA1UdEgQcMBqGGGh0dHA6Ly93d3cuc3RhcnRzc2wuY29tLzANBgkqhkiG9w0BAQUFAAOCAQEAPqjFybD6u6vLrRWZNe61WVZK2GMuv3Sm03acHLZIy8+zfoQfM9NhPvsmDmPpAelujJMOHISIkUxBB2qL9y/fDPqMsOg2wXIAJ96in+rmvjmnUd75bdbHJtVKOyx2m2HAjy4kA5bBF96asE0NVVTr8VR4N/NAoO0jPlNDLe8L5M5n1R4rttfjZ8IGQ6++DkGsatD8jh2ZuZUoI8q4gEo+8WKvBmX6bbRhp69ZO7eZoV+KBW66y5DfnBI0LHT0gC93otSO4wClyj8KGmRaAFzIt0f+f7VXl3Zjttc0lUF8mJoqnGDXByg9Q50h0nGp+rxjdURYkWW+WHvnVWo3ejSYiQ==</X509Certificate>
</X509Data>
</KeyInfo>
</Signature>
I’ve made a small test program where I try it out.
Is it relevant to have a code coverage target? In a talk at NDC Oslo 2014 Uncle Bob said that the only reasonable goal is 100%. On the other hand Mark Seemann recently said on twitter and in a follow up blog post that “I thought it was common knowledge that nothing good comes from code coverage targets”. Those are two seemingly opposing views.
Before looking at the role of code coverage, I’d like to take a few steps back and look at the objectives of testing.
When working properly with test driven development, no production code is written unless there is a failing test driving the change. As soon as the test pass, no addition of production code is allowed until there is again a failing test. If that principle is followed, there will simply be no production code that is not covered by a test.
My objective is to achieve high code quality.
My way of achieving that is TDD.
An effect of that is that I do get full code coverage.
With git, it’s possible to do things that must be considered pure magic for anyone using older version control systems. Learn 7 simple tricks that will help you take the leap beyond commit, push and pull and let you leverage the powers of git. With these 7 tricks, you will be the git Wizard of the office.
This is a summary of my talk at Tech Days Sweden 2015. The explanations here are very brief, if you would like me to explain anything of this in more detail in a separate post, please leave a comment.