Multiple fixes for getting pub key from legacy DH PKEY

There were multiple issues with getting OSSL_PKEY_PARAM_PUB_KEY
from a legacy EVP_PKEY DH and DHX keys.

Fixes #16247

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16253)

2 files changed, 46 insertions, 11 deletions

diff --git a/crypto/evp/ctrl_params_translate.c b/crypto/evp/ctrl_params_translate.c
index a441c1f4b7..88945e13e6 100644
--- a/crypto/evp/ctrl_params_translate.c
+++ b/crypto/evp/ctrl_params_translate.c
@@ -654,9 +654,21 @@ static int default_fixup_args(enum state state,
             } 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 */
-
+                unsigned int param_data_type = translation->param_data_type;
+                size_t size = (size_t)ctx->p1;
+
+                if (state == PKEY)
+                    size = ctx->sz;
+                if (param_data_type == 0) {
+                    /* we must have a fixup_args function to work */
+                    if (!ossl_assert(translation->fixup_args != NULL)) {
+                        ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);
+                        return 0;
+                    }
+                    param_data_type = ctx->params->data_type;
+                }
                 /* When getting, we populate |*params| from |p1| and |p2| */
-                switch (translation->param_data_type) {
+                switch (param_data_type) {
                 case OSSL_PARAM_INTEGER:
                     return OSSL_PARAM_set_int(ctx->params, ctx->p1);
                 case OSSL_PARAM_UNSIGNED_INTEGER:
@@ -673,10 +685,10 @@ static int default_fixup_args(enum state state,
                     return OSSL_PARAM_set_utf8_string(ctx->params, ctx->p2);
                 case OSSL_PARAM_OCTET_STRING:
                     return OSSL_PARAM_set_octet_string(ctx->params, ctx->p2,
-                                                       (size_t)ctx->p1);
+                                                       size);
                 case OSSL_PARAM_OCTET_PTR:
                     return OSSL_PARAM_set_octet_ptr(ctx->params, ctx->p2,
-                                                    (size_t)ctx->p1);
+                                                    size);
                 default:
                     ERR_raise_data(ERR_LIB_EVP, ERR_R_UNSUPPORTED,
                                    "[action:%d, state:%d] "
@@ -1552,6 +1564,7 @@ static int get_payload_public_key(enum state state,
     ctx->p2 = NULL;
     switch (EVP_PKEY_get_base_id(pkey)) {
 #ifndef OPENSSL_NO_DH
+    case EVP_PKEY_DHX:
     case EVP_PKEY_DH:
         switch (ctx->params->data_type) {
         case OSSL_PARAM_OCTET_STRING:
@@ -2249,7 +2262,7 @@ static const struct translation_st evp_pkey_translations[] = {
       get_payload_private_key },
     { GET, -1, -1, -1, 0, NULL, NULL,
       OSSL_PKEY_PARAM_PUB_KEY,
-      0 /* no data type, let get_payload_pub_key() handle that */,
+      0 /* no data type, let get_payload_public_key() handle that */,
       get_payload_public_key },
 
     /* DH and DSA */
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index e03e2a252e..418b467f52 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -2481,13 +2481,21 @@ static int test_EVP_PKEY_set1_DH(void)
     EVP_PKEY *pkey1 = NULL, *pkey2 = NULL;
     int ret = 0;
     BIGNUM *p, *g = NULL;
+    BIGNUM *pubkey = NULL;
+    unsigned char pub[2048 / 8];
+    size_t len = 0;
 
     if (!TEST_ptr(p = BN_new())
             || !TEST_ptr(g = BN_new())
-            || !BN_set_word(p, 9999)
-            || !BN_set_word(g, 2)
+            || !TEST_ptr(pubkey = BN_new())
+            || !TEST_true(BN_set_word(p, 9999))
+            || !TEST_true(BN_set_word(g, 2))
+            || !TEST_true(BN_set_word(pubkey, 4321))
             || !TEST_ptr(noqdh = DH_new())
-            || !DH_set0_pqg(noqdh, p, NULL, g))
+            || !TEST_true(DH_set0_pqg(noqdh, p, NULL, g))
+            || !TEST_true(DH_set0_key(noqdh, pubkey, NULL))
+            || !TEST_ptr(pubkey = BN_new())
+            || !TEST_true(BN_set_word(pubkey, 4321)))
         goto err;
     p = g = NULL;
 
@@ -2497,21 +2505,35 @@ static int test_EVP_PKEY_set1_DH(void)
     if (!TEST_ptr(x942dh)
             || !TEST_ptr(noqdh)
             || !TEST_ptr(pkey1)
-            || !TEST_ptr(pkey2))
+            || !TEST_ptr(pkey2)
+            || !TEST_true(DH_set0_key(x942dh, pubkey, NULL)))
         goto err;
+    pubkey = NULL;
 
-    if(!TEST_true(EVP_PKEY_set1_DH(pkey1, x942dh))
+    if (!TEST_true(EVP_PKEY_set1_DH(pkey1, x942dh))
             || !TEST_int_eq(EVP_PKEY_get_id(pkey1), EVP_PKEY_DHX))
         goto err;
 
-    if(!TEST_true(EVP_PKEY_set1_DH(pkey2, noqdh))
+    if (!TEST_true(EVP_PKEY_get_bn_param(pkey1, OSSL_PKEY_PARAM_PUB_KEY,
+                                         &pubkey))
+            || !TEST_ptr(pubkey))
+        goto err;
+
+    if (!TEST_true(EVP_PKEY_set1_DH(pkey2, noqdh))
             || !TEST_int_eq(EVP_PKEY_get_id(pkey2), EVP_PKEY_DH))
         goto err;
 
+    if (!TEST_true(EVP_PKEY_get_octet_string_param(pkey2,
+                                                   OSSL_PKEY_PARAM_PUB_KEY,
+                                                   pub, sizeof(pub), &len))
+            || !TEST_size_t_ne(len, 0))
+        goto err;
+
     ret = 1;
  err:
     BN_free(p);
     BN_free(g);
+    BN_free(pubkey);
     EVP_PKEY_free(pkey1);
     EVP_PKEY_free(pkey2);
     DH_free(x942dh);