dangerous secgroup rule created when the "SourceSecurityGroupName" property string doesn't refer to an existing security group

[Marc Heckmann]

Given the following resources in a template:

    "WikiDatabaseSecurityGroup" : {
      "Type" : "AWS::EC2::SecurityGroup",
      "Properties" : {
        "GroupDescription" : "Enable HTTP access via port 80 plus SSH access",
        "SecurityGroupIngress" : [
          {"IpProtocol" : "icmp", "FromPort" : "-1", "ToPort" : "-1", "CidrIp" : "10.1.1.0/24"},
          {"IpProtocol" : "tcp", "FromPort" : "80", "ToPort" : "80", "CidrIp" : "10.1.1.0/24"},
          {"IpProtocol" : "tcp", "FromPort" : "22", "ToPort" : "22", "CidrIp" : "10.1.1.0/24"},
          {"IpProtocol" : "tcp", "FromPort" : "3306", "ToPort" : "3306", "SourceSecurityGroupName" : "WebServerSecurityGroup"}
        ]
      }
    },

    "WebServerSecurityGroup" : {
      "Type" : "AWS::EC2::SecurityGroup",
      "Properties" : {
        "GroupDescription" : "Enable HTTP access via port 80 plus SSH access",
        "SecurityGroupIngress" : [
          {"IpProtocol" : "icmp", "FromPort" : "-1", "ToPort" : "-1", "CidrIp" : "10.1.1.0/24"},
          {"IpProtocol" : "tcp", "FromPort" : "80", "ToPort" : "80", "CidrIp" : "10.1.1.0/24"},
          {"IpProtocol" : "tcp", "FromPort" : "22", "ToPort" : "22", "CidrIp" : "10.1.1.0/24"}
        ]
      }
    },

If, the string WebServerSecurityGroup does not refer to an existing security group, the following rules will be created:

 IP Protocol | From Port | To Port | IP Range | Source Group |
+-------------+-----------+---------+----------------+--------------+
| icmp | -1 | -1 | 10.1.1.0/24 | |
| tcp | 80 | 80 | 10.1.1.0/24 | |
| tcp | 22 | 22 | 10.1.1.0/24 | |
| tcp | 3306 | 3306 | 0.0.0.0/0 | |
+-------------+-----------+---------+----------------+--------------+

Of course this is not the behaviour that the user expected and no warning is thrown.

The correct syntax in this case would be to use '"SourceSecurityGroupName" : { "Ref" : "WebServerSecurityGroup" }'

The fact that it creates a rule with source IP 0.0.0.0/0 unbeknownst to the user is dangerous.

If the string does not refer to the name of an existing security group than an error should be thrown.

[Steve Baker (steve-stevebaker)]

Can you see potential for this affecting OS::Neutron::SecurityGroup too?

[Marc Heckmann (marc-w-heckmann)]

I have not idea about OS::Neutron::SecurityGroup, because we're not using Neutron. We're still on nova-network for various technical reasons.

[Jeremy Stanley (fungi)]

