EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) #5800

polhenarejos · 2022-05-02T14:37:33Z

Description

This PR implements EdDSA signatures, with Ed25519 and Ed448 curves. It supports Pure, Context and Prehash types, resulting onto ed25519, ed25519ctx, ed25519ph, ed448 and ed448ph. I followed the RFC 8032, which specifies the procedure for signing and verifying. It also contains verification.

Ed448 uses SHAKE256 function, which is also added. Note that there are no specific CPU optimizations.

I added test suites for EdDSA and SHAKE256 with the test vectors provided in the RFC.

I took the PR from @aurel32, which defined the curves and arithmetic (I fixed some bugs), from #2452.

About SHA3

During this journey, I started with SHAKE256, since it is necessary for Ed448. However, SHAKE and SHA3 functions are very similar and I extended the sha3 module to include SHA-3, SHAKE, cSHAKE and KMAC (with and without XOF).
Ideally I would make two separate PR, but since EdDSA and SHA-3 are closely dependent I put altogether in this PR.
SHA-3 and SHAKE are based on FIPS 202. cSHAKE and KMAC are based on SP800-185. Test vectors are taken from NIST repository.
Note: it will only work for architectures that support real qwords (i.e., 64 bits). On smaller CPU Keccak permutation function has to be recompiled with other parameters.

Status

READY

Requires Backporting

NO

Migrations

NO

Additional comments

This PR does not contain restarting capability, but it can be extended easily.

Note that this PR obsoletes #3245, since it is contained here and it was not merged.

Todos

Splitted PR

This PR is being splitted into the following PR:

Related issues

Largely inspired by the curve25519 definition. Signed-off-by: Aurelien Jarno <[email protected]>

Largely inspired by the curve448 definition. Signed-off-by: Aurelien Jarno <[email protected]>

Also tweak mbedtls_ecp_get_type to handle the edwards case. FIXME: there is probably a cleaner way of doing that. Signed-off-by: Aurelien Jarno <[email protected]>

The RFC8032 recommends to implement ed25519 in extended coordinates and ed448 in projective coordinates. The two coordinates systems are very close with T=x*y for extended coordinates. The corresponding point addition and point doubling formulas are actually very similar. Note that the formulas given in the RFC are the EFD one simplified with a = -1 for ed25519 and a = 1 for ed448. The Mbed TLS philosophy is to share as much code as possible, so projective coordinates are used for both Twisted Edwards and Edwards curves, using the formulas from EFD, which do include the a parameter of the quation. This also matches the mbedtls_ecp_point implementation which supports up to three coordinates. In order to verify EdDSA signatures, it is necessary to add two points. A new mbedtls_ecp_add function is therefore exported in the API. It is only implemented for Edwards curve and returns MBEDTLS_ERR_ECP_FEATURE_UNAVAILABLE for other curves. The ecp_check_pubkey function simply checks that the points is on the curve as there are no recommended checks in the RFC8032 nor in the FIPS 186-5 draft. FIXME: There is currently no check for mbedtls_ecp_check_privkey. It's not clear what check should be implemented. FIXME: mbedtls_ecp_set_zero and mbedtls_ecp_is_zero are wrong for Edwards curve, the neutral element being (0, 1). This is not possible to fix that without changing the API as the group parameter is currently not in argument. FIXME: ecp_normalize_edxyz uses a modular inversion. For elliptic curves over GF(p) modular inversions could be done using x^-1 = x^(p-2) (mod p). As it doesn't seem to bring any measureable speed improvement, I kept the modular inversion. FIXME: The mbedtls_ecp_keypair type and the related functions (mbedtls_ecp_gen_key, mbedtls_ecp_read_key, mbedtls_ecp_check_pub_priv) assume that the secret key is a scalar and that the public key is a curve point that is obtained by the scalar multiplication of the base point by the secret key. This is only partially true for the EdDSA algorithm, which employs point enconding and hashes to manipulate those. Signed-off-by: Aurelien Jarno <[email protected]>

Those tests are extracted from RFC8032 sec 7 and correspond to the unique SECRET KEY and PUBLIC KEY tuples in the Ed25519 and Ed448 test vectors. The eddsa2 Python library defined from Appendix A is used to expand the SECRET KEY entries to a scalar value and to decode the PUBLIC KEY entries to a point value. Signed-off-by: Aurelien Jarno <[email protected]>

