Merge pull request #328 from rhenium/ky/pkey-refactor-serialization

pkey: refactor PEM/DER serialization code

14 files changed, 282 insertions, 514 deletions

diff --git a/ext/openssl/ossl.c b/ext/openssl/ossl.c
index 2f54b861..ab418764 100644
--- a/ext/openssl/ossl.c
+++ b/ext/openssl/ossl.c
@@ -679,13 +679,13 @@ ossl_crypto_fixed_length_secure_compare(VALUE dummy, VALUE str1, VALUE str2)
  *
  * A key can also be loaded from a file.
  *
- *   key2 = OpenSSL::PKey::RSA.new File.read 'private_key.pem'
+ *   key2 = OpenSSL::PKey.read File.read 'private_key.pem'
  *   key2.public? # => true
  *   key2.private? # => true
  *
  * or
  *
- *   key3 = OpenSSL::PKey::RSA.new File.read 'public_key.pem'
+ *   key3 = OpenSSL::PKey.read File.read 'public_key.pem'
  *   key3.public? # => true
  *   key3.private? # => false
  *
@@ -697,7 +697,7 @@ ossl_crypto_fixed_length_secure_compare(VALUE dummy, VALUE str1, VALUE str2)
  *
  *   key4_pem = File.read 'private.secure.pem'
  *   pass_phrase = 'my secure pass phrase goes here'
- *   key4 = OpenSSL::PKey::RSA.new key4_pem, pass_phrase
+ *   key4 = OpenSSL::PKey.read key4_pem, pass_phrase
  *
  * == RSA Encryption
  *
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index 23204087..610a83fd 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -95,7 +95,7 @@ const rb_data_type_t ossl_evp_pkey_type = {
 static VALUE
 pkey_new0(EVP_PKEY *pkey)
 {
-    VALUE obj;
+    VALUE klass, obj;
     int type;
 
     if (!pkey || (type = EVP_PKEY_base_id(pkey)) == EVP_PKEY_NONE)
@@ -103,26 +103,22 @@ pkey_new0(EVP_PKEY *pkey)
 
     switch (type) {
 #if !defined(OPENSSL_NO_RSA)
-    case EVP_PKEY_RSA:
-	return ossl_rsa_new(pkey);
+      case EVP_PKEY_RSA: klass = cRSA; break;
 #endif
 #if !defined(OPENSSL_NO_DSA)
-    case EVP_PKEY_DSA:
-	return ossl_dsa_new(pkey);
+      case EVP_PKEY_DSA: klass = cDSA; break;
 #endif
 #if !defined(OPENSSL_NO_DH)
-    case EVP_PKEY_DH:
-	return ossl_dh_new(pkey);
+      case EVP_PKEY_DH:  klass = cDH; break;
 #endif
 #if !defined(OPENSSL_NO_EC)
-    case EVP_PKEY_EC:
-	return ossl_ec_new(pkey);
+      case EVP_PKEY_EC:  klass = cEC; break;
 #endif
-    default:
-	obj = NewPKey(cPKey);
-	SetPKey(obj, pkey);
-	return obj;
+      default:           klass = cPKey; break;
     }
+    obj = NewPKey(klass);
+    SetPKey(obj, pkey);
+    return obj;
 }
 
 VALUE
@@ -140,6 +136,35 @@ ossl_pkey_new(EVP_PKEY *pkey)
     return obj;
 }
 
+EVP_PKEY *
+ossl_pkey_read_generic(BIO *bio, VALUE pass)
+{
+    void *ppass = (void *)pass;
+    EVP_PKEY *pkey;
+
+    if ((pkey = d2i_PrivateKey_bio(bio, NULL)))
+	goto out;
+    OSSL_BIO_reset(bio);
+    if ((pkey = d2i_PKCS8PrivateKey_bio(bio, NULL, ossl_pem_passwd_cb, ppass)))
+	goto out;
+    OSSL_BIO_reset(bio);
+    if ((pkey = d2i_PUBKEY_bio(bio, NULL)))
+	goto out;
+    OSSL_BIO_reset(bio);
+    /* PEM_read_bio_PrivateKey() also parses PKCS #8 formats */
+    if ((pkey = PEM_read_bio_PrivateKey(bio, NULL, ossl_pem_passwd_cb, ppass)))
+	goto out;
+    OSSL_BIO_reset(bio);
+    if ((pkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL)))
+	goto out;
+    OSSL_BIO_reset(bio);
+    if ((pkey = PEM_read_bio_Parameters(bio, NULL)))
+	goto out;
+
+  out:
+    return pkey;
+}
+
 /*
  *  call-seq:
  *     OpenSSL::PKey.read(string [, pwd ]) -> PKey
@@ -164,30 +189,11 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self)
     VALUE data, pass;
 
     rb_scan_args(argc, argv, "11", &data, &pass);
-    pass = ossl_pem_passwd_value(pass);
-
     bio = ossl_obj2bio(&data);
-    if ((pkey = d2i_PrivateKey_bio(bio, NULL)))
-	goto ok;
-    OSSL_BIO_reset(bio);
-    if ((pkey = d2i_PKCS8PrivateKey_bio(bio, NULL, ossl_pem_passwd_cb, (void *)pass)))
-	goto ok;
-    OSSL_BIO_reset(bio);
-    if ((pkey = d2i_PUBKEY_bio(bio, NULL)))
-	goto ok;
-    OSSL_BIO_reset(bio);
-    /* PEM_read_bio_PrivateKey() also parses PKCS #8 formats */
-    if ((pkey = PEM_read_bio_PrivateKey(bio, NULL, ossl_pem_passwd_cb, (void *)pass)))
-	goto ok;
-    OSSL_BIO_reset(bio);
-    if ((pkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL)))
-	goto ok;
-
-    BIO_free(bio);
-    ossl_raise(ePKeyError, "Could not parse PKey");
-
-ok:
+    pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass));
     BIO_free(bio);
+    if (!pkey)
+	ossl_raise(ePKeyError, "Could not parse PKey");
     return ossl_pkey_new(pkey);
 }
 
@@ -335,6 +341,52 @@ ossl_pkey_inspect(VALUE self)
                       OBJ_nid2sn(nid));
 }
 
