More ECDH code cleanups

4 files changed, 72 insertions, 101 deletions

diff --git a/g10/ChangeLog b/g10/ChangeLog
index 801146459..3ce4d1fac 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,5 +1,9 @@
 2011-01-25  Werner Koch  <[email protected]>
 
+	* ecdh.c (pk_ecdh_default_params_to_mpi): Remove.
+	(pk_ecdh_default_params): Rewrite.  Factor KEK table out to ..
+	(kek_params_table): .. here.
+
 	* pubkey-enc.c (get_it): Fix assertion.
 	Use GPG_ERR_WRONG_SECKEY instead of log_fatal.  Add safety checks
 	for NFRAME.
@@ -14,6 +18,8 @@
 	(pk_ecc_keypair_gen): Fold it into ..
 	(gen_ecc): .. this.
 	(ask_keysize): Use proper rounding for ECC.
+	(pk_ecc_build_key_params): Remove NBITSSTR.
+
 	* verify-stubs.c: Remove.
 
 2011-01-20  Werner Koch  <[email protected]>
diff --git a/g10/ecdh.c b/g10/ecdh.c
index cb251fef2..1fa36aa21 100644
--- a/g10/ecdh.c
+++ b/g10/ecdh.c
@@ -30,26 +30,15 @@
 #include "main.h"
 #include "options.h"
 
-gcry_mpi_t
-pk_ecdh_default_params_to_mpi (int qbits)
+/* A table with the default KEK parameters used by GnuPG.  */
+static const struct
 {
-  gpg_error_t err;
-  gcry_mpi_t result;
-  /* Defaults are the strongest possible choices. Performance is not
-     an issue here, only interoperability.  */
-  byte kek_params[4] = { 
-	3	/*size of following field*/, 
-	1	/*fixed version for KDF+AESWRAP*/, 
-	DIGEST_ALGO_SHA512	/* KEK MD */, 
-	CIPHER_ALGO_AES256 	/*KEK AESWRAP alg*/
-  };
-  int i;
-
-  static const struct {
-    int qbits;
-    int openpgp_hash_id;
-    int openpgp_cipher_id;
-  } kek_params_table[] = {
+  unsigned int qbits;
+  int openpgp_hash_id;   /* KEK digest algorithm. */
+  int openpgp_cipher_id; /* KEK cipher algorithm. */
+} kek_params_table[] = 
+  /* Note: Must be sorted by ascending values for QBITS.  */
+  {
     { 256, DIGEST_ALGO_SHA256, CIPHER_ALGO_AES    },
     { 384, DIGEST_ALGO_SHA384, CIPHER_ALGO_AES256 },
 
@@ -57,77 +46,44 @@ pk_ecdh_default_params_to_mpi (int qbits)
     { 528, DIGEST_ALGO_SHA512, CIPHER_ALGO_AES256 }
   };
 
-  for (i=0; i<sizeof(kek_params_table)/sizeof(kek_params_table[0]); i++)
-    {
-      if (kek_params_table[i].qbits >= qbits)
-        {
-          kek_params[2] = kek_params_table[i].openpgp_hash_id;
-          kek_params[3] = kek_params_table[i].openpgp_cipher_id;
-          break;
-        }
-    }
-  if (DBG_CIPHER)
-    log_printhex ("ecdh kek params are", kek_params, sizeof(kek_params) );
-
-  err = gcry_mpi_scan (&result, GCRYMPI_FMT_USG,
-                       kek_params, sizeof(kek_params), NULL);
-  if (err)
-    log_fatal ("mpi_scan failed: %s\n", gpg_strerror (err));
-
-  return result;
-}
 
 
 /* Returns allocated (binary) KEK parameters; the size is returned in
- * sizeout.  The caller must free the returned value with xfree.
- * Returns NULL on error.
- */
+   sizeout.  The caller must free the returned value.  Returns NULL
+   and sets ERRNO on error.  */
 byte *
