Skip to content

Commit

Permalink
pkey: deprecate PKey#set_* methods
Browse files Browse the repository at this point in the history 
OpenSSL 3.0 made EVP_PKEY immutable. This means we can only have a const
pointer of the low level struct and the following methods can no longer
be provided when linked against OpenSSL 3.0:

 - OpenSSL::PKey::RSA#set_key
 - OpenSSL::PKey::RSA#set_factors
 - OpenSSL::PKey::RSA#set_crt_params
 - OpenSSL::PKey::DSA#set_pqg
 - OpenSSL::PKey::DSA#set_key
 - OpenSSL::PKey::DH#set_pqg
 - OpenSSL::PKey::DH#set_key
 - OpenSSL::PKey::EC#group=
 - OpenSSL::PKey::EC#private_key=
 - OpenSSL::PKey::EC#public_key=

There is no direct replacement for this functionality at the moment.
I plan to introduce a wrapper around EVP_PKEY_fromdata(), which takes
all key components at once to construct an EVP_PKEY.
  • Loading branch information
@rhenium
rhenium committed Dec 20, 2021
1 parent 5e2e66c commit 6848d2d
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 68 deletions.
16 changes: 16 additions & 0 deletions ext/openssl/ossl_pkey.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ static VALUE ossl_##_keytype##_get_##_name(VALUE self) \
OSSL_PKEY_BN_DEF_GETTER0(_keytype, _type, a2, \
_type##_get0_##_group(obj, NULL, &bn))

#if !OSSL_OPENSSL_PREREQ(3, 0, 0)
#define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \
/* \
* call-seq: \
Expand Down Expand Up @@ -181,6 +182,21 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
} \
return self; \
}
#else
#define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \
static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALUE v3) \
{ \
rb_raise(ePKeyError, \
#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
}

#define OSSL_PKEY_BN_DEF_SETTER2(_keytype, _type, _group, a1, a2) \
static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
{ \
rb_raise(ePKeyError, \
#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
}
#endif

#define OSSL_PKEY_BN_DEF3(_keytype, _type, _group, a1, a2, a3) \
OSSL_PKEY_BN_DEF_GETTER3(_keytype, _type, _group, a1, a2, a3) \
Expand Down
12 changes: 12 additions & 0 deletions ext/openssl/ossl_pkey_ec.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ ossl_ec_key_get_group(VALUE self)
static VALUE
ossl_ec_key_set_group(VALUE self, VALUE group_v)
{
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
#else
EC_KEY *ec;
EC_GROUP *group;

Expand All @@ -247,6 +250,7 @@ ossl_ec_key_set_group(VALUE self, VALUE group_v)
ossl_raise(eECError, "EC_KEY_set_group");

return group_v;
#endif
}

/*
Expand Down Expand Up @@ -275,6 +279,9 @@ static VALUE ossl_ec_key_get_private_key(VALUE self)
*/
static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key)
{
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
#else
EC_KEY *ec;
BIGNUM *bn = NULL;

Expand All @@ -294,6 +301,7 @@ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key)
}

return private_key;
#endif
}

/*
Expand Down Expand Up @@ -322,6 +330,9 @@ static VALUE ossl_ec_key_get_public_key(VALUE self)
*/
static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key)
{
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
#else
EC_KEY *ec;
EC_POINT *point = NULL;

Expand All @@ -341,6 +352,7 @@ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key)
}

return public_key;
#endif
}

