From e6083e7e4d5f05795e40db857f1e349378012a56 Mon Sep 17 00:00:00 2001 From: inoguchi <> Date: Mon, 8 Jul 2019 11:56:18 +0000 Subject: Clean up pvkfmt.c - Replace EVP_CIPHER_CTX_init with EVP_CIPHER_CTX_new and handle return value - Replace EVP_CIPHER_CTX_cleanup with EVP_CIPHER_CTX_free - Change two 'return -1;' to 'goto err;' for avoiding leak - Remove the case if enclevel == 0 - Change enclevel checking to make more consistent - Change all goto label to 'err' and insert space before goto label ok and advise from tb@ --- src/lib/libcrypto/pem/pvkfmt.c | 129 +++++++++++++++++++++-------------------- 1 file changed, 66 insertions(+), 63 deletions(-) diff --git a/src/lib/libcrypto/pem/pvkfmt.c b/src/lib/libcrypto/pem/pvkfmt.c index c7b7207964..abb7f7eec9 100644 --- a/src/lib/libcrypto/pem/pvkfmt.c +++ b/src/lib/libcrypto/pem/pvkfmt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pvkfmt.c,v 1.21 2019/07/07 10:52:56 inoguchi Exp $ */ +/* $OpenBSD: pvkfmt.c,v 1.22 2019/07/08 11:56:18 inoguchi Exp $ */ /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL * project 2005. */ @@ -300,7 +300,7 @@ do_b2i_bio(BIO *in, int ispub) else ret = b2i_rsa(&p, length, bitlen, ispub); -err: + err: free(buf); return ret; } @@ -320,27 +320,27 @@ b2i_dss(const unsigned char **in, unsigned int length, unsigned int bitlen, dsa = DSA_new(); ret = EVP_PKEY_new(); if (!dsa || !ret) - goto memerr; + goto err; if (!read_lebn(&p, nbyte, &dsa->p)) - goto memerr; + goto err; if (!read_lebn(&p, 20, &dsa->q)) - goto memerr; + goto err; if (!read_lebn(&p, nbyte, &dsa->g)) - goto memerr; + goto err; if (ispub) { if (!read_lebn(&p, nbyte, &dsa->pub_key)) - goto memerr; + goto err; } else { if (!read_lebn(&p, 20, &dsa->priv_key)) - goto memerr; + goto err; /* Calculate public key */ if (!(dsa->pub_key = BN_new())) - goto memerr; + goto err; if (!(ctx = BN_CTX_new())) - goto memerr; + goto err; if (!BN_mod_exp_ct(dsa->pub_key, dsa->g, dsa->priv_key, dsa->p, ctx)) - goto memerr; + goto err; BN_CTX_free(ctx); } @@ -349,7 +349,7 @@ b2i_dss(const unsigned char **in, unsigned int length, unsigned int bitlen, *in = p; return ret; -memerr: + err: PEMerror(ERR_R_MALLOC_FAILURE); DSA_free(dsa); EVP_PKEY_free(ret); @@ -371,27 +371,27 @@ b2i_rsa(const unsigned char **in, unsigned int length, unsigned int bitlen, rsa = RSA_new(); ret = EVP_PKEY_new(); if (!rsa || !ret) - goto memerr; + goto err; rsa->e = BN_new(); if (!rsa->e) - goto memerr; + goto err; if (!BN_set_word(rsa->e, read_ledword(&p))) - goto memerr; + goto err; if (!read_lebn(&p, nbyte, &rsa->n)) - goto memerr; + goto err; if (!ispub) { if (!read_lebn(&p, hnbyte, &rsa->p)) - goto memerr; + goto err; if (!read_lebn(&p, hnbyte, &rsa->q)) - goto memerr; + goto err; if (!read_lebn(&p, hnbyte, &rsa->dmp1)) - goto memerr; + goto err; if (!read_lebn(&p, hnbyte, &rsa->dmq1)) - goto memerr; + goto err; if (!read_lebn(&p, hnbyte, &rsa->iqmp)) - goto memerr; + goto err; if (!read_lebn(&p, nbyte, &rsa->d)) - goto memerr; + goto err; } EVP_PKEY_set1_RSA(ret, rsa); @@ -399,7 +399,7 @@ b2i_rsa(const unsigned char **in, unsigned int length, unsigned int bitlen, *in = p; return ret; -memerr: + err: PEMerror(ERR_R_MALLOC_FAILURE); RSA_free(rsa); EVP_PKEY_free(ret); @@ -548,20 +548,20 @@ check_bitlen_dsa(DSA *dsa, int ispub, unsigned int *pmagic) bitlen = BN_num_bits(dsa->p); if ((bitlen & 7) || (BN_num_bits(dsa->q) != 160) || (BN_num_bits(dsa->g) > bitlen)) - goto badkey; + goto err; if (ispub) { if (BN_num_bits(dsa->pub_key) > bitlen) - goto badkey; + goto err; *pmagic = MS_DSS1MAGIC; } else { if (BN_num_bits(dsa->priv_key) > 160) - goto badkey; + goto err; *pmagic = MS_DSS2MAGIC; } return bitlen; -badkey: + err: PEMerror(PEM_R_UNSUPPORTED_KEY_COMPONENTS); return 0; } @@ -572,7 +572,7 @@ check_bitlen_rsa(RSA *rsa, int ispub, unsigned int *pmagic) int nbyte, hnbyte, bitlen; if (BN_num_bits(rsa->e) > 32) - goto badkey; + goto err; bitlen = BN_num_bits(rsa->n); nbyte = BN_num_bytes(rsa->n); hnbyte = (BN_num_bits(rsa->n) + 15) >> 4; @@ -585,17 +585,17 @@ check_bitlen_rsa(RSA *rsa, int ispub, unsigned int *pmagic) * hnbyte. */ if (BN_num_bytes(rsa->d) > nbyte) - goto badkey; + goto err; if ((BN_num_bytes(rsa->iqmp) > hnbyte) || (BN_num_bytes(rsa->p) > hnbyte) || (BN_num_bytes(rsa->q) > hnbyte) || (BN_num_bytes(rsa->dmp1) > hnbyte) || (BN_num_bytes(rsa->dmq1) > hnbyte)) - goto badkey; + goto err; } return bitlen; -badkey: + err: PEMerror(PEM_R_UNSUPPORTED_KEY_COMPONENTS); return 0; } @@ -723,9 +723,12 @@ do_PVK_body(const unsigned char **in, unsigned int saltlen, const unsigned char *p = *in; unsigned int magic; unsigned char *enctmp = NULL, *q; - EVP_CIPHER_CTX cctx; + EVP_CIPHER_CTX *cctx = NULL; - EVP_CIPHER_CTX_init(&cctx); + if ((cctx = EVP_CIPHER_CTX_new()) == NULL) { + PEMerror(ERR_R_MALLOC_FAILURE); + goto err; + } if (saltlen) { char psbuf[PEM_BUFSIZE]; unsigned char keybuf[20]; @@ -758,23 +761,23 @@ do_PVK_body(const unsigned char **in, unsigned int saltlen, } inlen = keylen - 8; q = enctmp + 8; - if (!EVP_DecryptInit_ex(&cctx, EVP_rc4(), NULL, keybuf, NULL)) + if (!EVP_DecryptInit_ex(cctx, EVP_rc4(), NULL, keybuf, NULL)) goto err; - if (!EVP_DecryptUpdate(&cctx, q, &enctmplen, p, inlen)) + if (!EVP_DecryptUpdate(cctx, q, &enctmplen, p, inlen)) goto err; - if (!EVP_DecryptFinal_ex(&cctx, q + enctmplen, &enctmplen)) + if (!EVP_DecryptFinal_ex(cctx, q + enctmplen, &enctmplen)) goto err; magic = read_ledword((const unsigned char **)&q); if (magic != MS_RSA2MAGIC && magic != MS_DSS2MAGIC) { q = enctmp + 8; memset(keybuf + 5, 0, 11); - if (!EVP_DecryptInit_ex(&cctx, EVP_rc4(), NULL, keybuf, + if (!EVP_DecryptInit_ex(cctx, EVP_rc4(), NULL, keybuf, NULL)) goto err; explicit_bzero(keybuf, 20); - if (!EVP_DecryptUpdate(&cctx, q, &enctmplen, p, inlen)) + if (!EVP_DecryptUpdate(cctx, q, &enctmplen, p, inlen)) goto err; - if (!EVP_DecryptFinal_ex(&cctx, q + enctmplen, + if (!EVP_DecryptFinal_ex(cctx, q + enctmplen, &enctmplen)) goto err; magic = read_ledword((const unsigned char **)&q); @@ -789,8 +792,8 @@ do_PVK_body(const unsigned char **in, unsigned int saltlen, ret = b2i_PrivateKey(&p, keylen); -err: - EVP_CIPHER_CTX_cleanup(&cctx); + err: + EVP_CIPHER_CTX_free(cctx); if (enctmp && saltlen) free(enctmp); return ret; @@ -827,7 +830,7 @@ b2i_PVK_bio(BIO *in, pem_password_cb *cb, void *u) } ret = do_PVK_body(&p, saltlen, keylen, cb, u); -err: + err: freezero(buf, buflen); return ret; } @@ -838,19 +841,22 @@ i2b_PVK(unsigned char **out, EVP_PKEY*pk, int enclevel, pem_password_cb *cb, { int outlen = 24, pklen; unsigned char *p = NULL, *start = NULL, *salt = NULL; - EVP_CIPHER_CTX cctx; + EVP_CIPHER_CTX *cctx = NULL; - EVP_CIPHER_CTX_init(&cctx); - if (enclevel) + if ((cctx = EVP_CIPHER_CTX_new()) == NULL) { + PEMerror(ERR_R_MALLOC_FAILURE); + goto err; + } + if (enclevel != 0) outlen += PVK_SALTLEN; pklen = do_i2b(NULL, pk, 0); if (pklen < 0) - return -1; + goto err; outlen += pklen; start = p = malloc(outlen); if (!p) { PEMerror(ERR_R_MALLOC_FAILURE); - return -1; + goto err; } write_ledword(&p, MS_PVKMAGIC); @@ -862,16 +868,13 @@ i2b_PVK(unsigned char **out, EVP_PKEY*pk, int enclevel, pem_password_cb *cb, write_ledword(&p, enclevel ? 1 : 0); write_ledword(&p, enclevel ? PVK_SALTLEN : 0); write_ledword(&p, pklen); - if (enclevel) { + if (enclevel != 0) { arc4random_buf(p, PVK_SALTLEN); salt = p; p += PVK_SALTLEN; } do_i2b(&p, pk, 0); - if (enclevel == 0) { - *out = start; - return outlen; - } else { + if (enclevel != 0) { char psbuf[PEM_BUFSIZE]; unsigned char keybuf[20]; int enctmplen, inlen; @@ -881,28 +884,28 @@ i2b_PVK(unsigned char **out, EVP_PKEY*pk, int enclevel, pem_password_cb *cb, inlen = PEM_def_callback(psbuf, PEM_BUFSIZE, 1, u); if (inlen <= 0) { PEMerror(PEM_R_BAD_PASSWORD_READ); - goto error; + goto err; } if (!derive_pvk_key(keybuf, salt, PVK_SALTLEN, (unsigned char *)psbuf, inlen)) - goto error; + goto err; if (enclevel == 1) memset(keybuf + 5, 0, 11); p = salt + PVK_SALTLEN + 8; - if (!EVP_EncryptInit_ex(&cctx, EVP_rc4(), NULL, keybuf, NULL)) - goto error; + if (!EVP_EncryptInit_ex(cctx, EVP_rc4(), NULL, keybuf, NULL)) + goto err; explicit_bzero(keybuf, 20); - if (!EVP_EncryptUpdate(&cctx, p, &enctmplen, p, pklen - 8)) - goto error; - if (!EVP_EncryptFinal_ex(&cctx, p + enctmplen, &enctmplen)) - goto error; + if (!EVP_EncryptUpdate(cctx, p, &enctmplen, p, pklen - 8)) + goto err; + if (!EVP_EncryptFinal_ex(cctx, p + enctmplen, &enctmplen)) + goto err; } - EVP_CIPHER_CTX_cleanup(&cctx); + EVP_CIPHER_CTX_free(cctx); *out = start; return outlen; -error: - EVP_CIPHER_CTX_cleanup(&cctx); + err: + EVP_CIPHER_CTX_free(cctx); free(start); return -1; } -- cgit v1.2.3-55-g6feb