pkey: track whether pkey is private key or not

There are multiple places where it's necessary to know whether a pkey
is a private key, a public key, or just key parameters. This is due to
two reasons:

 1. It's currently a responsibility of the caller to give a properly
    populated pkey instance to _some_ OpenSSL functions. For example,
    calling EVP_PKEY_sign() with an RSA pkey instance without the
    necessary components is known to cause a segfault.

 2. OpenSSL::PKey::{RSA,DSA,EC}#to_der behaves differently depending on
    it: they use the X.509 SubjectPublicKeyInfo structure instead of
    private key structures if the receiver pkey is a public key.

Currently, whenever this is necessary, we check the backing object, such
as RSA, and see if the fields are set or not. This approach won't always
work on OpenSSL 3.0 because of the architecture change. Unfortunately,
OpenSSL doesn't expose an API for this purpose (even though it has one
for its internal use).

As a workaround, let's manually track this information in an instance
variable. This has been partly done for ENGINE-backed pkeys. Now all
pkeys get this flag. This is straightforward on OpenSSL 3.0 as pkeys
are immutable once instantiated. On OpenSSL 1.1 or before (and current
versions of LibreSSL), it must be updated whenever a modification is
made to the object.

This commit comes with a slight behavior change. Pkeys returned by
following method will be explicitly marked as "public", even if it
happens to point at an EVP_PKEY struct containing private key
components. I expect the effect is minimum since these methods
explicitly say "public key".

 - OpenSSL::X509::Certificate#public_key
 - OpenSSL::X509::Request#public_key
 - OpenSSL::Netscape::SPKI#public_key

Author: rhenium

ext/openssl/openssl_missing.h

@@ -140,6 +140,7 @@ IMPL_KEY_ACCESSOR3(RSA, crt_params, dmp1, dmq1, iqmp, (dmp1 == obj->dmp1 || dmq1
 IMPL_PKEY_GETTER(DSA, dsa)
 IMPL_KEY_ACCESSOR2(DSA, key, pub_key, priv_key, (pub_key == obj->pub_key || (obj->priv_key && priv_key == obj->priv_key)))
 IMPL_KEY_ACCESSOR3(DSA, pqg, p, q, g, (p == obj->p || q == obj->q || g == obj->g))
+static inline ENGINE *DSA_get0_engine(DSA *dsa) { return dsa->engine; }
 #endif
 
 #if !defined(OPENSSL_NO_DH)

ext/openssl/ossl_engine.c

@@ -373,8 +373,7 @@ ossl_engine_load_privkey(int argc, VALUE *argv, VALUE self)
     GetEngine(self, e);
     pkey = ENGINE_load_private_key(e, sid, NULL, sdata);
     if (!pkey) ossl_raise(eEngineError, NULL);
-    obj = ossl_pkey_new(pkey);
-    OSSL_PKEY_SET_PRIVATE(obj);
+    obj = ossl_pkey_new(pkey, OSSL_PKEY_HAS_PRIVATE);
 
     return obj;
 }
@@ -403,7 +402,7 @@ ossl_engine_load_pubkey(int argc, VALUE *argv, VALUE self)
     pkey = ENGINE_load_public_key(e, sid, NULL, sdata);
     if (!pkey) ossl_raise(eEngineError, NULL);
 
-    return ossl_pkey_new(pkey);
+    return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC);
 }
 
 /*

ext/openssl/ossl_ns_spki.c

@@ -190,7 +190,7 @@ ossl_spki_get_public_key(VALUE self)
 	ossl_raise(eSPKIError, NULL);
     }
 
-    return ossl_pkey_new(pkey); /* NO DUP - OK */
+    return ossl_pkey_new(pkey, OSSL_PKEY_HAS_PUBLIC); /* NO DUP - OK */
 }
 
 /*

ext/openssl/ossl_pkcs12.c

@@ -152,7 +152,7 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self)
 static VALUE
 ossl_pkey_new_i(VALUE arg)
 {
-    return ossl_pkey_new((EVP_PKEY *)arg);
+    return ossl_pkey_new((EVP_PKEY *)arg, OSSL_PKEY_HAS_PRIVATE);
 }
 
 static VALUE

ext/openssl/ossl_pkey.c

@@ -19,7 +19,7 @@
 VALUE mPKey;
 VALUE cPKey;
 VALUE ePKeyError;
-static ID id_private_q;
+ID ossl_pkey_feature_id;
 
 static void
 ossl_evp_pkey_free(void *ptr)
@@ -65,7 +65,7 @@ pkey_new0(VALUE arg)
 }
 
 VALUE
-ossl_pkey_new(EVP_PKEY *pkey)
+ossl_pkey_new(EVP_PKEY *pkey, enum ossl_pkey_feature ps)
 {
     VALUE obj;
     int status;
@@ -75,6 +75,7 @@ ossl_pkey_new(EVP_PKEY *pkey)
 	EVP_PKEY_free(pkey);
 	rb_jump_tag(status);
     }
+    ossl_pkey_set(obj, ps);
 
     return obj;
 }
@@ -83,29 +84,28 @@ ossl_pkey_new(EVP_PKEY *pkey)
 # include <openssl/decoder.h>
 
 EVP_PKEY *
-ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type)
+ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type, enum ossl_pkey_feature *ps)
 {
     void *ppass = (void *)pass;
     OSSL_DECODER_CTX *dctx;
     EVP_PKEY *pkey = NULL;
     int pos = 0, pos2;
+    size_t i;
 
     dctx = OSSL_DECODER_CTX_new_for_pkey(&pkey, "DER", NULL, input_type, 0, NULL, NULL);
     if (!dctx)
         goto out;
     if (OSSL_DECODER_CTX_set_pem_password_cb(dctx, ossl_pem_passwd_cb, ppass) != 1)
         goto out;
 
-    /* First check DER */
-    if (OSSL_DECODER_from_bio(dctx, bio) == 1)
-        goto out;
-    OSSL_BIO_reset(bio);
-
-    /* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed */
-    if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1)
-        goto out;
     /*
-     * First check for private key formats. This is to keep compatibility with
+     * This is very inefficient as it will try to decode the same DER/PEM
+     * encoding repeatedly, but OpenSSL 3.0 doesn't provide an API to return
+     * what kind of information an EVP_PKEY is holding.
+     * OpenSSL issue: https://github.com/openssl/openssl/issues/9467
+     *
+     * The attempt order of selections is important: private key formats are
+     * checked first. This is to keep compatibility with
      * ruby/openssl < 3.0 which decoded the following as a private key.
      *
      *     $ openssl ecparam -name prime256v1 -genkey -outform PEM
@@ -125,59 +125,94 @@ ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type)
      * Note that normally, the input is supposed to contain a single decodable
      * PEM block only, so this special handling should not create a new problem.
      */
