Reject X25519 points with low order

lib/Crypto/PublicKey/ECC.py

@@ -1429,12 +1429,16 @@ def generate(**kwargs):
     if kwargs:
         raise TypeError("Unknown parameters: " + str(kwargs))
 
-    if _curves[curve_name].name in ("ed25519", "curve25519"):
+    if _curves[curve_name].name == "ed25519":
         seed = randfunc(32)
         new_key = EccKey(curve=curve_name, seed=seed)
     elif _curves[curve_name].name == "ed448":
         seed = randfunc(57)
         new_key = EccKey(curve=curve_name, seed=seed)
+    elif _curves[curve_name].name == "curve25519":
+        seed = randfunc(32)
+        new_key = EccKey(curve=curve_name, seed=seed)
+        _validate_x25519_public_key(new_key)
     else:
         d = Integer.random_range(min_inclusive=1,
                                  max_exclusive=curve.order,
@@ -1490,6 +1494,8 @@ def construct(**kwargs):
             kwargs["point"] = EccXPoint(point_x, curve_name)
         new_key = EccKey(**kwargs)
 
+        _validate_x25519_public_key(new_key)
+
     else:
 
         if None not in (point_x, point_y):
@@ -1916,12 +1922,40 @@ def _import_curve25519_public_key(encoded):
         raise ValueError("Incorrect length. Only Curve25519 public keys are supported.")
 
     x = bytearray(encoded)
+    # RFC 7741, Section 5
     x[31] &= 0x7F
     point_x = Integer.from_bytes(x, byteorder='little')
 
     return point_x
 
 
+def _validate_x25519_public_key(new_key):
+
+    p = _curves['curve25519'].p
+    p2 = p * 2
+    x1 = 325606250916557431795983626356110631294008115727848805560023387167927233504
+    x2 = 39382357235489614581723060781553021112529911719440698176882885853963445705823
+
+    # http://cr.yp.to/ecdh.html#validate
+    deny_list = (
+        0,
+        1,
+        x1,
+        x2,
+        p - 1,
+        p,
+        p + 1,
+        p + x1,
+        p + x2,
+        p2 - 1,
+        p2,
+        p2 + 1,
+    )
+
+    if new_key.pointQ.x in deny_list:
+        raise ValueError("Invalid Curve25519 public key")
+
+
 def _import_ed448_public_key(encoded):
     """Import an Ed448 ECC public key, encoded as raw bytes as described
     in RFC8032_.

lib/Crypto/SelfTest/Protocol/test_ecdh.py

@@ -344,6 +344,31 @@ def test_rfc7748_3(self):
         self.assertEqual(result1, secret)
         self.assertEqual(result2, secret)
 
+    def test_weak(self):
+
+        weak_keys = (
+            "0000000000000000000000000000000000000000000000000000000000000000",
+            "0100000000000000000000000000000000000000000000000000000000000000",
+            "e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800",
+            "5f9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f1157",
+            "ecffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f",
+            "edffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f",
+            "eeffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff7f",
+            # The implementation will accept these value, but only because
+            # it will set the MSB to zero (as required by RFC7748, Section 5),
+            # therefore leading to another public key (and to a point which is
+            # not of low order anymore).
+            #"cdeb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b880",
+            #"4c9c95bca3508c24b1d0b1559c83ef5b04445cc4581c8e86d8224eddd09f11d7",
+            #"d9ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+            #"daffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+            #"dbffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
+        )
+
+        for x in weak_keys:
+            self.assertRaises(ValueError,
+                              DH.import_x25519_public_key,
+                              unhexlify(x))
 
 class TestVectorsXECDHWycheproof(unittest.TestCase):
 
@@ -402,7 +427,14 @@ def shortDescription(self):
     def test_verify(self, tv):
 
         if tv.encoding == "XdhComp":
-            public_key = import_x25519_public_key(tv.public)
+            try:
+                public_key = import_x25519_public_key(tv.public)
+            except ValueError as e:
+                assert tv.valid
+                assert tv.warning
+                assert "LowOrderPublic" in tv.flags
+                assert "Invalid Curve25519" in str(e)
+                return
             private_key = import_x25519_private_key(tv.private)
         elif tv.encoding in ("XdhAsnComp", "XdhPemComp"):
             try:
@@ -412,6 +444,11 @@ def test_verify(self, tv):
                 assert not tv.valid
                 assert "Unsupported ECC" in str(e)
                 return
+            except ValueError as e:
+                assert tv.valid
+                assert tv.warning
+                assert "LowOrderPublic" in tv.flags
+                return
         elif tv.encoding == "XdhJwkComp":
 
             if 'y' in tv.public:
@@ -442,9 +479,16 @@ def base64url_decode(input_str):
                 public_key = import_x25519_public_key(jwk_public)
                 private_key = import_x25519_private_key(jwk_private)
             except ValueError as e:
-                assert not tv.valid
-                assert "Incorrect length" in str(e)
-                return
+                if tv.valid:
+                    assert tv.warning
+                    assert "LowOrderPublic" in tv.flags
+                    assert "Invalid Curve25519" in str(e)
+                    return
+                else:
+                    assert "Incorrect length" in str(e)
+                    return
+            except ValueError as e:
+                assert tv.valid
         else:
             raise ValueError("Unknown encoding", tv.encoding)
 

lib/Crypto/SelfTest/PublicKey/test_ECC_Curve25519.py

@@ -234,6 +234,33 @@ def test_negative_construct(self):
         self.assertRaises(ValueError, ECC.construct, curve="Curve25519", d=2, **coordG)
         self.assertRaises(ValueError, ECC.construct, curve="Curve25519", seed=b'H'*31)
 
+        # Verify you cannot construct weak keys (small-order points)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=0)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=1)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=325606250916557431795983626356110631294008115727848805560023387167927233504)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=39382357235489614581723060781553021112529911719440698176882885853963445705823)
+        p = 2**255 - 19
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p-1)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p+1)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p+325606250916557431795983626356110631294008115727848805560023387167927233504)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p+39382357235489614581723060781553021112529911719440698176882885853963445705823)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p*2-1)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p*2)
+        self.assertRaises(ValueError, ECC.construct, curve="Curve25519",
+                          point_x=p*2+1)
+
 
 def get_tests(config={}):
     tests = []

lib/Crypto/SelfTest/PublicKey/test_import_Curve25519.py

@@ -339,12 +339,41 @@ def test_error_params1(self):
                           passphrase="secret")
 
 
+class TestImport_Curve25519_Weak(unittest.TestCase):
+
+    def test_weak_pem(self):
+
+        p = 2**255 - 19
+        weak_x = (0,
+                  1,
+                  325606250916557431795983626356110631294008115727848805560023387167927233504,
+                  39382357235489614581723060781553021112529911719440698176882885853963445705823,
+                  p - 1,
+                  p,
+                  p + 1,
+                  p + 325606250916557431795983626356110631294008115727848805560023387167927233504,
+                  p + 39382357235489614581723060781553021112529911719440698176882885853963445705823,
+                  p * 2 - 1,
+                  p * 2,
+                  p * 2 + 1)
+
+        for x in weak_x:
+            low_order_point = ECC.EccXPoint(x, "curve25519")
+            weak_key = ECC.EccKey(point=low_order_point, curve="curve25519")
+            encoded = weak_key.export_key(format="PEM")
+
+            self.assertRaises(ValueError,
+                              ECC.import_key,
+                              encoded)
+
+
 def get_tests(config={}):
     tests = []
     try:
         tests += list_test_cases(TestImport)
         tests += list_test_cases(TestImport_Curve25519)
         tests += list_test_cases(TestExport_Curve25519)
+        tests += list_test_cases(TestImport_Curve25519_Weak)
     except SkipTest:
         pass
     return tests

lib/Crypto/SelfTest/loader.py

@@ -240,6 +240,7 @@ class TestVector(object):
 
             tv.valid = test['result'] != "invalid"
             tv.warning = test['result'] == "acceptable"
+            tv.flags = test.get('flags')
 
             tv.filename = file_name