Implement deterministic ECDSA sign (RFC6979) by Jemmy1228 · Pull Request #9223 · openssl/openssl

Jemmy1228 · 2019-06-23T15:32:27Z

Implements RFC6979 deterministic ECDSA/DSA sign (in this commit, only ECDSA is available)

ECDSA deterministic sign is now available by specifying "ecdsa_nonce_type:deterministic" when signing.
for example:

openssl dgst -sha1  -sign private.key -sigopt ecdsa_nonce_type:deterministic < test.data > test.sha1

Maybe fixed #2078

Checklist

documentation is added or updated
tests are added or updated

I haven't thought of that there would be so many comments in this PR.
I read through this thread and list the up-to-date TO-DOs here:

Add Test Vectors (Finished here but not pushed in this PR)
Prevent the use of rand_local.h
Modify CHANGES
Move the code into EVP
Rename ctrls
Prevent the use of RUN_ONCE (Fixed in 5cf5342)

beldmit · 2019-06-23T16:36:58Z

Is it possible to implement the deterministic signature schema via special EVP ctrl call?

Jemmy1228 · 2019-06-23T16:42:05Z

@beldmit It seems all signing calls would finally call EC_KEY_METHOD->sign_sig, in this case it would be ossl_ecdsa_sign_sig defined in crypto/ec/ec_kmeth.c . However, this method doesn't accept parameter about hash method, as I think, this has to be changed...

Jemmy1228 · 2019-06-23T16:57:32Z

@beldmit I missunderstood your meaning just now....
Yes, it is implemented via EVP ctrl call. ctrl_call can only set hash algorithm in the ctx. However, when producing ECDSA signatures, neither the ctx nor the hash nid will be passed to the real method that does the signing (ossl_ecdsa_sign_sig)

Jemmy1228 · 2019-06-23T17:18:58Z

Maybe call sign_setup directly in EVP, initialize k_reverse and r before calling sign_sig is a good way?

Jemmy1228 · 2019-06-23T23:03:44Z

Okay, I will change the code tonnight.
But, should I add a new user visible API for deterministic sign ?

bbbrumley · 2019-06-24T06:41:12Z

@JemmyLoveJenny At a minimum, you should integrate the test vectors from RFC 6979 as plaintext EVP tests.

If you need help with that, my team has some tooling. Tagging @romen

kroeckx · 2019-06-24T06:46:28Z

I suggest that you only add new APIs when it's actually needed. We want users to switch to the EVP API. If it's easy to do with the EVP API, I suggest you only add support for it in the EVP API.

Jemmy1228 · 2019-06-24T06:54:59Z

@kroeckx Actually, the EVP ctx pmeth->sign would call ECDSA_sign. You mean that I can skip that method and call the kmeth->sign_sig directly in EVP ?

Jemmy1228 · 2019-06-24T06:56:50Z

@bbbrumley Yes, I will integrate these test vectors into EVP test finally.

Jemmy1228 · 2019-06-24T13:18:32Z

@kroeckx
Please have a look at the new commit.
User visible APIs are not modified this time.

Jemmy1228 · 2019-06-24T14:18:47Z

I have some questions about the checks

[Solved] I should fill in the ICLA form in my language or English? If in Englilsh, the "Full Name" field would be my legal name in English form or the name that Engilsh-speaking people call me?
[Solved] AppVeyor build fails and show the following result ..........
~~Some of the Travis CI fail reasons are the same as AppVeyor,~~ the others are related to documentation and MAKE UPDATE (mknum.pl). Whether it means there are no more errors with the code?

mattcaswell · 2019-06-24T15:47:49Z

I should fill in the ICLA form in my language or English?

Yes.

If in Englilsh, the "Full Name" field would be my legal name in English form or the name that Engilsh-speaking people call me?

Please give your legal name in English.

Jemmy1228 · 2019-06-24T16:26:35Z

@beldmit Updated according to your suggestions

slontis

I thought this code looked familiar.
Did you look at HMAC_DRBG.c ?
@paulidale would there be some way to rework this so it can use the drbg code?

Have a look at test/drbg_cavs_test.c for an example of how to drive the code..

slontis · 2019-06-24T23:40:15Z

Travis failures are relevant. You have a memory leak somewhere.
Use valgrind on ./test/ecdsatest to track it down

