asn1: refactor converting ASN1_OBJECT to string

ruby/openssl exposes OIDs to Ruby as strings in many places, but the
conversion logic has been duplicated and the behavior is inconsistent.
There are mainly two patterns:

 - Returns the short name associated with the OID/NID, or the dotted
   decimal notation if it is unknown to OpenSSL.
 - Returns the long name, or the dotted decimal notation.

These patterns are implemented using different OpenSSL APIs and that
caused subtle differences. Add helper functions ossl_asn1obj_to_string()
and ossl_asn1obj_to_string_long_name() to unify the logic.

Also, document the current behaviors where it is not yet done. The
inconsistency was likely unintentional, but since it dates back to the
original implementations, standardizing it now would cause more issues
than it resolves.

Author: rhenium

ext/openssl/ossl_asn1.c

@@ -147,6 +147,48 @@ asn1integer_to_num_i(VALUE arg)
     return asn1integer_to_num((ASN1_INTEGER *)arg);
 }
 
+/*
+ * ASN1_OBJECT conversions
+ */
+VALUE
+ossl_asn1obj_to_string_oid(const ASN1_OBJECT *a1obj)
+{
+    VALUE str;
+    int len;
+
+    str = rb_usascii_str_new(NULL, 127);
+    len = OBJ_obj2txt(RSTRING_PTR(str), RSTRING_LENINT(str), a1obj, 1);
+    if (len <= 0 || len == INT_MAX)
+	ossl_raise(eOSSLError, "OBJ_obj2txt");
+    if (len > RSTRING_LEN(str)) {
+	/* +1 is for the \0 terminator added by OBJ_obj2txt() */
+	rb_str_resize(str, len + 1);
+	len = OBJ_obj2txt(RSTRING_PTR(str), len + 1, a1obj, 1);
+	if (len <= 0)
+	    ossl_raise(eOSSLError, "OBJ_obj2txt");
+    }
+    rb_str_set_len(str, len);
+    return str;
+}
+
+VALUE
+ossl_asn1obj_to_string(const ASN1_OBJECT *obj)
+{
+    int nid = OBJ_obj2nid(obj);
+    if (nid != NID_undef)
+        return rb_str_new_cstr(OBJ_nid2sn(nid));
+    return ossl_asn1obj_to_string_oid(obj);
+}
+
+VALUE
+ossl_asn1obj_to_string_long_name(const ASN1_OBJECT *obj)
+{
+    int nid = OBJ_obj2nid(obj);
+    if (nid != NID_undef)
+        return rb_str_new_cstr(OBJ_nid2ln(nid));
+    return ossl_asn1obj_to_string_oid(obj);
+}
+
 /********/
 /*
  * ASN1 module
@@ -393,32 +435,27 @@ decode_null(unsigned char* der, long length)
     return Qnil;
 }
 
+VALUE
+asn1obj_to_string_i(VALUE arg)
+{
+    return ossl_asn1obj_to_string((const ASN1_OBJECT *)arg);
+}
+
 static VALUE
 decode_obj(unsigned char* der, long length)
 {
     ASN1_OBJECT *obj;
     const unsigned char *p;
     VALUE ret;
-    int nid;
-    BIO *bio;
+    int state;
 
     p = der;
-    if(!(obj = d2i_ASN1_OBJECT(NULL, &p, length)))
-	ossl_raise(eASN1Error, NULL);
-    if((nid = OBJ_obj2nid(obj)) != NID_undef){
-	ASN1_OBJECT_free(obj);
-	ret = rb_str_new2(OBJ_nid2sn(nid));
-    }
-    else{
-	if(!(bio = BIO_new(BIO_s_mem()))){
-	    ASN1_OBJECT_free(obj);
-	    ossl_raise(eASN1Error, NULL);
-	}
-	i2a_ASN1_OBJECT(bio, obj);
-	ASN1_OBJECT_free(obj);
-	ret = ossl_membio2str(bio);
-    }
-
+    if (!(obj = d2i_ASN1_OBJECT(NULL, &p, length)))
+        ossl_raise(eASN1Error, "d2i_ASN1_OBJECT");
+    ret = rb_protect(asn1obj_to_string_i, (VALUE)obj, &state);
+    ASN1_OBJECT_free(obj);
+    if (state)
+        rb_jump_tag(state);
     return ret;
 }
 
@@ -1172,23 +1209,7 @@ ossl_asn1obj_get_ln(VALUE self)
 static VALUE
 asn1obj_get_oid_i(VALUE vobj)
 {
-    ASN1_OBJECT *a1obj = (void *)vobj;
-    VALUE str;
-    int len;
-
-    str = rb_usascii_str_new(NULL, 127);
-    len = OBJ_obj2txt(RSTRING_PTR(str), RSTRING_LENINT(str), a1obj, 1);
-    if (len <= 0 || len == INT_MAX)
-	ossl_raise(eASN1Error, "OBJ_obj2txt");
-    if (len > RSTRING_LEN(str)) {
-	/* +1 is for the \0 terminator added by OBJ_obj2txt() */
-	rb_str_resize(str, len + 1);
-	len = OBJ_obj2txt(RSTRING_PTR(str), len + 1, a1obj, 1);
-	if (len <= 0)
-	    ossl_raise(eASN1Error, "OBJ_obj2txt");
-    }
-    rb_str_set_len(str, len);
-    return str;
+    return ossl_asn1obj_to_string_oid((const ASN1_OBJECT *)vobj);
 }
 
 /*

ext/openssl/ossl_asn1.h

@@ -35,6 +35,16 @@ ASN1_INTEGER *num_to_asn1integer(VALUE, ASN1_INTEGER *);
  * ASN1_OBJECT conversions
  */
 ASN1_OBJECT *ossl_to_asn1obj(VALUE obj);
