This change enhances the IptablesFirewallDriver with support for remote
address groups. Previously, this feature was only available in the
OVSFirewallDriver. This commit harmonizes the capabilities across both
firewall drivers, and by inheritance also to OVSHybridIptablesFirewallDriver.
Background -
The Neutron API allows operators to configure remote address groups [1],
however the OVSHybridIptablesFirewallDriver and IptablesFirewallDriver do
not implement these remote group restrictions. When configuring security
group rules with remote address groups, connections get enabled
based on other rule parameters, ignoring the configured remote address
group restrictions.
This behaviour undocumented, and may lead to more-open-than-configured network
access.
Closes-Bug: #2058138
Change-Id: I76b3cb46ee603fa5e829537af41316bb42a6f30f
iptables-save uses a system-dependent value, usually that
found in /etc/protocols, when 'ipip' is given as the
security group protocol. The intent is to always use the
string value for IP protocol '4', as iptables-save has no
'-n' flag to print values numerically.
This updates a previous change (793dfb04d) that hard-coded
that string to 'ipencap', which broke CentOS/Fedora, which
uses 'ipv4'.
For this reason we cannot hard-code anything in neutron-lib,
this needs to be added dynamically, so this one-line change
needs to stay here, and effectively closes the bug.
Closes-bug: #2054324
Change-Id: Ic40b539c9ef5cfa4cbbd6575e19e653342e8342b
Map 'ipip' to use the string 'ipencap' so the
IptablesFirewallDriver class in neutron works correctly.
Once neutron-lib is bumped this can be removed.
Add tests for IP protocol 'ipip', '4' and '94' to make
sure the IptablesFirewallDriver class in neutron treats
them correctly.
Long description below.
This is one of those confusing edge cases and I think
Linux is conspiring against us. Let me explain.
1) neutron-lib does correctly define the protocol name 'ipip' as 4.
2) The linux kernel uses the same in in.h:
IPPROTO_IPIP = 4
IPPROTO_BEETPH = 94 (?)
3) iptables maps 'ipip' to 94 and 'ipencap' to 4.
# for num in {0..255}; do iptables -A INPUT -p $num; done
# iptables-save | grep -E 'ipip|ipencap'
-A INPUT -p ipencap
-A INPUT -p ipip
4) /etc/protocols does the same as iptables:
grep -E 'ipencap|ipip' /etc/protocols
ipencap 4 IP-ENCAP # IP encapsulated in IP (officially ``IP'')
ipip 94 IPIP # IP-within-IP Encapsulation Protocol
5) getprotoby{name|number} does what /etc/protocols does:
$ getprotobyname ipip
struct protoent: (0x7fbbbcca9c60)
p_name ipip
p_aliases IPIP
p_proto 94
$ getprotobynumber 4
struct protoent: (0x7fc51ad86be0)
p_name ipencap
p_aliases IP-ENCAP
p_proto 4
Neutron actually builds a mapping based on the getprotoby*
calls, so in the iptables case it winds-up doing the wrong
thing.
Partial-bug: #2054324
Change-Id: Icc84b54be07d39059723d6c233c03aa130102423
When deployed with k8s, k8s service types like NodePort
or ExternalIP will affect vm traffic on nat table's
PREROUTING chain. This PS try to mitigate the effect
by allowing vm traffic to bypass those rules.
Change-Id: Iae12d9c2f37bc0fca9c3d5e85e46c642263e4a77
Closes-Bug: #1908957
Since [1], the RPC method "security_group_info_for_devices" returns
not only the IP address but a list with the IP and the MAC addresses.
The returned blob is like this:
{sg_id: {'IPv4': [[ip1, mac1], [ip2, mac2], 'IPv6': [...]}, ...}
The method "_get_sg_members" only needs the IP address, the MAC
address should be discarded.
NOTE: this bug was detected when testing
"neutron-tempest-plugin-scenario-linuxbridge" CI job with
"enable_ipset" set to False. By default, this value is set to
True (it is more efficient). In order to properly test this
patch, I pushed [2], creating a CI job disabling this option.
To avoid overpopulating the Neutron CI, this job is not going
to be added to the check gate (maybe to the periodic CI tasks).
[1]https://review.opendev.org/q/I6cb2ba05fa3337be46eb01f2d9f869efa41e4db6
[2]https://review.opendev.org/c/openstack/neutron/+/783103
Change-Id: Ia5e899a4133a0952be8f607a61282e71fb0545d1
Closes-Bug: #1922127
Replace rootwrap execution with privsep context execution.
This series of patches will progressively replace any
rootwrap call.
This patch migrates some missing execution methods present in
the code and removes unneeded rootwrap filters.
Story: #2007686
Task: #41558
Change-Id: I1542dc4cf98658fc9a40018192498c7a5cd1c3fe
In case when value of port['device'] don't starts with "tap_",
in comments to the conntrack or stateless rules in the iptables there
should be full port['device'] written. It will make things easier to
debug for the operators e.g. when using iptables_hybrid driver.
Change-Id: I427321fbb87865931b2b28abf7687d37e8d01a53
Closes-bug: #1880691
Now that we are python3 only, we should move to using the built
in version of mock that supports all of our testing needs and
remove the dependency on the "mock" package.
This patch moves all references to "import mock" to
"from unittest import mock". It also cleans up some new line
inconsistency.
Fixed an inconsistency in the OVSBridge.deferred() definition
as it needs to also have an *args argument.
Fixed an issue where an l3-agent test was mocking
functools.partial, causing a python3.8 failure.
Unit tests only, removing from tests/base.py affects
functional tests which need additional work.
Change-Id: I40e8a8410840c3774c72ae1a8054574445d66ece
When add allowed-address-pair 0.0.0.0/0 to one port, it will
unexpectedly open all others' protocol under same security
group. IPv6 has the same problem.
The root cause is the openflow rules calculation of the
security group, it will unexpectedly allow all IP(4&6)
traffic to get through.
For openvswitch openflow firewall, this patch adds a source
mac address match for the allowed-address-pair which has
prefix lenght 0, that means all ethernet packets from this
mac will be accepted. It exactly will meet the request of
accepting any IP address from the configured VM.
Test result shows that the remote security group and
allowed address pair works:
1. Port has 0.0.0.0/0 allowed-address-pair clould send any
IP (src) packet out.
2. Port has x.x.x.x/y allowed-address-pair could be accepted
for those VMs under same security group.
3. Ports under same network can reach each other (remote
security group).
4. Protocol port number could be accessed only when there
has related rule.
Closes-bug: #1867119
Change-Id: I2e3aa7c400d7bb17cc117b65faaa160b41013dde
If the configuration flag "debug_iptables_rules" is enabled, the
IPTables rules applied will be logged.
Similar to [1], when the IPTables firewall is enabled, it checks the
status of the following sysctl knobs:
* net.bridge.bridge-nf-call-arptables
* net.bridge.bridge-nf-call-ip6tables
* net.bridge.bridge-nf-call-iptables
In this case, the firewall is not enabling them but just checking the
status and logging it, to make easier the debugging process.
[1] https://review.opendev.org/#/c/371523/
Change-Id: I2ec953228d1d45e1d4c493c0b261901e6dbec0f7
Related-Bug: #1843259
Removed E125 (continuation line does not distinguish itself
from next logical line) from the ignore list and fixed all
the indentation issues. Didn't think it was going to be
close to 100 files when I started.
Change-Id: I0a6f5efec4b7d8d3632dd9dbb43e0ab58af9dff3
The conntrack command does not allow '-p 0' as an argument,
but does allow it's equivalent '-p ip'. Use it instead
so it doesn't generate an error.
Change-Id: Ica69eb85a6835952904a6390bb8a31e6afdecf69
Closes-bug: #1820744
After taking a closer look at bug 1818385, I found a couple
of follow-on things to fix in the security group code.
First, there are very few protocols that accept ports,
especially via iptables. For this reason I think it's
acceptable that the API rejects them as invalid.
Second, UDPlite has some interesting support in iptables. It
does not support using --dport directly, but does using
'-m multiport --dports 123', and also supports port ranges using
'-m multiport --dports 123:124'. Added code for this special
case.
Change-Id: Ifb2e6bb6c7a2e2987ba95040ef5a98ed50aa36d4
Closes-Bug: #1818385
Since iptables-restore doesn't support --dport with protocol vrrp,
it errors out setting the security groups on the hypervisor.
Marking this a partial fix, since we need a change to prevent
adding those incompatible rules in the first place, but this
patch will stop the bleeding.
Change-Id: If5e557a8e61c3aa364ba1e2c60be4cbe74c1ec8f
Partial-Bug: #1818385
Today the neutron common exceptions already live in neutron-lib and are
shimmed from neutron. This patch removes the neutron.common.exceptions
module and changes neutron's imports over to use their respective
neutron-lib exception module instead.
NeutronLibImpact
Change-Id: I9704f20eb21da85d2cf024d83338b3d94593671e
Merge the system protocol assignments into the iptables name
to protocol mapping array, IPTABLES_PROTOCOL_NAME_MAP, so that
systems with updated or new values in /etc/protocols can
successfully install iptables rules.
This was done as an IptablesFirewallDriver() instance mapping
since there is typically only a single instance per-agent,
and it also allows us to more easily unit test it.
Change-Id: Ib73def4e2a9e3644462fdee312768382fcb800a5
Closes-Bug: #1783378
Fixed all pep8 E265 errors and changed tox.ini to no longer
ignore them. Also removed an N536 comment missed from a
previous change.
Change-Id: Ie6db8406c3b884c95b2a54a7598ea83476b8dba1
Iptables firewall driver now adds rules for trusted ports to FORWARD
chain in EGRESS and INGRESS directions.
Unfiltered and trusted port rules are too wide. We have to match
traffic against security groups first.
Change-Id: I61e4dc92669e33a207adfb72a1692184884143e1
Closes-Bug: #1762736
This change:
* avoids creating lots of short-lived threads
(because: why would we ?)
* adds a try/except block to be sure we can log
any issue that would otherwise be hidden by
spawn_n.
* changes class from Queue to LightQueue after
eventlet.queue code inspection, since it's a
little smaller and should be faster
Change-Id: Ic348c08af375099a919116188ae17d2017695ecb
Closes-Bug: 1750777
Patch [1] added configuration of forward rule for trusted ports in
iptables firewall driver.
This patch fixes issue with many "duplicate iptables rule detected"
warning messages due to try to add such forward rule each time when
trusted port is updated.
Now such rule is added only once for port.
[1] https://review.openstack.org/#/c/525607/
Change-Id: Ib816887f07f16b6ac865bb81d0f27f12d0b47dfb
Closes-Bug: #1754770
With a large number of instances and/or security group rules,
conntrack updates when ports are removed or rules are changed
can take a long time to process. By enqueuing these to a set
or worker threads, the agent can continue with other work while
they are processed in the background.
This is a change in behavior in the agent since it could
program a new set of security group rules before all existing
conntrack entries are deleted, but since the iptables or OVSfw
NAT rules will have been removed, it should not pose a
security issue.
Change-Id: Ibf858c7fdf7a822a30e4a0c4722d70fd272741b6
Closes-bug: #1745468
SG with hybrid-iptables driver uses per port conntrack zones.
FWaaS port security uses per network conntrack zones based on
local vlans assigned by ovs l2 agent.
In case both SG iptables-hybrid driver and FWaaS port security is enabled,
there is a posibility of iptables-hybrid and OVS based FWaaS driver
allocating overlapping zone and creating security holes.
This patch changes the zone allocation range for iptables and
hybrid_iptables driver to 4097 - 65535. While OVS based
port security driver can use zones based on local vlan range 1 - 4096
Closes-Bug: #1745642
Change-Id: I4d51637ed1de8fe85b4982a03410d4a3f637ea3f
When a security group rule is created with icmp/ipv6-icmp alias such as
protocol number 1(ICMP), 58(ICMPv6) or string icmpv6(legacy name for
ipv6-icmp) as its protocol along with ICMP/ICMPv6 message type
specified, _port_arg will generate a wrong str for iptables/ip6tables.
Change-Id: Iae01b9a0da34797a5f061a110f06e18be9bbec5a
Closes-Bug: #1743552
When the support of protocol numbers in security groups
was fixed in [1], it introduced two deficiencies in the
iptables code:
- it was missing some protocols, for example, 'icmp', 'tcp'
and 'udp', so when rules were added by number we did not
use their name as iptables expects
- it used a dictionary to map numbers to names, but protocol
numbers are stored as strings (i.e. '1' != 1)
Updated the iptables number mapping dict to have all
currently-known values, even those that are already well-known
and should have been using a string instead of a number.
Also changed the iptables number mapping dict to use
strings as the keys instead of numbers, since that's
what will be passed from the security group code.
Removed IPTABLES_PROTOCOL_MAP as it lives in neutron-lib,
and accidentally snuck-in in [1].
[1] I5895250b47ddf664d214cf085be693c3897e0c87
Change-Id: I6b7575eb531b4f35579960c3feb47000cd259b86
Closes-Bug: 1719711
There are several places where base class setUp() method call was
called unnecessary. In this patchset, they are removed.
TrivialFix
Change-Id: I2961fa4a0216f7f1223ab87a249151f0feb91518
There was only a single list of allowed ICMPv6
types, but the defaults allowed for ingress
and egress are different when it comes to
Router Advertisements and Router Solicitations.
Change-Id: I737f07065cf2fb0b574a7f0f49e084488bf23ac0
Closes-bug: #1685237
according to https://wiki.openstack.org/wiki/Python3,
now we should avoid using six.iteritems and replace
it with dict.items.
Change-Id: I8753e80b34c0f86cf70aebc3bcbd3392ee933f62
Partial-Bug: #1680761
When adding an egress rule to sg assigned remote CIDR, vms using
this sg will add a rule in iptables like -oxxxxxxxx-x -d CIDR.
But test cases for egress with prefix always using -s CIDR, which
is not correct.
Closes-Bug: #1523835
Change-Id: Ifabfe3278aa0516a222f71153e47149ff4562d5e
The regular IPTablesFirewall needs zones to support safely
clearly conntrack entries.
In order to support the single bridge use case, the conntrack
manager had to be refactored slightly to allow zones to be
either unique to ports or unique to networks.
Since all ports in a network share a bridge in the IPTablesDriver
use case, a zone per port cannot be used since there is no way
to distinguish which zone traffic should be checked against when
traffic enters the bridge from outside the system.
A zone per network is adequate for the single bridge per network
solution since it implicitly does not suffer from the double-bridge
cross in a single network that led to per port usage in OVS.[1]
This had to adjust the functional firewall tests to use the correct
bridge name now that it's relevant in the non hybrid IPTables case.
1. Ibe9e49653b2a280ea72cb95c2da64cd94c7739da
Closes-Bug: #1668958
Closes-Bug: #1657260
Change-Id: Ie88237d3fe4807b712a7ec61eb932748c38952cc
Refactoring Neutron configuration options for agent common config to be
in neutron/conf/agent/common. This will allow centralization of all
configuration options and provide an easy way to import.
Partial-Bug: #1563069
Change-Id: Iebac0cdd3bcfd0135349128921b7ad7a1a939ab8
Needed-By: Ib676003bbe909b5a9013a3178b12dbe291d936af
The conntrack clearing code was belligerenty killing connections
without a conntrack zone specifier when it couldn't get the zone
for a given device. This means it would kill all connections based
on an IP address match, which meant hitting innocent bystanders
in other tenant networks with overlapping IP addresses.
This bad fallback was being triggered every time because it was
using the wrong identifier for a port to look up the zone.
This patch fixes the port lookup and adjusts the fallback behavior
to never clear conntrack entries if we can't find the conntrack
zone for a port.
This triggered the bug below (in the cases I root-caused) by
killing a metadata connection right in the middle of retrieving
a key.
Closes-Bug: #1668958
Change-Id: Ia4ee9b3305e89c958ac927980d80119c53ea519b
CT zones are used only in OVSHybridIptablesFirewallDriver.
Such zones are not set in IptablesFirewallDriver class but
even if iptables driver was is not using CT zones, it was
used by conntrack manager class during delete of conntrack
entry.
This cause issue that for Linuxbridge agent established and
active connection stayed active even after security group
rule was deleted.
This patch changes conntrack manager class that it will not
use CT zone (-w option) if zone for port was not assigned
earlier.
Change-Id: Ib9c8d0a09d0858ff6f36db406c6b2a9191f304d1
Closes-bug: 1657260
There were a couple of protocols missing from the iptables
name to module map - dccp and sctp. Also took the chance
to move it to the constants file and use the neutron-lib
constants for protocol names.
Change-Id: I2b770b029cbfbcb851ea71090b8e3aae314bdb62
When creating a new filter for a port, there is no need
to remove any iptables chains as none should exist. This
just leads to more work and increases the number of logging
messages about non-existent chains.
Change-Id: I1cecd39cf5fd046d84a1ef47c245a0a22e9323cb
Partial-bug: #1642770
When deleting vm, the conntrack for this vm is still
exist. This patch deletes removed port's conntrack entries
only in port's hosting node. Another patch [1] removes
this port's conntrack entries in remote hosts.
[1] https://review.openstack.org/#/c/352440/
Closes-Bug: #1580377
Change-Id: Ia8132a62050eaa5e01377f653337b5792f158e2f
Through [1] ipset members are updated in update_security_group_members
instead of updating during firewall apply. In the same way, we will
delete conntrack entries immediately after deleting remote ipset
members(in update_security_group_members) instead of deleting them after
firewall apply.
As explained in [2], this change partially fixes bug #1580377 i.e it
deletes conntrack entries on remote hosts for a removed port.
[1] https://review.openstack.org/#/c/347068/
[2] https://bugs.launchpad.net/neutron/+bug/1580377/comments/13
Co-Authored-By:shihanzhang <shihanzhang@huawei.com>
Partial-Bug: #1580377
Change-Id: Iea3344a24e2a068b794c44796b4c945432379c13
While -m protocol is valid before both source and dest
port arguments, the second is superfluous and can be
removed with a small refactor.
Noticed during a fwaas review.
Change-Id: I4d8bc5bd640be79178b0e948724bef0637b1b0fa
IpConntrackManager class should be a singleton
to be used by both SG and FWaaS v2 API at the same time
Change-Id: I4a9f3d9b3ac7afe989c0efb1fa4e7fd792cd9610
Closes-Bug: 1595515
For new kernels (3.18+), bridge module is split into two pieces: bridge
and br_netfilter. The latter provides firewall support for bridged
traffic, as well as the following sysctl knobs:
* net.bridge.bridge-nf-call-arptables
* net.bridge.bridge-nf-call-ip6tables
* net.bridge.bridge-nf-call-iptables
Before kernel 3.18, any brctl command was loading the 'bridge' module
with the knobs, so at the moment where we reached iptables setup, they
were always available.
With new 3.18+ kernels, brctl still loads 'bridge' module, but not
br_netfilter. So bridge existance no longer guarantees us knobs'
presence. If we reach _enable_netfilter_for_bridges before the new
module is loaded, then the code will fail, triggering agent resync. It
will also fail to enable bridge firewalling on systems where it's
disabled by default (examples of those systems are most if not all Red
Hat/Fedora based systems), making security groups completely
ineffective.
Systems that don't override default settings for those knobs would work
fine except for this exception in the log file and agent resync. This is
because the first attempt to add a iptables rule using 'physdev' module
(-m physdev) will trigger the kernel module loading. In theory, we could
silently swallow missing knobs, and still operate correctly. But on
second thought, it's quite fragile to rely on that implicit module
loading. In the case where we can't detect whether firewall is enabled,
it's better to fail than hope for the best.
An alternative to the proposed path could be trying
to fix broken deployment, meaning we would need to load the missing
kernel module on agent startup. It's not even clear whether we can
assume the operation would be available to us. Even with that, adding a
rootwrap filter to allow loading code in the kernel sounds quite scary.
If we would follow the path, we would also hit an issue of
distinguishing between cases of built-in kernel module vs. modular one.
A complexity that is probably beyond what Neutron should fix.
The patch introduces a sanity check that would fail on missing
configuration knobs.
DocImpact: document the new deployment requirement in operations guide
UpgradeImpact: deployers relying on agents fixing wrong sysctl defaults
will need to make sure bridge firewalling is enabled.
Also, the kernel module providing sysctl knobs must be
loaded before starting the agent, otherwise it will fail
to start.
Depends-On: Id6bfd9595f0772a63d1096ef83ebbb6cd630fafd
Change-Id: I9137ea017624ac92a05f73863b77f9ee4681bbe7
Related-Bug: #1622914
Check if lla,mac tuple is in pairs before appending
it again. Otherwise we end up generating duplicate
iptables rules.
Closes-Bug: #1622938
Change-Id: I43658a31f9853cbc94784f497193210990f769dd
Robert Breker