paulidale · 2019-06-24T23:50:44Z

Did you look at HMAC_DRBG.c

The similarity is great. Direct sharing of internal code between the two possibly isn't a great idea but if there was a way of using the HMAC-DRBG to get the same output, that would be worthwhile.

Jemmy1228 · 2019-06-25T00:38:51Z

@slontis Is the Travis ECDSA tests still failing? I thought it was solved in the most recent commit.

slontis · 2019-06-25T00:47:46Z

Maybe you havent pushed the new commits?

Jemmy1228 · 2019-06-25T00:56:35Z

@slontis
No, I've pushed it .
AppVeyor build contain ECDSA tests, it shows that tests passed.
I think the reason why Travis CI fails is that I didn't MAKE UPDATE (update libcrypto.num) and I didn't update the documentation.
Is the ECDSA tests still failing now? I didnt't find ECDSA failure in the Travis build log

slontis · 2019-06-25T01:05:28Z

I looked in travis 26035.2 (open the raw log and search for not ok)

slontis · 2019-06-25T01:10:39Z

If you look at the options for each build e.g for 26035.2 you will see what options it uses..
i.e "no-asm -Werror --debug no-afalgeng no-shared enable-crypto-mdebug enable-rc5 enable-md2"
So you can do the same locally and see if you reproduce the error. It is finding a problem because of the enable-crypto-mdebug option.
If you are using linux I would just run valgrind on the test.

Jemmy1228 · 2019-06-25T01:28:34Z

@slontis Oh, I see it... but it is confusing to me, some ECDH tests failed, even tests unrelated to EC failed...
I'm sorry that I'm at school now and won't be able to make commits until I get home.
I'll re-formate code and look into the ECDSA failure later.

slontis · 2019-06-25T01:33:44Z

No problems.. All those tests use ECDSA. Fix the one problem in a simpler test (which is a memory leak) and the others will most likely all be fixed also..

Jemmy1228 · 2020-08-27T17:30:21Z

@mspncp I noticed that OpenSSL has recently changed the RAND_DRBG API as described in #12509
I have read the documentation and find that it suggests using EVP_RAND_CTX instead.
I was suggested to write something like this example

If I understood it correctly, the fetched RAND_DRBG_CTX would call the real random generator to provide entropy and nonce, which means I have no chance to provide my own ones.

I read the evp_test.c in order to find out how to specify custom entropy and nonce.
It seems that it first fetches a EVP_RAND-TEST-RAND and its EVP_RAND_CTX
Then, it fetches the real EVP_RAND and set the previous EVP_RAND_CTX as the parent.
Finally, set OSSL_RAND_PARAM_TEST_ parameter for the first EVP_RAND_CTX

I wonder if this is the correct way of specifying custom entropy and nonce?
I don't know the purpose of chaining EVP_RAND_CTX, does it mean that the child will obtain entropy and nonce from the parent?

paulidale · 2020-08-28T00:17:54Z

I read the evp_test.c in order to find out how to specify custom entropy and nonce.
It seems that it first fetches a EVP_RAND-TEST-RAND and its EVP_RAND_CTX
Then, it fetches the real EVP_RAND and set the previous EVP_RAND_CTX as the parent.
Finally, set OSSL_RAND_PARAM_TEST_ parameter for the first EVP_RAND_CTX

Currently, this is the only way to feed a random number generator specific seed material. Another place that does this is the FIPS known answer tests. Look for self_test_drbg() in providers/fips/self_test_kats.c. The code is pretty similar.

This reverts commit 9b0e818. Turns out ECDSA signatures aren't deterministic at the moment, meaning the signed executables are always different when re-built, due to the nonce included in the signature. We need to wait till OpenSSL gets support for deterministic ECDSA signatures, or run ahead and switch to EdDSA/ED25519, which uses a deterministic nonce, out of the box. Could not find evidence that Windows supports such code signatures, so either solutions will have to wait. Relevant links: - OpenSSL PR: openssl/openssl#9223 - OpenSSL Issue: openssl/openssl#2078 - RFC: https://tools.ietf.org/html/rfc6979

Sajjon · 2021-04-10T06:08:57Z

