From 87337ff255b581a57809e7ea9d901c00b3514d45 Mon Sep 17 00:00:00 2001 From: Yves-Gwenael Bourhis Date: Thu, 2 Mar 2017 16:13:18 +0100 Subject: [PATCH] Allow any port or protocol in security group rules Neutron allows setting port or protocol wildcard by not specifying any value for them. Example, these are allowed by neutron: neutron security-group-rule-create --direction egress neutron security-group-rule-create --direction egress --protocol tcp Specifying '-1' for IP protocol means a wildcard IP protocol. validate_ip_protocol is updated accordingly. 'All ports' choice is added to 'Open Port' field. Change-Id: I4a7262eda89e3206c743fee14c78aa6b49308ce6 Closes-Bug: 1669467 --- horizon/test/tests/utils.py | 4 +- horizon/utils/validators.py | 2 +- .../project/security_groups/forms.py | 25 ++++- .../project/security_groups/tests.py | 91 +++++++++++++++++++ ...col-and-port-support-7dd6f5acfaba55ba.yaml | 9 ++ 5 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/security-group-rule-wildcard-protocol-and-port-support-7dd6f5acfaba55ba.yaml diff --git a/horizon/test/tests/utils.py b/horizon/test/tests/utils.py index 2372727f27..436682c75b 100644 --- a/horizon/test/tests/utils.py +++ b/horizon/test/tests/utils.py @@ -255,8 +255,8 @@ class ValidatorsTests(test.TestCase): icmp_code) def test_ip_proto_validator(self): - VALID_PROTO = (0, 255) - INVALID_PROTO = (-1, 256) + VALID_PROTO = (0, 255, -1) + INVALID_PROTO = (-2, 256) for proto in VALID_PROTO: self.assertIsNone(validators.validate_ip_protocol(proto)) diff --git a/horizon/utils/validators.py b/horizon/utils/validators.py index ef526ffac7..a72de0ab94 100644 --- a/horizon/utils/validators.py +++ b/horizon/utils/validators.py @@ -43,7 +43,7 @@ def validate_icmp_code_range(icmp_code): def validate_ip_protocol(ip_proto): - if ip_proto not in range(0, 256): + if ip_proto < -1 or ip_proto > 255: raise ValidationError(_("Not a valid IP protocol number")) diff --git a/openstack_dashboard/dashboards/project/security_groups/forms.py b/openstack_dashboard/dashboards/project/security_groups/forms.py index 45ee9f5853..440af34e1e 100644 --- a/openstack_dashboard/dashboards/project/security_groups/forms.py +++ b/openstack_dashboard/dashboards/project/security_groups/forms.py @@ -261,9 +261,11 @@ class AddRule(forms.SelfHandlingForm): backend = api.network.security_group_backend(self.request) rules_dict = getattr(settings, 'SECURITY_GROUP_RULES', []) - common_rules = [(k, rules_dict[k]['name']) - for k in rules_dict - if rules_dict[k].get('backend', backend) == backend] + common_rules = [ + (k, rules_dict[k]['name']) + for k in rules_dict + if rules_dict[k].get('backend', backend) == backend + ] common_rules.sort() custom_rules = [('tcp', _('Custom TCP Rule')), ('udp', _('Custom UDP Rule')), @@ -276,6 +278,10 @@ class AddRule(forms.SelfHandlingForm): if backend == 'neutron': self.fields['direction'].choices = [('ingress', _('Ingress')), ('egress', _('Egress'))] + self.fields['ip_protocol'].help_text = _( + "Enter an integer value between -1 and 255 " + "(-1 means wild card)." + ) else: # direction and ethertype are not supported in Nova secgroup. self.fields['direction'].widget = forms.HiddenInput() @@ -284,6 +290,13 @@ class AddRule(forms.SelfHandlingForm): # and it is available only for neutron security group. self.fields['ip_protocol'].widget = forms.HiddenInput() + if backend == 'neutron': + self.fields['port_or_range'].choices = [ + ('port', _('Port')), + ('range', _('Port Range')), + ('all', _('All ports')), + ] + if not getattr(settings, 'OPENSTACK_NEUTRON_NETWORK', {}).get('enable_ipv6', True): self.fields['cidr'].version = forms.IPv4 @@ -323,7 +336,11 @@ class AddRule(forms.SelfHandlingForm): self._update_and_pop_error(cleaned_data, 'ip_protocol', rule_menu) self._update_and_pop_error(cleaned_data, 'icmp_code', None) self._update_and_pop_error(cleaned_data, 'icmp_type', None) - if port_or_range == "port": + if port_or_range == 'all': + self._update_and_pop_error(cleaned_data, 'port', None) + self._update_and_pop_error(cleaned_data, 'from_port', None) + self._update_and_pop_error(cleaned_data, 'to_port', None) + elif port_or_range == "port": self._update_and_pop_error(cleaned_data, 'from_port', port) self._update_and_pop_error(cleaned_data, 'to_port', port) if port is None: diff --git a/openstack_dashboard/dashboards/project/security_groups/tests.py b/openstack_dashboard/dashboards/project/security_groups/tests.py index 88593c175d..156eb8310d 100644 --- a/openstack_dashboard/dashboards/project/security_groups/tests.py +++ b/openstack_dashboard/dashboards/project/security_groups/tests.py @@ -1029,3 +1029,94 @@ class SecurityGroupsNeutronTests(SecurityGroupsViewTests): res = self.client.post(self.edit_url, formData) self.assertFormError(res, 'form', 'cidr', 'Invalid version for IP address') + + @test.create_stubs({api.network: ('security_group_list', + 'security_group_backend')}) + def test_detail_add_rule_invalid_port(self): + sec_group = self.security_groups.first() + sec_group_list = self.security_groups.list() + rule = self.security_group_rules.first() + + api.network.security_group_backend( + IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) + api.network.security_group_list( + IsA(http.HttpRequest)).AndReturn(sec_group_list) + if django.VERSION >= (1, 9): + api.network.security_group_backend( + IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) + api.network.security_group_list( + IsA(http.HttpRequest)).AndReturn(sec_group_list) + + self.mox.ReplayAll() + + formData = {'method': 'AddRule', + 'id': sec_group.id, + 'port_or_range': 'port', + 'port': -1, + 'rule_menu': rule.ip_protocol, + 'cidr': rule.ip_range['cidr'], + 'remote': 'cidr'} + res = self.client.post(self.edit_url, formData) + self.assertNoMessages() + self.assertContains(res, "Not a valid port number") + + @test.create_stubs({api.network: ('security_group_rule_create', + 'security_group_list', + 'security_group_backend',)}) + def test_detail_add_rule_ingress_tcp_without_port(self): + sec_group = self.security_groups.first() + sec_group_list = self.security_groups.list() + rule = self.security_group_rules.list()[3] + + api.network.security_group_backend( + IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) + api.network.security_group_rule_create(IsA(http.HttpRequest), + sec_group.id, 'ingress', 'IPv4', + 'tcp', + None, + None, + rule.ip_range['cidr'], + None).AndReturn(rule) + api.network.security_group_list( + IsA(http.HttpRequest)).AndReturn(sec_group_list) + self.mox.ReplayAll() + + formData = {'id': sec_group.id, + 'direction': 'ingress', + 'port_or_range': 'all', + 'rule_menu': 'tcp', + 'cidr': rule.ip_range['cidr'], + 'remote': 'cidr'} + res = self.client.post(self.edit_url, formData) + self.assertRedirectsNoFollow(res, self.detail_url) + + @test.create_stubs({api.network: ('security_group_rule_create', + 'security_group_list', + 'security_group_backend',)}) + def test_detail_add_rule_custom_without_protocol(self): + sec_group = self.security_groups.first() + sec_group_list = self.security_groups.list() + rule = self.security_group_rules.list()[3] + + api.network.security_group_backend( + IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) + api.network.security_group_rule_create(IsA(http.HttpRequest), + sec_group.id, 'ingress', 'IPv4', + None, + None, + None, + rule.ip_range['cidr'], + None).AndReturn(rule) + api.network.security_group_list( + IsA(http.HttpRequest)).AndReturn(sec_group_list) + self.mox.ReplayAll() + + formData = {'id': sec_group.id, + 'direction': 'ingress', + 'port_or_range': 'port', + 'rule_menu': 'custom', + 'ip_protocol': -1, + 'cidr': rule.ip_range['cidr'], + 'remote': 'cidr'} + res = self.client.post(self.edit_url, formData) + self.assertRedirectsNoFollow(res, self.detail_url) diff --git a/releasenotes/notes/security-group-rule-wildcard-protocol-and-port-support-7dd6f5acfaba55ba.yaml b/releasenotes/notes/security-group-rule-wildcard-protocol-and-port-support-7dd6f5acfaba55ba.yaml new file mode 100644 index 0000000000..222b06f501 --- /dev/null +++ b/releasenotes/notes/security-group-rule-wildcard-protocol-and-port-support-7dd6f5acfaba55ba.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Securtiy group "Add rule" form now allows to specify 'any' IP protocol + and 'any' port number (for TCP and UDP protocols). This feature is + available when neutron is used as a networking back-end. + You can specify 'any' IP protocol for 'Other Protocol' and ``-1`` means + 'any' IP protocol. You can also see ``All ports`` choice in 'Open Port' + field in case of TCP or UDP protocol is selected.