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/execute.rb b/appliances/VRouter/NAT4/execute.rb index bc28d161..197592e3 100644 --- a/appliances/VRouter/NAT4/execute.rb +++ b/appliances/VRouter/NAT4/execute.rb @@ -46,12 +46,21 @@ 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 + <%- 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] %>' + <%- 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..7053726d 100644 --- a/appliances/VRouter/NAT4/main.rb +++ b/appliances/VRouter/NAT4/main.rb @@ -14,6 +14,31 @@ 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 + + # 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 @mgmt ||= detect_mgmt_nics @@ -59,6 +84,56 @@ def parse_env end end + # 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 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)" + 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..11d928da 100644 --- a/appliances/VRouter/NAT4/tests.rb +++ b/appliances/VRouter/NAT4/tests.rb @@ -74,4 +74,371 @@ 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 = [] + @warnings = [] + allow(Service::NAT4).to receive(:bash) { |script| scripts << script; '' } + allow(Service::NAT4).to receive(:toggle) + 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/) + 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 + + # 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. + 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 + + # 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