openstack/neutron-lbaas
Code Issues Proposed changes

Replace HTTPSConnection in NEC plugin

Browse Source 
Replace HTTPSConnection in NEC plugin PFC driver with Requests.

SSL Verification is from now on enabled by default.

This changes the default behaviour and is the primary intention of this
change: verify SSL certificates.

This might break existing configuration/setups where the SSL certificate
used by the NEC PFC driver would not pass the verification.

SecurityImpact
DocImpact
Partial-Bug: 1188189

Change-Id: I1e5fdc9c2ed5b812aa6509d1639bd499acc5c337
This commit is contained in:
Daniel Gollub 2014-03-02 09:33:38 +01:00 committed by Akihiro Motoki
parent 9e7bfac63b
commit 264b4a2523
6 changed files with 82 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
etc/neutron/plugins/nec/nec.ini
View File

@ -45,6 +45,9 @@ firewall_driver = neutron.agent.linux.iptables_firewall.OVSHybridIptablesFirewal
# Certificate file
# cert_file =
# Disable SSL certificate verification
# insecure_ssl = false
# Maximum attempts per OFC API request. NEC plugin retries
# API request to OFC when OFC returns ServiceUnavailable (503).
# The value must be greater than 0.

2
neutron/plugins/nec/common/config.py
View File

