gpg: Allow ECDH with a smartcard returning just the x-coordinate.

* g10/ecdh.c (pk_ecdh_encrypt_with_shared_point): Factor extraction
part out to  ...
(extract_secret_x): new.  Allow for x-only coordinate.
(pk_ecdh_encrypt_with_shared_point): Change arg shared_mpi
to (shared,nshared).  Move param check to the top.  Add extra safety
check.
(pk_ecdh_decrypt): Adjust for change.
* g10/pkglue.c (get_data_from_sexp): New.
(pk_encrypt): Use it for "s" and adjusted for changed
pk_ecdh_encrypt_with_shared_point.
* g10/pubkey-enc.c (get_it): Remove conversion to an MPI and call
pk_ecdh_decrypt with the frame buffer.
--

Backported-from-master: f129b0e97730b47d62482fba9599db39b526f3d2)
Signed-off-by: Werner Koch <[email protected]>

4 files changed, 143 insertions, 100 deletions

diff --git a/g10/ecdh.c b/g10/ecdh.c
index 5bbea96c0..97d483838 100644
--- a/g10/ecdh.c
+++ b/g10/ecdh.c
@@ -82,15 +82,73 @@ pk_ecdh_default_params (unsigned int qbits)
 }
 
 
+/* Extract x-component from the point (SHARED,NSHARED) and strore it
+ * in a new buffer at R_SECRET_X.  POINT_NBYTES is the size to
+ * represent an EC point which is determined by the public key.
+ * SECRET_X_SIZE is the size of x component to represent an integer
+ * which is determined by the curve. */
+static gpg_error_t
+extract_secret_x (byte **r_secret_x,
+                  const char *shared, size_t nshared,
+                  size_t point_nbytes, size_t secret_x_size)
+{
+  byte *secret_x;
+
+  *r_secret_x = NULL;
+
+  /* Extract X from the result.  It must be in the format of:
+     04 || X || Y
+     40 || X
+     41 || X
+
+     Since it may come with the prefix, the size of point is larger
+     than or equals to the size of an integer X.  We also better check
+     that the provided shared point is not larger than the size needed
+     to represent the point.  */
+  if (point_nbytes < secret_x_size)
+    return gpg_error (GPG_ERR_BAD_DATA);
+  if (point_nbytes < nshared)
+    return gpg_error (GPG_ERR_BAD_DATA);
+
+  /* Extract x component of the shared point: this is the actual
+     shared secret. */
+  secret_x = xtrymalloc_secure (point_nbytes);
+  if (!secret_x)
+    return gpg_error_from_syserror ();
+
+  memcpy (secret_x, shared, nshared);
+
+  /* Wrangle the provided point unless only the x-component w/o any
+   * prefix was provided.  */
+  if (nshared != secret_x_size)
+    {
+      /* Remove the prefix.  */
+      if ((point_nbytes & 1))
+        memmove (secret_x, secret_x+1, secret_x_size);
+
+      /* Clear the rest of data.  */
+      if (point_nbytes - secret_x_size)
+        memset (secret_x+secret_x_size, 0, point_nbytes-secret_x_size);
+    }
+
+  if (DBG_CRYPTO)
+    log_printhex (secret_x, secret_x_size, "ECDH shared secret X is:");
+
+  *r_secret_x = secret_x;
+  return 0;
+}
+
+
 /* Encrypts/decrypts DATA using a key derived from the ECC shared
-   point SHARED_MPI using the FIPS SP 800-56A compliant method
+   point (SHARED,NSHARED) using the FIPS SP 800-56A compliant method
    key_derivation+key_wrapping.  If IS_ENCRYPT is true the function
    encrypts; if false, it decrypts.  PKEY is the public key and PK_FP
    the fingerprint of this public key.  On success the result is
    stored at R_RESULT; on failure NULL is stored at R_RESULT and an
    error code returned.  */
 gpg_error_t
-pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi,
+pk_ecdh_encrypt_with_shared_point (int is_encrypt,
+                                   const char *shared, size_t nshared,
                                    const byte pk_fp[MAX_FINGERPRINT_LEN],
                                    gcry_mpi_t data, gcry_mpi_t *pkey,
                                    gcry_mpi_t *r_result)