Note that decoding a point for curves over prime field p with p = 1 (mod 8) is not supported. This is not the case of Ed25519 and Ed448. Supporting that would require a more complex algorithm to compute the square root, for instance the Tonelli-Shanks algorithm. Note that the square root formulas come from the RFC Erratas 5758 and 5759. FIXME: mbedtls_ecp_point_read_binary_ed() has to be defined after the mbedtls_mpi_xxx_mod functions as some computation are needed to read a point. Currently mbedtls_ecp_point_read_binary() is defined before those functions, so a forward declaration is needed. This should probably be fixed by reshuffling functions in ecp.c. Signed-off-by: Aurelien Jarno <[email protected]>

…el32-edwards Signed-off-by: Pol Henarejos <[email protected]> # Conflicts: # library/ecp.c # library/version_features.c # programs/test/query_config.c # tests/suites/test_suite_ecp.data

Signed-off-by: Pol Henarejos <[email protected]>

Added method to derive public point from private ed25519 point. Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

EdDSA by definition is always deterministic. So, I do not distinguish between DET versions. This version does not support restarting functions. Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

…ably also affects to curve448. Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/secure-hashing Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

gilles-peskine-arm · 2022-05-04T09:28:10Z

@polhenarejos The Arm Jenkins instance is not accessible externally, so if you need to see logs, please ask here. We're in the process of moving to OpenCI, which is publicly visible. So far OpenCI is running the same Linux and FreeBSD jobs, but not Windows yet.

On 525ec1f, builds with MSVC are failing due to this error:

C:\builds\workspace\mbed-tls-pr-head_PR-5800-head\src\library\sha3.c(266): error C2220: warning treated as error - no 'object' file generated [C:\builds\workspace\mbed-tls-pr-head_PR-5800-head\src\library\mbedcrypto.vcxproj]
C:\builds\workspace\mbed-tls-pr-head_PR-5800-head\src\library\sha3.c(266): warning C4244: '=' : conversion from 'uint64_t' to 'unsigned char', possible loss of data [C:\builds\workspace\mbed-tls-pr-head_PR-5800-head\src\library\mbedcrypto.vcxproj]

Signed-off-by: Pol Henarejos <[email protected]>

Also added NIST test vectors. Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

Added test vectors for SHA-3 KMAC. Signed-off-by: Pol Henarejos <[email protected]>

Signed-off-by: Pol Henarejos <[email protected]>

polhenarejos · 2022-05-05T06:52:49Z

Thanks @gilles-peskine-arm for the comments and suggestions. PR passes all checks and I think the PR includes all necessary to run EdDSA and SHA3 hashing. Please let me know if it is still missing something or if I can do something else to speed up the merge.

gilles-peskine-arm · 2022-05-05T08:49:12Z

@polhenarejos Thank you very much! And wow, I blinked and now there's the whole SHA3 family? Wow!

Unfortunately our bottleneck is review bandwidth. As this is a very large contribution, we need to schedule it, and I'm afraid we have no room until late 2022.

polhenarejos · 2022-05-05T13:35:28Z

Sure! Once you have shake working is straightforward to add sha3, cshake or kmac.

In any case, if you think I can help for reviewing to mitigate the bottleneck, tell it to me and I will be glad to give a hand.

gilles-peskine-arm · 2022-05-06T19:00:02Z

We would gladly welcome your help with reviews! While I can't promise anything right now, we do normally prioritize the reviews of contributions from people who are also reviewing. If you're willing, please read a presentation to set the mood and our review guidelines. Pull requests normally need two approvals from trusted reviewers.

Pull requests looking for a reviewer should have the "needs: reviewer" label. They should also be on the review board. We tend to prioritize pull requests that already have one approval and so are in the "Has Approval" column.

I'm guessing that your interest is mainly on cryptographic code, so here are a few suggestions:

An ongoing series of ECC performance improvements. (I'm not sure which ones are on the critical path. We're in the process of adding priority labels, so anything labelled "priority: high" should come first.)
LMS, the first post-quantum scheme that's going into Mbed TLS, which is somewhat of a business priority for Arm. Both David and I have started to review it, but if you'd review the core cryptography part, I could focus on the library integration and I'd have more time for something else.
PKCS#7 parsing (and its follow-up generation. If you're willing to take this one, please check with us first so we can find a second reviewer around the same time.

gilles-peskine-arm · 2022-05-06T19:13:13Z

Speaking of reviews, would you mind splitting this pull request? It's very large, which makes it difficult to find two reviewers who will have enough time in a reasonably contiguous period. We've never managed to integrate the previous SHA3 contribution originally submitted in 2016... (By the way, did you reuse that for SHA3? Does your code cover the same ground?)

Would you mind splitting this into at least three parts?

An EdDSA interface, implemented for Ed25519.
SHA3.
Extend EdDSA to Ed448.
SHAKE (and possibly cSHAKE and KMAC because they are really close).

We'd planned an even finer breakdown, but since you've already done the work, I think this split would be fine.

polhenarejos · 2022-05-06T20:49:30Z

Honestly, I did not review other contributions for SHA-3 #1549 nor #479, as my intended PR was originally to provide Edwards signatures (and this is why I only added shake256). By a quick overview, I think both do the same but I also add cSHAKE and KMAC (with less code lines). I do not interface each keccak intermediate function because I think this is not useful for anyone and even for the entire keccak-f, which is declared static.

If it may help, I do no have any objection on splitting in 4 parts. Do you mean submitting as many PR covering each point?

and 2) are fine, but 3) requires 4) (Ed448 is defined for shake256 rather than sha3). So, the logic sort would be ed25519+shake256 (and 128)+ed448+sha3+cshake+kmac. Another sort could be ed25519+sha3+shake256/128+edd48+cshake+kmac, depending on how you would prioritize sha3 over ed448 or viceversa. I do not have preferences on that, so whatever you decide is fine to me.

gilles-peskine-arm · 2022-05-06T20:59:39Z

Do you mean submitting as many PR covering each point?

Yes, please. Or split differently if you think that's more meaningful. I think this is a reasonable split in terms of manageable review size. (That's with code that's already in a good shape. In general we'd ask for even smaller pieces, to reduce the amount of rework if there are a lot of review comments, but I can see that your code is already in a very good shape so I don't expect major difficulties.)

and 2) are fine, but 3) requires 4)

Yes, sorry, I meant SHA3 then SHAKE then (in either order) Ed448 and the rest of the SHA3 family. Or you could go directly for SHAKE then Ed448 and rest of SHA3 family including SHA3 itself.

polhenarejos · 2022-05-08T23:46:56Z

I added PR:

but I am not sure that they are completely splitted. #5823 adds KMAC support but it also contains the previous commits from the rest of PR.

In addition to this, I do not know how to add a separated PR for Ed448, as it depends on #5819 and #5821. Which is the best approach to do not repeat previous commits?

polhenarejos · 2022-05-09T07:59:18Z

I close this PR as it has been splitted to #5819, #5820, #5821, #5822, #5823 and #5824.

aurel32 and others added 30 commits April 27, 2020 14:48

Add edwards25519 curve as defined in RFC7748

a29a565

Largely inspired by the curve25519 definition. Signed-off-by: Aurelien Jarno <[email protected]>

Add edwards448 curve as defined in RFC7748

d949b06

Largely inspired by the curve448 definition. Signed-off-by: Aurelien Jarno <[email protected]>

Add MBEDTLS_ECP_TYPE_EDWARDS to mbedtls_ecp_curve_type

2785a30

Also tweak mbedtls_ecp_get_type to handle the edwards case. FIXME: there is probably a cleaner way of doing that. Signed-off-by: Aurelien Jarno <[email protected]>

Merge branch 'edwards' of https://github.com/aurel32/mbedtls into aur…

ff1b57e

…el32-edwards Signed-off-by: Pol Henarejos <[email protected]> # Conflicts: # library/ecp.c # library/version_features.c # programs/test/query_config.c # tests/suites/test_suite_ecp.data

Some fixes.

19a4ee1

Signed-off-by: Pol Henarejos <[email protected]>

Merge branch 'aurel32-edwards' into eddsa

1e3b722

Adding number of bits of Edwards curves.

10bbbad

Signed-off-by: Pol Henarejos <[email protected]>

Added macro to the header.

eec9fcd

Added method to derive public point from private ed25519 point. Signed-off-by: Pol Henarejos <[email protected]>

Added method to derive public point from private ed25519 point.

ed3725b

Signed-off-by: Pol Henarejos <[email protected]>

Modifying names for better homogeneity.

c4b9d6e

Signed-off-by: Pol Henarejos <[email protected]>

Adding EdDSA procedures. At this moment, only genkey and sign.

df08b41

EdDSA by definition is always deterministic. So, I do not distinguish between DET versions. This version does not support restarting functions. Signed-off-by: Pol Henarejos <[email protected]>

SHA512 is only used by Ed25519 curve.

40feb3e

Signed-off-by: Pol Henarejos <[email protected]>

Adding EdDSA verification for Ed25519.

59d21d4

Signed-off-by: Pol Henarejos <[email protected]>

Adding SHAKE256 hash functions.

0465142

Signed-off-by: Pol Henarejos <[email protected]>

FIX: Ed448 needs setting N to 0 before setting the proper value. Prob…

958094a

