Re: [openstack-dev] [neutron] "Setup firewall filters only for required ports" bug

[Daniel Alvarez Sanchez]

On Wed, Jan 18, 2017 at 10:45 PM, Bernard Cafarelli 
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] [neutron] "Setup firewall filters only for required ports" bug

[Bernard Cafarelli]

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.

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)?

[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-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev