<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jan 18, 2017 at 10:45 PM, Bernard Cafarelli <span dir="ltr"><<a href="mailto:bcafarel@redhat.com" target="_blank">bcafarel@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi neutrinos,<br>
<br>
I would like your feedback on the mentioned changeset in title[1]<br>
(yes, added since Liberty).<br>
<br>
With this patch, we (should) skip ports with<br>
port_security_enabled=False or with an empty list of security groups<br>
when processing added ports [2]. But we found multiple problems here<br>
<br>
* Ports create with port_security_enabled=False<br>
<br>
This is the original bug that started this mail: if the FORWARD<br>
iptables chain has a REJECT default policy/last rule, the traffic is<br>
still blocked[3]. There is also a launchpad bug with similar details<br>
[4]<br>
The problem here: these ports must not be skipped, as we add specific<br>
firewall rules to allow all traffic. These iptables rules have the<br>
following comment:<br>
"/* Accept all packets when port security is disabled. */"<br>
<br>
With the current code, any port created with port security will not<br>
have these rules (and updates do not work).<br>
I initially sent a patch to process these ports again [5], but there<br>
is more (as detailed by some in the launchpad bug)<br>
<br>
* Ports with no security groups, current code<br>
<br>
There is a bug in the  current agent code [6]: even with no security<br>
groups, the check will return true as, the security_groups key exists<br>
in the port details (with value "[]").<br>
So the port will not be skipped<br>
<br>
* Ports with no security groups, updated code<br>
<br>
Next step was to update checks (security groups list not empy, port<br>
security True or None), and test again. The port this time was<br>
skipped, but this showed up in openvswitch-agent.log:<br>
2017-01-18 16:19:56.780 7458 INFO<br>
neutron.agent.linux.iptables_<wbr>firewall<br>
[req-c49ca24f-1df8-40d7-8c48-<wbr>6aab842ba34a - - - - -] Attempted to<br>
update port filter which is not filtered<br>
c2c58f8f-3b76-4c00-b792-<wbr>f1726b28d2fc<br>
2017-01-18 16:19:56.853 7458 INFO<br>
neutron.plugins.ml2.drivers.<wbr>openvswitch.agent.ovs_neutron_<wbr>agent<br>
[req-c49ca24f-1df8-40d7-8c48-<wbr>6aab842ba34a - - - - -] Configuration for<br>
devices up [u'c2c58f8f-3b76-4c00-b792-<wbr>f1726b28d2fc'] and devices down<br>
[] completed.<br>
<br>
Which is the kind of logs we saw in the first bug report. So as an<br>
additional test, I tried to update this port, adding a security group.<br>
New log entries:<br>
2017-01-18 17:36:53.164 7458 INFO neutron.agent.securitygroups_<wbr>rpc<br>
[req-c49ca24f-1df8-40d7-8c48-<wbr>6aab842ba34a - - - - -] Refresh firewall<br>
rules<br>
2017-01-18 17:36:55.873 7458 INFO<br>
neutron.agent.linux.iptables_<wbr>firewall<br>
[req-c49ca24f-1df8-40d7-8c48-<wbr>6aab842ba34a - - - - -] Attempted to<br>
update port filter which is not filtered<br>
0f2eea88-0e6a-4ea9-819c-<wbr>e26eb692cb25<br>
2017-01-18 17:36:58.587 7458 INFO<br>
neutron.plugins.ml2.drivers.<wbr>openvswitch.agent.ovs_neutron_<wbr>agent<br>
[req-c49ca24f-1df8-40d7-8c48-<wbr>6aab842ba34a - - - - -] Configuration for<br>
devices up [u'0f2eea88-0e6a-4ea9-819c-<wbr>e26eb692cb25'] and devices down<br>
[] completed.<br>
<br>
And the iptables configuration did not change to show the newly allowed ports.<br>
<br>
So with a fixed check, wend up back in the same buggy situation as the<br>
first one.<br>
<br>
* Feedback<br>
<br>
So which course of action should we take? After checking these 3 cases<br>
out, I am in favour of reverting this commit entirely, as in its<br>
current state it does not help for ports without security groups, and<br>
breaks ports with port security disabled.<br>
<br></blockquote><div><br></div><div>After having gone through the code and debugged the situation I'm also in<br></div><div><div>favor of reverting the patch. We should explicitly setup a rule which allows</div><div>traffic for that tap device exactly as we do when the port_security_enabled</div><div>is switched from True to False. We can't relay on traffic to be implicitly</div><div>allowed.</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Also, on the tests side, should we add more tests only using create<br>
calls (port_security tests mostly update an existing port)? How to<br>
make sure these iptables rules are correctly applied (the ping tests<br>
are not enough, especially if the host system does not reject packets<br>
by default)?</blockquote><div><br></div><div>Tests are incomplete so we should add either functional or fullstack/tempest</div><div>tests that validate these cases (ports created with port_security_enabled set</div><div>to False, ports created with no security groups, etc.). I can try to do that.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
[1] <a href="https://review.openstack.org/#/c/210321/" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/210321/</a><br>
[2] <a href="https://github.com/openstack/neutron/blob/a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1640" rel="noreferrer" target="_blank">https://github.com/openstack/<wbr>neutron/blob/<wbr>a66c27193573ce015c6c1234b0f2a1<wbr>d86fb85a22/neutron/plugins/<wbr>ml2/drivers/openvswitch/agent/<wbr>ovs_neutron_agent.py#L1640</a><br>
[3] <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1406263" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/<wbr>show_bug.cgi?id=1406263</a><br>
[4] <a href="https://bugs.launchpad.net/neutron/+bug/1549443" rel="noreferrer" target="_blank">https://bugs.launchpad.net/<wbr>neutron/+bug/1549443</a><br>
[5] <a href="https://review.openstack.org/#/c/421832/" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/421832/</a><br>
[6] <a href="https://github.com/openstack/neutron/blob/a66c27193573ce015c6c1234b0f2a1d86fb85a22/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L1521" rel="noreferrer" target="_blank">https://github.com/openstack/<wbr>neutron/blob/<wbr>a66c27193573ce015c6c1234b0f2a1<wbr>d86fb85a22/neutron/plugins/<wbr>ml2/drivers/openvswitch/agent/<wbr>ovs_neutron_agent.py#L1521</a><br>
<br>
Thanks!<br>
<span class="gmail-HOEnZb"><font color="#888888"><br>
--<br>
Bernard Cafarelli<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.<wbr>openstack.org?subject:<wbr>unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/<wbr>cgi-bin/mailman/listinfo/<wbr>openstack-dev</a><br>
</font></span></blockquote></div><br></div></div>