From 61bcc512e5ba8e99df5984e01897563053cadeab Mon Sep 17 00:00:00 2001 From: Joshua Miller Date: Wed, 22 May 2024 08:49:05 -0700 Subject: [PATCH] Validate given IP addresses - 1% Differential Revision: D48831692 fbshipit-source-id: c6d0508915f7f21d8d4ecaf2a7d5af560bd177bd --- cookbooks/fb_networkd/resources/default.rb | 40 ++++++ cookbooks/fb_networkd/spec/default_spec.rb | 149 +++++++++++++++++++++ 2 files changed, 189 insertions(+) diff --git a/cookbooks/fb_networkd/resources/default.rb b/cookbooks/fb_networkd/resources/default.rb index bb6b2ff5..b41fb9fc 100644 --- a/cookbooks/fb_networkd/resources/default.rb +++ b/cookbooks/fb_networkd/resources/default.rb @@ -15,10 +15,48 @@ # See the License for the specific language governing permissions and # limitations under the License. # +require 'ipaddr' unified_mode(false) if Chef::VERSION >= 18 # TODO(T144966423) default_action :manage +action_class do + # TODO: A more reusable approach community-wise would be to create custom + # resources for the different networkd units and move this validation to those + # custom resources + def validate_network_addresses(conf) + return if !node.in_shard?(0) + conf.dig('config', 'Network', 'Address')&.each do |ip| + ::IPAddr.new(ip) + rescue ::IPAddr::Error + raise "Trying to use bad Network Address IP: '#{ip}' from conf: #{conf}" + end + + conf.dig('config', 'Address')&.each do |addr| + if (ip = addr['Address']) + begin + ::IPAddr.new(ip) + rescue ::IPAddr::Error + raise "Trying to use bad Address IP: '#{ip}' from conf: #{conf}" + end + end + end + + conf.dig('config', 'Route')&.each do |route| + ['Gateway', 'Destination', 'Source'].each do |route_type| + if route[route_type] + ip = route[route_type] + begin + ::IPAddr.new(ip) + rescue ::IPAddr::Error + raise "Trying to use bad route #{route_type} IP: '#{ip}' from route: #{route}" + end + end + end + end + end +end + action :manage do # There are some situations (i.e. changing the primary interface and # corresponding addresses) where we need to restart systemd-networkd to make @@ -102,6 +140,8 @@ "#{conf['priority']}-fb_networkd-#{conf['name']}.network", ) + validate_network_addresses conf + # Set up the template for this interface fb_helpers_gated_template conffile do allow_changes node.interface_change_allowed?(conf['name']) diff --git a/cookbooks/fb_networkd/spec/default_spec.rb b/cookbooks/fb_networkd/spec/default_spec.rb index e6f2681c..03bf83e5 100644 --- a/cookbooks/fb_networkd/spec/default_spec.rb +++ b/cookbooks/fb_networkd/spec/default_spec.rb @@ -95,4 +95,153 @@ with_content(tc.fixture('50-fb_networkd-tap0.netdev')) end end + + context 'use of bad ip addresses' do + it 'should fail the run with bad Network Address' do + expect do + tc.chef_run( + :step_into => ['fb_networkd', 'fb_helpers_gated_template'], + ) do |node| + allow(node).to receive(:systemd?).and_return(true) + allow(node).to receive(:in_shard?).and_return(true) + + # These enable the fb_helpers_gated_template resources + allow(node).to receive(:interface_change_allowed?).and_return(true) + allow(Chef::Resource::Template).to receive(:updated_by_last_action?).and_call_original + allow_any_instance_of(Chef::Resource::Template).to receive(:updated_by_last_action?).and_return(true) + end.converge(described_recipe) do |node| + node.default['fb_networkd']['networks']['eth0'] = { + 'priority' => 1, + 'config' => { + 'Network' => { + 'Address' => [ + '2001::db00:1/64', + '2001::bad1::1/64', # Extra colon + ], + }, + }, + } + end + end.to raise_error(RuntimeError, %r{fb_networkd:.*Trying to use bad Network Address IP: '2001::bad1::1/64'.*}) + end + + it 'should fail the run with bad Address Address' do + expect do + tc.chef_run( + :step_into => ['fb_networkd', 'fb_helpers_gated_template'], + ) do |node| + allow(node).to receive(:systemd?).and_return(true) + allow(node).to receive(:in_shard?).and_return(true) + + # These enable the fb_helpers_gated_template resources + allow(node).to receive(:interface_change_allowed?).and_return(true) + allow(Chef::Resource::Template).to receive(:updated_by_last_action?).and_call_original + allow_any_instance_of(Chef::Resource::Template).to receive(:updated_by_last_action?).and_return(true) + end.converge(described_recipe) do |node| + node.default['fb_networkd']['networks']['eth0'] = { + 'config' => { + 'Address' => [ + { + 'Address' => '2001:db00::1/64', + 'PreferredLifetime' => 'infinity', + }, + { + 'Address' => '2001::bad1::1/64', + 'PreferredLifetime' => 'infinity', + }, + ], + }, + } + end + end.to raise_error(RuntimeError, %r{fb_networkd:.*Trying to use bad Address IP: '2001::bad1::1/64'.*}) + end + + it 'should fail the run with bad Route Gateway' do + expect do + tc.chef_run( + :step_into => ['fb_networkd', 'fb_helpers_gated_template'], + ) do |node| + allow(node).to receive(:systemd?).and_return(true) + allow(node).to receive(:in_shard?).and_return(true) + + # These enable the fb_helpers_gated_template resources + allow(node).to receive(:interface_change_allowed?).and_return(true) + allow(Chef::Resource::Template).to receive(:updated_by_last_action?).and_call_original + allow_any_instance_of(Chef::Resource::Template).to receive(:updated_by_last_action?).and_return(true) + end.converge(described_recipe) do |node| + node.default['fb_networkd']['networks']['eth0'] = { + 'priority' => 1, + 'config' => { + 'Route' => [ + { + 'Gateway' => '2001::bad1::1', + 'Source' => '::/0', + 'Destination' => '::/0', + 'Metric' => '1', + }, + ], + }, + } + end + end.to raise_error(RuntimeError, /fb_networkd:.*Trying to use bad .*bad1.*/) + end + it 'should fail the run with bad Route Source' do + expect do + tc.chef_run( + :step_into => ['fb_networkd', 'fb_helpers_gated_template'], + ) do |node| + allow(node).to receive(:systemd?).and_return(true) + allow(node).to receive(:in_shard?).and_return(true) + + # These enable the fb_helpers_gated_template resources + allow(node).to receive(:interface_change_allowed?).and_return(true) + allow(Chef::Resource::Template).to receive(:updated_by_last_action?).and_call_original + allow_any_instance_of(Chef::Resource::Template).to receive(:updated_by_last_action?).and_return(true) + end.converge(described_recipe) do |node| + node.default['fb_networkd']['networks']['eth0'] = { + 'priority' => 1, + 'config' => { + 'Route' => [ + { + 'Gateway' => '2001::db00:1', + 'Source' => '::/bad0', + 'Destination' => '::/0', + 'Metric' => '1', + }, + ], + }, + } + end + end.to raise_error(RuntimeError, /fb_networkd:.*Trying to use bad .*bad0.*/) + end + it 'should fail the run with bad Route Destination' do + expect do + tc.chef_run( + :step_into => ['fb_networkd', 'fb_helpers_gated_template'], + ) do |node| + allow(node).to receive(:systemd?).and_return(true) + allow(node).to receive(:in_shard?).and_return(true) + + # These enable the fb_helpers_gated_template resources + allow(node).to receive(:interface_change_allowed?).and_return(true) + allow(Chef::Resource::Template).to receive(:updated_by_last_action?).and_call_original + allow_any_instance_of(Chef::Resource::Template).to receive(:updated_by_last_action?).and_return(true) + end.converge(described_recipe) do |node| + node.default['fb_networkd']['networks']['eth0'] = { + 'priority' => 1, + 'config' => { + 'Route' => [ + { + 'Gateway' => '2001::db00:1', + 'Source' => '::/0', + 'Destination' => '::/bad2', + 'Metric' => '1', + }, + ], + }, + } + end + end.to raise_error(RuntimeError, /fb_networkd:.*Trying to use bad .*bad2.*/) + end + end end