support Ed25519 sign/verify scheme #5486

sa-kib · 2022-08-15T09:27:36Z

This PR adds Ed25519 support as defined in GlobalPlatform TEE Internal Core API v1.3, using libtomcrypt as the backend crypto implementation.
Companion PR for optee_test: #610

jenswi-linaro

Comment for the commit "libutee: add Ed25519 support"

In the description of the commit. Which version 1.3 or 1.3.1? I guess we would prefer version 1.3.1 as that's the latest.

jenswi-linaro · 2022-08-16T09:08:35Z

lib/libutee/include/tee_api_defines.h

@@ -192,6 +192,7 @@
 #define TEE_ALG_ECDSA_P256                      0x70003041
 #define TEE_ALG_ECDSA_P384                      0x70004041
 #define TEE_ALG_ECDSA_P521                      0x70005041
+#define TEE_ALG_ED25519                         0x70006043


Please add a comment at the end of the define that this is from the v1.3 or v1.3.1 spec.

Same for the other new defines in this commit.

jenswi-linaro

Comments for the commit "core: crypto: add Ed25519 support"

jenswi-linaro · 2022-08-16T09:45:46Z

core/tee/tee_svc_cryp.c

+	if (res != TEE_SUCCESS)
+		return res;
+
+	tee_ed25519_key = (struct ed25519_keypair *)o->attr;


Cast is not needed.

jenswi-linaro · 2022-08-16T09:46:20Z

core/tee/tee_svc_cryp.c

