You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2021/09/12 13:50:19 UTC
svn commit: r1893278 - in /apr/apr-util/branches/1.7.x: ./
crypto/apr_crypto_openssl.c crypto/apr_crypto_prng.c include/apr_crypto.h
test/testcrypto.c
Author: ylavic
Date: Sun Sep 12 13:50:18 2021
New Revision: 1893278
URL: http://svn.apache.org/viewvc?rev=1893278&view=rev
Log:
Merge r1893199, r1893201, r1893203, r1893270, r1893271 from trunk:
apr_crypto: fixes for thread local storage destructor.
cprng_thread_destroy() should destroy the given cprng first because
this might set cprng_global to NULL.
apr_crypto_prng_init() shouldn't delete cprng_thread_key on failure
unless APR_CRYPTO_PRNG_PER_THREAD is used.
apr_crypto: fix potential memory leaks of cprng_stream_ctx_t.
This can happen in cprng_stream_ctx_make() on error paths, or at thread exit
with APR_CRYPTO_PRNG_PER_THREAD like the below.
Direct leak of 64 byte(s) in 8 object(s) allocated from:
#0 0x7efd954c7628 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x107628)
#1 0x7efd921db6ca (<unknown module>)
#2 0x7efd952937a2 in apr_crypto_prng_create crypto/apr_crypto_prng.c:367
#3 0x7efd95292c1e in apr_crypto_random_thread_bytes crypto/apr_crypto_prng.c:218
#4 0x5611dbbb9440 in thread_func /home/yle/src/apache/apr/trunk.ro/test/testcrypto.c:2597
#5 0x7efd9537dd86 in dummy_worker threadproc/unix/thread.c:148
#6 0x7efd951efea6 in start_thread nptl/pthread_create.c:477
apr_crypto: s/APR_CRYPTO_CIPHER_CHACHA20_CTR/APR_CRYPTO_CIPHER_CHACHA20/g
Chacha is a stream cipher, not a block cipher in counter mode.
apr_crypto: cprng_stream_ctx_make() to return NULL *ctx on failure.
apr_crypto_prng_create() relies on cprng_stream_ctx_make() to not set a
freed/dangling cprng->ctx on failure when given NULL.
apr_crypto_prng: cleanup after ourselves when apr_crypto_prng_init() fails.
Submitted by: ylavic
Modified:
apr/apr-util/branches/1.7.x/ (props changed)
apr/apr-util/branches/1.7.x/crypto/apr_crypto_openssl.c
apr/apr-util/branches/1.7.x/crypto/apr_crypto_prng.c
apr/apr-util/branches/1.7.x/include/apr_crypto.h
apr/apr-util/branches/1.7.x/test/testcrypto.c
Propchange: apr/apr-util/branches/1.7.x/
------------------------------------------------------------------------------
Merged /apr/apr/trunk:r1893199,1893201,1893203,1893270-1893271
Modified: apr/apr-util/branches/1.7.x/crypto/apr_crypto_openssl.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.7.x/crypto/apr_crypto_openssl.c?rev=1893278&r1=1893277&r2=1893278&view=diff
==============================================================================
--- apr/apr-util/branches/1.7.x/crypto/apr_crypto_openssl.c (original)
+++ apr/apr-util/branches/1.7.x/crypto/apr_crypto_openssl.c Sun Sep 12 13:50:18 2021
@@ -106,6 +106,7 @@ struct apr_crypto_digest_t {
struct cprng_stream_ctx_t {
EVP_CIPHER_CTX *ctx;
+ int malloced;
};
static struct apr_crypto_block_key_digest_t key_digests[] =
@@ -1531,6 +1532,16 @@ static apr_status_t crypto_digest(
return status;
}
+static void cprng_stream_ctx_free(cprng_stream_ctx_t *sctx)
+{
+ if (sctx->ctx) {
+ EVP_CIPHER_CTX_free(sctx->ctx);
+ }
+ if (sctx->malloced) {
+ free(sctx);
+ }
+}
+
static apr_status_t cprng_stream_ctx_make(cprng_stream_ctx_t **psctx,
apr_crypto_t *f, apr_crypto_cipher_e cipher, apr_pool_t *pool)
{
@@ -1538,18 +1549,22 @@ static apr_status_t cprng_stream_ctx_mak
EVP_CIPHER_CTX *ctx;
const EVP_CIPHER *ecipher;
+ *psctx = NULL;
+
if (pool) {
- *psctx = sctx = apr_palloc(pool, sizeof(cprng_stream_ctx_t));
+ sctx = apr_palloc(pool, sizeof(cprng_stream_ctx_t));
}
else {
- *psctx = sctx = malloc(sizeof(cprng_stream_ctx_t));
+ sctx = malloc(sizeof(cprng_stream_ctx_t));
}
if (!sctx) {
return APR_ENOMEM;
}
+ sctx->malloced = !pool;
sctx->ctx = ctx = EVP_CIPHER_CTX_new();
if (!ctx) {
+ cprng_stream_ctx_free(sctx);
return APR_ENOMEM;
}
@@ -1567,6 +1582,7 @@ static apr_status_t cprng_stream_ctx_mak
#elif defined(NID_aes_256_ctr)
ecipher = EVP_aes_256_ctr();
#else
+ cprng_stream_ctx_free(sctx);
return APR_ENOCIPHER;
#endif
}
@@ -1575,35 +1591,34 @@ static apr_status_t cprng_stream_ctx_mak
ecipher = EVP_aes_256_ctr();
break;
#else
+ cprng_stream_ctx_free(sctx);
return APR_ENOCIPHER;
#endif
}
- case APR_CRYPTO_CIPHER_CHACHA20_CTR: {
+ case APR_CRYPTO_CIPHER_CHACHA20: {
#if defined(NID_chacha20)
ecipher = EVP_chacha20();
break;
#else
+ cprng_stream_ctx_free(sctx);
return APR_ENOCIPHER;
#endif
}
default: {
+ cprng_stream_ctx_free(sctx);
return APR_ENOCIPHER;
}
}
if (EVP_EncryptInit_ex(ctx, ecipher, f->config->engine, NULL, NULL) <= 0) {
- EVP_CIPHER_CTX_free(ctx);
+ cprng_stream_ctx_free(sctx);
return APR_ENOMEM;
}
+ *psctx = sctx;
return APR_SUCCESS;
}
-static void cprng_stream_ctx_free(cprng_stream_ctx_t *sctx)
-{
- EVP_CIPHER_CTX_free(sctx->ctx);
-}
-
static APR_INLINE
void cprng_stream_setkey(cprng_stream_ctx_t *sctx,
const unsigned char *key,
Modified: apr/apr-util/branches/1.7.x/crypto/apr_crypto_prng.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.7.x/crypto/apr_crypto_prng.c?rev=1893278&r1=1893277&r2=1893278&view=diff
==============================================================================
--- apr/apr-util/branches/1.7.x/crypto/apr_crypto_prng.c (original)
+++ apr/apr-util/branches/1.7.x/crypto/apr_crypto_prng.c Sun Sep 12 13:50:18 2021
@@ -118,11 +118,14 @@ static apr_threadkey_t *cprng_thread_key
static void cprng_thread_destroy(void *cprng)
{
+ apr_threadkey_private_set(NULL, cprng_thread_key);
+ if (cprng) {
+ apr_crypto_prng_destroy(cprng);
+ }
if (!cprng_global) {
apr_threadkey_private_delete(cprng_thread_key);
cprng_thread_key = NULL;
}
- apr_crypto_prng_destroy(cprng);
}
#else /* !APR_HAS_THREADS */
@@ -141,6 +144,12 @@ APU_DECLARE(apr_status_t) apr_crypto_prn
return APR_EREINIT;
}
+ cprng_ring = apr_palloc(pool, sizeof(*cprng_ring));
+ if (!cprng_ring) {
+ return APR_ENOMEM;
+ }
+ APR_RING_INIT(cprng_ring, apr_crypto_prng_t, link);
+
if (flags & APR_CRYPTO_PRNG_PER_THREAD) {
#if !APR_HAS_THREADS
return APR_ENOTIMPL;
@@ -153,18 +162,14 @@ APU_DECLARE(apr_status_t) apr_crypto_prn
#endif
}
- cprng_ring = apr_palloc(pool, sizeof(*cprng_ring));
- if (!cprng_ring) {
- return APR_ENOMEM;
- }
- APR_RING_INIT(cprng_ring, apr_crypto_prng_t, link);
-
#if APR_HAS_THREADS
rv = apr_thread_mutex_create(&cprng_ring_mutex, APR_THREAD_MUTEX_DEFAULT,
pool);
if (rv != APR_SUCCESS) {
- apr_threadkey_private_delete(cprng_thread_key);
- cprng_thread_key = NULL;
+ if (flags & APR_CRYPTO_PRNG_PER_THREAD) {
+ apr_threadkey_private_delete(cprng_thread_key);
+ cprng_thread_key = NULL;
+ }
return rv;
}
@@ -172,8 +177,17 @@ APU_DECLARE(apr_status_t) apr_crypto_prn
flags = (flags | APR_CRYPTO_PRNG_LOCKED) & ~APR_CRYPTO_PRNG_PER_THREAD;
#endif
- return apr_crypto_prng_create(&cprng_global, crypto, cipher, bufsize, flags,
- seed, pool);
+ rv = apr_crypto_prng_create(&cprng_global, crypto, cipher, bufsize, flags,
+ seed, pool);
+ if (rv != APR_SUCCESS) {
+ if (flags & APR_CRYPTO_PRNG_PER_THREAD) {
+ apr_threadkey_private_delete(cprng_thread_key);
+ cprng_thread_key = NULL;
+ }
+ return rv;
+ }
+
+ return APR_SUCCESS;
}
APU_DECLARE(apr_status_t) apr_crypto_prng_term(void)
Modified: apr/apr-util/branches/1.7.x/include/apr_crypto.h
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.7.x/include/apr_crypto.h?rev=1893278&r1=1893277&r2=1893278&view=diff
==============================================================================
--- apr/apr-util/branches/1.7.x/include/apr_crypto.h (original)
+++ apr/apr-util/branches/1.7.x/include/apr_crypto.h Sun Sep 12 13:50:18 2021
@@ -165,7 +165,7 @@ typedef enum
{
APR_CRYPTO_CIPHER_AUTO, /** Choose the recommended cipher / autodetect the cipher */
APR_CRYPTO_CIPHER_AES_256_CTR, /** AES 256 - CTR mode */
- APR_CRYPTO_CIPHER_CHACHA20_CTR, /** ChaCha20 - CTR mode */
+ APR_CRYPTO_CIPHER_CHACHA20, /** ChaCha20 */
} apr_crypto_cipher_e;
/**
Modified: apr/apr-util/branches/1.7.x/test/testcrypto.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.7.x/test/testcrypto.c?rev=1893278&r1=1893277&r2=1893278&view=diff
==============================================================================
--- apr/apr-util/branches/1.7.x/test/testcrypto.c (original)
+++ apr/apr-util/branches/1.7.x/test/testcrypto.c Sun Sep 12 13:50:18 2021
@@ -2514,7 +2514,7 @@ static void test_crypto_prng_aes256(abts
static void test_crypto_prng_chacha20(abts_case *tc, void *data)
{
- return test_crypto_prng(tc, APR_CRYPTO_CIPHER_CHACHA20_CTR, test_PRNG_kat0_chacha20);
+ return test_crypto_prng(tc, APR_CRYPTO_CIPHER_CHACHA20, test_PRNG_kat0_chacha20);
}
#if APR_HAS_FORK