From 90317dad4d0fbe919520440a56e53d543d5c5849 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:06:23 -0300 Subject: [PATCH 1/6] fix(ipmi): reject ambiguous IPv4 literals --- xCAT-server/lib/xcat/plugins/ipmi.pm | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/xCAT-server/lib/xcat/plugins/ipmi.pm b/xCAT-server/lib/xcat/plugins/ipmi.pm index 87065d4a9..6afceba10 100644 --- a/xCAT-server/lib/xcat/plugins/ipmi.pm +++ b/xCAT-server/lib/xcat/plugins/ipmi.pm @@ -873,6 +873,19 @@ sub _resolve_ipv4_octets { my $value = shift; return unless defined($value) and length($value); + # inet_aton also accepts octal, hexadecimal, partial, and single-number + # forms, and platforms parse them differently. A value whose components + # are all numeric is a literal, and a literal must be a plain + # dotted-decimal quad so every platform encodes the same address. + my @components = split(/\./, $value, -1); + unless (grep { $_ !~ m/^(?:0[xX][0-9a-fA-F]+|[0-9]+)$/ } @components) { + return unless scalar(@components) == 4; + foreach my $octet (@components) { + return unless $octet =~ m/^(?:0|[1-9][0-9]{0,2})$/ and $octet <= 255; + } + return ($value, map { $_ + 0 } @components); + } + my $packed_address = inet_aton($value); return unless defined($packed_address) and length($packed_address) == 4; From 96d682312ca6cb6dcb3bfe07c0ea75b7c5ed2bc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:07:12 -0300 Subject: [PATCH 2/6] test(ipmi): cover ambiguous IPv4 literal rejection --- xCAT-test/unit/ipmi_ipv4_command_encoding.t | 69 +++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/xCAT-test/unit/ipmi_ipv4_command_encoding.t b/xCAT-test/unit/ipmi_ipv4_command_encoding.t index 812ee6dc6..a3a2ebb85 100644 --- a/xCAT-test/unit/ipmi_ipv4_command_encoding.t +++ b/xCAT-test/unit/ipmi_ipv4_command_encoding.t @@ -113,6 +113,11 @@ subtest 'IPv4 settings preserve their wire command bytes' => sub { data => [ 2, 0x03, 192, 168, 1, 10 ], value => '192.168.1.10', }, + { + setting => 'ip=0.0.0.0', + data => [ 2, 0x03, 0, 0, 0, 0 ], + value => '0.0.0.0', + }, { setting => 'gateway=10.20.30.1', data => [ 2, 0x0c, 10, 20, 30, 1 ], @@ -192,6 +197,70 @@ subtest 'IPv4 resolver contract' => sub { is_deeply( \@invalid, [], 'an invalid address produces no result' ); }; +subtest 'ambiguous IPv4 literals are rejected' => sub { + plan skip_all => 'the shared IPv4 resolver is not present on the base revision' + unless xCAT_plugin::ipmi->can('_resolve_ipv4_octets'); + + my @ambiguous = ( + [ '192.168.001.010', 'leading-zero octets are octal to some resolvers' ], + [ '192.168.257', 'out-of-range octets overflow into neighbours' ], + [ '3232235786', 'single-number form' ], + [ '192.168.1', 'partial three-part form' ], + [ '192.168.1.10.5', 'five-part form' ], + [ '256.1.1.1', 'octet above 255' ], + [ '192.168..1', 'empty octet' ], + [ '0xc0a8010a', 'single hexadecimal form' ], + [ '0xc0.0xa8.0x01.0x0a', 'dotted hexadecimal form' ], + [ '0X0A141E02', 'uppercase hexadecimal form' ], + [ '127.0x0.0.1', 'mixed decimal and hexadecimal form' ], + ); + foreach my $case (@ambiguous) { + my ( $value, $reason ) = @{$case}; + is_deeply( [ xCAT_plugin::ipmi::_resolve_ipv4_octets($value) ], + [], "'$value' is rejected: $reason" ); + } + + is_deeply( + [ xCAT_plugin::ipmi::_resolve_ipv4_octets('0.0.0.0') ], + [ '0.0.0.0', 0, 0, 0, 0 ], + 'the all-zero address used to clear settings stays accepted' + ); + is_deeply( + [ xCAT_plugin::ipmi::_resolve_ipv4_octets('255.255.255.255') ], + [ '255.255.255.255', 255, 255, 255, 255 ], + 'the top of the address range stays accepted' + ); + + { + local *xCAT_plugin::ipmi::inet_aton = sub { + return pack( 'C4', 203, 0, 113, 8 ) if $_[0] eq 'beef.face'; + return $system_inet_aton->(@_); + }; + is_deeply( + [ xCAT_plugin::ipmi::_resolve_ipv4_octets('beef.face') ], + [ '203.0.113.8', 203, 0, 113, 8 ], + 'a hostname made of hexadecimal characters still resolves' + ); + } + + foreach my $setting ( + 'ip=192.168.001.010', 'gateway=192.168.257', + 'ip=0xc0a8010a', 'gateway=0xc0.0xa8.0x01.0x0a', + 'backupgateway=0X0A141E02', 'snmpdest1=0xcb007107', + ) { + my ( undef, $value ) = split /=/, $setting; + my $result = run_setting($setting); + ok( $result->{succeeded}, "$setting does not raise a Perl exception" ) + or diag $result->{error}; + is_deeply( $result->{calls}, [], "$setting sends no IPMI command" ); + is_deeply( + $result->{messages}->[0]->[0], + [ 1, "Unable to resolve '$value' to an IPv4 address" ], + "$setting reports the rejection" + ); + } +}; + 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'); From 5d4d2938813d983965be7e2d50935ec6ba19fdbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:54:25 -0300 Subject: [PATCH 3/6] fix(ipmi): resolve IPv4 settings once per transaction --- xCAT-server/lib/xcat/plugins/ipmi.pm | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/ipmi.pm b/xCAT-server/lib/xcat/plugins/ipmi.pm index 6afceba10..94db10a66 100644 --- a/xCAT-server/lib/xcat/plugins/ipmi.pm +++ b/xCAT-server/lib/xcat/plugins/ipmi.pm @@ -892,6 +892,22 @@ sub _resolve_ipv4_octets { return (inet_ntoa($packed_address), unpack("C4", $packed_address)); } +sub _session_ipv4_octets { + my $sessdata = shift; + my $value = shift; + return unless defined($value) and length($value); + + # setnetinfo re-enters itself between the transaction steps. Keep the + # first resolution for the whole session so a repeated lookup cannot + # fail after Set In Progress opened the transaction. + my $cached = $sessdata->{resolved_ipv4}{$value}; + unless ($cached) { + $cached = [ _resolve_ipv4_octets($value) ]; + $sessdata->{resolved_ipv4}{$value} = $cached; + } + return @{$cached}; +} + sub _report_unresolvable_ipv4 { my $value = shift; my $sessdata = shift; @@ -974,7 +990,7 @@ sub setnetinfo { } elsif ($subcommand =~ m/snmpdest(\d+)/) { my $destination = $1; - my ($dstip, @dip) = _resolve_ipv4_octets($argument); + my ($dstip, @dip) = _session_ipv4_octets($sessdata, $argument); unless (defined($dstip)) { _report_unresolvable_ipv4($argument, $sessdata); return; @@ -990,7 +1006,7 @@ sub setnetinfo { @cmd = (0x01, $channel_number, 0x6, @mask); } } elsif ($subcommand eq "gateway" and $argument) { - my ($gw, @octets) = _resolve_ipv4_octets($argument); + my ($gw, @octets) = _session_ipv4_octets($sessdata, $argument); unless (defined($gw)) { _report_unresolvable_ipv4($argument, $sessdata); return; @@ -998,7 +1014,7 @@ sub setnetinfo { $sessdata->{setnetinfo_value} = $gw; @cmd = (0x01, $channel_number, 0x0C, @octets); } elsif ($subcommand eq "backupgateway" and $argument) { - my ($gw, @octets) = _resolve_ipv4_octets($argument); + my ($gw, @octets) = _session_ipv4_octets($sessdata, $argument); unless (defined($gw)) { _report_unresolvable_ipv4($argument, $sessdata); return; @@ -1034,7 +1050,7 @@ sub setnetinfo { return; } } - my ($mip, @octets) = _resolve_ipv4_octets($argument); + my ($mip, @octets) = _session_ipv4_octets($sessdata, $argument); unless (defined($mip)) { _report_unresolvable_ipv4($argument, $sessdata); return; From 7fb2fac0fbacbee142395e1fa42ace1e7da986d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 13:55:02 -0300 Subject: [PATCH 4/6] test(ipmi): cover transaction-scoped IPv4 resolution --- xCAT-test/unit/ipmi_ipv4_command_encoding.t | 81 +++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/xCAT-test/unit/ipmi_ipv4_command_encoding.t b/xCAT-test/unit/ipmi_ipv4_command_encoding.t index a3a2ebb85..c6dc3a76f 100644 --- a/xCAT-test/unit/ipmi_ipv4_command_encoding.t +++ b/xCAT-test/unit/ipmi_ipv4_command_encoding.t @@ -59,6 +59,25 @@ sub subcmd { return; } +package Local::IPMIFlowSession; + +sub new { + my ( $class, $channel ) = @_; + return bless { currentchannel => $channel, calls => [] }, $class; +} + +sub subcmd { + my ( $self, %args ) = @_; + push @{ $self->{calls} }, \%args; + + # Answer LAN configuration writes so the transaction chain advances; + # leave the final read-back request unanswered to end the exchange. + if ( $args{command} == 0x01 ) { + $args{callback}->( {}, $args{callback_args} ); + } + return; +} + package main; no warnings qw(once redefine); @@ -261,6 +280,68 @@ subtest 'ambiguous IPv4 literals are rejected' => sub { } }; +subtest 'IPv4 settings resolve once per transaction' => sub { + plan skip_all => 'the shared IPv4 resolver is not present on the base revision' + unless xCAT_plugin::ipmi->can('_resolve_ipv4_octets'); + + my @cases = ( + { + setting => 'gateway=bmc.flaky.test', + sequence => [ + [ 2, 0x00, 0x01 ], + [ 2, 0x0c, 10, 20, 30, 40 ], + [ 2, 0x00, 0x00 ], + ], + }, + { + setting => 'ip=bmc.flaky.test', + sequence => [ + [ 2, 0x00, 0x01 ], + [ 2, 0x04, 0x01 ], + [ 2, 0x03, 10, 20, 30, 40 ], + [ 2, 0x00, 0x00 ], + ], + }, + ); + + foreach my $case (@cases) { + my $lookups = 0; + my $session; + my $session_data; + { + # The lookup succeeds once and then fails, like transient DNS. + local *xCAT_plugin::ipmi::inet_aton = sub { + return pack( 'C4', 10, 20, 30, 40 ) if ++$lookups == 1; + return; + }; + $session = Local::IPMIFlowSession->new( 2 ); + $session_data = { + bmcnum => 1, + ipmisession => $session, + node => 'node01', + subcommand => $case->{setting}, + }; + @messages = (); + my $ok = eval { xCAT_plugin::ipmi::setnetinfo($session_data); 1 }; + ok( $ok, "$case->{setting} completes the transaction" ) or diag $@; + } + is( $lookups, 1, "$case->{setting} resolves exactly once" ); + my @writes = grep { $_->{command} == 0x01 } @{ $session->{calls} }; + is_deeply( + [ map { $_->{data} } @writes ], + $case->{sequence}, + "$case->{setting} opens, writes, and closes the transaction" + ); + # netinfo_set issues the read-back twice (it schedules Set Complete + # and falls through); require only that the chain reaches it. + cmp_ok( scalar( grep { $_->{command} == 0x02 } @{ $session->{calls} } ), + '>=', 1, "$case->{setting} reaches the read-back request" ); + is_deeply( [@messages], [], "$case->{setting} reports no error" ); + is( $session_data->{setnetinfo_value}, '10.20.30.40', + "$case->{setting} keeps the resolved readback value" ); + } +}; + 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'); From 0518b916b1d6f0db4e9bb1012e9642441b57e684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:25:55 -0300 Subject: [PATCH 5/6] fix(ipmi): validate netmask values before writing them --- xCAT-server/lib/xcat/plugins/ipmi.pm | 48 +++++++++++++++++++++------- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/ipmi.pm b/xCAT-server/lib/xcat/plugins/ipmi.pm index 94db10a66..825e220b5 100644 --- a/xCAT-server/lib/xcat/plugins/ipmi.pm +++ b/xCAT-server/lib/xcat/plugins/ipmi.pm @@ -869,6 +869,30 @@ sub next_setnetinfo { &setnetinfo($sessdata); } +sub _ipv4_literal_octets { + my $value = shift; + return unless defined($value) and length($value); + + my @components = split(/\./, $value, -1); + return unless scalar(@components) == 4; + foreach my $octet (@components) { + return unless $octet =~ m/^(?:0|[1-9][0-9]{0,2})$/ and $octet <= 255; + } + return map { $_ + 0 } @components; +} + +sub _netmask_octets { + my $value = shift; + + # A netmask is a literal, never a hostname, and its set bits must be + # contiguous or the BMC would apply a mask the operator did not intend. + my @octets = _ipv4_literal_octets($value); + return unless @octets; + my $host_bits = ~unpack('N', pack('C4', @octets)) & 0xffffffff; + return unless ($host_bits & ($host_bits + 1)) == 0; + return @octets; +} + sub _resolve_ipv4_octets { my $value = shift; return unless defined($value) and length($value); @@ -879,11 +903,9 @@ sub _resolve_ipv4_octets { # dotted-decimal quad so every platform encodes the same address. my @components = split(/\./, $value, -1); unless (grep { $_ !~ m/^(?:0[xX][0-9a-fA-F]+|[0-9]+)$/ } @components) { - return unless scalar(@components) == 4; - foreach my $octet (@components) { - return unless $octet =~ m/^(?:0|[1-9][0-9]{0,2})$/ and $octet <= 255; - } - return ($value, map { $_ + 0 } @components); + my @octets = _ipv4_literal_octets($value); + return unless @octets; + return ($value, @octets); } my $packed_address = inet_aton($value); @@ -997,14 +1019,16 @@ sub setnetinfo { } @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; - foreach (0 .. 3) { - $mask[$_] = $mask[$_] + 0; - } - $sessdata->{setnetinfo_value} = join(".", @mask); - @cmd = (0x01, $channel_number, 0x6, @mask); + my @mask = _netmask_octets($argument); + unless (@mask) { + $argument = '' unless defined($argument); + xCAT::SvrUtils::sendmsg( + [ 1, "'$argument' is not a valid netmask" ], + $callback, $sessdata->{node}, %allerrornodes); + return; } + $sessdata->{setnetinfo_value} = join(".", @mask); + @cmd = (0x01, $channel_number, 0x6, @mask); } elsif ($subcommand eq "gateway" and $argument) { my ($gw, @octets) = _session_ipv4_octets($sessdata, $argument); unless (defined($gw)) { From d96ddf1f893e69ad4b5732ba9bb0a8bdd6981725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vin=C3=ADcius=20Ferr=C3=A3o?= <2031761+viniciusferrao@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:26:26 -0300 Subject: [PATCH 6/6] test(ipmi): cover netmask validation --- xCAT-test/unit/ipmi_ipv4_command_encoding.t | 46 +++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/xCAT-test/unit/ipmi_ipv4_command_encoding.t b/xCAT-test/unit/ipmi_ipv4_command_encoding.t index c6dc3a76f..176239310 100644 --- a/xCAT-test/unit/ipmi_ipv4_command_encoding.t +++ b/xCAT-test/unit/ipmi_ipv4_command_encoding.t @@ -137,6 +137,11 @@ subtest 'IPv4 settings preserve their wire command bytes' => sub { data => [ 2, 0x03, 0, 0, 0, 0 ], value => '0.0.0.0', }, + { + setting => 'netmask=255.255.255.0', + data => [ 2, 0x06, 255, 255, 255, 0 ], + value => '255.255.255.0', + }, { setting => 'gateway=10.20.30.1', data => [ 2, 0x0c, 10, 20, 30, 1 ], @@ -342,6 +347,47 @@ subtest 'IPv4 settings resolve once per transaction' => sub { } }; +subtest 'netmask values must be contiguous dotted-decimal masks' => sub { + plan skip_all => 'netmask validation is not present on the base revision' + unless xCAT_plugin::ipmi->can('_resolve_ipv4_octets'); + + my @accepted = ( + [ '255.255.255.255', [ 2, 0x06, 255, 255, 255, 255 ] ], + [ '128.0.0.0', [ 2, 0x06, 128, 0, 0, 0 ] ], + [ '0.0.0.0', [ 2, 0x06, 0, 0, 0, 0 ] ], + ); + foreach my $case (@accepted) { + my ( $value, $data ) = @{$case}; + my $result = run_setting("netmask=$value"); + is( scalar( @{ $result->{calls} } ), 1, "netmask=$value sends one command" ); + is_deeply( $result->{calls}->[0]->{data}, $data, + "netmask=$value keeps its wire bytes" ); + is_deeply( $result->{messages}, [], "netmask=$value reports no error" ); + } + + my @rejected = ( + [ '999.999.999.999', 'octets above 255' ], + [ '255.255.255', 'three-part form' ], + [ '255.255.255.255.255', 'five-part form' ], + [ '255.0.255.0', 'noncontiguous mask bits' ], + [ '255.255.255.256', 'octet above 255' ], + [ '255.255.255.010', 'leading-zero octet' ], + [ '24', 'prefix length form' ], + ); + foreach my $case (@rejected) { + my ( $value, $reason ) = @{$case}; + my $result = run_setting("netmask=$value"); + ok( $result->{succeeded}, "netmask=$value does not raise a Perl exception" ) + or diag $result->{error}; + is_deeply( $result->{calls}, [], "netmask=$value sends no IPMI command: $reason" ); + is_deeply( + $result->{messages}->[0]->[0], + [ 1, "'$value' is not a valid netmask" ], + "netmask=$value reports the rejection" + ); + } +}; + 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');