From: Nicolas Blais-Miko Date: Sat, 22 Mar 2025 12:14:50 +0000 (-0400) Subject: Fix silent error in EVP_CIPHER_CTX_get_updated_iv. X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a0ff819e537854e7e96d26a0deb56d703006b40f;p=thirdparty%2Fopenssl.git Fix silent error in EVP_CIPHER_CTX_get_updated_iv. Added new params API function OSSL_PARAM_set_octet_string_or_ptr to only call the correct setter for OSSL_CIPHER_PARAM_IV and OSSL_CIPHER_PARAM_UPDATED_IV. Both OSSL_PARAM_set_octet_string and OSSL_PARAM_set_octet_ptr could be called with only one expected to succeed. This would put a silent error on the error stack when calling EVP_CIPHER_CTX_get_updated_iv. Fixes #27117 Reviewed-by: Paul Dale Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/27120) --- diff --git a/crypto/params.c b/crypto/params.c index 6f88a853930..7fb116c6364 100644 --- a/crypto/params.c +++ b/crypto/params.c @@ -1699,3 +1699,20 @@ int OSSL_PARAM_get_octet_string_ptr(const OSSL_PARAM *p, const void **val, return rv || get_string_ptr_internal(p, val, used_len, OSSL_PARAM_OCTET_STRING); } + +int OSSL_PARAM_set_octet_string_or_ptr(OSSL_PARAM *p, const void *val, + size_t len) +{ + if (p == NULL) { + err_null_argument; + return 0; + } + + if (p->data_type == OSSL_PARAM_OCTET_STRING) + return OSSL_PARAM_set_octet_string(p, val, len); + else if (p->data_type == OSSL_PARAM_OCTET_PTR) + return OSSL_PARAM_set_octet_ptr(p, val, len); + + err_bad_type; + return 0; +} diff --git a/doc/man3/OSSL_PARAM_int.pod b/doc/man3/OSSL_PARAM_int.pod index dae0de083a6..8de0e86b7ac 100644 --- a/doc/man3/OSSL_PARAM_int.pod +++ b/doc/man3/OSSL_PARAM_int.pod @@ -31,6 +31,7 @@ OSSL_PARAM_set_time_t, OSSL_PARAM_set_uint, OSSL_PARAM_set_uint32, OSSL_PARAM_set_uint64, OSSL_PARAM_set_ulong, OSSL_PARAM_set_BN, OSSL_PARAM_set_utf8_string, OSSL_PARAM_set_octet_string, OSSL_PARAM_set_utf8_ptr, OSSL_PARAM_set_octet_ptr, +OSSL_PARAM_set_octet_string_or_ptr, OSSL_PARAM_UNMODIFIED, OSSL_PARAM_modified, OSSL_PARAM_set_all_unmodified - OSSL_PARAM helpers @@ -107,6 +108,9 @@ OSSL_PARAM_UNMODIFIED, OSSL_PARAM_modified, OSSL_PARAM_set_all_unmodified int OSSL_PARAM_modified(const OSSL_PARAM *param); void OSSL_PARAM_set_all_unmodified(OSSL_PARAM *params); +int OSSL_PARAM_set_octet_string_or_ptr(OSSL_PARAM *p, const void *val, + size_t len); + =head1 DESCRIPTION A collection of utility functions that simplify and add type safety to the @@ -309,6 +313,12 @@ using the calls defined herein. OSSL_PARAM_set_all_unmodified() resets the unused indicator for all parameters in the array I. +OSSL_PARAM_set_octet_string_or_ptr() sets an OCTET string or string pointer +from the parameter pointed to by I

to the value referenced by I. If +I

