Support ipv6 for masquerade and dnat in nspawn and networkd #18007

fw-strlen · 2020-12-17T12:18:40Z

This extends the nft backend to also support IPv6.
networkd and nspawn are extended to expose/masquerade the ipv6 address as well.
Masquerade behavior can be returned to ipv4-only mode via "IPMasquerade=ipv4".
This changes the default behaviour; ipv6 addresses are masqueraded and
ipv6 forwarding is enabled.

In the nspawn case, there is currently no way to turn off the ipv6 dnat exposure;
ideas on syntax to do that welcome.

This would close issue #10254.

poettering

quick!

src/shared/firewall-util-nft.c

src/nspawn/nspawn-expose-ports.c

src/network/networkd-address.c

src/network/networkd-network-gperf.gperf

bluca · 2020-12-17T22:30:20Z

semaphore CI has a package build failure in nss which looks like a transient issue, and mkosi as well looks like some transient repository metadata issue (kicked the latter, don't have permission/see a button to kick the former)

yuwata

I think it is better to split out the address handling for start and end in nft_message_add_setelem_ip6range(), and add several tests for that.

src/shared/firewall-util-nft.c

yuwata · 2020-12-18T07:32:23Z

src/network/networkd-util.c

@@ -53,6 +53,30 @@ DEFINE_CONFIG_PARSE_ENUM(config_parse_link_local_address_family, link_local_addr
                         AddressFamily, "Failed to parse option");
 DEFINE_STRING_TABLE_LOOKUP(dhcp_lease_server_type, sd_dhcp_lease_server_type);

+int config_parse_address_family (


Please use DEFINE_CONFIG_PARSE_ENUM() macro.

I will get rid of this completely, looks like folks don't want auto-masquerade for ipv6 after all.

src/shared/firewall-util-nft.c

andir · 2020-12-25T07:10:16Z

I am not sure defaulting to masquerade IPv6 traffic is a sensible choice when set to true. In most cases you will have plenty of addresses and no need for that on IPv6. This should be two options. One for v4 and one for v6. IPv6Masquerade must default to off.

mweinelt · 2020-12-27T02:36:36Z

I agree that defaulting to IPv6 NAT when coming from older versions of systemd is highly undesirable.

poettering · 2021-01-04T15:21:26Z

I am not sure defaulting to masquerade IPv6 traffic is a sensible choice when set to true. In most cases you will have plenty of addresses and no need for that on IPv6. This should be two options. One for v4 and one for v6. IPv6Masquerade must default to off.

Hmm, I am not entirely sure I agree. ip masquerading isn't just about having too few addresses. It's also about hiding stuff behind a single IP address, i.e. abstracting the used backend routing from the frontend. I'd argue that's totally ok as a reason to have it....

That said, I see the compat problem: so far people assumed "yes" means only "ipv4" here. i.e. i figure i'd be fine if "yes" means "ipv4" here, and to get the behaviour on both ipv4 and ipv6 people should specify "both" here, or so?

Requires special mentioning in the docs, since this is different from other cases of ipv4/ipv6 bools...

andir · 2021-01-05T12:30:04Z

I am not sure defaulting to masquerade IPv6 traffic is a sensible choice when set to true. In most cases you will have plenty of addresses and no need for that on IPv6. This should be two options. One for v4 and one for v6. IPv6Masquerade must default to off.

Hmm, I am not entirely sure I agree. ip masquerading isn't just about having too few addresses. It's also about hiding stuff behind a single IP address, i.e. abstracting the used backend routing from the frontend. I'd argue that's totally ok as a reason to have it....

I am not arguing that having the option to do so is bad. I also use S- & DNAT for some IPv6 setups. And while there are uses for this (e.g. in nspawn when you do not have a subnet available or would rather not use it)

My point is that this is absolutely unexpected to see this happen as part of the IPMasquerade option after upgrading the system.

That said, I see the compat problem: so far people assumed "yes" means only "ipv4" here. i.e. i figure i'd be fine if "yes" means "ipv4" here, and to get the behaviour on both ipv4 and ipv6 people should specify "both" here, or so?

Requires special mentioning in the docs, since this is different from other cases of ipv4/ipv6 bools...

Why not rename IPMasquerade to IPv4Masquerade and add a new option for the IPv6 equivalent? This would remove any kind of upgrade woes and also fix the terminology used within networkd.

poettering · 2021-01-06T19:22:28Z

Why not rename IPMasquerade to IPv4Masquerade and add a new option for the IPv6 equivalent? This would remove any kind of upgrade woes and also fix the terminology used within networkd.

well, we usually avoid having two settings if they control the same stuff just for the two protos. I think we should stick to that, even if we now say that in some very exceptional cases "yes" only means "ipv4" instead of "ipv4+ipv6" like it usually does

fw-strlen · 2021-01-19T11:53:26Z

New version:

factor out part of ipv6 nft code to add test cases for the range element creation.
keep IPMasquerade=yes behavior: its same as IPMasquerade=ipv4. IPMasquerade=both will enable ipv4 and ipv6, =ipv6 just ipv6 masquerade.
nspawn still exposes ipv6 automatically.

src/shared/firewall-util-nft.c

man/systemd.network.xml

src/network/networkd-address.c

src/test/test-firewall-util.c

closely mirrors the existing ipv4 ruleset: table ip6 io.systemd.nat { set masq_saddr { type ipv6_addr flags interval } map map_port_ipport { type inet_proto . inet_service : ipv6_addr . inet_service } chain prerouting { type nat hook prerouting priority dstnat + 1; policy accept; fib daddr type local dnat ip6 addr . port to meta l4proto . th dport map @map_port_ipport } chain output { type nat hook output priority -99; policy accept; ip6 daddr != ::1 oif "lo" dnat ip6 addr . port to meta l4proto . th dport map @map_port_ipport } chain postrouting { type nat hook postrouting priority srcnat + 1; policy accept; ip6 saddr @masq_saddr masquerade } } Only difference is the use of ipv6 addresses instead of ipv4 ones. Currently has no effect: all in-tree callers pass AF_INET exclusively. Followup patches will make nspawn expose ipv6 too and rework IPMasquerade option to support both/v4/v6.

Extend nspawn so it can keep track of one ipv4 and one ipv6 address.

Extend IPMasquerade to also cover ipv6. For compatibility reasons with earlier releases IPMasquerade=yes is identical to IPMasquerade=ipv4. Use IPMasquerade=both to cover ipv6 as well as ipv4. IPForward will now also enable ipv6 forwarding if IPMasquerade for ipv6 is enabled.

In case external entity wiped the ruleset, we need to clear the 'previous' address -- its already gone. This prevents the transaction from succeeding: the delete operation fails.

The nft set backend doesn't support network masks, it works with ranges. Inputs like dead::/64 thus need to be translated to two 'start' and 'end' elements. The 'start' element is the first element in the range (i.e., dead::). The 'stop' element is the first element *past* the range, (dead:0:0100:: in the example). This adds a few test cases.

poettering · 2021-02-16T22:41:28Z

lgtm

yuwata · 2021-02-17T12:06:44Z

@fw-strlen I am sorry that I forgot to review this. I created two follow-up PRs. PTAL.

keszybz · 2021-02-17T18:07:29Z

I know I'm late to the party, but I'm not convinced the decision to make "yes"=="ipv4" good long term. I understand backwards compat is important, but I'd expect most people want to do both if they want at least one. (Or at least in a majority of simple cases where you just turn on masquerading without any additional configuration.) I think we should start emitting a warning if "yes" is used, to move people off the option with surprising semantics.

github-actions bot added documentation network labels Dec 17, 2020

poettering requested changes Dec 17, 2020

View reviewed changes

poettering added the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Dec 17, 2020

poettering linked an issue Dec 17, 2020 that may be closed by this pull request

systemd-nspawn should support IPv6 too for exposing ports (currently it only does ipv4) #10254

Closed

fw-strlen force-pushed the ipv6_masq_and_dnat branch from 5f1ccbf to 21adc2c Compare December 17, 2020 18:08

bluca added needs-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Dec 17, 2020

yuwata requested changes Dec 18, 2020

View reviewed changes

yuwata added nspawn reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Dec 18, 2020

bluca removed the needs-review label Dec 18, 2020

poettering added the needs-rebase label Dec 18, 2020

poettering requested changes Dec 18, 2020

View reviewed changes

src/shared/firewall-util-nft.c Outdated Show resolved Hide resolved

src/shared/firewall-util-nft.c Show resolved Hide resolved

fw-strlen force-pushed the ipv6_masq_and_dnat branch from 21adc2c to 159a35e Compare January 19, 2021 11:49

github-actions bot removed the documentation label Jan 19, 2021

yuwata added needs-review and removed needs-rebase reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 19, 2021

poettering requested changes Jan 19, 2021

View reviewed changes

poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed needs-review labels Jan 19, 2021

Florian Westphal added 4 commits January 19, 2021 16:22

nspawn: expose container ipv6 address too

deff68e

Extend nspawn so it can keep track of one ipv4 and one ipv6 address.

firewall-util-nft: clear previous address on replay

f4fca22

In case external entity wiped the ruleset, we need to clear the 'previous' address -- its already gone. This prevents the transaction from succeeding: the delete operation fails.

fw-strlen force-pushed the ipv6_masq_and_dnat branch from 159a35e to 79dd224 Compare January 19, 2021 15:42

yuwata added needs-review and removed reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks labels Jan 19, 2021

Base automatically changed from master to main January 21, 2021 11:55

poettering merged commit b1b4e92 into systemd:main Feb 16, 2021

poettering removed the needs-review label Feb 16, 2021

This was referenced Feb 17, 2021

network: several follow-ups for recent IPMasquerade= change #18646

Merged

in-addr-util: fix in_addr_prefix_nth() and use the function to get ip address range in firewall-util #18653

Merged

yuwata mentioned this pull request May 5, 2021

IPMasquerade not work with IPv6 #14909

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support ipv6 for masquerade and dnat in nspawn and networkd #18007

Support ipv6 for masquerade and dnat in nspawn and networkd #18007

fw-strlen commented Dec 17, 2020

poettering left a comment

bluca commented Dec 17, 2020

yuwata left a comment

yuwata Dec 18, 2020

fw-strlen Jan 4, 2021

andir commented Dec 25, 2020

mweinelt commented Dec 27, 2020

poettering commented Jan 4, 2021

andir commented Jan 5, 2021

poettering commented Jan 6, 2021

fw-strlen commented Jan 19, 2021

poettering commented Feb 16, 2021

yuwata commented Feb 17, 2021

keszybz commented Feb 17, 2021 •

edited

Support ipv6 for masquerade and dnat in nspawn and networkd #18007

Support ipv6 for masquerade and dnat in nspawn and networkd #18007

Conversation

fw-strlen commented Dec 17, 2020

poettering left a comment

Choose a reason for hiding this comment

bluca commented Dec 17, 2020

yuwata left a comment

Choose a reason for hiding this comment

yuwata Dec 18, 2020

Choose a reason for hiding this comment

fw-strlen Jan 4, 2021

Choose a reason for hiding this comment

andir commented Dec 25, 2020

mweinelt commented Dec 27, 2020

poettering commented Jan 4, 2021

andir commented Jan 5, 2021

poettering commented Jan 6, 2021

fw-strlen commented Jan 19, 2021

poettering commented Feb 16, 2021

yuwata commented Feb 17, 2021

keszybz commented Feb 17, 2021 • edited

keszybz commented Feb 17, 2021 •

edited