Skip to content

Commit 1eb1366

Browse files
rheniumrhenium
committed
pkey: inline {rsa,dsa,dh,ec}_instance()
Merge the code into the callers so that the wrapping Ruby object is allocated before the raw key object is allocated. This prevents possible memory leak on Ruby object allocation failure, and also reduces the lines of code.
1 parent 94aeab2 commit 1eb1366

File tree

4 files changed

+76
-156
lines changed

4 files changed

+76
-156
lines changed

‎ext/openssl/ossl_pkey_dh.c

Lines changed: 21 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -29,31 +29,6 @@
2929
VALUE cDH;
3030
VALUE eDHError;
3131

32-
/*
33-
* Public
34-
*/
35-
static VALUE
36-
dh_instance(VALUE klass, DH *dh)
37-
{
38-
EVP_PKEY *pkey;
39-
VALUE obj;
40-
41-
if (!dh) {
42-
return Qfalse;
43-
}
44-
obj = NewPKey(klass);
45-
if (!(pkey = EVP_PKEY_new())) {
46-
return Qfalse;
47-
}
48-
if (!EVP_PKEY_assign_DH(pkey, dh)) {
49-
EVP_PKEY_free(pkey);
50-
return Qfalse;
51-
}
52-
SetPKey(obj, pkey);
53-
54-
return obj;
55-
}
56-
5732
/*
5833
* Private
5934
*/
@@ -84,7 +59,7 @@ dh_generate(int size, int gen)
8459
if (!dh || !cb) {
8560
DH_free(dh);
8661
BN_GENCB_free(cb);
87-
return NULL;
62+
ossl_raise(eDHError, "malloc failure");
8863
}
8964

9065
if (rb_block_given_p())
@@ -110,12 +85,12 @@ dh_generate(int size, int gen)
11085
ossl_clear_error();
11186
rb_jump_tag(cb_arg.state);
11287
}
113-
return NULL;
88+
ossl_raise(eDHError, "DH_generate_parameters_ex");
11489
}
11590

11691
if (!DH_generate_key(dh)) {
11792
DH_free(dh);
118-
return NULL;
93+
ossl_raise(eDHError, "DH_generate_key");
11994
}
12095

12196
return dh;
@@ -136,20 +111,22 @@ dh_generate(int size, int gen)
136111
static VALUE
137112
ossl_dh_s_generate(int argc, VALUE *argv, VALUE klass)
138113
{
114+
EVP_PKEY *pkey;
139115
DH *dh ;
140116
int g = 2;
141117
VALUE size, gen, obj;
142118

143119
if (rb_scan_args(argc, argv, "11", &size, &gen) == 2) {
144120
g = NUM2INT(gen);
145121
}
122+
obj = rb_obj_alloc(klass);
123+
GetPKey(obj, pkey);
124+
146125
dh = dh_generate(NUM2INT(size), g);
147-
obj = dh_instance(klass, dh);
148-
if (obj == Qfalse) {
149-
DH_free(dh);
150-
ossl_raise(eDHError, NULL);
126+
if (!EVP_PKEY_assign_DH(pkey, dh)) {
127+
DH_free(dh);
128+
ossl_raise(eDHError, "EVP_PKEY_assign_DH");
151129
}
152-
153130
return obj;
154131
}
155132

@@ -195,9 +172,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self)
195172
if (!NIL_P(gen)) {
196173
g = NUM2INT(gen);
197174
}
198-
if (!(dh = dh_generate(NUM2INT(arg), g))) {
199-
ossl_raise(eDHError, NULL);
200-
}
175+
dh = dh_generate(NUM2INT(arg), g);
201176
}
202177
else {
203178
arg = ossl_to_der_if_possible(arg);
@@ -434,17 +409,21 @@ ossl_dh_to_text(VALUE self)
434409
static VALUE
435410
ossl_dh_to_public_key(VALUE self)
436411
{
412+
EVP_PKEY *pkey;
437413
DH *orig_dh, *dh;
438414
VALUE obj;
439415

416+
obj = rb_obj_alloc(rb_obj_class(self));
417+
GetPKey(obj, pkey);
418+
440419
GetDH(self, orig_dh);
441-
dh = DHparams_dup(orig_dh); /* err check perfomed by dh_instance */
442-
obj = dh_instance(rb_obj_class(self), dh);
443-
if (obj == Qfalse) {
444-
DH_free(dh);
445-
ossl_raise(eDHError, NULL);
420+
dh = DHparams_dup(orig_dh);
421+
if (!dh)
422+
ossl_raise(eDHError, "DHparams_dup");
423+
if (!EVP_PKEY_assign_DH(pkey, dh)) {
424+
DH_free(dh);
425+
ossl_raise(eDHError, "EVP_PKEY_assign_DH");
446426
}
447-
448427
return obj;
449428
}
450429

‎ext/openssl/ossl_pkey_dsa.c

Lines changed: 25 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -43,31 +43,6 @@ DSA_PRIVATE(VALUE obj, DSA *dsa)
4343
VALUE cDSA;
4444
VALUE eDSAError;
4545

46-
/*
47-
* Public
48-
*/
49-
static VALUE
50-
dsa_instance(VALUE klass, DSA *dsa)
51-
{
52-
EVP_PKEY *pkey;
53-
VALUE obj;
54-
55-
if (!dsa) {
56-
return Qfalse;
57-
}
58-
obj = NewPKey(klass);
59-
if (!(pkey = EVP_PKEY_new())) {
60-
return Qfalse;
61-
}
62-
if (!EVP_PKEY_assign_DSA(pkey, dsa)) {
63-
EVP_PKEY_free(pkey);
64-
return Qfalse;
65-
}
66-
SetPKey(obj, pkey);
67-
68-
return obj;
69-
}
70-
7146
/*
7247
* Private
7348
*/
@@ -100,9 +75,9 @@ dsa_generate(int size)
10075
unsigned long h;
10176

10277
if (!dsa || !cb) {
103-
DSA_free(dsa);
104-
BN_GENCB_free(cb);
105-
return NULL;
78+
DSA_free(dsa);
79+
BN_GENCB_free(cb);
80+
ossl_raise(eDSAError, "malloc failure");
10681
}
10782

10883
if (rb_block_given_p())
@@ -132,12 +107,12 @@ dsa_generate(int size)
132107
ossl_clear_error();
133108
rb_jump_tag(cb_arg.state);
134109
}
135-
return NULL;
110+
ossl_raise(eDSAError, "DSA_generate_parameters_ex");
136111
}
137112

