more change on Curve25519 ECDH (was: [git] GCRYPT - branch, master, updated. libgcrypt-1.6.0-376-gee7e1a0)

[NIIBE Yutaka]

On 04/12/2016 10:12 AM, by NIIBE Yutaka wrote:
>     ecc: Fix X25519 computation on Curve25519.
>     
>     * cipher/ecc.c (ecc_encrypt_raw): Tweak of bits when
>     PUBKEY_FLAG_DJB_TWEAK is enabled.
>     (ecc_decrypt_raw): Return 0 when PUBKEY_FLAG_DJB_TWEAK is enabled.
>     * tests/t-cv25519.c (test_cv): Update by using gcry_pk_encrypt.
>     
>     --
>     
>     X25519 function is not a plain scalar multiplication, but does
>     two things; the scalar bits are tweaked before applying scalar
>     multiplication and X0 function is applied to the result of
>     scalar multiplication.
>     
>     In libgcrypt, _gcry_mpi_ec_mul_point is a plain scalar multiplication
>     and those two things are done in functions for ECDH with X25519.

I thought this was the final change of mine, but trying to fix GnuPG,
I think that another patch is needed for libgcrypt.

Here is the change.

(1) For ecc_encrypt_raw, it won't abort but properly use X0 function
    to map infinity to 0

(2) In ecc_decrypt_raw, avoiding validation (to be X25519 compatible) is
    based on the flag not the name of the curve.

(3) In ecc_decrypt_raw, since our major use case is GnuPG, we handle
    the case of infinity differently.  While X25519 returns 0,
    libgcrypt returns an error.

For (3), I don't implement this as input validation but as result
handling.  If we do input validation, the values are:

    0x0000000000000000000000000000000000000000000000000000000000000000
    0x0000000000000000000000000000000000000000000000000000000000000001
    0x00b8495f16056286fdb1329ceb8d09da6ac49ff1fae35616aeb8413b7c7aebe0
    0x57119fd0dd4e22d8868e1c58c45c44045bef839c55b1d0b1248c50a3bc959c5f
    0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffec
    0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffed
    0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffee

Those other values (which is listed in https://cr.yp.to/ecdh.html )

    0x80b8495f16056286fdb1329ceb8d09da6ac49ff1fae35616aeb8413b7c7aebcd
    0xd7119fd0dd4e22d8868e1c58c45c44045bef839c55b1d0b1248c50a3bc959c4c
    0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffd9
    0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffda
    0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffdb

are not possible by the definition of X25519 where the MSB is cleared.

diff --git a/cipher/ecc.c b/cipher/ecc.c
index f6b2b69..663f4a8 100644
--- a/cipher/ecc.c
+++ b/cipher/ecc.c
@@ -1394,7 +1394,13 @@ ecc_encrypt_raw (gcry_sexp_t *r_ciph, gcry_sexp_t
s_data, gcry_sexp_t keyparms)
     /* R = kQ  <=>  R = kdG  */
     _gcry_mpi_ec_mul_point (&R, data, &pk.Q, ec);

-    if (_gcry_mpi_ec_get_affine (x, y, &R, ec))
+    /*
+     * Here, X is 0.  In the X25519 computation on Curve25519, X0
+     * function maps infinity to zero.  So, when PUBKEY_FLAG_DJB_TWEAK
+     * is enabled, skip the check to get the result of 0.
+     */
+    if (_gcry_mpi_ec_get_affine (x, y, &R, ec)
+        && !(flags & PUBKEY_FLAG_DJB_TWEAK))
       log_fatal ("ecdh: Failed to get affine coordinates for kdG\n");
     if (y)
       mpi_s = _gcry_ecc_ec2os (x, y, pk.E.p);
@@ -1598,8 +1604,8 @@ ecc_decrypt_raw (gcry_sexp_t *r_plain, gcry_sexp_t
s_data, gcry_sexp_t keyparms)
   if (DBG_CIPHER)
     log_printpnt ("ecc_decrypt    kG", &kG, NULL);

-  if (!(curvename && !strcmp (curvename, "Curve25519"))
-      /* For Curve25519, by its definition, validation should not be
done.  */
+  if (!(flags & PUBKEY_FLAG_DJB_TWEAK)
+      /* For X25519, by its definition, validation should not be done.  */
       && !_gcry_mpi_ec_curve_point (&kG, ec))
     {
       rc = GPG_ERR_INV_DATA;
@@ -1619,14 +1625,35 @@ ecc_decrypt_raw (gcry_sexp_t *r_plain,
gcry_sexp_t s_data, gcry_sexp_t keyparms)
     else
       y = mpi_new (0);

-    /*
-     * Here, x is 0.  In the X25519 computation on Curve25519, X0
-     * function maps infinity to zero.  So, when PUBKEY_FLAG_DJB_TWEAK
-     * is enabled, we can just skip the check to get the result of 0.
-     */
-    if (_gcry_mpi_ec_get_affine (x, y, &R, ec)
-        && !(flags & PUBKEY_FLAG_DJB_TWEAK))
-      log_fatal ("ecdh: Failed to get affine coordinates\n");
+    if (_gcry_mpi_ec_get_affine (x, y, &R, ec))
+      {
+        if  (!(flags & PUBKEY_FLAG_DJB_TWEAK))
+          log_fatal ("ecdh: Failed to get affine coordinates\n");
+        else
+          {
+            /*
+             * By the definition of X25519, this is the case where it
+             * returns 0, mapping infinity to zero.  However, we
+             * deliberately let it return an error.
+             *
+             * Comming here means that it might be decrypted by anyone
+             * with the shared secret of 0 (the result of this
+             * function could be always 0 by other scalar values,
+             * other than the private key of SK.D).
+             *
+             * So, it looks like an encrypted message but it can be
+             * decrypted by anyone, or at least something wrong
+             * happens.  Recipient should not proceed as if it were
+             * properly encrypted message.
+             *
+             * This handling is needed for our major usage of GnuPG,
+             * where it does the One-Pass Diffie-Hellman method,
+             * C(1, 1, ECC CDH), with an ephemeral key.
+             */
+            rc = GPG_ERR_INV_DATA;
+            goto leave;
+          }
+      }

     if (y)
       r = _gcry_ecc_ec2os (x, y, sk.E.p);
--