Some fixes for EdDSA signatures + test coverage

[Jakuje]

Description

There was a logic error in creating the eddsa signature params for pkcs11, that when OSSL_SIGNATURE_PARAM_INSTANCE OSSL_PARAM was not provided, the pkcs11 parameters were not created and Ed448 operations were failing.

For some reason, the pkcs11 parameters are required for the Ed448 even if no prehashing nor context is used.

Additionally, if only the context is provided by OpenSSL/caller, we also need to provide the correct parameters, which was not handled before.

Unfortunately, this is not the issue I was after during last days.

Checklist

• Code modified for feature
• Test suite updated with functionality tests
• Test suite updated with negative tests
• Documentation updated

Reviewer's checklist:

• Any issues marked for closing are addressed
• There is a test suite reasonably covering new functionality or modifications
• This feature/change has adequate documentation added
• Code conform to coding style that today cannot yet be enforced via the check style test
• Commits have short titles and sensible commit messages
• Coverity Scan has run if needed (code PR) and no new defects were found

[simo5]

I think the fix needs changes, but it is correct in abstract

[simo5]

I do not think this is correct, rather we should select an instance, presumably based on the default and bit lengths and then call through the same code at lines 2322 onward.

What are you doing here is effectively setting

    sigctx->use_eddsa_params = CK_TRUE;
    sigctx->eddsa_params.phFlag = CK_FALSE;

by omitting to set the phFlag.

I think the correct fix here involves spinning off the instance selection to a helper function, and if no params are set, calling it with "Ed25519" is size == ED25519_BIT_SIZE and with "Ed448" if ED448_BIT_SIZE

An option is to also pre-set instance constants, then if there are params they will overwrite the instance variable with whatever came from the OSSL_SIGNATURE_PARAM_INSTANCE

Once all params are handled, unconditionally call the cose that sets the various flags based on instance name.

[Jakuje]

Please, check the table in the pkcs#11 specs:
https://docs.oasis-open.org/pkcs11/pkcs11-curr/v3.0/os/pkcs11-curr-v3.0-os.html#_Toc30061191

What are you doing here is effectively setting

this is intentional. The phFlag is false in this default default. I can set it explicitly if it will make it more clear

Missing instance name on the openssl level results in Ed448/Ed25519. I find it overcomplicated going through the openssl instance if we do not have any from the caller. The code from lines 2322 just really sets use_eddsa_params and phFlag. Moreover we return on the line 2311 when there are no parameters provided.

[simo5]

It is a side effect, I anted it explicit, and that can be done easily, I can do it as a followup PR if you like

[simo5]

at this point we have many checks scattered through setup.sh that test for this or that OS and it makes it difficult to find and change them.

What about having an initial section in setup.sh that sets some generic variables like:
ED448SUPPORT=1
ED25519SUPPORT=1
ECASPARAMETERS=1

and then we have a set of checks that will turn these variables off if the specific system/library we are testing on does not support the primitive.

The actual setup code then just tests for the ED448SUPPORT (or other) variable to decide whether to run the specific setup part.

[Jakuje]

Its not that many. Just for Ed25519, Ed448 and the explicit curves (which are now moved to the meson). The Ed448 depends on the Ed25519 and the Ed448 needs specific OpenSC version (where I just figured out there is no easy way to find opensc version from any cli). Given that the mapping is sometimes based on the software token, sometimes on opensc version, I am not sure if moving the checks to some other place would make it more readable.