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
After updating pylint, it started emitting additional "W"
warnings in some cases, fix some of them.
modified-iterating-list,
implicit-str-concat,
global-variable-not-assigned
Trivialfix
Change-Id: I7deb5f1e0aa2852cb033c78dcb4c8bc87e34be1e
After updating pylint, it started emitting additional "R"
warnings in some cases, fix some of them.
use-a-generator,
unnecessary-lambda-assignment,
consider-using-max-builtin,
consider-using-generator,
consider-using-in,
use-list-literal,
consider-using-from-import
Trivialfix
Change-Id: Ife6565cefcc30b4e8a0df9121c9454cf744225df
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
Running with a stricter .pylintrc generates a lot of
C0330 warnings (hanging/continued indentation). Fix
the ones in neutron/agent and neutron/privileged/agent.
Trivialfix
Change-Id: Ib94257481e62b99b3d7648ae5137af5411b4867a
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
As spotted in Focal testing patch [0], pep8 test fails with many
C0321 false-positives, reported in pylint as current version does not
support python 3.8 [1]
Use a newer version of pylint and astroid, fixing or disabling some of
the new checks: no-else-*, unnecessary-comprehension, import-outside-toplevel
[0] https://review.opendev.org/#/c/738163/
[1] https://github.com/PyCQA/pylint/issues/2737
Change-Id: Ie646b7093aa8634fd950c136a0eba9adcf56591c
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
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
Seems that is_enabled_and_bind_by_default() from
neutron.common.ipv6_utils was copied directly into
oslo_utils.netutils, so start using it instead.
Trivialfix
Change-Id: I00fa441e7a20fcd1115485bb8ab75750e6a8cf07
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
Currently, 'icmp', 'ipv6-icmp' and 'icmpv6' can be
specified as an IPv6 ICMP protocol value. This can
lead to duplicate entries in the DB for doing exactly
the same thing.
Change to always be 'ipv6-icmp' so this doesn't happen.
Existing rules using one of the old values will now be
returned with 'ipv6-icmp' as the protocol value.
Depends-on: https://review.opendev.org/660206
Depends-on: https://review.opendev.org/660387
Change-Id: I7cd146691dce1a690e1d2c309dfd54b4a0032f76
Partial-Bug: #1582500
All of the externally consumed variables from neutron.common.constants
now live in neutron-lib. This patch removes neutron.common.constants
and switches all uses over to lib.
NeutronLibImpact
Depends-On: https://review.openstack.org/#/c/647836/
Change-Id: I3c2f28ecd18996a1cee1ae3af399166defe9da87
Almost all of neutron.common.constants is rehomed into
neutron_lib.constants now and as per the discussion in [1] it seems
most folks think the remaining constants should stay in neutron as they
are only used internally within neutron.
This patch moves the neutron only neutron.common.constants into a
private neutron.common._constants. The former will be removed once we
consume the final constants from neutron-lib.
[1] https://review.openstack.org/#/c/647807/
Change-Id: I2d65f8fcfa08984ccf60c4d023f9a9d72b89d79c
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
Reduces E128 warnings by ~260 to just ~900,
no way we're getting rid of all of them at once (or ever).
Files under neutron/tests still have a ton of E128 warnings.
Change-Id: I9137150ccf129bf443e33428267cd4bc9c323b54
Co-Authored-By: Akihiro Motoki <amotoki@gmail.com>
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
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
In corner cases, the firewall code could try and remove
non-existent conntrack zone jump rules if a zone has never
been allocated. This could happen on an agent restart
when there are no longer ports in the zone on the
compute node. Skip the removal since it will just generate
an iptables warning complaining the existing rule does
not exist.
Change-Id: Ie32733b4a06b6d75cf1eb78915a510a4bb78f619
Closes-bug: #1765208
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
Iptables firewall driver can now add process trusted ports and
adds rules for them to FORWARD chain.
Change-Id: I67d0f17b4b56671fc2e2dd6e2fc4518dc42cd131
Closes-Bug: #1720205
The EGRESS_DIRECTION and INGRESS_DIRECTION constants live in neutron-lib
now. This patch removes them from neutron and uses lib's version of
them.
NeutronLibImpact
Change-Id: I1b81f5c3de9e6f2c0967c2db23ddb716ee7ec6b9
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
Somewhere along the way we broke supporting numbers in
the security group API that were not in our known list
of protocols. In order to fix this properly we must
use the correct arguments when using iptables-save, as
it could use a name instead of a number, or vice-versa.
Determined the list of mappings by doing:
for num in {0..255}; do iptables -A INPUT -p $num; done
# iptables-save
Change-Id: I5895250b47ddf664d214cf085be693c3897e0c87
Closes-bug: #1716045
Closes-bug: #1716790
The neutron-lib commit I360545b6ee4291547e0c5c8e668ad03d3efa4725 moved
the externally consumed globals from neutron.common.constants into lib.
With the exception of PROVISIONAL_IPV6_PD_PREFIX all other constants
in neutron.common.constants should only be used in neutron, and will
hopefully remain that way. External consumers needing access to other
common constants should move them into lib first.
NeutronLibImpact
Change-Id: Ie4bcffccf626a6e1de84af01f3487feb825f8b65
Since Pike log messages should not be translated.
This patch removes calls to i18n _LC, _LI, _LE, _LW from
logging logic throughout the code. Translators definition
from neutron._i18n is removed as well.
This patch also removes log translation verification from
ignore directive in tox.ini.
Change-Id: If9aa76fcf121c0e61a7c08088006c5873faee56e
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
updated_rule_sg_ids and updated_sg_members can be updated
concurrently by an RPC security_group_updated cast from the
server which will result in a RuntimeError due to set
size changing during iteration.
This adjusts the logic to just iterate over a copy of the set.
Change-Id: I0a7cf13157de256403cfd6196f64fafdfa65f180
Closes-Bug: #1696874
While reading the iptables_firewall file, I didn't see these
functions being used anywhere in openstack.
If I'm mistaken, simply abandon the patch.
Change-Id: Ifb3e4a6cec8266dfebb1d76ec67c9c569cf9798b
according to https://wiki.openstack.org/wiki/Python3, now we should avoid
using six.iteritems and replace it with dict.items.
Change-Id: I58a399baa2275f280acc0e6d649f81838648ce5c
Closes-Bug: #1680761
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
Those are different on different kernel versions, and have reasonable
default values on all newer kernel versions, including RHEL. We
nevertheless made devstack to set those in the past; now I propose to
clean the code from neutron tree and leave it up to deployment tools to
fix in an unlikely case the system has broken default values.
Now that iptables firewall code does not trigger sysctl, we can also
remove this filter from the corresponding rootwrap .filters file.
DocImpact make sure deployment docs mention the expected sysctl knob
values.
Change-Id: Iabf61021c90b0536be274463d48fb5a572ecc023
Related-Bug: #1622914
When the process is using the IptablesFirewall driver
and is running in namespaces, there is no
/proc/sys/net/bridge in the namespace available and
enable of netfilter for bridge fails with stacktrace
in logs.
This patch handles the exception thrown during a
failed attempted to retrieve net.bridge variable names
and prints an info message in agent logs instead of
printing a stacktrace.
Change-Id: I1ff6cedbf933ac54ef4bbf1d44fc8f57f68d57fc
Closes-bug: 1658343
Robert Breker