…ably also affects to curve448. Signed-off-by: Pol Henarejos <[email protected]>

Adding point compression to Ed448-

34c2aef

Signed-off-by: Pol Henarejos <[email protected]>

Adding signature and verification for Ed448.

52da672

Signed-off-by: Pol Henarejos <[email protected]>

Changing error code for shake256.

b2f299e

Signed-off-by: Pol Henarejos <[email protected]>

Adding EDDSA and SHAKE256 modules for error generation.

bf0c9c8

Signed-off-by: Pol Henarejos <[email protected]>

Removing some safe warnings.

afa2070

Signed-off-by: Pol Henarejos <[email protected]>

Adding EdDSA and SHAKE256 to Makefile.

b16315a

Signed-off-by: Pol Henarejos <[email protected]>

Adding test suites for EdDSA.

e00ba91

Signed-off-by: Pol Henarejos <[email protected]>

Adding tests for Ed448 and verification.

dae62fb

Signed-off-by: Pol Henarejos <[email protected]>

Added test suite for SHAKE256 with test vectors from the NIST web.

062fa43

https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/secure-hashing Signed-off-by: Pol Henarejos <[email protected]>

Warning clean.

c12b5cc

Signed-off-by: Pol Henarejos <[email protected]>

Fix checking curve type and operation type.

ccf83a9

Signed-off-by: Pol Henarejos <[email protected]>

Added check in verify following the RFC.

35b9877

Signed-off-by: Pol Henarejos <[email protected]>

Fix casting

2a61634

Signed-off-by: Pol Henarejos <[email protected]>

polhenarejos changed the title ~~EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types~~ EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) May 4, 2022

polhenarejos added 5 commits May 4, 2022 16:05

Added SHA-3 CSHAKE functions.

6b0a7e7

Also added NIST test vectors. Signed-off-by: Pol Henarejos <[email protected]>

Small fixes

cbe5acb

Signed-off-by: Pol Henarejos <[email protected]>

Added NIST SHA-3 KMAC.

ffea1aa

Added test vectors for SHA-3 KMAC. Signed-off-by: Pol Henarejos <[email protected]>

Added KMAC XOF support.

3c7012c

Signed-off-by: Pol Henarejos <[email protected]>

Added test vectors for KMAC XOF.

e782306

Signed-off-by: Pol Henarejos <[email protected]>

gilles-peskine-arm added 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 and removed needs-work labels May 6, 2022

This was referenced May 8, 2022

Add support for EdDSA with ed25519 curve (pure ed25519, ed25519ctx and ed25519ph) #5819

Open

SHA-3 support #5820

Merged

SHA-3 SHAKE128 and SHAKE256 support #5821

Open

SHA-3 cSHAKE128 and cSHAKE256 support #5822

Open

Add support for SHA-3 KMAC #5823

Open

polhenarejos closed this May 9, 2022

rrottmann mentioned this pull request Nov 6, 2022

Are there plans to add support for ed25519? polhenarejos/pico-hsm#12

Closed

polhenarejos deleted the eddsa branch November 14, 2022 08:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) #5800

EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) #5800

polhenarejos commented May 2, 2022 •

edited

Loading

gilles-peskine-arm commented May 4, 2022

polhenarejos commented May 5, 2022

gilles-peskine-arm commented May 5, 2022

polhenarejos commented May 5, 2022

gilles-peskine-arm commented May 6, 2022

gilles-peskine-arm commented May 6, 2022

polhenarejos commented May 6, 2022

gilles-peskine-arm commented May 6, 2022

polhenarejos commented May 8, 2022 •

edited

Loading

polhenarejos commented May 9, 2022

EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) #5800

EdDSA support (Ed25519 and Ed448 curves) with Pure, Context and Prehash types + SHA3 (SHA3, SHAKE, CSHAKE and KMAC) #5800

Conversation

polhenarejos commented May 2, 2022 • edited Loading

Description

About SHA3

Status

Requires Backporting

Migrations

Additional comments

Todos

Splitted PR

Related issues

gilles-peskine-arm commented May 4, 2022

polhenarejos commented May 5, 2022

gilles-peskine-arm commented May 5, 2022

polhenarejos commented May 5, 2022

gilles-peskine-arm commented May 6, 2022

gilles-peskine-arm commented May 6, 2022

polhenarejos commented May 6, 2022

gilles-peskine-arm commented May 6, 2022

polhenarejos commented May 8, 2022 • edited Loading

polhenarejos commented May 9, 2022

polhenarejos commented May 2, 2022 •

edited

Loading

polhenarejos commented May 8, 2022 •

edited

Loading