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

Add support for raw private/public keys #646

Merged
merged 19 commits into from
Jul 12, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 130 additions & 0 deletions ext/openssl/ossl_pkey.c
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,71 @@ ossl_pkey_initialize_copy(VALUE self, VALUE other)
}
#endif

/*
* call-seq:
* OpenSSL::PKey.private_new(algo, string) -> PKey
*
* See the OpenSSL documentation for EVP_PKEY_new_raw_private_key()
*/

#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
Copy link
Member

@rhenium rhenium Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think #ifdef needs to be moved above the doc comment so that rdoc can parse it correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @ 202cac7 .
Maybe the first and second #ifdef blocks can be merged into one, but I thought having it separated would make it easier to understand that each method needs this condition to be met.

static VALUE
ossl_pkey_initialize_private(VALUE self, VALUE type, VALUE key)
{

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this empty line.

EVP_PKEY *pkey;
const EVP_PKEY_ASN1_METHOD *ameth;
int pkey_id;
size_t keylen;

StringValue(type);
ameth = EVP_PKEY_asn1_find_str(NULL, RSTRING_PTR(type), RSTRING_LENINT(type));
if (!ameth)
ossl_raise(ePKeyError, "algorithm %"PRIsVALUE" not found", type);
EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, ameth);

keylen = RSTRING_LEN(key);
rhenium marked this conversation as resolved.
Show resolved Hide resolved

pkey = EVP_PKEY_new_raw_private_key(pkey_id, NULL, (unsigned char *)RSTRING_PTR(key), keylen);
if (!pkey)
ossl_raise(ePKeyError, "Could not parse PKey");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message should be adjusted because parsing isn't happening here (perhaps just "EVP_PKEY_new_raw_private_key" would work).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @ e9ccf58


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line has trailing spaces.

return ossl_pkey_new(pkey);
}
#endif

/*
* call-seq:
* OpenSSL::PKey.public_new(algo, string) -> PKey
*
* See the OpenSSL documentation for EVP_PKEY_new_raw_public_key()
*/

#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
static VALUE
ossl_pkey_initialize_public(VALUE self, VALUE type, VALUE key)
{
EVP_PKEY *pkey;
const EVP_PKEY_ASN1_METHOD *ameth;
int pkey_id;
size_t keylen;

StringValue(type);
ameth = EVP_PKEY_asn1_find_str(NULL, RSTRING_PTR(type), RSTRING_LENINT(type));
if (!ameth)
ossl_raise(ePKeyError, "algorithm %"PRIsVALUE" not found", type);
EVP_PKEY_asn1_get0_info(&pkey_id, NULL, NULL, NULL, NULL, ameth);

keylen = RSTRING_LEN(key);

pkey = EVP_PKEY_new_raw_public_key(pkey_id, NULL, (unsigned char *)RSTRING_PTR(key), keylen);
if (!pkey)
ossl_raise(ePKeyError, "Could not parse PKey");

return ossl_pkey_new(pkey);
}
#endif

/*
* call-seq:
* pkey.oid -> string
Expand Down Expand Up @@ -816,6 +881,33 @@ ossl_pkey_private_to_pem(int argc, VALUE *argv, VALUE self)
return do_pkcs8_export(argc, argv, self, 0);
}

/*
* call-seq:
* key.private_to_raw => string
*
* See the OpenSSL documentation for EVP_PKEY_get_raw_private_key()
*/

#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
static VALUE ossl_pkey_private_to_raw(VALUE self)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: please insert a newline after VALUE.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @ 26bd41d

{
EVP_PKEY *pkey;
VALUE str;
size_t len;

GetPKey(self, pkey);
EVP_PKEY_get_raw_private_key(pkey, NULL, &len);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably needs an error check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @ 6628df1.
Maybe we could have a different error message for this call and the next call, but it could be difficult to tell what exactly caused the error on each call so I think leaving as it is now would be reasonable.

str = rb_str_new(NULL, len);

if (EVP_PKEY_get_raw_private_key(pkey, (unsigned char *)RSTRING_PTR(str), &len) != 1)
ossl_raise(ePKeyError, "EVP_PKEY_get_raw_private_key");

rb_str_set_len(str, len);

return str;
}
#endif

