feat: support wildcards in --user (fix #461)

ovh · Jul 2, 2024 · cc6ccd0 · cc6ccd0
1 parent 8e74e66
commit cc6ccd0
Show file tree

Hide file tree

Showing 20 changed files with 159 additions and 81 deletions.
diff --git a/bin/plugin/group-aclkeeper/groupAddServer b/bin/plugin/group-aclkeeper/groupAddServer
@@ -10,9 +10,10 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "adding a server to a group",
-    options => {
+    argv               => \@ARGV,
+    header             => "adding a server to a group",
+    userAllowWildcards => 1,
+    options            => {
         "group=s"          => \my $group,
         "user-any"         => \my $userAny,
         "port-any"         => \my $portAny,
@@ -33,8 +34,9 @@ Usage: --osh SCRIPT_NAME --group GROUP [OPTIONS]
   --group GROUP            Specify which group this machine should be added to (it should have the public group key of course)
   --host HOST|IP|NET/CIDR  Host(s) to add access to, either a HOST which will be resolved to an IP immediately, or an IP,
                              or a whole network using the NET/CIDR notation
-  --user USER              Specify which remote user should be allowed (root, run, etc...)
-  --user-any               Allow any remote user (the remote user should still have the public group key in all cases)
+  --user USER              Specify which remote user should be allowed (root, run, etc...).
+                             Globbing characters '*' and '?' are supported.
+  --user-any               Synonym of '--user *', allows any remote user (the remote user should still have the public group key in all cases)
   --port PORT              Only allow access to this port (e.g. 22)
   --port-any               Allow access to any port
   --scpup                  Allow SCP upload, you--bastion-->server (omit --user in this case)

diff --git a/bin/plugin/group-aclkeeper/groupDelServer b/bin/plugin/group-aclkeeper/groupDelServer
@@ -10,9 +10,10 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "removing a server from a group",
-    options => {
+    argv               => \@ARGV,
+    header             => "removing a server from a group",
+    userAllowWildcards => 1,
+    options            => {
         "group=s"  => \my $group,
         "user-any" => \my $userAny,
         "port-any" => \my $portAny,

diff --git a/bin/plugin/group-gatekeeper/groupAddGuestAccess b/bin/plugin/group-gatekeeper/groupAddGuestAccess
@@ -11,9 +11,10 @@ use OVH::Bastion::Plugin::groupSetRole;
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "add access to one server of a group to an account",
-    options => {
+    argv               => \@ARGV,
+    header             => "add access to one server of a group to an account",
+    userAllowWildcards => 1,
+    options            => {
         "group=s"   => \my $group,
         "account=s" => \my $account,
         "user-any"  => \my $userAny,

diff --git a/bin/plugin/group-gatekeeper/groupDelGuestAccess b/bin/plugin/group-gatekeeper/groupDelGuestAccess
@@ -11,9 +11,10 @@ use OVH::Bastion::Plugin::groupSetRole;
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "remove access from one server of a group from an account",
-    options => {
+    argv               => \@ARGV,
+    header             => "remove access from one server of a group from an account",
+    userAllowWildcards => 1,
+    options            => {
         "group=s"   => \my $group,
         "account=s" => \my $account,
         "user-any"  => \my $userAny,

diff --git a/bin/plugin/restricted/accountAddPersonalAccess b/bin/plugin/restricted/accountAddPersonalAccess
@@ -10,10 +10,11 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    loadConfig => 1,
-    argv       => \@ARGV,
-    header     => "adding personal access to a server on an account",
-    options    => {
+    loadConfig         => 1,
+    argv               => \@ARGV,
+    header             => "adding personal access to a server on an account",
+    userAllowWildcards => 1,
+    options            => {
         "account=s"        => \my $account,
         "user-any"         => \my $userAny,
         "port-any"         => \my $portAny,
@@ -32,8 +33,8 @@ Usage: --osh SCRIPT_NAME --account ACCOUNT --host HOST [OPTIONS]
 
   --account                Bastion account to add the access to
   --host IP|HOST|IP/MASK   Server to add access to
-  --user USER              Remote login to use, if you want to allow any login, use --user-any
-  --user-any               Allow access with any remote login
+  --user USER              Remote login to use, globbing characters '?' and '*' are supported
+  --user-any               Allow access with any remote login (synonym of ``--user *``)
   --port PORT              Remote SSH port to use, if you want to allow any port, use --port-any
   --port-any               Allow access to all remote ports
   --scpup                  Allow SCP upload, you--bastion-->server (omit --user in this case)

diff --git a/bin/plugin/restricted/accountDelPersonalAccess b/bin/plugin/restricted/accountDelPersonalAccess
@@ -10,9 +10,10 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "removing personal access to a server from an account",
-    options => {
+    argv               => \@ARGV,
+    header             => "removing personal access to a server from an account",
+    userAllowWildcards => 1,
+    options            => {
         "account=s" => \my $account,
         "user-any"  => \my $userAny,
         "port-any"  => \my $portAny,

diff --git a/bin/plugin/restricted/selfAddPersonalAccess b/bin/plugin/restricted/selfAddPersonalAccess
@@ -10,10 +10,11 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    loadConfig => 1,
-    argv       => \@ARGV,
-    header     => "adding personal access to a server on your account",
-    options    => {
+    loadConfig         => 1,
+    argv               => \@ARGV,
+    header             => "adding personal access to a server on your account",
+    userAllowWildcards => 1,
+    options            => {
         "user-any"         => \my $userAny,
         "port-any"         => \my $portAny,
         "scpup"            => \my $scpUp,
@@ -31,8 +32,8 @@ Add a personal server access on your account
 Usage: --osh SCRIPT_NAME --host HOST [OPTIONS]
 
   --host IP|HOST|IP/MASK   Server to add access to
-  --user USER              Remote login to use, if you want to allow any login, use --user-any
-  --user-any               Allow access with any remote login
+  --user USER              Remote login to use, globbing characters '?' and '*' are supported
+  --user-any               Allow access with any remote login (synonym of ``--user *``)
   --port PORT              Remote SSH port to use, if you want to allow any port, use --port-any
   --port-any               Allow access to all remote ports
   --scpup                  Allow SCP upload, you--bastion-->server (omit --user in this case)

diff --git a/bin/plugin/restricted/selfDelPersonalAccess b/bin/plugin/restricted/selfDelPersonalAccess
@@ -10,9 +10,10 @@ use OVH::Bastion::Plugin qw( :DEFAULT help );
 use OVH::Bastion::Plugin::ACL;
 
 my $remainingOptions = OVH::Bastion::Plugin::begin(
-    argv    => \@ARGV,
-    header  => "removing personal access to a server from an account",
-    options => {
+    argv               => \@ARGV,
+    header             => "removing personal access to a server from an account",
+    userAllowWildcards => 1,
+    options            => {
         "user-any" => \my $userAny,
         "port-any" => \my $portAny,
         "scpup"    => \my $scpUp,

diff --git a/bin/shell/osh.pl b/bin/shell/osh.pl
@@ -595,7 +595,9 @@ sub main_exit {
     }
 }
 
-if ($user && !OVH::Bastion::is_valid_remote_user(user => $user)) {
+# for plugins (osh_command), do a first check with allowWildcards, it'll be re-done in Plugin::start with
+# either allowWildcards set to 0 or 1 depending on the plugin configuration that we don't have at this stage yet
+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._/:-]+$}) {

diff --git a/doc/sphinx/plugins/group-aclkeeper/groupAddServer.rst b/doc/sphinx/plugins/group-aclkeeper/groupAddServer.rst
@@ -25,11 +25,12 @@ Add an IP or IP block to a group's servers list
                              or a whole network using the NET/CIDR notation
 .. option:: --user USER
 
-   Specify which remote user should be allowed (root, run, etc...)
+   Specify which remote user should be allowed (root, run, etc...).
 
+                             Globbing characters '*' and '?' are supported.
 .. option:: --user-any
 
-   Allow any remote user (the remote user should still have the public group key in all cases)
+   Synonym of '--user *', allows any remote user (the remote user should still have the public group key in all cases)
 
 .. option:: --port PORT
 

diff --git a/doc/sphinx/plugins/restricted/accountAddPersonalAccess.rst b/doc/sphinx/plugins/restricted/accountAddPersonalAccess.rst
@@ -24,11 +24,11 @@ Add a personal server access to an account
 
 .. option:: --user USER
 
-   Remote login to use, if you want to allow any login, use --user-any
+   Remote login to use, globbing characters '?' and '*' are supported
 
 .. option:: --user-any
 
-   Allow access with any remote login
+   Allow access with any remote login (synonym of ``--user *``)
 
 .. option:: --port PORT
 

diff --git a/doc/sphinx/plugins/restricted/selfAddPersonalAccess.rst b/doc/sphinx/plugins/restricted/selfAddPersonalAccess.rst
@@ -20,11 +20,11 @@ Add a personal server access on your account
 
 .. option:: --user USER
 
-   Remote login to use, if you want to allow any login, use --user-any
+   Remote login to use, globbing characters '?' and '*' are supported
 
 .. option:: --user-any
 
-   Allow access with any remote login
+   Allow access with any remote login (synonym of ``--user *``)
 
 .. option:: --port PORT
 

diff --git a/lib/perl/OVH/Bastion.pm b/lib/perl/OVH/Bastion.pm
@@ -735,9 +735,14 @@ sub is_valid_port {
 }
 
 sub is_valid_remote_user {
-    my %params = @_;
-    my $user   = $params{'user'};
-    if ($user =~ /^([a-zA-Z0-9._@!-]{1,128})$/) {
+    my %params         = @_;
+    my $user           = $params{'user'};
+    my $allowWildcards = $params{'allowWildcards'};
+
+    # if allowWildcards, then additional chars are allowed in the regex
+    my $extraChars = ($allowWildcards ? '?*' : '');
+
+    if ($user =~ /^([\Q${extraChars}\Ea-zA-Z0-9._@!-]{1,128})$/) {
         return R('OK', value => $1);
     }
     return R('ERR_INVALID_PARAMETER', msg => "Specified user doesn't seem to be valid");

diff --git a/lib/perl/OVH/Bastion/Plugin.pm b/lib/perl/OVH/Bastion/Plugin.pm
@@ -24,12 +24,13 @@ sub help { osh_info $_helptext; return 1; }
 sub begin {
     my %params = @_;
 
-    my $options      = $params{'options'};
-    my $header       = $params{'header'};
-    my $argv         = $params{'argv'};
-    my $loadConfig   = $params{'loadConfig'};
-    my $exitOnSignal = $params{'exitOnSignal'};
-    my $helpfunc     = $params{'help'};
+    my $options            = $params{'options'};
+    my $header             = $params{'header'};
+    my $argv               = $params{'argv'};
+    my $loadConfig         = $params{'loadConfig'};
+    my $exitOnSignal       = $params{'exitOnSignal'};
+    my $helpfunc           = $params{'help'};
+    my $userAllowWildcards = $params{'userAllowWildcards'};
     $_helptext = $params{'helptext'};
 
     my $fnret;
@@ -63,7 +64,7 @@ sub begin {
     # validate user, ip, port when specified, undef them otherwise (instead of '')
 
     if (defined $user && $user ne '') {
-        $fnret = OVH::Bastion::is_valid_remote_user(user => $user);
+        $fnret = OVH::Bastion::is_valid_remote_user(user => $user, allowWildcards => $userAllowWildcards);
         $fnret or osh_exit $fnret;
         $user = $fnret->value;
     }

diff --git a/lib/perl/OVH/Bastion/Plugin/handleSessions.pm b/lib/perl/OVH/Bastion/Plugin/handleSessions.pm
@@ -45,7 +45,7 @@ sub kill_sessions {
     if ($problems) {
         return R('ERR_CANNOT_TERMINATE_PROCESSES', msg => "Couldn't terminate $problems out of $count processes");
     }
-    return R('OK');
+    return R('OK', value => {count => $count, terminated => ($count - $problems)});
 }
 
 1;
diff --git a/lib/perl/OVH/Bastion/allowdeny.inc b/lib/perl/OVH/Bastion/allowdeny.inc
@@ -80,9 +80,9 @@ sub is_access_way_granted {
     my $ignoreUser = $params{'ignoreUser'};  # ignore remote user COMPLETELY (plop@, or root@, or <nil>@ will all match)
     my $ignorePort = $params{'ignorePort'};  # ignore port COMPLETELY (port 22, 2345, or port-wildcard will all match)
 
-    my $wantedUser = $params{'user'};        # if undef, means we look for a user wildcard allow
+    my $wantedUser = $params{'user'};        # if undef, means we look for a user-any allow
     my $wantedIp   = $params{'ip'};          # can be a single IP or a prefix
-    my $wantedPort = $params{'port'};        # if undef, means we look for a port wildcard allow
+    my $wantedPort = $params{'port'};        # if undef, means we look for a port-any allow
 
     my $way     = $params{'way'};            # personal|group|groupguest|legacy
     my $group   = $params{'group'};          # only meaningful and needed if type=group or type=groupguest
@@ -124,7 +124,6 @@ sub is_access_way_granted {
         # if we get ignorePort, we skip the checks entirely
         if (not $ignorePort) {
             if ($exactPortMatch) {
-
                 # we want an exact match
                 if (not defined $allowedPort) {
                     if (not defined $wantedPort) {
@@ -144,7 +143,7 @@ sub is_access_way_granted {
                 }
             }
             else {
-                # we don't want an exact match (aka wildcards allowed)
+                # we don't want an exact match (aka port-any allowed)
                 if (not defined $allowedPort) {
                     ;                                             # it's a wildcard, will always match
                 }
@@ -163,7 +162,6 @@ sub is_access_way_granted {
         # if we get ignoreUser, we skip the checks entirely
         if (not $ignoreUser) {
             if ($exactUserMatch) {
-
                 # we want an exact match
                 if (not defined $allowedUser) {
                     if (not defined $wantedUser) {
@@ -183,7 +181,7 @@ sub is_access_way_granted {
                 }
             }
             else {
-                # we don't want an exact match (aka wildcards allowed)
+                # we don't want an exact match (aka user-any allowed)
                 if (not defined $allowedUser) {
                     ;                                             # it's a wildcard, will always match
                 }
@@ -192,7 +190,18 @@ sub is_access_way_granted {
                         next;                                     # we want a wildcard, but we don't have it
                     }
                     else {
-                        next if ($wantedUser ne $allowedUser);    # both defined but unequal, not a match
+                        # handle the case where $allowedUser contains wildcards such as '?' or '*'
+                        if (index($allowedUser, '*') >= 0 || index($allowedUser, '?') >= 0) {
+                            # turn wildcards into a regexp
+                            my $allowedUserRe = quotemeta($allowedUser);
+                            $allowedUserRe =~ s{\\\?}{.}g;
+                            $allowedUserRe =~ s{\\\*}{.*}g;
+                            next if ($wantedUser !~ /^$allowedUserRe$/);
+                        }
+                        else {
+                            # doesn't contain a wildcard, simple comparison
+                            next if ($wantedUser ne $allowedUser);    # both defined but unequal, not a match
+                        }
                     }
                 }
             }
@@ -574,8 +583,8 @@ sub print_acls {
             $entry->{'reverseDns'} = $ipReverse;
 
             my @row = (
-                $ipReverse ? $ipReverse : $entry->{'ip'}, $entry->{'port'} ? $entry->{'port'} : '(any)',
-                $entry->{'user'} ? $entry->{'user'} : '(any)', $accessType,
+                $ipReverse ? $ipReverse : $entry->{'ip'}, $entry->{'port'} ? $entry->{'port'} : '*',
+                $entry->{'user'} ? $entry->{'user'} : '*', $accessType,
                 $addedBy,  $addedDate,
                 $expiry,   $entry->{'userComment'} || '-',
                 $forceKey, $forcePassword
@@ -937,10 +946,15 @@ sub ssh_test_access_way {
     $user = OVH::Bastion::config("defaultLogin")->value if not $user;
     $user = $account                                    if not $user;    # defaultLogin empty means the user himself
     $user = OVH::Bastion::get_user_from_env()->value    if not $user;    # no user or account ? get from env then
-    $fnret = OVH::Bastion::is_valid_remote_user(user => $user);
+    $fnret = OVH::Bastion::is_valid_remote_user(user => $user, allowWildcards => 1);
     $fnret or return $fnret;
     $user = $fnret->value;
 
+    # skip special users and wildcarded-users which are not actual remote users
+    if ((grep { $user eq $_ } qw{ !scpupload !scpdownload !sftp }) || ($user =~ /[*?]/)) {
+        return R('OK_MAGIC_USER', msg => "Didn't really test the connection, as the specified user is special");
+    }
+
     if ($group) {
         $fnret = OVH::Bastion::is_valid_group_and_existing(group => $group, groupType => "key");
         $fnret or return $fnret;
@@ -976,10 +990,6 @@ sub ssh_test_access_way {
         );
     }
 
-    if (grep { $user eq $_ } qw{ !scpupload !scpdownload !sftp }) {
-        return R('OK_MAGIC_USER', msg => "Didn't really test the connection, as the specified user is special");
-    }
-
     my $preferredAuthentications = 'publickey';
     $preferredAuthentications .= ',keyboard-interactive' if $ENV{'OSH_KBD_INTERACTIVE'};
 
@@ -1285,7 +1295,7 @@ sub _get_acl_from_file {
 
         # extract custom user if present
         if ($line =~ s/^(\S+)\@//) {
-            $fnret = OVH::Bastion::is_valid_remote_user(user => $1);
+            $fnret = OVH::Bastion::is_valid_remote_user(user => $1, allowWildcards => 1);
             if (!$fnret) {
                 osh_debug("skipping line <$line> because user ($1) is invalid");
                 next;