@ -51,6 +51,8 @@ ofc_opts = [
help=_("Key file")),
cfg.StrOpt('cert_file', default=None,
help=_("Certificate file")),
cfg.BoolOpt('insecure_ssl', default=False,
help=_("Disable SSL certificate verification")),
cfg.IntOpt('api_max_attempts', default=3,
help=_("Maximum attempts per OFC API request."
"NEC plugin retries API request to OFC "

72
neutron/plugins/nec/common/ofc_client.py
View File

@ -15,11 +15,11 @@
# under the License.
# @author: Ryota MIBU
import httplib
import json
import socket
import time
import requests
from neutron.openstack.common import excutils
from neutron.openstack.common import log as logging
from neutron.plugins.nec.common import config
@ -33,7 +33,7 @@ class OFCClient(object):
"""A HTTP/HTTPS client for OFC Drivers."""
def __init__(self, host="127.0.0.1", port=8888, use_ssl=False,
key_file=None, cert_file=None):
key_file=None, cert_file=None, insecure_ssl=False):
"""Creates a new client to some OFC.
:param host: The host where service resides
@ -41,35 +41,40 @@ class OFCClient(object):
:param use_ssl: True to use SSL, False to use HTTP
:param key_file: The SSL key file to use if use_ssl is true
:param cert_file: The SSL cert file to use if use_ssl is true
:param insecure_ssl: Don't verify SSL certificate
"""
self.host = host
self.port = port
self.use_ssl = use_ssl
self.key_file = key_file
self.cert_file = cert_file
self.insecure_ssl = insecure_ssl
self.connection = None
def get_connection(self):
"""Returns the proper connection."""
if self.use_ssl:
connection_type = httplib.HTTPSConnection
else:
connection_type = httplib.HTTPConnection
# Open connection and send request, handling SSL certs
certs = {'key_file': self.key_file, 'cert_file': self.cert_file}
certs = dict((x, certs[x]) for x in certs if certs[x] is not None)
if self.use_ssl and len(certs):
conn = connection_type(self.host, self.port, **certs)
else:
conn = connection_type(self.host, self.port)
return conn
def _format_error_message(self, status, detail):
detail = ' ' + detail if detail else ''
return (_("Operation on OFC failed: %(status)s%(msg)s") %
{'status': status, 'msg': detail})
def _get_response(self, method, action, body=None):
headers = {"Content-Type": "application/json"}
protocol = "http"
certs = {'key_file': self.key_file, 'cert_file': self.cert_file}
certs = dict((x, certs[x]) for x in certs if certs[x] is not None)
verify = True
if self.use_ssl:
protocol = "https"
if self.insecure_ssl:
verify = False
url = "%s://%s:%d%s" % (protocol, self.host, int(self.port),
action)
res = requests.request(method, url, data=body, headers=headers,
cert=certs, verify=verify)
return res
def do_single_request(self, method, action, body=None):
action = config.OFC.path_prefix + action
LOG.debug(_("Client request: %(host)s:%(port)s "
@ -79,13 +84,10 @@ class OFCClient(object):
if type(body) is dict:
body = json.dumps(body)
try:
conn = self.get_connection()
headers = {"Content-Type": "application/json"}
conn.request(method, action, body, headers)
res = conn.getresponse()
data = res.read()
res = self._get_response(method, action, body)
data = res.text
LOG.debug(_("OFC returns [%(status)s:%(data)s]"),
{'status': res.status,
{'status': res.status_code,
'data': data})
# Try to decode JSON data if possible.
@ -94,33 +96,33 @@ class OFCClient(object):
except (ValueError, TypeError):
pass
if res.status in (httplib.OK,
httplib.CREATED,
httplib.ACCEPTED,
httplib.NO_CONTENT):
if res.status_code in (requests.codes.OK,
requests.codes.CREATED,
requests.codes.ACCEPTED,
requests.codes.NO_CONTENT):
return data
elif res.status == httplib.SERVICE_UNAVAILABLE:
retry_after = res.getheader('retry-after')
elif res.status_code == requests.codes.SERVICE_UNAVAILABLE:
retry_after = res.headers.get('retry-after')
LOG.warning(_("OFC returns ServiceUnavailable "
"(retry-after=%s)"), retry_after)
raise nexc.OFCServiceUnavailable(retry_after=retry_after)
elif res.status == httplib.NOT_FOUND:
elif res.status_code == requests.codes.NOT_FOUND:
LOG.info(_("Specified resource %s does not exist on OFC "),
action)
raise nexc.OFCResourceNotFound(resource=action)
else:
LOG.warning(_("Operation on OFC failed: "
"status=%(status)s, detail=%(detail)s"),
{'status': res.status, 'detail': data})
{'status': res.status_code, 'detail': data})
params = {'reason': _("Operation on OFC failed"),
'status': res.status}
'status': res.status_code}
if isinstance(data, dict):
params['err_code'] = data.get('err_code')
params['err_msg'] = data.get('err_msg')
else:
params['err_msg'] = data
raise nexc.OFCException(**params)
except (socket.error, IOError) as e:
except requests.exceptions.RequestException as e:
reason = _("Failed to connect OFC : %s") % e
LOG.error(reason)
raise nexc.OFCException(reason=reason)

3
neutron/plugins/nec/drivers/pfc.py
View File

@ -57,7 +57,8 @@ class PFCDriverBase(ofc_driver_base.OFCDriverBase):
port=conf_ofc.port,
use_ssl=conf_ofc.use_ssl,
key_file=conf_ofc.key_file,
cert_file=conf_ofc.cert_file)
cert_file=conf_ofc.cert_file,
insecure_ssl=conf_ofc.insecure_ssl)
@classmethod
def filter_supported(cls):

80
neutron/tests/unit/nec/test_ofc_client.py
View File

