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

Bug #1163469 reported by Kashyap Chamarthy
18
This bug affects 4 people
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Fix Released
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.

Revision history for this message
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
Revision history for this message
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

Revision history for this message
Brent Eagles (beagles) wrote :

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

Revision history for this message
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)
Revision history for this message
Amandeep (rattenpal-amandeep) wrote : Re: 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)
Revision history for this message
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.

Revision history for this message
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)
Changed in nova:
milestone: none → kilo-1
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in nova:
milestone: kilo-1 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.