+VALUE
+ossl_pkey_export_traditional(int argc, VALUE *argv, VALUE self, int to_der)
+{
+    EVP_PKEY *pkey;
+    VALUE cipher, pass;
+    const EVP_CIPHER *enc = NULL;
+    BIO *bio;
+
+    GetPKey(self, pkey);
+    rb_scan_args(argc, argv, "02", &cipher, &pass);
+    if (!NIL_P(cipher)) {
+	enc = ossl_evp_get_cipherbyname(cipher);
+	pass = ossl_pem_passwd_value(pass);
+    }
+
+    bio = BIO_new(BIO_s_mem());
+    if (!bio)
+	ossl_raise(ePKeyError, "BIO_new");
+    if (to_der) {
+	if (!i2d_PrivateKey_bio(bio, pkey)) {
+	    BIO_free(bio);
+	    ossl_raise(ePKeyError, "i2d_PrivateKey_bio");
+	}
+    }
+    else {
+#if OPENSSL_VERSION_NUMBER >= 0x10100000 && !defined(LIBRESSL_VERSION_NUMBER)
+	if (!PEM_write_bio_PrivateKey_traditional(bio, pkey, enc, NULL, 0,
+						  ossl_pem_passwd_cb,
+						  (void *)pass)) {
+#else
+	char pem_str[80];
+	const char *aname;
+
+	EVP_PKEY_asn1_get0_info(NULL, NULL, NULL, NULL, &aname, pkey->ameth);
+	snprintf(pem_str, sizeof(pem_str), "%s PRIVATE KEY", aname);
+	if (!PEM_ASN1_write_bio((i2d_of_void *)i2d_PrivateKey, pem_str, bio,
+				pkey, enc, NULL, 0, ossl_pem_passwd_cb,
+				(void *)pass)) {
+#endif
+	    BIO_free(bio);
+	    ossl_raise(ePKeyError, "PEM_write_bio_PrivateKey_traditional");
+	}
+    }
+    return ossl_membio2str(bio);
+}
+
 static VALUE
 do_pkcs8_export(int argc, VALUE *argv, VALUE self, int to_der)
 {
@@ -404,8 +456,8 @@ ossl_pkey_private_to_pem(int argc, VALUE *argv, VALUE self)
     return do_pkcs8_export(argc, argv, self, 0);
 }
 
-static VALUE
-do_spki_export(VALUE self, int to_der)
+VALUE
+ossl_pkey_export_spki(VALUE self, int to_der)
 {
     EVP_PKEY *pkey;
     BIO *bio;
@@ -438,7 +490,7 @@ do_spki_export(VALUE self, int to_der)
 static VALUE
 ossl_pkey_public_to_der(VALUE self)
 {
-    return do_spki_export(self, 1);
+    return ossl_pkey_export_spki(self, 1);
 }
 
 /*
@@ -450,7 +502,7 @@ ossl_pkey_public_to_der(VALUE self)
 static VALUE
 ossl_pkey_public_to_pem(VALUE self)
 {
-    return do_spki_export(self, 0);
+    return ossl_pkey_export_spki(self, 0);
 }
 
 /*
diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h
index 0db59305..7dbaed47 100644
--- a/ext/openssl/ossl_pkey.h
+++ b/ext/openssl/ossl_pkey.h
@@ -45,9 +45,24 @@ void ossl_generate_cb_stop(void *ptr);
 
 VALUE ossl_pkey_new(EVP_PKEY *);
 void ossl_pkey_check_public_key(const EVP_PKEY *);
+EVP_PKEY *ossl_pkey_read_generic(BIO *, VALUE);
 EVP_PKEY *GetPKeyPtr(VALUE);
 EVP_PKEY *DupPKeyPtr(VALUE);
 EVP_PKEY *GetPrivPKeyPtr(VALUE);
+
+/*
+ * Serializes _self_ in X.509 SubjectPublicKeyInfo format and returns the
+ * resulting String. Sub-classes use this when overriding #to_der.
+ */
+VALUE ossl_pkey_export_spki(VALUE self, int to_der);
+/*
+ * Serializes the private key _self_ in the traditional private key format
+ * and returns the resulting String. Sub-classes use this when overriding
+ * #to_der.
+ */
+VALUE ossl_pkey_export_traditional(int argc, VALUE *argv, VALUE self,
+				   int to_der);
+
 void Init_ossl_pkey(void);
 
 /*
@@ -56,7 +71,6 @@ void Init_ossl_pkey(void);
 extern VALUE cRSA;
 extern VALUE eRSAError;
 
-VALUE ossl_rsa_new(EVP_PKEY *);
 void Init_ossl_rsa(void);
 
 /*
@@ -65,7 +79,6 @@ void Init_ossl_rsa(void);
 extern VALUE cDSA;
 extern VALUE eDSAError;
 
-VALUE ossl_dsa_new(EVP_PKEY *);
 void Init_ossl_dsa(void);
 
 /*
@@ -74,7 +87,6 @@ void Init_ossl_dsa(void);
 extern VALUE cDH;
 extern VALUE eDHError;
 
-VALUE ossl_dh_new(EVP_PKEY *);
 void Init_ossl_dh(void);
 
 /*
diff --git a/ext/openssl/ossl_pkey_dh.c b/ext/openssl/ossl_pkey_dh.c
index bf4e3f93..bc50e556 100644
--- a/ext/openssl/ossl_pkey_dh.c
+++ b/ext/openssl/ossl_pkey_dh.c
@@ -30,52 +30,6 @@ VALUE cDH;
 VALUE eDHError;
 
 /*
- * Public
- */
-static VALUE
-dh_instance(VALUE klass, DH *dh)
-{
-    EVP_PKEY *pkey;
-    VALUE obj;
-
-    if (!dh) {
-	return Qfalse;
-    }
-    obj = NewPKey(klass);
-    if (!(pkey = EVP_PKEY_new())) {
-	return Qfalse;
-    }
-    if (!EVP_PKEY_assign_DH(pkey, dh)) {
-	EVP_PKEY_free(pkey);
-	return Qfalse;
-    }
-    SetPKey(obj, pkey);
-
-    return obj;
-}
-
-VALUE
-ossl_dh_new(EVP_PKEY *pkey)
-{
-    VALUE obj;
-
-    if (!pkey) {
-	obj = dh_instance(cDH, DH_new());
-    } else {
-	obj = NewPKey(cDH);
-	if (EVP_PKEY_base_id(pkey) != EVP_PKEY_DH) {
-	    ossl_raise(rb_eTypeError, "Not a DH key!");
-	}
-	SetPKey(obj, pkey);
-    }
-    if (obj == Qfalse) {
-	ossl_raise(eDHError, NULL);
-    }
-
-    return obj;
-}
-
-/*
  * Private
  */
 struct dh_blocking_gen_arg {
@@ -105,7 +59,7 @@ dh_generate(int size, int gen)
     if (!dh || !cb) {
 	DH_free(dh);
 	BN_GENCB_free(cb);
-	return NULL;
+        ossl_raise(eDHError, "malloc failure");
     }
 
     if (rb_block_given_p())
@@ -131,12 +85,12 @@ dh_generate(int size, int gen)
 	    ossl_clear_error();
 	    rb_jump_tag(cb_arg.state);
 	}
-	return NULL;
+        ossl_raise(eDHError, "DH_generate_parameters_ex");
     }
 
     if (!DH_generate_key(dh)) {
         DH_free(dh);
-        return NULL;
+        ossl_raise(eDHError, "DH_generate_key");
     }
 
     return dh;
@@ -157,6 +111,7 @@ dh_generate(int size, int gen)
 static VALUE
 ossl_dh_s_generate(int argc, VALUE *argv, VALUE klass)
 {
+    EVP_PKEY *pkey;
     DH *dh ;
     int g = 2;
     VALUE size, gen, obj;
@@ -164,13 +119,14 @@ ossl_dh_s_generate(int argc, VALUE *argv, VALUE klass)
     if (rb_scan_args(argc, argv, "11", &size, &gen) == 2) {
 	g = NUM2INT(gen);
     }
+    obj = rb_obj_alloc(klass);
+    GetPKey(obj, pkey);
+
     dh = dh_generate(NUM2INT(size), g);
-    obj = dh_instance(klass, dh);
-    if (obj == Qfalse) {
-	DH_free(dh);
-	ossl_raise(eDHError, NULL);
+    if (!EVP_PKEY_assign_DH(pkey, dh)) {
+        DH_free(dh);
+        ossl_raise(eDHError, "EVP_PKEY_assign_DH");
     }
-
     return obj;
 }
 
@@ -216,9 +172,7 @@ ossl_dh_initialize(int argc, VALUE *argv, VALUE self)
 	if (!NIL_P(gen)) {
 	    g = NUM2INT(gen);
 	}
-	if (!(dh = dh_generate(NUM2INT(arg), g))) {
-	    ossl_raise(eDHError, NULL);
-	}
+        dh = dh_generate(NUM2INT(arg), g);
     }
     else {
 	arg = ossl_to_der_if_possible(arg);
@@ -455,17 +409,21 @@ ossl_dh_to_text(VALUE self)
 static VALUE
 ossl_dh_to_public_key(VALUE self)
 {
+    EVP_PKEY *pkey;
     DH *orig_dh, *dh;
     VALUE obj;
 
+    obj = rb_obj_alloc(rb_obj_class(self));
+    GetPKey(obj, pkey);
+
     GetDH(self, orig_dh);
-    dh = DHparams_dup(orig_dh); /* err check perfomed by dh_instance */
-    obj = dh_instance(rb_obj_class(self), dh);
-    if (obj == Qfalse) {
-	DH_free(dh);
-	ossl_raise(eDHError, NULL);
+    dh = DHparams_dup(orig_dh);
+    if (!dh)
+        ossl_raise(eDHError, "DHparams_dup");
+    if (!EVP_PKEY_assign_DH(pkey, dh)) {
+        DH_free(dh);
+        ossl_raise(eDHError, "EVP_PKEY_assign_DH");
     }
-
     return obj;
 }
 
diff --git a/ext/openssl/ossl_pkey_dsa.c b/ext/openssl/ossl_pkey_dsa.c
index 431c20e0..0e68f7f2 100644
--- a/ext/openssl/ossl_pkey_dsa.c
+++ b/ext/openssl/ossl_pkey_dsa.c
@@ -44,52 +44,6 @@ VALUE cDSA;
 VALUE eDSAError;
 
 /*
- * Public
- */
-static VALUE
-dsa_instance(VALUE klass, DSA *dsa)
-{
-    EVP_PKEY *pkey;
-    VALUE obj;
-
-    if (!dsa) {
-	return Qfalse;
-    }
-    obj = NewPKey(klass);
-    if (!(pkey = EVP_PKEY_new())) {
-	return Qfalse;
-    }
-    if (!EVP_PKEY_assign_DSA(pkey, dsa)) {
-	EVP_PKEY_free(pkey);
-	return Qfalse;
-    }
-    SetPKey(obj, pkey);
-
-    return obj;
-}
-
-VALUE
-ossl_dsa_new(EVP_PKEY *pkey)
-{
-    VALUE obj;
-
-    if (!pkey) {
-	obj = dsa_instance(cDSA, DSA_new());
-    } else {
-	obj = NewPKey(cDSA);
-	if (EVP_PKEY_base_id(pkey) != EVP_PKEY_DSA) {
-	    ossl_raise(rb_eTypeError, "Not a DSA key!");
-	}
-	SetPKey(obj, pkey);
-    }
-    if (obj == Qfalse) {
-	ossl_raise(eDSAError, NULL);
-    }
-
-    return obj;
-}
-
-/*
  * Private
  */
 struct dsa_blocking_gen_arg {
@@ -121,9 +75,9 @@ dsa_generate(int size)
     unsigned long h;
 
     if (!dsa || !cb) {
-	DSA_free(dsa);
-	BN_GENCB_free(cb);
-	return NULL;
+        DSA_free(dsa);
+        BN_GENCB_free(cb);
+        ossl_raise(eDSAError, "malloc failure");
     }
 
     if (rb_block_given_p())
@@ -153,12 +107,12 @@ dsa_generate(int size)
 	    ossl_clear_error();
 	    rb_jump_tag(cb_arg.state);
 	}
-	return NULL;
+        ossl_raise(eDSAError, "DSA_generate_parameters_ex");
     }
 
     if (!DSA_generate_key(dsa)) {
-	DSA_free(dsa);
-	return NULL;
+        DSA_free(dsa);
+        ossl_raise(eDSAError, "DSA_generate_key");
     }
 
     return dsa;
@@ -178,14 +132,18 @@ dsa_generate(int size)
 static VALUE
 ossl_dsa_s_generate(VALUE klass, VALUE size)
 {
-    DSA *dsa = dsa_generate(NUM2INT(size)); /* err handled by dsa_instance */
-    VALUE obj = dsa_instance(klass, dsa);
+    EVP_PKEY *pkey;
+    DSA *dsa;
+    VALUE obj;
 
-    if (obj == Qfalse) {
-	DSA_free(dsa);
-	ossl_raise(eDSAError, NULL);
-    }
+    obj = rb_obj_alloc(klass);
+    GetPKey(obj, pkey);
 
+    dsa = dsa_generate(NUM2INT(size));
+    if (!EVP_PKEY_assign_DSA(pkey, dsa)) {
+        DSA_free(dsa);
+        ossl_raise(eDSAError, "EVP_PKEY_assign_DSA");
+    }
     return obj;
 }
 
@@ -212,37 +170,34 @@ ossl_dsa_s_generate(VALUE klass, VALUE size)
 static VALUE
 ossl_dsa_initialize(int argc, VALUE *argv, VALUE self)
 {
-    EVP_PKEY *pkey;
-    DSA *dsa;
+    EVP_PKEY *pkey, *tmp;
+    DSA *dsa = NULL;
     BIO *in;
     VALUE arg, pass;
 
     GetPKey(self, pkey);
-    if(rb_scan_args(argc, argv, "02", &arg, &pass) == 0) {
+    rb_scan_args(argc, argv, "02", &arg, &pass);
+    if (argc == 0) {
         dsa = DSA_new();
+        if (!dsa)
+            ossl_raise(eDSAError, "DSA_new");
     }
-    else if (RB_INTEGER_TYPE_P(arg)) {
-	if (!(dsa = dsa_generate(NUM2INT(arg)))) {
-	    ossl_raise(eDSAError, NULL);
-	}
+    else if (argc == 1 && RB_INTEGER_TYPE_P(arg)) {
+        dsa = dsa_generate(NUM2INT(arg));
     }
     else {
 	pass = ossl_pem_passwd_value(pass);
 	arg = ossl_to_der_if_possible(arg);
 	in = ossl_obj2bio(&arg);
-	dsa = PEM_read_bio_DSAPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass);
-	if (!dsa) {
-	    OSSL_BIO_reset(in);
-	    dsa = PEM_read_bio_DSA_PUBKEY(in, NULL, NULL, NULL);
-	}
-	if (!dsa) {
-	    OSSL_BIO_reset(in);
-	    dsa = d2i_DSAPrivateKey_bio(in, NULL);
-	}
-	if (!dsa) {
-	    OSSL_BIO_reset(in);
-	    dsa = d2i_DSA_PUBKEY_bio(in, NULL);
-	}
+
+        tmp = ossl_pkey_read_generic(in, pass);
+        if (tmp) {
+            if (EVP_PKEY_base_id(tmp) != EVP_PKEY_DSA)
+                rb_raise(eDSAError, "incorrect pkey type: %s",
+                         OBJ_nid2sn(EVP_PKEY_base_id(tmp)));
+            dsa = EVP_PKEY_get1_DSA(tmp);
+            EVP_PKEY_free(tmp);
+        }
 	if (!dsa) {
 	    OSSL_BIO_reset(in);
 #define PEM_read_bio_DSAPublicKey(bp,x,cb,u) (DSA *)PEM_ASN1_read_bio( \
@@ -341,34 +296,12 @@ static VALUE
 ossl_dsa_export(int argc, VALUE *argv, VALUE self)
 {
     DSA *dsa;
-    BIO *out;
-    const EVP_CIPHER *ciph = NULL;
-    VALUE cipher, pass, str;
 
     GetDSA(self, dsa);
-    rb_scan_args(argc, argv, "02", &cipher, &pass);
-    if (!NIL_P(cipher)) {
-	ciph = ossl_evp_get_cipherbyname(cipher);
-	pass = ossl_pem_passwd_value(pass);
-    }
-    if (!(out = BIO_new(BIO_s_mem()))) {
-	ossl_raise(eDSAError, NULL);
-    }
-    if (DSA_HAS_PRIVATE(dsa)) {
-	if (!PEM_write_bio_DSAPrivateKey(out, dsa, ciph, NULL, 0,
-					 ossl_pem_passwd_cb, (void *)pass)){
-	    BIO_free(out);
-	    ossl_raise(eDSAError, NULL);
-	}
-    } else {
-	if (!PEM_write_bio_DSA_PUBKEY(out, dsa)) {
-	    BIO_free(out);
-	    ossl_raise(eDSAError, NULL);
-	}
-    }
-    str = ossl_membio2str(out);
-
-    return str;
+    if (DSA_HAS_PRIVATE(dsa))
+        return ossl_pkey_export_traditional(argc, argv, self, 0);
+    else
+        return ossl_pkey_export_spki(self, 0);
 }
 
 /*
@@ -382,25 +315,12 @@ static VALUE
 ossl_dsa_to_der(VALUE self)
 {
     DSA *dsa;
-    int (*i2d_func)(DSA *, unsigned char **);
-    unsigned char *p;
-    long len;
-    VALUE str;
 
     GetDSA(self, dsa);
-    if(DSA_HAS_PRIVATE(dsa))
-	i2d_func = (int (*)(DSA *,unsigned char **))i2d_DSAPrivateKey;
+    if (DSA_HAS_PRIVATE(dsa))
+        return ossl_pkey_export_traditional(0, NULL, self, 1);
     else
-	i2d_func = i2d_DSA_PUBKEY;
-    if((len = i2d_func(dsa, NULL)) <= 0)
-	ossl_raise(eDSAError, NULL);
-    str = rb_str_new(0, len);
-    p = (unsigned char *)RSTRING_PTR(str);
-    if(i2d_func(dsa, &p) < 0)
-	ossl_raise(eDSAError, NULL);
-    ossl_str_adjust(str, p);
-
-    return str;
+        return ossl_pkey_export_spki(self, 1);
 }
 
 
@@ -481,20 +401,23 @@ ossl_dsa_to_text(VALUE self)
 static VALUE
 ossl_dsa_to_public_key(VALUE self)
 {
-    EVP_PKEY *pkey;
+    EVP_PKEY *pkey, *pkey_new;
     DSA *dsa;
     VALUE obj;
 
     GetPKeyDSA(self, pkey);
-    /* err check performed by dsa_instance */
+    obj = rb_obj_alloc(rb_obj_class(self));
+    GetPKey(obj, pkey_new);
+
 #define DSAPublicKey_dup(dsa) (DSA *)ASN1_dup( \
 	(i2d_of_void *)i2d_DSAPublicKey, (d2i_of_void *)d2i_DSAPublicKey, (char *)(dsa))
     dsa = DSAPublicKey_dup(EVP_PKEY_get0_DSA(pkey));
 #undef DSAPublicKey_dup
-    obj = dsa_instance(rb_obj_class(self), dsa);
-    if (obj == Qfalse) {
-	DSA_free(dsa);
-	ossl_raise(eDSAError, NULL);
+    if (!dsa)
+        ossl_raise(eDSAError, "DSAPublicKey_dup");
+    if (!EVP_PKEY_assign_DSA(pkey_new, dsa)) {
+        DSA_free(dsa);
+        ossl_raise(eDSAError, "EVP_PKEY_assign_DSA");
     }
     return obj;
 }
diff --git a/ext/openssl/ossl_pkey_ec.c b/ext/openssl/ossl_pkey_ec.c
index fc2bc6c8..6fe2533e 100644
--- a/ext/openssl/ossl_pkey_ec.c
+++ b/ext/openssl/ossl_pkey_ec.c
@@ -63,47 +63,6 @@ static ID id_i_group;
 static VALUE ec_group_new(const EC_GROUP *group);
 static VALUE ec_point_new(const EC_POINT *point, const EC_GROUP *group);
 
-static VALUE ec_instance(VALUE klass, EC_KEY *ec)
-{
-    EVP_PKEY *pkey;
-    VALUE obj;
-
-    if (!ec) {
-	return Qfalse;
-    }
-    obj = NewPKey(klass);
-    if (!(pkey = EVP_PKEY_new())) {
-	return Qfalse;
-    }
-    if (!EVP_PKEY_assign_EC_KEY(pkey, ec)) {
-	EVP_PKEY_free(pkey);
-	return Qfalse;
-    }
-    SetPKey(obj, pkey);
-
-    return obj;
-}
-
-VALUE ossl_ec_new(EVP_PKEY *pkey)
-{
-    VALUE obj;
-
-    if (!pkey) {
-	obj = ec_instance(cEC, EC_KEY_new());
-    } else {
-	obj = NewPKey(cEC);
-	if (EVP_PKEY_base_id(pkey) != EVP_PKEY_EC) {
-	    ossl_raise(rb_eTypeError, "Not a EC key!");
-	}
-	SetPKey(obj, pkey);
-    }
-    if (obj == Qfalse) {
-	ossl_raise(eECError, NULL);
-    }
-
-    return obj;
-}
-
 /*
  * Creates a new EC_KEY on the EC group obj. arg can be an EC::Group or a String
  * representing an OID.
@@ -150,17 +109,18 @@ ec_key_new_from_group(VALUE arg)
 static VALUE
 ossl_ec_key_s_generate(VALUE klass, VALUE arg)
 {
+    EVP_PKEY *pkey;
     EC_KEY *ec;
     VALUE obj;
 
-    ec = ec_key_new_from_group(arg);
+    obj = rb_obj_alloc(klass);
+    GetPKey(obj, pkey);
 
-    obj = ec_instance(klass, ec);
-    if (obj == Qfalse) {
-	EC_KEY_free(ec);
-	ossl_raise(eECError, NULL);
+    ec = ec_key_new_from_group(arg);
+    if (!EVP_PKEY_assign_EC_KEY(pkey, ec)) {
+        EC_KEY_free(ec);
+        ossl_raise(eECError, "EVP_PKEY_assign_EC_KEY");
     }
-
     if (!EC_KEY_generate_key(ec))
 	ossl_raise(eECError, "EC_KEY_generate_key");
 
@@ -181,7 +141,7 @@ ossl_ec_key_s_generate(VALUE klass, VALUE arg)
 static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self)
 {
     EVP_PKEY *pkey;
-    EC_KEY *ec;
+    EC_KEY *ec = NULL;
     VALUE arg, pass;
 
     GetPKey(self, pkey);
@@ -202,24 +162,17 @@ static VALUE ossl_ec_key_initialize(int argc, VALUE *argv, VALUE self)
     } else if (rb_obj_is_kind_of(arg, cEC_GROUP)) {
 	ec = ec_key_new_from_group(arg);
     } else {
-	BIO *in;
-
-	pass = ossl_pem_passwd_value(pass);
-	in = ossl_obj2bio(&arg);
-
-	ec = PEM_read_bio_ECPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass);
-	if (!ec) {
-	    OSSL_BIO_reset(in);
-	    ec = PEM_read_bio_EC_PUBKEY(in, NULL, ossl_pem_passwd_cb, (void *)pass);
-	}
-	if (!ec) {
-	    OSSL_BIO_reset(in);
-	    ec = d2i_ECPrivateKey_bio(in, NULL);
-	}
-	if (!ec) {
-	    OSSL_BIO_reset(in);
-	    ec = d2i_EC_PUBKEY_bio(in, NULL);
-	}
+        BIO *in = ossl_obj2bio(&arg);
+        EVP_PKEY *tmp;
+        pass = ossl_pem_passwd_value(pass);
+        tmp = ossl_pkey_read_generic(in, pass);
+        if (tmp) {
+            if (EVP_PKEY_base_id(tmp) != EVP_PKEY_EC)
+                rb_raise(eECError, "incorrect pkey type: %s",
+                         OBJ_nid2sn(EVP_PKEY_base_id(tmp)));
+            ec = EVP_PKEY_get1_EC_KEY(tmp);
+            EVP_PKEY_free(tmp);
+        }
 	BIO_free(in);
 
 	if (!ec) {
@@ -425,66 +378,6 @@ static VALUE ossl_ec_key_is_private(VALUE self)
     return EC_KEY_get0_private_key(ec) ? Qtrue : Qfalse;
 }
 
-static VALUE ossl_ec_key_to_string(VALUE self, VALUE ciph, VALUE pass, int format)
-{
-    EC_KEY *ec;
-    BIO *out;
-    int i = -1;
-    int private = 0;
-    VALUE str;
-    const EVP_CIPHER *cipher = NULL;
-
-    GetEC(self, ec);
-
-    if (EC_KEY_get0_public_key(ec) == NULL)
-        ossl_raise(eECError, "can't export - no public key set");
-
-    if (EC_KEY_check_key(ec) != 1)
-	ossl_raise(eECError, "can't export - EC_KEY_check_key failed");
-
-    if (EC_KEY_get0_private_key(ec))
-        private = 1;
-
-    if (!NIL_P(ciph)) {
-	cipher = ossl_evp_get_cipherbyname(ciph);
-	pass = ossl_pem_passwd_value(pass);
-    }
-
-    if (!(out = BIO_new(BIO_s_mem())))
-        ossl_raise(eECError, "BIO_new(BIO_s_mem())");
-
-    switch(format) {
-    case EXPORT_PEM:
-    	if (private) {
-            i = PEM_write_bio_ECPrivateKey(out, ec, cipher, NULL, 0, ossl_pem_passwd_cb, (void *)pass);
-    	} else {
-            i = PEM_write_bio_EC_PUBKEY(out, ec);
-        }
-
-    	break;
-    case EXPORT_DER:
-        if (private) {
-            i = i2d_ECPrivateKey_bio(out, ec);
-        } else {
-            i = i2d_EC_PUBKEY_bio(out, ec);
-        }
-
-    	break;
-    default:
-        BIO_free(out);
-    	ossl_raise(rb_eRuntimeError, "unknown format (internal error)");
-    }
-
-    if (i != 1) {
-        BIO_free(out);
-        ossl_raise(eECError, "outlen=%d", i);
-    }
-
-    str = ossl_membio2str(out);
-
-    return str;
-}
-
 /*
  *  call-seq:
  *     key.export([cipher, pass_phrase]) => String
@@ -495,11 +388,16 @@ static VALUE ossl_ec_key_to_string(VALUE self, VALUE ciph, VALUE pass, int forma
  * instance. Note that encryption will only be effective for a private key,
  * public keys will always be encoded in plain text.
  */
-static VALUE ossl_ec_key_export(int argc, VALUE *argv, VALUE self)
+static VALUE
+ossl_ec_key_export(int argc, VALUE *argv, VALUE self)
 {
-    VALUE cipher, passwd;
-    rb_scan_args(argc, argv, "02", &cipher, &passwd);
-    return ossl_ec_key_to_string(self, cipher, passwd, EXPORT_PEM);
+    EC_KEY *ec;
+
+    GetEC(self, ec);
+    if (EC_KEY_get0_private_key(ec))
+        return ossl_pkey_export_traditional(argc, argv, self, 0);
+    else
+        return ossl_pkey_export_spki(self, 0);
 }
 
 /*
@@ -508,9 +406,16 @@ static VALUE ossl_ec_key_export(int argc, VALUE *argv, VALUE self)
  *
  *  See the OpenSSL documentation for i2d_ECPrivateKey_bio()
  */
-static VALUE ossl_ec_key_to_der(VALUE self)
+static VALUE
+ossl_ec_key_to_der(VALUE self)
 {
-    return ossl_ec_key_to_string(self, Qnil, Qnil, EXPORT_DER);
+    EC_KEY *ec;
+
+    GetEC(self, ec);
+    if (EC_KEY_get0_private_key(ec))
+        return ossl_pkey_export_traditional(0, NULL, self, 1);
+    else
+        return ossl_pkey_export_spki(self, 1);
 }
 
 /*
diff --git a/ext/openssl/ossl_pkey_rsa.c b/ext/openssl/ossl_pkey_rsa.c
index 761866c6..3c298a2a 100644
--- a/ext/openssl/ossl_pkey_rsa.c
+++ b/ext/openssl/ossl_pkey_rsa.c
@@ -45,53 +45,6 @@ VALUE cRSA;
 VALUE eRSAError;
 
 /*
- * Public
- */
-static VALUE
-rsa_instance(VALUE klass, RSA *rsa)
-{
-    EVP_PKEY *pkey;
-    VALUE obj;
-
-    if (!rsa) {
-	return Qfalse;
-    }
-    obj = NewPKey(klass);
-    if (!(pkey = EVP_PKEY_new())) {
-	return Qfalse;
-    }
-    if (!EVP_PKEY_assign_RSA(pkey, rsa)) {
-	EVP_PKEY_free(pkey);
-	return Qfalse;
-    }
-    SetPKey(obj, pkey);
-
-    return obj;
-}
-
-VALUE
-ossl_rsa_new(EVP_PKEY *pkey)
-{
-    VALUE obj;
-
-    if (!pkey) {
-	obj = rsa_instance(cRSA, RSA_new());
-    }
-    else {
-	obj = NewPKey(cRSA);
-	if (EVP_PKEY_base_id(pkey) != EVP_PKEY_RSA) {
-	    ossl_raise(rb_eTypeError, "Not a RSA key!");
-	}
-	SetPKey(obj, pkey);
-    }
-    if (obj == Qfalse) {
-	ossl_raise(eRSAError, NULL);
-    }
-
-    return obj;
-}
-
-/*
  * Private
  */
 struct rsa_blocking_gen_arg {
@@ -124,7 +77,7 @@ rsa_generate(int size, unsigned long exp)
 	RSA_free(rsa);
 	BN_free(e);
 	BN_GENCB_free(cb);
-	return NULL;
+        ossl_raise(eRSAError, "malloc failure");
     }
     for (i = 0; i < (int)sizeof(exp) * 8; ++i) {
 	if (exp & (1UL << i)) {
@@ -132,7 +85,7 @@ rsa_generate(int size, unsigned long exp)
 		BN_free(e);
 		RSA_free(rsa);
 		BN_GENCB_free(cb);
-		return NULL;
+                ossl_raise(eRSAError, "BN_set_bit");
 	    }
 	}
     }
@@ -161,7 +114,7 @@ rsa_generate(int size, unsigned long exp)
 	    ossl_clear_error();
 	    rb_jump_tag(cb_arg.state);
 	}
-	return NULL;
+        ossl_raise(eRSAError, "RSA_generate_key_ex");
     }
 
     return rsa;
@@ -180,26 +133,26 @@ static VALUE
 ossl_rsa_s_generate(int argc, VALUE *argv, VALUE klass)
 {
 /* why does this method exist?  why can't initialize take an optional exponent? */
+    EVP_PKEY *pkey;
     RSA *rsa;
     VALUE size, exp;
     VALUE obj;
 
     rb_scan_args(argc, argv, "11", &size, &exp);
+    obj = rb_obj_alloc(klass);
+    GetPKey(obj, pkey);
 
-    rsa = rsa_generate(NUM2INT(size), NIL_P(exp) ? RSA_F4 : NUM2ULONG(exp)); /* err handled by rsa_instance */
-    obj = rsa_instance(klass, rsa);
-
-    if (obj == Qfalse) {
-	RSA_free(rsa);
-	ossl_raise(eRSAError, NULL);
+    rsa = rsa_generate(NUM2INT(size), NIL_P(exp) ? RSA_F4 : NUM2ULONG(exp));
+    if (!EVP_PKEY_assign_RSA(pkey, rsa)) {
+        RSA_free(rsa);
+        ossl_raise(eRSAError, "EVP_PKEY_assign_RSA");
     }
-
     return obj;
 }
 
 /*
  * call-seq:
- *   RSA.new(key_size)                 => RSA instance
+ *   RSA.new(size [, exponent])        => RSA instance
  *   RSA.new(encoded_key)              => RSA instance
  *   RSA.new(encoded_key, pass_phrase) => RSA instance
  *
@@ -220,36 +173,34 @@ ossl_rsa_s_generate(int argc, VALUE *argv, VALUE klass)
 static VALUE
 ossl_rsa_initialize(int argc, VALUE *argv, VALUE self)
 {
-    EVP_PKEY *pkey;
-    RSA *rsa;
+    EVP_PKEY *pkey, *tmp;
+    RSA *rsa = NULL;
     BIO *in;
     VALUE arg, pass;
 
     GetPKey(self, pkey);
-    if(rb_scan_args(argc, argv, "02", &arg, &pass) == 0) {
+    rb_scan_args(argc, argv, "02", &arg, &pass);
+    if (argc == 0) {
 	rsa = RSA_new();
+        if (!rsa)
+            ossl_raise(eRSAError, "RSA_new");
     }
     else if (RB_INTEGER_TYPE_P(arg)) {
 	rsa = rsa_generate(NUM2INT(arg), NIL_P(pass) ? RSA_F4 : NUM2ULONG(pass));
-	if (!rsa) ossl_raise(eRSAError, NULL);
     }
     else {
 	pass = ossl_pem_passwd_value(pass);
 	arg = ossl_to_der_if_possible(arg);
 	in = ossl_obj2bio(&arg);
-	rsa = PEM_read_bio_RSAPrivateKey(in, NULL, ossl_pem_passwd_cb, (void *)pass);
-	if (!rsa) {
-	    OSSL_BIO_reset(in);
-	    rsa = PEM_read_bio_RSA_PUBKEY(in, NULL, NULL, NULL);
-	}
-	if (!rsa) {
-	    OSSL_BIO_reset(in);
-	    rsa = d2i_RSAPrivateKey_bio(in, NULL);
-	}
-	if (!rsa) {
-	    OSSL_BIO_reset(in);
-	    rsa = d2i_RSA_PUBKEY_bio(in, NULL);
-	}
+
+        tmp = ossl_pkey_read_generic(in, pass);
+        if (tmp) {
+            if (EVP_PKEY_base_id(tmp) != EVP_PKEY_RSA)
+                rb_raise(eRSAError, "incorrect pkey type: %s",
+                         OBJ_nid2sn(EVP_PKEY_base_id(tmp)));
+            rsa = EVP_PKEY_get1_RSA(tmp);
+            EVP_PKEY_free(tmp);
+        }
 	if (!rsa) {
 	    OSSL_BIO_reset(in);
 	    rsa = PEM_read_bio_RSAPublicKey(in, NULL, NULL, NULL);
@@ -260,12 +211,13 @@ ossl_rsa_initialize(int argc, VALUE *argv, VALUE self)
 	}
 	BIO_free(in);
 	if (!rsa) {
+            ossl_clear_error();
 	    ossl_raise(eRSAError, "Neither PUB key nor PRIV key");
 	}
     }
     if (!EVP_PKEY_assign_RSA(pkey, rsa)) {
 	RSA_free(rsa);
-	ossl_raise(eRSAError, NULL);
+	ossl_raise(eRSAError, "EVP_PKEY_assign_RSA");
     }
 
     return self;
@@ -327,6 +279,21 @@ ossl_rsa_is_private(VALUE self)
     return RSA_PRIVATE(self, rsa) ? Qtrue : Qfalse;
 }
 
+static int
+can_export_rsaprivatekey(VALUE self)
+{
+    RSA *rsa;
+    const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp;
+
+    GetRSA(self, rsa);
+
+    RSA_get0_key(rsa, &n, &e, &d);
+    RSA_get0_factors(rsa, &p, &q);
+    RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp);
+
+    return n && e && d && p && q && dmp1 && dmq1 && iqmp;
+}
+
 /*
  * call-seq:
  *   rsa.export([cipher, pass_phrase]) => PEM-format String
@@ -340,41 +307,10 @@ ossl_rsa_is_private(VALUE self)
 static VALUE
 ossl_rsa_export(int argc, VALUE *argv, VALUE self)
 {
-    RSA *rsa;
-    const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp;
-    BIO *out;
-    const EVP_CIPHER *ciph = NULL;
-    VALUE cipher, pass, str;
-
-    GetRSA(self, rsa);
-
-    rb_scan_args(argc, argv, "02", &cipher, &pass);
-
-    if (!NIL_P(cipher)) {
-	ciph = ossl_evp_get_cipherbyname(cipher);
-	pass = ossl_pem_passwd_value(pass);
-    }
-    if (!(out = BIO_new(BIO_s_mem()))) {
-	ossl_raise(eRSAError, NULL);
-    }
-    RSA_get0_key(rsa, &n, &e, &d);
-    RSA_get0_factors(rsa, &p, &q);
-    RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp);
-    if (n && e && d && p && q && dmp1 && dmq1 && iqmp) {
-	if (!PEM_write_bio_RSAPrivateKey(out, rsa, ciph, NULL, 0,
-					 ossl_pem_passwd_cb, (void *)pass)) {
-	    BIO_free(out);
-	    ossl_raise(eRSAError, NULL);
-	}
-    } else {
-	if (!PEM_write_bio_RSA_PUBKEY(out, rsa)) {
-	    BIO_free(out);
-	    ossl_raise(eRSAError, NULL);
-	}
-    }
-    str = ossl_membio2str(out);
-
-    return str;
+    if (can_export_rsaprivatekey(self))
+        return ossl_pkey_export_traditional(argc, argv, self, 0);
+    else
+        return ossl_pkey_export_spki(self, 0);
 }
 
 /*
@@ -386,30 +322,10 @@ ossl_rsa_export(int argc, VALUE *argv, VALUE self)
 static VALUE
 ossl_rsa_to_der(VALUE self)
 {
-    RSA *rsa;
-    const BIGNUM *n, *e, *d, *p, *q, *dmp1, *dmq1, *iqmp;
-    int (*i2d_func)(const RSA *, unsigned char **);
-    unsigned char *ptr;
-    long len;
-    VALUE str;
-
-    GetRSA(self, rsa);
-    RSA_get0_key(rsa, &n, &e, &d);
-    RSA_get0_factors(rsa, &p, &q);
-    RSA_get0_crt_params(rsa, &dmp1, &dmq1, &iqmp);
-    if (n && e && d && p && q && dmp1 && dmq1 && iqmp)
-	i2d_func = i2d_RSAPrivateKey;
+    if (can_export_rsaprivatekey(self))
+        return ossl_pkey_export_traditional(0, NULL, self, 1);
     else
-	i2d_func = (int (*)(const RSA *, unsigned char **))i2d_RSA_PUBKEY;
-    if((len = i2d_func(rsa, NULL)) <= 0)
-	ossl_raise(eRSAError, NULL);
-    str = rb_str_new(0, len);
-    ptr = (unsigned char *)RSTRING_PTR(str);
-    if(i2d_func(rsa, &ptr) < 0)
-	ossl_raise(eRSAError, NULL);
-    ossl_str_adjust(str, ptr);
-
-    return str;
+        return ossl_pkey_export_spki(self, 1);
 }
 
 /*
@@ -809,17 +725,20 @@ ossl_rsa_to_text(VALUE self)
 static VALUE
 ossl_rsa_to_public_key(VALUE self)
 {
-    EVP_PKEY *pkey;
+    EVP_PKEY *pkey, *pkey_new;
     RSA *rsa;
     VALUE obj;
 
     GetPKeyRSA(self, pkey);
-    /* err check performed by rsa_instance */
+    obj = rb_obj_alloc(rb_obj_class(self));
+    GetPKey(obj, pkey_new);
+
     rsa = RSAPublicKey_dup(EVP_PKEY_get0_RSA(pkey));
-    obj = rsa_instance(rb_obj_class(self), rsa);
-    if (obj == Qfalse) {
-	RSA_free(rsa);
-	ossl_raise(eRSAError, NULL);
+    if (!rsa)
+        ossl_raise(eRSAError, "RSAPublicKey_dup");
+    if (!EVP_PKEY_assign_RSA(pkey_new, rsa)) {
+        RSA_free(rsa);
+        ossl_raise(eRSAError, "EVP_PKEY_assign_RSA");
     }
     return obj;
 }
diff --git a/sample/echo_cli.rb b/sample/echo_cli.rb
index 069a21ec..3fbadf33 100644
--- a/sample/echo_cli.rb
+++ b/sample/echo_cli.rb
@@ -15,7 +15,7 @@ ca_path   = options["C"]
 ctx = OpenSSL::SSL::SSLContext.new()
 if cert_file && key_file
   ctx.cert = OpenSSL::X509::Certificate.new(File::read(cert_file))
-  ctx.key  = OpenSSL::PKey::RSA.new(File::read(key_file))
+  ctx.key  = OpenSSL::PKey.read(File::read(key_file))
 end
 if ca_path
   ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
diff --git a/sample/echo_svr.rb b/sample/echo_svr.rb
index ba15394e..1cc07b9b 100644
--- a/sample/echo_svr.rb
+++ b/sample/echo_svr.rb
@@ -13,7 +13,7 @@ ca_path   = options["C"]
 
 if cert_file && key_file
   cert = OpenSSL::X509::Certificate.new(File::read(cert_file))
-  key  = OpenSSL::PKey::RSA.new(File::read(key_file))
+  key  = OpenSSL::PKey.read(File::read(key_file))
 else
   key = OpenSSL::PKey::RSA.new(512){ print "." }
   puts
diff --git a/sample/gen_csr.rb b/sample/gen_csr.rb
index a061260c..2602b68a 100644
--- a/sample/gen_csr.rb
+++ b/sample/gen_csr.rb
@@ -25,7 +25,7 @@ name = X509::Name.parse(name_str)
 
 keypair = nil
 if keypair_file
-  keypair = PKey::RSA.new(File.open(keypair_file).read)
+  keypair = PKey.read(File.read(keypair_file))
 else
   keypair = PKey::RSA.new(1024) { putc "." }
   puts
diff --git a/sample/smime_read.rb b/sample/smime_read.rb
index 17394f9b..a70105fd 100644
--- a/sample/smime_read.rb
+++ b/sample/smime_read.rb
@@ -11,7 +11,7 @@ ca_path   = options["C"]
 data = $stdin.read
 
 cert = X509::Certificate.new(File::read(cert_file))
-key = PKey::RSA.new(File::read(key_file))
+key = PKey::read(File::read(key_file))
 p7enc = PKCS7::read_smime(data)
 data = p7enc.decrypt(key, cert)
 
diff --git a/sample/smime_write.rb b/sample/smime_write.rb
index 5a5236c7..20c933b2 100644
--- a/sample/smime_write.rb
+++ b/sample/smime_write.rb
@@ -9,7 +9,7 @@ key_file  = options["k"]
 rcpt_file = options["r"]
 
 cert = X509::Certificate.new(File::read(cert_file))
-key = PKey::RSA.new(File::read(key_file))
+key = PKey::read(File::read(key_file))
 
 data  = "Content-Type: text/plain\r\n"
 data << "\r\n"
diff --git a/test/openssl/test_pkey_dh.rb b/test/openssl/test_pkey_dh.rb
index fd2c7a66..4a05626a 100644
--- a/test/openssl/test_pkey_dh.rb
+++ b/test/openssl/test_pkey_dh.rb
@@ -36,6 +36,8 @@ class OpenSSL::TestPKeyDH < OpenSSL::PKeyTestCase
     EOF
     key = OpenSSL::PKey::DH.new(pem)
     assert_same_dh dup_public(dh1024), key
+    key = OpenSSL::PKey.read(pem)
+    assert_same_dh dup_public(dh1024), key
 
     assert_equal asn1.to_der, dh1024.to_der
     assert_equal pem, dh1024.export
diff --git a/test/openssl/utils.rb b/test/openssl/utils.rb
index 3776fbac..c1d737b2 100644
--- a/test/openssl/utils.rb
+++ b/test/openssl/utils.rb
@@ -42,9 +42,6 @@ module OpenSSL::TestUtils
 
     def pkey(name)
       OpenSSL::PKey.read(read_file("pkey", name))
-    rescue OpenSSL::PKey::PKeyError
-      # TODO: DH parameters can be read by OpenSSL::PKey.read atm
-      OpenSSL::PKey::DH.new(read_file("pkey", name))
     end
 
     def read_file(category, name)