From 35e0c5fc71872ee59f511bd1a926b2664097f313 Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Wed, 28 Jun 2017 16:07:32 +0200 Subject: [PATCH] clean-up a bit around DSA * comments * dsa_test() * order of alloc/free of key parts --- src/headers/tomcrypt_pk.h | 2 +- src/math/radix_to_bin.c | 6 +++--- src/pk/dsa/dsa_free.c | 2 +- src/pk/dsa/dsa_generate_pqg.c | 2 +- src/pk/dsa/dsa_import.c | 2 +- src/pk/dsa/dsa_set.c | 12 ++++++------ src/pk/rsa/rsa_set.c | 2 +- tests/dsa_test.c | 34 ++++++++++++++++++++-------------- 8 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 963d0d1..6e263a0 100644 --- a/src/headers/tomcrypt_pk.h +++ b/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, diff --git a/src/math/radix_to_bin.c b/src/math/radix_to_bin.c index 7486919..72742b5 100644 --- a/src/math/radix_to_bin.c +++ b/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 diff --git a/src/pk/dsa/dsa_free.c b/src/pk/dsa/dsa_free.c index 812464e..5cac656 100644 --- a/src/pk/dsa/dsa_free.c +++ b/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; } diff --git a/src/pk/dsa/dsa_generate_pqg.c b/src/pk/dsa/dsa_generate_pqg.c index 5d8c691..d6e3ac7 100644 --- a/src/pk/dsa/dsa_generate_pqg.c +++ b/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 */ diff --git a/src/pk/dsa/dsa_import.c b/src/pk/dsa/dsa_import.c index e1edaab..d71cdd5 100644 --- a/src/pk/dsa/dsa_import.c +++ b/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; } diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index 32e5d3b..f7c6b5c 100755 --- a/src/pk/dsa/dsa_set.c +++ b/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->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->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->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; diff --git a/src/pk/rsa/rsa_set.c b/src/pk/rsa/rsa_set.c index c454320..0d540c4 100755 --- a/src/pk/rsa/rsa_set.c +++ b/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; diff --git a/tests/dsa_test.c b/tests/dsa_test.c index 9cee7af..9dd14d7 100644 --- a/tests/dsa_test.c +++ b/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); - DO((memcmp(tmp, openssl_priv_dsa, sizeof(openssl_priv_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR); + if (compare_testvector(tmp, x, openssl_priv_dsa, sizeof(openssl_priv_dsa), + "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); - DO((memcmp(tmp, openssl_pub_dsa, sizeof(openssl_pub_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR); + if (compare_testvector(tmp, x, openssl_pub_dsa, sizeof(openssl_pub_dsa), + "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); - DO((memcmp(tmp, openssl_pub_dsa, sizeof(openssl_pub_dsa)) == 0)?CRYPT_OK:CRYPT_ERROR); + if (compare_testvector(tmp, x, openssl_pub_dsa, sizeof(openssl_pub_dsa), + "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)) { - fprintf(stderr, "DSA private export failed to match dsa_import_radix(16, ..)\n"); - return 1; + if (compare_testvector(buf, len, openssl_priv_dsa, sizeof(openssl_priv_dsa), + "DSA private export failed from dsa_set_pqg() & dsa_set_key()\n", 0)) { + 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)) { - fprintf(stderr, "DSA public export failed to match dsa_import_radix(16, ..)\n"); - return 1; + if (compare_testvector(buf, len, openssl_pub_dsa, sizeof(openssl_pub_dsa), + "DSA public export failed from dsa_set_pqg() & dsa_set_key()\n", 0)) { + 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));