From a0f1f161452e9a8c3d017ffbfce5744c775b668f Mon Sep 17 00:00:00 2001
From: Kazuki Yamaguchi <k@rhe.jp>
Date: Sat, 24 Aug 2024 13:29:50 +0900
Subject: [PATCH] asn1: fix ObjectId#==

Compare by the dotted decimal notation rather than the NID.

OpenSSL::ASN1::ObjectId can store OIDs that are not registered in
OpenSSL's internal table. NID is not defined for such an OID, but it is
not an error.

The == method also should not raise TypeError if the other object is
not an instance of OpenSSL::ASN1::ObjectId.

Fixes: https://github.com/ruby/openssl/issues/791
---
 ext/openssl/ossl_asn1.c   | 43 +++++++++++++++++----------------------
 test/openssl/test_asn1.rb | 17 +++++++++++-----
 2 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c
index 1676defd0e..f57657c802 100644
--- a/ext/openssl/ossl_asn1.c
+++ b/ext/openssl/ossl_asn1.c
@@ -1167,30 +1167,6 @@ ossl_asn1obj_get_ln(VALUE self)
     return ret;
 }
 
-/*
- *  call-seq:
- *     oid == other_oid => true or false
- *
- *  Returns +true+ if _other_oid_ is the same as _oid_
- */
-static VALUE
-ossl_asn1obj_eq(VALUE self, VALUE other)
-{
-    VALUE valSelf, valOther;
-    int nidSelf, nidOther;
-
-    valSelf = ossl_asn1_get_value(self);
-    valOther = ossl_asn1_get_value(other);
-
-    if ((nidSelf = OBJ_txt2nid(StringValueCStr(valSelf))) == NID_undef)
-	ossl_raise(eASN1Error, "OBJ_txt2nid");
-
-    if ((nidOther = OBJ_txt2nid(StringValueCStr(valOther))) == NID_undef)
-	ossl_raise(eASN1Error, "OBJ_txt2nid");
-
-    return nidSelf == nidOther ? Qtrue : Qfalse;
-}
-
 static VALUE
 asn1obj_get_oid_i(VALUE vobj)
 {
@@ -1235,6 +1211,25 @@ ossl_asn1obj_get_oid(VALUE self)
     return str;
 }
 
+/*
+ *  call-seq:
+ *     oid == other_oid => true or false
+ *
+ *  Returns +true+ if _other_oid_ is the same as _oid_.
+ */
+static VALUE
+ossl_asn1obj_eq(VALUE self, VALUE other)
+{
+    VALUE oid1, oid2;
+
+    if (!rb_obj_is_kind_of(other, cASN1ObjectId))
+        return Qfalse;
+
+    oid1 = ossl_asn1obj_get_oid(self);
+    oid2 = ossl_asn1obj_get_oid(other);
+    return rb_str_equal(oid1, oid2);
+}
+
 #define OSSL_ASN1_IMPL_FACTORY_METHOD(klass) \
 static VALUE ossl_asn1_##klass(int argc, VALUE *argv, VALUE self)\
 { return rb_funcallv_public(cASN1##klass, rb_intern("new"), argc, argv); }
diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb
index 7b1722e5df..354b587895 100644
--- a/test/openssl/test_asn1.rb
+++ b/test/openssl/test_asn1.rb
@@ -326,7 +326,9 @@ class  OpenSSL::TestASN1 < OpenSSL::TestCase
     oid = (0...100).to_a.join(".").b
     obj = OpenSSL::ASN1::ObjectId.new(oid)
     assert_equal oid, obj.oid
+  end
 
+  def test_object_identifier_equality
     aki = [
       OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier"),
       OpenSSL::ASN1::ObjectId.new("X509v3 Authority Key Identifier"),
@@ -341,17 +343,22 @@ class  OpenSSL::TestASN1 < OpenSSL::TestCase
 
     aki.each do |a|
       aki.each do |b|
-        assert a == b
+        assert_equal true, a == b
       end
 
       ski.each do |b|
-        refute a == b
+        assert_equal false, a == b
       end
     end
 
-    assert_raise(TypeError) {
-      OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier") == nil
-    }
+    obj1 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.10")
+    obj2 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.10")
+    obj3 = OpenSSL::ASN1::ObjectId.new("1.2.34.56789.11")
+    omit "OID 1.2.34.56789.10 is registered" if obj1.sn
+    assert_equal true, obj1 == obj2
+    assert_equal false, obj1 == obj3
+
+    assert_equal false, OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier") == nil
   end
 
   def test_sequence