@ -17,10 +17,10 @@
# @author: Akihiro Motoki
import json
import socket
import mock
from oslo.config import cfg
import requests
from neutron.plugins.nec.common import config
from neutron.plugins.nec.common import exceptions as nexc
@ -28,19 +28,25 @@ from neutron.plugins.nec.common import ofc_client
from neutron.tests import base
class FakeResponse(requests.Response):
def __init__(self, status_code=None, text=None, headers=None):
self._text = text
self.status_code = status_code
if headers is not None:
self.headers = headers
@property
def text(self):
return self._text
class OFCClientTest(base.BaseTestCase):
def _test_do_request(self, status, resbody, expected_data, exctype=None,
exc_checks=None, path_prefix=None):
res = mock.Mock()
res.status = status
res.read.return_value = resbody
req = mock.Mock(return_value=(FakeResponse(status, resbody)))
conn = mock.Mock()
conn.getresponse.return_value = res
with mock.patch.object(ofc_client.OFCClient, 'get_connection',
return_value=conn):
with mock.patch.object(requests, 'request', req):
client = ofc_client.OFCClient()
path = '/somewhere'
realpath = path_prefix + path if path_prefix else path
@ -56,11 +62,9 @@ class OFCClientTest(base.BaseTestCase):
self.assertEqual(response, expected_data)
headers = {"Content-Type": "application/json"}
expected = [
mock.call.request('GET', realpath, '{}', headers),
mock.call.getresponse(),
]
conn.assert_has_calls(expected)
req.assert_called_with('GET', 'http://127.0.0.1:8888' + realpath,
verify=True, cert={}, data='{}',
headers=headers)
def test_do_request_200_json_value(self):
self._test_do_request(200, json.dumps([1, 2, 3]), [1, 2, 3])
@ -108,13 +112,12 @@ class OFCClientTest(base.BaseTestCase):
exc_checks)
def test_do_request_socket_error(self):
conn = mock.Mock()
conn.request.side_effect = socket.error
data = _("An OFC exception has occurred: Failed to connect OFC : ")
with mock.patch.object(ofc_client.OFCClient, 'get_connection',
return_value=conn):
req = mock.Mock()
req.side_effect = requests.exceptions.RequestException
with mock.patch.object(requests, 'request', req):
client = ofc_client.OFCClient()
e = self.assertRaises(nexc.OFCException, client.do_request,
@ -124,10 +127,9 @@ class OFCClientTest(base.BaseTestCase):
self.assertIsNone(getattr(e, k))
headers = {"Content-Type": "application/json"}
expected = [
mock.call.request('GET', '/somewhere', '{}', headers),
]
conn.assert_has_calls(expected)
req.assert_called_with('GET', 'http://127.0.0.1:8888/somewhere',
verify=True, cert={}, data='{}',
headers=headers)
def test_do_request_retry_fail_after_one_attempts(self):
self._test_do_request_retry_after(1, api_max_attempts=1)
@ -148,24 +150,17 @@ class OFCClientTest(base.BaseTestCase):
cfg.CONF.set_override('api_max_attempts', api_max_attempts,
group='OFC')
res_unavail = mock.Mock()
res_unavail.status = 503
res_unavail.read.return_value = None
res_unavail.getheader.return_value = '10'
res_unavail = FakeResponse(503, headers={'retry-after': '10'})
res_ok = FakeResponse(200)
res_ok = mock.Mock()
res_ok.status = 200
res_ok.read.return_value = None
conn = mock.Mock()
req = mock.Mock()
if succeed_final:
side_effect = [res_unavail] * (exp_request_count - 1) + [res_ok]
req.side_effect = ([res_unavail] * (exp_request_count - 1)
+ [res_ok])
else:
side_effect = [res_unavail] * exp_request_count
conn.getresponse.side_effect = side_effect
req.side_effect = [res_unavail] * exp_request_count
with mock.patch.object(ofc_client.OFCClient, 'get_connection',
return_value=conn):
with mock.patch.object(requests, 'request', req):
with mock.patch('time.sleep') as sleep:
client = ofc_client.OFCClient()
if succeed_final:
@ -176,11 +171,10 @@ class OFCClientTest(base.BaseTestCase):
client.do_request,
'GET', '/somewhere')
self.assertEqual('10', e.retry_after)
headers = {"Content-Type": "application/json"}
expected = [
mock.call.request('GET', '/somewhere', None, headers),
mock.call.getresponse(),
] * exp_request_count
conn.assert_has_calls(expected)
self.assertEqual(exp_request_count, conn.request.call_count)
req.assert_called_with('GET', 'http://127.0.0.1:8888/somewhere',
verify=True, cert={}, data=None,
headers=headers)
self.assertEqual(exp_request_count, req.call_count)
self.assertEqual(exp_request_count - 1, sleep.call_count)

1
neutron/tests/unit/nec/test_pfc_driver.py
View File

@ -39,6 +39,7 @@ class TestConfig(object):
use_ssl = False
key_file = None
cert_file = None
insecure_ssl = False
def _ofc(id):