VALUE
ossl_pkey_export_spki(VALUE self, int to_der)
{
Expand Down Expand Up @@ -867,6 +959,34 @@ ossl_pkey_public_to_pem(VALUE self)

/*
* call-seq:
* key.public_to_raw => string
*
* See the OpenSSL documentation for EVP_PKEY_get_raw_public_key()
*/

#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
static VALUE ossl_pkey_public_to_raw(VALUE self)
{
EVP_PKEY *pkey;
VALUE str;
size_t len;

GetPKey(self, pkey);
EVP_PKEY_get_raw_public_key(pkey, NULL, &len);
str = rb_str_new(NULL, len);

if (EVP_PKEY_get_raw_public_key(pkey, (unsigned char *)RSTRING_PTR(str), &len) != 1)
ossl_raise(ePKeyError, "EVP_PKEY_get_raw_public_key");

rb_str_set_len(str, len);

return str;
}
#endif

/*
* call-seq:
* pkey.sign(digest, data) -> String
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line looks like a copy-and-paste error.

* pkey.compare?(another_pkey) -> true | false
*
* Used primarily to check if an OpenSSL::X509::Certificate#public_key compares to its private key.
Expand Down Expand Up @@ -1602,6 +1722,10 @@ Init_ossl_pkey(void)
rb_define_module_function(mPKey, "read", ossl_pkey_new_from_data, -1);
rb_define_module_function(mPKey, "generate_parameters", ossl_pkey_s_generate_parameters, -1);
rb_define_module_function(mPKey, "generate_key", ossl_pkey_s_generate_key, -1);
#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
rb_define_module_function(mPKey, "private_new", ossl_pkey_initialize_private, 2);
rb_define_module_function(mPKey, "public_new", ossl_pkey_initialize_public, 2);
#endif

rb_define_alloc_func(cPKey, ossl_pkey_alloc);
rb_define_method(cPKey, "initialize", ossl_pkey_initialize, 0);
Expand All @@ -1615,8 +1739,14 @@ Init_ossl_pkey(void)
rb_define_method(cPKey, "to_text", ossl_pkey_to_text, 0);
rb_define_method(cPKey, "private_to_der", ossl_pkey_private_to_der, -1);
rb_define_method(cPKey, "private_to_pem", ossl_pkey_private_to_pem, -1);
#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
rb_define_method(cPKey, "private_to_raw", ossl_pkey_private_to_raw, 0);
#endif
rb_define_method(cPKey, "public_to_der", ossl_pkey_public_to_der, 0);
rb_define_method(cPKey, "public_to_pem", ossl_pkey_public_to_pem, 0);
#ifdef HAVE_EVP_PKEY_NEW_RAW_PRIVATE_KEY
rb_define_method(cPKey, "public_to_raw", ossl_pkey_public_to_raw, 0);
#endif
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we group the #raw_*_key methods together?

rb_define_method(cPKey, "compare?", ossl_pkey_compare, 1);

rb_define_method(cPKey, "sign", ossl_pkey_sign, -1);
Expand Down
6 changes: 6 additions & 0 deletions lib/openssl/pkey.rb
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,12 @@ def to_bn(conversion_form = group.point_conversion_form)
end
end

class PKey
def self._load(string)
OpenSSL::PKey.read(string)
end
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be removed too, as this would be unreachable now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @ 4d67841

class RSA
include OpenSSL::Marshal

Expand Down
39 changes: 39 additions & 0 deletions test/openssl/test_pkey.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ def test_ed25519
assert_equal pub_pem, priv.public_to_pem
assert_equal pub_pem, pub.public_to_pem

begin
assert_equal "4ccd089b28ff96da9db6c346ec114e0f5b8a319f35aba624da8cf6ed4fb8a6fb",
priv.private_to_raw.unpack1("H*")
assert_equal OpenSSL::PKey.private_new("ED25519", priv.private_to_raw).private_to_pem,
priv.private_to_pem
assert_equal "3d4017c3e843895a92b70aa74d1b7ebc9c982ccf2ec4968cc0cd55f12af4660c",
priv.public_to_raw.unpack1("H*")
assert_equal OpenSSL::PKey.public_new("ED25519", priv.public_to_raw).public_to_pem,
pub.public_to_pem
rescue NoMethodError
pend "running OpenSSL version does not have raw public key support"
end

sig = [<<~EOF.gsub(/[^0-9a-f]/, "")].pack("H*")
92a009a9f0d4cab8720e820b5f642540
a2b27b5416503f8fb3762223ebdb69da
Expand Down Expand Up @@ -155,6 +168,32 @@ def test_x25519
assert_equal alice_pem, alice.private_to_pem
assert_equal bob_pem, bob.public_to_pem
assert_equal [shared_secret].pack("H*"), alice.derive(bob)
begin
alice_private = OpenSSL::PKey.private_new("X25519", alice.private_to_raw)
bob_public = OpenSSL::PKey.public_new("X25519", bob.public_to_raw)
alice_private_raw = alice.private_to_raw.unpack1("H*")
bob_public_raw = bob.public_to_raw.unpack1("H*")
rescue NoMethodError
# OpenSSL < 1.1.1
pend "running OpenSSL version does not have raw public key support"
end
assert_equal alice_private.private_to_pem,
alice.private_to_pem
assert_equal bob_public.public_to_pem,
bob.public_to_pem
assert_equal "77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a",
alice_private_raw
assert_equal "de9edb7d7b7dc1b4d35b61c2ece435373f8343c85b78674dadfc7e146f882b4f",
bob_public_raw
end

def raw_initialize
pend "Ed25519 is not implemented" unless OpenSSL::OPENSSL_VERSION_NUMBER >= 0x10101000 && # >= v1.1.1

assert_raise(OpenSSL::PKey::PKeyError) { OpenSSL::PKey.private_new("foo123", "xxx") }
assert_raise(OpenSSL::PKey::PKeyError) { OpenSSL::PKey.private_new("ED25519", "xxx") }
assert_raise(OpenSSL::PKey::PKeyError) { OpenSSL::PKey.public_new("foo123", "xxx") }
assert_raise(OpenSSL::PKey::PKeyError) { OpenSSL::PKey.public_new("ED25519", "xxx") }
end

def test_compare?
Expand Down