openstack
/
python-glanceclient
Code Issues Proposed changes

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:
Stuart McLaren 2015-02-17 17:36:56 +00:00
parent a3eaafefbd
commit ef9fd9fca0
2 changed files with 89 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

133
glanceclient/common/https.py
View File

@ -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)

21
tests/test_ssl.py
View File

@ -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):
"""