x509: do not escape UTF-8 charactersreject/x509name-to_s-unescape-utf8

Avoid escaping of non-ASCII characters OpenSSL::X509::Name#to_s. Set
proper encoding information to the resulting String instead of
ASCII-8BIT.

X509_NAME_print_ex() escapes "high characters" (with MSB set to 1) with
the predefined XN_FLAG_* flags. This is to protect terminals that don't
process non-ASCII characters.

Clearly we don't want this. Ruby can handle Unicode characters properly.
For XN_FLAG_{RFC2253,ONELINE,MULTILINE} formats, we can do that by
removing ASN1_STRFLGS_ESC_MSB flag from them.

Note that this does not fix the escape in the default format (with no
argument, traditional /attr1=value/attr2=value format) and
XN_FLAG_COMPAT format. They are fundamentally broken and are not worth
fixing.

Backwards compatibility considerations:

  * Programs using the workaround suggested at ruby/openssl#26 break
    because the value of the constants OpenSSL::X509::Name::* is no
    longer the raw flag value (Integer).

Fixes: https://github.com/ruby/openssl/issues/26

2 files changed, 52 insertions, 7 deletions

diff --git a/ext/openssl/ossl_x509name.c b/ext/openssl/ossl_x509name.c
index a10ae2c3..94b46995 100644
--- a/ext/openssl/ossl_x509name.c
+++ b/ext/openssl/ossl_x509name.c
@@ -38,6 +38,7 @@
  */
 VALUE cX509Name;
 VALUE eX509NameError;
+static VALUE sym_compat, sym_rfc2253, sym_oneline, sym_multiline;
 
 static void
 ossl_x509name_free(void *ptr)
@@ -243,7 +244,7 @@ ossl_x509name_to_s_old(VALUE self)
 
     GetX509Name(self, name);
     buf = X509_NAME_oneline(name, NULL, 0);
-    str = rb_str_new2(buf);
+    str = rb_usascii_str_new_cstr(buf);
     OPENSSL_free(buf);
 
     return str;
@@ -272,7 +273,23 @@ ossl_x509name_to_s(int argc, VALUE *argv, VALUE self)
     rb_scan_args(argc, argv, "01", &flag);
     if (NIL_P(flag))
 	return ossl_x509name_to_s_old(self);
-    else iflag = NUM2ULONG(flag);
+
+    if (flag == sym_compat)
+	iflag = XN_FLAG_COMPAT;
+    else if (flag == sym_rfc2253)
+	iflag = XN_FLAG_RFC2253 & ~ASN1_STRFLGS_ESC_MSB;
+    else if (flag == sym_oneline)
+	iflag = XN_FLAG_ONELINE & ~ASN1_STRFLGS_ESC_MSB;
+    else if (flag == sym_multiline)
+	iflag = XN_FLAG_MULTILINE & ~ASN1_STRFLGS_ESC_MSB | ASN1_STRFLGS_UTF8_CONVERT;
+    else if (RB_INTEGER_TYPE_P(flag)) {
+	/* OpenSSL::X509::Name::* constants used to be the raw flag integer. */
+	iflag = NUM2ULONG(flag);
+	rb_warn("OpenSSL::X509::Name#to_s(integer) is deprecated");
+    }
+    else
+	ossl_raise(rb_eArgError, "invalid format: %+"PRIsVALUE, flag);
+
     if (!(out = BIO_new(BIO_s_mem())))
 	ossl_raise(eX509NameError, NULL);
     GetX509Name(self, name);
@@ -281,6 +298,7 @@ ossl_x509name_to_s(int argc, VALUE *argv, VALUE self)
 	ossl_raise(eX509NameError, NULL);
     }
     str = ossl_membio2str(out);
+    rb_enc_associate_index(str, rb_utf8_encindex());
 
     return str;
 }
@@ -510,32 +528,37 @@ Init_ossl_x509name(void)
      */
     rb_define_const(cX509Name, "OBJECT_TYPE_TEMPLATE", hash);
 
