From a83e1f9658d6aa60bafa280d34c774b11f6769ed Mon Sep 17 00:00:00 2001
From: Patrick Schmidt <patrick.schmidt@innogames.com>
Date: Wed, 8 Nov 2023 12:47:50 +0100
Subject: [PATCH] Fix ifconfig parse logic failing on some tunnels

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.
---
 lib/facter/resolvers/networking.rb       | 33 ++++++++++++++----------
 spec/facter/resolvers/networking_spec.rb | 21 ++++++++++-----
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/lib/facter/resolvers/networking.rb b/lib/facter/resolvers/networking.rb
index 9fbd9edc2a..51dd1be759 100644
--- a/lib/facter/resolvers/networking.rb
+++ b/lib/facter/resolvers/networking.rb
@@ -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
diff --git a/spec/facter/resolvers/networking_spec.rb b/spec/facter/resolvers/networking_spec.rb
index 32e4511595..d583164500 100644
--- a/spec/facter/resolvers/networking_spec.rb
+++ b/spec/facter/resolvers/networking_spec.rb
@@ -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)
@@ -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
 
@@ -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)
@@ -160,7 +169,6 @@
 
       it 'returns dhcp server ip as nil' do
         expect(networking.resolve(:dhcp)).to be(nil)
-        raise
       end
     end
 
@@ -169,7 +177,6 @@
 
       it 'returns interfaces as nil' do
         expect(networking.resolve(:interfaces)).to be(nil)
-        raise
       end
     end
   end