Support mbedtls crypto library #2345

edison-ai · 2018-05-23T08:22:43Z

Pull request for mbedtls crypto supporting again here.
The mbedtls source path is "lib/libmebdtls" which is same as TA's.

jenswi-linaro · 2018-05-23T09:02:14Z

core/lib/libmbedtls/sub.mk

@@ -0,0 +1,3 @@
+cflags-lib-$(CFG_CRYPTO_SIZE_OPTIMIZATION) += -Os
+
+subdirs-y += src


I'd prefer if we put everything below lib/libmbedtls instead, having two different libmbedtls directories will be confusing.

The src directory will be core only we, so we could rename it to core and make this line:

subdirs-$(sm-core) += core

Yes, it will be better to remove that confuse. I will update as that.

jenswi-linaro · 2018-05-23T09:05:22Z

core/lib/libmbedtls/include/mbedtls_config.h

@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: BSD-2-Clause */


We already have a libmbedtls/include/mbedtls_config_kernel.h, please put the content of this file there instead.

OK. I will update it.

edison-ai · 2018-05-24T06:55:51Z

@jenswi-linaro Update. But I do not know why the patch sequences are disorder. The patch "libmbedtls: configure mbedTLS for user mode TA" should be the first.

jforissier · 2018-05-24T21:05:53Z

@edison-ai it's OK, GitHub shows the commits in chronological order, which is not always parent/child order.

edison-ai · 2018-05-25T01:56:32Z

@jforissier OK, got. Thanks your explanation.

jenswi-linaro · 2018-05-28T07:48:21Z

lib/libmbedtls/core/sub.mk

+SRCS += sha256.c
+SRCS += sha512.c
+
+srcs-y += $(addprefix ../mbedtls/library/, $(SRCS))


I'd prefer if we did this in lib/libmbedtls/sub.mk.
lib/libmbedtls/sub.mk will need to be restructured a bit. We could do it a bit like:

SRCS_CRYPTO := SRCS_CRYPTO += aes.c ... SRCS_X509 := SRCS_X509 += certs.c ... SRCS_TLS := SRCS_TLS += debug.c ... srcs-y += $(addprefix mbedtls/library/, $(SRCS_CRYPTO)) srcs-$(sm-$(ta-target)) += $(addprefix mbedtls/library/, $(SRCS_X509)) srcs-$(sm-$(ta-target)) += $(addprefix mbedtls/library/, $(SRCS_TLS))

It doesn't matter if a few too many files are included as it's the lib/libmbedtls/include/mbedtls_config_kernel.h and lib/libmbedtls/include/mbedtls_config_uta.h files that controls what's really compiled.

It doesn't matter if a few too many files are included as it's the lib/libmbedtls/include/mbedtls_config_kernel.h and lib/libmbedtls/include/mbedtls_config_uta.h files that controls what's really compiled.

By that logic, why not add all the files as it's done currently?

By that logic, why not add all the files as it's done currently

Fine with me, if we think it adds too much time to compiling we can address that then.

edison-ai · 2018-06-19T08:10:45Z

@jenswi-linaro Any update?

jenswi-linaro · 2018-06-19T08:27:37Z

@edison-ai, I was actually going to ask you the same question. :-)
I'd like to get the sub.mk stuff sorted out first.

edison-ai · 2018-06-20T08:31:12Z

@jenswi-linaro Sorry for that I had a few mistake to understand what your meaning. Of course I agree with your suggestion, and I will push a new version to refine it.

edison-ai · 2018-06-22T06:58:30Z

Update and re-base it on newest master branch.

jenswi-linaro

Comment for "libmbedtls: support mbedTLS in kernel mode"

jenswi-linaro · 2018-06-25T07:54:24Z

lib/libmbedtls/sub.mk

@@ -88,3 +88,7 @@ srcs-$(sm-$(ta-target)) += $(addprefix mbedtls/library/, $(SRCS_TLS))
 cflags-lib-y += -Wno-redundant-decls
 cflags-lib-y += -Wno-switch-default
 cflags-lib-$(CFG_ULIBS_GPROF) += -pg
