Add SM3 to mbedtls and PSA Crypto #4492

th0ma5b · 2021-05-12T13:41:42Z

GB/T 32905-2016 compliant SM3 implementation.
The SM3 algorithm was designed by Xiaoyun Wang et al in 2010.

http://www.gmbz.org.cn/upload/2018-07-24/1532401392982079739.pdf

Signed-off-by: Thomas Bourlard [email protected]

paul-elliott-arm · 2021-05-12T13:57:04Z

Hi @th0ma5b - Thankyou for your contribution, this looks very useful.

A couple of points, could you sign off your commits (DCO check failed) and possibly add a changelog entry (https://github.com/ARMmbed/mbedtls/blob/development/ChangeLog.d/00README.md)

Unfortunately this also already requires rebase, but this could be done before or after review.

th0ma5b · 2021-05-13T09:36:30Z

Update:

DCO fixed.
conflicts resolved, rebased and re-tested.
Added a changelog entry.

Can you provide access to error log from CI? (not accessible)

library/sm3.c

ChangeLog.d/add-sm3.txt

library/error.c

yuhaoth · 2021-05-18T10:02:27Z

Update:
* DCO fixed.

* conflicts resolved, rebased and re-tested.

* Added a changelog entry.
Can you provide access to error log from CI? (not accessible)

I put travis CI link in my comment. Please take a look

gilles-peskine-arm

Thank you for your contribution! I have done a first round of review, focusing on the project conventions and structure. I have not looked at the cryptographic code at all.

include/mbedtls/config.h

include/mbedtls/sm3.h

tests/suites/test_suite_psa_crypto.data

include/mbedtls/sm3.h

tests/suites/test_suite_mdx.function

ChangeLog.d/add-sm3.txt

library/sm3.c

scripts/generate_errors.pl

Add psa dispatch code Signed-off-by: Thomas Bourlard <[email protected]>

Signed-off-by: Thomas Bourlard <[email protected]>

Add SM3 tests to test_suite_psa_crypto_metadata.data Add SM3 tests to test_suite_md.data to test dispatch via mbedtls_md_xxx() Create test_suite_sm3 to test low-level mbedtls_sm3_xxx() Remove SM3 test from test_suite_mdx.data Change SM3 test vectors to lower case in test_suite_psa_crypto_hash.data Signed-off-by: Thomas Bourlard <[email protected]>

Remove unused MBEDTLS_ERR_SM3_HW_ACCEL_FAILED Fix Travis CI error reported by -fsanitize=undefined option Reorder MBEDTLS_XXX_ALT in alphabetical order Add reference to SM3 specification in sm3.h Remove MBEDTLS_INTERNAL_VALIDATE_XXX() parameter checks Fix Travis CI error [PSA] Update SM3 #define flags to fix no driver config: (full config + set MBEDTLS_PSA_CRYPTO_CONFIG + unset MBEDTLS_PSA_CRYPTO_DRIVERS + unset MBEDTLS_USE_PSA_CRYPTO) Add SM3 to tests/scripts/all.sh Remove NULL pointer parameter check test Update comment about number of SM3 error Add missing SM3 case in is_hash_accelerated Extend mbedtls SM3 tests Update tests/scripts/depends-hashes.pl with SM3 Signed-off-by: Thomas Bourlard <[email protected]>

Add SM3 to the "good standing" group of hash algos. Signed-off-by: Thomas Bourlard <[email protected]>

Signed-off-by: Thomas Bourlard <[email protected]>

Co-authored-by: Jerry Yu <[email protected]> Signed-off-by: Thomas Bourlard <[email protected]>

Signed-off-by: Thomas Bourlard <[email protected]>

gilles-peskine-arm · 2022-04-05T19:14:05Z

This PR will need a rebase and a small additional patch once #5644 is merged, to add the new algorithm to crypto_knowledge.py.

gaoqingshui · 2022-04-21T01:23:43Z

Hi, what is the status of this MR? I want to use SM3/SM4 in my private project, what can I do to make this MR merged ASAP?

gilles-peskine-arm · 2022-04-21T08:19:08Z

@gaoqingshui This pull request is waiting for review. Review bandwidth is the main bottleneck of the Mbed TLS team, and I'm afraid I can't give a date for when we'll be able to.

If you're willing, you can help the project by becoming a reviewer! Meaning reading the review guidelines, start reviewing other people's pull requests, and reach the point where you're considered a trusted reviewer and so reduce the workload of other reviewers.

gilles-peskine-arm · 2022-10-12T09:58:03Z

library/sm3.c

+#if defined(MBEDTLS_PLATFORM_C)
+#include "mbedtls/platform.h"
+#else
+#include <stdio.h>
+#define mbedtls_printf printf
+#endif /* MBEDTLS_PLATFORM_C */


We've realized that the conditional inclusion of platform.h on MBEDTLS_PLATFORM_C was not a good idea, and we're moving to unconditional inclusion. Please use the new style in this new file.

Suggested change

#if defined(MBEDTLS_PLATFORM_C)

#include "mbedtls/platform.h"

#else

#include <stdio.h>

#define mbedtls_printf printf

#endif /* MBEDTLS_PLATFORM_C */

#include "mbedtls/platform.h"

Or even include mbedtls/platform.h regardless of MBEDTLS_SELF_TEST.

tom-daubney-arm · 2023-07-07T10:56:40Z

We are now converting older PRs to draft PRs where the following conditions are met: They have not been updated in the last 3 months, and they need more than non-trivial work to complete.

I've added "needs-work" since conflicts have now appeared.

th0ma5b force-pushed the sm3 branch from b8e4b8d to 81979a6 Compare May 12, 2021 15:10

th0ma5b force-pushed the sm3 branch from 81979a6 to 8a24292 Compare May 16, 2021 21:11

th0ma5b mentioned this pull request May 17, 2021

Add SM2 curve parameters #4091

Draft

yuhaoth self-requested a review May 17, 2021 10:01

yuhaoth reviewed May 18, 2021

View reviewed changes

library/sm3.c Outdated Show resolved Hide resolved

yuhaoth reviewed May 18, 2021

View reviewed changes

library/sm3.c Outdated Show resolved Hide resolved

yuhaoth reviewed May 18, 2021

View reviewed changes

ChangeLog.d/add-sm3.txt Show resolved Hide resolved

yuhaoth reviewed May 18, 2021

View reviewed changes

library/error.c Outdated Show resolved Hide resolved

gilles-peskine-arm requested changes May 18, 2021

View reviewed changes

gilles-peskine-arm removed component-psa PSA keystore/dispatch layer (storage, drivers, …) needs: changelog needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels May 18, 2021

gilles-peskine-arm reviewed May 19, 2021

View reviewed changes

scripts/generate_errors.pl Show resolved Hide resolved

th0ma5b force-pushed the sm3 branch from 4d07eed to 86a5ff8 Compare May 21, 2021 00:02

Thomas Bourlard and others added 10 commits April 5, 2022 12:34

[PSA] Add SM3 support to PSA Crypto

2e77a3e

Add psa dispatch code Signed-off-by: Thomas Bourlard <[email protected]>

[PSA] Add HMAC-SM3 to PSA Crypto

621ac4f

Signed-off-by: Thomas Bourlard <[email protected]>

[PSA] Add PSA SM3 tests.

5c43d33

Signed-off-by: Thomas Bourlard <[email protected]>

fixup! Fix SM3 review comments and CI test

609484a

Add SM3 to the "good standing" group of hash algos. Signed-off-by: Thomas Bourlard <[email protected]>

fixup! Fix SM3 review comments and CI test

952d156

Signed-off-by: Thomas Bourlard <[email protected]>

fixup! fix-up after rebasing onto mbedtls3 development branch

2d3a003

Signed-off-by: Thomas Bourlard <[email protected]>

Move internal SM3 API to library/sm3_internal.h

4b6a051

Co-authored-by: Jerry Yu <[email protected]> Signed-off-by: Thomas Bourlard <[email protected]>

Replace PUT/GET_UINT32_BE local macros with global ones

a2df8ec

Signed-off-by: Thomas Bourlard <[email protected]>

th0ma5b force-pushed the sm3 branch from d129245 to a2df8ec Compare April 5, 2022 11:35

gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, and removed needs-work needs-ci Needs to pass CI tests labels Apr 5, 2022

gilles-peskine-arm requested a review from yuhaoth April 5, 2022 19:08

daverodgman added priority-scheduled This PR is big - it will require time to be scheduled for review and removed Community labels May 13, 2022

daverodgman added the historical-reviewing Currently reviewing (for legacy PR/issues) label Jun 17, 2022

daverodgman added historical-reviewed Reviewed & agreed to keep legacy PR/issue and removed historical-reviewing Currently reviewing (for legacy PR/issues) labels Jul 1, 2022

gilles-peskine-arm reviewed Oct 12, 2022

View reviewed changes

tom-daubney-arm added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Jul 7, 2023

tom-daubney-arm marked this pull request as draft July 7, 2023 10:57

liyi77 mentioned this pull request Sep 9, 2024

Cryptopkg mbedtls reimplement sm3 tianocore/edk2#6165

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SM3 to mbedtls and PSA Crypto #4492

Add SM3 to mbedtls and PSA Crypto #4492

th0ma5b commented May 12, 2021 •

edited

Loading

paul-elliott-arm commented May 12, 2021

th0ma5b commented May 13, 2021 •

edited

Loading

yuhaoth commented May 18, 2021

gilles-peskine-arm left a comment

gilles-peskine-arm commented Apr 5, 2022

gaoqingshui commented Apr 21, 2022

gilles-peskine-arm commented Apr 21, 2022

gilles-peskine-arm Oct 12, 2022 •

edited

Loading

tom-daubney-arm commented Jul 7, 2023

Add SM3 to mbedtls and PSA Crypto #4492

Are you sure you want to change the base?

Add SM3 to mbedtls and PSA Crypto #4492

Conversation

th0ma5b commented May 12, 2021 • edited Loading

paul-elliott-arm commented May 12, 2021

th0ma5b commented May 13, 2021 • edited Loading

yuhaoth commented May 18, 2021

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm commented Apr 5, 2022

gaoqingshui commented Apr 21, 2022

gilles-peskine-arm commented Apr 21, 2022

gilles-peskine-arm Oct 12, 2022 • edited Loading

Choose a reason for hiding this comment

tom-daubney-arm commented Jul 7, 2023

th0ma5b commented May 12, 2021 •

edited

Loading

th0ma5b commented May 13, 2021 •

edited

Loading

gilles-peskine-arm Oct 12, 2022 •

edited

Loading