@@ -103,78 +161,15 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi,
   size_t kek_params_size;
   int kdf_hash_algo;
   int kdf_encr_algo;
+  size_t kek_size;
   unsigned char message[256];
   size_t message_size;
 
   *r_result = NULL;
 
-  nbits = pubkey_nbits (PUBKEY_ALGO_ECDH, pkey);
-  if (!nbits)
-    return gpg_error (GPG_ERR_TOO_SHORT);
-
-  {
-    size_t nbytes;
-
-    /* Extract x component of the shared point: this is the actual
-       shared secret. */
-    nbytes = (mpi_get_nbits (pkey[1] /* public point */)+7)/8;
-    secret_x = xtrymalloc_secure (nbytes);
-    if (!secret_x)
-      return gpg_error_from_syserror ();
-
-    err = gcry_mpi_print (GCRYMPI_FMT_USG, secret_x, nbytes,
-                          &nbytes, shared_mpi);
-    if (err)
-      {
-        xfree (secret_x);
-        log_error ("ECDH ephemeral export of shared point failed: %s\n",
-                   gpg_strerror (err));
-        return err;
-      }
-
-    /* Expected size of the x component */
-    secret_x_size = (nbits+7)/8;
-
-    /* Extract X from the result.  It must be in the format of:
-           04 || X || Y
-           40 || X
-           41 || X
-
-       Since it always comes with the prefix, it's larger than X.  In
-       old experimental version of libgcrypt, there is a case where it
-       returns X with no prefix of 40, so, nbytes == secret_x_size
-       is allowed.  */
-    if (nbytes < secret_x_size)
-      {
-        xfree (secret_x);
-        return gpg_error (GPG_ERR_BAD_DATA);
-      }
-
-    /* Remove the prefix.  */
-    if ((nbytes & 1))
-      memmove (secret_x, secret_x+1, secret_x_size);
-
-    /* Clear the rest of data.  */
-    if (nbytes - secret_x_size)
-      memset (secret_x+secret_x_size, 0, nbytes-secret_x_size);
-
-    if (DBG_CRYPTO)
-      log_printhex (secret_x, secret_x_size, "ECDH shared secret X is:");
-  }
-
-  /*** We have now the shared secret bytes in secret_x. ***/
-
-  /* At this point we are done with PK encryption and the rest of the
-   * function uses symmetric key encryption techniques to protect the
-   * input DATA.  The following two sections will simply replace
-   * current secret_x with a value derived from it.  This will become
-   * a KEK.
-   */
   if (!gcry_mpi_get_flag (pkey[2], GCRYMPI_FLAG_OPAQUE))
-    {
-      xfree (secret_x);
-      return gpg_error (GPG_ERR_BUG);
-    }
+    return gpg_error (GPG_ERR_BUG);
+
   kek_params = gcry_mpi_get_opaque (pkey[2], &nbits);
   kek_params_size = (nbits+7)/8;
 
@@ -183,10 +178,7 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi,
 
   /* Expect 4 bytes  03 01 hash_alg symm_alg.  */
   if (kek_params_size != 4 || kek_params[0] != 3 || kek_params[1] != 1)
-    {
-      xfree (secret_x);
-      return gpg_error (GPG_ERR_BAD_PUBKEY);
-    }
+    return gpg_error (GPG_ERR_BAD_PUBKEY);
 
   kdf_hash_algo = kek_params[2];
   kdf_encr_algo = kek_params[3];
