Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support more complex SANs in verify_certificate_identity #325

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions lib/openssl/ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,26 +270,21 @@ def verify_certificate_identity(cert, hostname)
should_verify_common_name = true
cert.extensions.each{|ext|
next if ext.oid != "subjectAltName"
ext.value.split(/,\s+/).each { |general_name|
#case san.tag
# MRI 2.2.3 (JRuby parses ASN.1 differently)
#when 2 # dNSName in GeneralName (RFC5280)
if /\ADNS:(.*)/ =~ general_name
ostr = OpenSSL::ASN1.decode(ext.to_der).value.last
sequence = OpenSSL::ASN1.decode(ostr.value)
sequence.value.each{|san|
case san.tag
when 2 # dNSName in GeneralName (RFC5280)
should_verify_common_name = false
return true if verify_hostname(hostname, $1)
# MRI 2.2.3 (JRuby parses ASN.1 differently)
#when 7 # iPAddress in GeneralName (RFC5280)
elsif /\AIP(?: Address)?:(.*)/ =~ general_name
return true if verify_hostname(hostname, san.value.last.value)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ruby-openssl this was verify_hostname(hostname, san.value), but I've had to add .last.value to get this to work here, as I'm getting a single element Array returned instead of a plain string. Maybe this isn't the right solution though. Should san.value instead return the exact same thing across both implementations?? I've not worked out where this is done though, and maybe it'd break loads of other things...

when 7 # iPAddress in GeneralName (RFC5280)
should_verify_common_name = false
return true if $1 == hostname
# NOTE: bellow logic makes little sense JRuby reads exts differently
# follows GENERAL_NAME_print() in x509v3/v3_alt.c
# if san.value.size == 4 || san.value.size == 16
# begin
# return true if $1 == IPAddr.new(hostname).to_s
# rescue IPAddr::InvalidAddressError
# end
# end
if san.value.last.value.size == 4 || san.value.last.value.size == 16
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. san.value is changed to san.value.last.value

begin
return true if san.value.last.value == IPAddr.new(hostname).hton
rescue IPAddr::InvalidAddressError
end
end
end
}
}
Expand Down
15 changes: 15 additions & 0 deletions src/test/ruby/ssl/fixtures/othername_cert/othername.cnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[ req ]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't work out how to generate a test certificate with the otherName extension programatically, so used openssl on the command line with this config file.

openssl req -newkey rsa:2048 -nodes -x509 -days 36500 -keyout othername.key -out othername.crt -config othername.cnf -extensions req_ext

distinguished_name = req_distinguished_name
req_extensions = req_ext
prompt = no

[ req_distinguished_name ]
CN = TestOtherName

[ req_ext ]
subjectAltName = @alt_names

[ alt_names ]
otherName.1 = 1.3.6.1.4.1.311.20.2.3;UTF8:example-other-name@localdomain
DNS.2 = localhost.localdomain
IP.3 = 127.0.0.1
19 changes: 19 additions & 0 deletions src/test/ruby/ssl/fixtures/othername_cert/othername.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-----BEGIN CERTIFICATE-----
MIIDDzCCAfegAwIBAgIJALtkg6B2cBoDMA0GCSqGSIb3DQEBCwUAMBgxFjAUBgNV
BAMMDVRlc3RPdGhlck5hbWUwIBcNMjUwMTAzMTQ1MDA5WhgPMjEyNDEyMTAxNDUw
MDlaMBgxFjAUBgNVBAMMDVRlc3RPdGhlck5hbWUwggEiMA0GCSqGSIb3DQEBAQUA
A4IBDwAwggEKAoIBAQCbLUib/ie6WijVTtHsHo4hTK3cG1xsrpb1W8JpmNMGh/AA
hC9qpqEgH0tjrDCYWt8BYFwv5xrp54JzssslSdTgmtXg5TeLn90BtyFtBURmvO/U
uRM9qCkwewZodv22sZ8+D6I+zEUbGBkwrJsWB65MBxGqcjUCATgKn2BdTlJEVSSA
oZeuDFfLlfgJmSQALlkFHmacV6qVYIbOQ4iANodwjZezmQuJZNByVwzkUh4b3KlF
xDoEKkzAJFssxk2ONFCrA7TIwuVFpu1QHWDyDAYDCMatLo20fjrBbaabIa5ORM8R
qDXlJs+OU1/fI+lrWaPuGcG3m66QZgsuXaB4EfrnAgMBAAGjWjBYMFYGA1UdEQRP
ME2gLgYKKwYBBAGCNxQCA6AgDB5leGFtcGxlLW90aGVyLW5hbWVAbG9jYWxkb21h
aW6CFWxvY2FsaG9zdC5sb2NhbGRvbWFpbocEfwAAATANBgkqhkiG9w0BAQsFAAOC
AQEAc8u8+FNMOonSns+eeUxfeqERcdBsHhxagOVwN+D5U8nIUZ5Ys3AT3DqlIIpC
O/HmUqkY2YR7iWck91BO9OryTCqc6z5Lk/yCksjKpMjzWm4dUDqXB3wm3TGrdXJ2
LkPk7oDPyUqP12ZE/kFaUhThpOWqQa/ga1kvi40St1F8nI8Fagk9Ls5LMklAEEQ0
/HyqXDvh2nUC0nmTybLVU/ShOiZRP4+t1r8TOEQouoGz+XQVu2FlZNGsz0w7UL5U
3WDftyU3oSaYFN2E3pV1ZWQ2OUsESeCwCjMpJEF1L3lQBFKXmXQhLaGppXu/G4gF
PtwVm0nkXHe6qd+icwNMYon6Rg==
-----END CERTIFICATE-----
28 changes: 28 additions & 0 deletions src/test/ruby/ssl/fixtures/othername_cert/othername.key
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
-----BEGIN PRIVATE KEY-----
MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQCbLUib/ie6WijV
TtHsHo4hTK3cG1xsrpb1W8JpmNMGh/AAhC9qpqEgH0tjrDCYWt8BYFwv5xrp54Jz
ssslSdTgmtXg5TeLn90BtyFtBURmvO/UuRM9qCkwewZodv22sZ8+D6I+zEUbGBkw
rJsWB65MBxGqcjUCATgKn2BdTlJEVSSAoZeuDFfLlfgJmSQALlkFHmacV6qVYIbO
Q4iANodwjZezmQuJZNByVwzkUh4b3KlFxDoEKkzAJFssxk2ONFCrA7TIwuVFpu1Q
HWDyDAYDCMatLo20fjrBbaabIa5ORM8RqDXlJs+OU1/fI+lrWaPuGcG3m66QZgsu
XaB4EfrnAgMBAAECggEAU+bxKn8KJgVXrLX4ZQ3EVZbrLmaQ26ausq1Red0KxK49
g1n1dVnxJrglABh7EfWYPFtInRH9CFmHvF0A4AGCm4q8d+QX22OtkJ7e8N53NIGV
v1PuZlkFJFRwi41C80Qo0NqNJB04hETMYfgBxrCSekMcNyeXJ6VXEs9HwEt3yb/T
8otVis+L/eMxjhGL5W7OpuTqh+0bekk5/rU7VyzFkp7eFFq2N0oHKq0oZhsoweZe
tyD/CDOLpu8YrKpXGJ6hkQrOiaQjkK8GJFip6zf/Gqdi0iRADnKV2W/S9Ebe2sCR
9nQsHDaerqA8G6qFfDgeoL+yXsOGucOqBeDuyfjviQKBgQDK1wivonuBSy7m2ZBP
LbEvum+/B5HrJOjG1jDmJWVEcdsgWTXW/KdUes8/YdsC+rE16orhVBZviy4lzIln
Lox0cKvYkhonRdJHcOA0874i6lcESPA8YQeMypVuy2uo3yV2WBHn5jD/imP3xssO
aczDYNMyYzJxN9YDP7OcsBVuAwKBgQDD2G6BVzhU/9WKelDnNW42lGlNuVhwMr5q
+StVmyLkoghWt+rXT3err9UNwyvIzH2RJ15gxx6bpCB7Wqc7U6hkvox8iUm9E20s
OSnlHVzrP4tOoTIr/854L1evJu8B0Tw5llLJpcpK/wXric2hkryTCovIvU3mCwTw
dpQxJP1MTQKBgQCjSdQuP6kY/oM1R7OKaggXmghXSirHlyDk4Izg2P3ayaVTNz3G
YgH6Whr4tTfwAAwkkTlYB69AAFUYDL4YGDrtib5tS8BOGUEcTdvQBN/tj6SZSHZM
xPek+XiuhoeWnHy19lQD0SVkse2kC55vbfBu4EVkbMKwrfhVCTT4UDAQLQKBgH8u
Cr0FpfhU1xiBTA6JfRQGlse5iDv8R/nx63lBMIymyKF/+ApwebdNjR7N0p4oZZag
qUJRrIfIMqvId/cn4Z/iuhqZkQAvIGNqj9FQmynN7ypVtd4q1arom5mLwAQ/G0wO
WZ7HgjHnoLGPoLC/OKSIYbQvcunj2AZMCvpLGg61AoGBALDXcv0+vrSXwFDDyWLw
RG4wVwU1xdjXPq0YJWKQC21vpCgLykBbae8INfPCQFnH0ArQF5K08d3LqqjD6lfE
04PY2xuJiqhplsCyMeLWeuRpKoOOIo+V8L2qdVpySobAv2dHvNfQ/vcyGF/Jq6O9
gvuz+L76RzERQpLm1lzixC4p
-----END PRIVATE KEY-----
27 changes: 27 additions & 0 deletions src/test/ruby/ssl/test_ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,33 @@ def test_post_connection_check
end
end

def test_post_connection_check_othername_subject_alt_name
sslerr = OpenSSL::SSL::SSLError

fixtures_path = File.expand_path('fixtures/othername_cert', File.dirname(__FILE__))

@ca_cert = OpenSSL::X509::Certificate.new(File.read(File.join(fixtures_path, 'othername.crt')))
@svr_cert = OpenSSL::X509::Certificate.new(File.read(File.join(fixtures_path, 'othername.crt')))
@svr_key = OpenSSL::PKey.read(File.read(File.join(fixtures_path, 'othername.key')))

start_server0(PORT, OpenSSL::SSL::VERIFY_NONE, true) do |server, port|
sock = TCPSocket.new("127.0.0.1", port)
ssl = OpenSSL::SSL::SSLSocket.new(sock)
ssl.connect

assert ssl.post_connection_check("localhost.localdomain")
assert ssl.post_connection_check("127.0.0.1")
assert_raise(sslerr) { ssl.post_connection_check("localhost") }
assert_raise(sslerr) { ssl.post_connection_check("foo.example.com") }

cert = ssl.peer_cert
assert OpenSSL::SSL.verify_certificate_identity(cert, "localhost.localdomain")
assert OpenSSL::SSL.verify_certificate_identity(cert, "127.0.0.1")
refute OpenSSL::SSL.verify_certificate_identity(cert, "localhost")
refute OpenSSL::SSL.verify_certificate_identity(cert, "foo.example.com")
end
end

def test_post_connect_check_with_anon_ciphers
unless OpenSSL::ExtConfig::TLS_DH_anon_WITH_AES_256_GCM_SHA384
return skip('OpenSSL::ExtConfig::TLS_DH_anon_WITH_AES_256_GCM_SHA384 not enabled')
Expand Down