-pk_ecdh_default_params (int qbits, size_t *sizeout)
+pk_ecdh_default_params (unsigned int qbits, size_t *sizeout)
 {
-  /* Defaults are the strongest possible choices. Performance is not
-     an issue here, only interoperability. */
-  byte kek_params[4] = { 
-	3	/*size of following field*/, 
-	1	/*fixed version for KDF+AESWRAP*/, 
-	DIGEST_ALGO_SHA512	/* KEK MD */, 
-	CIPHER_ALGO_AES256 	/* KEK AESWRAP alg */
-  };
+  byte kek_params[4];
   int i;
-  
-  static const struct {
-    int qbits;
-    int openpgp_hash_id;
-    int openpgp_cipher_id;
-  } kek_params_table[] = {
-    { 256, DIGEST_ALGO_SHA256, CIPHER_ALGO_AES    },
-    { 384, DIGEST_ALGO_SHA384, CIPHER_ALGO_AES256 },
-    /* Note: 528 is 521 rounded to the 8 bit boundary  */
-    { 528, DIGEST_ALGO_SHA512, CIPHER_ALGO_AES256 }	
-  };
-
-  byte *p;
+  byte *buffer;
 
-  *sizeout = 0;
+  kek_params[0] = 3; /* Number of bytes to follow. */
+  kek_params[1] = 1; /* Version for KDF+AESWRAP.   */ 
   
-  for (i=0; i<sizeof(kek_params_table)/sizeof(kek_params_table[0]); i++)
+  /* Search for matching KEK parameter.  Defaults to the strongest
+     possible choices.  Performance is not an issue here, only
+     interoperability.  */
+  for (i=0; i < DIM (kek_params_table); i++)
     {
-      if (kek_params_table[i].qbits >= qbits)
+      if (kek_params_table[i].qbits >= qbits
+          || i+1 == DIM (kek_params_table))
         {
           kek_params[2] = kek_params_table[i].openpgp_hash_id;
           kek_params[3] = kek_params_table[i].openpgp_cipher_id;
           break;
         }
     }
-  if (DBG_CIPHER )
-    log_printhex ("ecdh kek params are", kek_params, sizeof(kek_params));
-
-  p = xtrymalloc (sizeof(kek_params));
-  if (!p)
+  assert (i < DIM (kek_params_table));
+  if (DBG_CIPHER)
+    log_printhex ("ECDH KEK params are", kek_params, sizeof(kek_params) );
+  
+  buffer = xtrymalloc (sizeof(kek_params));
+  if (!buffer)
     return NULL;
-  memcpy (p, kek_params, sizeof(kek_params));
-  *sizeout = sizeof(kek_params);
-  return p;
+  memcpy (buffer, kek_params, sizeof (kek_params));
+  *sizeout = sizeof (kek_params);
+  return buffer;
 }
 
 
diff --git a/g10/keygen.c b/g10/keygen.c
index 366bedf0e..b42121b28 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -1329,7 +1329,7 @@ gen_dsa (unsigned int nbits, KBNODE pub_root,
 
 
 
-/* Create an S-expression string out QBITS, ALGO and the TRANSIENT
+/* Create an S-expression string out of QBITS, ALGO and the TRANSIENT
    flag.  On success a malloced string is returned, on failure NULL
    and ERRNO is set.  */
 static char *
@@ -1337,56 +1337,63 @@ pk_ecc_build_key_params (int qbits, int algo, int transient)
 {
   byte *kek_params = NULL;
   size_t kek_params_size;
-  char nbitsstr[35];
   char qbitsstr[35];
-  char *keyparms;
-  int n;
+  char *result;
+  size_t n;
 
   /* KEK parameters are only needed for long term key generation.  */
   if (!transient && algo == PUBKEY_ALGO_ECDH)
-    kek_params = pk_ecdh_default_params (qbits, &kek_params_size);
+    {
+      kek_params = pk_ecdh_default_params (qbits, &kek_params_size);
+      if (!kek_params)
+        return NULL;
+    }
   else
     kek_params = NULL;
 
-  snprintf (nbitsstr, sizeof nbitsstr, "%u", qbits);
   snprintf (qbitsstr, sizeof qbitsstr, "%u", qbits);
   if (algo == PUBKEY_ALGO_ECDSA || !kek_params)
     {
-      keyparms = xtryasprintf ("(genkey(%s(nbits %zu:%s)"
+      result = xtryasprintf ("(genkey(%s(nbits %zu:%s)"
                                /**/      "(qbits %zu:%s)"
                                /**/      "(transient-key 1:%d)))",
                                algo == PUBKEY_ALGO_ECDSA ? "ecdsa" : "ecdh",
-                               strlen (nbitsstr), nbitsstr,
+                               strlen (qbitsstr), qbitsstr,
                                strlen (qbitsstr), qbitsstr,
                                transient);
     }
   else
     {
-      assert (kek_params);
+      char *tmpstr;
 
-      keyparms = xtryasprintf ("(genkey(ecdh(nbits %zu:%s)"
-                               /**/        "(qbits %zu:%s)"
-                               /**/        "(transient-key 1:%d)"
-                               /**/        "(kek-params %zu:",
-                               strlen (nbitsstr), nbitsstr,
-                               strlen (qbitsstr), qbitsstr,
-                               transient,
-                               kek_params_size);
-      if (keyparms)
+      assert (kek_params);
+      tmpstr = xtryasprintf ("(genkey(ecdh(nbits %zu:%s)"
+                             /**/        "(qbits %zu:%s)"
+                             /**/        "(transient-key 1:%d)"
+                             /**/        "(kek-params %zu:",
+                             strlen (qbitsstr), qbitsstr,
+                             strlen (qbitsstr), qbitsstr,
+                             transient,
+                             kek_params_size);
+      if (!tmpstr)
         {
-          n = strlen (keyparms);
-          keyparms = xtryrealloc (keyparms, n + kek_params_size + 4);
+          xfree (kek_params);
+          return NULL;
         }
-      if (!keyparms)
+      /* Append the binary KEK parmas.  */
+      n = strlen (tmpstr);
+      result = xtryrealloc (tmpstr, n + kek_params_size + 4);
+      if (!result)
         {
+          xfree (tmpstr);
           xfree (kek_params);
           return NULL;
         }
-      memcpy (keyparms+n, kek_params, kek_params_size);
-      xfree (kek_params);
-      memcpy (keyparms+n+kek_params_size, ")))", 4);
+      memcpy (result + n, kek_params, kek_params_size);
+      strcpy (result + n + kek_params_size, ")))");
     }
-  return keyparms;
+  xfree (kek_params);
+  return result;
 }
 
 
diff --git a/g10/pkglue.h b/g10/pkglue.h
index 0ceb43f55..b1cfbe507 100644
--- a/g10/pkglue.h
+++ b/g10/pkglue.h
@@ -20,6 +20,7 @@
 #ifndef GNUPG_G10_PKGLUE_H
 #define GNUPG_G10_PKGLUE_H
 
+/*-- pkglue.c --*/
 gcry_mpi_t mpi_from_sexp (gcry_sexp_t sexp, const char * item);
 
 int pk_verify (int algo, gcry_mpi_t hash, gcry_mpi_t *data,
@@ -29,12 +30,13 @@ int pk_encrypt (int algo, gcry_mpi_t *resarr, gcry_mpi_t data,
                 gcry_mpi_t *pkey);
 int pk_check_secret_key (int algo, gcry_mpi_t *skey);
 
+
+/*-- ecdh.c --*/
+byte *pk_ecdh_default_params (unsigned int qbits, size_t *sizeout);
 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 pk_ecdh_default_params_to_mpi (int qbits);
-byte *pk_ecdh_default_params (int qbits, size_t *sizeout);
 
 #endif /*GNUPG_G10_PKGLUE_H*/