What is the status of this PR? How can we get it merged? And also, sorry if this is completely off-topic, but how does BoringSSL and OpenSSL sync feature wise? Does Google's BoringSSL get updated with features X, Y, Z when added to OpenSSL? I.e. if we get deterministic signing (RFC6979) merged into OpenSSL, is it likely that BoringSSL will follow and implement that/cherry-pick that commit and merge it into their code base?

vdukhovni · 2021-10-10T18:10:13Z

May I ask whether there are still plans to get this PR (deterministic signing ala RFC6979) merged.

For example, "Knot DNS" now supports deterministic zone signing, but only when linked with GnuTLS. I really do think we should have an interoperable implementation in at least a 3.1 release, if not in any earlier versions...

t8m · 2021-10-11T09:05:27Z

May I ask whether there are still plans to get this PR (deterministic signing ala RFC6979) merged.

For example, "Knot DNS" now supports deterministic zone signing, but only when linked with GnuTLS. I really do think we should have an interoperable implementation in at least a 3.1 release, if not in any earlier versions...

I'd say why not. If someone brings it up-to-date with the current master branch which means making it into a provided implementation.

vdukhovni · 2021-10-11T20:24:06Z

I'd say why not. If someone brings it up-to-date with the current master branch which means making it into a provided implementation.

What do you mean by a "provided implementation"? Do you mean a separate provider? Or do you mean a feature (EVP_PKEY_CTX control operation?) of the default implementation?

Any volunteers to resume this effort?

t8m · 2021-10-12T07:09:12Z

I mean a feature of the existing implementation. The question is if it should be a separate algorithm or just a parameter of the existing ECDSA.

mattcaswell · 2021-12-14T10:44:39Z

OTC: Can be reviewed through the normal review process.

Please rebase

romen · 2021-12-14T10:47:24Z

@bbbrumley I just don't know what to do next...... I read through this PR just now, and I find that there are many suggestions. However, these things can't be done by myself.

For example, OpenSSL officials should provide an answer to the question "How to name the control parameters?" and "How to disable the DRBG length requirement without including rand_local.h"

@openssl/otc These questions should be answered to allow @JemmyLoveJenny to continue the work

@JemmyLoveJenny , despite the delays, are you still willing to work on this PR?

Jemmy1228 · 2021-12-14T11:40:04Z

@JemmyLoveJenny , despite the delays, are you still willing to work on this PR?

Yes, I'm still willing to work on this PR. But as you have mentioned, I need to know what to do next.
I will try to rebase this PR in a few days.

openssl-machine · 2022-03-15T00:07:33Z

This PR has been closed. It was waiting for the creator to make requested changes but it has not been updated for 90 days.

This PR is based off the contributions in PR openssl#9223 by Jemmy1228. It has been modified and reworked to: (1) Work with providers (2) Support ECDSA and DSA (3) Use OpenSSL's HMAC_DRBG via the EVP_RAND_CTX interface. To do this some behaviour of the DRBG needed to be disabled via a new OSSL_PARAM. A nonce_type is passed around in the API's, just in case there are future deterministic algorithms.

slontis · 2022-07-15T11:43:10Z

Hi @Jemmy1228 I have created a new PR #18809 based off your great work. (I have referenced you in the commit - hope that is ok).

This PR is based off the contributions in PR openssl#9223 by Jemmy1228. It has been modified and reworked to: (1) Work with providers (2) Support ECDSA and DSA (3) Use a variant of OpenSSL's HMAC_DRBG via the EVP_RAND_CTX interface. A nonce_type is passed around in the API's, just in case there are future deterministic algorithms.

This PR is based off the contributions in PR openssl#9223 by Jemmy1228. It has been modified and reworked to: (1) Work with providers (2) Support ECDSA and DSA (3) Add a KDF HMAC_DRBG implementation that shares code with the RAND HMAC_DRBG. A nonce_type is passed around inside the Signing API's, in order to support any future deterministic algorithms.

This PR is based off the contributions in PR #9223 by Jemmy1228. It has been modified and reworked to: (1) Work with providers (2) Support ECDSA and DSA (3) Add a KDF HMAC_DRBG implementation that shares code with the RAND HMAC_DRBG. A nonce_type is passed around inside the Signing API's, in order to support any future deterministic algorithms. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from #18809)