+
+cflags-lib-$(sm-core) += -Wno-pedantic


There's already a workaround for the "empty translation unit" errors in lib/libmbedtls/mbedtls/include/mbedtls/check_config.h, please use that instead by including that file from ib/libmbedtls/include/mbedtls_config_kernel.h just as in lib/libmbedtls/include/mbedtls_config_uta.h

Yes, this will be better.

jenswi-linaro · 2018-06-25T08:14:44Z

When compiling "libmbedtls: support mbedTLS in kernel mode", I get the following error:

lib/libmbedtls/core/tee_lmd_provider.c:559:12: error: no previous prototype for ‘crypto_rng_add_entropy’ [-Werror=missing-prototypes]
 TEE_Result crypto_rng_add_entropy(const uint8_t *inbuf __unused,
            ^~~~~~~~~~~~~~~~~~~~~~
lib/libmbedtls/core/tee_lmd_provider.c:579:12: error: no previous prototype for ‘rng_generate’ [-Werror=missing-prototypes]
 TEE_Result rng_generate(void *buffer __unused, size_t len __unused)
            ^~~~~~~~~~~~

jenswi-linaro · 2018-06-25T08:17:18Z

When compiling the entire PR I get:

lib/libmbedtls/core/tee_lmd_provider.c: In function ‘crypto_acipher_rsaes_decrypt’:
lib/libmbedtls/core/tee_lmd_provider.c:1014:60: error: passing argument 5 of ‘pk_info->decrypt_func’ from incompatible pointer type [-Werror=incompatible-pointer-types]
   lmd_res = pk_info->decrypt_func(&rsa, src, src_len, buf, &blen,
                                                            ^
lib/libmbedtls/core/tee_lmd_provider.c:1014:60: note: expected ‘size_t * {aka unsigned int *}’ but argument is of type ‘long unsigned int *’
lib/libmbedtls/core/tee_lmd_provider.c:1017:60: error: passing argument 5 of ‘pk_info->decrypt_func’ from incompatible pointer type [-Werror=incompatible-pointer-types]
   lmd_res = pk_info->decrypt_func(&rsa, src, src_len, buf, &blen,
                                                            ^
lib/libmbedtls/core/tee_lmd_provider.c:1017:60: note: expected ‘size_t * {aka unsigned int *}’ but argument is of type ‘long unsigned int *’

edison-ai · 2018-06-26T05:53:59Z

@jenswi-linaro For the second compiling error, I cannot reappear it on my side. Is it relative with gcc version? Which version of gcc you are using?

jenswi-linaro · 2018-06-26T16:54:14Z

It's a type mismatch, blen should be of type size_t instead.

edison-ai · 2018-06-29T02:52:10Z

@jenswi-linaro Update, I had fix the 3 issue mentioned above.

jenswi-linaro · 2018-06-29T06:34:12Z

With 6e954a6 ("core: add new RNG implementation") is core/crypto/rng_*.c responsible to provide RNG. Crypto libraries should themselves use crypto_rng_read() to get random when needed. So "libmbedtls: support mbedtls PRNG function" isn't needed any longer. I suppose some internal wrapper function in mbedtls will be needed to use crypto_rng_read() instead of what's used previously.

jenswi-linaro

With my comment addressed, for these commits
907b555 ("libmbedtls: support mbedtls hash algorithm")
9fc0bb5 ("libmbedtls: support mbedTLS in kernel mode")
33cffad ("libmbedtls: configure mbedTLS for different modes")
please apply:
Reviewed-by: Jens Wiklander <[email protected]>

jenswi-linaro · 2018-06-29T06:35:08Z

lib/libmbedtls/core/tee_lmd_provider.c

+/******************************************************************************
+ * Pseudo Random Number Generator
+ ******************************************************************************/
+TEE_Result __weak crypto_rng_read(void *buf __unused, size_t blen __unused)


This function doesn't belong ere any more.

Yes, I will remove PRNG patch.

jenswi-linaro · 2018-06-29T06:53:45Z

ae549da ("libmbedtls: add interfaces in mbedtls for context memory operation")
is a change to upstream and need to go via the branch import/mbedtls-2.6.1 for proper bookkeeping, I'll take care of that when we're merging the rest of these changes (I hope that you will drive the work to try to upstream this though).
As an example check out
c6672fd ("libmbedtls: refine mbedtls license header")

jenswi-linaro

Comments for "libmbedtls: support symmetrical ciphers"

jenswi-linaro · 2018-06-29T07:02:46Z

lib/libmbedtls/core/tee_lmd_provider.c

+/*
+ * Get the Mbedtls chipher info given a TEE Algorithm "algo"
+ * Return
+ * - TEE_SUCCESS in case of success,


A pointer to a valid mbedtls_cipher_definition_t on success

Yes, I will fix it.

jenswi-linaro · 2018-06-29T07:03:06Z

lib/libmbedtls/core/tee_lmd_provider.c

+#if defined(_CFG_CRYPTO_WITH_CIPHER) || defined(_CFG_CRYPTO_WITH_MAC) || \
+	defined(_CFG_CRYPTO_WITH_AUTHENC)
+/*
+ * Get the Mbedtls chipher info given a TEE Algorithm "algo"


s/chipher/cipher/

Yes, I will fix it.

jenswi-linaro · 2018-06-29T07:04:02Z

lib/libmbedtls/core/tee_lmd_provider.c

+static const mbedtls_cipher_info_t *get_cipher_info(uint32_t algo,
+						size_t key_len)
+{
+	/* Only support key_length is 128 bits of AES in Optee_os */


I can't make sense of this comment.

This comment should be deleted. I will fix it.

jenswi-linaro · 2018-06-29T07:29:15Z

lib/libmbedtls/core/tee_lmd_provider.c

+	switch (algo) {
+#if defined(CFG_CRYPTO_ECB)
+	case TEE_ALG_AES_ECB_NOPAD:
+		cipher_info = get_cipher_info(algo, 128);


Why 128 as key size?

It needs cipher_info to allocate context operation for mbedTLS cipher ctx. For getting cipher info, it need to know the key size. But the key size is not provided yet, it will be provided in init function. So, here, we only use 128 as a default key length. It will be ok, for the mbedtls_cipher_base_t(used for allocate context) are same for all AES operate(different key length). I will add some comments in here.

jenswi-linaro · 2018-06-29T07:30:52Z

lib/libmbedtls/core/tee_lmd_provider.c

+	if (cipher_info == NULL)
+		return TEE_ERROR_NOT_SUPPORTED;
+
+	lmd_res = mbedtls_cipher_setup_info(ctx, cipher_info);


Now we're changing cipher_info, this deserves a comment explaining why and how it's safe.

jenswi-linaro · 2018-06-29T07:45:53Z

lib/libmbedtls/core/tee_lmd_provider.c

+			EMSG("update failed, res is 0x%x", -lmd_res);
+			return TEE_ERROR_BAD_STATE;
+		}
+		lmd_res = mbedtls_cipher_finish(ctx, dst + olen, &finish_olen);


Please add a comment explaining why we're doing mbedtls_cipher_reset() and mbedtls_cipher_finish() here, same for TEE_ALG_AES_CTR below.

The comments doesn't answer the "why".

I was looking for something like:

/* * Do the mbedtls_cipher_reset(), mbedtls_cipher_update(), mbedtls_cipher_finish() * sequence to catch and report partial blocks early. */

Yes, I will use the comments as you suggest.

jenswi-linaro · 2018-06-29T08:19:17Z

"libmbedtls: implement two AES interfaces" need to be split up in two separate commits, one with the changes outside mbedtls and one with the stuff inside tee_lmd_provider.c. If you rather fold the tee_lmd_provider.cchanges into some other suitable commit or not is up to you.

jenswi-linaro

Comments for "libmbedtls: support mbedtls HMAC algorithm"

jenswi-linaro · 2018-06-29T08:25:27Z

lib/libmbedtls/core/tee_lmd_provider.c

+	case TEE_ALG_AES_CMAC:
+		res = TEE_ERROR_NOT_SUPPORTED;
+		break;
+		break;


Redundant break

Yes, I will fix it.

jenswi-linaro · 2018-06-29T08:30:16Z

lib/libmbedtls/core/tee_lmd_provider.c

+	if (res == TEE_SUCCESS)
+		*ctx_ret = ctx;
+	else
+		crypto_mac_free_ctx(ctx, algo);


I don't think it's safe to call this function unless mbedtls_md_setup() has succeeded.

The mbedtls_md_free will check if the freed pointer is valid or not before free it. So it does not matter to do that.

If the second allocation fails, then we'll have a double free.

Second allocation inside mbedtls_md_setup(), that is.

Yes, sorry for not reading code clearly.

jenswi-linaro · 2018-06-29T08:33:16Z

lib/libmbedtls/core/tee_lmd_provider.c

+		res = TEE_ERROR_NOT_SUPPORTED;
+	}
+
+	if (res == TEE_SUCCESS)


I'd prefer doing the epilogue of this function as:

*ctx_ret = ctx; return TEE_SUCCESS; err: free(ctx); return res;

It's more like it's done in the Linux kernel and also at each point where we break out we know if it's because of an error or not so doing "goto err" wouldn't be harder.

Ok, we can do like that. But I still want to use crypto_mac_free_ctx() for err. Because there are may allocate operation in mbedtls_md_setup(), it maybe failed in middle process. So we need to call crypto_mac_free_ctx() to free allocated memory.

So, as discuss above, we should use free(ctx) here.
But I find another issue, caller will call crypto_mac_free_ctx() if crypto_mac_alloc_ctx() failed. it will cause double free for ctx, look this example:

optee_os/core/tee/tee_cryp_hkdf.c

Lines 37 to 62 in af8149d

res = crypto_mac_alloc_ctx(&ctx, hmac_algo);

if (res)

goto out;

/*

* RFC 5869 section 2.1: "Note that in the extract step, 'IKM' is used

* as the HMAC input, not as the HMAC key."

* Therefore, salt is the HMAC key in the formula from section 2.2:

* "PRK = HMAC-Hash(salt, IKM)"

*/

res = crypto_mac_init(ctx, hmac_algo, salt, salt_len);

if (res != TEE_SUCCESS)

goto out;

res = crypto_mac_update(ctx, hmac_algo, ikm, ikm_len);

if (res != TEE_SUCCESS)

goto out;

res = crypto_mac_final(ctx, hmac_algo, prk, *prk_len);

if (res != TEE_SUCCESS)

goto out;

res = tee_hash_get_digest_size(hash_algo, prk_len);

out:

crypto_mac_free_ctx(ctx, hmac_algo);

return res;

Calling crypto_mac_free_ctx() on a ctx being NULL is harmless, it's even preferred over testing if ctx is NULL or not before calling the function.

If I would have written that function, I would have returned on the crypto_mac_alloc_ctx() error instead of doing a goto. However, this is a matter of taste so we leave that to the author.

You're free to do the function epilogue as you like, I just think what I proposed instead is less complicated.

Ok, got, thanks for your explanation.

jenswi-linaro

Comments for "libmbedtls: support CMAC algorithm"

jenswi-linaro · 2018-06-29T08:48:27Z

lib/libmbedtls/core/tee_lmd_provider.c

+		if (!cipher_info)
+			return TEE_ERROR_NOT_SUPPORTED;
+
+		lmd_res = mbedtls_cipher_setup_info(ctx, cipher_info);


Please comment why it's safe to change cipher_info here.

jenswi-linaro · 2018-06-29T08:48:57Z

lib/libmbedtls/core/tee_lmd_provider.c

@@ -1138,8 +1143,22 @@ TEE_Result crypto_mac_alloc_ctx(void **ctx_ret, uint32_t algo)
 #endif
 #if defined(CFG_CRYPTO_CMAC)
 	case TEE_ALG_AES_CMAC:
-		res = TEE_ERROR_NOT_SUPPORTED;
-		break;
+		cipher_info = get_cipher_info(TEE_ALG_AES_ECB_NOPAD, 128);


Why keysize 128?

See the comment as above.

jenswi-linaro

Comments for "libmbedtls: support mbedtls bignum functions"

jenswi-linaro · 2018-06-29T09:14:31Z

lib/libmbedtls/core/tee_lmd_provider.c

+	assert(b != NULL);
+	ret = mbedtls_mpi_cmp_mpi((const mbedtls_mpi *)a,
+		(const mbedtls_mpi *)b);
+	if (ret < 0)


This is where CMP_TRILEAN() is handy, compare (pardon the pun) with:
return CMP_TRILEAN(ret, 0);

Yes, it will be better. I will fix it.

jenswi-linaro · 2018-06-29T09:16:15Z

lib/libmbedtls/core/tee_lmd_provider.c

+	assert(a != NULL);
+	assert(b != NULL);
+	ret = mbedtls_mpi_cmp_mpi((const mbedtls_mpi *)a,
+		(const mbedtls_mpi *)b);


Please align with the opening ( above.

Yes, I will fix it.

jenswi-linaro · 2018-06-29T09:24:05Z

lib/libmbedtls/core/tee_lmd_provider.c

+		return NULL;
+	mbedtls_mpi_init(bn);
+	if (mbedtls_mpi_grow(bn, BITS_TO_LIMBS(size_bits)) != 0)
+		return NULL;


bn leakage

Sorry, I do not understand why you say it.
mbedtls_mpi_grow() will allocate a new buffer if the older is not enough, and swap them.

bn is just allocated with calloc(), if we return NULL here without freeing bn we have a memory leak.

Yes, I will add free(bn) here.

jenswi-linaro · 2018-06-29T09:25:39Z

lib/libmbedtls/core/tee_lmd_provider.c

-struct bignum *crypto_bignum_allocate(size_t size_bits __unused)
+#define ciL		(sizeof(mbedtls_mpi_uint))	/* chars in limb  */
+#define biL		(ciL << 3)			/* bits  in limb  */
+#define biH		(ciL << 2)			/* half limb size */


This define seems unused

Actually, only biH is unused and I will delete it. ciL and biL are used in BITS_TO_LIMBS.

jenswi-linaro · 2018-06-29T11:18:49Z

Bignum allocations should use the <mempool.h> interface to be able to guarantee that symmetric cipher operations don't fail due to out of memory. The normal heap used via malloc() and friends can't make such guarantees. This guarantee is a requirement for GP compliance.

jenswi-linaro

Some comments for "libmbedtls: support mbedtls acipher RSA function"

jenswi-linaro · 2018-06-29T11:00:38Z

lib/libmbedtls/core/tee_lmd_provider.c

+	lmd_res = mbedtls_rsa_public(&rsa, buf, buf);
+	switch (lmd_res) {
+	case MBEDTLS_ERR_RSA_BAD_INPUT_DATA:
+		EMSG("mbedtls_rsa_public() returned 0x%x", -lmd_res);


This is a debug print that if kept should be made with FMSG

jenswi-linaro · 2018-06-29T11:00:54Z

lib/libmbedtls/core/tee_lmd_provider.c

+		break;
+	default:
+		/* This will result in a panic */
+		EMSG("mbedtls_rsa_public() returned 0x%x", -lmd_res);


FMSG, if kept at all

There are many EMSG in this PR, do I need to change all of them to FMSG?

All which are like this one.

jenswi-linaro · 2018-06-29T11:01:11Z

lib/libmbedtls/core/tee_lmd_provider.c

+	case 0:
+		break;
+	default:
+		/* This will result in a panic */


panic in the calling TA

Split mbedTLS into 3 partitions: CRYPTO, X509 and TLS. CRYPTO is for kernel and user mode. X509 and TLS are mainly for user mode. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

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]>

This patch will be disscuss in branch "import/mbedtls-2.6.1" first. In NO_CRT mode, Q and P may be invalid. But Q and P will be re-filled again if PRNG function is valid. So add judgement process if it is in NO_CRT mode. Signed-off-by: Summer Qin <[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]>

This patch will be disscuss in branch "import/mbedtls-2.6.1" first. 1. Add clone interface for GCM and CCM to support optee os. 2. Split ccm_auth_crypt() into 3 interfaces: mbedtls_ccm_starts(), mbedtls_ccm_update(), mbedtls_ccm_finish(). They are used to support optee interfaces. mbedTLS selftest and test_suite_ccm are all pass. Signed-off-by: Edison Ai <[email protected]>

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

1. Split tomcrypt into two partitions, while tomcrypt_lite will include the algos that mbedtls does not support. 2. Rename some crypto interfaces for distinguish them with those in crypto.c. Signed-off-by: Edison Ai <[email protected]>

jbech-linaro · 2018-08-31T10:31:45Z

In IBART there are various aborts happening so it never ends the test suite and then after 600 seconds it time out and the test fails. I've tracked this PR and I've seen the same issue showing up, but for other PR's it is not happening so I suspect this would be reproducible on a local build also.

edison-ai · 2018-09-06T01:59:30Z

@jbech-linaro Thanks for your information, I will try to reproduce it locally.

edison-ai · 2018-09-23T14:55:58Z

@jenswi-linaro @jforissier , Sorry not to update it as soon as possible. for I am rushing for another things. I had try several time on local with newest optee os, client and test "master" breach, and all test are passed with no errors. So I have no idea for this "IBART" failed now. Do you have any suggest?

jenswi-linaro · 2018-10-19T09:06:24Z

@edison-ai, are you still stuck on IBART?

edison-ai · 2018-10-19T14:02:54Z

@jenswi-linaro Yes, I cannot reproduce the error on my side. So, if we can start the review process, and to see if can fix the IBART problem later?

jenswi-linaro · 2018-10-19T14:18:29Z

So it's ready to be reviewed again (ignoring IBART)?

edison-ai · 2018-10-20T14:01:15Z

@jenswi-linaro Yes,I had refine all the patches according your last comment. So you can continue to review them.

jenswi-linaro · 2018-10-29T11:53:40Z

For "libmbedtls: support mbedtls ccm function" please apply:
Reviewed-by: Jens Wiklander <[email protected]>

jenswi-linaro · 2018-10-29T12:11:07Z

Comments for "libmbedtls: support mbedtls unsupported algorithms":

libtomcrypt_lite is entirely an mbedTLS internal thing, there's no need to create a crypto_lite api or anything like that.
If mbedTLS isn't used then libtomcrypt_lite isn't either.
There's no changes needed in core/lib/libtomcrypt/src/tee_ltc_provider.c

jenswi-linaro · 2018-10-29T12:13:07Z

If not not mistaken then only thing left in the PR is to fix "libmbedtls: support mbedtls unsupported algorithms".

Then we can take care of the stuff to merge in the branch import/mbedtls-2.6.1 and finally merge this.

edison-ai · 2018-12-13T06:15:54Z

@jenswi-linaro Sorry for not reply this so long time because I am blocked by other things.
For libtomcrypt_lite, of course, if mbedTLS isn't used then libtomcrypt_lite isn't either. So, in your opinion, I should to call tomcrypt functions directly in tee_lmd_provider.c and only build the relative tomcrypt files?
Do you mean I should push those patches on import/mbedtls-2.6.1 now?

jenswi-linaro · 2018-12-13T08:14:25Z

For libtomcrypt_lite, of course, if mbedTLS isn't used then libtomcrypt_lite isn't either. So, in your opinion, I should to call tomcrypt functions directly in tee_lmd_provider.c and only build the relative tomcrypt files?

Yes, that would probably be the best. I think we can accept some shortcuts here as I assume that mbedtls sooner or later will have the missing features.

Do you mean I should push those patches on import/mbedtls-2.6.1 now?

Please create a pull request against import/mbedtls-2.6.1 for the patches that should first go into that branch.

Also, please apply the last tags and rebase on master.

edison-ai · 2018-12-13T08:43:21Z

OK, I will try to do as it and create a new PR to import/mbedtls-2.6.1.

edison-ai · 2018-12-17T08:34:00Z

Move this PR to #2705. Close it now.

jenswi-linaro requested changes May 23, 2018

View reviewed changes

edison-ai force-pushed the support-mbedtls branch from 207b928 to dbd59ea Compare May 24, 2018 06:46

jenswi-linaro reviewed May 28, 2018

View reviewed changes

edison-ai force-pushed the support-mbedtls branch from dbd59ea to 58522ba Compare June 22, 2018 06:57

edison-ai force-pushed the support-mbedtls branch from 58522ba to dfb2f39 Compare June 22, 2018 07:47

jenswi-linaro reviewed Jun 25, 2018

View reviewed changes

edison-ai force-pushed the support-mbedtls branch from dfb2f39 to 3909a19 Compare June 29, 2018 02:50

jenswi-linaro reviewed Jun 29, 2018

View reviewed changes

libmbedtls: configure mbedTLS for different modes

040dda5

Split mbedTLS into 3 partitions: CRYPTO, X509 and TLS. CRYPTO is for kernel and user mode. X509 and TLS are mainly for user mode. Signed-off-by: Edison Ai <[email protected]> Reviewed-by: Jens Wiklander <[email protected]>

Edison Ai and others added 13 commits August 30, 2018 18:19

libmbedtls: support symmetrical ciphers

ee5ee19

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

2ebc5ad

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

5d71f5b

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

1718730

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

libmbedtls: support CMAC algorithm

5f00cde

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

364e297

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

8fe65b7

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

6fe32e1

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

3a69c1b

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

libmbedtls: support mbedtls ccm function.

1432307

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

libmbedtls: support mbedtls unsupported algorithms

c7d0bc4

1. Split tomcrypt into two partitions, while tomcrypt_lite will include the algos that mbedtls does not support. 2. Rename some crypto interfaces for distinguish them with those in crypto.c. Signed-off-by: Edison Ai <[email protected]>

edison-ai force-pushed the support-mbedtls branch from a15ac30 to c7d0bc4 Compare August 30, 2018 11:07

edison-ai mentioned this pull request Dec 17, 2018

Support mbedtls crypto library #2705

Closed

edison-ai closed this Dec 17, 2018

		@@ -0,0 +1,3 @@
		cflags-lib-$(CFG_CRYPTO_SIZE_OPTIMIZATION) += -Os

		subdirs-y += src

	res = crypto_mac_alloc_ctx(&ctx, hmac_algo);
	if (res)
	goto out;

	/*
	* RFC 5869 section 2.1: "Note that in the extract step, 'IKM' is used
	* as the HMAC input, not as the HMAC key."
	* Therefore, salt is the HMAC key in the formula from section 2.2:
	* "PRK = HMAC-Hash(salt, IKM)"
	*/
	res = crypto_mac_init(ctx, hmac_algo, salt, salt_len);
	if (res != TEE_SUCCESS)
	goto out;

	res = crypto_mac_update(ctx, hmac_algo, ikm, ikm_len);
	if (res != TEE_SUCCESS)
	goto out;

	res = crypto_mac_final(ctx, hmac_algo, prk, *prk_len);
	if (res != TEE_SUCCESS)
	goto out;

	res = tee_hash_get_digest_size(hash_algo, prk_len);
	out:
	crypto_mac_free_ctx(ctx, hmac_algo);
	return res;

Support mbedtls crypto library #2345

Support mbedtls crypto library #2345

Conversation

edison-ai commented May 23, 2018

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 commented May 24, 2018

jforissier commented May 24, 2018

edison-ai commented May 25, 2018

jenswi-linaro May 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edison-ai commented Jun 19, 2018 • edited Loading

jenswi-linaro commented Jun 19, 2018

edison-ai commented Jun 20, 2018

edison-ai commented Jun 22, 2018

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

jenswi-linaro commented Jun 25, 2018

jenswi-linaro commented Jun 25, 2018

edison-ai commented Jun 26, 2018

jenswi-linaro commented Jun 26, 2018

edison-ai commented Jun 29, 2018

jenswi-linaro commented Jun 29, 2018

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

jenswi-linaro commented Jun 29, 2018

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

jenswi-linaro commented Jun 29, 2018

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

edison-ai Jul 3, 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 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

jenswi-linaro May 28, 2018 •

edited

Loading

edison-ai commented Jun 19, 2018 •

edited

Loading

edison-ai Jul 3, 2018 •

edited

Loading