Open Stack

On Wed, Jan 18, 2017 at 10:45 PM, Bernard Cafarelli <bcafarel at redhat.com>
wrote:

> Hi neutrinos,
>
> I would like your feedback on the mentioned changeset in title[1]
> (yes, added since Liberty).
>
> With this patch, we (should) skip ports with
> port_security_enabled=False or with an empty list of security groups
> when processing added ports [2]. But we found multiple problems here
>
> * Ports create with port_security_enabled=False
>
> This is the original bug that started this mail: if the FORWARD
> iptables chain has a REJECT default policy/last rule, the traffic is
> still blocked[3]. There is also a launchpad bug with similar details
> [4]
> The problem here: these ports must not be skipped, as we add specific
> firewall rules to allow all traffic. These iptables rules have the
> following comment:
> "/* Accept all packets when port security is disabled. */"
>
> With the current code, any port created with port security will not
> have these rules (and updates do not work).
> I initially sent a patch to process these ports again [5], but there
> is more (as detailed by some in the launchpad bug)
>
> * Ports with no security groups, current code
>
> There is a bug in the  current agent code [6]: even with no security
> groups, the check will return true as, the security_groups key exists
> in the port details (with value "[]").
> So the port will not be skipped
>
> * Ports with no security groups, updated code
>
> Next step was to update checks (security groups list not empy, port
> security True or None), and test again. The port this time was
> skipped, but this showed up in openvswitch-agent.log:
> 2017-01-18 16:19:56.780 7458 INFO
> neutron.agent.linux.iptables_firewall
> [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Attempted to
> update port filter which is not filtered
> c2c58f8f-3b76-4c00-b792-f1726b28d2fc
> 2017-01-18 16:19:56.853 7458 INFO
> neutron.plugins.ml2.drivers.openvswitch.agent.ovs_neutron_agent
> [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Configuration for
> devices up [u'c2c58f8f-3b76-4c00-b792-f1726b28d2fc'] and devices down
> [] completed.
>
> Which is the kind of logs we saw in the first bug report. So as an
> additional test, I tried to update this port, adding a security group.
> New log entries:
> 2017-01-18 17:36:53.164 7458 INFO neutron.agent.securitygroups_rpc
> [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Refresh firewall
> rules
> 2017-01-18 17:36:55.873 7458 INFO
> neutron.agent.linux.iptables_firewall
> [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Attempted to
> update port filter which is not filtered
> 0f2eea88-0e6a-4ea9-819c-e26eb692cb25
> 2017-01-18 17:36:58.587 7458 INFO
> neutron.plugins.ml2.drivers.openvswitch.agent.ovs_neutron_agent
> [req-c49ca24f-1df8-40d7-8c48-6aab842ba34a - - - - -] Configuration for
> devices up [u'0f2eea88-0e6a-4ea9-819c-e26eb692cb25'] and devices down
> [] completed.
>
> And the iptables configuration did not change to show the newly allowed
> ports.
>
> So with a fixed check, wend up back in the same buggy situation as the
> first one.
>
> * Feedback
>
> So which course of action should we take? After checking these 3 cases
> out, I am in favour of reverting this commit entirely, as in its
> current state it does not help for ports without security groups, and
> breaks ports with port security disabled.
>
>
After having gone through the code and debugged the situation I'm also in
favor of reverting the patch. We should explicitly setup a rule which allows
traffic for that tap device exactly as we do when the port_security_enabled
is switched from True to False. We can't relay on traffic to be implicitly
allowed.

Also, on the tests side, should we add more tests only using create
> calls (port_security tests mostly update an existing port)? How to
> make sure these iptables rules are correctly applied (the ping tests
> are not enough, especially if the host system does not reject packets
> by default)?


Tests are incomplete so we should add either functional or fullstack/tempest
tests that validate these cases (ports created with port_security_enabled
set
to False, ports created with no security groups, etc.). I can try to do
that.




> [1] https://review.openstack.org/#/c/210321/
> [2] https://github.com/openstack/neutron/blob/
> a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/
> ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1640
> [3] https://bugzilla.redhat.com/show_bug.cgi?id=1406263
> [4] https://bugs.launchpad.net/neutron/+bug/1549443
> [5] https://review.openstack.org/#/c/421832/
> [6] https://github.com/openstack/neutron/blob/
> a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/
> ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1521
>
> Thanks!
>
> --
> Bernard Cafarelli
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20170119/e2b70ed3/attachment.html>