enh: config: better parsing and normalization

We now warn (to syslog) for invalid values where
we have to fallback to defaults, and for boolean
options, actual true/false JSON values are now
properly recognized. 0 and 1 will still always
be parsed correctly, to not break compatibility.
This commit is contained in:
Stéphane Lesimple 2020-12-14 12:55:42 +00:00
parent 398c04c273
commit ef6efa6dc3
No known key found for this signature in database
GPG key ID: 4B4A3289E9D35658
4 changed files with 579 additions and 158 deletions

View file

@ -55,13 +55,26 @@ my $_cache_config = undef;
sub load_configuration { sub load_configuration {
my %params = @_; my %params = @_;
my $mock_data = $params{'mock_data'}; my $mock_data = $params{'mock_data'};
my $noisy = $params{'noisy'}; # print warnings/errors on stdout in addition to syslog
my $test = $params{'test'}; # noisy + also print missing configuration options
if (defined $mock_data && !OVH::Bastion::is_mocking()) { # do NOT use warn_syslog in this func, or any other function that needs to read configuration,
# or we might end up in an infinite loop: store errors we wanna log at the end
my @errors;
$noisy = 1 if $test;
if (defined $mock_data) {
if (!OVH::Bastion::is_mocking()) {
# if we're overriding configuration with mock_data without being in mocking mode, we have a problem # if we're overriding configuration with mock_data without being in mocking mode, we have a problem
die("Attempted to load_configuration() with mock_data without being in mocking mode"); die("Attempted to load_configuration() with mock_data without being in mocking mode");
} }
# mock data always overrides cache
undef $_cache_config;
}
if (ref $_cache_config eq 'HASH') { if (ref $_cache_config eq 'HASH') {
return R('OK', value => $_cache_config); return R('OK', value => $_cache_config);
} }
@ -83,7 +96,8 @@ sub load_configuration {
$C = $mock_data; $C = $mock_data;
} }
# define deprecated <=> new key names association # define deprecated vs new key names association
# new old
my %new2old = qw( my %new2old = qw(
accountCreateDefaultPersonalAccesses accountCreateDefaultPrivateAccesses accountCreateDefaultPersonalAccesses accountCreateDefaultPrivateAccesses
adminAccounts adminLogins adminAccounts adminLogins
@ -103,152 +117,391 @@ sub load_configuration {
$C->{$new} //= $C->{$old}; $C->{$new} //= $C->{$old};
} }
# now validate, lint and normalize the conf ####################################################
# now validate, lint, normalize and untaint the conf
$C->{'bastionName'} ||= 'fix-my-config-please-missing-bastion-name'; my %unknownkeys = map { $_ => 1 } keys %$C;
$C->{'bastionCommand'} ||= "ssh ACCOUNT\@HOSTNAME -t -- "; # 1/6) Options that are strings, and must match given regex. Always include capturing parens in regex for untainting.
foreach my $o (
{name => 'bastionName', default => 'fix-my-config-please-missing-bastion-name', validre => qr/^([a-zA-Z0-9_.-]+)$/},
{name => 'bastionCommand', default => "ssh ACCOUNT\@HOSTNAME -t -- ", validre => qr/^(.+)$/},
{name => 'defaultLogin', default => "", validre => qr/^([a-zA-Z0-9_.-]*)$/, emptyok => 1},
{name => 'moshCommandLine', default => "", validre => qr/^(.*)$/, emptyok => 1},
{name => 'documentationURL', default => "https://ovh.github.io/the-bastion/", validre => qr'^([a-zA-Z0-9:/@&=",;_.-]+)$'},
{name => 'syslogFacility', default => 'local7', validre => qr/^([a-zA-Z0-9_]+)$/},
{name => 'syslogDescription', default => 'bastion', validre => qr/^([a-zA-Z0-9_.-]+)$/},
{name => 'ttyrecFilenameFormat', default => '%Y-%m-%d.%H-%M-%S.#usec#.&uniqid.ttyrec', validre => qr/^([a-zA-Z0-9%&#_.-]+)$/},
{name => 'accountExpiredMessage', default => '', validre => qr/^(.*)$/, emptyok => 1},
{name => 'accountExternalValidationProgram', default => '', validre => qr'^([a-zA-Z0-9/$_.-]*)$', emptyok => 1},
)
{
if (!$C->{$o->{'name'}} && !$o->{'emptyok'}) {
$C->{$o->{'name'}} = $o->{'default'};
push @errors, "Configuration error: missing option '" . $o->{'name'} . "', defaulting to '" . $o->{'default'} . "'" if $test;
}
if ($C->{$o->{'name'}} =~ $o->{'validre'}) {
$C->{'defaultLogin'} = "" if (not defined $C->{'defaultLogin'}); # untaint
$C->{'defaultLogin'} =~ s/[^a-zA-Z0-9_-]//g; $C->{$o->{'name'}} = $1;
}
$C->{'accountUidMin'} = 2000 if (not defined $C->{'accountUidMin'} or $C->{'accountUidMin'} !~ /^\d+$/); else {
$C->{'accountUidMin'} > 100 or $C->{'accountUidMin'} = 100; push @errors,
$C->{'accountUidMin'} < 999_999_999 or $C->{'accountUidMin'} = 999_999_999; # usually 2^31-2 but well... "Configuration error: value of option '" . $o->{'name'} . "' ('" . $C->{$o->{'name'}} . "') didn't match allowed regex, defaulting to '" . $o->{'default'} . "'";
$C->{$o->{'name'}} = $o->{'default'};
$C->{'accountUidMax'} = 99999 if (not defined $C->{'accountUidMax'} or $C->{'accountUidMax'} !~ /^\d+$/); }
$C->{'accountUidMax'} > 100 or $C->{'accountUidMax'} = 100; delete $unknownkeys{$o->{'name'}};
$C->{'accountUidMax'} < 999_999_999 or $C->{'accountUidMax'} = 999_999_999; # usually 2^31-2 but well...
$C->{'accountUidMax'} = $C->{'accountUidMin'} + 1000 if ($C->{'accountUidMin'} + 1000 > $C->{'accountUidMax'});
$C->{'ttyrecGroupIdOffset'} = 100_000 if (not defined $C->{'ttyrecGroupIdOffset'} or $C->{'ttyrecGroupIdOffset'} !~ /^\d+$/);
$C->{'ttyrecGroupIdOffset'} < 999_999_999 or $C->{'ttyrecGroupIdOffset'} = 999_999_999;
if ($C->{'ttyrecGroupIdOffset'} < $C->{'accountUidMax'} - $C->{'accountUidMin'}) {
# avoid overlap
$C->{'ttyrecGroupIdOffset'} = ($C->{'accountUidMax'} - $C->{'accountUidMin'}) + 1;
} }
foreach my $key (qw{ allowedIngressSshAlgorithms allowedIngressSshAlgorithms }) { # 2/6) Options that must be numbers, between min and max.
$C->{$key} = ['rsa', 'ecdsa', 'ed25519'] if (not defined $C->{$key} or ref $C->{$key} ne 'ARRAY'); foreach my $o (
{name => 'accountUidMin', min => 100, max => 999_999_999, default => 2000},
{name => 'accountUidMax', min => 100, max => 999_999_999, default => 99999},
{name => 'ttyrecGroupIdOffset', min => 1, max => 999_999_999, default => 100_000},
{name => 'minimumIngressRsaKeySize', min => 1024, max => 16384, default => 2048},
{name => 'minimumEgressRsaKeySize', min => 1024, max => 16384, default => 2048},
{name => 'maximumIngressRsaKeySize', min => 1024, max => 32768, default => 8192},
{name => 'maximumEgressRsaKeySize', min => 1024, max => 32768, default => 8192},
{name => 'moshTimeoutNetwork', min => 0, max => 86400 * 365, default => 86400},
{name => 'moshTimeoutSignal', min => 0, max => 86400 * 365, default => 30},
{name => 'idleLockTimeout', min => 0, max => 86400 * 365, default => 0},
{name => 'idleKillTimeout', min => 0, max => 86400 * 365, default => 0},
{name => 'warnBeforeLockSeconds', min => 0, max => 86400 * 365, default => 0},
{name => 'warnBeforeKillSeconds', min => 0, max => 86400 * 365, default => 0},
{name => 'MFAPasswordInactiveDays', min => -1, max => 365 * 5, default => -1},
{name => 'MFAPasswordMinDays', min => 0, max => 365 * 5, default => 0},
{name => 'MFAPasswordMaxDays', min => 0, max => 365 * 5, default => 90},
{name => 'MFAPasswordWarnDays', min => 0, max => 365 * 5, default => 15},
{name => 'sshClientDebugLevel', min => 0, max => 3, default => 0},
{name => 'accountMaxInactiveDays', min => 0, max => 365 * 5, default => 0},
{name => 'interactiveModeTimeout', min => 0, max => 86400 * 365, default => 15},
)
{
if (not defined $C->{$o->{'name'}}) {
$C->{$o->{'name'}} = $o->{'default'};
push @errors, "Configuration error: missing option '" . $o->{'name'} . "', defaulting to " . $o->{'default'} if $test;
}
if ($C->{$o->{'name'}} =~ /^(-?\d+)$/) {
# untaint
$C->{$o->{'name'}} = $1;
}
else {
push @errors, "Configuration error: value of option '" . $o->{'name'} . "' ('" . $C->{$o->{'name'}} . "') is not a number, defaulting to " . $o->{'default'};
$C->{$o->{'name'}} = $o->{'default'};
}
if ($C->{$o->{'name'}} > $o->{'max'}) {
push @errors,
"Configuration error: value of option '"
. $o->{'name'} . "' ("
. $C->{$o->{'name'}}
. ") is higher than allowed value ("
. $o->{'max'}
. "), defaulting to "
. $o->{'default'};
$C->{$o->{'name'}} = $o->{'default'};
}
elsif ($C->{$o->{'name'}} < $o->{'min'}) {
push @errors,
"Configuration error: value of option '"
. $o->{'name'} . "' ("
. $C->{$o->{'name'}}
. ") is lower than allowed value ("
. $o->{'min'}
. "), defaulting to "
. $o->{'default'};
$C->{$o->{'name'}} = $o->{'default'};
}
delete $unknownkeys{$o->{'name'}};
} }
foreach my $key (qw{ Ingress Egress }) { # 3/6) Booleans. Standard true/false values should be used in the JSON config file, but we normalize non-bool values here.
my $minkey = "minimum${key}RsaKeySize"; # We cast the strings "no", "false", "disabled" to false. The JSON null value is also false.
my $maxkey = "maximum${key}RsaKeySize"; # For all other values, standard Perl applies: 0, "0", "" are false, everything else is true.
$C->{$minkey} = 2048 if (not defined $C->{$minkey} or $C->{$minkey} !~ /^\d+$/); # We warn where we have to cast, except for 0/1/"0"/"1" for backwards compatibility.
$C->{$minkey} >= 1024 or $C->{$minkey} = 1024; foreach my $tuple (
$C->{$minkey} <= 16384 or $C->{$minkey} = 16384; {
$C->{$maxkey} = 8192 if (not defined $C->{$maxkey} or $C->{$maxkey} !~ /^\d+$/); default => 1,
$C->{$maxkey} >= 1024 or $C->{$maxkey} = 1024; options => [
$C->{$maxkey} <= 32768 or $C->{$maxkey} = 32768;
$C->{$minkey} = $C->{$maxkey} if ($C->{$minkey} > $C->{$maxkey}); # ensure min <= max
}
$C->{'defaultAccountEgressKeyAlgorithm'} ||= 'rsa';
if (!grep { $C->{'defaultAccountEgressKeyAlgorithm'} eq $_ } qw{ rsa ecdsa ed25519 }) {
$C->{'defaultAccountEgressKeyAlgorithm'} = 'rsa';
}
$C->{'defaultAccountEgressKeySize'} = 0 if $C->{'defaultAccountEgressKeySize'} !~ /^\d+$/;
if ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'rsa') {
$C->{'defaultAccountEgressKeySize'} ||= 4096;
$C->{'defaultAccountEgressKeySize'} = 1024 if $C->{'defaultAccountEgressKeySize'} < 1024;
$C->{'defaultAccountEgressKeySize'} = 32768 if $C->{'defaultAccountEgressKeySize'} > 32768;
}
elsif ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'ecdsa') {
$C->{'defaultAccountEgressKeySize'} ||= 521;
if (!grep { $C->{'defaultAccountEgressKeySize'} eq $_ } qw{ 256 384 521 }) {
$C->{'defaultAccountEgressKeySize'} = 521;
}
}
elsif ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'ed25519') {
$C->{'defaultAccountEgressKeySize'} = 256;
}
$C->{'sshClientDebugLevel'} = 0 if (not defined $C->{'sshClientDebugLevel'} or $C->{'sshClientDebugLevel'} !~ /^\d+$/);
$C->{'sshClientDebugLevel'} > 3 and $C->{'sshClientDebugLevel'} = 3;
$C->{'accountMaxInactiveDays'} = 0
if (not defined $C->{'accountMaxInactiveDays'} or $C->{'accountMaxInactiveDays'} !~ /^\d+$/);
$C->{'interactiveModeTimeout'} = 15
if (not defined $C->{'interactiveModeTimeout'} or $C->{'interactiveModeTimeout'} !~ /^\d+$/);
$C->{'syslogFacility'} = 'local7' if (not defined $C->{'syslogFacility'} or $C->{'syslogFacility'} !~ /^\S+$/);
$C->{'syslogDescription'} = 'bastion' if (not defined $C->{'syslogDescription'} or $C->{'syslogDescription'} !~ /^\S+$/);
$C->{'moshTimeoutNetwork'} = 86400 if (not defined $C->{'moshTimeoutNetwork'} or $C->{'moshTimeoutNetwork'} !~ /^\d+$/);
$C->{'moshTimeoutSignal'} = 30 if (not defined $C->{'moshTimeoutSignal'} or $C->{'moshTimeoutSignal'} !~ /^\d+$/);
$C->{'moshCommandLine'} = "" if (not defined $C->{'moshCommandLine'});
$C->{'ttyrecFilenameFormat'} = '%Y-%m-%d.%H-%M-%S.#usec#.&uniqid.ttyrec' if (not $C->{'ttyrecFilenameFormat'});
$C->{'idleLockTimeout'} = 0 if (not defined $C->{'idleLockTimeout'} or $C->{'idleLockTimeout'} !~ /^\d+$/);
$C->{'idleKillTimeout'} = 0 if (not defined $C->{'idleKillTimeout'} or $C->{'idleKillTimeout'} !~ /^\d+$/);
$C->{'warnBeforeLockSeconds'} = 0 if (not defined $C->{'warnBeforeLockSeconds'} or $C->{'warnBeforeLockSeconds'} !~ /^\d+$/);
$C->{'warnBeforeKillSeconds'} = 0 if (not defined $C->{'warnBeforeKillSeconds'} or $C->{'warnBeforeKillSeconds'} !~ /^\d+$/);
if (!grep { $C->{'accountMFAPolicy'} eq $_ } qw{ disabled enabled password-required totp-required any-required }) {
$C->{'accountMFAPolicy'} = 'enabled';
}
$C->{'MFAPasswordInactiveDays'} = -1 if (!defined $C->{'MFAPasswordInactiveDays'} || $C->{'MFAPasswordInactiveDays'} !~ /^-\d+$/);
$C->{'MFAPasswordMinDays'} = 0 if (!defined $C->{'MFAPasswordMinDays'} || $C->{'MFAPasswordMinDays'} !~ /^-?\d+$/);
$C->{'MFAPasswordMaxDays'} = 90 if (!defined $C->{'MFAPasswordMaxDays'} || $C->{'MFAPasswordMaxDays'} !~ /^-?\d+$/);
$C->{'MFAPasswordWarnDays'} = 15 if (!defined $C->{'MFAPasswordWarnDays'} || $C->{'MFAPasswordWarnDays'} !~ /^-?\d+$/);
# if kill timeout is lower than lock timeout, just unset lock timeout
$C->{'idleLockTimeout'} = 0 if ($C->{'idleKillTimeout'} <= $C->{'idleLockTimeout'});
# booleans that can only be 0 or 1 and default to 1
foreach my $key (
qw{ qw{
enableSyslog enableGlobalAccessLog enableAccountAccessLog enableGlobalSqlLog enableAccountSqlLog displayLastLogin enableSyslog enableGlobalAccessLog enableAccountAccessLog enableGlobalSqlLog enableAccountSqlLog displayLastLogin
interactiveModeByDefault interactiveModeByDefault
} }
) ],
},
{ {
$C->{$key} = 1 if (not defined $C->{$key} or $C->{$key} !~ /^\d+$/); default => 0,
$C->{$key} > 1 and $C->{$key} = 1; options => [
} qw{
interactiveModeAllowed readOnlySlaveMode sshClientHasOptionE ingressKeysFromAllowOverride
# booleans that can only be 0 or 1 and default to 0
foreach my $key (
qw{ interactiveModeAllowed readOnlySlaveMode sshClientHasOptionE ingressKeysFromAllowOverride
moshAllowed debug keyboardInteractiveAllowed passwordAllowed telnetAllowed remoteCommandEscapeByDefault moshAllowed debug keyboardInteractiveAllowed passwordAllowed telnetAllowed remoteCommandEscapeByDefault
accountExternalValidationDenyOnFailure } accountExternalValidationDenyOnFailure
}
],
}
) )
{ {
$C->{$key} = 0 if (not defined $C->{$key} or $C->{$key} !~ /^\d+$/); foreach my $o (@{$tuple->{'options'}}) {
$C->{$key} > 1 and $C->{$key} = 1;
# if not defined, set to default value
if (not defined $C->{$o}) {
$C->{$o} = $tuple->{'default'};
push @errors, "Configuration error: missing option '$o', defaulting to " . ($tuple->{'default'} ? 'true' : 'false') if $test;
} }
# arrays that default to empty # if set to "no", "false" or "disabled", be nice and cast to false (because a string is true, otherwise)
foreach my $key ( elsif (grep { lc($C->{$o}) eq lc($_) } qw{ no false disabled }) {
qw{ accountCreateSupplementaryGroups accountCreateDefaultPrivateAccesses alwaysActiveAccounts push @errors, "Configuration error: found value '" . $C->{$o} . "' for option '$o', but it's supposed to be a boolean, assuming false";
superOwnerAccounts ingressKeysFrom egressKeysFrom adminAccounts allowedNetworks forbiddenNetworks $C->{$o} = 0;
ttyrecAdditionalParameters MFAPostCommand } }
# if 0 or 1, normalize silently (backwards compatible)
elsif ($C->{$o} == 0 || $C->{$o} == 1) {
$C->{$o} = $C->{$o} ? 1 : 0;
}
# otherwise, normalize any true/false value to 1/0, and log a warning
else {
push @errors, "Configuration error: found value '" . $C->{$o} . "' for option '$o', but it's supposed to be a boolean, assuming " . ($C->{$o} ? 'true' : 'false');
$C->{$o} = $C->{$o} ? 1 : 0;
}
delete $unknownkeys{$o};
}
}
# 4/6) Strings that must be one item of a specific enum.
foreach my $o (
{name => 'defaultAccountEgressKeyAlgorithm', default => 'rsa', valid => [qw{ rsa ecdsa ed25519 }]},
{name => 'accountMFAPolicy', default => 'enabled', valid => [qw{ disabled enabled password-required totp-required any-required }]},
) )
{ {
$C->{$key} = [] if ref $C->{$key} ne 'ARRAY'; # if not defined, set to default value
if (not defined $C->{$o->{'name'}}) {
$C->{$o->{'name'}} = $o->{'default'};
push @errors, "Configuration error: missing option '" . $o->{'name'} . "', defaulting to '" . (join(" ", @{$o->{'default'}})) . "'" if $test;
} }
# lint the contents of some arrays # must be one of the allowed values
foreach my $key (qw{ ingressKeysFrom egressKeysFrom }) { elsif (my @untainted = grep { $C->{$o->{'name'}} eq $_ } @{$o->{'valid'}}) {
s=[^0-9.:/]==g for @{$C->{$key}}; $C->{$o->{'name'}} = $untainted[0];
} }
$C->{'adminAccounts'} = [grep { OVH::Bastion::is_bastion_account_valid_and_existing(account => $_) } @{$C->{'adminAccounts'}}]; else {
push @errors, "Configuration error: option '" . $o->{'name'} . "' should be one of [" . (join(",", @{$o->{'valid'}})) . "] instead of '" . $C->{$o->{'name'}} . "'";
$C->{'documentationURL'} ||= "https://ovh.github.io/the-bastion/"; $C->{$o->{'name'}} = $o->{'default'};
}
# we've checked everything. now forcibly untaint all of it. delete $unknownkeys{$o->{'name'}};
foreach my $key (keys %$C) {
ref $C->{$key} eq '' or next;
($C->{$key}) = $C->{$key} =~ m{(.+)};
$C->{$key} += 0 if $C->{$key} =~ /^\d+$/;
} }
# 5/6) Arrays whose values should match a specific regex.
foreach my $o (
{name => 'allowedIngressSshAlgorithms', default => [qw{ rsa ecdsa ed25519 }], validre => qr/^(rsa|ecdsa|ed25519)$/},
{name => 'allowedEgressSshAlgorithms', default => [qw{ rsa ecdsa ed25519 }], validre => qr/^(rsa|ecdsa|ed25519)$/},
{name => 'accountCreateSupplementaryGroups', default => [], validre => qr/^(.*)$/},
{name => 'accountCreateDefaultPersonalAccesses', default => [], validre => qr/^(.*)$/},
{name => 'alwaysActiveAccounts', default => [], validre => qr/^(.*)$/},
{name => 'superOwnerAccounts', default => [], validre => qr/^(.*)$/},
{name => 'ingressKeysFrom', default => [], validre => qr'^([0-9.:%/]+)$'},
{name => 'egressKeysFrom', default => [], validre => qr'^([0-9.:%/]+)$'},
{name => 'adminAccounts', default => [], validre => qr/^(.*)$/},
{name => 'allowedNetworks', default => [], validre => qr'^([0-9.:%/]+)$'},
{name => 'forbiddenNetworks', default => [], validre => qr'^([0-9.:%/]+)$'},
{name => 'ttyrecAdditionalParameters', default => [], validre => qr/^(.*)$/},
{name => 'MFAPostCommand', default => [], validre => qr/^(.*)$/},
)
{
# if not defined, set to default value
if (not defined $C->{$o->{'name'}}) {
$C->{$o->{'name'}} = $o->{'default'};
push @errors, "Configuration error: missing option '" . $o->{'name'} . "', defaulting to [" . (join(",", @{$o->{'default'}})) . "]" if $test;
}
# must be an array
elsif (ref $C->{$o->{'name'}} ne 'ARRAY') {
$C->{$o->{'name'}} = $o->{'default'};
push @errors, "Configuration error: options option '" . $o->{'name'} . "' should be an array, defaulting to [" . (join(" ", @{$o->{'default'}})) . "]";
}
# whose values validate the regex
else {
my @untainted;
foreach my $v (@{$C->{$o->{'name'}}}) {
if ($v =~ $o->{'validre'}) {
push @untainted, $1;
}
else {
push @errors,
"Configuration error: at least one of the values of the array defined by option '"
. $o->{'name'}
. "' is invalid, defaulting to ["
. (join(" ", @{$o->{'default'}})) . "]";
$C->{$o->{'name'}} = $o->{'default'};
last;
}
}
$C->{$o->{'name'}} = \@untainted;
}
delete $unknownkeys{$o->{'name'}};
}
# 6/6) Special cases and/or additional checks for already vetted options
# ... we must have enough room between min and max
if ($C->{'accountUidMin'} + 1000 > $C->{'accountUidMax'}) {
push @errors,
"Configuration error: 'accountUidMax' ("
. $C->{'accountUidMax'}
. ") is too close from 'accountUidMin' ("
. $C->{'accountUidMin'}
. "), setting accountUidMax="
. ($C->{'accountUidMin'} + 1000);
$C->{'accountUidMax'} = $C->{'accountUidMin'} + 1000;
}
# ... ttyrec group offset must be high enough to avoid overlap with the accounts uids
if ($C->{'ttyrecGroupIdOffset'} < $C->{'accountUidMax'} - $C->{'accountUidMin'}) {
my $fixed = ($C->{'accountUidMax'} - $C->{'accountUidMin'}) + 1;
push @errors, "Configuration error: the configured 'ttyrecGroupIdOffset' (" . $C->{'ttyrecGroupIdOffset'} . ") would overlap with account UIDs, setting it to $fixed";
$C->{'ttyrecGroupIdOffset'} = $fixed;
}
# ... ensure min <= max
foreach my $key (qw{ Ingress Egress }) {
my $minkey = "minimum${key}RsaKeySize";
my $maxkey = "maximum${key}RsaKeySize";
if ($C->{$minkey} > $C->{$maxkey}) {
push @errors, "Configuration error: '$minkey' (" . $C->{$minkey} . ") must be <= '$maxkey' (" . $C->{$maxkey} . "), setting $minkey=" . $C->{$maxkey};
$C->{$minkey} = $C->{$maxkey};
}
}
# ... defaultAccountEgressKeySize can only be checked after defaultAccountEgressKeyAlgorithm has been handled
{
if (not defined $C->{'defaultAccountEgressKeySize'}) {
$C->{'defaultAccountEgressKeySize'} = 0;
}
if ($C->{'defaultAccountEgressKeySize'} =~ /^(\d+)$/) {
$C->{'defaultAccountEgressKeySize'} = $1;
}
else {
push @errors, "Configuration error: value of option 'defaultAccountEgressKeySize' ('" . $C->{'defaultAccountEgressKeySize'} . "') is not a number, defaulting to 0";
$C->{'defaultAccountEgressKeySize'} = 0;
}
if ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'rsa') {
$C->{'defaultAccountEgressKeySize'} ||= 4096;
if ($C->{'defaultAccountEgressKeySize'} < 1024) {
push @errors,
"Configuration error: value of option 'defaultAccountEgressKeySize' ('" . $C->{'defaultAccountEgressKeySize'} . "') should be >= 1024, defaulting to 1024";
$C->{'defaultAccountEgressKeySize'} = 1024;
}
elsif ($C->{'defaultAccountEgressKeySize'} > 32768) {
push @errors,
"Configuration error: value of option 'defaultAccountEgressKeySize' ('" . $C->{'defaultAccountEgressKeySize'} . "') should be >= 1024, defaulting to 1024";
$C->{'defaultAccountEgressKeySize'} = 32768;
}
}
elsif ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'ecdsa') {
$C->{'defaultAccountEgressKeySize'} ||= 521;
if (!grep { $C->{'defaultAccountEgressKeySize'} eq $_ } qw{ 256 384 521 }) {
push @errors,
"Configuration error: value of option 'defaultAccountEgressKeySize' ('"
. $C->{'defaultAccountEgressKeySize'}
. "') should be one of [256,384,521], defaulting to 521";
$C->{'defaultAccountEgressKeySize'} = 521;
}
}
elsif ($C->{'defaultAccountEgressKeyAlgorithm'} eq 'ed25519') {
# for ed25519, it's always 256 anyway
if ($C->{'defaultAccountEgressKeySize'} != 256) {
push @errors, "Configuration error: option 'defaultAccountEgressKeySize' has to be 256 when 'defaultAccountEgressKeyAlgorithm' is set to 'ed25519', forcing 256";
$C->{'defaultAccountEgressKeySize'} = 256;
}
}
delete $unknownkeys{'defaultAccountEgressKeySize'};
}
# ... if kill timeout is lower than lock timeout, just unset lock timeout
if ($C->{'idleKillTimeout'} <= $C->{'idleLockTimeout'} && $C->{'idleLockTimeout'} != 0) {
push @errors,
"Configuration error: option 'idleKillTimeout' ("
. $C->{'idleKillTimeout'}
. ") is <= 'idleLockTimeout' ("
. $C->{'idleLockTimeout'}
. "), setting 'idleKillTimeout' to 0";
$C->{'idleLockTimeout'} = 0;
}
# ... check that adminAccounts are actually valid accounts
{
my @validAccounts;
foreach my $conf (qw{ adminAccounts superOwnerAccounts }) {
foreach my $account (@{$C->{$conf}}) {
my $fnret = OVH::Bastion::is_bastion_account_valid_and_existing(account => $account);
if (!$fnret) {
push @errors, "Configuration error: specified $conf '$account' is not a valid account, ignoring";
}
else {
push @validAccounts, $fnret->value->{'account'};
}
}
$C->{$conf} = \@validAccounts;
}
}
# ... this one is complicated; it's an array of (arrays of 3 items: (two arrays and a string))
if (not defined $C->{'ingressToEgressRules'}) {
$C->{'ingressToEgressRules'} = [];
push @errors, "Configuration error: missing option 'ingressToEgressRules', defaulting to []" if $test;
}
elsif (ref $C->{'ingressToEgressRules'} ne 'ARRAY') {
$C->{'ingressToEgressRules'} = [];
push @errors, "Configuration error: option 'ingressToEgressRules' is invalid, expected an array, defaulting to []";
}
else {
SKIP: foreach my $rule (@{$C->{'ingressToEgressRules'}}) {
if (ref $rule ne 'ARRAY') {
$C->{'ingressToEgressRules'} = [];
push @errors, "Configuration error: option 'ingressToEgressRules' has an invalid format (rules should be arrays), defaulting to []";
last;
}
elsif (@$rule != 3 || ref $rule->[0] ne 'ARRAY' || ref $rule->[1] ne 'ARRAY' || ref $rule->[2]) {
$C->{'ingressToEgressRules'} = [];
push @errors, "Configuration error: option 'ingressToEgressRules' has an invalid format (rules should have 3 items: array, array, scalar), defaulting to []";
last;
}
else {
foreach my $i (0 .. 1) {
foreach my $j (0 .. $#{$rule->[$i]}) {
if ($rule->[$i][$j] =~ m{^([0-9.:%/]+)$}) {
$rule->[$i][$j] = $1;
}
else {
$C->{'ingressToEgressRules'} = [];
push @errors,
"Configuration error: option 'ingressToEgressRules' has an invalid format ('" . $rule->[$i][$j] . "' doesn't look like an IP), defaulting to []";
last SKIP;
}
}
}
if (!grep { $rule->[2] eq $_ } qw{ ALLOW-EXCLUSIVE ALLOW DENY }) {
$C->{'ingressToEgressRules'} = [];
push @errors,
"Configuration error: option 'ingressToEgressRules' has an invalid format ('" . $rule->[2] . "' should be ALLOW, DENY or ALLOW-EXCLUSIVE), defaulting to []";
}
}
}
}
delete $unknownkeys{'ingressToEgressRules'};
# OK we're done
$_cache_config = $C; $_cache_config = $C;
# now that we cached our result, we can call warn_syslog() without risking an infinite loop
warn_syslog($_, $noisy) for @errors;
warn_syslog("Configuration error: got an unknown option '$_' in configuration, ignored", $noisy) for sort keys %unknownkeys;
osh_info("Configuration loaded with " . scalar(@errors) . " warnings.") if $test;
return R('OK', value => $C); return R('OK', value => $C);
} }
@ -346,6 +599,7 @@ sub plugin_config {
my %params = @_; my %params = @_;
my $plugin = $params{'plugin'}; my $plugin = $params{'plugin'};
my $key = $params{'key'}; my $key = $params{'key'};
my $mock_data = $params{'mock_data'};
my $fnret; my $fnret;
if (my @missingParameters = grep { not defined $params{$_} } qw{ plugin }) { if (my @missingParameters = grep { not defined $params{$_} } qw{ plugin }) {
@ -353,6 +607,18 @@ sub plugin_config {
return R('ERR_MISSING_PARAMETER', msg => "Missing @missingParameters on plugin_config() call"); return R('ERR_MISSING_PARAMETER', msg => "Missing @missingParameters on plugin_config() call");
} }
if (defined $mock_data) {
if (!OVH::Bastion::is_mocking()) {
# if we're overriding configuration with mock_data without being in mocking mode, we have a problem
die("Attempted to load_configuration() with mock_data without being in mocking mode");
}
# mock data always overrides our cache
delete $_plugin_config_cache{$plugin};
}
if (not exists $_plugin_config_cache{$plugin}) { if (not exists $_plugin_config_cache{$plugin}) {
# sanitize $plugin # sanitize $plugin
@ -363,6 +629,8 @@ sub plugin_config {
# if not in cache, load it # if not in cache, load it
my %config; my %config;
if (!defined $mock_data) {
# 1of2) load from builtin config (plugin.json) # 1of2) load from builtin config (plugin.json)
my $pluginPath = $OVH::Bastion::BASEPATH . '/bin/plugin'; my $pluginPath = $OVH::Bastion::BASEPATH . '/bin/plugin';
undef $fnret; undef $fnret;
@ -397,6 +665,10 @@ sub plugin_config {
$config{$key} = $fnret->value->{$key} if not exists $config{$key}; $config{$key} = $fnret->value->{$key} if not exists $config{$key};
} }
} }
}
else {
%config = %$mock_data;
}
# compat: we previously expected "yes" as a value for the 'disabled' option, instead of a boolean. # compat: we previously expected "yes" as a value for the 'disabled' option, instead of a boolean.
# To keep compatibility we still consider "yes" as a true value (as any non-empty string is), # To keep compatibility we still consider "yes" as a true value (as any non-empty string is),

View file

@ -101,6 +101,8 @@ sub syslogFormatted {
sub warn_syslog { sub warn_syslog {
my $msg = shift; my $msg = shift;
my $noisy = shift;
osh_warn("WARN: " . $msg) if $noisy;
return syslogFormatted( return syslogFormatted(
type => 'code-warning', type => 'code-warning',
fields => [['msg' => $msg]] fields => [['msg' => $msg]]

View file

@ -397,6 +397,10 @@ configchg()
runtests() runtests()
{ {
# ensure syslog is clean
ignorecodewarn 'Configuration error' # previous unit tests can provoke this
success bastion syslog_cleanup $r0 "\": > /var/log/bastion/bastion.log\""
# backup the original default configuration on target side # backup the original default configuration on target side
now=$(date +%s) now=$(date +%s)
success bastion backupconfig $r0 "dd if=$osh_etc/bastion.conf of=$osh_etc/bastion.conf.bak.$now" success bastion backupconfig $r0 "dd if=$osh_etc/bastion.conf of=$osh_etc/bastion.conf.bak.$now"

View file

@ -7,6 +7,7 @@ use File::Basename;
use lib dirname(__FILE__) . '/../../lib/perl'; use lib dirname(__FILE__) . '/../../lib/perl';
use OVH::Bastion; use OVH::Bastion;
use OVH::Result; use OVH::Result;
use JSON;
OVH::Bastion::enable_mocking(); OVH::Bastion::enable_mocking();
OVH::Bastion::set_mock_data( OVH::Bastion::set_mock_data(
@ -44,6 +45,17 @@ OVH::Bastion::load_configuration(
[["192.168.0.0/16"], ["192.168.0.0/16"], "DENY"] [["192.168.0.0/16"], ["192.168.0.0/16"], "DENY"]
], ],
bastionName => "mock", bastionName => "mock",
# all options below are bool, we'll test for their normalization
enableSyslog => 1,
enableGlobalAccessLog => JSON->new->allow_nonref->decode("true"),
enableAccountAccessLog => "yes",
enableGlobalSqlLog => 0,
enableAccountSqlLog => JSON->new->allow_nonref->decode("false"),
displayLastLogin => "",
debug => JSON->new->allow_nonref->decode("null"),
passwordAllowed => "no",
telnetAllowed => "false",
} }
); );
@ -88,4 +100,135 @@ is(OVH::Bastion::is_access_granted(account => "wildcard", user => "root", ipfrom
ok(OVH::Bastion::is_access_granted(account => "wildcard", user => "root", ipfrom => "192.168.43.1", ip => "5.6.7.8", port => "9876")->is_ok, ok(OVH::Bastion::is_access_granted(account => "wildcard", user => "root", ipfrom => "192.168.43.1", ip => "5.6.7.8", port => "9876")->is_ok,
"is_access_granted(wildcard) on allowed machine due to ingressToEgressRules catch-all"); "is_access_granted(wildcard) on allowed machine due to ingressToEgressRules catch-all");
# check that "bool" type options are correctly normalized
is(OVH::Bastion::config("enableSyslog")->value, 1, "config bool(1)");
is(OVH::Bastion::config("enableGlobalAccessLog")->value, 1, "config bool(true)");
is(OVH::Bastion::config("enableAccountAccessLog")->value, 1, "config bool(\"yes\")");
is(OVH::Bastion::config("enableGlobalSqlLog")->value, 0, "config bool(0)");
is(OVH::Bastion::config("enableAccountSqlLog")->value, 0, "config bool(false)");
is(OVH::Bastion::config("displayLastLogin")->value, 0, "config bool(\"\")");
is(OVH::Bastion::config("interactiveModeByDefault")->value, 1, "config bool(missing, default true)");
is(OVH::Bastion::config("interactiveModeAllowed")->value, 0, "config bool(missing, default false)");
is(OVH::Bastion::config("debug")->value, 0, "config bool(null)");
is(OVH::Bastion::config("passwordAllowed")->value, 0, "config bool(\"no\")");
is(OVH::Bastion::config("telnetAllowed")->value, 0, "config bool(\"false\")");
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => JSON->new->allow_nonref->decode("true")}
)->value ? 1 : 0,
1,
"is_plugin_disabled(disabled=true)"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => JSON->new->allow_nonref->decode("false")}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=false)"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => JSON->new->allow_nonref->decode("null")}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=null)"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "yes"}
)->value ? 1 : 0,
1,
"is_plugin_disabled(disabled=\"yes\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "no"}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=\"no\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "true"}
)->value ? 1 : 0,
1,
"is_plugin_disabled(disabled=\"true\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "false"}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=\"false\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => ""}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=\"\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "0"}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=\"0\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => "1"}
)->value ? 1 : 0,
1,
"is_plugin_disabled(disabled=\"1\")"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => 0}
)->value ? 1 : 0,
0,
"is_plugin_disabled(disabled=0)"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {disabled => 1}
)->value ? 1 : 0,
1,
"is_plugin_disabled(disabled=1)"
);
is(
OVH::Bastion::plugin_config(
plugin => "help",
key => "disabled",
mock_data => {}
)->value ? 1 : 0,
0,
"is_plugin_disabled()"
);
done_testing(); done_testing();