Fix RSA OAEP set/get label for legacy engine

[ljuzwiuk]

Fixes #21335

The EVP_PKEY_CTX_set0_rsa_oaep_label and EVP_PKEY_CTX_get0_rsa_oaep_label functions have been fixed for usage with the legacy engine. OSSL params are now correctly passed and set in the context.

Checklist

• documentation is added or updated
• tests are added or updated

[t8m]

@ljuzwiuk Would you be please able to sign a CLA? https://www.openssl.org/policies/cla.html

[ljuzwiuk]

@ljuzwiuk Would you be please able to sign a CLA? https://www.openssl.org/policies/cla.html

Already sent my ICLA. I am waiting for CCLA to be approved.

[t8m]

This change should be in a separate commit (can still be in this PR) because it is unrelated.

[t8m]

Instead of this change would it work to instead apply :

--- a/crypto/evp/ctrl_params_translate.c
+++ b/crypto/evp/ctrl_params_translate.c
@@ -2310,7 +2310,7 @@ static const struct translation_st evp_pkey_ctx_translations[] = {
       OSSL_ASYM_CIPHER_PARAM_OAEP_LABEL, OSSL_PARAM_OCTET_STRING, NULL },
     { GET, EVP_PKEY_RSA, 0, EVP_PKEY_OP_TYPE_CRYPT,
       EVP_PKEY_CTRL_GET_RSA_OAEP_LABEL, NULL, NULL,
-      OSSL_ASYM_CIPHER_PARAM_OAEP_LABEL, OSSL_PARAM_OCTET_STRING, NULL },
+      OSSL_ASYM_CIPHER_PARAM_OAEP_LABEL, OSSL_PARAM_OCTET_PTR, NULL },
 
     { SET, EVP_PKEY_RSA, 0, EVP_PKEY_OP_TYPE_CRYPT,
       EVP_PKEY_CTRL_RSA_IMPLICIT_REJECTION, NULL,

[ljuzwiuk]

It does not work. However, it does resolve an error that I had overlooked previously. Good catch.

The overlooked error message: 401745F7FF7F0000:error:07800081:common libcrypto routines:set_string_internal:param of incompatible type:crypto/params.c:1371

[t8m]

This adds a few necessary checks, please try it:

Suggested change

	} else if (state == PRE_PARAMS_TO_CTRL && ctx->action_type == GET) {
	/* Assign output */
	ctx->p2 = ctx->params->data;
	} else if (state == PRE_PARAMS_TO_CTRL && ctx->action_type == GET
	&& translation->param_data_type == OSSL_PARAM_OCTET_PTR) {
	/* Assign output */
	if (!OSSL_PARAM_get_octet_ptr(ctx->params, &ctx->p2, &ctx->sz))
	return 0;

[ljuzwiuk]

After calling OSSL_PARAM_get_octet_ptr, ctx->p2 remains NULL.
To mitigate the potential side effects in other functions, I have refactored the code.

[t8m]

After calling OSSL_PARAM_get_octet_ptr, ctx->p2 remains NULL. To mitigate the potential side effects in other functions, I have refactored the code.

This is weird. I thought it should work. Using special fixup function is not right as there are other parameters that should be handled in the exactly same way and currently aren't working.

[t8m]

Please try this instead:

diff --git a/crypto/evp/ctrl_params_translate.c b/crypto/evp/ctrl_params_translate.c
index 480d48429b..313f7d2b72 100644
--- a/crypto/evp/ctrl_params_translate.c
+++ b/crypto/evp/ctrl_params_translate.c
@@ -647,6 +647,9 @@ static int default_fixup_args(enum state state,
                                    translation->param_data_type);
                     return 0;
                 }
+            } else if (state == PRE_PARAMS_TO_CTRL && ctx->action_type == GET) {
+                if (translation->param_data_type == OSSL_PARAM_OCTET_PTR)
+                    ctx->p2 = &ctx->bufp;
             } else if ((state == POST_PARAMS_TO_CTRL || state == PKEY)
                        && ctx->action_type == GET) {
                 /* For the POST state, only getting needs some work to be done */

[ljuzwiuk]

That works. Now, I need to find a suitable place to add an extra dereference to obtain the correct value of the label. Specifically, I have to place ctx->p2 = *(unsigned char**)ctx->p2 within the POST_PARAMS_TO_CTRL step. However, it does not seem to fit well.

[t8m]

That works. Now, I need to find a suitable place to add an extra dereference to obtain the correct value of the label. Specifically, I have to place ctx->p2 = *(unsigned char**)ctx->p2 within the POST_PARAMS_TO_CTRL step. However, it does not seem to fit well.

You don't. This line handles it https://github.com/openssl/openssl/pull/21401/files#diff-910515c87ac6c81bba3f87a197f149cdcc050330964f3ea26877217df4315e0eL683

[ljuzwiuk]

We need one more dereference, as without it, I am getting random data in the current state.

[t8m]

Actually sorry, the line 686 is the right one and the size handling is not correct there either. What about changing that line 686 to: return OSSL_PARAM_set_octet_ptr(ctx->params, *((void **)ctx->p2), ret);

[ljuzwiuk]

Everything seems to be working correctly. There are no errors, and the returned values are correct. Thanks!

[openssl-machine]

This pull request is ready to merge

[paulidale]

Merged, thanks for the contribution.