Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

contextualized extension of the Ed25519 scheme #597

Merged
merged 4 commits into from
Aug 21, 2022
Merged

contextualized extension of the Ed25519 scheme #597

merged 4 commits into from
Aug 21, 2022

Conversation

sa-kib
Copy link

@sa-kib sa-kib commented Aug 16, 2022

This PR adds Ed25519 context support and message pre-hashing support when signin/verifying.

Checklist

  • documentation is updated
  • tests are added

@sjaeckel
Copy link
Member

87fbc59 is valid but I think we should keep the tweetnacl code as is (as much as possible). IMO a better fix would be to use msglen + siglen in the call of zeromem() inside ed25519_verify_private().

@sjaeckel
Copy link
Member

@sa-kib thanks for the PR!

Please check the changes I did and confirm whether this is fine for you.

I'll rebase&squash then as necessary, force-push to your fork and will merge this PR afterwards.

@sjaeckel sjaeckel force-pushed the ed25519ctx branch 2 times, most recently from 45f4f60 to 83dfc2c Compare August 17, 2022 09:58
@sjaeckel sjaeckel mentioned this pull request Aug 17, 2022
Merged
@sa-kib
Copy link
Author

sa-kib commented Aug 18, 2022

@sjaeckel thank you for taking a look!
I'm fine with the changes you've made (we weren't sure if we should touch tweetnacl), thank you for taking care.
This PR originated from OP-TEE Ed25519ph support, so I wanted to gather some feedback from optee community regarding usage of these API calls we introduced.

@sjaeckel
Copy link
Member

@jenswi-linaro @larperaxis you're the consumers of the API, are you fine with these changes as well?

@sjaeckel sjaeckel requested a review from jenswi-linaro August 18, 2022 09:48
Valerii Chubar and others added 4 commits August 20, 2022 14:38
@sjaeckel
ed25519: Add testcase for segfault on verify
a6c6492 
In case when the signature is not verified the "mlen" variable
is equal to ULONG_MAX. When LTC_CLEAN_STACK has been defined
this results in a segmentation fault.

Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
@sjaeckel
Add ed25519ctx and ed25519ph support
b761cc0 
Signed-off-by: Valerii Chubar <valerii_chubar@epam.com>
Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com>
@sjaeckel
re-factor some parts of the Ed25519ctx and Ed25519ph implementation
9dec999 
* The RFC doesn't limit the context to be a string.
  It talks about `octets` which means it could be any binary data.
* Move the context-preprocessing function out of tweetnacl.c
* Fix potential segfaults when Ed25519 signature verification fails and
  `LTC_CLEAN_STACK` is enabled.
* Fix all the warnings.
* Update documentation.

Signed-off-by: Steffen Jaeckel <s@jaeckel.eu>
@sjaeckel
Update makefiles
23d7091
@sjaeckel sjaeckel merged commit ddfe2e8 into libtom:develop Aug 21, 2022
@sa-kib sa-kib deleted the ed25519ctx branch August 22, 2022 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jenswi-linaro jenswi-linaro Awaiting requested review from jenswi-linaro

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sa-kib @sjaeckel