-    OSSL_DECODER_CTX_set_selection(dctx, EVP_PKEY_KEYPAIR);
-    while (1) {
+    struct { int selection; enum ossl_pkey_feature ps; } selections[] = {
+#if OSSL_OPENSSL_PREREQ(3, 0, 0) && !OSSL_OPENSSL_PREREQ(3, 0, 3)
+        /*
+         * Public key formats are checked last, with 0 (any) selection to avoid
+         * segfault in OpenSSL <= 3.0.3.
+         * Fixed by https://github.com/openssl/openssl/pull/18462
+         *
+         * This workaround is mainly for Ubuntu 22.04, which currently has yet
+         * to backport it (as of 2022-07-26).
+         */
+        { EVP_PKEY_KEYPAIR, OSSL_PKEY_HAS_PRIVATE },
+        { EVP_PKEY_KEY_PARAMETERS, OSSL_PKEY_HAS_NONE },
+        { 0, OSSL_PKEY_HAS_PUBLIC },
+#else
+        { EVP_PKEY_KEYPAIR, OSSL_PKEY_HAS_PRIVATE },
+        { EVP_PKEY_PUBLIC_KEY, OSSL_PKEY_HAS_PUBLIC },
+        { EVP_PKEY_KEY_PARAMETERS, OSSL_PKEY_HAS_NONE },
+#endif
+    };
+
+    /* First check DER */
+    for (i = 0; i < sizeof(selections)/sizeof(selections[0]); i++) {
+        OSSL_DECODER_CTX_set_selection(dctx, selections[i].selection);
+        *ps = selections[i].ps;
         if (OSSL_DECODER_from_bio(dctx, bio) == 1)
             goto out;
-        if (BIO_eof(bio))
-            break;
-        pos2 = BIO_tell(bio);
-        if (pos2 < 0 || pos2 <= pos)
-            break;
-        ossl_clear_error();
-        pos = pos2;
+        OSSL_BIO_reset(bio);
     }
 
-    OSSL_BIO_reset(bio);
-    OSSL_DECODER_CTX_set_selection(dctx, 0);
-    while (1) {
-        if (OSSL_DECODER_from_bio(dctx, bio) == 1)
-            goto out;
-        if (BIO_eof(bio))
-            break;
-        pos2 = BIO_tell(bio);
-        if (pos2 < 0 || pos2 <= pos)
-            break;
-        ossl_clear_error();
-        pos = pos2;
+    /* Then check PEM; multiple OSSL_DECODER_from_bio() calls may be needed */
+    if (OSSL_DECODER_CTX_set_input_type(dctx, "PEM") != 1)
+        goto out;
+    for (i = 0; i < sizeof(selections)/sizeof(selections[0]); i++) {
+        OSSL_DECODER_CTX_set_selection(dctx, selections[i].selection);
+        *ps = selections[i].ps;
+        while (1) {
+            if (OSSL_DECODER_from_bio(dctx, bio) == 1)
+                goto out;
+            if (BIO_eof(bio))
+                break;
+            pos2 = BIO_tell(bio);
+            if (pos2 < 0 || pos2 <= pos)
+                break;
+            ossl_clear_error();
+            pos = pos2;
+        }
+        OSSL_BIO_reset(bio);
     }
 
   out:
     OSSL_DECODER_CTX_free(dctx);
+#if OSSL_OPENSSL_PREREQ(3, 0, 0) && !OSSL_OPENSSL_PREREQ(3, 0, 3)
+    /* It also leaks an error queue entry in the success path */
+    if (pkey) ossl_clear_error();
+#endif
     return pkey;
 }
 #else
 EVP_PKEY *
-ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type)
+ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type, enum ossl_pkey_feature *ps)
 {
     void *ppass = (void *)pass;
     EVP_PKEY *pkey;
 
+    *ps = OSSL_PKEY_HAS_PRIVATE;
     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;
+
+    *ps = OSSL_PKEY_HAS_PUBLIC;
     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 */
+    *ps = OSSL_PKEY_HAS_PRIVATE;
+    OSSL_BIO_reset(bio);
     if ((pkey = PEM_read_bio_PrivateKey(bio, NULL, ossl_pem_passwd_cb, ppass)))
 	goto out;
+
+    *ps = OSSL_PKEY_HAS_PUBLIC;
     OSSL_BIO_reset(bio);
     if ((pkey = PEM_read_bio_PUBKEY(bio, NULL, NULL, NULL)))
 	goto out;
+
+    *ps = OSSL_PKEY_HAS_NONE;
     OSSL_BIO_reset(bio);
     if ((pkey = PEM_read_bio_Parameters(bio, NULL)))
 	goto out;
@@ -234,14 +269,15 @@ ossl_pkey_new_from_data(int argc, VALUE *argv, VALUE self)
     EVP_PKEY *pkey;
     BIO *bio;
     VALUE data, pass;
+    enum ossl_pkey_feature ps;
 
     rb_scan_args(argc, argv, "11", &data, &pass);
     bio = ossl_obj2bio(&data);
-    pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass), NULL);
+    pkey = ossl_pkey_read_generic(bio, ossl_pem_passwd_value(pass), NULL, &ps);
     BIO_free(bio);
     if (!pkey)
 	ossl_raise(ePKeyError, "Could not parse PKey");