@@ -199,17 +191,43 @@ pk_ecdh_encrypt_with_shared_point (int is_encrypt, gcry_mpi_t shared_mpi,
   if (kdf_hash_algo != GCRY_MD_SHA256
       && kdf_hash_algo != GCRY_MD_SHA384
       && kdf_hash_algo != GCRY_MD_SHA512)
-    {
-      xfree (secret_x);
-      return gpg_error (GPG_ERR_BAD_PUBKEY);
-    }
+    return gpg_error (GPG_ERR_BAD_PUBKEY);
+
   if (kdf_encr_algo != CIPHER_ALGO_AES
       && kdf_encr_algo != CIPHER_ALGO_AES192
       && kdf_encr_algo != CIPHER_ALGO_AES256)
-    {
-      xfree (secret_x);
-      return gpg_error (GPG_ERR_BAD_PUBKEY);
-    }
+    return gpg_error (GPG_ERR_BAD_PUBKEY);
+
+  kek_size = gcry_cipher_get_algo_keylen (kdf_encr_algo);
+  if (kek_size > gcry_md_get_algo_dlen (kdf_hash_algo))
+    return gpg_error (GPG_ERR_BAD_PUBKEY);
+
+
+  nbits = pubkey_nbits (PUBKEY_ALGO_ECDH, pkey);
+  if (!nbits)
+    return gpg_error (GPG_ERR_TOO_SHORT);
+
+  /* Expected size of the x component */
+  secret_x_size = (nbits+7)/8;
+
+  if (kek_size > secret_x_size)
+    return gpg_error (GPG_ERR_BAD_PUBKEY);
+
+  err = extract_secret_x (&secret_x, shared, nshared,
+                          (mpi_get_nbits (pkey[1] /* public point */)+7)/8,
+                          secret_x_size);
+  if (err)
+    return err;
+
+  /*** We have now the shared secret bytes in secret_x. ***/
+
+  /* At this point we are done with PK encryption and the rest of the
+   * function uses symmetric key encryption techniques to protect the
+   * input DATA.  The following two sections will simply replace
+   * current secret_x with a value derived from it.  This will become
+   * a KEK.
+   */
+
 
   /* Build kdf_params.  */
   {
@@ -484,12 +502,15 @@ pk_ecdh_generate_ephemeral_key (gcry_mpi_t *pkey, gcry_mpi_t *r_k)
 
 /* Perform ECDH decryption.   */
 int
-pk_ecdh_decrypt (gcry_mpi_t * result, const byte sk_fp[MAX_FINGERPRINT_LEN],
-                 gcry_mpi_t data, gcry_mpi_t shared, gcry_mpi_t * skey)
+pk_ecdh_decrypt (gcry_mpi_t *result, const byte sk_fp[MAX_FINGERPRINT_LEN],
+                 gcry_mpi_t data,
+                 const char *shared, size_t nshared,
+                 gcry_mpi_t *skey)
 {
   if (!data)
     return gpg_error (GPG_ERR_BAD_MPI);
-  return pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/, shared,
+  return pk_ecdh_encrypt_with_shared_point (0 /*=decryption*/,
+                                            shared, nshared,
                                             sk_fp, data/*encr data as an MPI*/,
                                             skey, result);
 }
diff --git a/g10/pkglue.c b/g10/pkglue.c
index 8021a94db..e053657ee 100644
--- a/g10/pkglue.c
+++ b/g10/pkglue.c
@@ -47,6 +47,28 @@ get_mpi_from_sexp (gcry_sexp_t sexp, const char *item, int mpifmt)
 }
 
 
+static byte *
+get_data_from_sexp (gcry_sexp_t sexp, const char *item, size_t *r_size)
+{
+  gcry_sexp_t list;
+  size_t valuelen;
+  const char *value;
+  byte *v;
+
+  if (DBG_CRYPTO)
+    log_printsexp ("get_data_from_sexp:", sexp);
+
+  list = gcry_sexp_find_token (sexp, item, 0);
+  log_assert (list);
+  value = gcry_sexp_nth_data (list, 1, &valuelen);
+  log_assert (value);
+  v = xtrymalloc (valuelen);
+  memcpy (v, value, valuelen);
+  gcry_sexp_release (list);
+  *r_size = valuelen;
+  return v;
+}
+
 
 /****************
  * Emulate our old PK interface here - sometime in the future we might
@@ -309,12 +331,19 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data,
     ;
   else if (algo == PUBKEY_ALGO_ECDH)
     {
-      gcry_mpi_t shared, public, result;
+      gcry_mpi_t public, result;
       byte fp[MAX_FINGERPRINT_LEN];
       size_t fpn;
+      byte *shared;
+      size_t nshared;
 
       /* Get the shared point and the ephemeral public key.  */
-      shared = get_mpi_from_sexp (s_ciph, "s", GCRYMPI_FMT_USG);
+      shared = get_data_from_sexp (s_ciph, "s", &nshared);
+      if (!shared)
+        {
+          rc = gpg_error_from_syserror ();
+          goto leave;
+        }
       public = get_mpi_from_sexp (s_ciph, "e", GCRYMPI_FMT_USG);
       gcry_sexp_release (s_ciph);
       s_ciph = NULL;
@@ -330,9 +359,10 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data,
       if (fpn != 20)
         rc = gpg_error (GPG_ERR_INV_LENGTH);
       else
-        rc = pk_ecdh_encrypt_with_shared_point (1 /*=encrypton*/, shared,
+        rc = pk_ecdh_encrypt_with_shared_point (1 /*=encrypton*/,
+                                                shared, nshared,
                                                 fp, data, pkey, &result);
-      gcry_mpi_release (shared);
+      xfree (shared);
       if (!rc)
         {
           resarr[0] = public;
@@ -352,6 +382,7 @@ pk_encrypt (pubkey_algo_t algo, gcry_mpi_t *resarr, gcry_mpi_t data,
         resarr[1] = get_mpi_from_sexp (s_ciph, "b", GCRYMPI_FMT_USG);
     }
 
+ leave:
   gcry_sexp_release (s_ciph);
   return rc;
 }
diff --git a/g10/pkglue.h b/g10/pkglue.h
index 77a380191..5780e2356 100644
--- a/g10/pkglue.h
+++ b/g10/pkglue.h
@@ -36,7 +36,7 @@ int pk_check_secret_key (pubkey_algo_t algo, gcry_mpi_t *skey);
 gcry_mpi_t  pk_ecdh_default_params (unsigned int qbits);
 gpg_error_t pk_ecdh_generate_ephemeral_key (gcry_mpi_t *pkey, gcry_mpi_t *r_k);
 gpg_error_t pk_ecdh_encrypt_with_shared_point
-/*         */  (int is_encrypt, gcry_mpi_t shared_mpi,
+/*         */  (int is_encrypt, const char *shared, size_t nshared,
                 const byte pk_fp[MAX_FINGERPRINT_LEN],
                 gcry_mpi_t data, gcry_mpi_t *pkey,
                 gcry_mpi_t *out);
@@ -44,7 +44,8 @@ gpg_error_t pk_ecdh_encrypt_with_shared_point
 int pk_ecdh_encrypt (gcry_mpi_t *resarr, const byte pk_fp[MAX_FINGERPRINT_LEN],
                      gcry_mpi_t data, gcry_mpi_t * pkey);
 int pk_ecdh_decrypt (gcry_mpi_t *result, const byte sk_fp[MAX_FINGERPRINT_LEN],
-                     gcry_mpi_t data, gcry_mpi_t shared, gcry_mpi_t * skey);
+                     gcry_mpi_t data, const char *shared, size_t nshared,
+                     gcry_mpi_t *skey);
 
 
 #endif /*GNUPG_G10_PKGLUE_H*/
diff --git a/g10/pubkey-enc.c b/g10/pubkey-enc.c
index 30a4bc099..91dfb7798 100644
--- a/g10/pubkey-enc.c
+++ b/g10/pubkey-enc.c
@@ -264,20 +264,10 @@ get_it (ctrl_t ctrl,
 
   if (sk->pubkey_algo == PUBKEY_ALGO_ECDH)
     {
-      gcry_mpi_t shared_mpi;
       gcry_mpi_t decoded;
 
-      /* At the beginning the frame are the bytes of shared point MPI.  */
-      err = gcry_mpi_scan (&shared_mpi, GCRYMPI_FMT_USG, frame, nframe, NULL);
-      if (err)
-        {
-          err = gpg_error (GPG_ERR_WRONG_SECKEY);
-          goto leave;
-        }
-
       err = pk_ecdh_decrypt (&decoded, fp, enc->data[1]/*encr data as an MPI*/,
-                             shared_mpi, sk->pkey);
-      mpi_release (shared_mpi);
+                             frame, nframe, sk->pkey);
       if(err)
         goto leave;