is an OCTET string and the parameter's I field is NULL, then only +its I field will be assigned the size the parameter's I +buffer should have. The length of the OCTET string is provided by I. + =head1 RETURN VALUES OSSL_PARAM_construct_TYPE(), OSSL_PARAM_construct_BN(), @@ -400,6 +410,8 @@ L, L These APIs were introduced in OpenSSL 3.0. +OSSL_PARAM_set_octet_string_or_ptr was added in OpenSSL 3.6. + =head1 COPYRIGHT Copyright 2019-2023 The OpenSSL Project Authors. All Rights Reserved. diff --git a/include/openssl/params.h b/include/openssl/params.h index d4b855dffb1..652ba21b2e9 100644 --- a/include/openssl/params.h +++ b/include/openssl/params.h @@ -157,6 +157,9 @@ OSSL_PARAM *OSSL_PARAM_dup(const OSSL_PARAM *p); OSSL_PARAM *OSSL_PARAM_merge(const OSSL_PARAM *p1, const OSSL_PARAM *p2); void OSSL_PARAM_free(OSSL_PARAM *p); +int OSSL_PARAM_set_octet_string_or_ptr(OSSL_PARAM *p, const void *val, + size_t len); + # ifdef __cplusplus } # endif diff --git a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c index 76f53e0c7cc..d6e269b217e 100644 --- a/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c +++ b/providers/implementations/ciphers/cipher_aes_cbc_hmac_sha.c @@ -271,15 +271,13 @@ static int aes_get_ctx_params(void *vctx, OSSL_PARAM params[]) } p = OSSL_PARAM_locate(params, OSSL_CIPHER_PARAM_IV); if (p != NULL - && !OSSL_PARAM_set_octet_string(p, ctx->base.oiv, ctx->base.ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->base.oiv, ctx->base.ivlen)) { + && !OSSL_PARAM_set_octet_string_or_ptr(p, ctx->base.oiv, ctx->base.ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } p = OSSL_PARAM_locate(params, OSSL_CIPHER_PARAM_UPDATED_IV); if (p != NULL - && !OSSL_PARAM_set_octet_string(p, ctx->base.iv, ctx->base.ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->base.iv, ctx->base.ivlen)) { + && !OSSL_PARAM_set_octet_string_or_ptr(p, ctx->base.iv, ctx->base.ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } diff --git a/providers/implementations/ciphers/cipher_aes_ocb.c b/providers/implementations/ciphers/cipher_aes_ocb.c index 876ff294246..5cb67fef67a 100644 --- a/providers/implementations/ciphers/cipher_aes_ocb.c +++ b/providers/implementations/ciphers/cipher_aes_ocb.c @@ -443,8 +443,7 @@ static int aes_ocb_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->base.oiv, ctx->base.ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->base.oiv, ctx->base.ivlen)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->base.oiv, ctx->base.ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } @@ -455,8 +454,7 @@ static int aes_ocb_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->base.iv, ctx->base.ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->base.iv, ctx->base.ivlen)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->base.iv, ctx->base.ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } diff --git a/providers/implementations/ciphers/ciphercommon.c b/providers/implementations/ciphers/ciphercommon.c index 3b09506447f..3ce7de7b3e9 100644 --- a/providers/implementations/ciphers/ciphercommon.c +++ b/providers/implementations/ciphers/ciphercommon.c @@ -619,15 +619,13 @@ int ossl_cipher_generic_get_ctx_params(void *vctx, OSSL_PARAM params[]) } p = OSSL_PARAM_locate(params, OSSL_CIPHER_PARAM_IV); if (p != NULL - && !OSSL_PARAM_set_octet_ptr(p, &ctx->oiv, ctx->ivlen) - && !OSSL_PARAM_set_octet_string(p, &ctx->oiv, ctx->ivlen)) { + && !OSSL_PARAM_set_octet_string_or_ptr(p, ctx->oiv, ctx->ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } p = OSSL_PARAM_locate(params, OSSL_CIPHER_PARAM_UPDATED_IV); if (p != NULL - && !OSSL_PARAM_set_octet_ptr(p, &ctx->iv, ctx->ivlen) - && !OSSL_PARAM_set_octet_string(p, &ctx->iv, ctx->ivlen)) { + && !OSSL_PARAM_set_octet_string_or_ptr(p, ctx->iv, ctx->ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } diff --git a/providers/implementations/ciphers/ciphercommon_ccm.c b/providers/implementations/ciphers/ciphercommon_ccm.c index 80008a81518..d39b6c40842 100644 --- a/providers/implementations/ciphers/ciphercommon_ccm.c +++ b/providers/implementations/ciphers/ciphercommon_ccm.c @@ -171,8 +171,7 @@ int ossl_ccm_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->iv, p->data_size) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->iv, p->data_size)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->iv, p->data_size)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } @@ -184,8 +183,7 @@ int ossl_ccm_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->iv, p->data_size) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->iv, p->data_size)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->iv, p->data_size)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } diff --git a/providers/implementations/ciphers/ciphercommon_gcm.c b/providers/implementations/ciphers/ciphercommon_gcm.c index 475693f336a..bd343d5b74f 100644 --- a/providers/implementations/ciphers/ciphercommon_gcm.c +++ b/providers/implementations/ciphers/ciphercommon_gcm.c @@ -187,8 +187,7 @@ int ossl_gcm_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->iv, ctx->ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->iv, ctx->ivlen)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->iv, ctx->ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } @@ -201,8 +200,7 @@ int ossl_gcm_get_ctx_params(void *vctx, OSSL_PARAM params[]) ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_IV_LENGTH); return 0; } - if (!OSSL_PARAM_set_octet_string(p, ctx->iv, ctx->ivlen) - && !OSSL_PARAM_set_octet_ptr(p, &ctx->iv, ctx->ivlen)) { + if (!OSSL_PARAM_set_octet_string_or_ptr(p, ctx->iv, ctx->ivlen)) { ERR_raise(ERR_LIB_PROV, PROV_R_FAILED_TO_SET_PARAMETER); return 0; } diff --git a/test/params_api_test.c b/test/params_api_test.c index 715c2718bb3..20cf4fdf217 100644 --- a/test/params_api_test.c +++ b/test/params_api_test.c @@ -597,7 +597,7 @@ static int test_param_construct(int tstid) OSSL_PARAM params[20]; char buf[100], buf2[100], *bufp, *bufp2; unsigned char ubuf[100]; - void *vp, *vpn = NULL, *vp2; + void *vp, *vp2, *vpn = NULL, *vpn2 = NULL; OSSL_PARAM *cp; int i, n = 0, ret = 0; unsigned int u; @@ -624,8 +624,10 @@ static int test_param_construct(int tstid) params[n++] = OSSL_PARAM_construct_BN("bignum", ubuf, sizeof(ubuf)); params[n++] = OSSL_PARAM_construct_utf8_string("utf8str", buf, sizeof(buf)); params[n++] = OSSL_PARAM_construct_octet_string("octstr", buf, sizeof(buf)); + params[n++] = OSSL_PARAM_construct_octet_string("octstr2", buf, sizeof(buf)); params[n++] = OSSL_PARAM_construct_utf8_ptr("utf8ptr", &bufp, 0); params[n++] = OSSL_PARAM_construct_octet_ptr("octptr", &vp, 0); + params[n++] = OSSL_PARAM_construct_octet_ptr("octptr2", &vp, 0); params[n] = OSSL_PARAM_construct_end(); switch (tstid) { @@ -722,6 +724,25 @@ static int test_param_construct(int tstid) || !TEST_mem_eq(vp, sizeof("abcdefghi"), "abcdefghi", sizeof("abcdefghi"))) goto err; + /* OCTET string using OSSL_PARAM_set_octet_string_or_ptr */ + if (!TEST_ptr(cp = OSSL_PARAM_locate(p, "octstr2")) + || !TEST_true(OSSL_PARAM_set_octet_string_or_ptr(cp, "jklmnopqr", + sizeof("jklmnopqr"))) + || !TEST_size_t_eq(cp->return_size, sizeof("jklmnopqr"))) + goto err; + /* Match the return size to avoid trailing garbage bytes */ + cp->data_size = cp->return_size; + if (!TEST_true(OSSL_PARAM_get_octet_string(cp, &vpn2, 0, &s)) + || !TEST_size_t_eq(s, sizeof("jklmnopqr")) + || !TEST_mem_eq(vpn2, sizeof("jklmnopqr"), + "jklmnopqr", sizeof("jklmnopqr"))) + goto err; + vp = buf2; + if (!TEST_true(OSSL_PARAM_get_octet_string(cp, &vp, sizeof(buf2), &s)) + || !TEST_size_t_eq(s, sizeof("jklmnopqr")) + || !TEST_mem_eq(vp, sizeof("jklmnopqr"), + "jklmnopqr", sizeof("jklmnopqr"))) + goto err; /* OCTET pointer */ vp = &l; if (!TEST_ptr(cp = OSSL_PARAM_locate(p, "octptr")) @@ -731,6 +752,21 @@ static int test_param_construct(int tstid) goto err; /* Match the return size to avoid trailing garbage bytes */ cp->data_size = cp->return_size; + if (!TEST_true(OSSL_PARAM_get_octet_ptr(cp, (const void **)&vp2, &k)) + || !TEST_size_t_eq(k, sizeof(ul)) + || (tstid <= 1 && !TEST_ptr_eq(vp2, vp))) + goto err; + /* OCTET pointer using OSSL_PARAM_set_octet_string_or_ptr */ + vp = &l; + vp2 = NULL; + k = 0; + if (!TEST_ptr(cp = OSSL_PARAM_locate(p, "octptr2")) + || !TEST_true(OSSL_PARAM_set_octet_string_or_ptr(cp, &ul, sizeof(ul))) + || !TEST_size_t_eq(cp->return_size, sizeof(ul)) + || (tstid <= 1 && !TEST_ptr_eq(vp, &ul))) + goto err; + /* Match the return size to avoid trailing garbage bytes */ + cp->data_size = cp->return_size; if (!TEST_true(OSSL_PARAM_get_octet_ptr(cp, (const void **)&vp2, &k)) || !TEST_size_t_eq(k, sizeof(ul)) || (tstid <= 1 && !TEST_ptr_eq(vp2, vp))) @@ -752,6 +788,7 @@ err: OPENSSL_free(p); OPENSSL_free(p1); OPENSSL_free(vpn); + OPENSSL_free(vpn2); BN_free(bn); BN_free(bn2); return ret; diff --git a/util/libcrypto.num b/util/libcrypto.num index 0242023c01f..ad6f400cb82 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -5926,3 +5926,4 @@ OSSL_AA_DIST_POINT_it ? 3_5_0 EXIST::FUNCTION: PEM_ASN1_write_bio_ctx ? 3_5_0 EXIST::FUNCTION: OPENSSL_sk_set_thunks ? 3_6_0 EXIST::FUNCTION: i2d_PKCS8PrivateKey ? 3_6_0 EXIST::FUNCTION: +OSSL_PARAM_set_octet_string_or_ptr ? 3_6_0 EXIST::FUNCTION: