Should return error if buffer too small when get ec pubkey param

[liyi77]

How to reproduce:

This problem was found during debugging https://github.com/openssl/openssl/pull/20781, When setting KEMID to EC curve (such OSSL_HPKE_KEM_ID_P256), the 'short publen test' in test_hkpe.c will be broken. https://github.com/openssl/openssl/blob/42a6a25ba4ddb40333e92e6e2fc57625d9567090/test/hpke_test.c#L1376-L1381

Issue discription

This is because when getting ec pubkey param, a successful value is returned even if the buffer len is small than the pubkey len. https://github.com/openssl/openssl/blob/42a6a25ba4ddb40333e92e6e2fc57625d9567090/crypto/hpke/hpke.c#L1317-L1321 ec_get_params() should return -1 in this case. https://github.com/openssl/openssl/blob/42a6a25ba4ddb40333e92e6e2fc57625d9567090/providers/implementations/keymgmt/ec_kmgmt.c#L738-L757

[liyi77]

emm, things seem a bit complicated. I raised PR about this issue: https://github.com/openssl/openssl/pull/20890, but CI failed.

I checked code and found that the appropriate length will be filled in out_len only when 1 is returned. This is the expected behavior in EVP_PKEY_get1_encoded_public_key():

So ecx_get_params() should be consistent with ec_get_params() and should return 1 instead of 0 in this case?

[t8m]

So ecx_get_params() should be consistent with ec_get_params() and should return 1 instead of 0 in this case?

Is it inconsistent? It is calling OSSL_PARAM_set_octet_string(p, ecx->pubkey, ecx->keylen) but that should behave exactly like that - set the p->return_size if p->data is NULL and return 1.

[t8m]

This is weird.

 p->return_size = EC_POINT_point2oct(ecg, ecp, 
                                         POINT_CONVERSION_UNCOMPRESSED, 
                                         p->data, p->return_size, bnctx);

This call should fail if p->data != NULL and p->return_size is too small for the encoded key to fit.

[liyi77]

So ecx_get_params() should be consistent with ec_get_params() and should return 1 instead of 0 in this case?

Is it inconsistent? It is calling OSSL_PARAM_set_octet_string(p, ecx->pubkey, ecx->keylen) but that should behave exactly like that - set the p->return_size if p->data is NULL and return 1.

It is consistent if 'p->data == NULL'.

It is inconsistent if 'p->data != NULL' and 'p->data_size < pubkey_len'. ec_get_params() will return 1 and set the return_size, OSSL_PARAM_set_octet_string() will return 0 and set the return_size.

[liyi77]

emm, things seem a bit complicated. I raised PR about this issue: #20890, but CI failed.

So ecx_get_params() should be consistent with ec_get_params() and should return 1 instead of 0 in this case?

Oh It is wrong! I found the previous patch also changed the behavior when 'p->data == NULL'.

[liyi77]

https://github.com/openssl/openssl/pull/20890 patch updated.

[t8m]

It is inconsistent if 'p->data != NULL' and 'p->data_size < pubkey_len'. ec_get_params() will return 1 and set the return_size, OSSL_PARAM_set_octet_string() will return 0 and set the return_size.

The problem is I am reading the EC_POINT_point2oct() code and I do not see how this can happen. It should return 0 in such case.

[t8m]

Ah! Now I see what is wrong. The call to EC_POINT_point2oct() is seriously wrong.