-    return ossl_pkey_new(pkey);
+    return ossl_pkey_new(pkey, ps);
 }
 
 static VALUE
@@ -445,7 +481,7 @@ pkey_generate(int argc, VALUE *argv, VALUE self, int genparam)
         }
     }
 
-    return ossl_pkey_new(gen_arg.pkey);
+    return ossl_pkey_new(gen_arg.pkey, OSSL_PKEY_HAS_PRIVATE);
 }
 
 /*
@@ -567,18 +603,9 @@ GetPrivPKeyPtr(VALUE obj)
     EVP_PKEY *pkey;
 
     GetPKey(obj, pkey);
-    if (OSSL_PKEY_IS_PRIVATE(obj))
-        return pkey;
-    /*
-     * The EVP API does not provide a way to check if the EVP_PKEY has private
-     * components. Assuming it does...
-     */
-    if (!rb_respond_to(obj, id_private_q))
-        return pkey;
-    if (RTEST(rb_funcallv(obj, id_private_q, 0, NULL)))
-        return pkey;
-
-    rb_raise(rb_eArgError, "private key is needed");
+    if (!ossl_pkey_has(obj, OSSL_PKEY_HAS_PRIVATE))
+        rb_raise(rb_eArgError, "private key is needed");
+    return pkey;
 }
 
 EVP_PKEY *
@@ -654,6 +681,33 @@ ossl_pkey_oid(VALUE self)
     return rb_str_new_cstr(OBJ_nid2sn(nid));
 }
 
