Activity log for bug #1779003

Date Who What changed Old value New value Message
2018-06-27 22:06:44 Cliff Parsons bug added bug
2018-06-28 15:28:34 Cliff Parsons description During policy testing using Patrole with the new multi-policy support, it became apparent that the Neutron policy enforcement code is using the new value of an attribute (on a PUT request) to validate access to that same attribute. Here are two scenarios that expose this problem, using devstack: Scenario 1: 1. Set up Patrole as a Tempest plugin and set the [patrole] parameters in tempest.conf. 2. Set the rbac_test_role to "Member". 3. Change /etc/neutron/policy.json so that "update_network" and "update_network:shared" are defined as "rule:admin_only" (the "Member" role is not allowed to update the network) 4. Change /etc/neutron/policy.json so that "get_network" is defined as "rule:shared", so that a "Member" is allowed to view only a shared network, but not a non-shared network. 5. Run the patrole_tempest_plugin.tests.api.network.test_networks_rbac. NetworksRbacTest.test_update_network_shared test case. This test has recently been modified to test the multi-policy nature of Neutron's policy enforcement algorithm. - The test case expects a 404 NotFound exception to be raised if the role being tested ("Member" in this case) is not allowed to perform "get_network". A 403 Forbidden exception is expected if the role is not allowed to perform either of the "update_network" or "update_network:shared" actions. Result: With the current Neutron code in master, this test fails with the given policy because a 403 Forbidden is thrown instead of the expected 404 NotFound exception. The reason it does not throw the expected 404 is because the "Member" role IS allowed to perform "get_network". That is because the Neutron policy enforcement code uses the NEW value of the network's "shared" attribute in order to check if the rule "get_network" is allowed instead of the original value. The original value is shared=False as set by the test case, and the test case is modifying it to True. So "get_network" should not have been allowed for the Member role here. See code snippet: https://github.com/openstack/neutron/blob/ 9cb68ce7777f3016f6affe0abbc7ebe3b859f7c3/neutron/pecan_wsgi/hooks/ policy_enforcement.py#L123 Scenario 2: (setup is similar to scenario 1) The same problem occurs by running the following test case against the "Member" role: patrole_tempest_plugin.tests.api.network.test_networks_rbac. NetworksRbacTest.test_update_network_router_external using this set of policy rules: - "update_network" and "update_network:router:external" defined as "rule:admin_only" - "get_network" is defined as "rule:external", so that a "Member" is allowed to view only external networks. Result: Although 404 is expected to be thrown by Neutron due to the "get_network" rule, Neutron actually throws a 403 exception, so the test case fails. That is because the Neutron policy enforcement code uses the NEW value of the network's "external" attribute in order to check if the rule "get_network" is allowed instead of the original value. The original value is external=False as set by the test case, and the test case is modifying it to True. So "get_network" should not have been allowed for the Member role here. During policy testing using Patrole with the new multi-policy support, it became apparent that the Neutron policy enforcement code is using the new value of an attribute (on a PUT request) to validate access to that same attribute. Here are two scenarios that expose this problem, using devstack: Scenario 1: ----------  1. Set up Patrole as a Tempest plugin and set the [patrole] parameters in     tempest.conf.  2. Set the rbac_test_role to "Member".  3. Change /etc/neutron/policy.json so that "update_network" and     "update_network:shared" are defined as "rule:admin_only" (the "Member"     role is not allowed to update the network)  4. Change /etc/neutron/policy.json so that "get_network" is defined as    "rule:shared", so that a "Member" is allowed to view only a shared     network, but not a non-shared network.  5. Run the patrole_tempest_plugin.tests.api.network.test_networks_rbac.     NetworksRbacTest.test_update_network_shared test case. This test has     recently been modified to test the multi-policy nature of Neutron's     policy enforcement algorithm.       - The test case expects a 404 NotFound exception to be raised if the         role being tested ("Member" in this case) is not allowed to perform         "get_network". A 403 Forbidden exception is expected if the role is         not allowed to perform either of the "update_network" or         "update_network:shared" actions.  Result:      With the current Neutron code in master, this test fails with the given policy because a 403 Forbidden is thrown instead of the expected 404 NotFound exception.      The reason it does not throw the expected 404 is because the "Member"      role IS allowed to perform "get_network". That is because the Neutron      policy enforcement code uses the NEW value of the network's "shared"      attribute in order to check if the rule "get_network" is allowed      instead of the original value. The original value is shared=False as      set by the test case, and the test case is modifying it to True. So      "get_network" should not have been allowed for the Member role here.      See code snippet: https://github.com/openstack/neutron/blob/      9cb68ce7777f3016f6affe0abbc7ebe3b859f7c3/neutron/pecan_wsgi/hooks/      policy_enforcement.py#L123 Scenario 2: (setup is similar to scenario 1) ---------- The same problem occurs by running the following test case against the  "Member" role:   patrole_tempest_plugin.tests.api.network.test_networks_rbac.       NetworksRbacTest.test_update_network_router_external  using this set of policy rules:      - "update_network" and "update_network:router:external" defined as        "rule:admin_only"      - "get_network" is defined as "rule:external", so that a "Member" is        allowed to view only external networks.  Result:      Although 404 is expected to be thrown by Neutron due to the      "get_network" rule, Neutron actually throws a 403 exception, so the      test case fails. That is because the Neutron policy enforcement code      uses the NEW value of the network's "external" attribute in order to      check if the rule "get_network" is allowed instead of the original      value. The original value is external=False as set by the test case,      and the test case is modifying it to True. So "get_network" should      not have been allowed for the Member role here.
2018-07-03 15:23:11 Cliff Parsons description During policy testing using Patrole with the new multi-policy support, it became apparent that the Neutron policy enforcement code is using the new value of an attribute (on a PUT request) to validate access to that same attribute. Here are two scenarios that expose this problem, using devstack: Scenario 1: ----------  1. Set up Patrole as a Tempest plugin and set the [patrole] parameters in     tempest.conf.  2. Set the rbac_test_role to "Member".  3. Change /etc/neutron/policy.json so that "update_network" and     "update_network:shared" are defined as "rule:admin_only" (the "Member"     role is not allowed to update the network)  4. Change /etc/neutron/policy.json so that "get_network" is defined as    "rule:shared", so that a "Member" is allowed to view only a shared     network, but not a non-shared network.  5. Run the patrole_tempest_plugin.tests.api.network.test_networks_rbac.     NetworksRbacTest.test_update_network_shared test case. This test has     recently been modified to test the multi-policy nature of Neutron's     policy enforcement algorithm.       - The test case expects a 404 NotFound exception to be raised if the         role being tested ("Member" in this case) is not allowed to perform         "get_network". A 403 Forbidden exception is expected if the role is         not allowed to perform either of the "update_network" or         "update_network:shared" actions.  Result:      With the current Neutron code in master, this test fails with the given policy because a 403 Forbidden is thrown instead of the expected 404 NotFound exception.      The reason it does not throw the expected 404 is because the "Member"      role IS allowed to perform "get_network". That is because the Neutron      policy enforcement code uses the NEW value of the network's "shared"      attribute in order to check if the rule "get_network" is allowed      instead of the original value. The original value is shared=False as      set by the test case, and the test case is modifying it to True. So      "get_network" should not have been allowed for the Member role here.      See code snippet: https://github.com/openstack/neutron/blob/      9cb68ce7777f3016f6affe0abbc7ebe3b859f7c3/neutron/pecan_wsgi/hooks/      policy_enforcement.py#L123 Scenario 2: (setup is similar to scenario 1) ---------- The same problem occurs by running the following test case against the  "Member" role:   patrole_tempest_plugin.tests.api.network.test_networks_rbac.       NetworksRbacTest.test_update_network_router_external  using this set of policy rules:      - "update_network" and "update_network:router:external" defined as        "rule:admin_only"      - "get_network" is defined as "rule:external", so that a "Member" is        allowed to view only external networks.  Result:      Although 404 is expected to be thrown by Neutron due to the      "get_network" rule, Neutron actually throws a 403 exception, so the      test case fails. That is because the Neutron policy enforcement code      uses the NEW value of the network's "external" attribute in order to      check if the rule "get_network" is allowed instead of the original      value. The original value is external=False as set by the test case,      and the test case is modifying it to True. So "get_network" should      not have been allowed for the Member role here. During policy testing using Patrole with the new multi-policy support, it became apparent that the Neutron policy enforcement code is using the new value of an attribute (on a PUT request) to validate access to that same attribute. Here are two scenarios that expose this problem, using devstack: Scenario 1: ----------  1. Set up Patrole as a Tempest plugin and set the [patrole] parameters in     tempest.conf.  2. Set the rbac_test_role to "Member".  3. Change /etc/neutron/policy.json so that "update_network" and     "update_network:shared" are defined as "rule:admin_only" (the "Member"     role is not allowed to update the network)  4. Change /etc/neutron/policy.json so that "get_network" is defined as     "rule:shared", so that a "Member" is allowed to view only a shared     network, but not a non-shared network.  5. Run the patrole_tempest_plugin.tests.api.network.test_networks_rbac.     NetworksRbacTest.test_update_network_shared test case. This test has     recently been modified to test the multi-policy nature of Neutron's     policy enforcement algorithm.       - The test case expects a 404 NotFound exception to be raised if the         role being tested ("Member" in this case) is not allowed to perform         "get_network". A 403 Forbidden exception is expected if the role is         not allowed to perform either of the "update_network" or         "update_network:shared" actions.  Result:      With the current Neutron code in master, this test fails with the      given policy because a 403 Forbidden is thrown instead of the expected      404 NotFound exception.      The reason it does not throw the expected 404 is because the "Member"      role IS allowed to perform "get_network". That is because the Neutron      policy enforcement code uses the NEW value of the network's "shared"      attribute in order to check if the rule "get_network" is allowed      instead of the original value. The original value is shared=False as      set by the test case, and the test case is modifying it to True. So      "get_network" should not have been allowed for the Member role here.      See code snippet: https://github.com/openstack/neutron/blob/      9cb68ce7777f3016f6affe0abbc7ebe3b859f7c3/neutron/pecan_wsgi/hooks/      policy_enforcement.py#L123 Scenario 2: (setup is similar to scenario 1) ----------  The same problem occurs by running the following test case against the  "Member" role:   patrole_tempest_plugin.tests.api.network.test_networks_rbac.       NetworksRbacTest.test_update_network_router_external  using this set of policy rules:      - "update_network" and "update_network:router:external" defined as        "rule:admin_only"      - "get_network" is defined as "rule:external", so that a "Member" is        allowed to view only external networks.  Result:      Although 404 is expected to be thrown by Neutron due to the      "get_network" rule, Neutron actually throws a 403 exception, so the      test case fails. That is because the Neutron policy enforcement code      uses the NEW value of the network's "external" attribute in order to      check if the rule "get_network" is allowed instead of the original      value. The original value is external=False as set by the test case,      and the test case is modifying it to True. So "get_network" should      not have been allowed for the Member role here.
2018-07-13 18:51:25 OpenStack Infra neutron: status New In Progress
2018-07-13 18:51:25 OpenStack Infra neutron: assignee Cliff Parsons (cliffhparsons)
2018-07-13 21:35:24 Hongbin Lu neutron: status In Progress Incomplete
2018-07-27 15:27:23 OpenStack Infra neutron: status Incomplete In Progress
2018-08-31 07:36:43 Slawek Kaplonski neutron: status In Progress New
2018-08-31 07:36:43 Slawek Kaplonski neutron: assignee Cliff Parsons (cliffhparsons)
2018-08-31 07:36:48 Slawek Kaplonski tags timeout-abandon