From c67e16e726ef261ddb20452fe73ef76b7e325ae8 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 a61d3eefb..f2c545106 100644
--- a/ext/openssl/ossl_asn1.c
+++ b/ext/openssl/ossl_asn1.c
@@ -1297,30 +1297,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)
 {
@@ -1365,6 +1341,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_funcall3(cASN1##klass, rb_intern("new"), argc, argv); }
diff --git a/test/openssl/test_asn1.rb b/test/openssl/test_asn1.rb
index 7e5b9692a..0e8efe343 100644
--- a/test/openssl/test_asn1.rb
+++ b/test/openssl/test_asn1.rb
@@ -331,7 +331,9 @@ def test_object_identifier
       pend "OBJ_obj2txt() not working (LibreSSL?)" if $!.message =~ /OBJ_obj2txt/
       raise
     end
+  end
 
+  def test_object_identifier_equality
     aki = [
       OpenSSL::ASN1::ObjectId.new("authorityKeyIdentifier"),
       OpenSSL::ASN1::ObjectId.new("X509v3 Authority Key Identifier"),
@@ -346,17 +348,22 @@ def test_object_identifier
 
     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