openssl icon indicating copy to clipboard operation
openssl copied to clipboard

Introduced optional values to ECDSA sign for deterministic k values

Open rickmark opened this issue 4 years ago • 4 comments

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

rickmark avatar Apr 02 '21 07:04 rickmark

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.

rhenium avatar Apr 02 '21 12:04 rhenium

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)

rickmark avatar Apr 02 '21 20:04 rickmark

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

rickmark avatar Apr 02 '21 20:04 rickmark

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.

rhenium avatar Apr 15 '21 06:04 rhenium