+/*
+ * call-seq:
+ *    pkey.public? -> true | false
+ *
+ * Indicates whether this PKey instance has a public key associated with it or
+ * not.
+ */
+static VALUE
+ossl_pkey_is_public(VALUE self)
+{
+    return ossl_pkey_has(self, OSSL_PKEY_HAS_PUBLIC) ? Qtrue : Qfalse;
+}
+
+/*
+ * call-seq:
+ *    pkey.private? -> true | false
+ *
+ * Indicates whether this PKey instance has a private key associated with it or
+ * not.
+ */
+static VALUE
+ossl_pkey_is_private(VALUE self)
+{
+    return ossl_pkey_has(self, OSSL_PKEY_HAS_PRIVATE) ? Qtrue : Qfalse;
+}
+
+
 /*
  * call-seq:
  *    pkey.inspect -> string
@@ -1620,6 +1674,8 @@ Init_ossl_pkey(void)
     rb_undef_method(cPKey, "initialize_copy");
 #endif
     rb_define_method(cPKey, "oid", ossl_pkey_oid, 0);
+    rb_define_method(cPKey, "public?", ossl_pkey_is_public, 0);
+    rb_define_method(cPKey, "private?", ossl_pkey_is_private, 0);
     rb_define_method(cPKey, "inspect", ossl_pkey_inspect, 0);
     rb_define_method(cPKey, "to_text", ossl_pkey_to_text, 0);
     rb_define_method(cPKey, "private_to_der", ossl_pkey_private_to_der, -1);
@@ -1637,7 +1693,7 @@ Init_ossl_pkey(void)
     rb_define_method(cPKey, "encrypt", ossl_pkey_encrypt, -1);
     rb_define_method(cPKey, "decrypt", ossl_pkey_decrypt, -1);
 
-    id_private_q = rb_intern("private?");
+    ossl_pkey_feature_id = rb_intern_const("state");
 
     /*
      * INIT rsa, dsa, dh, ec

ext/openssl/ossl_pkey.h

@@ -14,10 +14,7 @@ extern VALUE mPKey;
 extern VALUE cPKey;
 extern VALUE ePKeyError;
 extern const rb_data_type_t ossl_evp_pkey_type;
-
-/* For ENGINE */
-#define OSSL_PKEY_SET_PRIVATE(obj) rb_ivar_set((obj), rb_intern("private"), Qtrue)
-#define OSSL_PKEY_IS_PRIVATE(obj)  (rb_attr_get((obj), rb_intern("private")) == Qtrue)
+extern ID ossl_pkey_feature_id;
 
 #define GetPKey(obj, pkey) do {\
     TypedData_Get_Struct((obj), EVP_PKEY, &ossl_evp_pkey_type, (pkey)); \
@@ -26,10 +23,34 @@ extern const rb_data_type_t ossl_evp_pkey_type;
     } \
 } while (0)
 
+/*
+ * Store whether pkey contains public key, private key, or neither of them.
+ * This is ugly, but OpenSSL currently (3.0) doesn't provide a public API for
+ * this purpose.
+ */
+enum ossl_pkey_feature {
+    OSSL_PKEY_HAS_NONE, /* or parameters only */
+    OSSL_PKEY_HAS_PUBLIC,
+    OSSL_PKEY_HAS_PRIVATE,
+};
+
+static inline void
+ossl_pkey_set(VALUE self, enum ossl_pkey_feature feature)
+{
+    rb_ivar_set(self, ossl_pkey_feature_id, INT2FIX(feature));
+}
+
+static inline int
+ossl_pkey_has(VALUE self, enum ossl_pkey_feature feature)
+{
+    return FIX2INT(rb_attr_get(self, ossl_pkey_feature_id)) >= (int)feature;
+}
+
 /* Takes ownership of the EVP_PKEY */
-VALUE ossl_pkey_new(EVP_PKEY *);
+VALUE ossl_pkey_new(EVP_PKEY *pkey, enum ossl_pkey_feature ps);
 void ossl_pkey_check_public_key(const EVP_PKEY *);
-EVP_PKEY *ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type);
+EVP_PKEY *ossl_pkey_read_generic(BIO *bio, VALUE pass, const char *input_type,
+                                 enum ossl_pkey_feature *ps);
 EVP_PKEY *GetPKeyPtr(VALUE);
 EVP_PKEY *DupPKeyPtr(VALUE);
 EVP_PKEY *GetPrivPKeyPtr(VALUE);
@@ -145,6 +166,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2, VALU
 		BN_clear_free(bn3);					\
 		ossl_raise(ePKeyError, #_type"_set0_"#_group);		\
 	}								\
+	_keytype##_fix_selection(self, obj);				\
 	return self;							\
 }
 
@@ -172,6 +194,7 @@ static VALUE ossl_##_keytype##_set_##_group(VALUE self, VALUE v1, VALUE v2) \
 		BN_clear_free(bn2);					\
 		ossl_raise(ePKeyError, #_type"_set0_"#_group);		\
 	}								\
+	_keytype##_fix_selection(self, obj);				\
 	return self;							\
 }
 #else /* OSSL_HAVE_PROVIDER */