From 2505e3b6098d490536dfdda2e8fff0a7809f89f5 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Wed, 30 Aug 2017 00:04:07 +0200 Subject: [PATCH 01/15] add basic validity tests to dsa_set --- src/pk/dsa/dsa_set.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index a630974..d85ae8c 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -45,6 +45,12 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, key->qord = mp_unsigned_bin_size(key->q); + /* just a quick, basic test - use dsa_verify_key if you want more */ + if (mp_cmp_d(key->p, 1) != LTC_MP_GT || mp_cmp_d(key->g, 1) != LTC_MP_GT || mp_cmp_d(key->q, 1) != LTC_MP_GT) { + err= CRYPT_INVALID_ARG; + goto LBL_ERR; + } + if (key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA) { err = CRYPT_INVALID_PACKET; @@ -83,11 +89,19 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if (type == PK_PRIVATE) { key->type = PK_PRIVATE; if ((err = mp_read_unsigned_bin(key->x, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } + if (mp_cmp_d(key->x, 1) != LTC_MP_GT) { + err= CRYPT_INVALID_ARG; + goto LBL_ERR; + } if ((err = mp_exptmod(key->g, key->x, key->p, key->y)) != CRYPT_OK) { goto LBL_ERR; } } else { key->type = PK_PUBLIC; if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } + if (mp_cmp_d(key->y, 1) != LTC_MP_GT || mp_cmp(key->y, key->p) != LTC_MP_LT) { + err= CRYPT_INVALID_ARG; + goto LBL_ERR; + } } return CRYPT_OK; From 053ba6d6009b63a548ca08a6d5cf9dcace0aea42 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Mon, 11 Sep 2017 23:36:03 +0200 Subject: [PATCH 02/15] introducing dsa_verify_key_ex --- src/headers/tomcrypt_pk.h | 4 +++- src/pk/dsa/dsa_set.c | 22 +++++++--------------- src/pk/dsa/dsa_verify_key.c | 31 +++++++++++++++++++------------ 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 3171efd..8e0b191 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -479,7 +479,9 @@ int dsa_decrypt_key(const unsigned char *in, unsigned long inlen, int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key); int dsa_export(unsigned char *out, unsigned long *outlen, int type, dsa_key *key); int dsa_verify_key(dsa_key *key, int *stat); - +#ifdef LTC_SOURCE +int dsa_verify_key_ex(dsa_key *key, int *stat, int mode); +#endif int dsa_shared_secret(void *private_key, void *base, dsa_key *public_key, unsigned char *out, unsigned long *outlen); diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index d85ae8c..11ad650 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -45,12 +45,6 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, key->qord = mp_unsigned_bin_size(key->q); - /* just a quick, basic test - use dsa_verify_key if you want more */ - if (mp_cmp_d(key->p, 1) != LTC_MP_GT || mp_cmp_d(key->g, 1) != LTC_MP_GT || mp_cmp_d(key->q, 1) != LTC_MP_GT) { - err= CRYPT_INVALID_ARG; - goto LBL_ERR; - } - if (key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA) { err = CRYPT_INVALID_PACKET; @@ -76,7 +70,7 @@ LBL_ERR: */ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key *key) { - int err; + int err, stat = 0; LTC_ARGCHK(key != NULL); LTC_ARGCHK(key->x != NULL); @@ -89,19 +83,17 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if (type == PK_PRIVATE) { key->type = PK_PRIVATE; if ((err = mp_read_unsigned_bin(key->x, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } - if (mp_cmp_d(key->x, 1) != LTC_MP_GT) { - err= CRYPT_INVALID_ARG; - goto LBL_ERR; - } if ((err = mp_exptmod(key->g, key->x, key->p, key->y)) != CRYPT_OK) { goto LBL_ERR; } } else { key->type = PK_PUBLIC; if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } - if (mp_cmp_d(key->y, 1) != LTC_MP_GT || mp_cmp(key->y, key->p) != LTC_MP_LT) { - err= CRYPT_INVALID_ARG; - goto LBL_ERR; - } + } + + if ((err = dsa_verify_key_ex(key, &stat, 0)) != CRYPT_OK) { goto LBL_ERR; } + if (stat == 0) { + err = CRYPT_INVALID_ARG; + goto LBL_ERR; } return CRYPT_OK; diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index d263d4e..c5cdff7 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -22,6 +22,11 @@ @return CRYPT_OK if successful */ int dsa_verify_key(dsa_key *key, int *stat) +{ + return dsa_verify_key_ex(key, stat, 1); /* 1 = full check */ +} + +int dsa_verify_key_ex(dsa_key *key, int *stat, int mode) { void *tmp, *tmp2; int res, err; @@ -32,19 +37,21 @@ int dsa_verify_key(dsa_key *key, int *stat) /* default to an invalid key */ *stat = 0; - /* first make sure key->q and key->p are prime */ - if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { - return err; - } - if (res == 0) { - return CRYPT_OK; - } + if (mode == 1) { + /* first make sure key->q and key->p are prime */ + if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { + return err; + } + if (res == 0) { + return CRYPT_OK; + } - if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { - return err; - } - if (res == 0) { - return CRYPT_OK; + if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { + return err; + } + if (res == 0) { + return CRYPT_OK; + } } /* now make sure that g is not -1, 0 or 1 and

Date: Tue, 12 Sep 2017 00:25:21 +0200 Subject: [PATCH 03/15] re-factor & re-name internal dsa key validation --- src/headers/tomcrypt_pk.h | 2 +- src/pk/dsa/dsa_set.c | 2 +- src/pk/dsa/dsa_verify_key.c | 63 ++++++++++++++++++++++++------------- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 8e0b191..74fc548 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -480,7 +480,7 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key); int dsa_export(unsigned char *out, unsigned long *outlen, int type, dsa_key *key); int dsa_verify_key(dsa_key *key, int *stat); #ifdef LTC_SOURCE -int dsa_verify_key_ex(dsa_key *key, int *stat, int mode); +int dsa_int_validate_key(dsa_key *key, int *stat, int mode); #endif int dsa_shared_secret(void *private_key, void *base, dsa_key *public_key, diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index 11ad650..d6e1ee9 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -90,7 +90,7 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } } - if ((err = dsa_verify_key_ex(key, &stat, 0)) != CRYPT_OK) { goto LBL_ERR; } + if ((err = dsa_int_validate_key(key, &stat, 0)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { err = CRYPT_INVALID_ARG; goto LBL_ERR; diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index c5cdff7..ebdcd79 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -16,17 +16,55 @@ #ifdef LTC_MDSA /** - Verify a DSA key for validity - @param key The key to verify + Validate a DSA key + + Yeah, this function should've been called dsa_validate_key() + in the first place and for compat-reasons we keep it + as it was (for now). + + @param key The key to validate @param stat [out] Result of test, 1==valid, 0==invalid @return CRYPT_OK if successful */ int dsa_verify_key(dsa_key *key, int *stat) { - return dsa_verify_key_ex(key, stat, 1); /* 1 = full check */ + int res, err; + + LTC_ARGCHK(key != NULL); + LTC_ARGCHK(stat != NULL); + + /* default to an invalid key */ + *stat = 0; + + /* first make sure key->q and key->p are prime */ + if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { + return err; + } + if (res == LTC_MP_NO) { + return CRYPT_OK; + } + + if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { + return err; + } + if (res == LTC_MP_NO) { + return CRYPT_OK; + } + + return dsa_int_validate_key(key, stat); /* 1 = full check */ } -int dsa_verify_key_ex(dsa_key *key, int *stat, int mode) +/** + Non-complex part of the validation of a DSA key + + This is the computation-wise 'non-complex' part of the + DSA key validation + + @param key The key to validate + @param stat [out] Result of test, 1==valid, 0==invalid + @return CRYPT_OK if successful +*/ +int dsa_int_validate_key(dsa_key *key, int *stat) { void *tmp, *tmp2; int res, err; @@ -37,23 +75,6 @@ int dsa_verify_key_ex(dsa_key *key, int *stat, int mode) /* default to an invalid key */ *stat = 0; - if (mode == 1) { - /* first make sure key->q and key->p are prime */ - if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { - return err; - } - if (res == 0) { - return CRYPT_OK; - } - - if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { - return err; - } - if (res == 0) { - return CRYPT_OK; - } - } - /* now make sure that g is not -1, 0 or 1 and

g, 0) == LTC_MP_EQ || mp_cmp_d(key->g, 1) == LTC_MP_EQ) { return CRYPT_OK; From aa5b9dafc46f9c1f032e1633d96a7ac141365d47 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Tue, 12 Sep 2017 07:03:21 +0200 Subject: [PATCH 04/15] fix dsa_int_validate_key related compiler warnings --- src/headers/tomcrypt_pk.h | 2 +- src/pk/dsa/dsa_set.c | 3 ++- src/pk/dsa/dsa_verify_key.c | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 74fc548..3a9e2de 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -480,7 +480,7 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key); int dsa_export(unsigned char *out, unsigned long *outlen, int type, dsa_key *key); int dsa_verify_key(dsa_key *key, int *stat); #ifdef LTC_SOURCE -int dsa_int_validate_key(dsa_key *key, int *stat, int mode); +int dsa_int_validate_key(dsa_key *key, int *stat); #endif int dsa_shared_secret(void *private_key, void *base, dsa_key *public_key, diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index d6e1ee9..5cf4f6d 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -90,7 +90,8 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } } - if ((err = dsa_int_validate_key(key, &stat, 0)) != CRYPT_OK) { goto LBL_ERR; } + /* do only a quick validation, without primality testing */ + if ((err = dsa_int_validate_key(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { err = CRYPT_INVALID_ARG; goto LBL_ERR; diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index ebdcd79..3d507fa 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -51,7 +51,7 @@ int dsa_verify_key(dsa_key *key, int *stat) return CRYPT_OK; } - return dsa_int_validate_key(key, stat); /* 1 = full check */ + return dsa_int_validate_key(key, stat); } /** @@ -67,7 +67,7 @@ int dsa_verify_key(dsa_key *key, int *stat) int dsa_int_validate_key(dsa_key *key, int *stat) { void *tmp, *tmp2; - int res, err; + int err; LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); From 5fb4c9f89b25ed143f135e117d1eb78c22ca0e87 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 10:39:51 +0200 Subject: [PATCH 05/15] another approach for dsa_int_validate_* --- src/headers/tomcrypt_pk.h | 4 +- src/pk/dsa/dsa_set.c | 13 +++- src/pk/dsa/dsa_verify_key.c | 135 +++++++++++++++++++++++++----------- 3 files changed, 108 insertions(+), 44 deletions(-) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index 3a9e2de..a33638a 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -480,7 +480,9 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key); int dsa_export(unsigned char *out, unsigned long *outlen, int type, dsa_key *key); int dsa_verify_key(dsa_key *key, int *stat); #ifdef LTC_SOURCE -int dsa_int_validate_key(dsa_key *key, int *stat); +int dsa_int_validate_xy(dsa_key *key, int *stat); +int dsa_int_validate_pqg(dsa_key *key, int *stat); +int dsa_int_validate_primes(dsa_key *key, int *stat); #endif int dsa_shared_secret(void *private_key, void *base, dsa_key *public_key, diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index 5cf4f6d..ff5e006 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -27,7 +27,7 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, const unsigned char *g, unsigned long glen, dsa_key *key) { - int err; + int err, stat; LTC_ARGCHK(p != NULL); LTC_ARGCHK(q != NULL); @@ -50,6 +50,14 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, err = CRYPT_INVALID_PACKET; goto LBL_ERR; } + + /* do only a quick validation, without primality testing */ + if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } + if (stat == 0) { + err = CRYPT_INVALID_ARG; + goto LBL_ERR; + } + return CRYPT_OK; LBL_ERR: @@ -90,8 +98,7 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if ((err = mp_read_unsigned_bin(key->y, (unsigned char *)in, inlen)) != CRYPT_OK) { goto LBL_ERR; } } - /* do only a quick validation, without primality testing */ - if ((err = dsa_int_validate_key(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } + if ((err = dsa_int_validate_xy(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { err = CRYPT_INVALID_ARG; goto LBL_ERR; diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index 3d507fa..c429ce0 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -28,15 +28,81 @@ */ int dsa_verify_key(dsa_key *key, int *stat) { - int res, err; + int err; + err = dsa_int_validate_primes(key, stat); + if (err != CRYPT_OK || *stat == 0) return err; + + err = dsa_int_validate_pqg(key, stat); + if (err != CRYPT_OK || *stat == 0) return err; + + return dsa_int_validate_xy(key, stat); +} + +/** + Non-complex part (no primality testing) of the validation + of DSA params (p, q, g) + + @param key The key to validate + @param stat [out] Result of test, 1==valid, 0==invalid + @return CRYPT_OK if successful +*/ +int dsa_int_validate_pqg(dsa_key *key, int *stat) +{ + void *tmp, *tmp2; + int err; + + *stat = 0; LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); - /* default to an invalid key */ - *stat = 0; + /* now make sure that g is not -1, 0 or 1 and

