Skip to content

Commit

Permalink
Correctly verify abbreviated IPv6 SANs
Browse files Browse the repository at this point in the history
IPv6 SAN-verification accommodates
["zero-compression"](https://tools.ietf.org/html/rfc5952#section-2.2).
It also accommodates non-compressed addresses.

Previously the verification of IPv6 addresses would fail unless the
address syntax matched a specific format (no zero-compression, no
leading zeroes).

As an example, the IPv6 loopback address, if represented as `::1`, would
not verify.  Nor would it verify if represented as
`0000:0000:0000:0000:0000:0000:0000:0001`; however, both representations
are valid, RFC-compliant representations.  The library would only accept
a very specific representation (i.e.  `0:0:0:0:0:0:0:1`).

This commit addresses that shortcoming, and ensures that any valid IPv6
representation will correctly verify.
  • Loading branch information
cunnie committed Feb 6, 2018
1 parent f707996 commit 9322a10
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 6 deletions.
11 changes: 6 additions & 5 deletions lib/openssl/ssl.rb
Expand Up @@ -12,6 +12,7 @@

require "openssl/buffering"
require "io/nonblock"
require "ipaddr"

module OpenSSL
module SSL
Expand Down Expand Up @@ -272,11 +273,11 @@ def verify_certificate_identity(cert, 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
if san.value.size == 4
return true if san.value.unpack('C*').join('.') == hostname
elsif san.value.size == 16
return true if san.value.unpack('n*').map { |e| sprintf("%X", e) }.join(':') == hostname
if san.value.size == 4 || san.value.size == 16
begin
return true if san.value == IPAddr.new(hostname).hton
rescue IPAddr::InvalidAddressError
end
end
end
}
Expand Down
1 change: 1 addition & 0 deletions openssl.gemspec
Expand Up @@ -17,6 +17,7 @@ Gem::Specification.new do |spec|

spec.required_ruby_version = ">= 2.3.0"

spec.add_runtime_dependency "ipaddr"

This comment has been minimized.

Copy link
@nijikon

nijikon May 7, 2019

@cunnie why was this dependency added here? Why can't we rely on the stdlib one?

This comment has been minimized.

Copy link
@cunnie

cunnie May 13, 2019

Author Contributor

Hi @nijikon , thank you for reviewing my code! Originally I left this line out, but rhenium pointed out:

The ipaddr library is a default gem in Ruby 2.5, so it needs to be added to openssl.gemspec as a runtime dependency.

I believe this is a side-effect of the gemified standard library. In other words, my understanding is that we are relying on the stdlib one.

spec.add_development_dependency "rake"
spec.add_development_dependency "rake-compiler"
spec.add_development_dependency "test-unit", "~> 3.0"
Expand Down
6 changes: 5 additions & 1 deletion test/test_ssl.rb
Expand Up @@ -516,8 +516,12 @@ def test_verify_certificate_identity
assert_equal(true, OpenSSL::SSL.verify_certificate_identity(cert, "www.example.com\0.evil.com"))
assert_equal(false, OpenSSL::SSL.verify_certificate_identity(cert, '192.168.7.255'))
assert_equal(true, OpenSSL::SSL.verify_certificate_identity(cert, '192.168.7.1'))
assert_equal(false, OpenSSL::SSL.verify_certificate_identity(cert, '13::17'))
assert_equal(true, OpenSSL::SSL.verify_certificate_identity(cert, '13::17'))
assert_equal(false, OpenSSL::SSL.verify_certificate_identity(cert, '13::18'))
assert_equal(true, OpenSSL::SSL.verify_certificate_identity(cert, '13:0:0:0:0:0:0:17'))
assert_equal(false, OpenSSL::SSL.verify_certificate_identity(cert, '44:0:0:0:0:0:0:17'))
assert_equal(true, OpenSSL::SSL.verify_certificate_identity(cert, '0013:0000:0000:0000:0000:0000:0000:0017'))
assert_equal(false, OpenSSL::SSL.verify_certificate_identity(cert, '1313:0000:0000:0000:0000:0000:0000:0017'))
end
end

Expand Down

0 comments on commit 9322a10

Please sign in to comment.