add m_mp_free_multi, be more careful freeing when failing to load keys

This commit is contained in:
Matt Johnston 2017-05-26 21:08:43 +08:00
parent 7ab8f61974
commit c38927da47
5 changed files with 41 additions and 54 deletions

View File

@ -68,6 +68,22 @@ void m_mp_alloc_init_multi(mp_int **mp, ...)
va_end(args);
}
void m_mp_free_multi(mp_int **mp, ...)
{
mp_int** cur_arg = mp;
va_list args;
va_start(args, mp); /* init args to next argument from caller */
while (cur_arg != NULL) {
if (*cur_arg) {
mp_clear(*cur_arg);
}
m_free(*cur_arg);
cur_arg = va_arg(args, mp_int**);
}
va_end(args);
}
void bytes_to_mp(mp_int *mp, const unsigned char* bytes, unsigned int len) {
if (mp_read_unsigned_bin(mp, (unsigned char*)bytes, len) != MP_OKAY) {

View File

@ -30,6 +30,7 @@
void m_mp_init(mp_int *mp);
void m_mp_init_multi(mp_int *mp, ...) ATTRIB_SENTINEL;
void m_mp_alloc_init_multi(mp_int **mp, ...) ATTRIB_SENTINEL;
void m_mp_free_multi(mp_int **mp, ...) ATTRIB_SENTINEL;
void bytes_to_mp(mp_int *mp, const unsigned char* bytes, unsigned int len);
void hash_process_mp(const struct ltc_hash_descriptor *hash_desc,
hash_state *hs, mp_int *mp);

37
dss.c
View File

@ -44,6 +44,7 @@
* These should be freed with dss_key_free.
* Returns DROPBEAR_SUCCESS or DROPBEAR_FAILURE */
int buf_get_dss_pub_key(buffer* buf, dropbear_dss_key *key) {
int ret = DROPBEAR_FAILURE;
TRACE(("enter buf_get_dss_pub_key"))
dropbear_assert(key != NULL);
@ -56,17 +57,24 @@ int buf_get_dss_pub_key(buffer* buf, dropbear_dss_key *key) {
|| buf_getmpint(buf, key->g) == DROPBEAR_FAILURE
|| buf_getmpint(buf, key->y) == DROPBEAR_FAILURE) {
TRACE(("leave buf_get_dss_pub_key: failed reading mpints"))
return DROPBEAR_FAILURE;
ret = DROPBEAR_FAILURE;
goto out;
}
if (mp_count_bits(key->p) < MIN_DSS_KEYLEN) {
dropbear_log(LOG_WARNING, "DSS key too short");
TRACE(("leave buf_get_dss_pub_key: short key"))
return DROPBEAR_FAILURE;
ret = DROPBEAR_FAILURE;
goto out;
}
ret = DROPBEAR_SUCCESS;
TRACE(("leave buf_get_dss_pub_key: success"))
return DROPBEAR_SUCCESS;
out:
if (ret == DROPBEAR_FAILURE) {
m_mp_free_multi(&key->p, &key->q, &key->g, &key->y, NULL);
}
return ret;
}
/* Same as buf_get_dss_pub_key, but reads a private "x" key at the end.
@ -86,7 +94,7 @@ int buf_get_dss_priv_key(buffer* buf, dropbear_dss_key *key) {
m_mp_alloc_init_multi(&key->x, NULL);
ret = buf_getmpint(buf, key->x);
if (ret == DROPBEAR_FAILURE) {
m_free(key->x);
m_mp_free_multi(&key->x);
}
return ret;
@ -101,26 +109,7 @@ void dss_key_free(dropbear_dss_key *key) {
TRACE2(("enter dsa_key_free: key == NULL"))
return;
}
if (key->p) {
mp_clear(key->p);
m_free(key->p);
}
if (key->q) {
mp_clear(key->q);
m_free(key->q);
}
if (key->g) {
mp_clear(key->g);
m_free(key->g);
}
if (key->y) {
mp_clear(key->y);
m_free(key->y);
}
if (key->x) {
mp_clear(key->x);
m_free(key->x);
}
m_mp_free_multi(&key->p, &key->q, &key->g, &key->y, &key->x, NULL);
m_free(key);
TRACE2(("leave dsa_key_free"))
}

28
rsa.c
View File

@ -72,8 +72,7 @@ int buf_get_rsa_pub_key(buffer* buf, dropbear_rsa_key *key) {
ret = DROPBEAR_SUCCESS;
out:
if (ret == DROPBEAR_FAILURE) {
m_free(key->e);
m_free(key->n);
m_mp_free_multi(&key->e, &key->n, NULL);
}
return ret;
}
@ -121,9 +120,7 @@ int buf_get_rsa_priv_key(buffer* buf, dropbear_rsa_key *key) {
ret = DROPBEAR_SUCCESS;
out:
if (ret == DROPBEAR_FAILURE) {
m_free(key->d);
m_free(key->p);
m_free(key->q);
m_mp_free_multi(&key->d, &key->p, &key->q, NULL);
}
TRACE(("leave buf_get_rsa_priv_key"))
return ret;
@ -139,26 +136,7 @@ void rsa_key_free(dropbear_rsa_key *key) {
TRACE2(("leave rsa_key_free: key == NULL"))
return;
}
if (key->d) {
mp_clear(key->d);
m_free(key->d);
}
if (key->e) {
mp_clear(key->e);
m_free(key->e);
}
if (key->n) {
mp_clear(key->n);
m_free(key->n);
}
if (key->p) {
mp_clear(key->p);
m_free(key->p);
}
if (key->q) {
mp_clear(key->q);
m_free(key->q);
}
m_mp_free_multi(&key->d, &key->e, &key->p, &key->q, &key->n, NULL);
m_free(key);
TRACE2(("leave rsa_key_free"))
}

View File

@ -167,7 +167,8 @@ int buf_get_pub_key(buffer *buf, sign_key *key, enum signkey_type *type) {
key->dsskey = m_malloc(sizeof(*key->dsskey));
ret = buf_get_dss_pub_key(buf, key->dsskey);
if (ret == DROPBEAR_FAILURE) {
m_free(key->dsskey);
dss_key_free(key->dsskey);
key->dsskey = NULL;
}
}
#endif
@ -177,7 +178,8 @@ int buf_get_pub_key(buffer *buf, sign_key *key, enum signkey_type *type) {
key->rsakey = m_malloc(sizeof(*key->rsakey));
ret = buf_get_rsa_pub_key(buf, key->rsakey);
if (ret == DROPBEAR_FAILURE) {
m_free(key->rsakey);
rsa_key_free(key->rsakey);
key->rsakey = NULL;
}
}
#endif
@ -201,7 +203,6 @@ int buf_get_pub_key(buffer *buf, sign_key *key, enum signkey_type *type) {
TRACE2(("leave buf_get_pub_key"))
return ret;
}
/* returns DROPBEAR_SUCCESS on success, DROPBEAR_FAILURE on fail.
@ -236,7 +237,8 @@ int buf_get_priv_key(buffer *buf, sign_key *key, enum signkey_type *type) {
key->dsskey = m_malloc(sizeof(*key->dsskey));
ret = buf_get_dss_priv_key(buf, key->dsskey);
if (ret == DROPBEAR_FAILURE) {
m_free(key->dsskey);
dss_key_free(key->dsskey);
key->dsskey = NULL;
}
}
#endif
@ -246,7 +248,8 @@ int buf_get_priv_key(buffer *buf, sign_key *key, enum signkey_type *type) {
key->rsakey = m_malloc(sizeof(*key->rsakey));
ret = buf_get_rsa_priv_key(buf, key->rsakey);
if (ret == DROPBEAR_FAILURE) {
m_free(key->rsakey);
rsa_key_free(key->rsakey);
key->rsakey = NULL;
}
}
#endif