This PR is based off the contributions in PR openssl#9223 by Jemmy1228. It has been modified and reworked to: (1) Work with providers (2) Support ECDSA and DSA (3) Add a KDF HMAC_DRBG implementation that shares code with the RAND HMAC_DRBG. A nonce_type is passed around inside the Signing API's, in order to support any future deterministic algorithms. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Hugo Landau <hlandau@openssl.org> (Merged from openssl#18809)

openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Jun 23, 2019

kroeckx reviewed Jun 23, 2019

View reviewed changes

Comment thread include/openssl/bn.h Outdated

kroeckx reviewed Jun 23, 2019

View reviewed changes

Comment thread include/openssl/bn.h Outdated

kroeckx reviewed Jun 23, 2019

View reviewed changes

Comment thread include/openssl/ec.h Outdated

Jemmy1228 force-pushed the ecdsa_deterministic_signature branch from fd0175c to 1bd48bf Compare June 24, 2019 12:33

beldmit reviewed Jun 24, 2019

View reviewed changes

Comment thread crypto/ec/ec_lcl.h Outdated

Comment thread crypto/ec/ec_pmeth.c Outdated

slontis requested changes Jun 24, 2019

View reviewed changes

Jemmy1228 closed this Jun 25, 2019

t8m added this to the Post 3.0.0 milestone Dec 4, 2020

wintersteiger mentioned this pull request Mar 26, 2021

Non-deterministic/randomized ECDSA microsoft/CCF#2363

Merged

rhenium mentioned this pull request Mar 31, 2021

There is no way to perform a ECDSA sign with a deterministic K value ruby/openssl#426

Open

t8m added the triaged: feature The issue/pr requests/adds a feature label May 4, 2021

panva mentioned this pull request Jul 17, 2021

Initial Draft for PASETO v3/v4 specifications paragonie/paseto#127

Merged

beldmit added the hold: substantial feature label Nov 30, 2021

t8m added the waiting-for: contributor response This pull request is awaiting a response by the contributor label Dec 14, 2021

romen removed the hold: substantial feature label Dec 14, 2021

openssl-machine closed this Mar 15, 2022

slontis mentioned this pull request Jul 15, 2022

Implement deterministic ECDSA sign (RFC6979) #18809

Closed

2 tasks

Uh oh!

Conversation

Jemmy1228 commented Jun 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

beldmit commented Jun 23, 2019

Uh oh!

Jemmy1228 commented Jun 23, 2019

Uh oh!

Jemmy1228 commented Jun 23, 2019

Uh oh!

Jemmy1228 commented Jun 23, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jemmy1228 commented Jun 23, 2019

Uh oh!

bbbrumley commented Jun 24, 2019

Uh oh!

kroeckx commented Jun 24, 2019 via email

Uh oh!

Jemmy1228 commented Jun 24, 2019

Uh oh!

Jemmy1228 commented Jun 24, 2019

Uh oh!

Jemmy1228 commented Jun 24, 2019

Uh oh!

Uh oh!

Uh oh!

Jemmy1228 commented Jun 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattcaswell commented Jun 24, 2019

Uh oh!

Jemmy1228 commented Jun 24, 2019

Uh oh!

slontis left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

slontis commented Jun 24, 2019

Uh oh!

paulidale commented Jun 24, 2019

Uh oh!

Jemmy1228 commented Jun 25, 2019

Uh oh!

slontis commented Jun 25, 2019

Uh oh!

Jemmy1228 commented Jun 25, 2019

Uh oh!

slontis commented Jun 25, 2019

Uh oh!

slontis commented Jun 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jemmy1228 commented Jun 25, 2019

Uh oh!

slontis commented Jun 25, 2019

Uh oh!

Jemmy1228 commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulidale commented Aug 28, 2020

Uh oh!

Sajjon commented Apr 10, 2021

Uh oh!

vdukhovni commented Oct 10, 2021

Uh oh!

Jemmy1228 commented Jun 23, 2019 •

edited

Loading

Jemmy1228 commented Jun 24, 2019 •

edited

Loading

slontis left a comment •

edited

Loading

slontis commented Jun 25, 2019 •

edited

Loading

Jemmy1228 commented Aug 27, 2020 •

edited

Loading