Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enh: 35% faster is_valid_ip #432

Closed
wants to merge 10 commits into from
1 change: 0 additions & 1 deletion .github/workflows/freebsd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ jobs:
--has-mfa=0 \
--has-mfa-password=1 \
--has-pamtester=1 \
--skip-consistency-check \
--remote-etc-bastion=/usr/local/etc/bastion \
--slowness-factor=2 \
--post-run="sudo tar xzf /opt/bastion/ssh_config.tar.gz -C / ; sudo /etc/rc.d/sshd restart" \
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@ on:

jobs:
tests_short:
name: Short (deb12 only, no cc)
name: Short (deb12 only, w/o cc)
runs-on: ubuntu-latest
if: contains(github.event.pull_request.labels.*.name, 'tests:short')
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
- name: run tests inside a debian12 docker
run: tests/functional/docker/docker_build_and_run_tests.sh debian12 --skip-consistency-check --no-pause-on-fail
run: tests/functional/docker/docker_build_and_run_tests.sh debian12 --no-pause-on-fail
env:
DOCKER_TTY: false

tests_long:
name: Long
name: Long (multi-distros, w/o cc)
strategy:
matrix:
platform: [rockylinux9, debian12, 'opensuse15@opensuse/leap:15.6', ubuntu2404]
Expand All @@ -30,12 +30,12 @@ jobs:
with:
persist-credentials: false
- name: run tests inside a ${{ matrix.platform }} docker
run: tests/functional/docker/docker_build_and_run_tests.sh ${{ matrix.platform }} --skip-consistency-check --no-pause-on-fail
run: tests/functional/docker/docker_build_and_run_tests.sh ${{ matrix.platform }} --no-pause-on-fail
env:
DOCKER_TTY: false

