implement more tests with EdDSA keys (export and comparison)

[Jakuje]

Fixes #272.

Now, it fails though. If I read it right, the OpenSSL returns OSSL_STORE_INFO_PARAMS type from the STORE API when I read the public PEM file exported from the provider, while I do not see anything missing in there so I will just drop it here and hope to get back to investigate the issue later.

[simo5]

Perhaps this needs #282 fixed first?

[sahanaprasad07]

I know what to do for #282 . It will be fixed in the next few days.

[Jakuje]

Perhaps this needs #282 fixed first?

yeah, sounds like the EdDSA keys have the same issue when used the code from your genkey_test_more branch.

But the Ed keys will likely have some other issue too.

I know what to do for 282 . It will be fixed in the next few days.

Thanks!

[Jakuje]

The #282 partially resolves the issue, but it looks like we still need to fix the encoders to work correctly with Edwards curves.

[Jakuje]

Resolved conflict now but the basic test still fails to import the exported Ed25519 key (also the newly generated ed keys by openssl only). The encoders implemented in the providers are now only for the RSA and ECDSA, but implementing one in the provided did not help (but I am not aware of any gotchas that would be needed for the EdDSA keys so the default ones should export them correctly).

Filled openssl/openssl#22246

[simo5]

@Jakuje I have not looked at this in deep but why do you need a separate function (p11prov_obj_get_ed_pub_key) for Edwards curves?
So far we handled classic EC curves and Edward curves with the same code, so what's missing in p11prov_obj_get_ec_public_x_y() that we'd have to do?

[Jakuje]

@Jakuje I have not looked at this in deep but why do you need a separate function (p11prov_obj_get_ed_pub_key) for Edwards curves? So far we handled classic EC curves and Edward curves with the same code, so what's missing in p11prov_obj_get_ec_public_x_y() that we'd have to do?

I though that overloading the ED (which has just one value on outside) with X and Y separate arguments would be more confusing than not. The Ed keys are much simpler than the whole bunch of parameters and separate X/Y values as in the generic ECDSA.

[simo5]

So looking at OpenSSL what we need to return is indeed just:
OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY
and
OSSL_PKEY_PARAM_PUB_KEY

[simo5]

But I do not see OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY handled in your patch, only OSSL_PKEY_PARAM_PUB_KEY ...

[simo5]

Just for OSSL_PKEY_PARAM_PUB_KEY we do not need a special function, you can call p11prov_obj_get_attr(obj, CKA_P11PROV_PUB_KEY); directly from p11prov_ed_get_params().

Are OSSL_PKEY_PARAM_PUB_KEY and OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY actually the same?
In OpenSSL I see OSSL_PKEY_PARAM_PUB_KEY is marked as gettable, but ecx_get_params() never returns it, it only returns OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY ...

[simo5]

Ah I see, I confused myself.
OSSL_PKEY_PARAM_ENCODED_PUBLIC_KEY is only used for ECX in OpenSSL which is ECDH keys ...

[simo5]

OTOH neither ed25519_get_params() nor ed448_get_params() seem to return the public key at all in OpenSSL ... they seem to return only ED25519/448_SECURITY_BITS.

[simo5]

The Key Export function in contrast does export OSSL_PKEY_PARAM_PUB_KEY, OSSL_PKEY_PARAM_PRIV_KEY, and only those params.

[Jakuje]

Rebased the PR + implemented the ed encoder for text as suggested offline, but I do not think implementing other encoders for various formats will solve the issue that the openssl is unable to read PEM file with the key when the pkcs11 provider is loaded (described in openssl/openssl#22246).

[simo5]

The direction looks good, but there is still a softhsm test failure?

[Jakuje]

Rebased, but the issue still persists.

[Jakuje]

Its still basically what is described in openssl/openssl#22246 -- when pkcs11 provider is loaded, openssl can not read PEM files with EDDSA keys. I tried to dig through that with gdb, but I assume it just skips some macros doing stuff with decoders so I did not get far enough.

I tried to rebuild rawhide openssl with traces to get some more information from the runs, and while updating to openssl-3.2.1-4.fc41.x86_64 version, it looks like it works just ok. It fails with the current openssl-3.1.1-4.fc39 in F39 so I believe it is some issue in OpenSSL fixed somewhere between these two versions. Fedora 40 should fix this as far as I know. I pushed a commit to build against fedora:rawhide (normal build, not the dist checks) and it works.

So what now? We can claim the OpenSSL 3.1 version broken for this functionality and focus on 3.2 branch skipping these tests on older openssl and document this (this might have some consequences for RHEL9) or we can try to hunt down the issue in OpenSSL and get it backported. @beldmit any guess what could be the change fixing the eddsa keys handling when there are provider handlers?

[beldmit]

No idea from the top of my head

[beldmit]

Probably this openssl/openssl#19705
Or, more possible, openssl/openssl#21219

[simo5]

We can definitely assert that certain stuff works only with a later openssl version, unless we have an easy workaround to apply to make it work with 3.0 and 3.1

It would be nice to have a fix in those distributions that ship with a previous version though.

[Jakuje]

Lets wait for resolution of openssl/openssl#24038. Regardless of it, after Fedora 40 GA (in following weeks) we should be able to merge this. Debian and Mac OS likely picked up 3.2 ...

[Jakuje]

Rebased. I think the typos still make sense to merge as well as ed text export and minor missing tests for completeness.

[simo5]

LGTM

[simo5]

The additional patch fixes the issue covscan found out, merging.