138113
if (!DSA_generate_key(dsa)) {
139-
DSA_free(dsa);
140-
return NULL;
114+
DSA_free(dsa);
115+
ossl_raise(eDSAError, "DSA_generate_key");
141116
}
142117

143118
return dsa;
@@ -157,14 +132,18 @@ dsa_generate(int size)
157132
static VALUE
158133
ossl_dsa_s_generate(VALUE klass, VALUE size)
159134
{
160-
DSA *dsa = dsa_generate(NUM2INT(size)); /* err handled by dsa_instance */
161-
VALUE obj = dsa_instance(klass, dsa);
135+
EVP_PKEY *pkey;
136+
DSA *dsa;
137+
VALUE obj;
162138

163-
if (obj == Qfalse) {
164-
DSA_free(dsa);
165-
ossl_raise(eDSAError, NULL);
166-
}
139+
obj = rb_obj_alloc(klass);
140+
GetPKey(obj, pkey);
167141

142+
dsa = dsa_generate(NUM2INT(size));
143+
if (!EVP_PKEY_assign_DSA(pkey, dsa)) {
144+
DSA_free(dsa);
145+
ossl_raise(eDSAError, "EVP_PKEY_assign_DSA");
146+
}
168147
return obj;
169148
}
170149

@@ -460,20 +439,23 @@ ossl_dsa_to_text(VALUE self)
460439
static VALUE
461440
ossl_dsa_to_public_key(VALUE self)
462441
{
463-
EVP_PKEY *pkey;
442+
EVP_PKEY *pkey, *pkey_new;
464443
DSA *dsa;
465444
VALUE obj;
466445

467446
GetPKeyDSA(self, pkey);
468-
/* err check performed by dsa_instance */
447+
obj = rb_obj_alloc(rb_obj_class(self));
448+
GetPKey(obj, pkey_new);
449+
469450
#define DSAPublicKey_dup(dsa) (DSA *)ASN1_dup( \
470451
(i2d_of_void *)i2d_DSAPublicKey, (d2i_of_void *)d2i_DSAPublicKey, (char *)(dsa))
471452
dsa = DSAPublicKey_dup(EVP_PKEY_get0_DSA(pkey));
472453
#undef DSAPublicKey_dup
473-
obj = dsa_instance(rb_obj_class(self), dsa);
474-
if (obj == Qfalse) {
475-
DSA_free(dsa);
476-
ossl_raise(eDSAError, NULL);
454+
if (!dsa)
455+
ossl_raise(eDSAError, "DSAPublicKey_dup");
456+
if (!EVP_PKEY_assign_DSA(pkey_new, dsa)) {
457+
DSA_free(dsa);
458+
ossl_raise(eDSAError, "EVP_PKEY_assign_DSA");
477459
}
478460
return obj;
479461
}

‎ext/openssl/ossl_pkey_ec.c

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,27 +63,6 @@ static ID id_i_group;
6363
static VALUE ec_group_new(const EC_GROUP *group);
6464
static VALUE ec_point_new(const EC_POINT *point, const EC_GROUP *group);
6565

66-
static VALUE ec_instance(VALUE klass, EC_KEY *ec)
67-
{
68-
EVP_PKEY *pkey;
69-
VALUE obj;
70-
71-
if (!ec) {
72-
return Qfalse;
73-
}
74-
obj = NewPKey(klass);
75-
if (!(pkey = EVP_PKEY_new())) {
76-
return Qfalse;
77-
}
78-
if (!EVP_PKEY_assign_EC_KEY(pkey, ec)) {
79-
EVP_PKEY_free(pkey);
80-
return Qfalse;
81-
}
82-
SetPKey(obj, pkey);
83-
84-
return obj;
85-
}
86-
8766
/*
8867
* Creates a new EC_KEY on the EC group obj. arg can be an EC::Group or a String
8968
* representing an OID.
@@ -130,17 +109,18 @@ ec_key_new_from_group(VALUE arg)
130109
static VALUE
131110
ossl_ec_key_s_generate(VALUE klass, VALUE arg)
132111
{
112+
EVP_PKEY *pkey;
133113
EC_KEY *ec;
134114
VALUE obj;
135115

136-
ec = ec_key_new_from_group(arg);
116+
obj = rb_obj_alloc(klass);
117+
GetPKey(obj, pkey);
137118

138-
obj = ec_instance(klass, ec);
139-
if (obj == Qfalse) {
140-
EC_KEY_free(ec);
141-
ossl_raise(eECError, NULL);
119+
ec = ec_key_new_from_group(arg);
120+
if (!EVP_PKEY_assign_EC_KEY(pkey, ec)) {
121+
EC_KEY_free(ec);
122+
ossl_raise(eECError, "EVP_PKEY_assign_EC_KEY");
142123
}
143-
144124
if (!EC_KEY_generate_key(ec))
145125
ossl_raise(eECError, "EC_KEY_generate_key");
146126

0 commit comments

Comments
 (0)