tests_full:
name: Full
name: Full (multi-distros all versions, w/ cc)
strategy:
matrix:
platform: [rockylinux8, rockylinux9, debian10, debian11, debian12, 'opensuse15@opensuse/leap:15.6', ubuntu1804, ubuntu2004, ubuntu2204, ubuntu2404]
Expand All @@ -46,6 +46,6 @@ jobs:
with:
persist-credentials: false
- name: run tests inside a ${{ matrix.platform }} docker
run: tests/functional/docker/docker_build_and_run_tests.sh ${{ matrix.platform }} --no-pause-on-fail
run: tests/functional/docker/docker_build_and_run_tests.sh ${{ matrix.platform }} --consistency-check --no-pause-on-fail
env:
DOCKER_TTY: false
4 changes: 1 addition & 3 deletions bin/helper/osh-accountModifyPersonalAccess
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,7 @@ if (not grep { $action eq $_ } qw{ add del }) {
#<PARAMS:ACTION

#>CODE
my $machine = $ip;
$port and $machine .= ":$port";
$user and $machine = $user . '@' . $machine;
my $machine = OVH::Bastion::machine_display(ip => $ip, port => $port, user => $user)->value;

my $plugin = ($target eq 'self' ? 'self' : 'account') . 'AddPersonalAccess';

Expand Down
4 changes: 1 addition & 3 deletions bin/helper/osh-groupAddServer
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ $fnret = OVH::Bastion::Helper::acquire_lock($lock_fh);
$fnret or HEXIT($fnret);

#>CODE
my $machine = $ip;
$port and $machine .= ":$port";
$user and $machine = $user . '@' . $machine;
my $machine = OVH::Bastion::machine_display(ip => $ip, port => $port, user => $user)->value;

# access_modify validates all its parameters, don't do it ourselves here for clarity
$fnret = OVH::Bastion::access_modify(
Expand Down
20 changes: 8 additions & 12 deletions bin/plugin/open/groupInfo
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,6 @@ foreach my $groupData (@groups) {
push @filtered_guests, $guest;
}

# deprecated in v2.18.00+
$ret{'full_members'} = \@members;
$ret{'partial_members'} = \@filtered_guests;
# /deprecated

$ret{'members'} = \@members;
$ret{'guests'} = \@filtered_guests;
$ret{'guests_accesses'} = \%guest_nb_accesses;
Expand Down Expand Up @@ -253,14 +248,15 @@ sub print_group_info {
if $ret{'members'};

# show guest info, along with the number of accesses each guest has
my @guest_text;
foreach my $guest (@{$ret{'guests'}}) {
my $nb = $ret{'guests_accesses'}{$guest};
push @guest_text, sprintf("%s[%s]", $guest, $nb // '?');
if ($ret{'guests'}) {
my @guest_text;
foreach my $guest (@{$ret{'guests'}}) {
my $nb = $ret{'guests_accesses'}{$guest};
push @guest_text, sprintf("%s[%s]", $guest, $nb // '?');
}
osh_info("Group ${groupName}'s Guests (with access to SOME of the group servers) are: "
. colored(@{$ret{'guests'}} ? join(" ", sort @guest_text) : '-', 'red'));
}
osh_info("Group ${groupName}'s Guests (with access to SOME of the group servers) are: "
. colored(@{$ret{'guests'}} ? join(" ", sort @guest_text) : '-', 'red'))
if $ret{'guests'};

# current user doesn't have enough rights to get this info, tell them that
if (!$ret{'members'}) {
Expand Down
164 changes: 94 additions & 70 deletions bin/plugin/restricted/accountInfo
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,11 @@ foreach my $accHash (@accounts) {
$ret{'max_inactive_days'} =
OVH::Bastion::account_config(account => $account, %{OVH::Bastion::OPT_ACCOUNT_MAX_INACTIVE_DAYS()})->value;

$ret{'osh_only'} = !!OVH::Bastion::account_config(
account => $account,
key => OVH::Bastion::OPT_ACCOUNT_OSH_ONLY
) + 0;

if ($withPasswordInfo) {
my @command = qw{ sudo -n -u root -- /usr/bin/env perl -T };
push @command, $OVH::Bastion::BASEPATH . '/bin/helper/osh-accountGetPasswordInfo';
Expand Down Expand Up @@ -478,7 +483,7 @@ sub print_account_info {
}
}

if ($ret{'creation_information'}) {
if (defined $ret{'creation_information'}) {
if ($ret{'creation_information'}{'datetime_utc'}) {
my $createdOnStr = $ret{'creation_information'}{'datetime_utc'};
if ( $ret{'creation_information'}{'datetime_local'}
Expand Down Expand Up @@ -507,87 +512,106 @@ sub print_account_info {
}
}

osh_info "\nAccount egress SSH config:";
if ($ret{'account_egress_ssh_config'}{'type'} eq 'default') {
osh_info "- (default)";
if (defined $ret{'account_egress_ssh_config'}) {
osh_info "\nAccount egress SSH config:";
if ($ret{'account_egress_ssh_config'}{'type'} eq 'default') {
osh_info "- (default)";
}
elsif ($ret{'account_egress_ssh_config'}{'type'} eq 'locally_modified') {
osh_info "- (locally modified!)";
}
elsif ($ret{'account_egress_ssh_config'}{'type'} eq 'custom') {
foreach my $key (sort keys %{$ret{'account_egress_ssh_config'}{'items'} || {}}) {
osh_info "- $key " . $ret{'account_egress_ssh_config'}{'items'}{$key};
}
}
else {
osh_info "- (unknown)";
}
}
elsif ($ret{'account_egress_ssh_config'}{'type'} eq 'locally_modified') {
osh_info "- (locally modified!)";

if (defined $ret{'osh_only'}) {
osh_info "\nThis account can only run commands (\"osh-only\"): "
. ($ret{'osh_only'} ? colored('yes', 'red') : colored('no', 'blue'));
}
elsif ($ret{'account_egress_ssh_config'}{'type'} eq 'custom') {
foreach my $key (sort keys %{$ret{'account_egress_ssh_config'}{'items'} || {}}) {
osh_info "- $key " . $ret{'account_egress_ssh_config'}{'items'}{$key};

if (exists $ret{'ingress_piv_policy'} && exists $ret{'ingress_piv_grace'}) {
osh_info "\nAccount PIV-only policy status:";
my $ingress_piv_policy_print = $ret{'ingress_piv_policy'} || 'default';
osh_info "- PIV policy for ingress keys on this account is set to "
. colored($ingress_piv_policy_print, $ingress_piv_policy_print eq 'default' ? 'blue' : 'green');

if ($ret{'ingress_piv_grace'} && $ret{'ingress_piv_grace'}{'seconds_remaining'}) {
$fnret = OVH::Bastion::duration2human(seconds => $ret{'ingress_piv_grace'}{'seconds_remaining'})->value;
osh_info("- PIV grace period for this account is "
. colored('set', 'green')
. " and expires in "
. $fnret->value->{'human'});
}
else {
osh_info "- PIV grace period for this account is " . colored('inactive', 'blue');
}
}
else {
osh_info "- (unknown)";
}

osh_info "\nAccount PIV-only policy status:";
my $ingress_piv_policy_print = $ret{'ingress_piv_policy'} || 'default';
osh_info "- PIV policy for ingress keys on this account is set to "
. colored($ingress_piv_policy_print, $ingress_piv_policy_print eq 'default' ? 'blue' : 'green');

if ($ret{'ingress_piv_grace'} && $ret{'ingress_piv_grace'}{'seconds_remaining'}) {
$fnret = OVH::Bastion::duration2human(seconds => $ret{'ingress_piv_grace'}{'seconds_remaining'})->value;
osh_info("- PIV grace period for this account is "
. colored('set', 'green')
. " and expires in "
. $fnret->value->{'human'});
if (defined $ret{'global_ingress_policy'}) {
osh_info "- Global PIV policy status is "
. ($ret{'global_ingress_policy'} ? colored('enabled', 'red') : colored('disabled', 'blue'));
}
else {
osh_info "- PIV grace period for this account is " . colored('inactive', 'blue');

if (defined $ret{'effective_ingress_piv_policy'}) {
osh_info "- As a consequence, PIV policy is "
. ($ret{'effective_ingress_piv_policy'} ? colored('enforced', 'red') : colored('inactive', 'blue'))
. " for this account";
}

osh_info "- Global PIV policy status is "
. ($ret{'global_ingress_policy'} ? colored('enabled', 'red') : colored('disabled', 'blue'));

osh_info "- As a consequence, PIV policy is "
. ($ret{'effective_ingress_piv_policy'} ? colored('enforced', 'red') : colored('inactive', 'blue'))
. " for this account";

osh_info "\nAccount Multi-Factor Authentication status:";
osh_info "- Additional password authentication is "
. ($ret{'mfa_password_required'} ? colored('required', 'green') : colored('not required', 'blue'))
. " for this account";
osh_info "- Additional password authentication bypass is "
. ($ret{'mfa_password_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'))
. " for this account";
osh_info "- Additional password authentication is "
. ($ret{'mfa_password_configured'} ? colored('enabled and active', 'green') : colored('disabled', 'blue'));

osh_info "- Additional TOTP authentication is "
. ($ret{'mfa_totp_required'} ? colored('required', 'green') : colored('not required', 'blue'))
. " for this account";
osh_info "- Additional TOTP authentication bypass is "
. ($ret{'mfa_totp_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'))
. " for this account";
osh_info "- Additional TOTP authentication is "
. ($ret{'mfa_totp_configured'} ? colored('enabled and active', 'green') : colored('disabled', 'blue'));

osh_info "- PAM authentication bypass is "
. ($ret{'pam_auth_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'));

osh_info "- Optional public key authentication is "
. ($ret{'pubkey_auth_optional'} ? colored('enabled', 'green') : colored('disabled', 'blue'));

osh_info "- MFA policy on personal accesses (using personal keys) on egress side is: "
. $ret{'personal_egress_mfa_required'};

osh_info "\n- Account is immune to idle counter-measures: "
. ($ret{'idle_ignore'} ? colored('yes', 'green') : colored('no', 'blue'));

if (!defined $ret{'max_inactive_days'}) {
osh_info "- Maximum number of days of inactivity before account is disabled: (default)";
if (exists $ret{'mfa_password_required'} && exists $ret{'mfa_totp_required'} && exists $ret{'pam_auth_bypass'}) {
osh_info "\nAccount Multi-Factor Authentication status:";
osh_info "- Additional password authentication is "
. ($ret{'mfa_password_required'} ? colored('required', 'green') : colored('not required', 'blue'))
. " for this account";
osh_info "- Additional password authentication bypass is "
. ($ret{'mfa_password_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'))
. " for this account";
osh_info "- Additional password authentication is "
. ($ret{'mfa_password_configured'} ? colored('enabled and active', 'green') : colored('disabled', 'blue'));

osh_info "- Additional TOTP authentication is "
. ($ret{'mfa_totp_required'} ? colored('required', 'green') : colored('not required', 'blue'))
. " for this account";
osh_info "- Additional TOTP authentication bypass is "
. ($ret{'mfa_totp_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'))
. " for this account";
osh_info "- Additional TOTP authentication is "
. ($ret{'mfa_totp_configured'} ? colored('enabled and active', 'green') : colored('disabled', 'blue'));

osh_info "- PAM authentication bypass is "
. ($ret{'pam_auth_bypass'} ? colored('enabled', 'green') : colored('disabled', 'blue'));

osh_info "- Optional public key authentication is "
. ($ret{'pubkey_auth_optional'} ? colored('enabled', 'green') : colored('disabled', 'blue'));

osh_info "- MFA policy on personal accesses (using personal keys) on egress side is: "
. $ret{'personal_egress_mfa_required'};
}
elsif ($ret{'max_inactive_days'} == 0) {
osh_info "- Maximum number of days of inactivity before account is disabled: never";

if (exists $ret{'idle_ignore'}) {
osh_info "\n- Account is immune to idle counter-measures: "
. ($ret{'idle_ignore'} ? colored('yes', 'green') : colored('no', 'blue'));
}
else {
osh_info "- Maximum number of days of inactivity before account is disabled: " . $ret{'max_inactive_days'};

if (exists $ret{'max_inactive_days'}) {
if (!defined $ret{'max_inactive_days'}) {
osh_info "- Maximum number of days of inactivity before account is disabled: (default)";
}
elsif ($ret{'max_inactive_days'} == 0) {
osh_info "- Maximum number of days of inactivity before account is disabled: never";
}
else {
osh_info "- Maximum number of days of inactivity before account is disabled: " . $ret{'max_inactive_days'};
}
}

if ($ret{'password'}) {
if (defined $ret{'password'}) {
osh_info "Account PAM UNIX password information (used for password MFA):";
if ($ret{'password'}{'password'} eq 'locked') {
osh_info "- No valid password is set";
Expand Down
21 changes: 13 additions & 8 deletions bin/shell/osh.pl
Original file line number Diff line number Diff line change
Expand Up @@ -600,8 +600,7 @@ sub main_exit {
if ($user && !OVH::Bastion::is_valid_remote_user(user => $user, allowWildcards => ($osh_command ? 1 : 0))) {
main_exit OVH::Bastion::EXIT_INVALID_REMOTE_USER, 'invalid_remote_user', "Remote user name '$user' seems invalid";
}
if ($host && $host !~ m{^[a-zA-Z0-9._/:-]+$}) {

if ($host && $host !~ m{^\[?[a-zA-Z0-9._/:-]+\]?$}) {
# can be an IP (v4 or v6), hostname, or prefix (with a /)
main_exit OVH::Bastion::EXIT_INVALID_REMOTE_HOST, 'invalid_remote_host', "Remote host name '$host' seems invalid";
}
Expand All @@ -612,7 +611,6 @@ sub main_exit {

# if: avoid loading Net::IP and BigInt if there's no host specified
if ($host) {

# probably this "host" is in fact an option, but we didn't parse it because it's an unknown one,
# so we call the long_help() for the user, before exiting
if ($host =~ m{^--}) {
Expand All @@ -624,14 +622,17 @@ sub main_exit {
$fnret = OVH::Bastion::get_ip(host => $host);
}
if (!$fnret) {

# exit error when not osh ...
# exit error when not a plugin call
if (!$osh_command) {
main_exit OVH::Bastion::EXIT_HOST_NOT_FOUND, 'host_not_found', "Unable to resolve host '$host' ($fnret)";
main_exit OVH::Bastion::EXIT_HOST_NOT_FOUND, 'host_not_found', $fnret->msg;
}
elsif ($host && $host !~ m{^[0-9.:]+/\d+$}) # in some osh plugins, ip/mask is accepted, don't yell.
{
osh_warn("I was unable to resolve host '$host'. Something shitty might happen.");
osh_warn($fnret->msg);
osh_warn("Trying to proceed with $osh_command anyway, but things might go wrong.");
if (index($host, ':') >= 0 && !OVH::Bastion::config('IPv6Allowed')->value) {
osh_warn("Note that '$host' looks like an IPv6 but IPv6 support has not been enabled.");
}
}
}
else {
Expand Down Expand Up @@ -1131,7 +1132,11 @@ sub main_exit {
# log request
osh_debug("final request : " . "$user\@$ip -p $port -- $command'\n");

my $displayLine = "$hostfrom:$portfrom => $self\@$bastionhost:$bastionport => $user\@$hostto:$port";
my $displayLine = sprintf("%s => %s => %s",
OVH::Bastion::machine_display(ip => $hostfrom, port => $portfrom)->value,
OVH::Bastion::machine_display(ip => $bastionhost, port => $bastionport, user => $self)->value,
OVH::Bastion::machine_display(ip => $hostto, port => $port, user => $user)->value,
);

if (!$quiet) {
osh_print("$displayLine ...");
Expand Down
Loading
Loading