Merge pull request #765 from libtom/pr/fix-issue764

Fix issue #764 - small-subgroup attack

Author: sjaeckel

src/pk/ecc/ecc_shared_secret.c

@@ -23,8 +23,8 @@ int ecc_shared_secret(const ecc_key *private_key, const ecc_key *public_key,
 {
    unsigned long  x;
    ecc_point     *result;
-   void          *prime, *a;
-   int            err;
+   void          *prime, *a, *mp = NULL;
+   int            err, inf;
 
    LTC_ARGCHK(private_key != NULL);
    LTC_ARGCHK(public_key  != NULL);
@@ -59,7 +59,15 @@ int ecc_shared_secret(const ecc_key *private_key, const ecc_key *public_key,
    prime = private_key->dp.prime;
    a     = private_key->dp.A;
 
-   if ((err = ltc_mp.ecc_ptmul(private_key->k, &public_key->pubkey, result, a, prime, 1)) != CRYPT_OK)   { goto done; }
+   if ((err = ltc_mp.ecc_ptmul(private_key->k, &public_key->pubkey, result, a, prime, 0)) != CRYPT_OK)   { goto done; }
+
+   /* reject small-subgroup / invalid-curve attacks: k*pubkey must not be O */
+   if ((err = ltc_ecc_is_point_at_infinity(result, prime, &inf)) != CRYPT_OK)                            { goto done; }
+   if (inf) { err = CRYPT_ERROR; goto done; }
+
+   /* map=0 above kept z meaningful for the infinity check; now finish the affine map */
+   if ((err = ltc_mp_montgomery_setup(prime, &mp)) != CRYPT_OK)                                          { goto done; }
+   if ((err = ltc_mp.ecc_map(result, prime, mp)) != CRYPT_OK)                                            { goto done; }
 
    x = (unsigned long)ltc_mp_unsigned_bin_size(prime);
    if (*outlen < x) {
@@ -73,6 +81,7 @@ int ecc_shared_secret(const ecc_key *private_key, const ecc_key *public_key,
    err     = CRYPT_OK;
    *outlen = x;
 done:
+   if (mp != NULL) ltc_mp_montgomery_free(mp);
    ltc_ecc_del_point(result);
    return err;
 }

src/pk/ecc/ltc_ecc_verify_key.c

@@ -40,10 +40,10 @@ int ltc_ecc_verify_key(const ecc_key *key)
 
    /* Test 3: does nG = O? (n = order, O = point at infinity, G = public key) */
    point = ltc_ecc_new_point();
-   if ((err = ltc_ecc_mulmod(order, &(key->pubkey), point, a, prime, 1)) != CRYPT_OK)     { goto done1; }
+   if ((err = ltc_ecc_mulmod(order, &(key->pubkey), point, a, prime, 0)) != CRYPT_OK)     { goto done1; }
 
    err = ltc_ecc_is_point_at_infinity(point, prime, &inf);
-   if (err != CRYPT_OK || inf) {
+   if (err != CRYPT_OK || !inf) {
       err = CRYPT_ERROR;
    }
    else {

tests/ecc_test.c

@@ -2549,6 +2549,59 @@ static int s_ecc_test_recovery(void)
 }
 #endif
 
+#ifdef LTC_ECC_SECP112R2
+/* https://github.com/libtom/libtomcrypt/issues/764 - small-subgroup attack regression test
+   SECP112R2 has cofactor 4, so the curve contains a subgroup of order 4 outside the prime-order generator subgroup
+   A point in that small subgroup is on the curve so Test 2 in ltc_ecc_verify_key passes. However n * P != O, so Test 3 must reject it
+   The point below has order 4: n * P gives -P, not the point at infinity. Therefore, importing it as a public key must fail
+*/
+static int s_ecc_issue764(void)
+{
+   const ltc_ecc_curve *cu;
+   ecc_key key, priv;
+   int err;
+   unsigned char shared[14];
+   unsigned long sharedlen;
+   const unsigned char pub[] = {
+      0x04, /* uncompressed */
+      0xB1,0xFD,0x8D,0xE1,0x27,0xD4,0x65,0x6B,0x57,0x3E,0xB5,0x13,0x98,0x4C, /* x = B1FD8DE127D4656B573EB513984C */
+      0x2F,0x8C,0xD8,0x80,0x3D,0xB9,0x62,0x0F,0xA3,0xA6,0x0E,0x5B,0x31,0xE2  /* y = 2F8CD8803DB9620FA3A60E5B31E2 */
+   };
+   DO(ecc_find_curve("SECP112R2", &cu));
+
+   /* ecc_set_key must reject the malicious public key (verify_key Test 3) */
+   DO(ecc_set_curve(cu, &key));
+   err = ecc_set_key(pub, sizeof(pub), PK_PUBLIC, &key); /* must fail */
+   if (err == CRYPT_OK) {
+      ecc_free(&key);
+      return CRYPT_FAIL_TESTVECTOR;
+   }
+
+   /* ecc_shared_secret must reject the malicious peer key even when the caller bypassed ecc_set_key by directly loading the pubkey
+      priv.k = 4 (a multiple of the malicious point's order 4) so that k * pubkey is deterministically the point at infinity
+   */
+   DO(ecc_set_curve(cu, &priv));
+   DO(ltc_mp_set(priv.k, 4));
+   priv.type = PK_PRIVATE;
+
+   /* load the malicious point directly to simulate an attacker-supplied pubkey reaching ecc_shared_secret without import checks */
+   DO(ecc_set_curve(cu, &key));
+   DO(ltc_mp_read_unsigned_bin(key.pubkey.x, (unsigned char *)pub + 1,  14));
+   DO(ltc_mp_read_unsigned_bin(key.pubkey.y, (unsigned char *)pub + 15, 14));
+   DO(ltc_mp_set(key.pubkey.z, 1));
+   key.type = PK_PUBLIC;
+
+   sharedlen = sizeof(shared);
+   err = ecc_shared_secret(&priv, &key, shared, &sharedlen); /* must fail */
+   ecc_free(&priv);
+   ecc_free(&key);
+   if (err == CRYPT_OK) {
+      return CRYPT_FAIL_TESTVECTOR;
+   }
+   return CRYPT_OK;
+}
+#endif
+
 int ecc_test(void)
 {
    if (ltc_mp.name == NULL) return CRYPT_NOP;
@@ -2574,6 +2627,9 @@ int ecc_test(void)
    DO(s_ecc_issue443_447());
    DO(s_ecc_issue630());
    DO(s_ecc_issue116());
+#ifdef LTC_ECC_SECP112R2
+   DO(s_ecc_issue764());
+#endif
 #ifdef LTC_ECC_SHAMIR
    DO(s_ecc_test_shamir());
    DO(s_ecc_test_recovery());