From 025ff59341c230b9f782da17f1a1202f785e779c Mon Sep 17 00:00:00 2001 From: Jan Gilla Date: Thu, 8 Oct 2026 11:37:37 +0200 Subject: [PATCH 1/3] F #-: VRouter: optional SNAT to the VIP or to a given address in NAT4 NAT4 masquerades the traffic leaving through the selected NICs, so the outside address is the primary address of the NIC of the node that happens to forward it, and it changes with a failover. Add two opt-in ways to get a stable source address; with neither set the rules are unchanged. * ONEAPP_VNF_NAT4_SNAT_ADDRESS: SNAT every outgoing NIC to this address. For example a public address that is statically routed to the VIP: the address does not have to be configured on the router for the NAT to work (the replies come back through the route and are translated back), but to reach it from outside it can be added as an extra VIP (ONEAPP_VROUTER_ETH_VIP=
/32), which puts it on the VRRP master only. * ONEAPP_VNF_NAT4_SNAT_TO_VIP: for a NIC that has a VIP, SNAT to the first IPv4 VIP of the NIC in index order (the OpenNebula floating IP counts). IPv6 VIPs are ignored. The address wins over the VIP. The address ends up in a shell command, so anything but a plain dotted quad is rejected (a prefix length and surrounding spaces are stripped): the outgoing NICs then keep MASQUERADE, also when SNAT to the VIP is enabled, and the problem is logged with the attribute or the NIC (at most 40 characters of the value are logged). A NIC without a VIP keeps MASQUERADE. Checked with unit tests, and on a kernel with three network namespaces (client, router, server): with SNAT to an address the server sees the client as that address, with and without the address configured on the router, and the replies are translated back; with MASQUERADE it sees the NIC address of the router. Signed-off-by: Jan Gilla --- appliances/VRouter/NAT4/execute.rb | 8 +- appliances/VRouter/NAT4/main.rb | 51 +++++++ appliances/VRouter/NAT4/tests.rb | 205 +++++++++++++++++++++++++++++ 3 files changed, 263 insertions(+), 1 deletion(-) diff --git a/appliances/VRouter/NAT4/execute.rb b/appliances/VRouter/NAT4/execute.rb index bc28d161..d59380a0 100644 --- a/appliances/VRouter/NAT4/execute.rb +++ b/appliances/VRouter/NAT4/execute.rb @@ -46,12 +46,18 @@ def execute end unless nat4_vars[:masq].empty? - # Add MASQUERADE rules. + snat = snat_sources nat4_vars[:masq] + + # Add MASQUERADE rules (SNAT to an address or to the VIP, if configured). bash ERB.new(<<~IPTABLES, trim_mode: '-').result(binding) iptables -t nat -F NAT4-MASQ <%- nat4_vars[:masq].each do |nic| -%> + <%- if snat.key?(nic) -%> + iptables -t nat -A NAT4-MASQ -o '<%= nic %>' -j SNAT --to-source '<%= snat[nic] %>' + <%- else -%> iptables -t nat -A NAT4-MASQ -o '<%= nic %>' -j MASQUERADE <%- end -%> + <%- end -%> IPTABLES end diff --git a/appliances/VRouter/NAT4/main.rb b/appliances/VRouter/NAT4/main.rb index a6940c51..4684a9d0 100644 --- a/appliances/VRouter/NAT4/main.rb +++ b/appliances/VRouter/NAT4/main.rb @@ -14,6 +14,10 @@ module NAT4 ONEAPP_VNF_NAT4_INTERFACES_OUT = env :ONEAPP_VNF_NAT4_INTERFACES_OUT, nil # nil -> none, empty -> all + ONEAPP_VNF_NAT4_SNAT_TO_VIP = env :ONEAPP_VNF_NAT4_SNAT_TO_VIP, 'NO' # YES -> SNAT to the VIP instead of MASQUERADE + + ONEAPP_VNF_NAT4_SNAT_ADDRESS = env :ONEAPP_VNF_NAT4_SNAT_ADDRESS, '' # SNAT to this address instead of MASQUERADE + def parse_env @interfaces_out ||= parse_interfaces ONEAPP_VNF_NAT4_INTERFACES_OUT @mgmt ||= detect_mgmt_nics @@ -59,6 +63,53 @@ def parse_env end end + IPV4_RE = /\A(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\z/ + + # NOTE: NIC -> source address, for the NICs that are not masqueraded. + # ONEAPP_VNF_NAT4_SNAT_ADDRESS (one address for every outgoing NIC, for example a public address + # routed to the VIP) wins over ONEAPP_VNF_NAT4_SNAT_TO_VIP (the first IPv4 VIP of the NIC). + # The address ends up in a shell command, so anything but a plain dotted quad is rejected + # (the NIC then keeps MASQUERADE, also an invalid address does not fall back to the VIP). + def snat_sources(nics) + nics = nics.grep(/\Aeth\d+\z/) + + return snat_address_sources(nics) unless ONEAPP_VNF_NAT4_SNAT_ADDRESS.to_s.strip.empty? + return {} unless ONEAPP_VNF_NAT4_SNAT_TO_VIP + + snat_vip_sources(nics) + end + + def snat_address_sources(nics) + addr = ONEAPP_VNF_NAT4_SNAT_ADDRESS.strip.split(%[/])[0].to_s + + unless IPV4_RE.match?(addr) + msg :error, "NAT4::snat_sources: invalid ONEAPP_VNF_NAT4_SNAT_ADDRESS #{addr[0, 40].inspect}, using MASQUERADE" + return {} + end + + nics.each_with_object({}) do |nic, acc| + acc[nic] = addr + msg :info, "NAT4::snat_sources: #{nic} -> #{addr} (ONEAPP_VNF_NAT4_SNAT_ADDRESS)" + end + end + + # NOTE: IPv6 VIPs (address with a ':') are ignored. + def snat_vip_sources(nics) + detect_vips.slice(*nics).each_with_object({}) do |(nic, vips), acc| + name, vip = vips.reject { |_, v| v.to_s.include?(':') }.min_by { |k, _| k[/_VIP(\d+)$/, 1].to_i } + next if vip.nil? + + addr = vip.split(%[/])[0].to_s + unless IPV4_RE.match?(addr) + msg :error, "NAT4::snat_sources: #{nic}: invalid #{name} #{addr[0, 40].inspect}, using MASQUERADE" + next + end + + acc[nic] = addr + msg :info, "NAT4::snat_sources: #{nic} -> #{addr} (#{name})" + end + end + def install(initdir: '/etc/init.d') msg :info, 'NAT4::install' diff --git a/appliances/VRouter/NAT4/tests.rb b/appliances/VRouter/NAT4/tests.rb index b142faa4..0d939f8b 100644 --- a/appliances/VRouter/NAT4/tests.rb +++ b/appliances/VRouter/NAT4/tests.rb @@ -74,4 +74,209 @@ def clear_vars(object) masq: %w[eth1] }) end + + describe 'SNAT to the VIP' do + def masq_rules(snat: nil, address: nil, out: 'eth0,eth1') + clear_env + + ENV['ONEAPP_VNF_NAT4_ENABLED'] = 'YES' + ENV['ONEAPP_VNF_NAT4_INTERFACES_OUT'] = out + ENV['ONEAPP_VNF_NAT4_SNAT_TO_VIP'] = snat unless snat.nil? + ENV['ONEAPP_VNF_NAT4_SNAT_ADDRESS'] = address unless address.nil? + + ENV['ETH0_IP'] = '14.15.16.17' + ENV['ETH0_MASK'] = '255.255.255.0' + ENV['ETH1_IP'] = '192.168.1.2' + ENV['ETH1_MASK'] = '255.255.255.0' + + yield if block_given? + + load './main.rb'; include Service::NAT4 + + clear_vars Service::NAT4 + + scripts = @scripts = [] + @errors = [] + allow(Service::NAT4).to receive(:bash) { |script| scripts << script; '' } + allow(Service::NAT4).to receive(:toggle) + allow(Service::NAT4).to receive(:msg) { |level, text| @errors << text if level == :error } + Service::NAT4.execute + + scripts.flat_map(&:lines).map(&:strip).grep(/NAT4-MASQ -o/) + end + + it 'should render MASQUERADE when the attribute is unset' do + rules = masq_rules { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(rules).to eq [ + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE", + "iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE" + ] + end + + it 'should render MASQUERADE when the attribute is NO' do + rules = masq_rules(snat: 'NO') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(rules).to eq [ + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE", + "iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE" + ] + end + + it 'should render SNAT with the ONEAPP_VROUTER_ETH_VIP address' do + rules = masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.86'"] + end + + it 'should render SNAT with the ETH_VROUTER_IP floating IP' do + rules = masq_rules(snat: 'YES', out: 'eth0') { ENV['ETH0_VROUTER_IP'] = '14.15.16.99' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.99'"] + end + + it 'should ignore an IPv6 VIP at a lower index and SNAT to the IPv4 one' do + rules = masq_rules(snat: 'YES', out: 'eth0') do + ENV['ONEAPP_VROUTER_ETH0_VIP0'] = 'fd77::f0/64' + ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '203.0.113.10/24' + end + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'"] + end + + it 'should ignore the IPv6 floating IP and SNAT to the IPv4 VIP' do + rules = masq_rules(snat: 'YES', out: 'eth0') do + ENV['ETH0_VROUTER_IP6'] = 'fd77::99' + ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '203.0.113.10' + end + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'"] + end + + it 'should keep MASQUERADE when a NIC has only IPv6 VIPs' do + rules = masq_rules(snat: 'YES', out: 'eth0') do + ENV['ETH0_VROUTER_IP6'] = 'fd77::99' + ENV['ONEAPP_VROUTER_ETH0_VIP0'] = 'fd77::f0/64' + end + + expect(rules.join).to include('MASQUERADE') + expect(rules.join).not_to include('SNAT') + end + + it 'should fall back to MASQUERADE for a NIC without a VIP' do + rules = masq_rules(snat: 'YES', out: 'eth1') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE"] + end + + it 'should mix SNAT and MASQUERADE per NIC' do + rules = masq_rules(snat: 'YES') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(rules).to eq [ + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.86'", + "iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE" + ] + end + + it 'should use the first VIP of a NIC in index order' do + rules = masq_rules(snat: 'YES', out: 'eth0') do + ENV['ONEAPP_VROUTER_ETH0_VIP2'] = '14.15.16.88' + ENV['ONEAPP_VROUTER_ETH0_VIP1'] = '14.15.16.87' + end + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.87'"] + end + + it 'should strip a prefix length from the VIP' do + rules = masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86/24' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.86'"] + end + + it 'should quote the address in the SNAT rule' do + expect(masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' }) + .to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '14.15.16.86'"] + end + + # NOTE: stock detect_vips needs a parseable address, so the hostile values carry an explicit /24. + [ + '1.2.3.4; reboot', '1.2.3.4 -j ACCEPT', '$(id)', '`id`', "1.2.3.4\nfoo", + '999.1.1.1', '1.2.3', '01.2.3.4', '1.2.3.4.5', ' 1.2.3.4', "1.2.3.4\n" + ].each do |bad| + it "should reject the VIP #{bad.inspect} and fall back to MASQUERADE" do + rules = masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = "#{bad}/24" } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE"] + expect(@scripts.join).not_to include('reboot', 'ACCEPT', '$(id)', '`id`', 'foo', '999') + end + end + + it 'should log an error naming the NIC for an invalid VIP' do + masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '1.2.3.4; reboot/24' } + + expect(@errors.join).to include('eth0') + end + + it 'should use the VIP when only a garbage prefix length is given' do + rules = masq_rules(snat: 'YES', out: 'eth0') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '1.2.3.4/abc' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '1.2.3.4'"] + end + + context 'with ONEAPP_VNF_NAT4_SNAT_ADDRESS' do + it 'should SNAT every outgoing NIC to the address, without any VIP' do + rules = masq_rules(address: '203.0.113.10') + + expect(rules).to eq [ + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'", + "iptables -t nat -A NAT4-MASQ -o 'eth1' -j SNAT --to-source '203.0.113.10'" + ] + end + + it 'should only touch the NICs listed as outgoing' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth1' -j SNAT --to-source '203.0.113.10'"] + end + + it 'should win over the VIP when SNAT to the VIP is enabled too' do + rules = masq_rules(snat: 'YES', address: '203.0.113.10', out: 'eth0') do + ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' + end + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'"] + end + + it 'should render MASQUERADE when the attribute is empty' do + expect(masq_rules(address: '', out: 'eth0')) + .to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE"] + end + + it 'should strip a prefix length and surrounding spaces' do + expect(masq_rules(address: ' 203.0.113.10/32 ', out: 'eth0')) + .to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'"] + end + + [ + '1.2.3.4; reboot', '1.2.3.4 -j ACCEPT', '$(id)', '`id`', "1.2.3.4\nfoo", + '999.1.1.1', '1.2.3', '01.2.3.4', '1.2.3.4.5', '2001:db8::1', 'eth0' + ].each do |bad| + it "should reject #{bad.inspect} and keep MASQUERADE, not the VIP" do + rules = masq_rules(snat: 'YES', address: bad, out: 'eth0') do + ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' + end + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE"] + expect(@scripts.join).not_to include('reboot', 'ACCEPT', '$(id)', '`id`', 'foo', '999') + end + end + + it 'should log an error that names the attribute, with at most 40 characters of the value' do + masq_rules(address: "x#{'y' * 80}", out: 'eth0') + + expect(@errors.join).to include('ONEAPP_VNF_NAT4_SNAT_ADDRESS') + expect(@errors.join).not_to include('y' * 41) + end + end + end end From 69782985d2dcae526e294b9eed2966ab127ab05e Mon Sep 17 00:00:00 2001 From: Jan Gilla Date: Fri, 16 Oct 2026 15:06:03 +0200 Subject: [PATCH 2/3] F #-: VRouter: do not SNAT locally generated traffic in NAT4 SNAT rewrote the VRRP advertisements the router sends itself to the NAT address, so both nodes appeared with the same source and kept the VIPs. Signed-off-by: Jan Gilla --- appliances/VRouter/NAT4/execute.rb | 3 ++ appliances/VRouter/NAT4/tests.rb | 48 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/appliances/VRouter/NAT4/execute.rb b/appliances/VRouter/NAT4/execute.rb index d59380a0..197592e3 100644 --- a/appliances/VRouter/NAT4/execute.rb +++ b/appliances/VRouter/NAT4/execute.rb @@ -51,6 +51,9 @@ def execute # Add MASQUERADE rules (SNAT to an address or to the VIP, if configured). bash ERB.new(<<~IPTABLES, trim_mode: '-').result(binding) iptables -t nat -F NAT4-MASQ + <%- unless snat.empty? -%> + iptables -t nat -A NAT4-MASQ -m addrtype --src-type LOCAL -j RETURN + <%- end -%> <%- nat4_vars[:masq].each do |nic| -%> <%- if snat.key?(nic) -%> iptables -t nat -A NAT4-MASQ -o '<%= nic %>' -j SNAT --to-source '<%= snat[nic] %>' diff --git a/appliances/VRouter/NAT4/tests.rb b/appliances/VRouter/NAT4/tests.rb index 0d939f8b..b6a5c6a2 100644 --- a/appliances/VRouter/NAT4/tests.rb +++ b/appliances/VRouter/NAT4/tests.rb @@ -278,5 +278,53 @@ def masq_rules(snat: nil, address: nil, out: 'eth0,eth1') expect(@errors.join).not_to include('y' * 41) end end + + # NOTE: SNAT rewrites everything that leaves the NIC, including the packets the router sends itself. + # VRRP advertisements would then carry the NAT address as source for both nodes, so neither + # can win the election, and both keep the VIPs. + context 'with locally generated traffic' do + let(:return_local) { 'iptables -t nat -A NAT4-MASQ -m addrtype --src-type LOCAL -j RETURN' } + + def all_rules + @scripts.flat_map(&:lines).map(&:strip).grep(/-A NAT4-MASQ/) + end + + it 'should not SNAT packets with a local source address' do + masq_rules(address: '203.0.113.10', out: 'eth0') + + expect(all_rules).to eq [ + return_local, + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j SNAT --to-source '203.0.113.10'" + ] + end + + it 'should return local traffic before any SNAT rule when SNAT to the VIP is used' do + masq_rules(snat: 'YES') { ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '14.15.16.86' } + + expect(all_rules.first).to eq return_local + expect(all_rules.index { |r| r.include?('SNAT --to-source') }).to be > 0 + end + + it 'should render the local return rule once, not once per NIC' do + masq_rules(address: '203.0.113.10') + + expect(all_rules.count(return_local)).to eq 1 + end + + it 'should leave the MASQUERADE rules unchanged' do + masq_rules(out: 'eth0,eth1') + + expect(all_rules).to eq [ + "iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE", + "iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE" + ] + end + + it 'should not add the local return rule when an invalid address keeps MASQUERADE' do + masq_rules(address: '1.2.3.4; reboot', out: 'eth0') + + expect(all_rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth0' -j MASQUERADE"] + end + end end end From d1e7d7e549a6608b1bb45f4564c89cd5e3fc56a8 Mon Sep 17 00:00:00 2001 From: Jan Gilla Date: Tue, 20 Oct 2026 23:05:03 +0200 Subject: [PATCH 3/3] F #-: VRouter: own the NAT4 SNAT address as /32 and serve load balancers on it The SNAT address is added as an extra Keepalived virtual_ipaddress /32 on the first outgoing NIC that gets a vrrp_instance, so only the MASTER owns it and the upstream route to the VIP does not loop. LVS serves load balancers on that address. NAT4 SNATs only to an address the router owns and can use (not inside a NIC subnet, not an alias or VIP) and falls back to MASQUERADE otherwise. Keepalived, LVS and NAT4 share one helper. Signed-off-by: Jan Gilla --- appliances/VRouter/Keepalived/main.rb | 19 +++ appliances/VRouter/Keepalived/tests.rb | 197 +++++++++++++++++++++++++ appliances/VRouter/LVS/execute.rb | 8 +- appliances/VRouter/LVS/tests.rb | 115 +++++++++++++++ appliances/VRouter/NAT4/main.rb | 32 +++- appliances/VRouter/NAT4/tests.rb | 116 ++++++++++++++- appliances/VRouter/vrouter.rb | 78 ++++++++++ 7 files changed, 559 insertions(+), 6 deletions(-) diff --git a/appliances/VRouter/Keepalived/main.rb b/appliances/VRouter/Keepalived/main.rb index 7dfdf0c9..17d13640 100644 --- a/appliances/VRouter/Keepalived/main.rb +++ b/appliances/VRouter/Keepalived/main.rb @@ -35,6 +35,7 @@ def parse_env(default_vrid = ONEAPP_VNF_KEEPALIVED_VRID) vrid: env("ONEAPP_VNF_KEEPALIVED_#{nic.upcase}_VRID", default_vrid), skip: env("ONEAPP_VNF_KEEPALIVED_#{nic.upcase}_SKIP", 'NO'), vips: @vips[nic]&.values || [], + extra_vips: [], noip: !@nics.include?(nic), gw: env("#{nic.upcase}_GATEWAY", ''), gw_default: false @@ -47,10 +48,25 @@ def parse_env(default_vrid = ONEAPP_VNF_KEEPALIVED_VRID) unless ((nic, _) = vars[:by_nic].find { |_, opt| opt[:noip] && !opt[:gw].empty? }).nil? vars[:by_nic][nic][:gw_default] = true end + unless ((nic, addr) = snat_address).nil? + vars[:by_nic][nic][:extra_vips] << "#{addr}/32" + end vars end end + # NOTE: ONEAPP_VNF_NAT4_SNAT_ADDRESS is usually not configured on the router (a public address routed by + # the upstream to the VIP). A packet for it without a conntrack entry would be forwarded and loop + # between the router and the upstream. The address is therefore a /32 virtual address of the first + # NAT4 outgoing NIC that gets a VRRP instance (not skipped, not floating only): Keepalived adds it on the MASTER only, removes it on BACKUP / fault + # / stop, and re-renders it when the value changes. It is not part of the real VIPs (detect_vips) + # and is ignored (with a warning) when it is inside a NIC subnet or already a VIP, NIC or alias address. Returns [nic, address] or nil. + def snat_address + nat4_snat_address(@mgmt).tap do |nic, addr| + msg :info, "Keepalived::snat_address: #{addr}/32 -> #{nic}" unless nic.nil? + end + end + def install(initdir: '/etc/init.d') msg :info, 'Keepalived::install' @@ -142,6 +158,9 @@ def configure(basedir: '/etc/keepalived') <%- opt[:vips].compact.reject(&:empty?).each do |vip| -%> <%= vip %> dev <%= nic.downcase %> <%- end -%> + <%- opt[:extra_vips].each do |vip| -%> + <%= vip %> dev <%= nic.downcase %> + <%- end -%> <%- end -%> }<%- -%> diff --git a/appliances/VRouter/Keepalived/tests.rb b/appliances/VRouter/Keepalived/tests.rb index 29ad5356..bca68dd1 100644 --- a/appliances/VRouter/Keepalived/tests.rb +++ b/appliances/VRouter/Keepalived/tests.rb @@ -303,4 +303,201 @@ def clear_vars(object) expect(result.strip).to eq output.strip end end + + # NOTE: ONEAPP_VNF_NAT4_SNAT_ADDRESS is not configured on the router. Keepalived owns it as a /32 + # virtual address on the first outgoing NIC, so it is present on the VRRP MASTER only. + describe 'NAT4 SNAT address' do + let(:addr) { '203.0.113.10' } + + def vrrp_conf(address: nil, out: 'eth1', enabled: 'YES', &block) + clear_env + + ENV['ONEGATE_ENDPOINT'] = 'http://127.0.0.1:5030' + ENV['ONEAPP_VNF_KEEPALIVED_INTERFACES'] = 'eth0 eth1' + ENV['ONEAPP_VNF_KEEPALIVED_ETH0_VRID'] = '30' + ENV['ONEAPP_VNF_KEEPALIVED_ETH1_VRID'] = '30' + ENV['ETH0_IP'] = '10.2.30.2' + ENV['ETH1_IP'] = '10.2.31.2' + ENV['ONEAPP_VROUTER_ETH0_VIP0'] = '10.2.30.69' + ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '10.2.31.69' + + ENV['ONEAPP_VNF_NAT4_ENABLED'] = enabled + ENV['ONEAPP_VNF_NAT4_INTERFACES_OUT'] = out unless out.nil? + ENV['ONEAPP_VNF_NAT4_SNAT_ADDRESS'] = address unless address.nil? + + block&.call + + load './main.rb'; include Service::Keepalived + + allow(OneGate.instance).to receive(:service_show).and_return(nil) + allow(Service::Keepalived).to receive(:ip_link_set_up).and_return(nil) + allow(Service::Keepalived).to receive(:detect_nics).and_return(%w[eth0 eth1]) + allow(Service::Keepalived).to receive(:toggle).and_return(nil) + + clear_vars Service::Keepalived + + Dir.mktmpdir do |dir| + Service::Keepalived.configure basedir: dir + return File.read("#{dir}/conf.d/vrrp.conf") + end + end + + def vip_lines(conf) + conf[/virtual_ipaddress \{\n(.*?)\n\s*\}/m, 1].lines.map(&:strip) + end + + it 'should render the address as /32 after the VIPs, on the outgoing NIC' do + expect(vip_lines(vrrp_conf(address: addr))).to eq [ + '10.2.30.69/8 dev eth0', '10.2.31.69/8 dev eth1', "#{addr}/32 dev eth1" + ] + end + + it 'should always use /32, whatever prefix length is given' do + expect(vip_lines(vrrp_conf(address: "#{addr}/24"))).to include "#{addr}/32 dev eth1" + end + + it 'should render a single copy on the first outgoing NIC' do + conf = vrrp_conf(address: addr, out: 'eth1,eth0') + + expect(conf.scan(addr).count).to eq 1 + expect(vip_lines(conf)).to include "#{addr}/32 dev eth0" + end + + it 'should leave the real VIPs untouched in the parsed vars' do + vrrp_conf(address: addr) + + expect(Service::Keepalived.parse_env[:by_nic]['eth1'][:vips]).to eq ['10.2.31.69/8'] + end + + it 'should render nothing when the attribute is unset' do + expect(vrrp_conf).not_to include '/32' + end + + it 'should render nothing when NAT4 is disabled' do + expect(vrrp_conf(address: addr, enabled: 'NO')).not_to include addr + end + + it 'should render nothing when NAT4 has no outgoing NIC' do + expect(vrrp_conf(address: addr, out: nil)).not_to include addr + end + + it 'should render nothing for a NIC that has no VRRP instance' do + expect(vrrp_conf(address: addr, out: 'eth5')).not_to include addr + end + + it 'should not use a management NIC' do + conf = vrrp_conf(address: addr, out: 'eth0,eth1') { ENV['ETH0_VROUTER_MANAGEMENT'] = 'YES' } + + expect(vip_lines(conf)).to include "#{addr}/32 dev eth1" + end + + ['1.2.3.4; reboot', '999.1.1.1', '2001:db8::1', 'eth0', '0.0.0.0', '127.0.0.1', '224.0.0.1', + '240.0.0.1', '255.255.255.255'].each do |bad| + it "should render nothing for #{bad.inspect}" do + expect(vip_lines(vrrp_conf(address: bad))).to eq ['10.2.30.69/8 dev eth0', '10.2.31.69/8 dev eth1'] + end + end + + it 'should not duplicate an address that is already a VIP' do + conf = vrrp_conf(address: '10.2.31.69') + + expect(conf.scan('10.2.31.69').count).to eq 1 + end + + it 'should not duplicate an address that is a NIC address' do + expect(vrrp_conf(address: '10.2.31.2')).not_to include '10.2.31.2' + end + + context 'when the outgoing NIC gets no VRRP instance' do + def eth_vrids(vrid0, vrid1) + ENV['ONEAPP_VNF_KEEPALIVED_ETH0_VRID'] = vrid0 + ENV['ONEAPP_VNF_KEEPALIVED_ETH1_VRID'] = vrid1 + end + + it 'should skip a NIC with SKIP=YES that is alone in its VRID group' do + conf = vrrp_conf(address: addr) { eth_vrids('30', '31'); ENV['ONEAPP_VNF_KEEPALIVED_ETH1_SKIP'] = 'YES' } + + expect(conf).not_to include addr + end + + it 'should use a later eligible NIC when the first outgoing NIC is skipped' do + conf = vrrp_conf(address: addr, out: 'eth0,eth1') { ENV['ONEAPP_VNF_KEEPALIVED_ETH0_SKIP'] = 'YES' } + + expect(vip_lines(conf)).to include "#{addr}/32 dev eth1" + expect(conf.scan(addr).count).to eq 1 + end + + it 'should use a later NIC in another VRID group when the first is skipped' do + conf = vrrp_conf(address: addr, out: 'eth0,eth1') { eth_vrids('30', '31'); ENV['ONEAPP_VNF_KEEPALIVED_ETH0_SKIP'] = 'YES' } + + expect(vip_lines(conf.split('vrrp_instance ').last)).to include "#{addr}/32 dev eth1" + end + + it 'should not use a floating-only NIC (no IP)' do + conf = vrrp_conf(address: addr, out: 'eth1') { ENV.delete('ETH1_IP') } + + expect(conf).not_to include addr + end + + it 'should use the next NIC with an IP when the first outgoing NIC is floating-only' do + conf = vrrp_conf(address: addr, out: 'eth0,eth1') { ENV.delete('ETH0_IP') } + + expect(vip_lines(conf)).to include "#{addr}/32 dev eth1" + end + + it 'should use the first outgoing NIC when each NIC has its own VRID' do + conf = vrrp_conf(address: addr, out: 'eth0,eth1') { eth_vrids('30', '31') } + + expect(conf.scan(addr).count).to eq 1 + expect(conf).to match(/vrrp_instance ETH0 \{.*#{addr}\/32 dev eth0.*vrrp_instance ETH1/m) + end + end + + context 'when the address is unusable' do + it 'should render nothing for an address inside the subnet of a NIC' do + expect(vrrp_conf(address: '10.9.9.9')).not_to include '10.9.9.9' + end + + it 'should render nothing for an address inside the subnet of a non-outgoing NIC' do + conf = vrrp_conf(address: '192.168.5.7', out: 'eth1') do + ENV['ETH0_IP'] = '192.168.5.2' + ENV['ETH0_MASK'] = '255.255.255.0' + end + + expect(conf).not_to include '192.168.5.7' + end + + it 'should render nothing for an address that is an alias of a NIC' do + expect(vrrp_conf(address: addr) { ENV['ETH1_ALIAS0_IP'] = addr }).not_to include addr + end + + it 'should render nothing for an address inside the subnet of an alias with a mask' do + conf = vrrp_conf(address: '172.20.0.9') do + ENV['ETH1_ALIAS0_IP'] = '172.20.0.2' + ENV['ETH1_ALIAS0_MASK'] = '255.255.255.0' + end + + expect(conf).not_to include '172.20.0.9' + end + + it 'should render a valid address next to unrelated aliases' do + conf = vrrp_conf(address: addr) { ENV['ETH1_ALIAS0_IP'] = '198.51.100.5' } + + expect(vip_lines(conf)).to include "#{addr}/32 dev eth1" + end + + it 'should not raise on odd alias values and render nothing' do + conf = nil + expect do + conf = vrrp_conf(address: addr) do + ENV['ETH1_ALIAS0_IP'] = '198.51.100.5' + ENV['ETH1_ALIAS0_MASK'] = 'bogus' + end + end.not_to raise_error + + expect(conf).not_to include addr + expect(vip_lines(conf)).to eq ['10.2.30.69/8 dev eth0', '10.2.31.69/8 dev eth1'] + end + end + end end diff --git a/appliances/VRouter/LVS/execute.rb b/appliances/VRouter/LVS/execute.rb index f2b1afd2..d6db577f 100644 --- a/appliances/VRouter/LVS/execute.rb +++ b/appliances/VRouter/LVS/execute.rb @@ -32,8 +32,14 @@ def render_lvs_conf(lvs_vars, basedir: '/etc/keepalived') @interfaces ||= parse_interfaces ONEAPP_VNF_LB_INTERFACES @mgmt ||= detect_mgmt_nics + # NOTE: The NAT4 SNAT address is also a valid virtual IP (a /32 that Keepalived adds on the MASTER only). + # The VRRP NICs come from the same helper as in Keepalived (keepalived_vrrp_nics, from the + # environment only), so this does not depend on the Keepalived or NAT4 modules being loaded. + @snat ||= nat4_snat_address(@mgmt) + @allowed ||= addrs_to_nics(@interfaces.keys - @mgmt).keys + - detect_vips.values.map(&:values).flatten.map { |v| v.split(%[/])[0] } + detect_vips.values.map(&:values).flatten.map { |v| v.split(%[/])[0] } + + [@snat&.last].compact file "#{basedir}/conf.d/lvs.conf", ERB.new(<<~LVS, trim_mode: '-').result(binding), mode: 'u=rw,g=r,o=', overwrite: true <%- lvs_vars[:by_endpoint]&.each do |(lb_idx, ip, port), servers| -%> diff --git a/appliances/VRouter/LVS/tests.rb b/appliances/VRouter/LVS/tests.rb index 4456637e..e043927a 100644 --- a/appliances/VRouter/LVS/tests.rb +++ b/appliances/VRouter/LVS/tests.rb @@ -742,4 +742,119 @@ def clear_vars(object) expect(result.strip).to eq output.strip end end + + # NOTE: ONEAPP_VNF_NAT4_SNAT_ADDRESS is not configured on the router. Keepalived owns it as a /32 on the + # MASTER only (see Keepalived::snat_address), and LVS accepts it as a virtual IP in the same cases. + describe 'NAT4 SNAT address' do + let(:addr) { '203.0.113.10' } + + def lvs_conf(lbs, address: nil, enabled: 'YES', out: 'eth1', &block) + clear_env + ENV.delete_if { |name| name.start_with?('ONEAPP_VNF_NAT4_') || name.start_with?('ONEAPP_VNF_KEEPALIVED_') } + ENV.delete_if { |name| name.start_with?('ONEAPP_VROUTER_') } + + ENV['ONEAPP_VNF_LB_ENABLED'] = 'YES' + ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '10.2.20.69' + + lbs.each_with_index do |ip, i| + ENV["ONEAPP_VNF_LB#{i}_IP"] = ip + ENV["ONEAPP_VNF_LB#{i}_PORT"] = '80' + ENV["ONEAPP_VNF_LB#{i}_PROTOCOL"] = 'TCP' + ENV["ONEAPP_VNF_LB#{i}_SERVER0_HOST"] = '10.2.100.10' + ENV["ONEAPP_VNF_LB#{i}_SERVER0_PORT"] = '80' + end + + ENV['ONEAPP_VNF_NAT4_ENABLED'] = enabled + ENV['ONEAPP_VNF_NAT4_INTERFACES_OUT'] = out unless out.nil? + ENV['ONEAPP_VNF_NAT4_SNAT_ADDRESS'] = address unless address.nil? + + block&.call + + load './main.rb'; include Service::LVS + + allow(Service::LVS).to receive(:toggle).and_return(nil) + allow(Service::LVS).to receive(:sleep).and_return(nil) + allow(Service::LVS).to receive(:detect_nics).and_return(%w[eth0 eth1 eth2 eth3]) + allow(Service::LVS).to receive(:addrs_to_nics).and_return({ + '10.2.10.69' => ['eth0'], + '10.2.20.2' => ['eth1'] + }) + + clear_vars Service::LVS + + Dir.mktmpdir do |dir| + Service::LVS.execute basedir: dir + return File.read("#{dir}/conf.d/lvs.conf") + end + end + + def servers(conf) + conf.scan(/^virtual_server (\S+) /).flatten + end + + it 'should render a load balancer on the SNAT address' do + expect(servers(lvs_conf([addr], address: addr))).to eq [addr] + end + + it 'should ignore a prefix length on the attribute' do + expect(servers(lvs_conf([addr], address: "#{addr}/24"))).to eq [addr] + end + + it 'should keep the NIC and VIP load balancers' do + conf = lvs_conf(['10.2.10.69', '10.2.20.69', addr], address: addr) + + expect(servers(conf)).to eq ['10.2.10.69', '10.2.20.69', addr] + end + + it 'should render NIC and VIP load balancers as before when the attribute is unset' do + expect(servers(lvs_conf(['10.2.10.69', '10.2.20.69', addr]))).to eq ['10.2.10.69', '10.2.20.69'] + end + + it 'should skip the SNAT address when NAT4 is disabled' do + expect(servers(lvs_conf([addr], address: addr, enabled: 'NO'))).to eq [] + end + + it 'should skip the SNAT address when NAT4 has no outgoing NIC' do + expect(servers(lvs_conf([addr], address: addr, out: nil))).to eq [] + end + + it 'should skip the SNAT address when the outgoing NIC has no VRRP instance' do + expect(servers(lvs_conf([addr], address: addr, out: 'eth5'))).to eq [] + end + + it 'should skip the SNAT address when the outgoing NIC is not in the Keepalived interfaces' do + conf = lvs_conf([addr], address: addr) { ENV['ONEAPP_VNF_KEEPALIVED_INTERFACES'] = 'eth0' } + + expect(servers(conf)).to eq [] + end + + ['1.2.3.4; reboot', '999.1.1.1', '2001:db8::1', 'eth0', '0.0.0.0', '127.0.0.1', '224.0.0.1', + '240.0.0.1', '255.255.255.255'].each do |bad| + it "should skip a load balancer on the invalid SNAT address #{bad.inspect}" do + expect(servers(lvs_conf([bad, '10.2.10.69'], address: bad))).to eq ['10.2.10.69'] + end + end + + it 'should still skip an address that is neither a NIC, a VIP nor the SNAT address' do + expect(servers(lvs_conf(['198.51.100.7', addr], address: addr))).to eq [addr] + end + + it 'should skip the SNAT address when the outgoing NIC is skipped in Keepalived' do + conf = lvs_conf([addr], address: addr) { ENV['ONEAPP_VNF_KEEPALIVED_ETH1_SKIP'] = 'YES' } + + expect(servers(conf)).to eq [] + end + + it 'should use a later eligible NIC when the first outgoing NIC is skipped in Keepalived' do + conf = lvs_conf([addr], address: addr, out: 'eth0,eth1') { ENV['ONEAPP_VNF_KEEPALIVED_ETH0_SKIP'] = 'YES' } + + expect(servers(conf)).to eq [addr] + end + + it 'should skip an address inside a NIC subnet' do + conf = lvs_conf(['10.2.20.77'], address: '10.2.20.77') { ENV['ETH1_IP'] = '10.2.20.2' } + + expect(servers(conf)).to eq [] + end + end end diff --git a/appliances/VRouter/NAT4/main.rb b/appliances/VRouter/NAT4/main.rb index 4684a9d0..7053726d 100644 --- a/appliances/VRouter/NAT4/main.rb +++ b/appliances/VRouter/NAT4/main.rb @@ -16,7 +16,28 @@ module NAT4 ONEAPP_VNF_NAT4_SNAT_TO_VIP = env :ONEAPP_VNF_NAT4_SNAT_TO_VIP, 'NO' # YES -> SNAT to the VIP instead of MASQUERADE - ONEAPP_VNF_NAT4_SNAT_ADDRESS = env :ONEAPP_VNF_NAT4_SNAT_ADDRESS, '' # SNAT to this address instead of MASQUERADE + # SNAT to this address instead of MASQUERADE. A unicast IPv4 address, a prefix length is ignored. + # The address is usually not configured on any NIC (a public address the upstream routes to the VIP). + # Without an owner, a packet for it with no conntrack entry (scans, flows not yet synced after a + # failover) would be forwarded, leave via the default route and be routed back to the VIP by the + # upstream: a loop until the TTL runs out. Keepalived therefore adds the address as an extra /32 + # virtual_ipaddress (no connected route) on the first outgoing NIC of the VRRP instance, see + # Keepalived::snat_address. It is present on the VRRP MASTER only; Keepalived removes it on BACKUP, + # fault, stop and when the value changes (it re-renders its config). NAT4 itself never touches addresses. + # + # Requirements and behavior (ONEAPP_VNF_NAT4_SNAT_ADDRESS / ONEAPP_VNF_NAT4_SNAT_TO_VIP): + # - The address is configured as a /32 on the VRRP MASTER only, never on BACKUP. + # - It must be outside the on-link subnet of every NIC and must not be a NIC, alias (ETHn_ALIASm_IP) or + # VIP address. Otherwise it is ignored with a warning. + # - It is only used when Keepalived owns it (same helper, nat4_snat_address: a NAT4 outgoing NIC that + # gets a VRRP instance, so it is in ONEAPP_VNF_KEEPALIVED_INTERFACES, has an IP and is not skipped). + # When it is configured but not owned, NAT4 logs a warning and keeps MASQUERADE (it does not fall + # back to the VIP). + # - Being a local address on the MASTER, services not bound to a specific address (sshd, DNS) also + # answer on it there, the same exposure as the VIP. + # - ONEAPP_VNF_NAT4_SNAT_ADDRESS wins over ONEAPP_VNF_NAT4_SNAT_TO_VIP. + # The upstream must route the address to the VIP and must not have it on-link (ARP). + ONEAPP_VNF_NAT4_SNAT_ADDRESS = env :ONEAPP_VNF_NAT4_SNAT_ADDRESS, '' def parse_env @interfaces_out ||= parse_interfaces ONEAPP_VNF_NAT4_INTERFACES_OUT @@ -63,8 +84,6 @@ def parse_env end end - IPV4_RE = /\A(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\z/ - # NOTE: NIC -> source address, for the NICs that are not masqueraded. # ONEAPP_VNF_NAT4_SNAT_ADDRESS (one address for every outgoing NIC, for example a public address # routed to the VIP) wins over ONEAPP_VNF_NAT4_SNAT_TO_VIP (the first IPv4 VIP of the NIC). @@ -82,11 +101,16 @@ def snat_sources(nics) def snat_address_sources(nics) addr = ONEAPP_VNF_NAT4_SNAT_ADDRESS.strip.split(%[/])[0].to_s - unless IPV4_RE.match?(addr) + unless unicast_ipv4?(addr) msg :error, "NAT4::snat_sources: invalid ONEAPP_VNF_NAT4_SNAT_ADDRESS #{addr[0, 40].inspect}, using MASQUERADE" return {} end + if nat4_snat_address(@mgmt || detect_mgmt_nics).nil? + msg :warn, "NAT4::snat_sources: ONEAPP_VNF_NAT4_SNAT_ADDRESS #{addr} is not owned by Keepalived on this router, using MASQUERADE" + return {} + end + nics.each_with_object({}) do |nic, acc| acc[nic] = addr msg :info, "NAT4::snat_sources: #{nic} -> #{addr} (ONEAPP_VNF_NAT4_SNAT_ADDRESS)" diff --git a/appliances/VRouter/NAT4/tests.rb b/appliances/VRouter/NAT4/tests.rb index b6a5c6a2..11d928da 100644 --- a/appliances/VRouter/NAT4/tests.rb +++ b/appliances/VRouter/NAT4/tests.rb @@ -97,9 +97,13 @@ def masq_rules(snat: nil, address: nil, out: 'eth0,eth1') scripts = @scripts = [] @errors = [] + @warnings = [] allow(Service::NAT4).to receive(:bash) { |script| scripts << script; '' } allow(Service::NAT4).to receive(:toggle) - allow(Service::NAT4).to receive(:msg) { |level, text| @errors << text if level == :error } + allow(Service::NAT4).to receive(:msg) do |level, text| + @errors << text if level == :error + @warnings << text if level == :warn + end Service::NAT4.execute scripts.flat_map(&:lines).map(&:strip).grep(/NAT4-MASQ -o/) @@ -279,6 +283,68 @@ def masq_rules(snat: nil, address: nil, out: 'eth0,eth1') end end + # NOTE: The address is only SNATed to when Keepalived owns it (same helper), otherwise a packet for it + # without a conntrack entry would loop between the router and the upstream. + context 'when Keepalived does not own the address' do + let(:masq_eth1) { ["iptables -t nat -A NAT4-MASQ -o 'eth1' -j MASQUERADE"] } + + it 'should render MASQUERADE, not SNAT, when the outgoing NIC has no VRRP instance' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') { ENV['ONEAPP_VNF_KEEPALIVED_INTERFACES'] = 'eth0' } + + expect(rules).to eq masq_eth1 + expect(@scripts.join).not_to include('SNAT --to-source') + end + + it 'should warn that the address is not owned and MASQUERADE is used' do + masq_rules(address: '203.0.113.10', out: 'eth1') { ENV['ONEAPP_VNF_KEEPALIVED_INTERFACES'] = 'eth0' } + + expect(@warnings.join).to include('ONEAPP_VNF_NAT4_SNAT_ADDRESS', 'MASQUERADE') + end + + it 'should not fall back to the VIP either' do + rules = masq_rules(snat: 'YES', address: '203.0.113.10', out: 'eth1') do + ENV['ONEAPP_VNF_KEEPALIVED_INTERFACES'] = 'eth0' + ENV['ONEAPP_VROUTER_ETH1_VIP0'] = '192.168.1.99' + end + + expect(rules).to eq masq_eth1 + end + + it 'should render MASQUERADE when the only outgoing NIC is skipped in Keepalived' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') { ENV['ONEAPP_VNF_KEEPALIVED_ETH1_SKIP'] = 'YES' } + + expect(rules).to eq masq_eth1 + end + + it 'should render MASQUERADE for an address inside a NIC subnet' do + rules = masq_rules(address: '192.168.1.77', out: 'eth1') + + expect(rules).to eq masq_eth1 + expect(@warnings.join).to include('192.168.1.77') + end + + it 'should render MASQUERADE for an address that is an alias of a NIC' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') { ENV['ETH0_ALIAS0_IP'] = '203.0.113.10' } + + expect(rules).to eq masq_eth1 + end + + it 'should keep SNAT for a valid address next to unrelated aliases' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') { ENV['ETH0_ALIAS0_IP'] = '198.51.100.5' } + + expect(rules).to eq ["iptables -t nat -A NAT4-MASQ -o 'eth1' -j SNAT --to-source '203.0.113.10'"] + end + + it 'should not raise on odd network values and render MASQUERADE' do + rules = masq_rules(address: '203.0.113.10', out: 'eth1') do + ENV['ETH0_ALIAS0_IP'] = '198.51.100.5' + ENV['ETH0_ALIAS0_MASK'] = 'bogus' + end + + expect(rules).to eq masq_eth1 + end + end + # NOTE: SNAT rewrites everything that leaves the NIC, including the packets the router sends itself. # VRRP advertisements would then carry the NAT address as source for both nodes, so neither # can win the election, and both keep the VIPs. @@ -327,4 +393,52 @@ def all_rules end end end + + # NOTE: The SNAT address is owned by Keepalived (a /32 virtual_ipaddress, MASTER only), see Keepalived/tests.rb. + # NAT4 itself never configures addresses, it only validates the value and renders the rules. + describe 'SNAT address ownership' do + def run(op, address:) + clear_env + + ENV['ONEAPP_VNF_NAT4_ENABLED'] = 'YES' + ENV['ONEAPP_VNF_NAT4_INTERFACES_OUT'] = 'eth0,eth1' + ENV['ONEAPP_VNF_NAT4_SNAT_ADDRESS'] = address + ENV['ETH0_IP'] = '14.15.16.17' + ENV['ETH1_IP'] = '192.168.1.2' + + load './main.rb'; include Service::NAT4 + + clear_vars Service::NAT4 + + @scripts = scripts = [] + allow(Service::NAT4).to receive(:bash) { |script, **_| scripts << script; '' } + allow(Service::NAT4).to receive(:toggle) + allow(Service::NAT4).to receive(:msg) + Service::NAT4.public_send(op) + end + + %i[execute cleanup].each do |op| + it "should not touch interface addresses in #{op}" do + run(op, address: '203.0.113.10') + + expect(@scripts.join).not_to match(/\bip\b/) + end + end + + ['0.0.0.0', '0.1.2.3', '127.0.0.1', '127.1.2.3', '224.0.0.1', '239.1.2.3', '240.0.0.1', + '255.255.255.255'].each do |bad| + it "should reject #{bad.inspect} and keep MASQUERADE" do + run(:execute, address: bad) + + expect(@scripts.join).to include("-o 'eth0' -j MASQUERADE") + expect(@scripts.join).not_to include('SNAT --to-source') + end + end + + it 'should accept a public unicast address' do + run(:execute, address: '203.0.113.10/24') + + expect(@scripts.join).to include("-o 'eth0' -j SNAT --to-source '203.0.113.10'") + end + end end diff --git a/appliances/VRouter/vrouter.rb b/appliances/VRouter/vrouter.rb index 5de0a72c..47a6f338 100644 --- a/appliances/VRouter/vrouter.rb +++ b/appliances/VRouter/vrouter.rb @@ -76,6 +76,84 @@ def do_request(req, keep_alive, expect_json: true) end end +IPV4_RE = /\A(?:(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\.){3}(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)\z/ + +# NOTE: A plain dotted quad that is not 0/8, loopback (127/8), multicast (224/4), reserved or broadcast (240/4). +def unicast_ipv4?(addr) + return false unless IPV4_RE.match?(addr.to_s) + + first = addr.split(%[.])[0].to_i + !(first.zero? || first == 127 || first >= 224) +end + +# NOTE: The NIC names that get a VRRP instance in Keepalived (rendered in Keepalived::configure): the ones in +# ONEAPP_VNF_KEEPALIVED_INTERFACES that are not management NICs, have an IP address (no "floating only" +# NICs) and are not skipped (ONEAPP_VNF_KEEPALIVED_ETHn_SKIP). Read from the environment only, so every +# module that asks gets the same answer. +def keepalived_vrrp_nics(mgmt) + nics = parse_interfaces(env(:ONEAPP_VNF_KEEPALIVED_INTERFACES, '')).keys - mgmt + withip = addrs_to_nics(nics).values.flatten.uniq + + nics.select { |nic| withip.include?(nic) && !env("ONEAPP_VNF_KEEPALIVED_#{nic.upcase}_SKIP", 'NO') } +end + +# NOTE: Why the NAT4 SNAT address cannot be used: the first match, or nil. It is an IPv4 address that Keepalived +# adds as a /32, so it must not be on-link anywhere and must not be a (virtual) address already. +def nat4_snat_address_conflict(addr) + ip = IPAddr.new(addr) + + own = [detect_addrs, detect_vips].flat_map { |h| h.values.flat_map(&:values) }.map { |a| a.split(%[/])[0] } + return 'it already is a NIC address or VIP' if own.include?(addr) + + aliases = ENV.each_with_object({}) do |(name, v), acc| + acc[$1] = v.strip.split(%[/])[0] if /\AETH(\d+_ALIAS\d+)_IP\z/ =~ name && IPV4_RE.match?(v.strip.split(%[/])[0].to_s) + end + return 'it already is an alias address of a NIC' if aliases.values.include?(addr) + + alias_subnets = aliases.filter_map do |key, a| + next if (mask = env("ETH#{key}_MASK", nil)).nil? + + IPAddr.new("#{a}/#{mask}").then { |net| "#{net}/#{net.prefix}" } + end + subnets = nics_to_subnets.values.flatten + vips_to_subnets.values + alias_subnets + subnet = subnets.find { |s| IPAddr.new(s).include?(ip) } + + subnet.nil? ? nil : "it is inside the on-link subnet #{subnet}" +end + +# NOTE: The NAT4 SNAT address (ONEAPP_VNF_NAT4_SNAT_ADDRESS) is not configured on the router as a real address. +# Keepalived owns it as a /32 virtual address of the first NAT4 outgoing NIC that gets a VRRP instance +# (MASTER only) and LVS accepts it as a virtual IP. NAT4 SNATs to it only when it is owned. All of them +# ask here, so they agree. `mgmt` are the management NICs. Returns [nic, address] or nil (NAT4 disabled, +# no or invalid address, address inside any NIC subnet or already a NIC address, alias or VIP, no +# outgoing NIC with a VRRP instance). A prefix length is ignored. Never raises: odd network values +# are logged and the address is treated as not configured. +def nat4_snat_address(mgmt) + return nil unless env(:ONEAPP_VNF_NAT4_ENABLED, 'NO') + + addr = ENV.fetch('ONEAPP_VNF_NAT4_SNAT_ADDRESS', '').strip.split(%[/])[0].to_s + return nil unless unicast_ipv4?(addr) + + unless (reason = nat4_snat_address_conflict(addr)).nil? + msg :warn, "nat4_snat_address: ignoring ONEAPP_VNF_NAT4_SNAT_ADDRESS #{addr}: #{reason}" + return nil + end + + out = parse_interfaces(env(:ONEAPP_VNF_NAT4_INTERFACES_OUT, nil)).keys - mgmt + nic = out.grep(/\Aeth\d+\z/).find { |n| keepalived_vrrp_nics(mgmt).include?(n) } + + if nic.nil? + msg :warn, "nat4_snat_address: ignoring ONEAPP_VNF_NAT4_SNAT_ADDRESS #{addr}: no NAT4 outgoing NIC has a VRRP instance" + return nil + end + + [nic, addr] +rescue StandardError => e + msg :error, "nat4_snat_address: #{e.class}: #{e.message}, treating ONEAPP_VNF_NAT4_SNAT_ADDRESS " \ + "#{ENV['ONEAPP_VNF_NAT4_SNAT_ADDRESS'].to_s[0, 40].inspect} as not configured" + nil +end + def ip_link_set_up(nic) stdout = bash "ip link set '#{nic}' up", terminate: false end