From 599bfa72332cf9dba6fff25dde5b17270c058635 Mon Sep 17 00:00:00 2001 From: nagachika Date: Mon, 13 Apr 2015 13:09:18 +0000 Subject: [PATCH] * ext/openssl/lib/openssl/ssl.rb: stricter hostname verification following RFC 6125. with the patch provided by Tony Arcieri and Hiroshi Nakamura [ruby-core:61545] [Bug #9644] * test/openssl/test_ssl.rb: add tests for above. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@50292 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 7 ++ ext/openssl/lib/openssl/ssl.rb | 62 +++++++++++++- test/openssl/test_ssl.rb | 150 +++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4f1eafe158..51a84a9f23 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +Mon Apr 13 21:20:20 2015 CHIKANAGA Tomoyuki + + * ext/openssl/lib/openssl/ssl.rb: stricter hostname verification + following RFC 6125. with the patch provided by Tony Arcieri and + Hiroshi Nakamura [ruby-core:61545] [Bug #9644] + * test/openssl/test_ssl.rb: add tests for above. + Sun Apr 12 18:40:04 2015 SHIBATA Hiroshi * ext/json/json.gemspec: bump version to 1.8.2. diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb index ff1d4efb32..caf0b9ae44 100644 --- a/ext/openssl/lib/openssl/ssl.rb +++ b/ext/openssl/lib/openssl/ssl.rb @@ -143,8 +143,7 @@ module OpenSSL case san.tag when 2 # dNSName in GeneralName (RFC5280) should_verify_common_name = false - reg = Regexp.escape(san.value).gsub(/\\\*/, "[^.]+") - return true if /\A#{reg}\z/i =~ hostname + return true if verify_hostname(hostname, san.value) when 7 # iPAddress in GeneralName (RFC5280) should_verify_common_name = false # follows GENERAL_NAME_print() in x509v3/v3_alt.c @@ -159,8 +158,7 @@ module OpenSSL if should_verify_common_name cert.subject.to_a.each{|oid, value| if oid == "CN" - reg = Regexp.escape(value).gsub(/\\\*/, "[^.]+") - return true if /\A#{reg}\z/i =~ hostname + return true if verify_hostname(hostname, value) end } end @@ -168,11 +166,67 @@ module OpenSSL end module_function :verify_certificate_identity + def verify_hostname(hostname, san) # :nodoc: + # RFC 5280, IA5String is limited to the set of ASCII characters + return false unless san.ascii_only? + return false unless hostname.ascii_only? + + # See RFC 6125, section 6.4.1 + # Matching is case-insensitive. + san_parts = san.downcase.split(".") + + # TODO: this behavior should probably be more strict + return san == hostname if san_parts.size < 2 + + # Matching is case-insensitive. + host_parts = hostname.downcase.split(".") + + # RFC 6125, section 6.4.3, subitem 2. + # If the wildcard character is the only character of the left-most + # label in the presented identifier, the client SHOULD NOT compare + # against anything but the left-most label of the reference + # identifier (e.g., *.example.com would match foo.example.com but + # not bar.foo.example.com or example.com). + return false unless san_parts.size == host_parts.size + + # RFC 6125, section 6.4.3, subitem 1. + # The client SHOULD NOT attempt to match a presented identifier in + # which the wildcard character comprises a label other than the + # left-most label (e.g., do not match bar.*.example.net). + return false unless verify_wildcard(host_parts.shift, san_parts.shift) + + san_parts.join(".") == host_parts.join(".") + end + module_function :verify_hostname + + def verify_wildcard(domain_component, san_component) # :nodoc: + parts = san_component.split("*", -1) + + return false if parts.size > 2 + return san_component == domain_component if parts.size == 1 + + # RFC 6125, section 6.4.3, subitem 3. + # The client SHOULD NOT attempt to match a presented identifier + # where the wildcard character is embedded within an A-label or + # U-label of an internationalized domain name. + return false if domain_component.start_with?("xn--") && san_component != "*" + + parts[0].length + parts[1].length < domain_component.length && + domain_component.start_with?(parts[0]) && + domain_component.end_with?(parts[1]) + end + module_function :verify_wildcard + class SSLSocket include Buffering include SocketForwarder include Nonblock + ## + # Perform hostname verification after an SSL connection is established + # + # This method MUST be called after calling #connect to ensure that the + # hostname of a remote peer has been verified. def post_connection_check(hostname) unless OpenSSL::SSL.verify_certificate_identity(peer_cert, hostname) raise SSLError, "hostname \"#{hostname}\" does not match the server certificate" diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index 3eddb0a8e8..3ffddf8884 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -426,6 +426,156 @@ class OpenSSL::TestSSL < OpenSSL::SSLTestCase end end + def test_verify_hostname + assert_equal(true, OpenSSL::SSL.verify_hostname("www.example.com", "*.example.com")) + assert_equal(false, OpenSSL::SSL.verify_hostname("www.subdomain.example.com", "*.example.com")) + end + + def test_verify_wildcard + assert_equal(false, OpenSSL::SSL.verify_wildcard("foo", "x*")) + assert_equal(true, OpenSSL::SSL.verify_wildcard("foo", "foo")) + assert_equal(true, OpenSSL::SSL.verify_wildcard("foo", "f*")) + assert_equal(true, OpenSSL::SSL.verify_wildcard("foo", "*")) + assert_equal(false, OpenSSL::SSL.verify_wildcard("abc*bcd", "abcd")) + assert_equal(false, OpenSSL::SSL.verify_wildcard("xn--qdk4b9b", "x*")) + assert_equal(false, OpenSSL::SSL.verify_wildcard("xn--qdk4b9b", "*--qdk4b9b")) + assert_equal(true, OpenSSL::SSL.verify_wildcard("xn--qdk4b9b", "xn--qdk4b9b")) + end + + # Comments in this test is excerpted from http://tools.ietf.org/html/rfc6125#page-27 + def test_post_connection_check_wildcard_san + # case-insensitive ASCII comparison + # RFC 6125, section 6.4.1 + # + # "..matching of the reference identifier against the presented identifier + # is performed by comparing the set of domain name labels using a + # case-insensitive ASCII comparison, as clarified by [DNS-CASE] (e.g., + # "WWW.Example.Com" would be lower-cased to "www.example.com" for + # comparison purposes) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*.example.com'), 'www.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*.Example.COM'), 'www.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*.example.com'), 'WWW.Example.COM')) + # 1. The client SHOULD NOT attempt to match a presented identifier in + # which the wildcard character comprises a label other than the + # left-most label (e.g., do not match bar.*.example.net). + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:www.*.com'), 'www.example.com')) + # 2. If the wildcard character is the only character of the left-most + # label in the presented identifier, the client SHOULD NOT compare + # against anything but the left-most label of the reference + # identifier (e.g., *.example.com would match foo.example.com but + # not bar.foo.example.com or example.com). + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*.example.com'), 'foo.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*.example.com'), 'bar.foo.example.com')) + # 3. The client MAY match a presented identifier in which the wildcard + # character is not the only character of the label (e.g., + # baz*.example.net and *baz.example.net and b*z.example.net would + # be taken to match baz1.example.net and foobaz.example.net and + # buzz.example.net, respectively). ... + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:baz*.example.com'), 'baz1.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*baz.example.com'), 'foobaz.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:b*z.example.com'), 'buzz.example.com')) + # Section 6.4.3 of RFC6125 states that client should NOT match identifier + # where wildcard is other than left-most label. + # + # Also implicitly mentions the wildcard character only in singular form, + # and discourages matching against more than one wildcard. + # + # See RFC 6125, section 7.2, subitem 2. + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*b*.example.com'), 'abc.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*b*.example.com'), 'ab.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:*b*.example.com'), 'bc.example.com')) + # ... However, the client SHOULD NOT + # attempt to match a presented identifier where the wildcard + # character is embedded within an A-label or U-label [IDNA-DEFS] of + # an internationalized domain name [IDNA-PROTO]. + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:xn*.example.com'), 'xn1ca.example.com')) + # part of A-label + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:xn--*.example.com'), 'xn--1ca.example.com')) + # part of U-label + # dNSName in RFC5280 is an IA5String so U-label should NOT be allowed + # regardless of wildcard. + # + # See Section 7.2 of RFC 5280: + # IA5String is limited to the set of ASCII characters. + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_san('DNS:á*.example.com'), 'á1.example.com')) + end + + def test_post_connection_check_wildcard_cn + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*.example.com'), 'www.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*.Example.COM'), 'www.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*.example.com'), 'WWW.Example.COM')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('www.*.com'), 'www.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*.example.com'), 'foo.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*.example.com'), 'bar.foo.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('baz*.example.com'), 'baz1.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*baz.example.com'), 'foobaz.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('b*z.example.com'), 'buzz.example.com')) + # Section 6.4.3 of RFC6125 states that client should NOT match identifier + # where wildcard is other than left-most label. + # + # Also implicitly mentions the wildcard character only in singular form, + # and discourages matching against more than one wildcard. + # + # See RFC 6125, section 7.2, subitem 2. + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*b*.example.com'), 'abc.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*b*.example.com'), 'ab.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('*b*.example.com'), 'bc.example.com')) + assert_equal(true, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('xn*.example.com'), 'xn1ca.example.com')) + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('xn--*.example.com'), 'xn--1ca.example.com')) + # part of U-label + # Subject in RFC5280 states case-insensitive ASCII comparison. + # + # See Section 7.2 of RFC 5280: + # IA5String is limited to the set of ASCII characters. + assert_equal(false, OpenSSL::SSL.verify_certificate_identity( + create_cert_with_name('á*.example.com'), 'á1.example.com')) + end + + def create_cert_with_san(san) + ef = OpenSSL::X509::ExtensionFactory.new + cert = OpenSSL::X509::Certificate.new + cert.subject = OpenSSL::X509::Name.parse("/DC=some/DC=site/CN=Some Site") + ext = ef.create_ext('subjectAltName', san) + cert.add_extension(ext) + cert + end + + def create_cert_with_name(name) + cert = OpenSSL::X509::Certificate.new + cert.subject = OpenSSL::X509::Name.new([['DC', 'some'], ['DC', 'site'], ['CN', name]]) + cert + end + + # Create NULL byte SAN certificate def create_null_byte_SAN_certificate(critical = false) ef = OpenSSL::X509::ExtensionFactory.new