+/*
+ * Returns the short name if available, the dotted decimal notation otherwise.
+ * This is the most common way to return ASN1_OBJECT to Ruby.
+ */
+VALUE ossl_asn1obj_to_string(const ASN1_OBJECT *a1obj);
+/*
+ * However, some places use long names instead. This is likely unintentional,
+ * but we keep the current behavior in existing methods.
+ */
+VALUE ossl_asn1obj_to_string_long_name(const ASN1_OBJECT *a1obj);
 
 /*
  * ASN1 module

ext/openssl/ossl_ocsp.c

@@ -1591,19 +1591,10 @@ ossl_ocspcid_get_hash_algorithm(VALUE self)
 {
     OCSP_CERTID *id;
     ASN1_OBJECT *oid;
-    BIO *out;
 
     GetOCSPCertId(self, id);
     OCSP_id_get0_info(NULL, &oid, NULL, NULL, id);
-
-    if (!(out = BIO_new(BIO_s_mem())))
-	ossl_raise(eOCSPError, "BIO_new");
-
-    if (!i2a_ASN1_OBJECT(out, oid)) {
-	BIO_free(out);
-	ossl_raise(eOCSPError, "i2a_ASN1_OBJECT");
-    }
-    return ossl_membio2str(out);
+    return ossl_asn1obj_to_string_long_name(oid);
 }
 
 /*

ext/openssl/ossl_ts.c

@@ -138,27 +138,6 @@ obj_to_asn1obj_i(VALUE obj)
     return (VALUE)ossl_to_asn1obj(obj);
 }
 
-static VALUE
-get_asn1obj(const ASN1_OBJECT *obj)
-{
-    BIO *out;
-    VALUE ret;
-    int nid;
-    if ((nid = OBJ_obj2nid(obj)) != NID_undef)
-        ret = rb_str_new2(OBJ_nid2sn(nid));
-    else{
-        if (!(out = BIO_new(BIO_s_mem())))
-            ossl_raise(eTimestampError, "BIO_new(BIO_s_mem())");
-        if (i2a_ASN1_OBJECT(out, obj) <= 0) {
-            BIO_free(out);
-            ossl_raise(eTimestampError, "i2a_ASN1_OBJECT");
-        }
-        ret = ossl_membio2str(out);
-    }
-
-    return ret;
-}
-
 static VALUE
 ossl_ts_req_alloc(VALUE klass)
 {
@@ -229,7 +208,7 @@ ossl_ts_req_get_algorithm(VALUE self)
     mi = TS_REQ_get_msg_imprint(req);
     algor = TS_MSG_IMPRINT_get_algo(mi);
     X509_ALGOR_get0(&obj, NULL, NULL, algor);
-    return get_asn1obj(obj);
+    return ossl_asn1obj_to_string(obj);
 }
 
 /*
@@ -358,7 +337,7 @@ ossl_ts_req_get_policy_id(VALUE self)
     GetTSRequest(self, req);
     if (!TS_REQ_get_policy_id(req))
         return Qnil;
-    return get_asn1obj(TS_REQ_get_policy_id(req));
+    return ossl_asn1obj_to_string(TS_REQ_get_policy_id(req));
 }
 
 /*
@@ -948,7 +927,7 @@ ossl_ts_token_info_get_policy_id(VALUE self)
     TS_TST_INFO *info;
 
     GetTSTokenInfo(self, info);
-    return get_asn1obj(TS_TST_INFO_get_policy_id(info));
+    return ossl_asn1obj_to_string(TS_TST_INFO_get_policy_id(info));
 }
 
 /*
@@ -976,7 +955,7 @@ ossl_ts_token_info_get_algorithm(VALUE self)
     mi = TS_TST_INFO_get_msg_imprint(info);
     algo = TS_MSG_IMPRINT_get_algo(mi);
     X509_ALGOR_get0(&obj, NULL, NULL, algo);
-    return get_asn1obj(obj);
+    return ossl_asn1obj_to_string(obj);
 }
 
 /*

ext/openssl/ossl_x509attr.c

@@ -164,29 +164,18 @@ ossl_x509attr_set_oid(VALUE self, VALUE oid)
 
 /*
  * call-seq:
- *    attr.oid => string
+ *    attr.oid -> string
+ *
+ * Returns the OID of the attribute. Returns the short name or the dotted
+ * decimal notation.
  */
 static VALUE
 ossl_x509attr_get_oid(VALUE self)
 {
     X509_ATTRIBUTE *attr;
-    ASN1_OBJECT *oid;
-    BIO *out;
-    VALUE ret;
-    int nid;
 
     GetX509Attr(self, attr);
-    oid = X509_ATTRIBUTE_get0_object(attr);
-    if ((nid = OBJ_obj2nid(oid)) != NID_undef)
-	ret = rb_str_new2(OBJ_nid2sn(nid));
-    else{
-	if (!(out = BIO_new(BIO_s_mem())))
-	    ossl_raise(eX509AttrError, NULL);
-	i2a_ASN1_OBJECT(out, oid);
-	ret = ossl_membio2str(out);
-    }
-
-    return ret;
+    return ossl_asn1obj_to_string(X509_ATTRIBUTE_get0_object(attr));
 }
 
 /*

ext/openssl/ossl_x509cert.c

@@ -318,27 +318,23 @@ ossl_x509_set_serial(VALUE self, VALUE num)
 /*
  * call-seq:
  *    cert.signature_algorithm => string
+ *
+ * Returns the signature algorithm used to sign this certificate. This returns
+ * the algorithm name found in the TBSCertificate structure, not the outer
+ * \Certificate structure.
+ *
+ * Returns the long name of the signature algorithm, or the dotted decimal
+ * notation if \OpenSSL does not define a long name for it.
  */
 static VALUE
 ossl_x509_get_signature_algorithm(VALUE self)
 {
     X509 *x509;
-    BIO *out;
     const ASN1_OBJECT *obj;
-    VALUE str;
 
     GetX509(self, x509);
-    out = BIO_new(BIO_s_mem());
-    if (!out) ossl_raise(eX509CertError, NULL);
-
     X509_ALGOR_get0(&obj, NULL, NULL, X509_get0_tbs_sigalg(x509));
-    if (!i2a_ASN1_OBJECT(out, obj)) {
-	BIO_free(out);
-	ossl_raise(eX509CertError, NULL);
-    }
-    str = ossl_membio2str(out);
-
-    return str;
+    return ossl_asn1obj_to_string_long_name(obj);
 }
 
 /*

ext/openssl/ossl_x509crl.c

@@ -166,26 +166,26 @@ ossl_x509crl_set_version(VALUE self, VALUE version)
     return version;
 }
 
+/*
+ * call-seq:
+ *    crl.signature_algorithm -> string
+ *
+ * Returns the signature algorithm used to sign this CRL.
+ *
+ * Returns the long name of the signature algorithm, or the dotted decimal
+ * notation if \OpenSSL does not define a long name for it.
+ */
 static VALUE
 ossl_x509crl_get_signature_algorithm(VALUE self)
 {
     X509_CRL *crl;
     const X509_ALGOR *alg;
     const ASN1_OBJECT *obj;
-    BIO *out;
 
     GetX509CRL(self, crl);
-    if (!(out = BIO_new(BIO_s_mem()))) {
-	ossl_raise(eX509CRLError, NULL);
-    }
     X509_CRL_get0_signature(crl, NULL, &alg);
     X509_ALGOR_get0(&obj, NULL, NULL, alg);
-    if (!i2a_ASN1_OBJECT(out, obj)) {
-	BIO_free(out);
-	ossl_raise(eX509CRLError, NULL);
-    }
-
-    return ossl_membio2str(out);
+    return ossl_asn1obj_to_string_long_name(obj);
 }
 
 static VALUE

ext/openssl/ossl_x509ext.c

@@ -359,27 +359,20 @@ ossl_x509ext_set_critical(VALUE self, VALUE flag)
     return flag;
 }
 
+/*
+ * call-seq:
+ *    ext.oid -> string
+ *
+ * Returns the OID of the extension. Returns the short name or the dotted
+ * decimal notation.
+ */
 static VALUE
 ossl_x509ext_get_oid(VALUE obj)
 {
     X509_EXTENSION *ext;
-    ASN1_OBJECT *extobj;
-    BIO *out;
-    VALUE ret;
-    int nid;
 
     GetX509Ext(obj, ext);
-    extobj = X509_EXTENSION_get_object(ext);
-    if ((nid = OBJ_obj2nid(extobj)) != NID_undef)
-	ret = rb_str_new2(OBJ_nid2sn(nid));
-    else{
-	if (!(out = BIO_new(BIO_s_mem())))
-	    ossl_raise(eX509ExtError, NULL);
-	i2a_ASN1_OBJECT(out, extobj);
-	ret = ossl_membio2str(out);
-    }
-
-    return ret;
+    return ossl_asn1obj_to_string(X509_EXTENSION_get_object(ext));
 }
 
 static VALUE