xCAT/xcat-core
2
0
Fork 0
mirror of https://github.com/xcat2/xcat-core.git synced 2026-09-04 12:07:56 +00:00
Code Issues Releases Wiki Activity

Merge pull request #7799 from VersatusHPC/fix/ipmi-ipv4-literal-guardrail

Browse Source 
fix(ipmi): reject ambiguous IPv4 literals
This commit is contained in:
Daniel Hilst
2026-09-03 20:27:14 -03:00
committed by GitHub
parent 9f4e53e380 d96ddf1f89
commit e08fe95959
2 changed files with 260 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
xCAT-server/lib/xcat/plugins/ipmi.pm
+64 -11
View File
@@ -869,16 +869,67 @@ 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);
# 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) {
my @octets = _ipv4_literal_octets($value);
return unless @octets;
return ($value, @octets);
}
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 _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;
@@ -961,23 +1012,25 @@ 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;
}
@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) = _resolve_ipv4_octets($argument);
my ($gw, @octets) = _session_ipv4_octets($sessdata, $argument);
unless (defined($gw)) {
_report_unresolvable_ipv4($argument, $sessdata);
return;
@@ -985,7 +1038,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;
@@ -1021,7 +1074,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;
xCAT-test/unit/ipmi_ipv4_command_encoding.t
+196
View File
@@ -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);
@@ -113,6 +132,16 @@ 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 => '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 ],
@@ -192,6 +221,173 @@ 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 '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 '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');