Skip to content

Commit 5d0df40

Browse files
rheniumrhenium
authored
Merge pull request ruby#480 from rhenium/ky/pkey-deprecate-modify
pkey: deprecate PKey::*#set_* and PKey::{DH,EC}#generate_key!
2 parents 88b7577 + 6848d2d commit 5d0df40

File tree

8 files changed

+221
-98
lines changed

8 files changed

+221
-98
lines changed

‎ext/openssl/ossl_pkey.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ static VALUE ossl_##_keytype##_get_##_name(VALUE self) \
116116
OSSL_PKEY_BN_DEF_GETTER0(_keytype, _type, a2, \
117117
_type##_get0_##_group(obj, NULL, &bn))
118118

119+
#if !OSSL_OPENSSL_PREREQ(3, 0, 0)
119120
#define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \
120121
/* \
121122
* call-seq: \
@@ -173,6 +174,21 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
173174
} \
174175
return self; \
175176
}
177+
#else
178+
#define OSSL_PKEY_BN_DEF_SETTER3(_keytype, _type, _group, a1, a2, a3) \
179+
static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALUE v3) \
180+
{ \
181+
rb_raise(ePKeyError, \
182+
#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
183+
}
184+
185+
#define OSSL_PKEY_BN_DEF_SETTER2(_keytype, _type, _group, a1, a2) \
186+
static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
187+
{ \
188+
rb_raise(ePKeyError, \
189+
#_keytype"#set_"#_group"= is incompatible with OpenSSL 3.0"); \
190+
}
191+
#endif
176192

177193
#define OSSL_PKEY_BN_DEF3(_keytype, _type, _group, a1, a2, a3) \
178194
OSSL_PKEY_BN_DEF_GETTER3(_keytype, _type, _group, a1, a2, a3) \

‎ext/openssl/ossl_pkey_dh.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,16 @@ VALUE eDHError;
5858
*
5959
* Examples:
6060
* # Creating an instance from scratch
61-
* dh = DH.new
61+
* # Note that this is deprecated and will not work on OpenSSL 3.0 or later.
62+
* dh = OpenSSL::PKey::DH.new
6263
* dh.set_pqg(bn_p, nil, bn_g)
6364
*
6465
* # Generating a parameters and a key pair
65-
* dh = DH.new(2048) # An alias of DH.generate(2048)
66+
* dh = OpenSSL::PKey::DH.new(2048) # An alias of OpenSSL::PKey::DH.generate(2048)
6667
*
6768
* # Reading DH parameters
68-
* dh = DH.new(File.read('parameters.pem')) # -> dh, but no public/private key yet
69-
* dh.generate_key! # -> dh with public and private key
69+
* dh_params = OpenSSL::PKey::DH.new(File.read('parameters.pem')) # loads parameters only
70+
* dh = OpenSSL::PKey.generate_key(dh_params) # generates a key pair
7071
*/
7172
static VALUE
7273
ossl_dh_initialize(int argc, VALUE *argv, VALUE self)

‎ext/openssl/ossl_pkey_ec.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,9 @@ ossl_ec_key_get_group(VALUE self)
248248
static VALUE
249249
ossl_ec_key_set_group(VALUE self, VALUE group_v)
250250
{
251+
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
252+
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
253+
#else
251254
EC_KEY *ec;
252255
EC_GROUP *group;
253256

@@ -258,6 +261,7 @@ ossl_ec_key_set_group(VALUE self, VALUE group_v)
258261
ossl_raise(eECError, "EC_KEY_set_group");
259262

260263
return group_v;
264+
#endif
261265
}
262266

263267
/*
@@ -286,6 +290,9 @@ static VALUE ossl_ec_key_get_private_key(VALUE self)
286290
*/
287291
static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key)
288292
{
293+
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
294+
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
295+
#else
289296
EC_KEY *ec;
290297
BIGNUM *bn = NULL;
291298

@@ -305,6 +312,7 @@ static VALUE ossl_ec_key_set_private_key(VALUE self, VALUE private_key)
305312
}
306313

307314
return private_key;
315+
#endif
308316
}
309317

310318
/*
@@ -333,6 +341,9 @@ static VALUE ossl_ec_key_get_public_key(VALUE self)
333341
*/
334342
static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key)
335343
{
344+
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
345+
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
346+
#else
336347
EC_KEY *ec;
337348
EC_POINT *point = NULL;
338349

@@ -352,6 +363,7 @@ static VALUE ossl_ec_key_set_public_key(VALUE self, VALUE public_key)
352363
}
353364

354365
return public_key;
366+
#endif
355367
}
356368

