Security fix for CVE-2009-0217

svn path=/trunk/mcs/; revision=137886

mcs/class/System.Security/System.Security.Cryptography.Xml/ChangeLog

@@ -1,3 +1,8 @@
+2009-07-14  Sebastien Pouliot  <[email protected]>
+
+	* SignedXml.cs: Fix HMACOutputLength to match XMLDSIG erratum (ref: 
+	CVE-2009-0217) and add stricter checks.
+
 2009-06-05  Marek Safar  <[email protected]>
 
 	* *.cs: Fixed NET_2_0 conditional to actually handle Mono.Security

mcs/class/System.Security/System.Security.Cryptography.Xml/SignedXml.cs

@@ -595,17 +595,28 @@ namespace System.Security.Cryptography.Xml {
 				return false;
 
 			byte[] actual = macAlg.ComputeHash (s);
-			// HMAC signature may be partial
+			// HMAC signature may be partial and specified by <HMACOutputLength>
 			if (m_signature.SignedInfo.SignatureLength != null) {
-				int length = actual.Length;
-				try {
-					// SignatureLength is in bits
-					length = (Int32.Parse (m_signature.SignedInfo.SignatureLength) >> 3);
-				}
-				catch {
-				}
-
-				if (length != actual.Length) {
+				int length = Int32.Parse (m_signature.SignedInfo.SignatureLength);
+				// we only support signatures with a multiple of 8 bits
+				// and the value must match the signature length
+				if ((length & 7) != 0)
+					throw new CryptographicException ("Signature length must be a multiple of 8 bits.");
+
+				// SignatureLength is in bits (and we works on bytes, only in multiple of 8 bits)
+				// and both values must match for a signature to be valid
+				length >>= 3;
+				if (length != m_signature.SignatureValue.Length)
+					throw new CryptographicException ("Invalid signature length.");
+
+				// is the length "big" enough to make the signature meaningful ? 
+				// we use a minimum of 80 bits (10 bytes) or half the HMAC normal output length
+				// e.g. HMACMD5 output 128 bits but our minimum is 80 bits (not 64 bits)
+				int minimum = Math.Max (10, actual.Length / 2);
+				if (length < minimum)
+					throw new CryptographicException ("HMAC signature is too small");
+
+				if (length < actual.Length) {
 					byte[] trunked = new byte [length];
 					Buffer.BlockCopy (actual, 0, trunked, 0, length);
 					actual = trunked;

mcs/class/System.Security/Test/System.Security.Cryptography.Xml/ChangeLog

@@ -1,3 +1,8 @@
+2009-07-14  Sebastien Pouliot  <[email protected]>
+
+	* SignedInfoTest.cs: Test case for Signature Length/Method mixup
+	* SignedXmlTest.cs: Test cases for HMACOutputLength
+
 2009-06-26  Robert Jordan  <[email protected]>
 
 	* *.cs: Upgrade to new NUnit style.

mcs/class/System.Security/Test/System.Security.Cryptography.Xml/SignedInfoTest.cs

@@ -5,7 +5,7 @@
 //	Sebastien Pouliot <[email protected]>
 //
 // (C) 2002, 2003 Motus Technologies Inc. (http://www.motus.com)
-// Copyright (C) 2005 Novell, Inc (http://www.novell.com)
+// Copyright (C) 2005, 2009 Novell, Inc (http://www.novell.com)
 //
 
 using System;
@@ -190,5 +190,27 @@ namespace MonoTests.System.Security.Cryptography.Xml {
 			sig.CanonicalizationMethod = "urn:foo";
 			XmlElement el = sig.GetXml ();
 		}
+
+		[Test]
+		public void SignatureLength ()
+		{
+			// we can set the length before the algorithm
+			SignedInfo si = new SignedInfo ();
+			si.SignatureLength = "128";
+			Assert.AreEqual ("128", si.SignatureLength, "SignatureLength-1");
+			Assert.IsNull (si.SignatureMethod, "SignatureMethod-1");
+
+			// zero
+			si.SignatureMethod = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
+			si.SignatureLength = "0";
+			Assert.AreEqual ("0", si.SignatureLength, "SignatureLength-2");
+			Assert.AreEqual ("http://www.w3.org/2000/09/xmldsig#rsa-sha1", si.SignatureMethod, "SignatureMethod-2");
+
+			// mixup length and method
+			si.SignatureLength = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
+			si.SignatureMethod = "0";
+			Assert.AreEqual ("http://www.w3.org/2000/09/xmldsig#rsa-sha1", si.SignatureLength, "SignatureLength-3");
+			Assert.AreEqual ("0", si.SignatureMethod, "SignatureMethod-3");
+		}
 	}
 }

mcs/class/System.Security/Test/System.Security.Cryptography.Xml/SignedXmlTest.cs

@@ -1294,5 +1294,209 @@ namespace MonoTests.System.Security.Cryptography.Xml {
 			Assert.IsTrue (sign.CheckSignature (new HMACRIPEMD160 (hmackey)));
 		}
 #endif
+		// CVE-2009-0217
+		// * a 0-length signature is the worse case - it accepts anything
+		// * between 1-7 bits length are considered invalid (not a multiple of 8)
+		// * a 8 bits signature would have one chance, out of 256, to be valid
+		// * and so on... until we hit (output-length / 2) or 80 bits (see ERRATUM)
+
+		static bool erratum = true; // xmldsig erratum for CVE-2009-0217
+
+		static SignedXml GetSignedXml (string xml)
+		{
+			XmlDocument doc = new XmlDocument ();
+			doc.LoadXml (xml);
+
+			SignedXml sign = new SignedXml (doc);
+			sign.LoadXml (doc.DocumentElement);
+			return sign;
+		}
+
+		static void CheckErratum (SignedXml signed, KeyedHashAlgorithm hmac, string message)
+		{
+			if (erratum) {
+				try {
+					signed.CheckSignature (hmac);
+					Assert.Fail (message + ": unexcepted success");
+				}
+				catch (CryptographicException) {
+				}
+				catch (Exception e) {
+					Assert.Fail (message + ": unexcepted " + e.ToString ());
+				}
+			} else {
+				Assert.IsTrue (signed.CheckSignature (hmac), message);
+			}
+		}
+
+		private void HmacMustBeMultipleOfEightBits (int bits)
+		{
+			string xml = @"<?xml version=""1.0"" encoding=""UTF-8""?>
+<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#hmac-sha1"" >
+      <HMACOutputLength>{0}</HMACOutputLength>
+    </SignatureMethod>
+    <Reference URI=""#object"">
+      <DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" />
+      <DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue>
+    </Reference>
+  </SignedInfo>
+  <SignatureValue>
+    gA==
+  </SignatureValue>
+  <Object Id=""object"">some other text</Object>
+</Signature>
+";
+			SignedXml sign = GetSignedXml (String.Format (xml, bits));
+			// only multiple of 8 bits are supported
+			sign.CheckSignature (new HMACSHA1 (Encoding.ASCII.GetBytes ("secret")));
+		}
+
+		[Test]
+		public void HmacMustBeMultipleOfEightBits ()
+		{
+			for (int i = 1; i < 160; i++) {
+				// The .NET framework only supports multiple of 8 bits
+				if (i % 8 == 0)
+					continue;
+
+				try {
+					HmacMustBeMultipleOfEightBits (i);
+					Assert.Fail ("Unexpected Success " + i.ToString ());
+				}
+				catch (CryptographicException) {
+				}
+				catch (Exception e) {
+					Assert.Fail ("Unexpected Exception " + i.ToString () + " : " + e.ToString ());
+				}
+			}
+		}
+
+		[Test]
+		[Category ("NotDotNet")] // will fail until a fix is available
+		public void VerifyHMAC_ZeroLength ()
+		{
+			string xml = @"<?xml version=""1.0"" encoding=""UTF-8""?>
+<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#hmac-sha1"" >
+      <HMACOutputLength>0</HMACOutputLength>
+    </SignatureMethod>
+    <Reference URI=""#object"">
+      <DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" />
+      <DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue>
+    </Reference>
+  </SignedInfo>
+  <SignatureValue>
+  </SignatureValue>
+  <Object Id=""object"">some other text</Object>
+</Signature>
+";
+			SignedXml sign = GetSignedXml (xml);
+
+			CheckErratum (sign, new HMACSHA1 (Encoding.ASCII.GetBytes ("no clue")), "1");
+			CheckErratum (sign, new HMACSHA1 (Encoding.ASCII.GetBytes ("")), "2");
+			CheckErratum (sign, new HMACSHA1 (Encoding.ASCII.GetBytes ("oops")), "3");
+			CheckErratum (sign, new HMACSHA1 (Encoding.ASCII.GetBytes ("secret")), "4");
+		}
+
+		[Test]
+		[Category ("NotDotNet")] // will fail until a fix is available
+		public void VerifyHMAC_SmallerThanMinimumLength ()
+		{
+			// 72 is a multiple of 8 but smaller than the minimum of 80 bits
+			string xml = @"<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#hmac-sha1""><HMACOutputLength>72</HMACOutputLength></SignatureMethod><Reference URI=""#object""><DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" /><DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue></Reference></SignedInfo><SignatureValue>2dimB+P5Aw5K</SignatureValue><Object Id=""object"">some other text</Object></Signature>";
+			SignedXml sign = GetSignedXml (xml);
+			CheckErratum (sign, new HMACSHA1 (Encoding.ASCII.GetBytes ("secret")), "72");
+		}
+
+		[Test]
+		public void VerifyHMAC_MinimumLength ()
+		{
+			// 80 bits is the minimum (and the half-size of HMACSHA1)
+			string xml = @"<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#hmac-sha1""><HMACOutputLength>80</HMACOutputLength></SignatureMethod><Reference URI=""#object""><DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" /><DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue></Reference></SignedInfo><SignatureValue>jVQPtLj61zNYjw==</SignatureValue><Object Id=""object"">some other text</Object></Signature>";
+			SignedXml sign = GetSignedXml (xml);
+			Assert.IsTrue (sign.CheckSignature (new HMACSHA1 (Encoding.ASCII.GetBytes ("secret"))));
+		}
+#if NET_2_0
+		[Test]
+		[Category ("NotDotNet")] // will fail until a fix is available
+		public void VerifyHMAC_SmallerHalfLength ()
+		{
+			// 80bits is smaller than the half-size of HMACSHA256
+			string xml = @"<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/2001/04/xmldsig-more#hmac-sha256""><HMACOutputLength>80</HMACOutputLength></SignatureMethod><Reference URI=""#object""><DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" /><DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue></Reference></SignedInfo><SignatureValue>vPtw7zKVV/JwQg==</SignatureValue><Object Id=""object"">some other text</Object></Signature>";
+			SignedXml sign = GetSignedXml (xml);
+			CheckErratum (sign, new HMACSHA256 (Encoding.ASCII.GetBytes ("secret")), "80");
+		}
+
+		[Test]
+		public void VerifyHMAC_HalfLength ()
+		{
+			// 128 is the half-size of HMACSHA256
+			string xml = @"<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/2001/04/xmldsig-more#hmac-sha256""><HMACOutputLength>128</HMACOutputLength></SignatureMethod><Reference URI=""#object""><DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" /><DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue></Reference></SignedInfo><SignatureValue>aegpvkAwOL8gN/CjSnW6qw==</SignatureValue><Object Id=""object"">some other text</Object></Signature>";
+			SignedXml sign = GetSignedXml (xml);
+			Assert.IsTrue (sign.CheckSignature (new HMACSHA256 (Encoding.ASCII.GetBytes ("secret"))));
+		}
+#endif
+		[Test]
+		public void VerifyHMAC_FullLength ()
+		{
+			string xml = @"<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#hmac-sha1"" /><Reference URI=""#object""><DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" /><DigestValue>7/XTsHaBSOnJ/jXD5v0zL6VKYsk=</DigestValue></Reference></SignedInfo><SignatureValue>a0goL9esBUKPqtFYgpp2KST4huk=</SignatureValue><Object Id=""object"">some text</Object></Signature>";
+			SignedXml sign = GetSignedXml (xml);
+			Assert.IsTrue (sign.CheckSignature (new HMACSHA1 (Encoding.ASCII.GetBytes ("secret"))));
+		}
+
+		[Test]
+		[ExpectedException (typeof (CryptographicException))]
+		public void VerifyHMAC_HMACOutputLength_Signature_Mismatch ()
+		{
+			string xml = @"<?xml version=""1.0"" encoding=""UTF-8""?>
+<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#hmac-sha1"" >
+      <HMACOutputLength>80</HMACOutputLength>
+    </SignatureMethod>
+    <Reference URI=""#object"">
+      <DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" />
+      <DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue>
+    </Reference>
+  </SignedInfo>
+  <SignatureValue>
+  </SignatureValue>
+  <Object Id=""object"">some other text</Object>
+</Signature>
+";
+			SignedXml sign = GetSignedXml (xml);
+			sign.CheckSignature (new HMACSHA1 (Encoding.ASCII.GetBytes ("no clue")));
+		}
+
+		[Test]
+		[ExpectedException (typeof (FormatException))]
+		public void VerifyHMAC_HMACOutputLength_Invalid ()
+		{
+			string xml = @"<?xml version=""1.0"" encoding=""UTF-8""?>
+<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#hmac-sha1"" >
+      <HMACOutputLength>I wish this was not a string property</HMACOutputLength>
+    </SignatureMethod>
+    <Reference URI=""#object"">
+      <DigestMethod Algorithm=""http://www.w3.org/2000/09/xmldsig#sha1"" />
+      <DigestValue>nz4GS0NbH2SrWlD/4fX313CoTzc=</DigestValue>
+    </Reference>
+  </SignedInfo>
+  <SignatureValue>
+  </SignatureValue>
+  <Object Id=""object"">some other text</Object>
+</Signature>
+";
+			SignedXml sign = GetSignedXml (xml);
+			sign.CheckSignature (new HMACSHA1 (Encoding.ASCII.GetBytes ("no clue")));
+		}
 	}
 }