+				 uint32_t param_count)
+{
+	TEE_Result res = TEE_ERROR_GENERIC;
+	struct ed25519_keypair *tee_ed25519_key = NULL;


We just key is enough for a local variable.

indeed, fixed

jenswi-linaro · 2022-08-16T09:49:06Z

core/tee/tee_svc_cryp.c

+	uint8_t ph_flag = 0;
+	uint8_t cx_flag = 0;
+
+	for (n = 0u; n < num_params; n++) {


The u is not needed here, 0 is enough.

jenswi-linaro · 2022-08-16T09:57:36Z

core/tee/tee_svc_cryp.c

+	size_t n;
+	size_t ctx_len;
+	uint8_t ctx[256] = {0};
+	uint8_t ph_flag = 0;


It looks like this should rather be a bool, same for cx_flag.

jenswi-linaro · 2022-08-16T10:04:38Z

core/include/crypto/crypto.h

+TEE_Result crypto_acipher_ed25519ctx_sign(struct ed25519_keypair *key,
+					  const uint8_t *msg, size_t msg_len,
+					  uint8_t *sig, size_t *sig_len,
+					  uint8_t flag, const uint8_t *ctx);


s/uint8_t flag/bool with_ph/ or something like that. Same for crypto_acipher_ed25519ctx_verify().

I've put ph_flag -- just to be consistent with the rest of the code

jenswi-linaro · 2022-08-16T10:12:54Z

core/tee/tee_svc_cryp.c

+{
+	size_t n;
+	size_t ctx_len;
+	uint8_t ctx[256] = {0};


This might be a bit much to keep on the stack. This could be allocated using mempool_alloc(mempool_default, ...) instead, pretty much guaranteed to succeed. If the allocation fails there's an error in the mempool configuration.

sure, then I shall use mempool_calloc(..)

jenswi-linaro

Regarding the commit "core: libtomcrypt: add ed25519ctx and ed25519ph support"

Do you have any plans to upstream this?

sjaeckel · 2022-08-17T09:59:09Z

c.f. libtom/libtomcrypt#597

larperaxis · 2022-08-17T11:06:13Z

core/crypto.mk

@@ -192,6 +193,7 @@ _CFG_CORE_LTC_XTS := $(CFG_CRYPTO_XTS)
 _CFG_CORE_LTC_CCM := $(CFG_CRYPTO_CCM)
 _CFG_CORE_LTC_AES_DESC := $(call cfg-one-enabled, CFG_CRYPTO_XTS CFG_CRYPTO_CCM)
 $(call force,CFG_CRYPTO_X25519,n,not supported by mbedtls)
+$(call force,CFG_CRYPTO_ED25519,n,not supported by mbedtls)


There is already plumbing in crypto.mk to include parts of tomcrypt to get full cipher support when using mbedtls. I would prefer that 25519 is available regardless of the chosen crypto library.

@larperaxis that would be nice indeed but since CFG_CRYPTO_X25519 already excludes mbedtls, that should be handled in a separate PR.

@larperaxis @jforissier then I shall make a separate PR for this, thank you for taking a look

sa-kib · 2022-08-18T09:00:19Z

Comment for the commit "libutee: add Ed25519 support"

In the description of the commit. Which version 1.3 or 1.3.1? I guess we would prefer version 1.3.1 as that's the latest.

hi @jenswi-linaro , thanks for a review.
Indeed it's a v1.3.1, I've put corresponding references in new declarations and also changed commit messsage.

jenswi-linaro · 2022-08-19T06:35:38Z

core/tee/tee_svc_cryp.c

 			ctx_len = params[n].content.ref.length;
 			if (ctx_len > 255)
 				return TEE_ERROR_BAD_PARAMETERS;
-
+			ctx = mempool_calloc(mempool_default, 1, 256);


This must be freed when we're done with it.

fixed by c54d157, along with other fixes that I've missed before.

sjaeckel · 2022-08-21T09:32:52Z

core/include/crypto/crypto.h

+TEE_Result crypto_acipher_ed25519ctx_verify(struct ed25519_keypair *key,
+					    const uint8_t *msg, size_t msg_len,
+					    const uint8_t *sig, size_t sig_len,
+					    bool ph_flag, const uint8_t *ctx);


This misses a ctx_len

fixed in 3eba17f, thank you

jforissier · 2022-08-23T09:15:33Z

core/crypto.mk

@@ -47,8 +47,9 @@ CFG_CRYPTO_ECC ?= y
 CFG_CRYPTO_SM2_PKE ?= y
 CFG_CRYPTO_SM2_DSA ?= y
 CFG_CRYPTO_SM2_KEP ?= y
-# X25519 is only supported by libtomcrypt
+# X25519 and ed25519 are only supported by libtomcrypt


jforissier · 2022-08-23T09:18:31Z

core/include/crypto/crypto.h

@@ -172,6 +172,11 @@ struct x25519_keypair {
 	uint8_t *pub;	/* Public value */
 };

+struct ed25519_keypair {
+	uint8_t *priv;	/* Private value */
+	uint8_t *pub;	/* Public value */


Comments not needed, the names are obvious

jforissier · 2022-08-23T09:25:24Z

core/crypto.mk

@@ -192,6 +193,7 @@ _CFG_CORE_LTC_XTS := $(CFG_CRYPTO_XTS)
 _CFG_CORE_LTC_CCM := $(CFG_CRYPTO_CCM)
 _CFG_CORE_LTC_AES_DESC := $(call cfg-one-enabled, CFG_CRYPTO_XTS CFG_CRYPTO_CCM)
 $(call force,CFG_CRYPTO_X25519,n,not supported by mbedtls)
+$(call force,CFG_CRYPTO_ED25519,n,not supported by mbedtls)


@larperaxis that would be nice indeed but since CFG_CRYPTO_X25519 already excludes mbedtls, that should be handled in a separate PR.

jforissier

Hi,

Two very minor comments below. Please also check the CI checkpatch errors, the alignment issue should be fixed.
I'd like the fixup commits to be squashed if @jenswi-linaro is OK with that. I did a git rebase -i --autosquash and there is an issue with the number of arguments to ed25519ph_sign() it seems.
For commit "libutee: add Ed25519 support":
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

jenswi-linaro · 2022-08-23T09:36:57Z

Yes, please squash the fixup commits.

sa-kib · 2022-08-23T11:52:23Z

Hi,

hi @jforissier, thanks for review!

Two very minor comments below. Please also check the CI checkpatch errors, the alignment issue should be fixed.

fixed

I'd like the fixup commits to be squashed if @jenswi-linaro is OK with that. I did a git rebase -i --autosquash and there is an issue with the number of arguments to ed25519ph_sign() it seems.

done squashing & rebasing, but what's the issue with ed25519ph_sign() ?

jforissier · 2022-08-23T12:05:45Z

what's the issue with ed25519ph_sign() ?

It is introduced with 7 arguments in patch number 2, then called with only 6 in patch number 3, then the final patch adds the missing argument (cxtlen).

Please change the subject of patch 2 from "core: libtomcrypt: add ed25519ctx and ed25519ph support" to "libtomcrypt: add ed25519ctx and ed25519ph support".

sa-kib · 2022-08-24T11:24:22Z

what's the issue with ed25519ph_sign() ?

It is introduced with 7 arguments in patch number 2, then called with only 6 in patch number 3, then the final patch adds the missing argument (cxtlen).

oh, I see, it's fixed already then

Please change the subject of patch 2 from "core: libtomcrypt: add ed25519ctx and ed25519ph support" to "libtomcrypt: add ed25519ctx and ed25519ph support".

yes, done

jforissier · 2022-08-24T11:48:58Z

oh, I see, it's fixed already then

No. The functions are introduced in patch 2 with a wrong signature, then modified in patch 3. I don't want that, please do the right thing the first time. Unless I'm missing something...

sa-kib · 2022-08-25T10:55:47Z

oh, I see, it's fixed already then

No. The functions are introduced in patch 2 with a wrong signature, then modified in patch 3. I don't want that, please do the right thing the first time. Unless I'm missing something...

my bad, I've messed with one of fixups.
Now I've done a proper fixup, squashed, rebased & verified.

etienne-lms · 2022-08-25T15:02:07Z

core/lib/libtomcrypt/src/headers/tomcrypt_pk.h

+                   const curve25519_key *private_key);
+int ed25519_verify(const unsigned char *msg, unsigned long msglen,
+                   const unsigned char *sig, unsigned long siglen,
+                         int *stat,


indentation issue?
not a strong opinion seen the already existing fuzzy indentation in this header file.

That's a LibTomCrypt file so coding style is different.

yep, I've just tried to retain LTC coding style there.
But I can do it differently ofc, this header seems to have slightly different identation styles mixed.

etienne-lms · 2022-08-25T15:05:45Z

core/lib/libtomcrypt/src/pk/ec25519/ec25519_crypto_ctx.c

@@ -0,0 +1,40 @@
+/* LibTomCrypt, modular cryptographic library -- Tom St Denis */
+/* SPDX-License-Identifier: Unlicense */


BSD-2-Clause?

It seems that LTC is using this license consistently.

@sjaeckel created that file & code, so I didn't dare to change it.

Yes indeed, keep as is.

etienne-lms · 2022-08-25T15:10:05Z

core/lib/libtomcrypt/src/pk/ec25519/tweetnacl.c

+  XMEMCPY(buf+cs,m,n);
+
+  err = tweetnacl_crypto_hash(out,buf,len);
+  zeromem(buf, len);


#ifdef LTC_CLEAN_STACK zeromem(buf, len); #endif

?

agree, done in 4d7340e

etienne-lms · 2022-08-25T15:12:50Z

core/lib/libtomcrypt/src/pk/ec25519/tweetnacl.c

+{
+  unsigned long len;
+  int err;
+  u8 buf[512];


Why 512?

Prevent headvy stack consumption. I think you could get your way using hash_memory_multi().

in tweetnacl they do all allocations on stack, so we followed their a style (though other allocations in this library are considerably smaller).

As for 512 size considerations are:
255 (context size) + 32 context prefix + 1 flag + 1 ctx len
PREFIX=b"SigEd25519 no Ed25519 collisions" so minimal allocation of 289 seems to be required.
But there are another algorithms that might require context prefix. So it was rounded up to 512.

@etienne-lms I also thought it could be worth it, but apparently tweetnacl itself is already very stack-heavy.

Each invocation of tweetnacl_crypto_hash[_ctx]() inside tweetnacl originates from a function that also calls other functions. Those other functions require way more stack than 512 bytes, ergo that stack size is already required.

If I counted correctly the call-stack of the two public API functions tweetnacl_crypto_sign[_open](): scalarbase() -> scalarmult() -> add() -> M() -> car25519() requires those stack variables:

typedef i64 gf[16]; int i; //4 or 8 i64 c; //8 i64 i,j,t[31]; //264 gf a,b,c,d,t,e,f,g,h; //1152 u8 b; //1 int i; //4 or 8 gf q[4]; //512 // max. sum of the above: 1953 bytes

I still liked the idea so there's the last commit of ~~libtom/libtomcrypt#598~~

[Edit] I've pulled that commit out into a separate PR libtom/libtomcrypt#599 together with some others.

Patch backported as a fixup: ff58986

etienne-lms · 2022-08-25T15:16:48Z

core/lib/libtomcrypt/src/pk/ec25519/ec25519_crypto_ctx.c

+  XMEMCPY(buf, &ctxlen8, 1);
+  buf++;
+
+  if (ctxlen > 0u) {


test not strictly needed.

indeed, fixed in 4d7340e

etienne-lms · 2022-08-25T15:55:48Z

core/tee/tee_svc_cryp.c

+			break;
+
+		case TEE_ATTR_EDDSA_CTX:
+			/* only first provided context if effective */


is it a programming error if ther are several TEE_ATTR_EDDSA_CTX attributes defined? Should the code warm or panic in such case?

Specification does not explicitly forbid passing several context strings, the decision what to do in this case seems to be left up to implementation. Yet the case with several context strings does look like a clear programming error, so it probably should panic indeed, I'll do a change.

done in 9f65a72 -- only one context is permitted, also disposed of boilerplate code.

etienne-lms · 2022-08-25T15:57:05Z

core/tee/tee_svc_cryp.c

+			if (cx_flag)
+				break;
+			ctx_len = params[n].content.ref.length;
+			if (ctx_len > 255)


This is a somewhat magic number. I believe the context size must be <256 as per the algo spec. If so, a macro with a explicit name would be better.

done in 785b4be

etienne-lms · 2022-08-25T15:58:39Z

core/tee/tee_svc_cryp.c

+			if (!ctx)
+				return TEE_ERROR_OUT_OF_MEMORY;
+			memcpy(ctx, params[n].content.ref.buffer, ctx_len);
+			ctx[ctx_len] = 0;


not needed since mempool_calloc() zeroes the allocated buffer.

fixed in 785b4be

etienne-lms · 2022-08-25T16:00:08Z

core/tee/tee_svc_cryp.c

+			   const TEE_Attribute *params, size_t num_params)
+{
+	TEE_Result err;
+	size_t n;


fixed in 785b4be

etienne-lms · 2022-08-25T16:00:29Z

core/tee/tee_svc_cryp.c

+			ctx_len = params[n].content.ref.length;
+			if (ctx_len > 255)
+				return TEE_ERROR_BAD_PARAMETERS;
+			ctx = mempool_calloc(mempool_default, 1, ctx_len + 1);


check ctx != NULL

fixed in 785b4be

@etienne-lms

@etienne-lms remarked in [0] that the stack usage could be minimized by using `hash_memory_multi()` instead of copying the data, so let's do that. [0] OP-TEE/optee_os#5486 (comment) Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>

@etienne-lms

@etienne-lms remarked in [0] that the stack usage could be minimized by using `hash_memory_multi()` instead of copying the data, so let's do that. [0] OP-TEE/optee_os#5486 (comment) Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>

@etienne-lms

@etienne-lms remarked in [0] that the stack usage could be minimized by using `hash_memory_multi()` instead of copying the data, so let's do that. [0] OP-TEE/optee_os#5486 (comment) Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>

jforissier · 2022-09-01T11:03:21Z

Please do not use the "core:" prefix for pure LibTomCrypt changes. I know LTC is used only in the TEE core not as a user space library, but I prefer to reserve "core: libtomcrypt: ..." for the OP-TEE specific adaptation layer and "libtomcrypt: ..." for the the rest. Makes it a bit easier to see which commit touches what.

Overall this looks good to me, I will review again once fixups are squashed, @etienne-lms let us now when you want them squashed. EDIT: oops, a compile issue has to be fixed (see CI).

@etienne-lms

@etienne-lms remarked in [0] that the stack usage could be minimized by using `hash_memory_multi()` instead of copying the data, so let's do that. [0] OP-TEE/optee_os#5486 (comment) Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>

etienne-lms

few minor comments.
@sa-kib, you can squash the fixup commits.

etienne-lms · 2022-09-05T09:46:33Z

core/include/crypto/crypto.h

@@ -172,6 +172,11 @@ struct x25519_keypair {
 	uint8_t *pub;	/* Public value */
 };

+struct ed25519_keypair {
+	uint8_t *priv;
+	uint8_t *pub;


uint8_t *priv; /* Private value */ uint8_t *pub; /* Public value */

for consistency?

it was agreed with @jforissier to be unnecessary.

etienne-lms · 2022-09-05T10:07:59Z

core/lib/libtomcrypt/src/pk/ec25519/ec25519_crypto_ctx.c

+  const unsigned long prefix_len = strlen(prefix);
+  const unsigned char ctxlen8 = (unsigned char)ctxlen;
+
+  if (ctxlen > 255u) return CRYPT_INPUT_TOO_LONG;


a well named macro would be nice but I guess it rather depends on libtomcrypt maintainers.

etienne-lms · 2022-09-05T10:10:20Z

core/lib/libtomcrypt/src/pk/ec25519/ec25519_crypto_ctx.c

@@ -0,0 +1,40 @@
+/* LibTomCrypt, modular cryptographic library -- Tom St Denis */
+/* SPDX-License-Identifier: Unlicense */


Yes indeed, keep as is.

etienne-lms · 2022-09-09T16:00:47Z

core/lib/libtomcrypt/ed25519.c

+				    ctx, ctxlen, &private_key) != CRYPT_OK)
+			return TEE_ERROR_BAD_PARAMETERS;
+	}
+


Public key content is not that sensible. I think it's not worth wiping its content.

@etienne-lms

@etienne-lms remarked in [0] that the stack usage could be minimized by using `hash_memory_multi()` instead of copying the data, so let's do that. [0] OP-TEE/optee_os#5486 (comment) Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>

jforissier

With the below minor comments addressed (and please also fix the long line in the commit description spotted by checkpatch):

"libtomcrypt: fix excessive memory clean"
"libtomcrypt: add ed25519ctx and ed25519ph support"
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>

"core: libtomcrypt: add Ed25519 support"
"core: crypto: add Ed25519 support "
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

jforissier · 2022-09-12T09:57:40Z

core/tee/tee_svc_cryp.c

+
+	*ctx = NULL;
+
+	for (; n < num_params; n++) {


Use n = 0 here (although it is already initialized)

jforissier · 2022-09-12T10:00:40Z

core/tee/tee_svc_cryp.c

+				break;
+
+			*ctx = params[n].content.ref.buffer;
+			if (*ctx == NULL)


if (!*ctx)

jforissier · 2022-09-12T10:01:09Z

core/tee/tee_svc_cryp.c

+			if (*ctx_len > TEE_ED25519_CTX_MAX_LENGTH)
+				return TEE_ERROR_BAD_PARAMETERS;
+
+			if (*ctx_len == 0)


if (!*ctx_len)

sa-kib · 2022-09-12T11:33:09Z

squashed & rebased. Also get rid of mempool_alloc(), as we don't modify context and to make things a bit simpler.

etienne-lms

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> for commit
"libtomcrypt: fix excessive memory clean"

Acked-by: Etienne Carriere <etienne.carriere@linaro.org> for commits:
"libutee: add Ed25519 support"
"libtomcrypt: add ed25519ctx and ed25519ph support"
"core: libtomcrypt: add Ed25519 support"
"core: crypto: add Ed25519 support"

sjaeckel · 2022-09-13T10:38:55Z

Maybe you want to update your libtomcrypt? c.f. libtom/libtomcrypt#599

jforissier · 2022-09-13T11:03:17Z

Maybe you want to update your libtomcrypt? c.f. libtom/libtomcrypt#599

That would be a good thing to do indeed. The last full sync dates back to 4 years ago, although we have picked a few fixes from upstream since then. It doesn't necessarily have to be done before this PR is merged however.

@sjaeckel can we consider that the current "develop" branch is stable?

sjaeckel · 2022-09-13T13:05:48Z

@sjaeckel can we consider that the current "develop" branch is stable?

yes, I'd say current develop is pretty stable.

The only things happening sometime would be:

what will happen with the ECC API after An attempt to fix #449 (ECDSA forcing DER/ASN.1) libtom/libtomcrypt#450 and Feature/rfc6979 libtom/libtomcrypt#477 are tackled... but I don't expect this to be really big changes, for 477 I've even a compatible way in mind.
whether we will follow up on [WIP/RFC] Remove prng registry libtom/libtomcrypt#515 and the other registries
Add PEM support libtom/libtomcrypt#587 will change the *_import_pkcs8() APIs, but I doubt you'll use them

The last full sync dates back to 4 years ago

Also I didn't mean a full sync, only the relevant changes of the mentioned PR

sa-kib · 2022-09-15T14:39:56Z

hi @jenswi-linaro, can this PR be considered in fit for inclusion?

jenswi-linaro · 2022-09-20T09:00:37Z

Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

jforissier · 2022-09-20T09:05:49Z

@sa-kib sorry for the delay on this PR. On second thought, I'd like to have it merged after LibTomCrypt is updated in the master branch (that is #5539 which will be squash-merged into master). In the meantime, I have tried re-basing your patches onto the new LTC (without the two libtomcrypt: patches of course, which are already included upstream) and I there were only minor conflicts/issues. You will need to apply the below diff to "core: libtomcrypt: add Ed25519 support" and all should be well.

diff --git a/core/lib/libtomcrypt/ed25519.c b/core/lib/libtomcrypt/ed25519.c
index 33c1d852..b85629b8 100644
--- a/core/lib/libtomcrypt/ed25519.c
+++ b/core/lib/libtomcrypt/ed25519.c
@@ -7,6 +7,7 @@
 #include <crypto/crypto.h>
 #include <stdlib.h>
 #include <string.h>
+#include <string_ext.h>
 #include <tee_api_types.h>
 #include <trace.h>
 #include <utee_defines.h>
@@ -65,7 +66,7 @@ TEE_Result crypto_acipher_ed25519_sign(struct ed25519_keypair *key,
 	unsigned long siglen;
 	curve25519_key private_key = {
 		.type = PK_PRIVATE,
-		.algo = PKA_ED25519,
+		.algo = LTC_OID_ED25519,
 	};
 
 	if (!key)
@@ -94,7 +95,7 @@ TEE_Result crypto_acipher_ed25519ctx_sign(struct ed25519_keypair *key,
 	unsigned long siglen;
 	curve25519_key private_key = {
 		.type = PK_PRIVATE,
-		.algo = PKA_ED25519,
+		.algo = LTC_OID_ED25519,
 	};
 
 	if (!key)
@@ -126,7 +127,7 @@ TEE_Result crypto_acipher_ed25519_verify(struct ed25519_keypair *key,
 	int stat = 0;
 	curve25519_key public_key = {
 		.type = PK_PUBLIC,
-		.algo = PKA_ED25519,
+		.algo = LTC_OID_ED25519,
 	};
 
 	if (!key)
@@ -153,7 +154,7 @@ TEE_Result crypto_acipher_ed25519ctx_verify(struct ed25519_keypair *key,
 	int stat = 0;
 	curve25519_key public_key = {
 		.type = PK_PUBLIC,
-		.algo = PKA_ED25519,
+		.algo = LTC_OID_ED25519,
 	};
 
 	if (!key)
diff --git a/core/lib/libtomcrypt/src/pk/ec25519/sub.mk b/core/lib/libtomcrypt/src/pk/ec25519/sub.mk
index e3d07c0e..5040c391 100644
--- a/core/lib/libtomcrypt/src/pk/ec25519/sub.mk
+++ b/core/lib/libtomcrypt/src/pk/ec25519/sub.mk
@@ -1,2 +1,3 @@
+srcs-y += ec25519_crypto_ctx.c
 srcs-y += ec25519_export.c
 srcs-y += tweetnacl.c
diff --git a/core/lib/libtomcrypt/src/pk/ed25519/sub.mk b/core/lib/libtomcrypt/src/pk/ed25519/sub.mk
index bdd41059..cb2206da 100644
--- a/core/lib/libtomcrypt/src/pk/ed25519/sub.mk
+++ b/core/lib/libtomcrypt/src/pk/ed25519/sub.mk
@@ -3,6 +3,5 @@ srcs-y += ed25519_import.c
 srcs-y += ed25519_import_pkcs8.c
 srcs-y += ed25519_import_x509.c
 srcs-y += ed25519_make_key.c
-srcs-y += ed25519_set_key.c
 srcs-y += ed25519_sign.c
 srcs-y += ed25519_verify.c

Please do not review this patch. The changes originate from: OP-TEE#5486 The changes are not expected to be merged with this pull request. This commit adds Ed25519 support as defined in TEE Internal Core API v1.3.1 Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> libtomcrypt: add ed25519ctx and ed25519ph support Support contextualized extension of the Ed25519 scheme. Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Acked-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> core: libtomcrypt: add Ed25519 support Enable Ed25519 implementation of libtomcrypt and add the OP-TEE wrappers. Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> core: crypto: add Ed25519 support Put in place Ed25519 core functionality and support it for OP-TEE crypto syscalls. Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

jforissier · 2022-09-29T08:16:04Z

@sa-kib @varder I have now merged the LibTomCrypt update, so could you please rebase this PR on top of master and apply the Ack/Review tags given above? Thanks!

This commit adds Ed25519 support as defined in TEE Internal Core API v1.3.1 Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Enable Ed25519 implementation of libtomcrypt and add the OP-TEE wrappers. Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

Put in place Ed25519 core functionality and support it for OP-TEE crypto syscalls. Signed-off-by: Valerii Chubar <valerii_chubar@epam.com> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org> Acked-by: Etienne Carriere <etienne.carriere@linaro.org> Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

jforissier · 2022-09-30T13:10:55Z

@sa-kib @varder thanks for this contribution and for working with upstream LibTomCrypt too. I am merging this despite the IBART failure because I know it is caused by the missing commit b7563ba in your branch.

sa-kib mentioned this pull request Aug 15, 2022

Ed25519 test cases OP-TEE/optee_test#610

Merged

jenswi-linaro reviewed Aug 16, 2022

View reviewed changes

larperaxis reviewed Aug 17, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch from 10a9fdd to 43d0098 Compare August 18, 2022 08:55

jenswi-linaro reviewed Aug 19, 2022

View reviewed changes

sjaeckel suggested changes Aug 21, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch 3 times, most recently from 84d92ca to c54d157 Compare August 22, 2022 16:34

jforissier reviewed Aug 23, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch from a5d54fc to 8450dc1 Compare August 23, 2022 11:48

sa-kib changed the title ~~[RFC] support Ed25519 sign/verify scheme~~ support Ed25519 sign/verify scheme Aug 23, 2022

sa-kib force-pushed the ed25519 branch from 8450dc1 to 2d37633 Compare August 24, 2022 11:23

sa-kib force-pushed the ed25519 branch from 2d37633 to f598f6a Compare August 25, 2022 10:52

etienne-lms reviewed Aug 25, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch from 332157e to a520052 Compare September 2, 2022 09:21

etienne-lms reviewed Sep 9, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch from 9f65a72 to 4e928c6 Compare September 12, 2022 09:37

jforissier reviewed Sep 12, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch 2 times, most recently from d8f03fb to a2f2291 Compare September 12, 2022 10:50

etienne-lms reviewed Sep 12, 2022

View reviewed changes

sa-kib force-pushed the ed25519 branch from a2f2291 to da0251e Compare September 13, 2022 10:14

sa-kib force-pushed the ed25519 branch from da0251e to 2986f5a Compare September 15, 2022 09:22

varder mentioned this pull request Sep 29, 2022

PKCS#11 Ed25519 #5559

Merged

sa-kib and others added 3 commits September 30, 2022 07:29

sa-kib force-pushed the ed25519 branch from 2986f5a to c7aa315 Compare September 30, 2022 07:33

jforissier merged commit 0aaad41 into OP-TEE:master Sep 30, 2022

sa-kib deleted the ed25519 branch October 6, 2022 10:53

sa-kib mentioned this pull request Dec 2, 2022

make X25519 & Ed25519 available for mbedtls configurations #5653

Merged

		@@ -0,0 +1,40 @@
		/* LibTomCrypt, modular cryptographic library -- Tom St Denis */
		/* SPDX-License-Identifier: Unlicense */

support Ed25519 sign/verify scheme #5486

support Ed25519 sign/verify scheme #5486

Conversation

sa-kib commented Aug 15, 2022 • edited Loading

jenswi-linaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenswi-linaro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenswi-linaro left a comment

Choose a reason for hiding this comment

sjaeckel commented Aug 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sa-kib commented Aug 18, 2022

Choose a reason for hiding this comment

sa-kib Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sa-kib Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jforissier left a comment

Choose a reason for hiding this comment

jenswi-linaro commented Aug 23, 2022

sa-kib commented Aug 23, 2022 • edited Loading

jforissier commented Aug 23, 2022

sa-kib commented Aug 24, 2022

jforissier commented Aug 24, 2022

sa-kib commented Aug 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etienne-lms Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjaeckel Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

sjaeckel Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jforissier commented Sep 1, 2022 • edited Loading

sa-kib commented Aug 15, 2022 •

edited

Loading

sa-kib Aug 22, 2022 •

edited

Loading

sa-kib Aug 22, 2022 •

edited

Loading

sa-kib commented Aug 23, 2022 •

edited

Loading

etienne-lms Aug 25, 2022 •

edited

Loading

sjaeckel Aug 31, 2022 •

edited

Loading

sjaeckel Aug 31, 2022 •

edited

Loading

jforissier commented Sep 1, 2022 •

edited

Loading