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.

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

Introduced optional values to ECDSA sign for deterministic k values #434

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

rickmark
Copy link
Contributor

@rickmark rickmark commented Apr 2, 2021

This change allows for use cases where the client would like to precompute inverse_k and r themselves or to simply provide k.

This makes OpenSSL in Ruby suitable for workloads that require RFC 6979 conformance (though this change doesn't implement that RFC itself)

Resolves #426

Rick Mark added 3 commits April 2, 2021 00:05
Performs ephemeral point R generation for deterministic ECDSA
Uses RFC test values to verify deterministic k sign
One curve to start
@rhenium
Copy link
Member

rhenium commented Apr 2, 2021

I can't merge this now because it depends on the ECDSA_sign_ex() deprecated in OpenSSL 3.0, which will likely be released this year.

There is a plan to rewrite methods using those legacy API with the new EVP API set before it happens. #370

Unfortunately, there is currently no replacement (yet) for this functionality in EVP API.

I'm not opposed to implementing this using ECDSA_sign_ex() to support older versions of OpenSSL (switching implementation with #ifdefs), but I want every new feature to be also available when compiled against the latest version.

@rickmark
Copy link
Contributor Author

rickmark commented Apr 2, 2021

I can't merge this now because it depends on the ECDSA_sign_ex() deprecated in OpenSSL 3.0, which will likely be released this year.

Is there a reason not to ship even with the deprecation, and later updating the implementation to call the as of yet existent API for OpenSSL 3 in the future? Deprecated should mean it still works?

I mean - it would be possible to fall down to doing the (by this stage fairly easy) math for the signature, here's a working implementation for ECDSA sign in a Ruby test I just wrote on this branch:

    group = OpenSSL::PKey::EC::Group.new 'prime192v1'

    private_key = OpenSSL::BN.new '6FAB034934E4C0FC9AE67F5B5659A9D7D1FEFD187EE09FD4', 16

    hash = OpenSSL::Digest.new('SHA256').digest('test')
    degree_bytes = group.degree / 8

    k = OpenSSL::BN.new '5C4CE89CF56D9E7C77C8585339B006B97B5F0680B4306C6C', 16
    expected_r = OpenSSL::BN.new '3A718BD8B4926C3B52EE6BBE67EF79B18CB6EB62B1AD97AE', 16
    expected_s = OpenSSL::BN.new '5662E6848A4A19B1F1AE2F72ACD4B8BBE50F1EAC65D9124F', 16

    inverse_k, r = group.send(:ecdsa_generate_signature_params, k)
    assert_equal(expected_r, r)

    # TODO: pad to degree when hash is smaller
    e = OpenSSL::BN.new hash[0..(degree_bytes - 1)], 2
    assert_equal(group.degree, e.num_bits)

    s = inverse_k.mod_mul(e + (r * private_key), group.order)
    assert_equal(expected_s, s)

This is just a helper test that shows that ECDSA results using raw math work as expected vs ECDSA_sign and friends
@rickmark
Copy link
Contributor Author

rickmark commented Apr 2, 2021

Crud: just found out that EC_POINT_get_affine_coordinates doesn't exist for all of the supported setups (2.3)

@rhenium
Copy link
Member

rhenium commented Apr 15, 2021

Is there a reason not to ship even with the deprecation, and later updating the implementation to call the as of yet existent API for OpenSSL 3 in the future? Deprecated should mean it still works?

The deprecated feature, in this case, the automatic "downgrade" of the backend of EVP_PKEY from Providers to EVP_PKEY_METHOD that is required to use ECDSA_*, should continue to work in OpenSSL 3.0 if the "default" Provider is selected at runtime. I'm not sure if it will work in an environment where the fips Provider will likely be used by default, such as Red Hat-based or Ubuntu systems.

Also, I'm not keen to maintain code that is known to produce unresolvable warnings - ruby-openssl, along with ruby itself and other stdlibs are currently warning-free.

As I mentioned in #426, there is currently an open Pull Request against openssl/openssl to add RFC 6979 support. It's labeled as "Post 3.0.0", but my understanding is that they generally try to make all features available through the EVP interface whenever deprecating something, so it should be still possible for it to be included in 3.0.0, if the conflicts, etc. are resolved.

That will still not add this level of fine-grained control, though. It hasn't been requested on their issue tracker or mailing lists. Possibly because the core part is simple to implement using BIGNUM directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

There is no way to perform a ECDSA sign with a deterministic K value
2 participants