/*
Expand Down
38 changes: 26 additions & 12 deletions test/openssl/test_pkey_dh.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,32 @@ def test_params_ok?
end

def test_dup
dh = Fixtures.pkey("dh1024")
dh2 = dh.dup
assert_equal dh.to_der, dh2.to_der # params
assert_equal_params dh, dh2 # keys
dh2.set_pqg(dh2.p + 1, nil, dh2.g)
assert_not_equal dh2.p, dh.p
assert_equal dh2.g, dh.g
# Parameters only
dh1 = Fixtures.pkey("dh1024")
dh2 = dh1.dup
assert_equal dh1.to_der, dh2.to_der
assert_not_equal nil, dh1.p
assert_not_equal nil, dh1.g
assert_equal [dh1.p, dh1.g], [dh2.p, dh2.g]
assert_equal nil, dh1.pub_key
assert_equal nil, dh1.priv_key
assert_equal [dh1.pub_key, dh1.priv_key], [dh2.pub_key, dh2.priv_key]

# PKey is immutable in OpenSSL >= 3.0
if !openssl?(3, 0, 0)
dh2.set_pqg(dh2.p + 1, nil, dh2.g)
assert_not_equal dh2.p, dh1.p
end

# With a key pair
dh3 = OpenSSL::PKey.generate_key(Fixtures.pkey("dh1024"))
dh4 = dh3.dup
assert_equal dh3.to_der, dh4.to_der
assert_equal dh1.to_der, dh4.to_der # encodes parameters only
assert_equal [dh1.p, dh1.g], [dh4.p, dh4.g]
assert_not_equal nil, dh3.pub_key
assert_not_equal nil, dh3.priv_key
assert_equal [dh3.pub_key, dh3.priv_key], [dh4.pub_key, dh4.priv_key]
end

def test_marshal
Expand All @@ -125,11 +144,6 @@ def test_marshal

private

def assert_equal_params(dh1, dh2)
assert_equal(dh1.g, dh2.g)
assert_equal(dh1.p, dh2.p)
end

def assert_no_key(dh)
assert_equal(false, dh.public?)
assert_equal(false, dh.private?)
Expand Down
8 changes: 6 additions & 2 deletions test/openssl/test_pkey_dsa.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,12 @@ def test_dup
key = Fixtures.pkey("dsa1024")
key2 = key.dup
assert_equal key.params, key2.params
key2.set_pqg(key2.p + 1, key2.q, key2.g)
assert_not_equal key.params, key2.params

# PKey is immutable in OpenSSL >= 3.0
if !openssl?(3, 0, 0)
key2.set_pqg(key2.p + 1, key2.q, key2.g)
assert_not_equal key.params, key2.params
end
end

def test_marshal
Expand Down
58 changes: 35 additions & 23 deletions test/openssl/test_pkey_ec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ def test_ec_key

key1 = OpenSSL::PKey::EC.generate("prime256v1")

key2 = OpenSSL::PKey::EC.new
key2.group = key1.group
key2.private_key = key1.private_key
key2.public_key = key1.public_key
assert_equal key1.to_der, key2.to_der
# PKey is immutable in OpenSSL >= 3.0; constructing an empty EC object is
# deprecated
if !openssl?(3, 0, 0)
key2 = OpenSSL::PKey::EC.new
key2.group = key1.group
key2.private_key = key1.private_key
key2.public_key = key1.public_key
assert_equal key1.to_der, key2.to_der
end

key3 = OpenSSL::PKey::EC.new(key1)
assert_equal key1.to_der, key3.to_der
Expand All @@ -35,10 +39,14 @@ def test_ec_key

key5 = key1.dup
assert_equal key1.to_der, key5.to_der
key_tmp = OpenSSL::PKey::EC.new("prime256v1").generate_key!
key5.private_key = key_tmp.private_key
key5.public_key = key_tmp.public_key
assert_not_equal key1.to_der, key5.to_der

# PKey is immutable in OpenSSL >= 3.0; EC object should not be modified
if !openssl?(3, 0, 0)
key_tmp = OpenSSL::PKey::EC.generate("prime256v1")
key5.private_key = key_tmp.private_key
key5.public_key = key_tmp.public_key
assert_not_equal key1.to_der, key5.to_der
end
end

def test_generate
Expand All @@ -65,22 +73,26 @@ def test_marshal
end

def test_check_key
key = OpenSSL::PKey::EC.new("prime256v1").generate_key!
assert_equal(true, key.check_key)
assert_equal(true, key.private?)
assert_equal(true, key.public?)
key2 = OpenSSL::PKey::EC.new(key.group)
assert_equal(false, key2.private?)
assert_equal(false, key2.public?)
key2.public_key = key.public_key
assert_equal(false, key2.private?)
assert_equal(true, key2.public?)
key2.private_key = key.private_key
key0 = Fixtures.pkey("p256")
assert_equal(true, key0.check_key)
assert_equal(true, key0.private?)
assert_equal(true, key0.public?)

key1 = OpenSSL::PKey.read(key0.public_to_der)
assert_equal(true, key1.check_key)
assert_equal(false, key1.private?)
assert_equal(true, key1.public?)

key2 = OpenSSL::PKey.read(key0.private_to_der)
assert_equal(true, key2.private?)
assert_equal(true, key2.public?)
assert_equal(true, key2.check_key)
key2.private_key += 1
assert_raise(OpenSSL::PKey::ECError) { key2.check_key }

# EC#private_key= is deprecated in 3.0 and won't work on OpenSSL 3.0
if !openssl?(3, 0, 0)
key2.private_key += 1
assert_raise(OpenSSL::PKey::ECError) { key2.check_key }
end
end

def test_sign_verify
Expand Down Expand Up @@ -112,7 +124,7 @@ def test_derive_key
assert_equal [zIUT].pack("H*"), a.derive(b)

assert_equal a.derive(b), a.dh_compute_key(b.public_key)
end
end if !openssl?(3, 0, 0) # TODO: Test it without using #private_key=

def test_sign_verify_raw
key = Fixtures.pkey("p256")
Expand Down
85 changes: 54 additions & 31 deletions test/openssl/test_pkey_rsa.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,18 @@ def test_private
assert(!key4.private?)
rsa1024 = Fixtures.pkey("rsa1024")

# Generated by RSA#set_key
key5 = OpenSSL::PKey::RSA.new
key5.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert(key5.private?)

# Generated by RSA#set_key, without d
key6 = OpenSSL::PKey::RSA.new
key6.set_key(rsa1024.n, rsa1024.e, nil)
assert(!key6.private?)
if !openssl?(3, 0, 0)
key = OpenSSL::PKey::RSA.new
# Generated by RSA#set_key
key5 = OpenSSL::PKey::RSA.new
key5.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert(key5.private?)

# Generated by RSA#set_key, without d
key6 = OpenSSL::PKey::RSA.new
key6.set_key(rsa1024.n, rsa1024.e, nil)
assert(!key6.private?)
end
end

def test_new
Expand Down Expand Up @@ -235,36 +238,52 @@ def test_encrypt_decrypt_legacy

def test_export
rsa1024 = Fixtures.pkey("rsa1024")
key = OpenSSL::PKey::RSA.new

# key has only n, e and d
key.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert_equal rsa1024.public_key.export, key.export
pub = OpenSSL::PKey.read(rsa1024.public_to_der)
assert_not_equal rsa1024.export, pub.export
assert_equal rsa1024.public_to_pem, pub.export

# PKey is immutable in OpenSSL >= 3.0
if !openssl?(3, 0, 0)
key = OpenSSL::PKey::RSA.new

# key has only n, e, d, p and q
key.set_factors(rsa1024.p, rsa1024.q)
assert_equal rsa1024.public_key.export, key.export
# key has only n, e and d
key.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert_equal rsa1024.public_key.export, key.export

# key has n, e, d, p, q, dmp1, dmq1 and iqmp
key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp)
assert_equal rsa1024.export, key.export
# key has only n, e, d, p and q
key.set_factors(rsa1024.p, rsa1024.q)
assert_equal rsa1024.public_key.export, key.export

# key has n, e, d, p, q, dmp1, dmq1 and iqmp
key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp)
assert_equal rsa1024.export, key.export
end
end

def test_to_der
rsa1024 = Fixtures.pkey("rsa1024")
key = OpenSSL::PKey::RSA.new

# key has only n, e and d
key.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert_equal rsa1024.public_key.to_der, key.to_der
pub = OpenSSL::PKey.read(rsa1024.public_to_der)
assert_not_equal rsa1024.to_der, pub.to_der
assert_equal rsa1024.public_to_der, pub.to_der

# key has only n, e, d, p and q
key.set_factors(rsa1024.p, rsa1024.q)
assert_equal rsa1024.public_key.to_der, key.to_der
# PKey is immutable in OpenSSL >= 3.0
if !openssl?(3, 0, 0)
key = OpenSSL::PKey::RSA.new

# key has n, e, d, p, q, dmp1, dmq1 and iqmp
key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp)
assert_equal rsa1024.to_der, key.to_der
# key has only n, e and d
key.set_key(rsa1024.n, rsa1024.e, rsa1024.d)
assert_equal rsa1024.public_key.to_der, key.to_der

# key has only n, e, d, p and q
key.set_factors(rsa1024.p, rsa1024.q)
assert_equal rsa1024.public_key.to_der, key.to_der

# key has n, e, d, p, q, dmp1, dmq1 and iqmp
key.set_crt_params(rsa1024.dmp1, rsa1024.dmq1, rsa1024.iqmp)
assert_equal rsa1024.to_der, key.to_der
end
end

def test_RSAPrivateKey
Expand Down Expand Up @@ -495,8 +514,12 @@ def test_dup
key = Fixtures.pkey("rsa1024")
key2 = key.dup
assert_equal key.params, key2.params
key2.set_key(key2.n, 3, key2.d)
assert_not_equal key.params, key2.params

# PKey is immutable in OpenSSL >= 3.0
if !openssl?(3, 0, 0)
key2.set_key(key2.n, 3, key2.d)
assert_not_equal key.params, key2.params
end
end

def test_marshal
Expand Down

0 comments on commit 6848d2d

Please sign in to comment.