Don't allow user to set firewall rule with port and no protocol
Creating firewall rules specifying a destination port and/or a source
port without a protocol, generates rules without src or dest port
restriction. This was a real security issue for cloud users.
This patch generates a 400 Bad request "Source/destination port
requires a protocol" in case of creation/update of firewall rules
specifying a destination port and/or a source port and without protocol.
DocImpact
Closes-Bug: #1365961
Change-Id: I4a3a1d9ae7ec4b2a864b3edc83d65ef7f80cbba5
(cherry picked from commit 6f3ae396ed
)
This commit is contained in:
parent
5dde74dd27
commit
50e15341fe
|
@ -342,6 +342,9 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
|
|||
LOG.debug(_("create_firewall_rule() called"))
|
||||
fwr = firewall_rule['firewall_rule']
|
||||
tenant_id = self._get_tenant_id_for_create(context, fwr)
|
||||
if not fwr['protocol'] and (fwr['source_port'] or
|
||||
fwr['destination_port']):
|
||||
raise firewall.FirewallRuleWithPortWithoutProtocolInvalid()
|
||||
src_port_min, src_port_max = self._get_min_max_ports_from_range(
|
||||
fwr['source_port'])
|
||||
dst_port_min, dst_port_max = self._get_min_max_ports_from_range(
|
||||
|
@ -383,6 +386,14 @@ class Firewall_db_mixin(firewall.FirewallPluginBase, base_db.CommonDbMixin):
|
|||
del fwr['destination_port']
|
||||
with context.session.begin(subtransactions=True):
|
||||
fwr_db = self._get_firewall_rule(context, id)
|
||||
protocol = fwr.get('protocol', fwr_db['protocol'])
|
||||
if not protocol:
|
||||
sport = fwr.get('source_port_range_min',
|
||||
fwr_db['source_port_range_min'])
|
||||
dport = fwr.get('destination_port_range_min',
|
||||
fwr_db['destination_port_range_min'])
|
||||
if sport or dport:
|
||||
raise firewall.FirewallRuleWithPortWithoutProtocolInvalid()
|
||||
fwr_db.update(fwr)
|
||||
if fwr_db.firewall_policy_id:
|
||||
fwp_db = self._get_firewall_policy(context,
|
||||
|
|
|
@ -80,6 +80,10 @@ class FirewallRuleInvalidAction(qexception.InvalidInput):
|
|||
"Only action values %(values)s are supported.")
|
||||
|
||||
|
||||
class FirewallRuleWithPortWithoutProtocolInvalid(qexception.InvalidInput):
|
||||
message = _("Source/destination port requires a protocol")
|
||||
|
||||
|
||||
class FirewallInvalidPortValue(qexception.InvalidInput):
|
||||
message = _("Invalid value for port %(port)s.")
|
||||
|
||||
|
|
|
@ -580,6 +580,20 @@ class TestFirewallDBPlugin(FirewallPluginDbTestCase):
|
|||
for k, v in attrs.iteritems():
|
||||
self.assertEqual(firewall_rule['firewall_rule'][k], v)
|
||||
|
||||
def test_create_firewall_rule_without_protocol_with_dport(self):
|
||||
attrs = self._get_test_firewall_rule_attrs()
|
||||
attrs['protocol'] = None
|
||||
attrs['source_port'] = None
|
||||
res = self._create_firewall_rule(self.fmt, **attrs)
|
||||
self.assertEqual(400, res.status_int)
|
||||
|
||||
def test_create_firewall_rule_without_protocol_with_sport(self):
|
||||
attrs = self._get_test_firewall_rule_attrs()
|
||||
attrs['protocol'] = None
|
||||
attrs['destination_port'] = None
|
||||
res = self._create_firewall_rule(self.fmt, **attrs)
|
||||
self.assertEqual(400, res.status_int)
|
||||
|
||||
def test_show_firewall_rule_with_fw_policy_not_associated(self):
|
||||
attrs = self._get_test_firewall_rule_attrs()
|
||||
with self.firewall_rule() as fw_rule:
|
||||
|
@ -675,6 +689,46 @@ class TestFirewallDBPlugin(FirewallPluginDbTestCase):
|
|||
for k, v in attrs.iteritems():
|
||||
self.assertEqual(res['firewall_rule'][k], v)
|
||||
|
||||
def test_update_firewall_rule_with_port_and_no_proto(self):
|
||||
with self.firewall_rule() as fwr:
|
||||
data = {'firewall_rule': {'protocol': None,
|
||||
'destination_port': 80}}
|
||||
req = self.new_update_request('firewall_rules', data,
|
||||
fwr['firewall_rule']['id'])
|
||||
res = req.get_response(self.ext_api)
|
||||
self.assertEqual(400, res.status_int)
|
||||
|
||||
def test_update_firewall_rule_without_ports_and_no_proto(self):
|
||||
with self.firewall_rule() as fwr:
|
||||
data = {'firewall_rule': {'protocol': None,
|
||||
'destination_port': None,
|
||||
'source_port': None}}
|
||||
req = self.new_update_request('firewall_rules', data,
|
||||
fwr['firewall_rule']['id'])
|
||||
res = req.get_response(self.ext_api)
|
||||
self.assertEqual(200, res.status_int)
|
||||
|
||||
def test_update_firewall_rule_with_port(self):
|
||||
with self.firewall_rule(source_port=None,
|
||||
destination_port=None,
|
||||
protocol=None) as fwr:
|
||||
data = {'firewall_rule': {'destination_port': 80}}
|
||||
req = self.new_update_request('firewall_rules', data,
|
||||
fwr['firewall_rule']['id'])
|
||||
res = req.get_response(self.ext_api)
|
||||
self.assertEqual(400, res.status_int)
|
||||
|
||||
def test_update_firewall_rule_with_port_and_protocol(self):
|
||||
with self.firewall_rule(source_port=None,
|
||||
destination_port=None,
|
||||
protocol=None) as fwr:
|
||||
data = {'firewall_rule': {'destination_port': 80,
|
||||
'protocol': 'tcp'}}
|
||||
req = self.new_update_request('firewall_rules', data,
|
||||
fwr['firewall_rule']['id'])
|
||||
res = req.get_response(self.ext_api)
|
||||
self.assertEqual(200, res.status_int)
|
||||
|
||||
def test_update_firewall_rule_with_policy_associated(self):
|
||||
name = "new_firewall_rule1"
|
||||
attrs = self._get_test_firewall_rule_attrs(name)
|
||||
|
|
Loading…
Reference in New Issue