x509cert, x509crl, x509req, ns_spki: check sanity of public keyky/pkey-check-sanity

The pub_encode routine of an EVP_PKEY_ASN1_METHOD seems to assume the
parameters and public key component(s) to be set properly. Calling that,
for example, through X509_set_pubkey(), with an incomplete object may
cause segfault.

Use ossl_pkey_check_public_key() to check that. It doesn't look pretty,
but unfortunately there isn't a generic way to do that with the EVP API.

Something similar applies to the verify routine of an EVP_PKEY_METHOD.
Do the same check before calling *_verify().

Reference: http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/83688
Reference: https://bugs.ruby-lang.org/issues/14087

4 files changed, 32 insertions, 24 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_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;