no obvious way to delete incorrect security rules (added to the default nova security group)

Bug #1163469 reported by Kashyap Chamarthy on 2013-04-02
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Medium
Amandeep

Bug Description

Try to add an incorrect security rules, like
  1] adding an icmp rule without providing cidr to the default nova security group
    - also, ICMP doesn't run on ports (so providing arbitrary ports for ICMP should be disabled ?)
  2] add an ssh rule, without cidr

And there appears to be no way to delete these incorrect rules

A small test:

=======
(~(keystone_admin)$ nova secgroup-list
+---------+-------------+
| Name | Description |
+---------+-------------+
| default | default |
+---------+-------------+

(~(keystone_admin)$ nova secgroup-list-rules default

(~(keystone_admin)$

(~(keystone_user1)]$ nova secgroup-add-group-rule default default icmp -1 -1
+-------------+-----------+---------+----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------+--------------+
| icmp | -1 | -1 | | default |
+-------------+-----------+---------+----------+--------------+

(~(keystone_user1)]$ nova secgroup-add-group-rule default default icmp 22 22
+-------------+-----------+---------+----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------+--------------+
| icmp | 22 | 22 | | default |
+-------------+-----------+---------+----------+--------------+

(~(keystone_user1)]$ nova secgroup-list-rules default
+-------------+-----------+---------+----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------+--------------+
| icmp | -1 | -1 | | default |
| icmp | 22 | 22 | | default |
+-------------+-----------+---------+----------+--------------+
=======

-> Now attempt to delete:
=======
(~(keystone_user1)]$ nova secgroup-delete-rule default icmp 22 22
usage: nova secgroup-delete-rule <secgroup> <ip-proto> <from-port> <to-port>
                                 <cidr>
error: too few arguments
=======
(~(keystone_user1)]$ nova secgroup-delete-rule default icmp 22 22 0.0.0.0/0
ERROR: 'cidr'
=======

-> For reference, correct way to add rules:
=========================================
(~(keystone_user1)]$ nova secgroup-add-rule default tcp 22 22 0.0.0.0/0
+-------------+-----------+---------+-----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+-----------+--------------+
| tcp | 22 | 22 | 0.0.0.0/0 | |
+-------------+-----------+---------+-----------+--------------+
(~(keystone_user1)]$ nova secgroup-add-rule default icmp -1 -1 0.0.0.0/0
+-------------+-----------+---------+-----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+-----------+--------------+
| icmp | -1 | -1 | 0.0.0.0/0 | |
+-------------+-----------+---------+-----------+--------------+
(~(keystone_user1)]$

-> Now, I end up with an inconsistent set of rules:
=============
(~(keystone_user1)]$ nova secgroup-list-rules default
+-------------+-----------+---------+-----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+-----------+--------------+
| icmp | -1 | -1 | | default |
| icmp | -1 | -1 | 0.0.0.0/0 | |
| icmp | 22 | 22 | | default |
| tcp | 22 | 22 | 0.0.0.0/0 | |
+-------------+-----------+---------+-----------+--------------+
(~(keystone_user1)]$
=============

Actual results:
Incorrect/invalid rules can be created.

Expected results:
Incorrect/invalid rules should be sanitized. In case they're allowed, there should be a way to delete them.

Dan Smith (danms) wrote :

What version of nova is this? I get a different error when I try:

dan@guaranine:/opt/stack/tempest$ nova secgroup-delete-rule default icmp -1 -1 1.1.1.1/8
ERROR: 'NoneType' object has no attribute 'upper'

Changed in nova:
importance: Undecided → Medium
status: New → Confirmed
git-harry (git-harry) wrote :

According to the following link there are two types of rules - one type specify an IP range the other a source group.

http://docs.openstack.org/developer/nova/nova.concepts.html#concept-security-groups

You are creating rules with a source group, they can be deleted using nova secgroup-delete-group-rule not nova secgroup-delete-rule

# nova secgroup-list-rules default
+-------------+-----------+---------+----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------+--------------+
| icmp | -1 | -1 | | default |
| icmp | 22 | 22 | | default |
+-------------+-----------+---------+----------+--------------+

# nova secgroup-delete-group-rule default default icmp 22 22

# nova secgroup-list-rules default
+-------------+-----------+---------+----------+--------------+
| IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------+--------------+
| icmp | -1 | -1 | | default |
+-------------+-----------+---------+----------+--------------+

When specifying ICMP the two arguments are not ports but ICMP code and type - http://docs.openstack.org/cli/quick-start/content/nova_client.html#nova_cli_security_groups

Brent Eagles (beagles) wrote :

Shouldn't this be closed? The usage described in the bug isn't valid.

Kashyap Chamarthy (kashyapc) wrote :

@Dan Smith: Sorry for such a late reply. I missed this. And I didn't mention the original version :( . It was with Folsom, I'm sure, but can't recall the exact package versions.

@Brent: Just curious, shouldn't adding dubious security rules be disallowed ? If you still think this usage invalid/trivial, it can be closed, if others agree.

I can test this w/ Grizzly and check if it still persists.

Thanks.

Changed in nova:
assignee: nobody → Amandeep (rattenpal-amandeep)
Changed in nova:
assignee: Amandeep (rattenpal-amandeep) → Kanchan Gupta (kanchan-gupta1)
Changed in nova:
assignee: Kanchan Gupta (kanchan-gupta1) → Amandeep (rattenpal-amandeep)
Changed in nova:
status: Confirmed → In Progress
summary: - no obvious way to delete incorrect security rules (added to the default
+ no obvious way to delete correct security rules (added to the default
nova security group)

In icmp protocol -1 port is user defined. Check conditions are given for tcp, udp, icmp protocol in nova.network.security_group.security_group_base.py where we can see that for icmp protocol -1 to 255 are valid port (user defined) . So accordingly in icmp protocol if we are using -1 port to add security group rule then it is a valid rule not a invalid rule and we are not able to add invalid rule, only valid rules are accepted to be add.
Moreover, we are unable to delete correct/valid rule also. So, i worked on this problem and now i am having a fix ready for this problem.

summary: - no obvious way to delete correct security rules (added to the default
+ no obvious way to delete incorrect security rules (added to the default
nova security group)
Amandeep (rattenpal-amandeep) wrote :

In nova/network/security_group/security_group_base.py the conditions are given below :

# Verify valid TCP, UDP port ranges
            if (ip_protocol.upper() in ['TCP', 'UDP'] and
                    (from_port < 1 or to_port > 65535)):
                raise exception.InvalidPortRange(from_port=from_port,
                      to_port=to_port, msg="Valid TCP ports should"
                                           " be between 1-65535")

# Verify ICMP type and code
            if (ip_protocol.upper() == "ICMP" and
                (from_port < -1 or from_port > 255 or
                 to_port < -1 or to_port > 255)):
                raise exception.InvalidPortRange(from_port=from_port,
                      to_port=to_port, msg="For ICMP, the"
                                           " type:code must be valid")

According to these condition -1 to 255 port are valid ports.
So these given rules are valid not invalid.

Moreover, In Icehouse/stable version :
nova secgroup-delete-group-rule default default icmp 22 22 also throwing AttributeError.
Which means only valid rules are throwing the error and invalid rules are not able to add.

Amandeep (rattenpal-amandeep) wrote :

Fix proposed to branch: master
Review: https://review.openstack.org/125015

Changed in nova:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2014-12-18
Changed in nova:
milestone: none → kilo-1
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers