From b48463076fb204ed5ae51096db563ddf104cf1cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lesimple?= Date: Thu, 2 Nov 2023 15:26:12 +0000 Subject: [PATCH] feat: osh.pl: jit mfa for plugins --- bin/admin/install | 18 +- bin/shell/osh.pl | 166 +++++++++++++++++- etc/bastion/osh-sync-watcher.rsyncfilter.dist | 2 + lib/perl/OVH/Bastion/configuration.inc | 6 +- tests/functional/launch_tests_on_instance.sh | 2 +- 5 files changed, 190 insertions(+), 4 deletions(-) diff --git a/bin/admin/install b/bin/admin/install index 34434dd..72eb4fa 100755 --- a/bin/admin/install +++ b/bin/admin/install @@ -52,6 +52,7 @@ set_default_options() opt[regen-hostkeys]=0 opt[logrotate]=1 opt[overwrite-logrotate]=1 + opt[generate-mfa-secret]=1 # special case: # If $autodetect_startup_system is 1, we'll autodetect whether we will install the @@ -132,7 +133,7 @@ while [ -n "$1" ]; do modify-pam-lastlog remove-weak-moduli regen-hostkeys overwrite-logrotate overwrite-cron \ overwrite-syslog-ng logrotate cron syslog-ng migration-grant-aclkeeper-to-gatekeepers \ init systemd-units profile modify-pam-sshd wait check-ttyrec install-fake-ttyrec \ - install-selinux-module + install-selinux-module generate-mfa-secret do if [ "$1" = "--no-$allowedopt" ]; then opt[$allowedopt]=0 @@ -214,6 +215,7 @@ Usage: --[no-]profile apply our hardened configuration in system profile.d/ [N] --[no-]remove-weak-moduli remove weak (<4096) moduli from the ssh server moduli file [N] --[no-]regen-hostkeys generate new host keys (and trash the old ones) for ssh server [N] + --[no-]generate-mfa-secret generate a new MFA token secret, will never overwrite a preexisting one [N, U] --[no-]logrotate put our logrotate config files in system logrotate.d/ [N, U, M] --[no-]overwrite-logrotate overwrite possibly existing files in system logrotate.d/ with our templates [N, U] @@ -1265,6 +1267,20 @@ if [ "${opt[remove-weak-moduli]}" = 1 ]; then fi fi +if [ "${opt[generate-mfa-secret]}" = 1 ]; then + action_doing "Generate the MFA HMAC secret if needed" + if [ -e "$BASTION_ETC_DIR/mfa-token.conf" ]; then + action_na + else + secret=$(tr -dc A-Za-z0-9 < /dev/urandom | head -c32) + touch "$BASTION_ETC_DIR/mfa-token.conf" + chown 0:bastion-users "$BASTION_ETC_DIR/mfa-token.conf" + chmod 640 "$BASTION_ETC_DIR/mfa-token.conf" + echo '{ "secret": "'"$secret"'" }' > "$BASTION_ETC_DIR/mfa-token.conf" + action_done + fi +fi + # lastly, check for ttyrec version and yell if it's not the proper one if [ "${opt[check-ttyrec]}" = 1 ] ; then action_doing "Checking ttyrec version" diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl index 9650360..f5d799e 100755 --- a/bin/shell/osh.pl +++ b/bin/shell/osh.pl @@ -376,6 +376,8 @@ my $remainingOptions; "kbd-interactive" => \my $userKbdInteractive, "proactive-mfa" => \my $proactiveMfa, "fallback-password-delay=i" => \my $fallbackPasswordDelay, + "generate-mfa-token" => \my $generateMfaToken, + "mfa-token=s" => \my $mfaToken, ); if (not defined $realOptions) { help(); @@ -463,6 +465,11 @@ if ($bind) { } } +if ($generateMfaToken && $mfaToken) { + main_exit OVH::Bastion::EXIT_CONFLICTING_OPTIONS, "conflicting_options", + "Can't specify both --generate-mfa-token and --mfa-token"; +} + # if proactive MFA has been requested, do it here, before the code diverts to either # handling interactive session, plugins/osh commands, or a connection request if ($proactiveMfa) { @@ -960,7 +967,6 @@ if ($osh_command) { } # check if we need JIT MFA to call this plugin, this can be configured per-plugin - # TODO: autodetect if the MFA check is done outside of the code by sshd+PAM, to avoid re-asking for it here my $MFArequiredForPlugin = OVH::Bastion::plugin_config(plugin => $osh_command, key => "mfa_required")->value; $MFArequiredForPlugin ||= 'none'; # no config means none @@ -993,6 +999,16 @@ if ($osh_command) { $bastion_details{'mfa'} = $fnret->value->{'mfaInfo'}; } + # now, check whether this plugin wants us to trigger a JIT MFA check depending on the + # specified user/host/ip, if this is configured in one of the matching bastion groups + # we are a part of (plugins such as sftp or scp will require us to do this, as they can't + # do it themselves, and as they're accessing a remote asset, JIT MFA should apply to them too) + my $pluginJitMfa = OVH::Bastion::plugin_config(plugin => $osh_command, key => "jit_mfa")->value; + if ($pluginJitMfa) { + $fnret = do_plugin_jit_mfa(pluginJitMfa => $pluginJitMfa); + # do_plugin_jit_mfa exits if needed, but just in case... + main_exit(OVH::Bastion::EXIT_MFA_FAILED, "jit_mfa_failed", $fnret->msg) if !$fnret; + } OVH::Bastion::set_terminal_mode_for_plugin(plugin => $osh_command, action => 'set'); # get the execution mode required by the plugin @@ -1724,6 +1740,154 @@ sub do_jit_mfa { return R('OK'); } +sub do_plugin_jit_mfa { + my %params = @_; + my $pluginJitMfa = $params{'pluginJitMfa'}; + my $localfnret; + + if (!$host) { + # if no host is specified, and the plugin has jit_mfa_allow_no_host, then + # allow the plugin to be called, for example to show its builtin help() + $localfnret = OVH::Bastion::plugin_config(plugin => $osh_command, key => 'jit_mfa_allow_no_host'); + if ($localfnret && $localfnret->value) { + if ($generateMfaToken) { + # return a dummy token so that our caller is happy, then exit + print "MFA_TOKEN=notrequired\n"; + main_exit(OVH::Bastion::EXIT_OK); + } + # tell our caller that the plugin can be executed without host + return R('OK_JIT_MFA_NOT_REQUIRED'); + } + # otherwise, we need a host + main_exit(OVH::Bastion::EXIT_NO_HOST, 'no_host', "A host is required for this plugin but none was specified"); + } + + # if no user yet, fix it to remote user + # we need it to be able to get the proper answer for is_access_granted, and we need to + # call is_access_granted so that we know whether we need to trigger JIT MFA for this + # plugin, based on the groups we find + my $remoteuser = $user || $config->{'defaultLogin'} || $remoteself || $sysself; + + $localfnret = OVH::Bastion::is_access_granted( + account => $self, + user => $user, + ipfrom => $ipfrom, + ip => $ip, + port => $port, + details => 1 + ); + + if (!$localfnret) { + # not allowed, exit + my $message = $localfnret->msg; + if ($remoteuser eq $self) { + $message .= " (tried with remote user '$remoteuser')"; + } + main_exit(OVH::Bastion::EXIT_ACCESS_DENIED, 'access_denied', $message); + } + + # else, get the access list + my @accessListForPlugin = @{$localfnret->value || []}; + + # and check whether we need JIT MFA + my $mfaRequired; + $localfnret = get_details_from_access_array( + accessList => \@accessListForPlugin, + quiet => ($quiet || $mfaToken), + useKey => $useKey + ); + if ($localfnret && $localfnret->value->{'mfaRequired'}) { + $mfaRequired = $localfnret->value->{'mfaRequired'}; + } + elsif (!$localfnret) { + main_exit(OVH::Bastion::EXIT_ACCESS_DENIED, "access_denied", $localfnret->msg); + } + + # not required? we're done + if (!$mfaRequired) { + if ($generateMfaToken) { + # return a dummy token so that our caller is happy, then exit + print "MFA_TOKEN=notrequired\n"; + main_exit(OVH::Bastion::EXIT_OK); + } + # no mfa required and our caller didn't request a token generation, just carry on + return R('OK_JIT_MFA_NOT_REQUIRED'); + } + + $localfnret = OVH::Bastion::load_configuration_file( + file => OVH::Bastion::main_configuration_directory() . '/mfa-token.conf', + rootonly => 1 + ); + if (!$localfnret || !$localfnret->value) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_no_secret', + "No MFA token HMAC secret has been configured, please report to your sysadmin"); + } + my $tokenConfig = $localfnret->value; + if (ref $tokenConfig ne 'HASH' || !$tokenConfig->{'secret'} || length($tokenConfig->{'secret'}) < 32) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_invalid_secret', + "An invalid MFA token HMAC secret has been configured, please report to your sysadmin"); + } + my $secret = $tokenConfig->{'secret'}; + + # so, if JIT MFA is required, we need to have either --generate-mfa-token, or --mfa-token + if ($mfaToken) { + # use stderr here because we might be called by scp or sftp and they don't expect anything on stdout + $ENV{'FORCE_STDERR'} = 1; + + # recompute the theoretical token value we should have + my ($then) = $mfaToken =~ m{^v1,(\d+),[a-f0-9]{64}$}; + if (!$then) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_invalid_format', + "Provided MFA token has invalid format"); + } + + # is the token expired? + if ($then + 15 < time()) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_expired_token', "Provided MFA token is expired"); + } + + # is the token in the future? + if ($then > time()) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_future_token', + "Provided MFA token has creation date in the future"); + } + + require Digest::SHA; + my $payload = join(',', $then, $self, $host, $port, $remoteuser); + my $theoreticalToken = sprintf("v1,%s,%s", $then, Digest::SHA::hmac_sha256_hex($payload, $secret)); + + # are both tokens identical? + if ($mfaToken ne $theoreticalToken) { + main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed_invalid_token', "Provided MFA token is invalid"); + } + + print STDERR "... MFA token is valid, proceeding\n"; + return R('OK_JIT_MFA_VALIDATED'); + } + elsif ($generateMfaToken) { + print "MFA token generation requested, entering MFA phase...\n"; + + $localfnret = OVH::Bastion::do_pamtester(self => $self, sysself => $sysself); + $localfnret or main_exit(OVH::Bastion::EXIT_MFA_FAILED, 'mfa_failed', $localfnret->msg); + + # if we're still here, MFA has been validated, generate a token, save and return it + require Digest::SHA; + my $now = time(); + my $payload = join(',', $now, $self, $host, $port, $remoteuser); + my $generated_token = sprintf("v1,%s,%s", $now, Digest::SHA::hmac_sha256_hex($payload, $secret)); + + # return token to caller + print "MFA_TOKEN=$generated_token\n"; + main_exit(OVH::Bastion::EXIT_OK, "mfa_token_generated", "MFA token has been generated"); + } + + # MFA required but none provided or requested: bail out + main_exit(OVH::Bastion::EXIT_MFA_FAILED, "jit_mfa_required_no_token", + "JIT MFA is required, but you didn't specify either --generate-mfa-token or --mfa-token"); + + return; # make perlcritic happy +} + # # Display help message # diff --git a/etc/bastion/osh-sync-watcher.rsyncfilter.dist b/etc/bastion/osh-sync-watcher.rsyncfilter.dist index d4c0223..5837a32 100644 --- a/etc/bastion/osh-sync-watcher.rsyncfilter.dist +++ b/etc/bastion/osh-sync-watcher.rsyncfilter.dist @@ -17,6 +17,8 @@ + /etc/gshadow + /etc/sudoers.d/ + /etc/sudoers.d/osh-* ++ /etc/bastion/ ++ /etc/bastion/mfa-token.conf + /etc/ssh/ + /etc/ssh/ssh_host_*_key + /etc/ssh/ssh_host_*_key.pub diff --git a/lib/perl/OVH/Bastion/configuration.inc b/lib/perl/OVH/Bastion/configuration.inc index 947f270..2d59d16 100644 --- a/lib/perl/OVH/Bastion/configuration.inc +++ b/lib/perl/OVH/Bastion/configuration.inc @@ -21,7 +21,7 @@ sub load_configuration_file { if ($secure || $rootonly) { my @stat = lstat($file); if (@stat) { - if ($stat[4] != 0 or $stat[5] != 0) { + if ($stat[4] != 0) { return R('ERR_SECURITY_VIOLATION', msg => "Configuration file ($file) is not owned by root, report to your sysadmin."); } @@ -29,6 +29,10 @@ sub load_configuration_file { return R('ERR_SECURITY_VIOLATION', msg => "Configuration file ($file) is not a regular file, report to your sysadmin."); } + if (S_IMODE($stat[2]) & S_IWGRP) { + return R('ERR_SECURITY_VIOLATION', + msg => "Configuration file ($file) is group-writable, report to your sysadmin."); + } if (S_IMODE($stat[2]) & S_IWOTH) { return R('ERR_SECURITY_VIOLATION', msg => "Configuration file ($file) is world-writable, report to your sysadmin."); diff --git a/tests/functional/launch_tests_on_instance.sh b/tests/functional/launch_tests_on_instance.sh index 0e51edf..2743e03 100755 --- a/tests/functional/launch_tests_on_instance.sh +++ b/tests/functional/launch_tests_on_instance.sh @@ -708,7 +708,7 @@ runtests() # craft a test that always work and will notice that the previous one failed, which'll display # the verbose error information modulename=main - success done true + success "done" true } COUNTONLY=0