diff options
| author | Nikola Pajkovsky <[email protected]> | 2026-07-15 09:41:06 +0200 |
|---|---|---|
| committer | Norbert Pocs <[email protected]> | 2026-07-20 11:06:53 +0200 |
| commit | a20fa498281912c5a34d6d14d2cfb0538fd836cf (patch) | |
| tree | 4eb86616b1e067be648a42ae9f590939e98e763f | |
| parent | test: include PID in valgrind log file names (diff) | |
| download | openssl-a20fa498281912c5a34d6d14d2cfb0538fd836cf.tar.gz openssl-a20fa498281912c5a34d6d14d2cfb0538fd836cf.zip | |
test/p_ossltest: fix uninitialised bytes written in TLS1-AAD GCM mode
The test provider's fake AES-128-GCM cipher works by running the real
sub-cipher for its side effects and then copying the memdup'd input
back over the output buffer, so the "encrypted" record is really the
plaintext.
In a TLS record the buffer handed to the cipher reserves an
uninitialised 8-byte explicit IV at the front and a 16-byte auth tag
at the end for the cipher to fill. OPENSSL_memdup() duplicated those
uninitialised IV/tag regions, and the subsequent memcpy(out, inbuf, inl)
copied them into out. That output was then written to the socket,
triggering valgrind's "Syscall param write(buf) points to uninitialised
byte(s)":
at sock_write (bss_sock.c:155)
by bwrite_conv (bio_meth.c:79)
by BIO_write (bio_lib.c:397)
by statem_flush (statem.c:963)
by ossl_statem_client_post_work (statem_clnt.c:881)
...
Track whether EVP_CTRL_AEAD_TLS1_AAD was set (recorded in
set_ctx_params) and, when encrypting a TLS record, copy only the
plaintext payload back, leaving the explicit IV and tag that the real
sub-cipher actually produced intact.
Signed-off-by: Nikola Pajkovsky <[email protected]>
Reviewed-by: Tim Hudson <[email protected]>
Reviewed-by: Paul Dale <[email protected]>
Reviewed-by: Neil Horman <[email protected]>
MergeDate: Mon Jul 20 09:07:08 2026
(Merged from https://github.com/openssl/openssl/pull/31961)
| -rw-r--r-- | test/p_ossltest.c | 45 |
1 files changed, 37 insertions, 8 deletions
diff --git a/test/p_ossltest.c b/test/p_ossltest.c index 2f545ebf16d..06bd9bc64c1 100644 --- a/test/p_ossltest.c +++ b/test/p_ossltest.c @@ -695,6 +695,7 @@ static const OSSL_DISPATCH ossl_testaes128_cbc_functions[] = { typedef struct { OSSL_LIB_CTX *libctx; EVP_CIPHER_CTX *sub_ctx; + int tls1_aad; } PROV_EVP_AES128_GCM_CTX; /** @@ -838,10 +839,16 @@ static int ossl_test_aes128gcm_update(void *vprovctx, char *out, size_t *outl, size_t inl) { PROV_EVP_AES128_GCM_CTX *ctx = (PROV_EVP_AES128_GCM_CTX *)vprovctx; - int ret, soutl; - uint8_t *inbuf; + int ret = 0, soutl = 0; + uint8_t *inbuf = NULL; - inbuf = OPENSSL_memdup(in, inl); + *outl = 0; + + if (in != NULL && inl > 0) { + inbuf = OPENSSL_memdup(in, inl); + if (inbuf == NULL) + goto end; + } if (EVP_CIPHER_CTX_is_encrypting(ctx->sub_ctx)) ret = EVP_EncryptUpdate(ctx->sub_ctx, (unsigned char *)out, @@ -849,16 +856,31 @@ static int ossl_test_aes128gcm_update(void *vprovctx, char *out, size_t *outl, else ret = EVP_DecryptUpdate(ctx->sub_ctx, (unsigned char *)out, &soutl, in, (int)inl); - *outl = soutl; /* * Once the cipher is complete, throw it away and use the * plaintext as our output */ - if (inbuf != NULL && out != NULL) - memcpy(out, inbuf, inl); - OPENSSL_free(inbuf); + if (ret > 0 && inbuf != NULL && out != NULL) { + if (ctx->tls1_aad && EVP_CIPHER_CTX_is_encrypting(ctx->sub_ctx)) { + if (inl < EVP_GCM_TLS_EXPLICIT_IV_LEN + EVP_GCM_TLS_TAG_LEN) { + ret = 0; + goto end; + } + memcpy(out + EVP_GCM_TLS_EXPLICIT_IV_LEN, + inbuf + EVP_GCM_TLS_EXPLICIT_IV_LEN, + inl - EVP_GCM_TLS_EXPLICIT_IV_LEN - EVP_GCM_TLS_TAG_LEN); + } else { + memcpy(out, inbuf, inl); + } + } + + *outl = soutl; + +end: + ctx->tls1_aad = 0; + OPENSSL_free(inbuf); return ret; } @@ -955,8 +977,15 @@ static int ossl_test_aes128gcm_get_ctx_params(void *vprovctx, OSSL_PARAM params[ static int ossl_test_aes128gcm_set_ctx_params(void *vprovctx, const OSSL_PARAM params[]) { PROV_EVP_AES128_GCM_CTX *ctx = (PROV_EVP_AES128_GCM_CTX *)vprovctx; + int tls1_aad; + int ret; - return EVP_CIPHER_CTX_set_params(ctx->sub_ctx, params); + tls1_aad = OSSL_PARAM_locate_const(params, OSSL_CIPHER_PARAM_AEAD_TLS1_AAD) != NULL; + ret = EVP_CIPHER_CTX_set_params(ctx->sub_ctx, params); + if (ret) + ctx->tls1_aad = tls1_aad; + + return ret; } /** |
