Support mbedtls crypto library #2705

edison-ai · 2018-12-17T08:32:36Z

Move #2345 here.

Initial step of mbedtls cryptos integration. Directory created and interface file is drafted. All function interfaces are set to "not supported". The mbedtls can be selected by specifying build flags "CFG_CRYPTOLIB_NAME=mbedtls" and "CFG_CRYPTOLIB_DIR=lib/libmbedtls" Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

1. Support mbedtls hash algorithm. 2. Add mbedtls source configure Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

jenswi-linaro · 2018-12-17T10:04:24Z

There seems to be some confusion here. Only the commits:
"libmbedtls: GCM and CCM enhance for integrate", "libmbedtls: fix no CRT issue" and "libmbedtls: add interfaces in mbedtls for context memory operation" should first be merged into import/mbedtls-2.6.1.

The purpose of the import/mbedtls-2.6.1 is to record all changes to the imports source code of mbedtls-2.6.1. This will then be used when we import a newer version of mbedtls.

Since import/mbedtls-2.6.1 happens to be quite close to master for the moment it's still meaningful to review the changes here. It's just that before anything can be merged we'll need a new PR against the master branch. I'll continue the review now.

jenswi-linaro

Comment for "libmbedtls: Support DSA algorithm"

lib/libmbedtls/core/sub.mk

jenswi-linaro

Comments for the commit "libmbedtls: Support cipher CTS and XTS algorithm"

jenswi-linaro · 2018-12-17T10:14:43Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -1931,11 +1990,13 @@ static TEE_Result cipher_get_ctx_size(uint32_t algo, size_t *size)
 #endif
 #if defined(CFG_CRYPTO_XTS)
 	case TEE_ALG_AES_XTS:
-		return TEE_ERROR_NOT_SUPPORTED;
+		*size = sizeof(struct tee_symmetric_cts);


tee_symmetric_xts

Yes, sorry for mistake.

jenswi-linaro · 2018-12-17T10:14:51Z

lib/libmbedtls/core/tee_lmd_provider.c

 #endif
 #if defined(CFG_CRYPTO_CTS)
 	case TEE_ALG_AES_CTS:
-		return TEE_ERROR_NOT_SUPPORTED;
+		*size = sizeof(struct tee_symmetric_xts);


tee_symmetric_cts

Yes, will fix it.

jenswi-linaro · 2018-12-17T10:23:40Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -2016,8 +2077,10 @@ TEE_Result crypto_cipher_alloc_ctx(void **ctx_ret, uint32_t algo)
 		return TEE_ERROR_NOT_SUPPORTED;
 	}