A couple questions here from a VMT perspective (trying to decide whether this warrants an advisory and how we'd draft the impact description if so). First, is this generally exploitable and can a more succinct exploit case be described for clarity? Also, is this applicable to Havanna (2013.2) and earlier, or just code in the master branch?

[Thierry Carrez (ttx)]

I'm not sure i would count this an an exploitable vulnerability. If you make mistakes in your securitygroup configuration, that may open holes. I agree we should try to reduce the risk of making such mistakes, but that can be fixed as a public bug ?

Also, does this affect havana, or is this a new feature in icehouse ?

[Marc Heckmann (marc-w-heckmann)]

This does affect Havana. I originally discovered the bug on Havana.

The problem here, is that the user doesn't think that he/she has a made a mistake. See my example in the bug description.

For a mistake like that (non-existent secgroup) the user would expect an error to be thrown. The problem is that the stack is created successfully without error and most people won't bother to audit what has been created by the stack. Even worse is that everything works as expected (in the example above, the web server can connect to the DB) because all source IPs are allowed to connect! The latter makes it even less likely for the user to check the rules that have been created.

The problem is that 0.0.0.0/0 source rule is created implicitely. Of course, if someone made an explicit mistake of creating a rule that contained 0.0.0.0/0, that would be another story.

[Zane Bitter (zaneb)]

So, this is not a security vulnerability in Heat per se, but it could certainly be a contributing cause of a security vulnerability in users' stacks. I'm not sure what the process we should apply there is. Certainly keeping it secret isn't helping anyone, since the error, where it exists, is in the user's template and the user will need to find and fix it. Obviously we should improve Heat to automatically detect this class of error in the future.

I believe that the problem lies in this for-loop: https://github.com/openstack/heat/blob/stable/havana/heat/engine/resources/security_group.py#L143
The loop should have an 'else' clause that raises an exception, since that means a security group name was designated but not found.

Assuming that diagnosis is correct, the bug will affect Havana also (link above is to the Havana version). FWIW this was a new feature in Havana (introduced by https://review.openstack.org/#/c/41917/ on 2013/8/26), so Grizzly is unaffected.

Even when using AWS::EC2::SecurityGroup with Neutron, a different code path is used. That path does not appear to be affected. OS::Neutron::SecurityGroup does not, afaict, contain any equivalent feature, so it should be unaffected.

[Thierry Carrez (ttx)]

I would just open this bug publicly and consider it a strengthening feature in Icehouse (avoid a case where users can shoot themselves in the foot).

[Marc Heckmann (marc-w-heckmann)]

I agree about making this one public and will do so, but I think it is a vulnerability simply because a very dangerous SG group rule is created silently.

Think about it: A user who, for whatever reason, allows anonymous access to MySQL running on an instance or is using very weak passwords but he/she thinks this is OK (I think that it's never OK, but this is just a hypothetical example) because they've applied ACLs in the form of SGs. They think these ACLs are working because a) they did not get an error and b) their stack works as expected. This is example is even worse when using something like Memcache which has no authentication capability and can only rely on network ACLs for security.

I actually think that this one is worse than the Nova EC2 API SG RBAC problem that I reported.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/81559

[Thierry Carrez (ttx)]

It's still not aIt could make sense to publish a security note to warn OpenStack users of this potential pitfall.

[Thierry Carrez (ttx)]

damn LP. I meant:

IMHO it's still not a vulnerability in the software. It's a dangerous pitfall that users should know about (and the software should do a better job at warning users of).

We have a mechanism to warn users of that: OpenStack Security Notes. They follow a slightly different process.

[Jeremy Stanley (fungi)]

Agreed, this sounds like the sort of situation which warrants some sort of documented and published recommendations, but I don't think it's something we would issue a security advisory around.

[Thierry Carrez (ttx)]

Adding an ossn task, for the OSSG to see if they are interested in covering that case with an OSSN.

This is a potential user configuration pitfall that it might be worth to document better.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/81558
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=7246abab34b4b32f9675e081c74bfb628932c8b3
Submitter: Jenkins
Branch: master

commit 7246abab34b4b32f9675e081c74bfb628932c8b3
Author: Zane Bitter <email address hidden>
Date: Mon Mar 17 12:30:18 2014 -0400

    Fail if non-existent security group referenced

    When using AWS::EC2::SecurityGroup resources with nova-network (not
    Neutron), if a SourceSecurityGroupName was specified in a rule but the
    name did not correspond to an extant security group it would be silently
    ignored. This change ensures that any such error in the template will cause
    creation of the security group to fail.

    Closes-bug: #1291091

    Change-Id: I9344451c83f4184ef46d4e9906b94abda08836df

[Nathan Kinder (nkinder)]

I have an OSSN drafted and out for review for this issue:

    https://review.openstack.org/#/c/85464/

[Nathan Kinder (nkinder)]

Published OSSN-0011 for this issue on <email address hidden>, <email address hidden>, and the wiki:

    https://wiki.openstack.org/wiki/OSSN/OSSN-0011
    http://git.openstack.org/cgit/openstack/openstack-security-notes/commit/?id=f291579bfb133f9f3b61c1f35d72849754d3c9b9

[OpenStack Infra (hudson-openstack)]

Change abandoned by Alan Pevec (<email address hidden>) on branch: stable/havana
Review: https://review.openstack.org/81559
Reason: Final Havana release 2012.3.4 has been cut and stable/havana is going to be removed in a week.