From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 16 Nov 2018 14:48:56 +0000 (+0100)
Subject: curve25519: Prevent Ed25519 signature malleability
X-Git-Tag: 5.7.2dr4~12^2~5
X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fstrongswan.git;a=commitdiff_plain;h=2571898d32609ffdd2fb8c5ce1659f3732efb026

curve25519: Prevent Ed25519 signature malleability

As per RFC 8032, section 5.1.7 (and section 8.4) we have to make sure s, which
is the scalar in the second half of the signature value, is smaller than L.
Without that check, L can be added to most signatures at least once to create
another valid signature for the same public key and message.

This could be problematic if, for instance, a blacklist is based on hashes
of certificates.  A new certificate could be created with a different
signature (without knowing the signature key) by simply adding L to s.

Currently, both OpenSSL 1.1.1 and Botan 2.8.0 are vulnerable to this, which is
why the unit test currently only warns about it.
---

diff --git a/src/libstrongswan/plugins/curve25519/curve25519_public_key.c b/src/libstrongswan/plugins/curve25519/curve25519_public_key.c
index 6eb80a1192..dfc1df4d0b 100644
--- a/src/libstrongswan/plugins/curve25519/curve25519_public_key.c
+++ b/src/libstrongswan/plugins/curve25519/curve25519_public_key.c
@@ -49,6 +49,13 @@ METHOD(public_key_t, get_type, key_type_t,
 	return KEY_ED25519;
 }
 
+/* L = 2^252+27742317777372353535851937790883648493 in little-endian form */
+static chunk_t curve25519_order = chunk_from_chars(
+								0xed, 0xd3, 0xf5, 0x5c, 0x1a, 0x63, 0x12, 0x58,
+								0xd6, 0x9c, 0xf7, 0xa2, 0xde, 0xf9, 0xde, 0x14,
+								0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+								0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10);
+
 METHOD(public_key_t, verify, bool,
 	private_curve25519_public_key_t *this, signature_scheme_t scheme,
 	void *params, chunk_t data, chunk_t signature)
@@ -94,6 +101,20 @@ METHOD(public_key_t, verify, bool,
 	{
 		return FALSE;
 	}
+	/* make sure 0 <= s < L, as per RFC 8032, section 5.1.7 to prevent signature
+	 * malleability.  Due to the three-bit check above (forces s < 2^253) there
+	 * is not that much room, but adding L once works with most signatures */
+	for (i = 31; ; i--)
+	{
+		if (sig[i+32] < curve25519_order.ptr[i])
+		{
+			break;
+		}
+		else if (sig[i+32] > curve25519_order.ptr[i] || i == 0)
+		{
+			return FALSE;
+		}
+	}
 
 	hasher = lib->crypto->create_hasher(lib->crypto, HASH_SHA512);
 	if (!hasher)
diff --git a/src/libstrongswan/tests/suites/test_ed25519.c b/src/libstrongswan/tests/suites/test_ed25519.c
index 35da95d21a..d69076d7dc 100644
--- a/src/libstrongswan/tests/suites/test_ed25519.c
+++ b/src/libstrongswan/tests/suites/test_ed25519.c
@@ -27,7 +27,7 @@ struct sig_test_t {
 };
 
 /**
- * Ed25519 Test Vectors from draft-irtf-cfrg-eddsa
+ * Ed25519 Test Vectors from RFC 8032
  */
 static sig_test_t sig_tests[] = {
 	/* Test 1 */
@@ -429,6 +429,16 @@ static chunk_t zero_pk = chunk_from_chars(
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
 	0x00, 0x00, 0x00, 0x00);
 
+/* sig_tests[0].sig with s+L */
+static chunk_t malleable_sig = chunk_from_chars(
+	0xe5, 0x56, 0x43, 0x00, 0xc3, 0x60, 0xac, 0x72, 0x90, 0x86,
+	0xe2, 0xcc, 0x80, 0x6e, 0x82, 0x8a, 0x84, 0x87, 0x7f, 0x1e,
+	0xb8, 0xe5, 0xd9, 0x74, 0xd8, 0x73, 0xe0, 0x65, 0x22, 0x49,
+	0x01, 0x55, 0x4c, 0x8c, 0x78, 0x72, 0xaa, 0x06, 0x4e, 0x04,
+	0x9d, 0xbb, 0x30, 0x13, 0xfb, 0xf2, 0x93, 0x80, 0xd2, 0x5b,
+	0xf5, 0xf0, 0x59, 0x5b, 0xbe, 0x24, 0x65, 0x51, 0x41, 0x43,
+	0x8e, 0x7a, 0x10, 0x1b);
+
 START_TEST(test_ed25519_fail)
 {
 	private_key_t *key;
@@ -479,6 +489,16 @@ START_TEST(test_ed25519_fail)
 	ck_assert(!pubkey->verify(pubkey, SIGN_ED25519, NULL, chunk_empty,
 							  chunk_empty));
 
+	/* RFC 8032, section 5.1.7 requires that 0 <= s < L to prevent signature
+	 * malleability.  Only a warning because Botan and OpenSSL are both
+	 * vulnerable to this. */
+	if (pubkey->verify(pubkey, SIGN_ED25519, NULL, sig_tests[0].msg,
+					   malleable_sig))
+	{
+		warn("Ed25519 signature verification is vulnerable to malleable "
+			 "signatures");
+	}
+
 	/* malformed signature */
 	sig = chunk_create(sig1, 64);
 	memcpy(sig1, sig_tests[0].sig.ptr, 64);