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 <ppzgs1@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27120)

diff --git a/crypto/params.c b/crypto/params.c
index 6f88a8539308657894d7e44c52e518e974998f48..7fb116c63642687db2e465d11aaadf2fa18d6f1d 100644 (file)
--- 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 dae0de083a62de0f5cc0b5484b617d5488e88bdb..8de0e86b7acd9eaba2dde74c4dc1c61426e9c6ca 100644 (file)
--- 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<params>.
 
+OSSL_PARAM_set_octet_string_or_ptr() sets an OCTET string or string pointer
+from the parameter pointed to by I<p> to the value referenced by I<val>. If
+I<p> is an OCTET string and the parameter's I<data> field is NULL, then only
+its I<return_size> field will be assigned the size the parameter's I<data>
+buffer should have. The length of the OCTET string is provided by I<len>.
+
 =head1 RETURN VALUES
 
 OSSL_PARAM_construct_TYPE(), OSSL_PARAM_construct_BN(),
@@ -400,6 +410,8 @@ L<openssl-core.h(7)>, L<OSSL_PARAM(3)>
 
 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 d4b855dffb1fe9480a8d49d3288d9605e9237a97..652ba21b2e9bc7261e8c04184f7dea49717d41ca 100644 (file)
--- 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 76f53e0c7cce81d959df241535640058c8be8232..d6e269b217ecae13af6632af06d24933b55da195 100644 (file)
--- 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 876ff294246c0d52cbf2ff53b4d499e8724cd1ed..5cb67fef67ad71ae22ebe2a1610f176af29e0af9 100644 (file)
--- 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 3b09506447f21e6a75a4edbfc361e61b564c8c73..3ce7de7b3e928a669d51782c788759aaef67d971 100644 (file)
--- 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 80008a81518266edf23888e254f4fb2aae1066fe..d39b6c4084225dd4ca32a2ee9d5fbea778e8e35c 100644 (file)
--- 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 475693f336a4598342de30edd8fe2ee77732d5f6..bd343d5b74f94124a80782131348677af0480484 100644 (file)
--- 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 715c2718bb3269f1f005ce0d0a81004eae1f04f1..20cf4fdf217dadf131faa874968324c47ef48973 100644 (file)
--- 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 0242023c01f17220513421345ec0f5e2d95c481e..ad6f400cb826e8816add422199400e80b70e0678 100644 (file)
--- 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: