clean-up a bit around DSA

* comments
* dsa_test()
* order of alloc/free of key parts

src/headers/tomcrypt_pk.h

@@ -129,7 +129,7 @@ int rsa_import_pkcs8(const unsigned char *in, unsigned long inlen,
 
 int rsa_set_key(const unsigned char *N,  unsigned long Nlen,
                 const unsigned char *e,  unsigned long elen,
-                const unsigned char *d,  unsigned long dlen, /* is NULL for public keys */
+                const unsigned char *d,  unsigned long dlen,
                 rsa_key *key);
 int rsa_set_factors(const unsigned char *p,  unsigned long plen,
                     const unsigned char *q,  unsigned long qlen,

src/math/radix_to_bin.c

@@ -10,15 +10,15 @@
 
 /**
    @file radix_to_bin.c
-   Convert an MPI from a specific radix to binary data.
+   Convert data from a specific radix to binary.
    Steffen Jaeckel
 */
 
 /**
-   Convert an MPI from a specific radix to binary data
+   Convert data from a specific radix to binary
 
    @param in    The input
-   @param radix The radix of the input
+   @param radix The radix of the input 2..64
    @param out   The output buffer
    @param len   [in/out] The length of the output buffer
 

src/pk/dsa/dsa_free.c

@@ -22,7 +22,7 @@
 void dsa_free(dsa_key *key)
 {
    LTC_ARGCHKVD(key != NULL);
-   mp_cleanup_multi(&key->g, &key->q, &key->p, &key->x, &key->y, NULL);
+   mp_cleanup_multi(&key->y, &key->x, &key->q, &key->g, &key->p, NULL);
    key->type = key->qord = 0;
 }
 

src/pk/dsa/dsa_generate_pqg.c

@@ -225,7 +225,7 @@ int dsa_generate_pqg(prng_state *prng, int wprng, int group_size, int modulus_si
   LTC_ARGCHK(ltc_mp.name != NULL);
 
   /* init mp_ints */
-  if ((err = mp_init_multi(&key->g, &key->q, &key->p, &key->x, &key->y, NULL)) != CRYPT_OK) {
+  if ((err = mp_init_multi(&key->p, &key->g, &key->q, &key->x, &key->y, NULL)) != CRYPT_OK) {
     return err;
   }
  /* generate params */

src/pk/dsa/dsa_import.c

@@ -125,7 +125,7 @@ LBL_OK:
 
   return CRYPT_OK;
 LBL_ERR:
-   mp_clear_multi(key->p, key->g, key->q, key->x, key->y, NULL);
+   dsa_free(key);
    return err;
 }
 

src/pk/dsa/dsa_set.c

@@ -12,7 +12,7 @@
 #ifdef LTC_MDSA
 
 /**
-  Import DSA public or private key from raw numbers
+  Import DSA's p, q & g from raw numbers
   @param p       DSA's p  in binary representation
   @param q       DSA's q  in binary representation
   @param g       DSA's g  in binary representation
@@ -42,9 +42,9 @@ int dsa_set_pqg(const unsigned char *p,  unsigned long plen,
    err = mp_init_multi(&key->p, &key->g, &key->q, &key->x, &key->y, NULL);
    if (err != CRYPT_OK) return err;
 
-   if ((err = mp_read_unsigned_bin(key->p , (unsigned char *)p , plen)) != CRYPT_OK) { goto LBL_ERR; }
+   if ((err = mp_read_unsigned_bin(key->p, (unsigned char *)p , plen)) != CRYPT_OK) { goto LBL_ERR; }
-   if ((err = mp_read_unsigned_bin(key->g , (unsigned char *)g , glen)) != CRYPT_OK) { goto LBL_ERR; }
+   if ((err = mp_read_unsigned_bin(key->g, (unsigned char *)g , glen)) != CRYPT_OK) { goto LBL_ERR; }
-   if ((err = mp_read_unsigned_bin(key->q , (unsigned char *)q , qlen)) != CRYPT_OK) { goto LBL_ERR; }
+   if ((err = mp_read_unsigned_bin(key->q, (unsigned char *)q , qlen)) != CRYPT_OK) { goto LBL_ERR; }
 
    key->qord = mp_unsigned_bin_size(key->q);
 
@@ -82,10 +82,10 @@ int dsa_set_key(const unsigned char *pub, unsigned long publen,
    LTC_ARGCHK(key->q      != NULL);
    LTC_ARGCHK(ltc_mp.name != NULL);
 
-   if ((err = mp_read_unsigned_bin(key->y , (unsigned char *)pub , publen)) != CRYPT_OK) { goto LBL_ERR; }
+   if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)pub , publen)) != CRYPT_OK) { goto LBL_ERR; }
    if (priv != NULL) {
       key->type = PK_PRIVATE;
-      if ((err = mp_read_unsigned_bin(key->x , (unsigned char *)priv , privlen)) != CRYPT_OK) { goto LBL_ERR; }
+      if ((err = mp_read_unsigned_bin(key->x, (unsigned char *)priv , privlen)) != CRYPT_OK) { goto LBL_ERR; }
    }
    else {
       key->type = PK_PUBLIC;

src/pk/rsa/rsa_set.c

@@ -25,7 +25,7 @@
 */
 int rsa_set_key(const unsigned char *N,  unsigned long Nlen,
                 const unsigned char *e,  unsigned long elen,
-                const unsigned char *d,  unsigned long dlen, /* is NULL for public keys */
+                const unsigned char *d,  unsigned long dlen,
                 rsa_key *key)
 {
    int err;

tests/dsa_test.c

@@ -112,21 +112,27 @@ static int _dsa_compat_test(void)
 
   x = sizeof(tmp);
   DO(dsa_export(tmp, &x, PK_PRIVATE | PK_STD, &key));
-  DO((x == sizeof(openssl_priv_dsa))?CRYPT_OK:CRYPT_ERROR);
+  if (compare_testvector(tmp, x, openssl_priv_dsa, sizeof(openssl_priv_dsa),
-  DO((memcmp(tmp, openssl_priv_dsa, sizeof(openssl_priv_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR);
+                         "DSA private export failed from dsa_import(priv_key)\n", 0)) {
+     return CRYPT_FAIL_TESTVECTOR;
+  }
 
   x = sizeof(tmp);
   DO(dsa_export(tmp, &x, PK_PUBLIC | PK_STD, &key));
-  DO((x == sizeof(openssl_pub_dsa))?CRYPT_OK:CRYPT_ERROR);
+  if (compare_testvector(tmp, x, openssl_pub_dsa, sizeof(openssl_pub_dsa),
-  DO((memcmp(tmp, openssl_pub_dsa, sizeof(openssl_pub_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR);
+                         "DSA public export failed from dsa_import(priv_key)\n", 0)) {
+     return CRYPT_FAIL_TESTVECTOR;
+  }
   dsa_free(&key);
 
   DO(dsa_import(openssl_pub_dsa, sizeof(openssl_pub_dsa), &key));
 
   x = sizeof(tmp);
   DO(dsa_export(tmp, &x, PK_PUBLIC | PK_STD, &key));
-  DO((x == sizeof(openssl_pub_dsa))?CRYPT_OK:CRYPT_ERROR);
+  if (compare_testvector(tmp, x, openssl_pub_dsa, sizeof(openssl_pub_dsa),
-  DO((memcmp(tmp, openssl_pub_dsa, sizeof(openssl_pub_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR);
+                         "DSA public export failed from dsa_import(pub_key)\n", 0)) {
+     return CRYPT_FAIL_TESTVECTOR;
+  }
   dsa_free(&key);
 
   /* try import private key from raw hexadecimal numbers */
@@ -148,9 +154,9 @@ static int _dsa_compat_test(void)
                  &key));
   len = sizeof(buf);
   DO(dsa_export(buf, &len, PK_PRIVATE | PK_STD, &key));
-  if (len != sizeof(openssl_priv_dsa) || memcmp(buf, openssl_priv_dsa, len)) {
+  if (compare_testvector(buf, len, openssl_priv_dsa, sizeof(openssl_priv_dsa),
-     fprintf(stderr, "DSA private export failed to match dsa_import_radix(16, ..)\n");
+                         "DSA private export failed from dsa_set_pqg() & dsa_set_key()\n", 0)) {
-     return 1;
+     return CRYPT_FAIL_TESTVECTOR;
   }
   dsa_free(&key);
 
@@ -164,13 +170,13 @@ static int _dsa_compat_test(void)
                  &key));
   len = sizeof(buf);
   DO(dsa_export(buf, &len, PK_PUBLIC | PK_STD, &key));
-  if (len != sizeof(openssl_pub_dsa) || memcmp(buf, openssl_pub_dsa, len)) {
+  if (compare_testvector(buf, len, openssl_pub_dsa, sizeof(openssl_pub_dsa),
-     fprintf(stderr, "DSA public export failed to match dsa_import_radix(16, ..)\n");
+                         "DSA public export failed from dsa_set_pqg() & dsa_set_key()\n", 0)) {
-     return 1;
+     return CRYPT_FAIL_TESTVECTOR;
   }
   dsa_free(&key);
 
-  return 0;
+  return CRYPT_OK;
 }
 
 int dsa_test(void)
@@ -181,7 +187,7 @@ int dsa_test(void)
    dsa_key key = LTC_DSA_KEY_INITIALIZER;
    dsa_key key2 = LTC_DSA_KEY_INITIALIZER;
 
-   _dsa_compat_test();
+   DO(_dsa_compat_test());
 
    /* make a random key */
    DO(dsa_generate_pqg(&yarrow_prng, find_prng("yarrow"), 20, 128, &key));