Merge pull request #168 from rhenium/ky/pkey-check-sanity

[Bug #14087] x509cert, x509crl, x509req, ns_spki: check sanity of public key

6 files changed, 38 insertions, 28 deletions

diff --git a/ext/openssl/ossl_ns_spki.c b/ext/openssl/ossl_ns_spki.c
index 4d978bd0..5ac8ef55 100644
--- a/ext/openssl/ossl_ns_spki.c
+++ b/ext/openssl/ossl_ns_spki.c
@@ -208,12 +208,13 @@ static VALUE
 ossl_spki_set_public_key(VALUE self, VALUE key)
 {
     NETSCAPE_SPKI *spki;
+    EVP_PKEY *pkey;
 
     GetSPKI(self, spki);
-    if (!NETSCAPE_SPKI_set_pubkey(spki, GetPKeyPtr(key))) { /* NO NEED TO DUP */
-	ossl_raise(eSPKIError, NULL);
-    }
-
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
+    if (!NETSCAPE_SPKI_set_pubkey(spki, pkey))
+	ossl_raise(eSPKIError, "NETSCAPE_SPKI_set_pubkey");
     return key;
 }
 
@@ -307,17 +308,20 @@ static VALUE
 ossl_spki_verify(VALUE self, VALUE key)
 {
     NETSCAPE_SPKI *spki;
+    EVP_PKEY *pkey;
 
     GetSPKI(self, spki);
-    switch (NETSCAPE_SPKI_verify(spki, GetPKeyPtr(key))) { /* NO NEED TO DUP */
-    case 0:
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
+    switch (NETSCAPE_SPKI_verify(spki, pkey)) {
+      case 0:
+	ossl_clear_error();
 	return Qfalse;
-    case 1:
+      case 1:
 	return Qtrue;
-    default:
-	ossl_raise(eSPKIError, NULL);
+      default:
+	ossl_raise(eSPKIError, "NETSCAPE_SPKI_verify");
     }
-    return Qnil; /* dummy */
 }
 
 /* Document-class: OpenSSL::Netscape::SPKI
diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c
index 314d1d94..aad3e2e4 100644
--- a/ext/openssl/ossl_pkey.c
+++ b/ext/openssl/ossl_pkey.c
@@ -163,8 +163,8 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self)
     return ossl_pkey_new(pkey);
 }
 
-static void
-pkey_check_public_key(EVP_PKEY *pkey)
+void
+ossl_pkey_check_public_key(const EVP_PKEY *pkey)
 {
     void *ptr;
     const BIGNUM *n, *e, *pubkey;
@@ -172,7 +172,8 @@ pkey_check_public_key(EVP_PKEY *pkey)
     if (EVP_PKEY_missing_parameters(pkey))
 	ossl_raise(ePKeyError, "parameters missing");
 
-    ptr = EVP_PKEY_get0(pkey);
+    /* OpenSSL < 1.1.0 takes non-const pointer */
+    ptr = EVP_PKEY_get0((EVP_PKEY *)pkey);
     switch (EVP_PKEY_base_id(pkey)) {
       case EVP_PKEY_RSA:
 	RSA_get0_key(ptr, &n, &e, NULL);
@@ -352,7 +353,7 @@ ossl_pkey_verify(VALUE self, VALUE digest, VALUE sig, VALUE data)
     int siglen, result;
 
     GetPKey(self, pkey);
-    pkey_check_public_key(pkey);
+    ossl_pkey_check_public_key(pkey);
     md = GetDigestPtr(digest);
     StringValue(sig);
     siglen = RSTRING_LENINT(sig);
diff --git a/ext/openssl/ossl_pkey.h b/ext/openssl/ossl_pkey.h
index e3b723cd..a0b49744 100644
--- a/ext/openssl/ossl_pkey.h
+++ b/ext/openssl/ossl_pkey.h
@@ -48,6 +48,7 @@ int ossl_generate_cb_2(int p, int n, BN_GENCB *cb);
 void ossl_generate_cb_stop(void *ptr);
 
 VALUE ossl_pkey_new(EVP_PKEY *);
+void ossl_pkey_check_public_key(const EVP_PKEY *);
 EVP_PKEY *GetPKeyPtr(VALUE);
 EVP_PKEY *DupPKeyPtr(VALUE);
 EVP_PKEY *GetPrivPKeyPtr(VALUE);
diff --git a/ext/openssl/ossl_x509cert.c b/ext/openssl/ossl_x509cert.c
index 87086a7c..91c25c4e 100644
--- a/ext/openssl/ossl_x509cert.c
+++ b/ext/openssl/ossl_x509cert.c
@@ -546,18 +546,19 @@ ossl_x509_get_public_key(VALUE self)
 
 /*
  * call-seq:
- *    cert.public_key = key => key
+ *    cert.public_key = key
  */
 static VALUE
 ossl_x509_set_public_key(VALUE self, VALUE key)
 {
     X509 *x509;
+    EVP_PKEY *pkey;
 
     GetX509(self, x509);
-    if (!X509_set_pubkey(x509, GetPKeyPtr(key))) { /* DUPs pkey */
-	ossl_raise(eX509CertError, NULL);
-    }
-
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
+    if (!X509_set_pubkey(x509, pkey))
+	ossl_raise(eX509CertError, "X509_set_pubkey");
     return key;
 }
 
@@ -594,9 +595,9 @@ ossl_x509_verify(VALUE self, VALUE key)
     X509 *x509;
     EVP_PKEY *pkey;
 
-    pkey = GetPKeyPtr(key); /* NO NEED TO DUP */
     GetX509(self, x509);
-
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
     switch (X509_verify(x509, pkey)) {
       case 1:
 	return Qtrue;
diff --git a/ext/openssl/ossl_x509crl.c b/ext/openssl/ossl_x509crl.c
index 035025ab..add72c6c 100644
--- a/ext/openssl/ossl_x509crl.c
+++ b/ext/openssl/ossl_x509crl.c
@@ -366,9 +366,12 @@ static VALUE
 ossl_x509crl_verify(VALUE self, VALUE key)
 {
     X509_CRL *crl;
+    EVP_PKEY *pkey;
 
     GetX509CRL(self, crl);
-    switch (X509_CRL_verify(crl, GetPKeyPtr(key))) {
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
+    switch (X509_CRL_verify(crl, pkey)) {
       case 1:
 	return Qtrue;
       case 0:
diff --git a/ext/openssl/ossl_x509req.c b/ext/openssl/ossl_x509req.c
index 15bc7052..0f7fecdc 100644
--- a/ext/openssl/ossl_x509req.c
+++ b/ext/openssl/ossl_x509req.c
@@ -330,11 +330,10 @@ ossl_x509req_set_public_key(VALUE self, VALUE key)
     EVP_PKEY *pkey;
 
     GetX509Req(self, req);
-    pkey = GetPKeyPtr(key); /* NO NEED TO DUP */
-    if (!X509_REQ_set_pubkey(req, pkey)) {
-	ossl_raise(eX509ReqError, NULL);
-    }
-
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
+    if (!X509_REQ_set_pubkey(req, pkey))
+	ossl_raise(eX509ReqError, "X509_REQ_set_pubkey");
     return key;
 }
 
@@ -365,7 +364,8 @@ ossl_x509req_verify(VALUE self, VALUE key)
     EVP_PKEY *pkey;
 
     GetX509Req(self, req);
-    pkey = GetPKeyPtr(key); /* NO NEED TO DUP */
+    pkey = GetPKeyPtr(key);
+    ossl_pkey_check_public_key(pkey);
     switch (X509_REQ_verify(req, pkey)) {
       case 1:
 	return Qtrue;