yahoo-eng-team team mailing list archive

[Cliff Parsons <1779003@xxxxxxxxxxxxxxxxxx>]

Public bug reported:

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.

** Affects: neutron
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to neutron.
https://bugs.launchpad.net/bugs/1779003

Title:
  Policy check using new attribute value to validate access to the same
  attribute

Status in neutron:
  New

Bug 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.

To manage notifications about this bug go to:
https://bugs.launchpad.net/neutron/+bug/1779003/+subscriptions