diff options
author | Richard Levitte <levitte@openssl.org> | 2022-09-29 13:57:34 +0200 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2022-10-05 14:02:03 +0200 |
commit | e077455e9e57ed4ee4676996b4a9aa11df6327a6 (patch) | |
tree | edcb7412024f95fbc97c2c7a780f78ad05d586e3 /crypto/pem | |
parent | 9167a47f78159b0578bc032401ab1d66e14eecdb (diff) |
Stop raising ERR_R_MALLOC_FAILURE in most places
Since OPENSSL_malloc() and friends report ERR_R_MALLOC_FAILURE, and
at least handle the file name and line number they are called from,
there's no need to report ERR_R_MALLOC_FAILURE where they are called
directly, or when SSLfatal() and RLAYERfatal() is used, the reason
`ERR_R_MALLOC_FAILURE` is changed to `ERR_R_CRYPTO_LIB`.
There were a number of places where `ERR_R_MALLOC_FAILURE` was reported
even though it was a function from a different sub-system that was
called. Those places are changed to report ERR_R_{lib}_LIB, where
{lib} is the name of that sub-system.
Some of them are tricky to get right, as we have a lot of functions
that belong in the ASN1 sub-system, and all the `sk_` calls or from
the CRYPTO sub-system.
Some extra adaptation was necessary where there were custom OPENSSL_malloc()
wrappers, and some bugs are fixed alongside these changes.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19301)
Diffstat (limited to 'crypto/pem')
-rw-r--r-- | crypto/pem/pem_info.c | 2 | ||||
-rw-r--r-- | crypto/pem/pem_lib.c | 105 | ||||
-rw-r--r-- | crypto/pem/pem_sign.c | 4 | ||||
-rw-r--r-- | crypto/pem/pvkfmt.c | 92 |
4 files changed, 107 insertions, 96 deletions
diff --git a/crypto/pem/pem_info.c b/crypto/pem/pem_info.c index 061c9b9f68..f8dc4416e2 100644 --- a/crypto/pem/pem_info.c +++ b/crypto/pem/pem_info.c @@ -67,7 +67,7 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk, if (sk == NULL) { if ((ret = sk_X509_INFO_new_null()) == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_PEM, ERR_R_CRYPTO_LIB); goto err; } } else diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c index 98050f8348..4c6b94da6b 100644 --- a/crypto/pem/pem_lib.c +++ b/crypto/pem/pem_lib.c @@ -218,18 +218,25 @@ static int check_pem(const char *nm, const char *name) return 0; } -static void pem_free(void *p, unsigned int flags, size_t num) +#define PEM_FREE(p, flags, num) \ + pem_free((p), (flags), (num), OPENSSL_FILE, OPENSSL_LINE) +static void pem_free(void *p, unsigned int flags, size_t num, + const char *file, int line) { if (flags & PEM_FLAG_SECURE) - OPENSSL_secure_clear_free(p, num); + CRYPTO_secure_clear_free(p, num, file, line); else - OPENSSL_free(p); + CRYPTO_free(p, file, line); } -static void *pem_malloc(int num, unsigned int flags) +#define PEM_MALLOC(num, flags) \ + pem_malloc((num), (flags), OPENSSL_FILE, OPENSSL_LINE) +static void *pem_malloc(int num, unsigned int flags, + const char *file, int line) { - return (flags & PEM_FLAG_SECURE) ? OPENSSL_secure_malloc(num) - : OPENSSL_malloc(num); + return (flags & PEM_FLAG_SECURE) ? CRYPTO_secure_malloc(num, file, line) + : CRYPTO_malloc(num, file, line); + } static int pem_bytes_read_bio_flags(unsigned char **pdata, long *plen, @@ -244,9 +251,9 @@ static int pem_bytes_read_bio_flags(unsigned char **pdata, long *plen, int ret = 0; do { - pem_free(nm, flags, 0); - pem_free(header, flags, 0); - pem_free(data, flags, len); + PEM_FREE(nm, flags, 0); + PEM_FREE(header, flags, 0); + PEM_FREE(data, flags, len); if (!PEM_read_bio_ex(bp, &nm, &header, &data, &len, flags)) { if (ERR_GET_REASON(ERR_peek_error()) == PEM_R_NO_START_LINE) ERR_add_error_data(2, "Expecting: ", name); @@ -268,10 +275,10 @@ static int pem_bytes_read_bio_flags(unsigned char **pdata, long *plen, err: if (!ret || pnm == NULL) - pem_free(nm, flags, 0); - pem_free(header, flags, 0); + PEM_FREE(nm, flags, 0); + PEM_FREE(header, flags, 0); if (!ret) - pem_free(data, flags, len); + PEM_FREE(data, flags, len); return ret; } @@ -345,10 +352,8 @@ int PEM_ASN1_write_bio(i2d_of_void *i2d, const char *name, BIO *bp, /* dsize + 8 bytes are needed */ /* actually it needs the cipher block size extra... */ data = OPENSSL_malloc((unsigned int)dsize + 20); - if (data == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (data == NULL) goto err; - } p = data; i = i2d(x, &p); @@ -608,11 +613,11 @@ int PEM_write_bio(BIO *bp, const char *name, const char *header, int nlen, n, i, j, outl; unsigned char *buf = NULL; EVP_ENCODE_CTX *ctx = EVP_ENCODE_CTX_new(); - int reason = ERR_R_BUF_LIB; + int reason = 0; int retval = 0; if (ctx == NULL) { - reason = ERR_R_MALLOC_FAILURE; + reason = ERR_R_EVP_LIB; goto err; } @@ -621,43 +626,53 @@ int PEM_write_bio(BIO *bp, const char *name, const char *header, if ((BIO_write(bp, "-----BEGIN ", 11) != 11) || (BIO_write(bp, name, nlen) != nlen) || - (BIO_write(bp, "-----\n", 6) != 6)) + (BIO_write(bp, "-----\n", 6) != 6)) { + reason = ERR_R_BIO_LIB; goto err; + } i = header != NULL ? strlen(header) : 0; if (i > 0) { - if ((BIO_write(bp, header, i) != i) || (BIO_write(bp, "\n", 1) != 1)) + if ((BIO_write(bp, header, i) != i) || (BIO_write(bp, "\n", 1) != 1)) { + reason = ERR_R_BIO_LIB; goto err; + } } buf = OPENSSL_malloc(PEM_BUFSIZE * 8); - if (buf == NULL) { - reason = ERR_R_MALLOC_FAILURE; + if (buf == NULL) goto err; - } i = j = 0; while (len > 0) { n = (int)((len > (PEM_BUFSIZE * 5)) ? (PEM_BUFSIZE * 5) : len); - if (!EVP_EncodeUpdate(ctx, buf, &outl, &(data[j]), n)) + if (!EVP_EncodeUpdate(ctx, buf, &outl, &(data[j]), n)) { + reason = ERR_R_EVP_LIB; goto err; - if ((outl) && (BIO_write(bp, (char *)buf, outl) != outl)) + } + if ((outl) && (BIO_write(bp, (char *)buf, outl) != outl)) { + reason = ERR_R_BIO_LIB; goto err; + } i += outl; len -= n; j += n; } EVP_EncodeFinal(ctx, buf, &outl); - if ((outl > 0) && (BIO_write(bp, (char *)buf, outl) != outl)) + if ((outl > 0) && (BIO_write(bp, (char *)buf, outl) != outl)) { + reason = ERR_R_BIO_LIB; goto err; + } if ((BIO_write(bp, "-----END ", 9) != 9) || (BIO_write(bp, name, nlen) != nlen) || - (BIO_write(bp, "-----\n", 6) != 6)) + (BIO_write(bp, "-----\n", 6) != 6)) { + reason = ERR_R_BIO_LIB; goto err; + } retval = i + outl; err: - if (retval == 0) + if (retval == 0 && reason != 0) ERR_raise(ERR_LIB_PEM, reason); EVP_ENCODE_CTX_free(ctx); OPENSSL_clear_free(buf, PEM_BUFSIZE * 8); @@ -747,11 +762,9 @@ static int get_name(BIO *bp, char **name, unsigned int flags) * Need to hold trailing NUL (accounted for by BIO_gets() and the newline * that will be added by sanitize_line() (the extra '1'). */ - linebuf = pem_malloc(LINESIZE + 1, flags); - if (linebuf == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + linebuf = PEM_MALLOC(LINESIZE + 1, flags); + if (linebuf == NULL) return 0; - } do { len = BIO_gets(bp, linebuf, LINESIZE); @@ -771,16 +784,14 @@ static int get_name(BIO *bp, char **name, unsigned int flags) || !HAS_PREFIX(linebuf + len - TAILLEN, TAILSTR)); linebuf[len - TAILLEN] = '\0'; len = len - BEGINLEN - TAILLEN + 1; - *name = pem_malloc(len, flags); - if (*name == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + *name = PEM_MALLOC(len, flags); + if (*name == NULL) goto err; - } memcpy(*name, linebuf + BEGINLEN, len); ret = 1; err: - pem_free(linebuf, flags, LINESIZE + 1); + PEM_FREE(linebuf, flags, LINESIZE + 1); return ret; } @@ -815,11 +826,9 @@ static int get_header_and_data(BIO *bp, BIO **header, BIO **data, char *name, /* Need to hold trailing NUL (accounted for by BIO_gets() and the newline * that will be added by sanitize_line() (the extra '1'). */ - linebuf = pem_malloc(LINESIZE + 1, flags); - if (linebuf == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + linebuf = PEM_MALLOC(LINESIZE + 1, flags); + if (linebuf == NULL) return 0; - } for (line = 0; ; line++) { flags_mask = ~0u; @@ -902,7 +911,7 @@ static int get_header_and_data(BIO *bp, BIO **header, BIO **data, char *name, ret = 1; err: - pem_free(linebuf, flags, LINESIZE + 1); + PEM_FREE(linebuf, flags, LINESIZE + 1); return ret; } @@ -935,7 +944,7 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, headerB = BIO_new(bmeth); dataB = BIO_new(bmeth); if (headerB == NULL || dataB == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_PEM, ERR_R_BIO_LIB); goto end; } @@ -953,7 +962,7 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, ctx = EVP_ENCODE_CTX_new(); if (ctx == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_PEM, ERR_R_EVP_LIB); goto end; } @@ -969,8 +978,8 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, buf_mem->length = len; headerlen = BIO_get_mem_data(headerB, NULL); - *header = pem_malloc(headerlen + 1, flags); - *data = pem_malloc(len, flags); + *header = PEM_MALLOC(headerlen + 1, flags); + *data = PEM_MALLOC(len, flags); if (*header == NULL || *data == NULL) goto out_free; if (headerlen != 0 && BIO_read(headerB, *header, headerlen) != headerlen) @@ -985,11 +994,11 @@ int PEM_read_bio_ex(BIO *bp, char **name_out, char **header, goto end; out_free: - pem_free(*header, flags, 0); - pem_free(*data, flags, 0); + PEM_FREE(*header, flags, 0); + PEM_FREE(*data, flags, 0); end: EVP_ENCODE_CTX_free(ctx); - pem_free(name, flags, 0); + PEM_FREE(name, flags, 0); BIO_free(headerB); BIO_free(dataB); return ret; diff --git a/crypto/pem/pem_sign.c b/crypto/pem/pem_sign.c index 6ad8e43037..f6b0ff4dda 100644 --- a/crypto/pem/pem_sign.c +++ b/crypto/pem/pem_sign.c @@ -33,10 +33,8 @@ int PEM_SignFinal(EVP_MD_CTX *ctx, unsigned char *sigret, unsigned int m_len; m = OPENSSL_malloc(EVP_PKEY_get_size(pkey)); - if (m == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (m == NULL) goto err; - } if (EVP_SignFinal(ctx, m, &m_len, pkey) <= 0) goto err; diff --git a/crypto/pem/pvkfmt.c b/crypto/pem/pvkfmt.c index 83166b5887..8931386fae 100644 --- a/crypto/pem/pvkfmt.c +++ b/crypto/pem/pvkfmt.c @@ -90,6 +90,7 @@ static EVP_PKEY *evp_pkey_new0_key(void *key, int evp_type) case EVP_PKEY_RSA: if (EVP_PKEY_set1_RSA(pkey, key)) break; + ERR_raise(ERR_LIB_PEM, ERR_R_EVP_LIB); EVP_PKEY_free(pkey); pkey = NULL; break; @@ -97,11 +98,14 @@ static EVP_PKEY *evp_pkey_new0_key(void *key, int evp_type) case EVP_PKEY_DSA: if (EVP_PKEY_set1_DSA(pkey, key)) break; + ERR_raise(ERR_LIB_PEM, ERR_R_EVP_LIB); EVP_PKEY_free(pkey); pkey = NULL; break; #endif } + } else { + ERR_raise(ERR_LIB_PEM, ERR_R_EVP_LIB); } switch (evp_type) { @@ -115,8 +119,6 @@ static EVP_PKEY *evp_pkey_new0_key(void *key, int evp_type) #endif } - if (pkey == NULL) - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); return pkey; } @@ -343,10 +345,8 @@ EVP_PKEY *ossl_b2i_bio(BIO *in, int *ispub) return NULL; } buf = OPENSSL_malloc(length); - if (buf == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (buf == NULL) goto err; - } p = buf; if (BIO_read(in, buf, length) != (int)length) { ERR_raise(ERR_LIB_PEM, PEM_R_KEYBLOB_TOO_SHORT); @@ -384,22 +384,22 @@ DSA *ossl_b2i_DSA_after_header(const unsigned char **in, unsigned int bitlen, dsa = DSA_new(); if (dsa == NULL) - goto memerr; + goto dsaerr; if (!read_lebn(&p, nbyte, &pbn)) - goto memerr; + goto bnerr; if (!read_lebn(&p, 20, &qbn)) - goto memerr; + goto bnerr; if (!read_lebn(&p, nbyte, &gbn)) - goto memerr; + goto bnerr; if (ispub) { if (!read_lebn(&p, nbyte, &pub_key)) - goto memerr; + goto bnerr; } else { if (!read_lebn(&p, 20, &priv_key)) - goto memerr; + goto bnerr; /* Set constant time flag before public key calculation */ BN_set_flags(priv_key, BN_FLG_CONSTTIME); @@ -407,28 +407,33 @@ DSA *ossl_b2i_DSA_after_header(const unsigned char **in, unsigned int bitlen, /* Calculate public key */ pub_key = BN_new(); if (pub_key == NULL) - goto memerr; + goto bnerr; if ((ctx = BN_CTX_new()) == NULL) - goto memerr; + goto bnerr; if (!BN_mod_exp(pub_key, gbn, priv_key, pbn, ctx)) - goto memerr; + goto bnerr; BN_CTX_free(ctx); ctx = NULL; } if (!DSA_set0_pqg(dsa, pbn, qbn, gbn)) - goto memerr; + goto dsaerr; pbn = qbn = gbn = NULL; if (!DSA_set0_key(dsa, pub_key, priv_key)) - goto memerr; + goto dsaerr; pub_key = priv_key = NULL; *in = p; return dsa; - memerr: - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + dsaerr: + ERR_raise(ERR_LIB_PEM, ERR_R_DSA_LIB); + goto err; + bnerr: + ERR_raise(ERR_LIB_PEM, ERR_R_BN_LIB); + + err: DSA_free(dsa); BN_free(pbn); BN_free(qbn); @@ -452,42 +457,48 @@ RSA *ossl_b2i_RSA_after_header(const unsigned char **in, unsigned int bitlen, rsa = RSA_new(); if (rsa == NULL) - goto memerr; + goto rsaerr; e = BN_new(); if (e == NULL) - goto memerr; + goto bnerr; if (!BN_set_word(e, read_ledword(&pin))) - goto memerr; + goto bnerr; if (!read_lebn(&pin, nbyte, &n)) - goto memerr; + goto bnerr; if (!ispub) { if (!read_lebn(&pin, hnbyte, &p)) - goto memerr; + goto bnerr; if (!read_lebn(&pin, hnbyte, &q)) - goto memerr; + goto bnerr; if (!read_lebn(&pin, hnbyte, &dmp1)) - goto memerr; + goto bnerr; if (!read_lebn(&pin, hnbyte, &dmq1)) - goto memerr; + goto bnerr; if (!read_lebn(&pin, hnbyte, &iqmp)) - goto memerr; + goto bnerr; if (!read_lebn(&pin, nbyte, &d)) - goto memerr; + goto bnerr; if (!RSA_set0_factors(rsa, p, q)) - goto memerr; + goto rsaerr; p = q = NULL; if (!RSA_set0_crt_params(rsa, dmp1, dmq1, iqmp)) - goto memerr; + goto rsaerr; dmp1 = dmq1 = iqmp = NULL; } if (!RSA_set0_key(rsa, n, e, d)) - goto memerr; + goto rsaerr; n = e = d = NULL; *in = pin; return rsa; - memerr: - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + + rsaerr: + ERR_raise(ERR_LIB_PEM, ERR_R_RSA_LIB); + goto err; + bnerr: + ERR_raise(ERR_LIB_PEM, ERR_R_BN_LIB); + + err: BN_free(e); BN_free(n); BN_free(p); @@ -579,7 +590,6 @@ static int do_i2b(unsigned char **out, const EVP_PKEY *pk, int ispub) p = *out; else { if ((p = OPENSSL_malloc(outlen)) == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); outlen = -1; goto end; } @@ -840,7 +850,7 @@ static void *do_PVK_body_key(const unsigned char **in, EVP_CIPHER_CTX *cctx = EVP_CIPHER_CTX_new(); if (cctx == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + ERR_raise(ERR_LIB_PEM, ERR_R_EVP_LIB); goto err; } @@ -860,10 +870,8 @@ static void *do_PVK_body_key(const unsigned char **in, goto err; } enctmp = OPENSSL_malloc(keylen + 8); - if (enctmp == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (enctmp == NULL) goto err; - } if (!derive_pvk_key(keybuf, sizeof(keybuf), p, saltlen, (unsigned char *)psbuf, inlen, libctx, propq)) goto err; @@ -941,10 +949,8 @@ static void *do_PVK_key_bio(BIO *in, pem_password_cb *cb, void *u, return 0; buflen = (int)keylen + saltlen; buf = OPENSSL_malloc(buflen); - if (buf == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (buf == NULL) return 0; - } p = buf; if (BIO_read(in, buf, buflen) != buflen) { ERR_raise(ERR_LIB_PEM, PEM_R_PVK_DATA_TOO_SHORT); @@ -1027,10 +1033,8 @@ static int i2b_PVK(unsigned char **out, const EVP_PKEY *pk, int enclevel, p = *out; } else { start = p = OPENSSL_malloc(outlen); - if (p == NULL) { - ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE); + if (p == NULL) return -1; - } } cctx = EVP_CIPHER_CTX_new(); |