Browse Source

fix possible free of not yet allocated key parameters

There would have been a call to mp_clear_multi() of all the key parameters
that are not yet allocated, in the case where the calculations of p, q,
tmp1 or tmp2 created an error.

This also includes a proposed improvement from the OLPC project to free
elements in the reverse order as they were allocated.
Steffen Jaeckel 11 years ago
parent
commit
98893c077b
1 changed files with 13 additions and 13 deletions
  1. 13 13
      src/pk/rsa/rsa_make_key.c

+ 13 - 13
src/pk/rsa/rsa_make_key.c

@@ -13,11 +13,11 @@
 /**
   @file rsa_make_key.c
   RSA key generation, Tom St Denis
-*/  
+*/
 
 #ifdef LTC_MRSA
 
-/** 
+/**
    Create an RSA key
    @param prng     An active PRNG state
    @param wprng    The index of the PRNG desired
@@ -51,26 +51,26 @@ int rsa_make_key(prng_state *prng, int wprng, int size, long e, rsa_key *key)
    }
 
    /* make primes p and q (optimization provided by Wayne Scott) */
-   if ((err = mp_set_int(tmp3, e)) != CRYPT_OK)                      { goto errkey; }  /* tmp3 = e */
+   if ((err = mp_set_int(tmp3, e)) != CRYPT_OK)                      { goto cleanup; }  /* tmp3 = e */
 
    /* make prime "p" */
    do {
-       if ((err = rand_prime( p, size/2, prng, wprng)) != CRYPT_OK)  { goto errkey; }
-       if ((err = mp_sub_d( p, 1,  tmp1)) != CRYPT_OK)               { goto errkey; }  /* tmp1 = p-1 */
-       if ((err = mp_gcd( tmp1,  tmp3,  tmp2)) != CRYPT_OK)          { goto errkey; }  /* tmp2 = gcd(p-1, e) */
+       if ((err = rand_prime( p, size/2, prng, wprng)) != CRYPT_OK)  { goto cleanup; }
+       if ((err = mp_sub_d( p, 1,  tmp1)) != CRYPT_OK)               { goto cleanup; }  /* tmp1 = p-1 */
+       if ((err = mp_gcd( tmp1,  tmp3,  tmp2)) != CRYPT_OK)          { goto cleanup; }  /* tmp2 = gcd(p-1, e) */
    } while (mp_cmp_d( tmp2, 1) != 0);                                                  /* while e divides p-1 */
 
    /* make prime "q" */
    do {
-       if ((err = rand_prime( q, size/2, prng, wprng)) != CRYPT_OK)  { goto errkey; }
-       if ((err = mp_sub_d( q, 1,  tmp1)) != CRYPT_OK)               { goto errkey; } /* tmp1 = q-1 */
-       if ((err = mp_gcd( tmp1,  tmp3,  tmp2)) != CRYPT_OK)          { goto errkey; } /* tmp2 = gcd(q-1, e) */
+       if ((err = rand_prime( q, size/2, prng, wprng)) != CRYPT_OK)  { goto cleanup; }
+       if ((err = mp_sub_d( q, 1,  tmp1)) != CRYPT_OK)               { goto cleanup; } /* tmp1 = q-1 */
+       if ((err = mp_gcd( tmp1,  tmp3,  tmp2)) != CRYPT_OK)          { goto cleanup; } /* tmp2 = gcd(q-1, e) */
    } while (mp_cmp_d( tmp2, 1) != 0);                                                 /* while e divides q-1 */
 
    /* tmp1 = lcm(p-1, q-1) */
-   if ((err = mp_sub_d( p, 1,  tmp2)) != CRYPT_OK)                   { goto errkey; } /* tmp2 = p-1 */
+   if ((err = mp_sub_d( p, 1,  tmp2)) != CRYPT_OK)                   { goto cleanup; } /* tmp2 = p-1 */
                                                                                       /* tmp1 = q-1 (previous do/while loop) */
-   if ((err = mp_lcm( tmp1,  tmp2,  tmp1)) != CRYPT_OK)              { goto errkey; } /* tmp1 = lcm(p-1, q-1) */
+   if ((err = mp_lcm( tmp1,  tmp2,  tmp1)) != CRYPT_OK)              { goto cleanup; } /* tmp1 = lcm(p-1, q-1) */
 
    /* make key */
    if ((err = mp_init_multi(&key->e, &key->d, &key->N, &key->dQ, &key->dP, &key->qP, &key->p, &key->q, NULL)) != CRYPT_OK) {
@@ -99,9 +99,9 @@ int rsa_make_key(prng_state *prng, int wprng, int size, long e, rsa_key *key)
    err       = CRYPT_OK;
    goto cleanup;
 errkey:
-   mp_clear_multi(key->d, key->e, key->N, key->dQ, key->dP, key->qP, key->p, key->q, NULL);
+   mp_clear_multi(key->q, key->p, key->qP, key->dP, key->dQ, key->N, key->d, key->e, NULL);
 cleanup:
-   mp_clear_multi(tmp3, tmp2, tmp1, p, q, NULL);
+   mp_clear_multi(tmp3, tmp2, tmp1, q, p, NULL);
    return err;
 }