Skip to content

Commit

Permalink
Fix ifconfig parse logic failing on some tunnels
Browse files Browse the repository at this point in the history
Tunnel interfaces that have a line similar to
"tunnel inet 192.0.2.1 --> 192.0.2.2" would fail the network resolver
because IPs were extracted through two separate IP + mask patterns.
However, in the example above, there is no such thing as a netmask.

This fix merges the patterns and extracts the IPs and netmasks
together so that no mismatches can occur. It might also fix some
cases where only the netmask would be nil and therefore produce an
invalid binding that could in turn resolve to the wrong (outer) IP of
the tunnel instead of the inner IP.
  • Loading branch information
Woellchen committed Apr 17, 2024
1 parent 61a17ff commit a83e1f9
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 20 deletions.
33 changes: 20 additions & 13 deletions lib/facter/resolvers/networking.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,28 +80,35 @@ def extract_dhcp(interface_name, raw_data, parsed_interface_data)
end

def extract_ip_data(raw_data, parsed_interface_data)
ip = extract_values(raw_data, /inet (\S+)/)
mask = extract_values(raw_data, /netmask (\S+)/).map { |val| val.hex.to_s(2).count('1') }
inets = extract_values(raw_data, /inet (\S+).+netmask (\S+)/, :extract_ip4_data)
bindings = create_bindings(inets)
parsed_interface_data[:bindings] = bindings unless bindings.empty?

ip6 = extract_values(raw_data, /inet6 (\S+)/).map { |val| val.gsub(/%.+/, '') }
mask6 = extract_values(raw_data, /prefixlen (\S+)/)

parsed_interface_data[:bindings] = create_bindings(ip, mask) unless ip.empty?
parsed_interface_data[:bindings6] = create_bindings(ip6, mask6) unless ip6.empty?
inets = extract_values(raw_data, /inet6 (\S+).+prefixlen (\S+)/, :extract_ip6_data)
bindings = create_bindings(inets)
parsed_interface_data[:bindings6] = bindings unless bindings.empty?
end

def extract_values(data, regex)
def extract_values(data, regex, ip_func)
results = []
data.scan(regex).flatten.each do |val|
results << val
data.scan(regex).flatten.each_slice(2) do |val|
results << method(ip_func).call(val)
end
results
end

def create_bindings(ips, masks)
def extract_ip4_data(inet)
[inet[0], inet[1].hex.to_s(2).count('1')]
end

def extract_ip6_data(inet)
[inet[0].gsub(/%.+/, ''), inet[1]]
end

def create_bindings(inets)
bindings = []
ips.zip(masks).each do |ip, mask|
bindings << Facter::Util::Resolvers::Networking.build_binding(ip, mask)
inets.each do |inet|
bindings << Facter::Util::Resolvers::Networking.build_binding(inet[0], inet[1])
end
bindings
end
Expand Down
21 changes: 14 additions & 7 deletions spec/facter/resolvers/networking_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@

let(:log_spy) { instance_spy(Facter::Log) }

before do
pending 'contains failing tests to reproduce a bug'
end

describe '#resolve' do
before do
networking.instance_variable_set(:@log, log_spy)
Expand Down Expand Up @@ -44,7 +40,7 @@
end

it 'detects all interfaces' do
expected = %w[lo0 gif0 stf0 en0 en0.1 en1 en2 bridge0 p2p0 awdl0 llw0 utun0 utun1 utun2 utun3 ib0 ib1]
expected = %w[lo0 gif0 stf0 en0 en0.1 en1 en2 bridge0 p2p0 awdl0 llw0 utun0 utun1 utun2 utun3 utun4 utun5 ib0 ib1]
expect(networking.resolve(:interfaces).keys).to match_array(expected)
end

Expand Down Expand Up @@ -142,6 +138,19 @@
expect(networking.resolve(:interfaces)['utun3']).to include(expected)
end

it 'checks interface utun4' do
expected = { bindings: [{ address: '192.0.2.100', netmask: '255.255.255.255', network: '192.0.2.100' }] }
expect(networking.resolve(:interfaces)['utun4']).to include(expected)
end

it 'checks interface utun5' do
expected = { bindings6: [
{ address: '2001:db8::1', netmask: 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff',
network: '2001:db8::1', scope6: 'global' }
] }
expect(networking.resolve(:interfaces)['utun5']).to include(expected)
end

it 'checks interface ib0 has the expected mac' do
expected = { mac: '80:00:02:08:FA:81:00:00:00:00:00:00:00:00:00:00:00:00:00:00' }
expect(networking.resolve(:interfaces)['ib0']).to include(expected)
Expand All @@ -160,7 +169,6 @@

it 'returns dhcp server ip as nil' do
expect(networking.resolve(:dhcp)).to be(nil)
raise
end
end

Expand All @@ -169,7 +177,6 @@

it 'returns interfaces as nil' do
expect(networking.resolve(:interfaces)).to be(nil)
raise
end
end
end
Expand Down

0 comments on commit a83e1f9

Please sign in to comment.