g, 0) == LTC_MP_EQ || mp_cmp_d(key->g, 1) == LTC_MP_EQ) { + return CRYPT_OK; + } + if ((err = mp_init_multi(&tmp, &tmp2, NULL)) != CRYPT_OK) { return err; } + if ((err = mp_sub_d(key->p, 1, tmp)) != CRYPT_OK) { goto error; } + if (mp_cmp(tmp, key->g) == LTC_MP_EQ || mp_cmp(key->g, key->p) != LTC_MP_LT) { + err = CRYPT_OK; + goto error; + } - /* first make sure key->q and key->p are prime */ + /* now we have to make sure that g^q = 1, and that p-1/q gives 0 remainder */ + if ((err = mp_div(tmp, key->q, tmp, tmp2)) != CRYPT_OK) { goto error; } + if (mp_iszero(tmp2) != LTC_MP_YES) { + err = CRYPT_OK; + goto error; + } + + if ((err = mp_exptmod(key->g, key->q, key->p, tmp)) != CRYPT_OK) { goto error; } + if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { + err = CRYPT_OK; + goto error; + } + + err = CRYPT_OK; + *stat = 1; +error: + mp_clear_multi(tmp, tmp2, NULL); + return err; +} + +/** + Primality testing of DSA params p and q + + @param key The key to validate + @param stat [out] Result of test, 1==valid, 0==invalid + @return CRYPT_OK if successful +*/ +int dsa_int_validate_primes(dsa_key *key, int *stat) +{ + int err, res; + + *stat = 0; + LTC_ARGCHK(key != NULL); + LTC_ARGCHK(stat != NULL); + + /* key->q prime? */ if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { return err; } @@ -44,6 +110,7 @@ int dsa_verify_key(dsa_key *key, int *stat) return CRYPT_OK; } + /* key->p prime? */ if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { return err; } @@ -51,74 +118,62 @@ int dsa_verify_key(dsa_key *key, int *stat) return CRYPT_OK; } - return dsa_int_validate_key(key, stat); + *stat = 1; + return CRYPT_OK; } /** - Non-complex part of the validation of a DSA key - - This is the computation-wise 'non-complex' part of the - DSA key validation + Validation of a DSA key (x and y values) @param key The key to validate @param stat [out] Result of test, 1==valid, 0==invalid @return CRYPT_OK if successful */ -int dsa_int_validate_key(dsa_key *key, int *stat) +int dsa_int_validate_xy(dsa_key *key, int *stat) { - void *tmp, *tmp2; - int err; + void *tmp; + int err; + *stat = 0; LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); - /* default to an invalid key */ - *stat = 0; - - /* now make sure that g is not -1, 0 or 1 and

g, 0) == LTC_MP_EQ || mp_cmp_d(key->g, 1) == LTC_MP_EQ) { - return CRYPT_OK; + /* 1 < y < p-1 */ + if ((err = mp_init(&tmp)) != CRYPT_OK) { + return err; } - if ((err = mp_init_multi(&tmp, &tmp2, NULL)) != CRYPT_OK) { return err; } - if ((err = mp_sub_d(key->p, 1, tmp)) != CRYPT_OK) { goto error; } - if (mp_cmp(tmp, key->g) == LTC_MP_EQ || mp_cmp(key->g, key->p) != LTC_MP_LT) { - err = CRYPT_OK; + if ((err = mp_sub_d(key->p, 1, tmp)) != CRYPT_OK) { goto error; } - - /* 1 < y < p-1 */ if (!(mp_cmp_d(key->y, 1) == LTC_MP_GT && mp_cmp(key->y, tmp) == LTC_MP_LT)) { err = CRYPT_OK; goto error; } - /* now we have to make sure that g^q = 1, and that p-1/q gives 0 remainder */ - if ((err = mp_div(tmp, key->q, tmp, tmp2)) != CRYPT_OK) { goto error; } - if (mp_iszero(tmp2) != LTC_MP_YES) { - err = CRYPT_OK; - goto error; - } - - if ((err = mp_exptmod(key->g, key->q, key->p, tmp)) != CRYPT_OK) { goto error; } - if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { - err = CRYPT_OK; - goto error; - } - /* now we have to make sure that y^q = 1, this makes sure y \in g^x mod p */ - if ((err = mp_exptmod(key->y, key->q, key->p, tmp)) != CRYPT_OK) { goto error; } + if ((err = mp_exptmod(key->y, key->q, key->p, tmp)) != CRYPT_OK) { + goto error; + } if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { err = CRYPT_OK; goto error; } - /* at this point we are out of tests ;-( */ + if (key->type == PK_PRIVATE) { + /* x > 1 */ + if (!(mp_cmp_d(key->x, 1) == LTC_MP_GT)) { + err = CRYPT_OK; + goto error; + } + } + err = CRYPT_OK; *stat = 1; error: - mp_clear_multi(tmp, tmp2, NULL); + mp_clear(tmp); return err; } + #endif /* ref: $Format:%D$ */ From 1ea4fecc818f707b1de21e4f45c4ffa07677b527 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 11:43:59 +0200 Subject: [PATCH 06/15] FIPS 186-4 DSA validity tests --- src/pk/dsa/dsa_verify_key.c | 61 ++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 25 deletions(-) diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index c429ce0..0f84ea0 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -49,33 +49,33 @@ int dsa_verify_key(dsa_key *key, int *stat) */ int dsa_int_validate_pqg(dsa_key *key, int *stat) { - void *tmp, *tmp2; + void *tmp1, *tmp2; int err; *stat = 0; LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); - /* now make sure that g is not -1, 0 or 1 and