+    sym_compat = ID2SYM(rb_intern("COMPAT"));
+    sym_rfc2253 = ID2SYM(rb_intern("RFC2253"));
+    sym_oneline = ID2SYM(rb_intern("ONELINE"));
+    sym_multiline = ID2SYM(rb_intern("MULTILINE"));
+
     /*
      * A flag for #to_s.
      *
      * Breaks the name returned into multiple lines if longer than 80
      * characters.
      */
-    rb_define_const(cX509Name, "COMPAT", ULONG2NUM(XN_FLAG_COMPAT));
+    rb_define_const(cX509Name, "COMPAT", sym_compat);
 
     /*
      * A flag for #to_s.
      *
      * Returns an RFC2253 format name.
      */
-    rb_define_const(cX509Name, "RFC2253", ULONG2NUM(XN_FLAG_RFC2253));
+    rb_define_const(cX509Name, "RFC2253", sym_rfc2253);
 
     /*
      * A flag for #to_s.
      *
      * Returns a more readable format than RFC2253.
      */
-    rb_define_const(cX509Name, "ONELINE", ULONG2NUM(XN_FLAG_ONELINE));
+    rb_define_const(cX509Name, "ONELINE", sym_oneline);
 
     /*
      * A flag for #to_s.
      *
      * Returns a multiline format.
      */
-    rb_define_const(cX509Name, "MULTILINE", ULONG2NUM(XN_FLAG_MULTILINE));
+    rb_define_const(cX509Name, "MULTILINE", sym_multiline);
 }
diff --git a/test/test_x509name.rb b/test/test_x509name.rb
index 83b1247d..44d6c8c6 100644
--- a/test/test_x509name.rb
+++ b/test/test_x509name.rb
@@ -1,4 +1,4 @@
-# coding: US-ASCII
+# coding: ASCII-8BIT
 # frozen_string_literal: false
 require_relative 'utils'
 
@@ -364,6 +364,28 @@ class OpenSSL::TestX509Name < OpenSSL::TestCase
     assert_equal false, name0.eql?(name2)
   end
 
+  def test_to_s
+    name = OpenSSL::X509::Name.new([
+      ["DC", "org"],
+      ["DC", "ruby-lang"],
+      ["CN", "ｌｏｃａｌｈｏｓｔ", OpenSSL::ASN1::UTF8STRING]
+    ])
+    assert_equal Encoding::UTF_8, name.to_s(OpenSSL::X509::Name::RFC2253).encoding
+    assert_equal "CN=ｌｏｃａｌｈｏｓｔ,DC=ruby-lang,DC=org", name.to_s(OpenSSL::X509::Name::RFC2253).b
+    assert_equal "DC = org, DC = ruby-lang, CN = ｌｏｃａｌｈｏｓｔ", name.to_s(OpenSSL::X509::Name::ONELINE).b
+    assert_match(/ｌｏｃａｌｈｏｓｔ/, name.to_s(OpenSSL::X509::Name::MULTILINE).b)
+
+    name = OpenSSL::X509::Name.new([
+      ["DC", "org"],
+      ["DC", "ruby-lang"],
+      ["CN", "ｌｏｃａｌｈｏｓｔ".encode("UTF-16BE", "UTF-8"), OpenSSL::ASN1::BMPSTRING]
+    ])
+    assert_equal Encoding::UTF_8, name.to_s(OpenSSL::X509::Name::RFC2253).encoding
+    assert_equal "CN=ｌｏｃａｌｈｏｓｔ,DC=ruby-lang,DC=org", name.to_s(OpenSSL::X509::Name::RFC2253).b
+    assert_equal "DC = org, DC = ruby-lang, CN = ｌｏｃａｌｈｏｓｔ", name.to_s(OpenSSL::X509::Name::ONELINE).b
+    assert_match(/ｌｏｃａｌｈｏｓｔ/, name.to_s(OpenSSL::X509::Name::MULTILINE).b)
+  end
+
   def test_dup
     name = OpenSSL::X509::Name.parse("/CN=ruby-lang.org")
     assert_equal(name.to_der, name.dup.to_der)