Merge pull request #967 from rhenium/ky/asn1-refactor-obj-str-convert

asn1: refactor converting ASN1_OBJECT to string

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
@@ -160,7 +202,7 @@ asn1integer_to_num_i(VALUE arg)
 #define ossl_asn1_set_indefinite_length(o,v) rb_ivar_set((o),sivINDEFINITE_LENGTH,(v))
 
 VALUE mASN1;
-VALUE eASN1Error;
+static VALUE eASN1Error;
 
 VALUE cASN1Data;
 static VALUE cASN1Primitive;
@@ -247,8 +289,8 @@ obj_to_asn1null(VALUE obj)
     return null;
 }
 
-static ASN1_OBJECT*
-obj_to_asn1obj(VALUE obj)
+ASN1_OBJECT *
+ossl_to_asn1obj(VALUE obj)
 {
     ASN1_OBJECT *a1obj;
 
@@ -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;
 }
 
@@ -544,7 +581,7 @@ ossl_asn1_get_asn1type(VALUE obj)
 	free_func = (free_func_type *)ASN1_STRING_free;
 	break;
     case V_ASN1_OBJECT:
-	ptr = obj_to_asn1obj(value);
+	ptr = ossl_to_asn1obj(value);
 	free_func = (free_func_type *)ASN1_OBJECT_free;
 	break;
     case V_ASN1_UTCTIME:
@@ -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);
 }
 
 /*
@@ -1205,7 +1226,7 @@ ossl_asn1obj_get_oid(VALUE self)
     ASN1_OBJECT *a1obj;
     int state;
 
-    a1obj = obj_to_asn1obj(ossl_asn1_get_value(self));
+    a1obj = ossl_to_asn1obj(ossl_asn1_get_value(self));
     str = rb_protect(asn1obj_get_oid_i, (VALUE)a1obj, &state);
     ASN1_OBJECT_free(a1obj);
     if (state)

ext/openssl/ossl_asn1.h

@@ -31,11 +31,25 @@ VALUE asn1str_to_str(const ASN1_STRING *);
 VALUE asn1integer_to_num(const ASN1_INTEGER *);
 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
  */
 extern VALUE mASN1;
-extern VALUE eASN1Error;
 
 extern VALUE cASN1Data;
 

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

@@ -132,44 +132,10 @@ asn1_to_der(void *template, int (*i2d)(void *template, unsigned char **pp))
     return str;
 }
 
-static ASN1_OBJECT*
-obj_to_asn1obj(VALUE obj)
-{
-    ASN1_OBJECT *a1obj;
-
-    StringValue(obj);
-    a1obj = OBJ_txt2obj(RSTRING_PTR(obj), 0);
-    if(!a1obj) a1obj = OBJ_txt2obj(RSTRING_PTR(obj), 1);
-    if(!a1obj) ossl_raise(eASN1Error, "invalid OBJECT ID");
-
-    return a1obj;
-}
-
 static VALUE
 obj_to_asn1obj_i(VALUE obj)
 {
-    return (VALUE)obj_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;
+    return (VALUE)ossl_to_asn1obj(obj);
 }
 
 static VALUE
@@ -242,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);
 }
 
 /*
@@ -264,7 +230,7 @@ ossl_ts_req_set_algorithm(VALUE self, VALUE algo)
     X509_ALGOR *algor;
 
     GetTSRequest(self, req);
-    obj = obj_to_asn1obj(algo);
+    obj = ossl_to_asn1obj(algo);
     mi = TS_REQ_get_msg_imprint(req);
     algor = TS_MSG_IMPRINT_get_algo(mi);
     if (!X509_ALGOR_set0(algor, obj, V_ASN1_NULL, NULL)) {
@@ -371,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));
 }
 
 /*
@@ -394,7 +360,7 @@ ossl_ts_req_set_policy_id(VALUE self, VALUE oid)
     int ok;
 
     GetTSRequest(self, req);
-    obj = obj_to_asn1obj(oid);
+    obj = ossl_to_asn1obj(oid);
     ok = TS_REQ_set_policy_id(req, obj);
     ASN1_OBJECT_free(obj);
     if (!ok)
@@ -961,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));
 }
 
 /*
@@ -989,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);
 }
 
 /*