https: Prevent leaking sockets for some operations
Other OpenStack services which instantiate a 'https' glanceclient using ssl_compression=False and insecure=False (eg Nova, Cinder) are leaking sockets due to glanceclient not closing the connection to the Glance server. This could happen for a sub-set of calls, eg 'show', 'delete', 'update'. netstat -nopd would show the sockets would hang around forever: ... 127.0.0.1:9292 ESTABLISHED 9552/python off (0.00/0/0) urllib's ConnectionPool relies on the garbage collector to tear down sockets which are no longer in use. The 'verify_callback' function used to validate SSL certs was holding a reference to the VerifiedHTTPSConnection instance which prevented the sockets being torn down. Change-Id: Idb3e68151c48ed623ab89d05d88ea48465429838 Closes-bug: 1423165
This commit is contained in:
parent
a3eaafefbd
commit
ef9fd9fca0
|
@ -53,6 +53,82 @@ except ImportError:
|
|||
from glanceclient import exc
|
||||
|
||||
|
||||
def verify_callback(host=None):
|
||||
"""
|
||||
We use a partial around the 'real' verify_callback function
|
||||
so that we can stash the host value without holding a
|
||||
reference on the VerifiedHTTPSConnection.
|
||||
"""
|
||||
def wrapper(connection, x509, errnum,
|
||||
depth, preverify_ok, host=host):
|
||||
return do_verify_callback(connection, x509, errnum,
|
||||
depth, preverify_ok, host=host)
|
||||
return wrapper
|
||||
|
||||
|
||||
def do_verify_callback(connection, x509, errnum,
|
||||
depth, preverify_ok, host=None):
|
||||
"""
|
||||
Verify the server's SSL certificate.
|
||||
|
||||
This is a standalone function rather than a method to avoid
|
||||
issues around closing sockets if a reference is held on
|
||||
a VerifiedHTTPSConnection by the callback function.
|
||||
"""
|
||||
if x509.has_expired():
|
||||
msg = "SSL Certificate expired on '%s'" % x509.get_notAfter()
|
||||
raise exc.SSLCertificateError(msg)
|
||||
|
||||
if depth == 0 and preverify_ok:
|
||||
# We verify that the host matches against the last
|
||||
# certificate in the chain
|
||||
return host_matches_cert(host, x509)
|
||||
else:
|
||||
# Pass through OpenSSL's default result
|
||||
return preverify_ok
|
||||
|
||||
|
||||
def host_matches_cert(host, x509):
|
||||
"""
|
||||
Verify that the x509 certificate we have received
|
||||
from 'host' correctly identifies the server we are
|
||||
connecting to, ie that the certificate's Common Name
|
||||
or a Subject Alternative Name matches 'host'.
|
||||
"""
|
||||
def check_match(name):
|
||||
# Directly match the name
|
||||
if name == host:
|
||||
return True
|
||||
|
||||
# Support single wildcard matching
|
||||
if name.startswith('*.') and host.find('.') > 0:
|
||||
if name[2:] == host.split('.', 1)[1]:
|
||||
return True
|
||||
|
||||
common_name = x509.get_subject().commonName
|
||||
|
||||
# First see if we can match the CN
|
||||
if check_match(common_name):
|
||||
return True
|
||||
# Also try Subject Alternative Names for a match
|
||||
san_list = None
|
||||
for i in range(x509.get_extension_count()):
|
||||
ext = x509.get_extension(i)
|
||||
if ext.get_short_name() == b'subjectAltName':
|
||||
san_list = str(ext)
|
||||
for san in ''.join(san_list.split()).split(','):
|
||||
if san.startswith('DNS:'):
|
||||
if check_match(san.split(':', 1)[1]):
|
||||
return True
|
||||
|
||||
# Server certificate does not match host
|
||||
msg = ('Host "%s" does not match x509 certificate contents: '
|
||||
'CommonName "%s"' % (host, common_name))
|
||||
if san_list is not None:
|
||||
msg = msg + ', subjectAltName "%s"' % san_list
|
||||
raise exc.SSLCertificateError(msg)
|
||||
|
||||
|
||||
def to_bytes(s):
|
||||
if isinstance(s, six.string_types):
|
||||
return six.b(s)
|
||||
|
@ -171,61 +247,6 @@ class VerifiedHTTPSConnection(HTTPSConnection):
|
|||
except excp_lst as e:
|
||||
raise exc.SSLConfigurationError(str(e))
|
||||
|
||||
@staticmethod
|
||||
def host_matches_cert(host, x509):
|
||||
"""
|
||||
Verify that the x509 certificate we have received
|
||||
from 'host' correctly identifies the server we are
|
||||
connecting to, ie that the certificate's Common Name
|
||||
or a Subject Alternative Name matches 'host'.
|
||||
"""
|
||||
def check_match(name):
|
||||
# Directly match the name
|
||||
if name == host:
|
||||
return True
|
||||
|
||||
# Support single wildcard matching
|
||||
if name.startswith('*.') and host.find('.') > 0:
|
||||
if name[2:] == host.split('.', 1)[1]:
|
||||
return True
|
||||
|
||||
common_name = x509.get_subject().commonName
|
||||
|
||||
# First see if we can match the CN
|
||||
if check_match(common_name):
|
||||
return True
|
||||
# Also try Subject Alternative Names for a match
|
||||
san_list = None
|
||||
for i in range(x509.get_extension_count()):
|
||||
ext = x509.get_extension(i)
|
||||
if ext.get_short_name() == b'subjectAltName':
|
||||
san_list = str(ext)
|
||||
for san in ''.join(san_list.split()).split(','):
|
||||
if san.startswith('DNS:'):
|
||||
if check_match(san.split(':', 1)[1]):
|
||||
return True
|
||||
|
||||
# Server certificate does not match host
|
||||
msg = ('Host "%s" does not match x509 certificate contents: '
|
||||
'CommonName "%s"' % (host, common_name))
|
||||
if san_list is not None:
|
||||
msg = msg + ', subjectAltName "%s"' % san_list
|
||||
raise exc.SSLCertificateError(msg)
|
||||
|
||||
def verify_callback(self, connection, x509, errnum,
|
||||
depth, preverify_ok):
|
||||
if x509.has_expired():
|
||||
msg = "SSL Certificate expired on '%s'" % x509.get_notAfter()
|
||||
raise exc.SSLCertificateError(msg)
|
||||
|
||||
if depth == 0 and preverify_ok:
|
||||
# We verify that the host matches against the last
|
||||
# certificate in the chain
|
||||
return self.host_matches_cert(self.host, x509)
|
||||
else:
|
||||
# Pass through OpenSSL's default result
|
||||
return preverify_ok
|
||||
|
||||
def set_context(self):
|
||||
"""
|
||||
Set up the OpenSSL context.
|
||||
|
@ -237,7 +258,7 @@ class VerifiedHTTPSConnection(HTTPSConnection):
|
|||
|
||||
if self.insecure is not True:
|
||||
self.context.set_verify(OpenSSL.SSL.VERIFY_PEER,
|
||||
self.verify_callback)
|
||||
verify_callback(host=self.host))
|
||||
else:
|
||||
self.context.set_verify(OpenSSL.SSL.VERIFY_NONE,
|
||||
lambda *args: True)
|
||||
|
|
|
@ -156,7 +156,7 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
self.assertEqual('0.0.0.0', cert.get_subject().commonName)
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('0.0.0.0', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
|
@ -171,7 +171,7 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
self.assertEqual('*.pong.example.com', cert.get_subject().commonName)
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('ping.pong.example.com', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
|
@ -186,13 +186,13 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
self.assertEqual('0.0.0.0', cert.get_subject().commonName)
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('alt1.example.com', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('alt2.example.com', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
|
@ -207,19 +207,19 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
self.assertEqual('0.0.0.0', cert.get_subject().commonName)
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('alt1.example.com', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('alt2.example.com', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
except Exception:
|
||||
self.fail('Unexpected exception.')
|
||||
|
||||
try:
|
||||
conn = https.VerifiedHTTPSConnection('alt3.example.net', 0)
|
||||
conn.verify_callback(None, cert, 0, 0, 1)
|
||||
https.do_verify_callback(None, cert, 0, 0, 1, host=conn.host)
|
||||
self.fail('Failed to raise assertion.')
|
||||
except exc.SSLCertificateError:
|
||||
pass
|
||||
|
@ -239,7 +239,8 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
self.fail('Failed to init VerifiedHTTPSConnection.')
|
||||
|
||||
self.assertRaises(exc.SSLCertificateError,
|
||||
conn.verify_callback, None, cert, 0, 0, 1)
|
||||
https.do_verify_callback, None, cert, 0, 0, 1,
|
||||
host=conn.host)
|
||||
|
||||
def test_ssl_expired_cert(self):
|
||||
"""
|
||||
|
@ -254,9 +255,11 @@ class TestVerifiedHTTPSConnection(testtools.TestCase):
|
|||
try:
|
||||
conn = https.VerifiedHTTPSConnection('openstack.example.com', 0)
|
||||
except Exception:
|
||||
raise
|
||||
self.fail('Failed to init VerifiedHTTPSConnection.')
|
||||
self.assertRaises(exc.SSLCertificateError,
|
||||
conn.verify_callback, None, cert, 0, 0, 1)
|
||||
https.do_verify_callback, None, cert, 0, 0, 1,
|
||||
host=conn.host)
|
||||
|
||||
def test_ssl_broken_key_file(self):
|
||||
"""
|
||||
|
|
Loading…
Reference in New Issue