Merge pull request opendnssec#231 from bellgrim/issue205

Issue opendnssec#205: ECDSA P-521 support for OpenSSL and better test coverage.
bellgrim · Sep 5, 2016 · 4dea95c · 4dea95c
2 parents 0380767 + 7d842c5
commit 4dea95c
Show file tree

Hide file tree

Showing 5 changed files with 313 additions and 95 deletions.
diff --git a/NEWS b/NEWS
@@ -19,10 +19,13 @@ SoftHSM develop
 * Issue #209: Possibility to test other PKCS#11 implementations with the
   CppUnit test.
   (Patch from Lars Silvén)
+* Issue #223: Mark public key as non private by default.
+  (Patch from Nikos Mavrogiannopoulos)
 
 Bugfixes:
 * Issue #201: Missing new source file and test configuration in the
   Windows build project.
+* Issue #205: ECDSA P-521 support for OpenSSL and better test coverage.
 * Issue #207: Fix segmentation faults in loadLibrary function.
   (Patch from Jaroslav Imrich)
 * Issue #218: Fix build warnings.

diff --git a/src/lib/crypto/OSSLUtil.cpp b/src/lib/crypto/OSSLUtil.cpp
@@ -88,16 +88,29 @@ ByteString OSSL::pt2ByteString(const EC_POINT* pt, const EC_GROUP* grp)
 	if (pt != NULL && grp != NULL)
 	{
 		size_t len = EC_POINT_point2oct(grp, pt, POINT_CONVERSION_UNCOMPRESSED, NULL, 0, NULL);
-		if (len > 0x7f)
+		// Definite, short
+		if (len <= 0x7f)
 		{
-			ERROR_MSG("Oversized EC point");
-
-			return rv;
+			rv.resize(2 + len);
+			rv[0] = V_ASN1_OCTET_STRING;
+			rv[1] = len & 0x7f;
+			EC_POINT_point2oct(grp, pt, POINT_CONVERSION_UNCOMPRESSED, &rv[2], len, NULL);
+		}
+		// Definite, long
+		else
+		{
+			// Get the number of length octets
+			ByteString length(len);
+			unsigned int counter = 0;
+			while (length[counter] == 0 && counter < (length.size()-1)) counter++;
+			ByteString lengthOctets(&length[counter], length.size() - counter);
+
+			rv.resize(len + 2 + lengthOctets.size());
+			rv[0] = V_ASN1_OCTET_STRING;
+			rv[1] = 0x80 | lengthOctets.size();
+			memcpy(&rv[2], &lengthOctets[0], lengthOctets.size());
+			EC_POINT_point2oct(grp, pt, POINT_CONVERSION_UNCOMPRESSED, &rv[2 + lengthOctets.size()], len, NULL);
 		}
-		rv.resize(len + 2);
-		rv[0] = V_ASN1_OCTET_STRING;
-		rv[1] = len & 0x7f;
-		EC_POINT_point2oct(grp, pt, POINT_CONVERSION_UNCOMPRESSED, &rv[2], len, NULL);
 	}
 
 	return rv;
@@ -107,42 +120,72 @@ ByteString OSSL::pt2ByteString(const EC_POINT* pt, const EC_GROUP* grp)
 EC_POINT* OSSL::byteString2pt(const ByteString& byteString, const EC_GROUP* grp)
 {
 	size_t len = byteString.size();
-	if (len < 2)
+	size_t controlOctets = 2;
+	if (len < controlOctets)
 	{
 		ERROR_MSG("Undersized EC point");
 
 		return NULL;
 	}
-	len -= 2;
-	if (len > 0x7f)
-	{
-		ERROR_MSG("Oversized EC point");
 
-		return NULL;
-	}
 	ByteString repr = byteString;
+
 	if (repr[0] != V_ASN1_OCTET_STRING)
 	{
 		ERROR_MSG("EC point tag is not OCTET STRING");
 
 		return NULL;
 	}
-	if (repr[1] != len)
+
+	// Definite, short
+	if (repr[1] < 0x80)
 	{
-		if (repr[1] < len)
+		if (repr[1] != (len - controlOctets))
 		{
-			ERROR_MSG("Underrun EC point");
+			if (repr[1] < (len - controlOctets))
+			{
+				ERROR_MSG("Underrun EC point");
+			}
+			else
+			{
+				ERROR_MSG("Overrun EC point");
+			}
+
+			return NULL;
 		}
-		else
+	}
+	// Definite, long
+	else
+	{
+		size_t lengthOctets = repr[1] & 0x7f;
+		controlOctets += lengthOctets;
+
+		if (controlOctets >= repr.size())
 		{
-			ERROR_MSG("Overrun EC point");
+			ERROR_MSG("Undersized EC point");
+
+			return NULL;
 		}
 
-		return NULL;
+		ByteString length(&repr[2], lengthOctets);
+
+		if (length.long_val() != (len - controlOctets))
+		{
+			if (length.long_val() < (len - controlOctets))
+			{
+				ERROR_MSG("Underrun EC point");
+			}
+			else
+			{
+				ERROR_MSG("Overrun EC point");
+			}
+
+			return NULL;
+		}
 	}
 
 	EC_POINT* pt = EC_POINT_new(grp);
-	if (!EC_POINT_oct2point(grp, pt, &repr[2], len, NULL))
+	if (!EC_POINT_oct2point(grp, pt, &repr[controlOctets], len - controlOctets, NULL))
 	{
 		EC_POINT_free(pt);
 		return NULL;

diff --git a/src/lib/crypto/test/ECDSATests.cpp b/src/lib/crypto/test/ECDSATests.cpp
@@ -76,6 +76,8 @@ void ECDSATests::testKeyGeneration()
 	curves.push_back(ByteString("06082a8648ce3d030107"));
 	// Add secp384r1
 	curves.push_back(ByteString("06052b81040022"));
+	// Add secp521r1
+	curves.push_back(ByteString("06052b81040023"));
 
 	for (std::vector<ByteString>::iterator c = curves.begin(); c != curves.end(); c++)
 	{
@@ -192,7 +194,8 @@ void ECDSATests::testSigningVerifying()
 	totest.push_back(std::make_pair(ByteString("06082a8648ce3d030107"), HashAlgo::SHA256));
 	// Add secp384r1
 	totest.push_back(std::make_pair(ByteString("06052b81040022"), HashAlgo::SHA384));
-
+	// Add secp521r1
+	totest.push_back(std::make_pair(ByteString("06052b81040023"), HashAlgo::SHA384));
 
 	for (std::vector<std::pair<ByteString, HashAlgo::Type> >::iterator k = totest.begin(); k != totest.end(); k++)
 	{