From b28985f60cf37859d48e815e8b6215c2702fdc29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:19:41 -0300 Subject: [PATCH 1/2] fix(ipmi): centralize IPv4 command encoding --- xCAT-server/lib/xcat/plugins/ipmi.pm | 60 +++++++++++++++++++--------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/ipmi.pm b/xCAT-server/lib/xcat/plugins/ipmi.pm index 5b48d0fd0..87065d4a9 100644 --- a/xCAT-server/lib/xcat/plugins/ipmi.pm +++ b/xCAT-server/lib/xcat/plugins/ipmi.pm @@ -869,6 +869,25 @@ sub next_setnetinfo { &setnetinfo($sessdata); } +sub _resolve_ipv4_octets { + my $value = shift; + return unless defined($value) and length($value); + + my $packed_address = inet_aton($value); + return unless defined($packed_address) and length($packed_address) == 4; + + return (inet_ntoa($packed_address), unpack("C4", $packed_address)); +} + +sub _report_unresolvable_ipv4 { + my $value = shift; + my $sessdata = shift; + xCAT::SvrUtils::sendmsg( + [ 1, "Unable to resolve '$value' to an IPv4 address" ], + $callback, $sessdata->{node}, %allerrornodes); + return; +} + sub setnetinfo { my $sessdata = shift; my $subcommand = $sessdata->{subcommand}; @@ -941,10 +960,13 @@ sub setnetinfo { @cmd = (1, $channel_number, 0x10, @clist); } elsif ($subcommand =~ m/snmpdest(\d+)/) { - my $dstip = $argument; #pop(@input); - $dstip = inet_ntoa(inet_aton($dstip)); - my @dip = split /\./, $dstip; - @cmd = (0x01, $channel_number, 0x13, $1, 0x00, 0x00, $dip[0], $dip[1], $dip[2], $dip[3], 0, 0, 0, 0, 0, 0); + my $destination = $1; + my ($dstip, @dip) = _resolve_ipv4_octets($argument); + unless (defined($dstip)) { + _report_unresolvable_ipv4($argument, $sessdata); + return; + } + @cmd = (0x01, $channel_number, 0x13, $destination, 0x00, 0x00, @dip, 0, 0, 0, 0, 0, 0); } elsif ($subcommand =~ m/netmask/) { if ($argument =~ /\./) { my @mask = split /\./, $argument; @@ -955,21 +977,21 @@ sub setnetinfo { @cmd = (0x01, $channel_number, 0x6, @mask); } } elsif ($subcommand eq "gateway" and $argument) { - my $gw = inet_ntoa(inet_aton($argument)); - my @mask = split /\./, $gw; - foreach (0 .. 3) { - $mask[$_] = $mask[$_] + 0; + my ($gw, @octets) = _resolve_ipv4_octets($argument); + unless (defined($gw)) { + _report_unresolvable_ipv4($argument, $sessdata); + return; } $sessdata->{setnetinfo_value} = $gw; - @cmd = (0x01, $channel_number, 0x0C, @mask); + @cmd = (0x01, $channel_number, 0x0C, @octets); } elsif ($subcommand eq "backupgateway" and $argument) { - my $gw = inet_ntoa(inet_aton($argument)); - my @mask = split /\./, $gw; - foreach (0 .. 3) { - $mask[$_] = $mask[$_] + 0; + my ($gw, @octets) = _resolve_ipv4_octets($argument); + unless (defined($gw)) { + _report_unresolvable_ipv4($argument, $sessdata); + return; } $sessdata->{setnetinfo_value} = $gw; - @cmd = (0x01, $channel_number, 0x0E, @mask); + @cmd = (0x01, $channel_number, 0x0E, @octets); } elsif ($subcommand =~ m/vlan/) { if ($argument =~ /^(off|disable|disabled)$/i) { @cmd = (0x01, $channel_number, 0x14, 0x00, 0x00); @@ -999,13 +1021,13 @@ sub setnetinfo { return; } } - my $mip = inet_ntoa(inet_aton($argument)); - my @mask = split /\./, $mip; - foreach (0 .. 3) { - $mask[$_] = $mask[$_] + 0; + my ($mip, @octets) = _resolve_ipv4_octets($argument); + unless (defined($mip)) { + _report_unresolvable_ipv4($argument, $sessdata); + return; } $sessdata->{setnetinfo_value} = $mip; - @cmd = (0x01, $channel_number, 0x3, @mask); + @cmd = (0x01, $channel_number, 0x3, @octets); } #elsif($subcommand eq "alert" ) { From 8e7d0dd7e4c6bb35ccf1c0d73a1f3a70371a2a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:19:47 -0300 Subject: [PATCH 2/2] test(ipmi): cover IPv4 command encoding --- xCAT-test/unit/ipmi_ipv4_command_encoding.t | 214 ++++++++++++++++++++ 1 file changed, 214 insertions(+) create mode 100644 xCAT-test/unit/ipmi_ipv4_command_encoding.t diff --git a/xCAT-test/unit/ipmi_ipv4_command_encoding.t b/xCAT-test/unit/ipmi_ipv4_command_encoding.t new file mode 100644 index 000000000..812ee6dc6 --- /dev/null +++ b/xCAT-test/unit/ipmi_ipv4_command_encoding.t @@ -0,0 +1,214 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +## no critic (Modules::RequireFilenameMatchesPackage, TestingAndDebugging::ProhibitNoStrict, TestingAndDebugging::ProhibitNoWarnings) + +BEGIN { + *CORE::GLOBAL::chmod = sub { return 1; }; + + my @stub_modules = qw( + xCAT::GlobalDef + xCAT_monitoring::monitorctrl + xCAT::SPD + xCAT::IPMI + xCAT::BMCUtils + xCAT::PasswordUtils + xCAT::Utils + xCAT::TableUtils + xCAT::IMMUtils + xCAT::ServiceNodeUtils + xCAT::SvrUtils + xCAT::NetworkUtils + xCAT::Usage + xCAT::data::ibmhwtypes + xCAT::data::ibmleds + xCAT::data::ipmigenericevents + xCAT::data::ipmisensorevents + ); + + foreach my $module (@stub_modules) { + ( my $module_file = $module ) =~ s{::}{/}g; + $INC{"$module_file.pm"} = __FILE__; + no strict 'refs'; + *{"${module}::import"} = sub { }; + } + + no strict 'refs'; + *{'xCAT::BMCUtils::rspconfig_bmc_setting'} = sub { return {}; }; + + package File::Path; + sub import { + my $caller = caller; + no strict 'refs'; + *{"${caller}::mkpath"} = sub { return 1; }; + } + $INC{'File/Path.pm'} = __FILE__; +} + +package Local::IPMISession; + +sub new { + my ( $class, $channel ) = @_; + return bless { currentchannel => $channel, calls => [] }, $class; +} + +sub subcmd { + my ( $self, %args ) = @_; + push @{ $self->{calls} }, \%args; + return; +} + +package main; + +no warnings qw(once redefine); +use FindBin; +use File::Spec; +use Test::More; + +my $repo_root = $ENV{XCAT_IPMI_PLUGIN_ROOT} + || File::Spec->catdir( $FindBin::Bin, '..', '..' ); +my $plugin = File::Spec->catfile( + $repo_root, 'xCAT-server', 'lib', 'xcat', 'plugins', 'ipmi.pm' ); +require $plugin; + +my $system_inet_aton = \&xCAT_plugin::ipmi::inet_aton; +my @messages; +*xCAT::SvrUtils::sendmsg = sub { + push @messages, [@_]; + return; +}; + +sub run_setting { + my ($setting) = @_; + my $session = Local::IPMISession->new( 2 ); + my $session_data = { + bmcnum => 1, + ipmisession => $session, + netinfo_setinprogress => 1, + node => 'node01', + set_ipsrc_static => 1, + subcommand => $setting, + }; + + @messages = (); + my $ok = eval { + xCAT_plugin::ipmi::setnetinfo($session_data); + 1; + }; + + return { + calls => $session->{calls}, + error => $@, + messages => [@messages], + session_data => $session_data, + succeeded => $ok, + }; +} + +subtest 'IPv4 settings preserve their wire command bytes' => sub { + my @cases = ( + { + setting => 'ip=192.168.1.10', + data => [ 2, 0x03, 192, 168, 1, 10 ], + value => '192.168.1.10', + }, + { + setting => 'gateway=10.20.30.1', + data => [ 2, 0x0c, 10, 20, 30, 1 ], + value => '10.20.30.1', + }, + { + setting => 'backupgateway=10.20.30.2', + data => [ 2, 0x0e, 10, 20, 30, 2 ], + value => '10.20.30.2', + }, + { + setting => 'snmpdest2=bmc.example.test', + hostname => 1, + data => [ + 2, 0x13, 2, 0, 0, 203, 0, 113, 7, + 0, 0, 0, 0, 0, 0, + ], + }, + ); + + foreach my $case (@cases) { + my $result; + if ( $case->{hostname} ) { + local *xCAT_plugin::ipmi::inet_aton = sub { + return pack( 'C4', 203, 0, 113, 7 ) + if $_[0] eq 'bmc.example.test'; + return $system_inet_aton->(@_); + }; + $result = run_setting( $case->{setting} ); + } else { + $result = run_setting( $case->{setting} ); + } + ok( $result->{succeeded}, "$case->{setting} is encoded" ) + or diag $result->{error}; + is( scalar( @{ $result->{calls} } ), 1, + "$case->{setting} sends one command" ); + my $call = $result->{calls}->[0]; + is( $call->{netfn}, 0x0c, "$case->{setting} uses the transport netfn" ); + is( $call->{command}, 0x01, + "$case->{setting} uses the LAN configuration command" ); + is_deeply( $call->{data}, $case->{data}, + "$case->{setting} preserves the command data" ); + is_deeply( $result->{messages}, [], + "$case->{setting} reports no error" ); + if ( exists $case->{value} ) { + is( $result->{session_data}->{setnetinfo_value}, $case->{value}, + "$case->{setting} preserves the canonical readback value" ); + } + } +}; + +subtest 'IPv4 resolver contract' => sub { + plan skip_all => 'shared IPv4 resolver is not present on the base revision' + unless xCAT_plugin::ipmi->can('_resolve_ipv4_octets'); + + my ( $canonical, @octets ) = + xCAT_plugin::ipmi::_resolve_ipv4_octets('192.168.1.10'); + is( $canonical, '192.168.1.10', 'a literal is canonicalized' ); + is_deeply( \@octets, [ 192, 168, 1, 10 ], + 'a literal produces exactly four octets' ); + + { + local *xCAT_plugin::ipmi::inet_aton = sub { + return pack( 'C4', 203, 0, 113, 7 ) + if $_[0] eq 'bmc.example.test'; + return $system_inet_aton->(@_); + }; + ( $canonical, @octets ) = + xCAT_plugin::ipmi::_resolve_ipv4_octets('bmc.example.test'); + } + is( $canonical, '203.0.113.7', 'a resolvable hostname remains supported' ); + is_deeply( \@octets, [ 203, 0, 113, 7 ], + 'a hostname produces the resolved octets' ); + + my @invalid = + xCAT_plugin::ipmi::_resolve_ipv4_octets('999.999.999.999'); + is_deeply( \@invalid, [], 'an invalid address produces no result' ); +}; + +subtest 'invalid IPv4 settings report an xCAT error' => sub { + plan skip_all => 'invalid setting handling is not present on the base revision' + unless xCAT_plugin::ipmi->can('_resolve_ipv4_octets'); + + foreach my $name (qw(ip gateway backupgateway snmpdest1)) { + my $result = run_setting("$name=999.999.999.999"); + ok( $result->{succeeded}, "$name does not raise a Perl exception" ) + or diag $result->{error}; + is_deeply( $result->{calls}, [], "$name sends no IPMI command" ); + is( scalar( @{ $result->{messages} } ), 1, + "$name reports one error" ); + is_deeply( + $result->{messages}->[0]->[0], + [ 1, "Unable to resolve '999.999.999.999' to an IPv4 address" ], + "$name explains the invalid value", + ); + } +}; + +done_testing();