Skip to content

ta: pkcs11: Fix key generation for Ed25519#5574

Merged
jforissier merged 1 commit intoOP-TEE:masterfrom
varder:fix_pkcs11_ed25519
Oct 11, 2022
Merged

ta: pkcs11: Fix key generation for Ed25519#5574
jforissier merged 1 commit intoOP-TEE:masterfrom
varder:fix_pkcs11_ed25519

Conversation

@varder
Copy link
Copy Markdown

@varder varder commented Oct 7, 2022

Fix the issues with the key generation in

OP-TEE/optee_test#618

Comment thread core/tee/tee_svc_cryp.c Outdated
{
.attr_id = TEE_ATTR_ECC_CURVE,
.flags = TEE_TYPE_ATTR_SIZE_INDICATOR | TEE_TYPE_ATTR_GEN_KEY_REQ,
.flags = TEE_TYPE_ATTR_SIZE_INDICATOR | TEE_TYPE_ATTR_GEN_KEY_OPT,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really what we want? See #5573. According to the GP spec (1.3.1) TEE_ATTR_ECC_CURVE is NOT a valid attribute for TEE_TYPE_ED25519_KEYPAIR.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, there no need for the CURVE attribute.
Previously the key size was deduced based on curve when
in CMD_CREATE_OBJECT.
It is reworked to get key size from PKCS11_CKA_EC_POINT.
How do you think, is it Ok?
Also removed the code for ECC_CURVE attribute.

@varder varder force-pushed the fix_pkcs11_ed25519 branch from a28f336 to b20a2fe Compare October 8, 2022 08:54
@jforissier
Copy link
Copy Markdown
Contributor

Please squash the fixup, rebase onto master and properly explain what is wrong in commit 03e0743 that you are fixing.

@varder varder force-pushed the fix_pkcs11_ed25519 branch 2 times, most recently from b583028 to 21dda43 Compare October 10, 2022 12:01
@varder
Copy link
Copy Markdown
Author

varder commented Oct 11, 2022

Please squash the fixup, rebase onto master and properly explain what is wrong in commit 03e0743 that you are fixing.

Fixed

Comment thread ta/pkcs11/src/processing.c Outdated
return 0;

return ec_params2tee_keysize(a_ptr, a_size);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary blank lines around case block, here ...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread ta/pkcs11/src/processing.c Outdated
return 0;

return a_size * 8;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Copy Markdown
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

The ECC curve is not an attribute of an Ed25519 key pair.
Remove it from the key generation attribute.

Add getting key size by using EC_POINT attribute.

Fixes: 03e0743 ("ta: pkcs11: Add Ed25519 support")
Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
@varder varder force-pushed the fix_pkcs11_ed25519 branch from bbecdcd to 2bc942f Compare October 11, 2022 11:48
@jforissier jforissier merged commit ecd7f42 into OP-TEE:master Oct 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants