sg rules are sometimes not applied

[Dr. Jens Harbott]

Failure of negative test in gate:

http://logs.openstack.org/19/523319/5/check/neutron-tempest-plugin-scenario-linuxbridge/47b85c6/job-output.txt.gz#_2017-12-01_23_09_02_843619

Reproducing locally with a debug patch, I see that iptables_manager first applies the correct rules and then removes them again immediately after that, see http://paste.openstack.org/show/628245/

Steps to reproduce (taken from neutron_tempest_plugin.scenario.test_security_groups.NetworkDefaultSecGroupTest.test_ip_prefix_negative, possibly not minimal):

- create two security groups
- add ssh access to first, icmp access to second one
- create an instance with these two security groups applied
- run iptables-save and discover no rules applied to the instance

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/525934

[Tristan Cacqueray (tristan-cacqueray)]

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.

[Dr. Jens Harbott (j-harbott)]

Logs for create/delete in test patch:

http://logs.openstack.org/34/525934/1/check/neutron-tempest-plugin-scenario-linuxbridge/1ce5729/logs/screen-q-agt.txt.gz#_Dec_06_08_47_31_891621

http://logs.openstack.org/34/525934/1/check/neutron-tempest-plugin-scenario-linuxbridge/1ce5729/logs/screen-q-agt.txt.gz#_Dec_06_08_47_32_498093

[Dr. Jens Harbott (j-harbott)]

Seems this is related to qos, see the line here http://logs.openstack.org/34/525934/1/check/neutron-tempest-plugin-scenario-linuxbridge/1ce5729/logs/screen-q-agt.txt.gz#_Dec_06_08_47_50_693519

DEBUG neutron.agent.l2.extensions.qos [None req-f6631a63-5ea4-4349-96be-06c131f2f3be None None] QoS extension did not have information on port 5f77c42c-ffa4-4ff2-b72f-4e83a5892bdb {{(pid=20181) clean_by_port /opt/stack/new/neutron/neutron/agent/l2/extensions/qos.py:190}}

I also fail to reproduce this when I remove the "enable_service neutron-qos" from the scenario.

[Dr. Jens Harbott (j-harbott)]

That log message points to https://review.openstack.org/449710 and reverting that patch seems to revert the issue, but I fail to see the error in that patch.

[Ihar Hrachyshka (ihar-hrachyshka)]

This is now called with the patch identified: https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/linuxbridge/agent/extension_drivers/qos_driver.py#L106-L110

Maybe it messes with iptables manager that is used by the agent to set rules?

Miguel will discuss the issue with Slawek.

[Slawek Kaplonski (slaweq)]

So it looks that there is kind of race condition here.
First instance of IptablesManager object is created in iptables_firewall driver and it applies wanted SG rules in filters table.
But than another instance of IptablesManager is created in LB qos driver and when "delete_dscp_marking" method is called it tries to remove QoS related rules from iptables. That works fine but as this second instance of IptablesManager don't have any rules in "filters" or "raw" tables it cleans it in _apply_synchronize method.

[Ihar Hrachyshka (ihar-hrachyshka)]

Solution here would be to expose iptables_manager from core agent into extensions. We should introduce an "API" object for l2 extension manager, like what we have for OVS, and pass it into qos driver where it then would be used.

[Slawek Kaplonski (slaweq)]

@Ihar: thx for this tip. I was looking at l2 extension api little bit and it looks that it should one quite easy to fix it this way (I hope so). I will soon push a patch to review for that

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Slawek Kaplonski (<email address hidden>) on branch: master
Review: https://review.openstack.org/527722

[OpenStack Infra (hudson-openstack)]

Change abandoned by Jens Harbott (frickler) (<email address hidden>) on branch: master
Review: https://review.openstack.org/525934
Reason: https://review.openstack.org/527965 seems to be working fine ... modulo gate instabilities

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/527965
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=cbee0f9f88ff34f70ff19590471b5405e06ff2a9
Submitter: Zuul
Branch: master

commit cbee0f9f88ff34f70ff19590471b5405e06ff2a9
Author: Sławek Kapłoński <email address hidden>
Date: Thu Dec 14 14:51:01 2017 +0100

    Use same instance of iptables_manager in L2 agent and extensions

    This commit adds common_agent_extension class which is agent API
    for L2 extension drivers used e.g. by Linuxbridge agent.
    This is necessary to be able to use instance of iptables_manager
    used in firewall driver also in L2 extension drivers (like qos).

    This patch refactors little bit iptables_manager code to make possible
    to initialize e.g. mangle or nat table on demand, even if iptables
    is created as "state_less"

    Change-Id: I3b66e49b7f176124e8aea3eb96d0d465f1ab1ea0
    Closes-Bug: #1736674

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/pike
Review: https://review.openstack.org/531735

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ocata
Review: https://review.openstack.org/531747

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/531735
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=b0f7be9c7651e9e49f548bb798cf9979b896e691
Submitter: Zuul
Branch: stable/pike

commit b0f7be9c7651e9e49f548bb798cf9979b896e691
Author: Sławek Kapłoński <email address hidden>
Date: Thu Dec 14 14:51:01 2017 +0100

    Use same instance of iptables_manager in L2 agent and extensions

    This commit adds common_agent_extension class which is agent API
    for L2 extension drivers used e.g. by Linuxbridge agent.
    This is necessary to be able to use instance of iptables_manager
    used in firewall driver also in L2 extension drivers (like qos).

    This patch refactors little bit iptables_manager code to make possible
    to initialize e.g. mangle or nat table on demand, even if iptables
    is created as "state_less"

    Change-Id: I3b66e49b7f176124e8aea3eb96d0d465f1ab1ea0
    Closes-Bug: #1736674
    (cherry picked from commit cbee0f9f88ff34f70ff19590471b5405e06ff2a9)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/531747
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=df8412b3e834bbe38f32e9748a1f1116e0920e2e
Submitter: Zuul
Branch: stable/ocata

commit df8412b3e834bbe38f32e9748a1f1116e0920e2e
Author: Sławek Kapłoński <email address hidden>
Date: Thu Dec 14 14:51:01 2017 +0100

    Use same instance of iptables_manager in L2 agent and extensions

    This commit adds common_agent_extension class which is agent API
    for L2 extension drivers used e.g. by Linuxbridge agent.
    This is necessary to be able to use instance of iptables_manager
    used in firewall driver also in L2 extension drivers (like qos).

    This patch refactors little bit iptables_manager code to make possible
    to initialize e.g. mangle or nat table on demand, even if iptables
    is created as "state_less"

    Change-Id: I3b66e49b7f176124e8aea3eb96d0d465f1ab1ea0
    Closes-Bug: #1736674
    (cherry picked from commit cbee0f9f88ff34f70ff19590471b5405e06ff2a9)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 12.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 10.0.5 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 11.0.3 release.

[Jeremy Stanley (fungi)]

Treating this as a security hardening opportunity, no advisory needed.