-	if (!cipher_info)
+	if (algo != TEE_ALG_AES_XTS && algo != TEE_ALG_AES_CTS &&
+	    !cipher_info) {


Minor: the braces aren't needed.

Yes, will fix it.

jenswi-linaro · 2018-12-17T10:27:29Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -2049,13 +2116,26 @@ void crypto_cipher_free_ctx(void *ctx, uint32_t algo __maybe_unused)
 	 * could never have succeded above.
 	 */
 	assert(!cipher_get_ctx_size(algo, &ctx_size));
-	mbedtls_cipher_free(ctx);
+#if defined(CFG_CRYPTO_XTS) || defined(CFG_CRYPTO_CTS)


Unless CFG_CRYPTO_XTS or CFG_CRYPTO_CTS is defined we'll potentially leak memory.
I see no harm in always having the if (...) below

Yes, it is a mistake. Should not add them here.

jenswi-linaro · 2018-12-17T10:29:24Z

lib/libmbedtls/core/tee_lmd_provider.c

+	if (algo == TEE_ALG_AES_XTS || algo == TEE_ALG_AES_CTS) {
+		res = cipher_get_ctx_size(algo, &ctx_size);
+		assert(!res);
+		memcpy(dst_ctx, src_ctx, ctx_size);


I think a return; is needed after this line.

OK, will add it.

jenswi-linaro · 2018-12-17T10:35:59Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -2071,6 +2151,58 @@ TEE_Result crypto_cipher_init(void *ctx, uint32_t algo,
 	const mbedtls_cipher_info_t *cipher_info = NULL;
 	int lmd_res;
 	int lmd_mode;
+#if defined(CFG_CRYPTO_CTS)
+	struct tee_symmetric_cts *cts;


These helper variables are only used in their if blocks below so please move the declarations.

jenswi-linaro · 2018-12-17T10:38:22Z

lib/libmbedtls/core/tee_lmd_provider.c

+
+#if defined(CFG_CRYPTO_XTS)
+	if (algo == TEE_ALG_AES_XTS) {
+		res = tee_algo_to_ltc_cipherindex(algo, &ltc_cipherindex);


Could move these three lines into the other if (algo == TEE_ALG_AES_XTS) block below.

jenswi-linaro · 2018-12-17T10:40:26Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -2127,6 +2259,13 @@ TEE_Result crypto_cipher_update(void *ctx, uint32_t algo,
 	int lmd_res;
 	size_t olen;
 	size_t finish_olen;
+#if defined(CFG_CRYPTO_CTS)
+	struct tee_symmetric_cts *cts;


I'd prefer if the #if was skipped and the __maybe_unused tag was used instead, here and the two other locals below.

Sorry for just copy code to here directly. I will fix it.

jenswi-linaro

Comments for commit "libmbedtls: Support CBC MAC algorithm"

jenswi-linaro · 2018-12-17T10:46:58Z

lib/libmbedtls/core/tee_lmd_provider.c

+
+#if defined(CFG_CRYPTO_CBC_MAC)
+/*
+ * CBC-MAC is not implemented in Libtomcrypt


s/Libtomcrypt/mbedtls/

Yes, I will fix it.

jenswi-linaro · 2018-12-17T10:54:26Z

lib/libmbedtls/core/tee_lmd_provider.c

-		return TEE_ERROR_NOT_SUPPORTED;
+		cbc = (struct cbc_state *)ctx;
+
+		res = tee_algo_to_ltc_cipherindex(algo, &ltc_cipherindex);


How come the mbedtls implementation of AES/DES/DES3 isn't used instead?

Sorry not notice it before. I will refine it by using mbedtls.

jenswi-linaro · 2018-12-17T10:56:24Z

For "libmbedtls: Support DSA algorithm" please apply:
Reviewed-by: Jens Wiklander <[email protected]>

jenswi-linaro · 2018-12-17T10:57:16Z

I'm done with the review for the moment.

edison-ai · 2018-12-18T09:42:03Z

For "libmbedtls: GCM and CCM enhance for integrate", "libmbedtls: fix no CRT issue" and "libmbedtls: add interfaces in mbedtls for context memory operation", I will drop them in this PR. Because they had been in #2439.

jenswi-linaro · 2018-12-18T09:59:33Z

We've advanced far enough in this PR that it would make sense to start merging that #2439.

Adds support for symmetrical ciphers. The CTS and XTS modes are not supported in mbedTLS and will be dealt with later. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Adds size of expanded AES encryption key to crypto_aes_expand_enc_key() and crypto_aes_enc_block() to make the functions more safe to call. Signed-off-by: Summer Qin <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

These two implement interfaces will be used by AES-GCM algo. Signed-off-by: Summer Qin <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Implement HMAC function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Implement CMAC function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Implement bignum function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Support RSA: RSASSA_PKCS1_V1_5 RSASSA_PKCS1_PSS_MGF1 RSAES_PKCS1_V1_5 RSAES_PKCS1_OAEP_MGF1 Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Implement DH function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Support mbedtls ECC: ecdh and ecdsa. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Implement CCM function which based on mbedtls. Signed-off-by: Shawn Shan <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

DSA is not supported in MbedTLS, call libtomcrypt directly. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Cipher CTS and XTS are not supported in MbedTLS, call libtomcrypt directly. Signed-off-by: Edison Ai <[email protected]>

CBC MAC supported in MbedTLS, implement is by using cipher. Signed-off-by: Edison Ai <[email protected]>

jbech-linaro · 2018-12-19T08:29:16Z

There are a couple of compiler errors in lib/libmbedtls/core/tee_lmd_provider.c (see IBART).

edison-ai · 2018-12-19T09:24:21Z

@jbech-linaro Yes. It is because I remove 3 patches form this PR for them are in another PR #2439. As discuss, I remove them here.
Is it possible to set the PR dependence, and include the #2439 when build this PR?

jbech-linaro · 2018-12-19T09:29:30Z

Is it possible to set the PR dependence, and include the #2439 when build this PR?

Not at this point, that's a feature on the IBART to-do list and was actually one of the reasons I started writing IBART. So if it's a cross dependency, then we have to live with that (the error) for now.

jenswi-linaro · 2019-04-09T11:20:16Z

Replaced by the now merged #2904

Edison Ai and others added 2 commits December 13, 2018 16:47

libmbedtls: support mbedtls hash algorithm

df65a02

1. Support mbedtls hash algorithm. 2. Add mbedtls source configure Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

edison-ai mentioned this pull request Dec 17, 2018

Support mbedtls crypto library #2345

Closed

edison-ai force-pushed the merge_to_mbedtls-2.6.1 branch from 1f52185 to 1ae0da7 Compare December 17, 2018 09:27

jenswi-linaro reviewed Dec 17, 2018

View reviewed changes

lib/libmbedtls/core/sub.mk Outdated Show resolved Hide resolved

jenswi-linaro requested changes Dec 17, 2018

View reviewed changes

jenswi-linaro reviewed Dec 17, 2018

View reviewed changes

edison-ai force-pushed the merge_to_mbedtls-2.6.1 branch from 1ae0da7 to fcc6a08 Compare December 19, 2018 07:03

Edison Ai and others added 13 commits December 19, 2018 15:50

libmbedtls: support symmetrical ciphers

b564296

Adds support for symmetrical ciphers. The CTS and XTS modes are not supported in mbedTLS and will be dealt with later. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

core: add encrypt key length in aes encrypt api

fed127f

Adds size of expanded AES encryption key to crypto_aes_expand_enc_key() and crypto_aes_enc_block() to make the functions more safe to call. Signed-off-by: Summer Qin <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: implement AES encrypt api

26d30b7

These two implement interfaces will be used by AES-GCM algo. Signed-off-by: Summer Qin <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls HMAC algorithm

28f1e18

Implement HMAC function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support CMAC algorithm

77004f0

Implement CMAC function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls bignum functions

b311106

Implement bignum function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls acipher RSA function

c89995c

Support RSA: RSASSA_PKCS1_V1_5 RSASSA_PKCS1_PSS_MGF1 RSAES_PKCS1_V1_5 RSAES_PKCS1_OAEP_MGF1 Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls DH function

f272b5c

Implement DH function which based on mbedtls. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls ECC function

790d139

Support mbedtls ECC: ecdh and ecdsa. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: support mbedtls ccm function.

9d23525

Implement CCM function which based on mbedtls. Signed-off-by: Shawn Shan <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: Support DSA algorithm

ecf1468

DSA is not supported in MbedTLS, call libtomcrypt directly. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

libmbedtls: Support cipher CTS and XTS algorithm

888fa68

Cipher CTS and XTS are not supported in MbedTLS, call libtomcrypt directly. Signed-off-by: Edison Ai <[email protected]>

libmbedtls: Support CBC MAC algorithm

a1a3d47

CBC MAC supported in MbedTLS, implement is by using cipher. Signed-off-by: Edison Ai <[email protected]>

edison-ai force-pushed the merge_to_mbedtls-2.6.1 branch from fcc6a08 to a1a3d47 Compare December 19, 2018 08:01

edison-ai mentioned this pull request Jan 29, 2019

libmbedtls: add interfaces in mbedtls for context memory operation #2439

Closed

jenswi-linaro mentioned this pull request Mar 4, 2019

[RFC] core: remove algo from crypto_hash_*() #2857

Merged

jenswi-linaro closed this Apr 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support mbedtls crypto library #2705

Support mbedtls crypto library #2705

edison-ai commented Dec 17, 2018

jenswi-linaro commented Dec 17, 2018

jenswi-linaro left a comment

jenswi-linaro left a comment

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018 •

edited

Loading

jenswi-linaro left a comment

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro Dec 17, 2018

edison-ai Dec 18, 2018

jenswi-linaro commented Dec 17, 2018

jenswi-linaro commented Dec 17, 2018

edison-ai commented Dec 18, 2018

jenswi-linaro commented Dec 18, 2018

jbech-linaro commented Dec 19, 2018

edison-ai commented Dec 19, 2018

jbech-linaro commented Dec 19, 2018

jenswi-linaro commented Apr 9, 2019

Support mbedtls crypto library #2705

Support mbedtls crypto library #2705

Conversation

edison-ai commented Dec 17, 2018

jenswi-linaro commented Dec 17, 2018

jenswi-linaro left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edison-ai Dec 18, 2018 • edited Loading

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

jenswi-linaro commented Dec 17, 2018

jenswi-linaro commented Dec 17, 2018

edison-ai commented Dec 18, 2018

jenswi-linaro commented Dec 18, 2018

jbech-linaro commented Dec 19, 2018

edison-ai commented Dec 19, 2018

jbech-linaro commented Dec 19, 2018

jenswi-linaro commented Apr 9, 2019

edison-ai Dec 18, 2018 •

edited

Loading