Security Group filtering hides rules from user
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| OpenStack Security Advisory |
Undecided
|
Unassigned | ||
| neutron |
Undecided
|
Unassigned |
Bug Description
Manage Rules part of the GUI hides the rules currently visible in the Launch Instance modal window.
It allows a malicious admin to add backdoor access rules that might be later added to VMs without the knowledge of owner of those VMs.
When sending GET request as below, it responds only with the rules that are created by user and this happens when using Manage Rules part of the GUI: <WSGIRequest: GET '/project/
On the other hand when using GET request as below, it responds with all SG and it includes all rules, and there is no filtering and this is used in Launch Instance modal window: <WSGIRequest: GET '/api/network/
Here is example of rules display in Manage Rules part of GUI:
> /opt/stack/
-> return api.neutron.
(Pdb) l
45 @memoized.
46 def _get_data(self):
47 sg_id = filters.
48 try:
49 from remote_pdb import RemotePdb; RemotePdb(
50 -> return api.neutron.
51 except Exception:
52 redirect = reverse(
53 exceptions.
54 _('Unable to retrieve security group.'),
55 redirect=redirect)
(Pdb) p api.neutron.
<SecurityGroup: {'description': 'Default security group', 'tags': [], 'tenant_id': '4e6e476afd784a
(Pdb)
(Pdb) p self.request
<WSGIRequest: GET '/project/
As you might have noticed there are no ports access 44 and 22 (SSH)
And from the Launch Instance Modal Window, as well as CLI we can see that there are two more rules that are invisible for user, port 44 and 22 (SSH) as displayed below:
> /opt/stack/
-> return {'items': [sg.to_dict() for sg in security_groups]}
(Pdb) l
42 """
43
44 security_groups = api.neutron.
45 from remote_pdb import RemotePdb; RemotePdb(
46
47 -> return {'items': [sg.to_dict() for sg in security_groups]}
48
49
50 @urls.register
51 class FloatingIP(
52 """API for a single floating IP address."""
(Pdb) p security_groups
[<SecurityGroup: {'description': 'Default security group', 'tags': [], 'tenant_id': '4e6e476afd784a
(Pdb)
(Pdb) p request
<WSGIRequest: GET '/api/network/
Thank you,
Robin
Robin Cernin (rcernin) wrote : | #2 |
There is easy reproducer:
1. Source the admin credentials
2. Add a security group rule to someone else project 'default' group
3. Login into Horizon as the user who owns that modified security group
4. Navigate to Network -> Security Groups -> Manage Rules for 'default' group
5. The rules are not visible as the API doesn't return see Bug description
It allows a malicious admin to add backdoor access rules that might be later added to VMs without the knowledge of owner of those VMs.
Akihiro Motoki (amotoki) wrote : | #3 |
This is not an issue specific to horizon. This is the behavior of neutron. This is an operation example: http://
Question to the bug author: Is it specific to horizon?
Akihiro Motoki (amotoki) wrote : | #4 |
In OpenStack deployments, admin role has a lot of power and this is not specific to this case reported here.
I think this is the security model we adopt now.
On the other hand, neutron needs to allow operators to configure a policy for create_
Akihiro Motoki (amotoki) wrote : | #5 |
I added 'neutron' as affected projects based on the above analysis.
Robin Cernin (rcernin) wrote : | #6 |
From Security perspective we should be transparent. I don't have much voice here but I would vote for exposing all of the rules in the Manage Rules part of the GUI rather hiding some rules from the user.
Robin Cernin (rcernin) wrote : | #7 |
> Question to the bug author: Is it specific to horizon?
This is neutron related.
Akihiro Motoki (amotoki) wrote : | #8 |
Horizon itself applies no filtering on API responses from neutron, so there is nothing to do in the horizon side and this needs to be discussed as neutron API behavior. It is not specific to GUI. It is same for CLI and API. As horizon, this is related to horizon but there is no room that horizon can do. All need to be discussed as neutron perspective.
no longer affects: | horizon |
Jeremy Stanley (fungi) wrote : | #9 |
In determining whether we should lift the embargo on this discussion, it would be useful to know if there are already plenty of other ways for an admin to backdoor network access controls besides the one specifically raised in this report.
Brian Haley (brian-haley) wrote : | #10 |
I guess I would agree with what Robin said - we should be transparent and at least show all the rules in the security group, even if the user can't update/delete some of them.
Regarding other ways to backdoor into an instance, or at least possibly get network access - in a related way I suppose it would be similar if an admin created a security group with a rule and added it to the instance. I'm guessing that SG wouldn't be visible either, but it's rule would be applied.
Jeremy Stanley (fungi) wrote : | #11 |
In that case, if it's already possible for an admin to create security groups which apply to an instance but are invisible to non-admins, and that is seen as a feature, then the ability for an admin to create a rule a non-admin can't see in a security group they normally can see doesn't seem like an added risk.
Am I understanding your comment correctly, Brian?
Brian Haley (brian-haley) wrote : | #12 |
Jeremy - I wouldn't call it a feature, just a (possible) other way to have a security group rule apply to an instance without the owner's knowledge. I just wanted to raise it to whoever fixes the rules to make sure they take one step up to the group as well. I guess the assumption has always been you trust your admin to some point to not do such things.
Jeremy Stanley (fungi) wrote : | #13 |
And this also assumes the admin in question lacks SSH access to the hypervisor host, control of the software deployed, or any of a myriad of other ways to have a similar influence over network filtering policy. Unfortunately I don't know enough about how typical of a scenario that is to be able to gauge whether we can safely continue the discussion and any related patch development in public. Hopefully someone who is subscribed to this bug has more operational insight than I do and can make a guess.
Slawek Kaplonski (slaweq) wrote : | #14 |
I just created what Brian said. And for security groups attached for port it works differently. Even if group is owned by other tenant than project and I was able to see all sec groups used by port. See http://
So IMO we should fix it in neutron and display rules assigned to security group, no matter to which project it belongs.
Brian Haley (brian-haley) wrote : | #15 |
Slawek - thanks for testing that out, but I did notice something in the paste. Although the tenant could see the security group ID, it couldn't list the rules associated with it, so there is still the possibility of a hidden rule allowing port access.
Slawek Kaplonski (slaweq) wrote : | #16 |
@Brian: yes, that is correct. But still user can at least see that admin attached SOME additional security group(s) to the port. If admin will attach additional rules to user's SG than user will not be aware of it at all.
Robin Cernin (rcernin) wrote : | #17 |
@slaweq: There is a way that user can get aware of it, if you open modal window of "Launch Instance" The SGs/rules there are unfiltered.
Slawek Kaplonski (slaweq) wrote : | #18 |
@Robin: I didn't know that. Is Horizon getting them as an admin maybe in this modal window? Or how it is different than in other places?
I think I was checking it with CLI and I didn't saw such rules but I will have to confirm that once again.
Robin Cernin (rcernin) wrote : | #19 |
@Slaweq I checked and it seems different API call:
<WSGIRequest: GET '/project/
VS
<WSGIRequest: GET '/api/network/
The last one is used in modal window and it does not have any filtering. Shows all rules.
Jeremy Stanley (fungi) wrote : | #20 |
So admin-added rules appear in the the Launch Instance modal, just not the normal Manage Security Groups view? If so, then coupling this with the fact that it relies on a malicious service admin to actually pull off any attack of this sort, I think we should be able to continue the discussion in public. I also lean toward categorizing this as a security hardening opportunity rather than a vulnerability in need of an advisory as it's at best only partly capable of hiding any "backdoor" network access rules.
Slawek Kaplonski (slaweq) wrote : | #21 |
@Robin: thx for info.
I know that "GET /v2.0/security-
But I have no idea what is GET /api/network/
@Jeremy: based on what I wrote above, I'm not sure that it isn't security issue from Neutron point of view. I don't know the way how user would be able to see such "extra" rules created by admin for his SG.
But I also don't think that this should still be keeped as private bug. It may also be "vulnerable" if admin user want's it. And if someone is admin already than probably he can do much more than only add some "hidden" security group rule.
Robin Cernin (rcernin) wrote : | #22 |
@Slaweq:
>I don't know the way how user would be able to see such "extra" rules created by admin for his SG.
Try adding rule as admin to demo, then login as demo and go to Compute -> Launch Instance -> it opens a modal window, switch to Security Groups -> Expand default security group and you should see the rules there created by admin.
Slawek Kaplonski (slaweq) wrote : | #23 |
@Robin: Yes, I understand how it can be checked via horizon. I'm now wondering what API calls Horizon is doing underneath that it has different results. And I don't know what is exactly "GET /api/network/
Maybe Akihiro or Brina will know it?
Brian Haley (brian-haley) wrote : | #24 |
I don't know about the GET, but looking at the horizon code it just uses the neutronclient library to make a list_security_
Jeremy Stanley (fungi) wrote : | #25 |
Does anyone object to continuing this discussion in public? Based on recent comments it seems that while there is still some confusion over which API calls Horizon is relying on, there are ways for a normal user to find these rules in the UI. Also, I doubt "malicious admin" is something we can reasonably expect to protect users against in the first place.
+1 to switching this to public based on above comments.
Matthias Runge (mrunge) wrote : | #27 |
In my understanding, there should be nothing exposable by horizon, which was not already known to the user. Showing data which could be accessed by the user via other ways (and still using the same credentials) is not a security breach in my view.
Jeremy Stanley (fungi) wrote : | #28 |
Matthias: The concern raised by this report is actually the opposite... that it's possible for an admin to "hide" some nefarious security group rules from normal users (though it turns out that's only partly effective if the users know which modal to look in).
Slawek Kaplonski (slaweq) wrote : | #29 |
I also think that this can be switched to be public.
@Matthias, so do You think that user (owner of security group) shouldn't see any rule which belongs to this group but is created by different user (admin)?
IMO user should be able to see such rules but shouldn't be able to modify or delete them.
Jeremy Stanley (fungi) wrote : | #30 |
Seems there's support for switching this to public and no real disagreement with my suggestion a month ago to treat this as a security hardening opportunity rather than a vulnerability, so I'll triage it publicly as such now and we can continue to debate/fix without any further embargo.
Treating as a class D report per https:/
description: | updated |
information type: | Private Security → Public |
Changed in ossa: | |
status: | Incomplete → Won't Fix |
tags: | added: security |
Fix proposed to branch: master
Review: https:/
Changed in neutron: | |
assignee: | nobody → Slawek Kaplonski (slaweq) |
status: | New → In Progress |
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-tempest-plugin (master) | #32 |
Related fix proposed to branch: master
Review: https:/
Fix proposed to branch: stable/stein
Review: https:/
Fix proposed to branch: stable/rocky
Review: https:/
Fix proposed to branch: stable/queens
Review: https:/
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit 1920a37a94b7a95
Author: Slawek Kaplonski <email address hidden>
Date: Mon May 20 18:47:18 2019 +0200
Show all SG rules belong to SG in group's details
If security group contains rule(s) which were created by different
user (admin), owner of this security group should see such rules
even if those rules don't belong to him.
This patch changes to use admin_context to get security group rules
in get_security_
Test to cover such case is added in neutron-
Change-Id: I890c81bb6eabc5
Closes-Bug: #1824248
Changed in neutron: | |
status: | In Progress → Fix Released |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/stein
commit acd081c298052e6
Author: Slawek Kaplonski <email address hidden>
Date: Mon May 20 18:47:18 2019 +0200
Show all SG rules belong to SG in group's details
If security group contains rule(s) which were created by different
user (admin), owner of this security group should see such rules
even if those rules don't belong to him.
This patch changes to use admin_context to get security group rules
in get_security_
Test to cover such case is added in neutron-
Change-Id: I890c81bb6eabc5
Closes-Bug: #1824248
(cherry picked from commit 1920a37a94b7a95
tags: | added: in-stable-stein |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/rocky
commit 252d80058cacc79
Author: Slawek Kaplonski <email address hidden>
Date: Mon May 20 18:47:18 2019 +0200
Show all SG rules belong to SG in group's details
If security group contains rule(s) which were created by different
user (admin), owner of this security group should see such rules
even if those rules don't belong to him.
This patch changes to use admin_context to get security group rules
in get_security_
Test to cover such case is added in neutron-
Conflicts:
Change-Id: I890c81bb6eabc5
Closes-Bug: #1824248
(cherry picked from commit 1920a37a94b7a95
tags: | added: in-stable-rocky |
tags: | added: in-stable-queens |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/queens
commit e02b66a858b5527
Author: Slawek Kaplonski <email address hidden>
Date: Mon May 20 18:47:18 2019 +0200
Show all SG rules belong to SG in group's details
If security group contains rule(s) which were created by different
user (admin), owner of this security group should see such rules
even if those rules don't belong to him.
This patch changes to use admin_context to get security group rules
in get_security_
Test to cover such case is added in neutron-
Conflicts:
Change-Id: I890c81bb6eabc5
Closes-Bug: #1824248
(cherry picked from commit 1920a37a94b7a95
(cherry picked from commit 252d80058cacc79
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-tempest-plugin (master) | #40 |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit 87c3f941a3dd168
Author: Slawek Kaplonski <email address hidden>
Date: Mon May 20 18:50:53 2019 +0200
Add API test case to check if SG displays all rules
This patch adds new API test which checks if owner of security group
can see rules which belongs to his security group even if rule was
created and belongs to other user (admin).
Patch for master branch:
Depends-On: https:/
Backport to stable/Stein:
Depends-On: https:/
Backport to stable/Rocky:
Depends-On: https:/
Backport to stable/Queens:
Depends-On: https:/
Change-Id: I728cd8252d27e2
Related-Bug: #1824248
tags: | added: neutron-proactive-backport-potential |
ZhangChi (chzhang8) wrote : | #42 |
https:/
This solution patch can result in another bug as follows:
https:/
This issue was fixed in the openstack/neutron 13.0.4 release.
This issue was fixed in the openstack/neutron 14.0.2 release.
This issue was fixed in the openstack/neutron 12.1.0 release.
Fix proposed to branch: master
Review: https:/
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron-tempest-plugin (master) | #47 |
Related fix proposed to branch: master
Review: https:/
This issue was fixed in the openstack/neutron 15.0.0.0b1 development milestone.
Fix proposed to branch: stable/train
Review: https:/
Fix proposed to branch: stable/stein
Review: https:/
Fix proposed to branch: stable/rocky
Review: https:/
Fix proposed to branch: stable/queens
Review: https:/
This bug has had a related patch abandoned and has been automatically un-assigned due to inactivity. Please re-assign yourself if you are continuing work or adjust the state as appropriate if it is no longer valid.
Changed in neutron: | |
assignee: | Slawek Kaplonski (slaweq) → nobody |
tags: | added: timeout-abandon |
Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: stable/rocky
Review: https:/
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.
This bug has had a related patch abandoned and has been automatically un-assigned due to inactivity. Please re-assign yourself if you are continuing work or adjust the state as appropriate if it is no longer valid.
Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: stable/queens
Review: https:/
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.
Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https:/
Reason: This review is > 4 weeks without comment, and failed Jenkins the last time it was checked. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and leaving a 'recheck' comment to get fresh test results.
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit b898d2e3c08b50e
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:02:52 2019 +0200
List SG rules which belongs to tenant's SG
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-
But it is also required to behave in same way for
GET /v2.0/security-
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_
ADMIN_
ports.
Change-Id: I09114712582d2d
Closes-Bug: #1824248
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/train
commit a6b55d760bee4ea
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:02:52 2019 +0200
List SG rules which belongs to tenant's SG
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-
But it is also required to behave in same way for
GET /v2.0/security-
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_
ADMIN_
ports.
Change-Id: I09114712582d2d
Closes-Bug: #1824248
tags: | added: in-stable-train |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/stein
commit 47890e9b8582695
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:02:52 2019 +0200
List SG rules which belongs to tenant's SG
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-
But it is also required to behave in same way for
GET /v2.0/security-
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_
ADMIN_
ports.
Change-Id: I09114712582d2d
Closes-Bug: #1824248
(cherry picked from commit b898d2e3c08b50e
Sam Morrison (sorrison) wrote : | #62 |
Just a heads up that this causes a huge performance regression see lp:1863201
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/rocky
commit 993a344559f293a
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:02:52 2019 +0200
List SG rules which belongs to tenant's SG
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-
But it is also required to behave in same way for
GET /v2.0/security-
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_
ADMIN_
ports.
Conflicts:
Change-Id: I09114712582d2d
Closes-Bug: #1824248
(cherry picked from commit b898d2e3c08b50e
This issue was fixed in the openstack/neutron 15.0.2 release.
This issue was fixed in the openstack/neutron 14.1.0 release.
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: stable/queens
commit e00ebee05318edb
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:02:52 2019 +0200
List SG rules which belongs to tenant's SG
In case when user's security group contains rules created e.g.
by admin, and such rules has got admin's tenant as tenant_id,
owner of security group should be able to see those rules.
Some time ago this was addressed for request:
GET /v2.0/security-
But it is also required to behave in same way for
GET /v2.0/security-
So this patch fixes this behaviour for listing of security
group rules.
To achieve that this patch also adds new policy rule:
ADMIN_
ADMIN_
ports.
Conflicts:
Change-Id: I09114712582d2d
Closes-Bug: #1824248
(cherry picked from commit b898d2e3c08b50e
(cherry picked from commit 36d1086569627af
This issue was fixed in the openstack/neutron 16.0.0.0b1 development milestone.
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron-tempest-plugin (master) | #68 |
Reviewed: https:/
Committed: https:/
Submitter: Zuul
Branch: master
commit 31c0006ded28255
Author: Slawek Kaplonski <email address hidden>
Date: Thu Sep 12 22:11:35 2019 +0200
Add list security group rules API test
This test checks that regular user can see all SG rules which belongs
to his tenant OR belongs to security group owned by his tenant.
This test also ensures that SG rules from different tenants and Security
Groups are not visible for regular user.
Fix for master branch
Depends-On: https:/
Fix for stable/train
Depends-On: https:/
Fix for stable/stein
Depends-On: https:/
Fix for stable/rocky
Depends-On: https:/
Fix for stable/queens
Depends-On: https:/
Change-Id: Ic2e97ab8162d10
Related-Bug: #1824248
This issue was fixed in the openstack/neutron 13.0.7 release.
tags: | removed: neutron-proactive-backport-potential |
Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.