Skip to content

libckteec: Add EDDSA attribute serialization#324

Merged
jforissier merged 1 commit intoOP-TEE:masterfrom
varder:eddsa
Oct 6, 2022
Merged

libckteec: Add EDDSA attribute serialization#324
jforissier merged 1 commit intoOP-TEE:masterfrom
varder:eddsa

Conversation

@varder
Copy link
Copy Markdown

@varder varder commented Sep 29, 2022

The PKCS#11 Specification:
https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/cs01/pkcs11-spec-v3.1-cs01.pdf

The pull request should be merged with the following pull requests:
OP-TEE/optee_os#5559
OP-TEE/optee_test#618

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.

LGTM aside these few comments.

Comment thread libckteec/include/pkcs11.h Outdated
CK_ULONG ulTagBits;
};

/* EDDSA */
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.

Suggestion: /* EdDSA (RFC 8032) */ as that's the reference we find in the litterature?

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.

done

Comment thread libckteec/include/pkcs11.h Outdated
#define CKK_DH 0x002
#define CKK_ECDSA 0x003
#define CKK_EC 0x003
#define CKK_EC_EDWARDS 0x040
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.

to move at the end (numerical order)

I propose you mention that this is from pkcs11 v3.1-cs01:
`#define CKK_EC_EDWARDS 0x040 /* from PKCS#11 v3.1-cs01 */

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 libckteec/include/pkcs11.h Outdated
#define CKM_ECDH1_COFACTOR_DERIVE 0x01051
#define CKM_ECMQV_DERIVE 0x01052
#define CKM_ECDH_AES_KEY_WRAP 0x01053
#define CKM_EDDSA 0x01057
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.

 #define CKM_ECMQV_DERIVE		0x01052
 #define CKM_ECDH_AES_KEY_WRAP		0x01053
 #define CKM_RSA_AES_KEY_WRAP		0x01054
+#define CKM_EC_EDWARDS_KEY_PAIR_GEN	0x01055
+#define CKM_EDDSA			0x01057
 #define CKM_AES_KEY_GEN			0x01080
 #define CKM_AES_ECB			0x01081

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 libckteec/include/pkcs11_ta.h Outdated
PKCS11_CKM_ECDSA_SHA512 = 0x01046,
PKCS11_CKM_ECDH1_DERIVE = 0x01050,
PKCS11_CKM_ECDH1_COFACTOR_DERIVE = 0x01051,
PKCS11_CKM_EDDSA = 0x01057,
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.

move below

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

@varder varder force-pushed the eddsa branch 2 times, most recently from aba80ff to be84913 Compare October 3, 2022 14:45
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.

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> with 2 minor comments addressed.

Comment thread libckteec/include/pkcs11.h Outdated
#define CKM_ECDH_AES_KEY_WRAP 0x01053
#define CKM_RSA_AES_KEY_WRAP 0x01054
#define CKM_EC_EDWARDS_KEY_PAIR_GEN 0x01055
#define CKM_EDDSA 0x01057
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.

nitpicking: remove 1 or 2 tabulations

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 libckteec/include/pkcs11_ta.h Outdated
PKCS11_CKM_ECMQV_DERIVE = 0x01052,
PKCS11_CKM_ECDH_AES_KEY_WRAP = 0x01053,
PKCS11_CKM_RSA_AES_KEY_WRAP = 0x01054,
PKCS11_CKM_EDDSA = 0x01057,
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.

remove 1 tab

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

The PKCS#11 Specification:
https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/cs01/
pkcs11-spec-v3.1-cs01.pdf
6.3.16 EC mechanism parameters

Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
@jforissier jforissier merged commit 140bf46 into OP-TEE:master Oct 6, 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.

3 participants