g, 0) == LTC_MP_EQ || mp_cmp_d(key->g, 1) == LTC_MP_EQ) { + /* FIPS 186-4 chapter 4.1: 1 < g < p */ + if (mp_cmp_d(key->g, 1) != LTC_MP_GT || mp_cmp(key->g, key->p) != LTC_MP_LT) { return CRYPT_OK; } - if ((err = mp_init_multi(&tmp, &tmp2, NULL)) != CRYPT_OK) { return err; } - if ((err = mp_sub_d(key->p, 1, tmp)) != CRYPT_OK) { goto error; } - if (mp_cmp(tmp, key->g) == LTC_MP_EQ || mp_cmp(key->g, key->p) != LTC_MP_LT) { - err = CRYPT_OK; - goto error; - } - /* now we have to make sure that g^q = 1, and that p-1/q gives 0 remainder */ - if ((err = mp_div(tmp, key->q, tmp, tmp2)) != CRYPT_OK) { goto error; } + if ((err = mp_init_multi(&tmp1, &tmp2, NULL)) != CRYPT_OK) { return err; } + + /* FIPS 186-4 chapter 4.1: q is a divisor of (p - 1) */ + if ((err = mp_sub_d(key->p, 1, tmp1)) != CRYPT_OK) { goto error; } + if ((err = mp_div(tmp1, key->q, tmp1, tmp2)) != CRYPT_OK) { goto error; } if (mp_iszero(tmp2) != LTC_MP_YES) { err = CRYPT_OK; goto error; } - if ((err = mp_exptmod(key->g, key->q, key->p, tmp)) != CRYPT_OK) { goto error; } - if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { + /* FIPS 186-4 chapter 4.1: g is a generator of a subgroup of order q in + * the multiplicative group of GF(p) - so we make sure that g^q mod p = 1 + */ + if ((err = mp_exptmod(key->g, key->q, key->p, tmp1)) != CRYPT_OK) { goto error; } + if (mp_cmp_d(tmp1, 1) != LTC_MP_EQ) { err = CRYPT_OK; goto error; } @@ -83,7 +83,7 @@ int dsa_int_validate_pqg(dsa_key *key, int *stat) err = CRYPT_OK; *stat = 1; error: - mp_clear_multi(tmp, tmp2, NULL); + mp_clear_multi(tmp1, tmp2, NULL); return err; } @@ -150,18 +150,29 @@ int dsa_int_validate_xy(dsa_key *key, int *stat) goto error; } - /* now we have to make sure that y^q = 1, this makes sure y \in g^x mod p */ - if ((err = mp_exptmod(key->y, key->q, key->p, tmp)) != CRYPT_OK) { - goto error; - } - if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { - err = CRYPT_OK; - goto error; - } - if (key->type == PK_PRIVATE) { - /* x > 1 */ - if (!(mp_cmp_d(key->x, 1) == LTC_MP_GT)) { + /* FIPS 186-4 chapter 4.1: 0 < x < q */ + if (mp_cmp_d(key->x, 0) != LTC_MP_GT || mp_cmp(key->x, key->q) != LTC_MP_LT) { + err = CRYPT_OK; + goto error; + } + /* FIPS 186-4 chapter 4.1: y = g^x mod p */ + if ((err = mp_exptmod(key->g, key->x, key->p, tmp)) != CRYPT_OK) { + goto error; + } + if (mp_cmp(tmp, key->y) != LTC_MP_EQ) { + err = CRYPT_OK; + goto error; + } + } + else { + /* with just a public key we cannot test y = g^x mod p therefore we + * only test that y^q mod p = 1, which makes sure y is in g^x mod p + */ + if ((err = mp_exptmod(key->y, key->q, key->p, tmp)) != CRYPT_OK) { + goto error; + } + if (mp_cmp_d(tmp, 1) != LTC_MP_EQ) { err = CRYPT_OK; goto error; } From c806ea17f9704182be0effe8a5777ec415640441 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 12:45:45 +0200 Subject: [PATCH 07/15] fix dsa_int_validate_xy --- src/pk/dsa/dsa_verify_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index 0f84ea0..7cfd117 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -145,7 +145,7 @@ int dsa_int_validate_xy(dsa_key *key, int *stat) if ((err = mp_sub_d(key->p, 1, tmp)) != CRYPT_OK) { goto error; } - if (!(mp_cmp_d(key->y, 1) == LTC_MP_GT && mp_cmp(key->y, tmp) == LTC_MP_LT)) { + if (mp_cmp_d(key->y, 1) != LTC_MP_GT || mp_cmp(key->y, tmp) != LTC_MP_LT) { err = CRYPT_OK; goto error; } From 45b6b947da3dd76df9fde69cf8e751ee3282d3c7 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 17:21:39 +0200 Subject: [PATCH 08/15] dsa_int_validate_primes & LTC_MILLER_RABIN_REPS --- src/pk/dsa/dsa_verify_key.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index 7cfd117..08d0a70 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -103,7 +103,7 @@ int dsa_int_validate_primes(dsa_key *key, int *stat) LTC_ARGCHK(stat != NULL); /* key->q prime? */ - if ((err = mp_prime_is_prime(key->q, 8, &res)) != CRYPT_OK) { + if ((err = mp_prime_is_prime(key->q, LTC_MILLER_RABIN_REPS, &res)) != CRYPT_OK) { return err; } if (res == LTC_MP_NO) { @@ -111,7 +111,7 @@ int dsa_int_validate_primes(dsa_key *key, int *stat) } /* key->p prime? */ - if ((err = mp_prime_is_prime(key->p, 8, &res)) != CRYPT_OK) { + if ((err = mp_prime_is_prime(key->p, LTC_MILLER_RABIN_REPS, &res)) != CRYPT_OK) { return err; } if (res == LTC_MP_NO) { From 9765befd6b6901f4a062b19f9fae9ffcc47f915b Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 17:25:28 +0200 Subject: [PATCH 09/15] do dsa_int_validate_pqg in dsa_set_pqg_dsaparam --- src/pk/dsa/dsa_set_pqg_dsaparam.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/pk/dsa/dsa_set_pqg_dsaparam.c b/src/pk/dsa/dsa_set_pqg_dsaparam.c index 454a941..d4dc397 100644 --- a/src/pk/dsa/dsa_set_pqg_dsaparam.c +++ b/src/pk/dsa/dsa_set_pqg_dsaparam.c @@ -24,7 +24,7 @@ int dsa_set_pqg_dsaparam(const unsigned char *dsaparam, unsigned long dsaparamlen, dsa_key *key) { - int err; + int err, stat; LTC_ARGCHK(dsaparam != NULL); LTC_ARGCHK(key != NULL); @@ -49,6 +49,16 @@ int dsa_set_pqg_dsaparam(const unsigned char *dsaparam, unsigned long dsaparamle err = CRYPT_INVALID_PACKET; goto LBL_ERR; } + + /* quick p, q, g validation, without primality testing */ + if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { + goto LBL_ERR; + } + if (stat == 0) { + err = CRYPT_INVALID_PACKET; + goto LBL_ERR; + } + return CRYPT_OK; LBL_ERR: From fd94e9540f9d694608b750f010c88b795670ee1f Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 17:37:39 +0200 Subject: [PATCH 10/15] move qord trest to dsa_int_validate_pqg --- src/pk/dsa/dsa_set.c | 6 ------ src/pk/dsa/dsa_set_pqg_dsaparam.c | 6 ------ src/pk/dsa/dsa_verify_key.c | 8 ++++++++ 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index ff5e006..cc53fc8 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -45,12 +45,6 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, key->qord = mp_unsigned_bin_size(key->q); - if (key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || - (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA) { - err = CRYPT_INVALID_PACKET; - goto LBL_ERR; - } - /* do only a quick validation, without primality testing */ if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { diff --git a/src/pk/dsa/dsa_set_pqg_dsaparam.c b/src/pk/dsa/dsa_set_pqg_dsaparam.c index d4dc397..edbed1c 100644 --- a/src/pk/dsa/dsa_set_pqg_dsaparam.c +++ b/src/pk/dsa/dsa_set_pqg_dsaparam.c @@ -44,12 +44,6 @@ int dsa_set_pqg_dsaparam(const unsigned char *dsaparam, unsigned long dsaparamle key->qord = mp_unsigned_bin_size(key->q); - if (key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || - (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA) { - err = CRYPT_INVALID_PACKET; - goto LBL_ERR; - } - /* quick p, q, g validation, without primality testing */ if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { goto LBL_ERR; diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index 08d0a70..2737cdd 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -56,6 +56,14 @@ int dsa_int_validate_pqg(dsa_key *key, int *stat) LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); + /* check q-order */ + if ( key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || + (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || + (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA ) { + err = CRYPT_OK; + goto error; + } + /* FIPS 186-4 chapter 4.1: 1 < g < p */ if (mp_cmp_d(key->g, 1) != LTC_MP_GT || mp_cmp(key->g, key->p) != LTC_MP_LT) { return CRYPT_OK; From 444d9f3fb744e4b977b93bca523d64eb2b9e05b8 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 17:38:12 +0200 Subject: [PATCH 11/15] do dsa_int_validate_* in dsa_import --- src/pk/dsa/dsa_import.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/pk/dsa/dsa_import.c b/src/pk/dsa/dsa_import.c index 3934765..08d64b7 100644 --- a/src/pk/dsa/dsa_import.c +++ b/src/pk/dsa/dsa_import.c @@ -24,7 +24,7 @@ */ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) { - int err; + int err, stat; unsigned long zero = 0; unsigned char* tmpbuf = NULL; unsigned char flags[1]; @@ -116,10 +116,21 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key) } LBL_OK: - key->qord = mp_unsigned_bin_size(key->q); + key->qord = mp_unsigned_bin_size(key->q); - if (key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || - (unsigned long)key->qord >= mp_unsigned_bin_size(key->p) || (mp_unsigned_bin_size(key->p) - key->qord) >= LTC_MDSA_DELTA) { + /* quick p, q, g validation, without primality testing */ + if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { + goto LBL_ERR; + } + if (stat == 0) { + err = CRYPT_INVALID_PACKET; + goto LBL_ERR; + } + /* validate x, y */ + if ((err = dsa_int_validate_xy(key, &stat)) != CRYPT_OK) { + goto LBL_ERR; + } + if (stat == 0) { err = CRYPT_INVALID_PACKET; goto LBL_ERR; } From d91d59421f10ec53c3301895d8fb5e8fca19809c Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 18:48:04 +0200 Subject: [PATCH 12/15] fix de-referencing stat before checking for NULL --- src/pk/dsa/dsa_verify_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index 2737cdd..a70da9f 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -52,9 +52,9 @@ int dsa_int_validate_pqg(dsa_key *key, int *stat) void *tmp1, *tmp2; int err; - *stat = 0; LTC_ARGCHK(key != NULL); LTC_ARGCHK(stat != NULL); + *stat = 0; /* check q-order */ if ( key->qord >= LTC_MDSA_MAX_GROUP || key->qord <= 15 || From 6200f301a5ea826be33a8446bf112672569557c1 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 18:49:42 +0200 Subject: [PATCH 13/15] add comment #ifdef LTC_SOURCE + internal helper functions --- src/headers/tomcrypt_pk.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/headers/tomcrypt_pk.h b/src/headers/tomcrypt_pk.h index a33638a..837baa7 100644 --- a/src/headers/tomcrypt_pk.h +++ b/src/headers/tomcrypt_pk.h @@ -480,6 +480,7 @@ int dsa_import(const unsigned char *in, unsigned long inlen, dsa_key *key); int dsa_export(unsigned char *out, unsigned long *outlen, int type, dsa_key *key); int dsa_verify_key(dsa_key *key, int *stat); #ifdef LTC_SOURCE +/* internal helper functions */ int dsa_int_validate_xy(dsa_key *key, int *stat); int dsa_int_validate_pqg(dsa_key *key, int *stat); int dsa_int_validate_primes(dsa_key *key, int *stat); From a990a8252e70aa289573059e65b4fd94ee32a6ed Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 18:51:02 +0200 Subject: [PATCH 14/15] mp_clear_multi - reverse the order --- src/pk/dsa/dsa_verify_key.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pk/dsa/dsa_verify_key.c b/src/pk/dsa/dsa_verify_key.c index a70da9f..e32777d 100644 --- a/src/pk/dsa/dsa_verify_key.c +++ b/src/pk/dsa/dsa_verify_key.c @@ -91,7 +91,7 @@ int dsa_int_validate_pqg(dsa_key *key, int *stat) err = CRYPT_OK; *stat = 1; error: - mp_clear_multi(tmp1, tmp2, NULL); + mp_clear_multi(tmp2, tmp1, NULL); return err; } From bb6a7e1c6c07e323e180797b782f238807e06311 Mon Sep 17 00:00:00 2001 From: Karel Miko Date: Thu, 14 Sep 2017 18:53:09 +0200 Subject: [PATCH 15/15] if dsa_int_validate_* fails return consistently CRYPT_INVALID_PACKET --- src/pk/dsa/dsa_set.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pk/dsa/dsa_set.c b/src/pk/dsa/dsa_set.c index cc53fc8..a4d4042 100644 --- a/src/pk/dsa/dsa_set.c +++ b/src/pk/dsa/dsa_set.c @@ -48,7 +48,7 @@ int dsa_set_pqg(const unsigned char *p, unsigned long plen, /* do only a quick validation, without primality testing */ if ((err = dsa_int_validate_pqg(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { - err = CRYPT_INVALID_ARG; + err = CRYPT_INVALID_PACKET; goto LBL_ERR; } @@ -94,7 +94,7 @@ int dsa_set_key(const unsigned char *in, unsigned long inlen, int type, dsa_key if ((err = dsa_int_validate_xy(key, &stat)) != CRYPT_OK) { goto LBL_ERR; } if (stat == 0) { - err = CRYPT_INVALID_ARG; + err = CRYPT_INVALID_PACKET; goto LBL_ERR; }