357369
/*
@@ -441,13 +453,17 @@ ossl_ec_key_to_der(VALUE self)
441453
*/
442454
static VALUE ossl_ec_key_generate_key(VALUE self)
443455
{
456+
#if OSSL_OPENSSL_PREREQ(3, 0, 0)
457+
rb_raise(ePKeyError, "pkeys are immutable on OpenSSL 3.0");
458+
#else
444459
EC_KEY *ec;
445460

446461
GetEC(self, ec);
447462
if (EC_KEY_generate_key(ec) != 1)
448463
ossl_raise(eECError, "EC_KEY_generate_key");
449464

450465
return self;
466+
#endif
451467
}
452468

453469
/*

‎lib/openssl/pkey.rb‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,19 @@ def public_key
4747
# * _pub_bn_ is a OpenSSL::BN, *not* the DH instance returned by
4848
# DH#public_key as that contains the DH parameters only.
4949
def compute_key(pub_bn)
50-
peer = dup
51-
peer.set_key(pub_bn, nil)
52-
derive(peer)
50+
# FIXME: This is constructing an X.509 SubjectPublicKeyInfo and is very
51+
# inefficient
52+
obj = OpenSSL::ASN1.Sequence([
53+
OpenSSL::ASN1.Sequence([
54+
OpenSSL::ASN1.ObjectId("dhKeyAgreement"),
55+
OpenSSL::ASN1.Sequence([
56+
OpenSSL::ASN1.Integer(p),
57+
OpenSSL::ASN1.Integer(g),
58+
]),
59+
]),
60+
OpenSSL::ASN1.BitString(OpenSSL::ASN1.Integer(pub_bn).to_der),
61+
])
62+
derive(OpenSSL::PKey.read(obj.to_der))
5363
end
5464

5565
# :call-seq:
@@ -61,14 +71,29 @@ def compute_key(pub_bn)
6171
# called first in order to generate the per-session keys before performing
6272
# the actual key exchange.
6373
#
74+
# <b>Deprecated in version 3.0</b>. This method is incompatible with
75+
# OpenSSL 3.0.0 or later.
76+
#
6477
# See also OpenSSL::PKey.generate_key.
6578
#
6679
# Example:
67-
# dh = OpenSSL::PKey::DH.new(2048)
68-
# public_key = dh.public_key #contains no private/public key yet
69-
# public_key.generate_key!
70-
# puts public_key.private? # => true
80+
# # DEPRECATED USAGE: This will not work on OpenSSL 3.0 or later
81+
# dh0 = OpenSSL::PKey::DH.new(2048)
82+
# dh = dh0.public_key # #public_key only copies the DH parameters (contrary to the name)
83+
# dh.generate_key!
84+
# puts dh.private? # => true
85+
# puts dh0.pub_key == dh.pub_key #=> false
86+
#
87+
# # With OpenSSL::PKey.generate_key
88+
# dh0 = OpenSSL::PKey::DH.new(2048)
89+
# dh = OpenSSL::PKey.generate_key(dh0)
90+
# puts dh0.pub_key == dh.pub_key #=> false
7191
def generate_key!
92+
if OpenSSL::OPENSSL_VERSION_NUMBER >= 0x30000000
93+
raise DHError, "OpenSSL::PKey::DH is immutable on OpenSSL 3.0; " \
94+
"use OpenSSL::PKey.generate_key instead"
95+
end
96+
7297
unless priv_key
7398
tmp = OpenSSL::PKey.generate_key(self)
7499
set_key(tmp.pub_key, tmp.priv_key)
@@ -249,9 +274,14 @@ def dsa_verify_asn1(data, sig)
249274
# This method is provided for backwards compatibility, and calls #derive
250275
# internally.
251276
def dh_compute_key(pubkey)
252-
peer = OpenSSL::PKey::EC.new(group)
253-
peer.public_key = pubkey
254-
derive(peer)
277+
obj = OpenSSL::ASN1.Sequence([
278+
OpenSSL::ASN1.Sequence([
279+
OpenSSL::ASN1.ObjectId("id-ecPublicKey"),
280+
group.to_der,
281+
]),
282+
OpenSSL::ASN1.BitString(pubkey.to_octet_string(:uncompressed)),
283+
])
284+
derive(OpenSSL::PKey.read(obj.to_der))
255285
end
256286
end
257287

‎test/openssl/test_pkey_dh.rb‎

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,19 @@ def test_new_break
2626
end
2727

2828
def test_derive_key
29-
dh1 = Fixtures.pkey("dh1024").generate_key!
30-
dh2 = Fixtures.pkey("dh1024").generate_key!
29+
params = Fixtures.pkey("dh1024")
30+
dh1 = OpenSSL::PKey.generate_key(params)
31+
dh2 = OpenSSL::PKey.generate_key(params)
3132
dh1_pub = OpenSSL::PKey.read(dh1.public_to_der)
3233
dh2_pub = OpenSSL::PKey.read(dh2.public_to_der)
34+
3335
z = dh1.g.mod_exp(dh1.priv_key, dh1.p).mod_exp(dh2.priv_key, dh1.p).to_s(2)
3436
assert_equal z, dh1.derive(dh2_pub)
3537
assert_equal z, dh2.derive(dh1_pub)
3638

39+
assert_raise(OpenSSL::PKey::PKeyError) { params.derive(dh1_pub) }
40+
assert_raise(OpenSSL::PKey::PKeyError) { dh1_pub.derive(params) }
41+
3742
assert_equal z, dh1.compute_key(dh2.pub_key)
3843
assert_equal z, dh2.compute_key(dh1.pub_key)
3944
end
@@ -74,19 +79,16 @@ def test_public_key
7479
end
7580

7681
def test_generate_key
77-
dh = Fixtures.pkey("dh1024").public_key # creates a copy
82+
# Deprecated in v3.0.0; incompatible with OpenSSL 3.0
83+
dh = Fixtures.pkey("dh1024").public_key # creates a copy with params only
7884
assert_no_key(dh)
7985
dh.generate_key!
8086
assert_key(dh)
81-
end
8287

83-
def test_key_exchange
84-
dh = Fixtures.pkey("dh1024")
8588
dh2 = dh.public_key
86-
dh.generate_key!
8789
dh2.generate_key!
8890
assert_equal(dh.compute_key(dh2.pub_key), dh2.compute_key(dh.pub_key))
89-
end
91+
end if !openssl?(3, 0, 0)
9092

9193
def test_params_ok?
9294
dh0 = Fixtures.pkey("dh1024")
@@ -105,13 +107,32 @@ def test_params_ok?
105107
end
106108

107109
def test_dup
108-
dh = Fixtures.pkey("dh1024")
109-
dh2 = dh.dup
110-
assert_equal dh.to_der, dh2.to_der # params
111-
assert_equal_params dh, dh2 # keys
112-
dh2.set_pqg(dh2.p + 1, nil, dh2.g)
113-
assert_not_equal dh2.p, dh.p
114-
assert_equal dh2.g, dh.g
110+
# Parameters only
111+
dh1 = Fixtures.pkey("dh1024")
112+
dh2 = dh1.dup
113+
assert_equal dh1.to_der, dh2.to_der
114+
assert_not_equal nil, dh1.p
115+
assert_not_equal nil, dh1.g
116+
assert_equal [dh1.p, dh1.g], [dh2.p, dh2.g]
117+
assert_equal nil, dh1.pub_key
118+
assert_equal nil, dh1.priv_key
119+
assert_equal [dh1.pub_key, dh1.priv_key], [dh2.pub_key, dh2.priv_key]
120+
121+
# PKey is immutable in OpenSSL >= 3.0
122+
if !openssl?(3, 0, 0)
123+
dh2.set_pqg(dh2.p + 1, nil, dh2.g)
124+
assert_not_equal dh2.p, dh1.p
125+
end
126+
127+
# With a key pair
128+
dh3 = OpenSSL::PKey.generate_key(Fixtures.pkey("dh1024"))
129+
dh4 = dh3.dup
130+
assert_equal dh3.to_der, dh4.to_der
131+
assert_equal dh1.to_der, dh4.to_der # encodes parameters only
132+
assert_equal [dh1.p, dh1.g], [dh4.p, dh4.g]
133+
assert_not_equal nil, dh3.pub_key
134+
assert_not_equal nil, dh3.priv_key
135+
assert_equal [dh3.pub_key, dh3.priv_key], [dh4.pub_key, dh4.priv_key]
115136
end
116137

117138
def test_marshal
@@ -123,11 +144,6 @@ def test_marshal
123144

124145
private
125146

126-
def assert_equal_params(dh1, dh2)
127-
assert_equal(dh1.g, dh2.g)
128-
assert_equal(dh1.p, dh2.p)
129-
end
130-
131147
def assert_no_key(dh)
132148
assert_equal(false, dh.public?)
133149
assert_equal(false, dh.private?)

‎test/openssl/test_pkey_dsa.rb‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,12 @@ def test_dup
208208
key = Fixtures.pkey("dsa1024")
209209
key2 = key.dup
210210
assert_equal key.params, key2.params
211-
key2.set_pqg(key2.p + 1, key2.q, key2.g)
212-
assert_not_equal key.params, key2.params
211+
212+
# PKey is immutable in OpenSSL >= 3.0
213+
if !openssl?(3, 0, 0)
214+
key2.set_pqg(key2.p + 1, key2.q, key2.g)
215+
assert_not_equal key.params, key2.params
216+
end
213217
end
214218

215219
def test_marshal

0 commit comments

Comments
 (0)