fix: sudogen: properly handle accounts & groups containing '.'

bin/admin/check-consistency.pl

@@ -686,9 +686,12 @@ if (1) {
 
         # TODO check 0440
         # TODO check there's a matching group (and the other way around)
+
+        # the unescaped group should be present in the file (see GROUPNAME below),
+        # but to backwards compatible, try to guess it by ourselves if it's not
         my $group = $sudoersfile;
         $group =~ s/^.*osh-group-key//;
-        $seensudogroupfile{$group} = 1;
+
         my $fh_sudoers;
         if (!open($fh_sudoers, '<', $sudoersfile)) {
             _err "can't open $sudoersfile file to check: $!";
@@ -698,12 +701,17 @@ if (1) {
         close($fh_sudoers);
         chomp @contents;
 
+        # now try to get it from the file
+        foreach (@contents) {
+            /^# GROUPNAME=key(.+)/ and $group = $1;
+        }
+        $seensudogroupfile{$group} = 1;
+
         my @expected = @template;
         do { s/%GROUP%/key$group/g; s=%BASEPATH%=/opt/bastion=g; }
           for @expected;
 
-        foreach (@expected) {
-            my $wantedline = $_;    # copy
+        foreach my $wantedline (@expected) {
             if (not grep { $_ eq $wantedline } @contents) {
                 _err "missing line in $sudoersfile: $wantedline";
             }

bin/admin/install

@@ -536,10 +536,10 @@ if [ "$nothing" = 0 ]; then
     action_done
 
     # regenerate all group sudoers files
-    "$basedir/bin/sudogen/generate-sudoers.sh" group
+    "$basedir/bin/sudogen/generate-sudoers.sh" create group
 
     # regenerate all accounts sudoers files
-    "$basedir/bin/sudogen/generate-sudoers.sh" account
+    "$basedir/bin/sudogen/generate-sudoers.sh" create account
 
     # create the bastionsync account (needed for master/slave)
     action_doing "Creating the bastionsync account"

bin/helper/osh-accountCreate

@@ -394,13 +394,7 @@ elsif (scalar(@$ipList) > 0) {
 
 # allowed to sudo for the account
 osh_info("Configuring sudoers for this account");
-my $sudoers_dir = OVH::Bastion::sys_getsudoersfolder();
-
-if (-e "$sudoers_dir/osh-account-$account") {
-    osh_debug "sudoers already in place, but overwriting it";
-}
-
-$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'account', $account], must_succeed => 1, noisy_stdout => 1);
+$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'create', 'account', $account], must_succeed => 1, noisy_stdout => 1);
 $fnret or HEXIT('ERR_CANNOT_CREATE_SUDOERS', msg => "An error occurred while creating sudoers for this account");
 
 my $bastionName    = $config->{'bastionName'};

bin/helper/osh-accountDelete

@@ -154,7 +154,10 @@ move("/home/allowkeeper/$account", "$fulldir/allowkeeper");
 $fnret = OVH::Bastion::execute(cmd => ['find', "$fulldir/$account-home", '-maxdepth', '1', '-name', "*.log", '-exec', 'chattr', '-a', '{}', ';']);
 
 # remove sudoers if it's there
-unlink(OVH::Bastion::sys_getsudoersfolder() . "/osh-account-$account");
+$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'delete', 'account', $account], must_succeed => 1, noisy_stdout => 1);
+if (!$fnret) {
+    warn_syslog("Error during account deletion of '$account', couldn't delete sudoers file: $fnret->msg");
+}
 
 # add a text file with all the groups the user was a member of
 $fnret = OVH::Bastion::get_user_groups(account => $account, extra => 1);

bin/helper/osh-groupCreate

@@ -261,13 +261,7 @@ foreach my $gr ("$group-owner", "$group-gatekeeper", "$group-aclkeeper", "osh-wh
 
 # allowed to sudo for the group
 osh_info("Configuring sudoers for this group");
-my $sudoers_dir = OVH::Bastion::sys_getsudoersfolder();
-
-if (-e "$sudoers_dir/osh-group-$group") {
-    osh_debug "sudoers already in place, but overwriting it";
-}
-
-$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'group', $group], must_succeed => 1, noisy_stdout => 1);
+$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'create', 'group', $group], must_succeed => 1, noisy_stdout => 1);
 $fnret or HEXIT('ERR_CANNOT_CREATE_SUDOERS', msg => "An error occurred while creating sudoers for this group");
 
 OVH::Bastion::syslogFormatted(

bin/helper/osh-groupDelete

@@ -185,7 +185,10 @@ foreach my $todelete ("$group-owner", "$group-aclkeeper", "$group-gatekeeper", $
 }
 
 # remove sudoers if it's there
-unlink(OVH::Bastion::sys_getsudoersfolder() . "/osh-group-$group");
+$fnret = OVH::Bastion::execute(cmd => [$OVH::Bastion::BASEPATH . '/bin/sudogen/generate-sudoers.sh', 'delete', 'group', $group], must_succeed => 1, noisy_stdout => 1);
+if (!$fnret) {
+    warn_syslog("Error during group deletion of '$group', couldn't delete sudoers file: $fnret->msg");
+}
 
 OVH::Bastion::syslogFormatted(
     severity => 'info',

bin/sudogen/generate-sudoers.sh

@@ -7,34 +7,48 @@ basedir=$(readlink -f "$(dirname "$0")"/../..)
 # shellcheck source=lib/shell/functions.inc
 . "$basedir"/lib/shell/functions.inc
 
-type="$1"
-name="$2"
+action="$1"
+type="$2"
+name="$3"
 
 die_usage() {
-    echo "Usage: $0 <account|group> [name]" >&2
+    echo "Usage: $0 <create|delete> <account|group> [name]" >&2
     exit 1
 }
 
-generate_account_sudoers()
+manage_account_sudoers()
 {
-    account="$1"
+    todo="$1"
+    account="$2"
     if ! getent passwd "$account" | grep -q ":$basedir/bin/shell/osh.pl$"; then
         action_error "$account is not a bastion account"
         return 1
     fi
-    dst="$SUDOERS_DIR/osh-account-$account"
-    if [ -e "$dst" ]; then
-        action_detail "... overwriting $dst"
-    else
-        action_detail "... generating $dst"
-    fi
-    # normalized account only contain [A-Z0-9_], case sensitive
+
+    # for accounts containing a ".", we need to do a little transformation
+    # as files containing a dot are ignored by sudo
     normalized_account=$(sed -re 's/[^A-Z0-9_]/_/gi' <<< "$account")
     # as we're reducing the amount of possible chars in normalized_account
     # we could have collisions: use MD5 to generate a uniq suffix
     account_suffix=$(md5sum_compat - <<< "$account" | cut -c1-6)
     normalized_account="${normalized_account}_${account_suffix}"
-    # lowercase is prohibited
+    dst="$SUDOERS_DIR/osh-account-$normalized_account"
+
+    if [ "$todo" = delete ]; then
+        action_detail "... deleting $dst"
+        rm -f "$dst"
+        return $?
+    fi
+
+    # or create
+    if [ -e "$dst" ]; then
+        action_detail "... overwriting $dst"
+    else
+        action_detail "... generating $dst"
+    fi
+
+    # within the sudoers file, for variables, lowercase is prohibited,
+    # names can only contain [A-Z0-9_], case sensitive, so we got a step further
     normalized_account=$(tr '[:lower:]' '[:upper:]' <<< "$normalized_account")
     # to avoid race conditions between this generation and master/slave sync,
     # first prepare our file as a .tmp (sudo ignores files containing a '.')
@@ -42,6 +56,7 @@ generate_account_sudoers()
     chmod 0440 "${dst}.tmp"
     {
         echo "# generated from install script"
+        echo "# ACCOUNTNAME=$account"
         for template in $(find "$basedir/etc/sudoers.account.template.d/" -type f -name "*.sudoers" | sort)
         do
             # if $template has two dots, then it's of the form XXX-name.$os.sudoers,
@@ -62,9 +77,10 @@ generate_account_sudoers()
     return 0
 }
 
-generate_group_sudoers()
+manage_group_sudoers()
 {
-    group="$1"
+    todo="$1"
+    group="$2"
     if ! test -f "/home/$group/allowed.ip"; then
         action_error "$group doesn't seem to be a valid bastion group"
         return 1
@@ -73,18 +89,39 @@ generate_group_sudoers()
         action_error "$group doesn't have a $group-gatekeeper counterpart"
         return 1
     fi
-    dst="$SUDOERS_DIR/osh-group-$group"
+
+    # for groups containing a ".", we need to do a little transformation
+    # as files containing a dot are ignored by sudo
+    normalized_group=$(sed -re 's/[^A-Z0-9_]/_/gi' <<< "$group")
+    # as we're reducing the amount of possible chars in normalized_group
+    # we could have collisions: use MD5 to generate a uniq suffix
+    group_suffix=$(md5sum_compat - <<< "$group" | cut -c1-6)
+    normalized_group="${normalized_group}_${group_suffix}"
+    dst="$SUDOERS_DIR/osh-group-$normalized_group"
+
+    if [ "$todo" = delete ]; then
+        action_detail "... deleting $dst"
+        rm -f "$dst"
+        return $?
+    fi
+
+    # or create
     if [ -e "$dst" ]; then
         action_detail "... overwriting $dst"
     else
         action_detail "... generating $dst"
     fi
+
+    # within the sudoers file, for variables, lowercase is prohibited,
+    # names can only contain [A-Z0-9_], case sensitive, so we got a step further
+    normalized_group=$(tr '[:lower:]' '[:upper:]' <<< "$normalized_group")
     # to avoid race conditions between this generation and master/slave sync,
     # first prepare our file as a .tmp (sudo ignores files containing a '.')
     touch "${dst}.tmp"
     chmod 0440 "${dst}.tmp"
     {
         echo "# generated from install script"
+        echo "# GROUPNAME=$group"
         for template in $(find "$basedir/etc/sudoers.group.template.d/" -type f | sort)
         do
             echo
@@ -104,13 +141,23 @@ fi
 nbfailed=0
 if [ "$type" = group ]; then
     if [ -z "$name" ]; then
-        action_doing "Regenerating all groups sudoers files from templates"
-        for group in $(getent group | cut -d: -f1 | grep -- '-gatekeeper$' | sed -e 's/-gatekeeper$//'); do
-            generate_group_sudoers "$group" || nbfailed=$((nbfailed + 1))
-        done
+        if [ "$action" = create ]; then
+            action_doing "Regenerating all groups sudoers files from templates"
+            for group in $(getent group | cut -d: -f1 | grep -- '-gatekeeper$' | sed -e 's/-gatekeeper$//'); do
+                manage_group_sudoers create "$group" || nbfailed=$((nbfailed + 1))
+            done
+        elif [ "$action" = delete ]; then
+            echo "Cowardly refusing to delete all group sudoers, a man needs a name" >&2
+            die_usage
+        fi
     else
-        action_doing "Regenerating group '$name' sudoers file from templates"
-        generate_group_sudoers "$name" || nbfailed=$((nbfailed + 1))
+        if [ "$action" = create ]; then
+            action_doing "Regenerating group '$name' sudoers file from templates"
+            manage_group_sudoers create "$name" || nbfailed=$((nbfailed + 1))
+        elif [ "$action" = delete ]; then
+            action_doing "Deleting group '$name' sudoers file"
+            manage_group_sudoers delete "$name"
+        fi
     fi
     if [ "$nbfailed" != 0 ]; then
         action_error "Failed generating $nbfailed sudoers"
@@ -120,13 +167,23 @@ if [ "$type" = group ]; then
     exit $nbfailed
 elif [ "$type" = account ]; then
     if [ -z "$name" ]; then
-        action_doing "Regenerating all accounts sudoers files from templates"
-        for account in $(getent passwd | grep ":$basedir/bin/shell/osh.pl$" | cut -d: -f1); do
-            generate_account_sudoers "$account"|| nbfailed=$((nbfailed + 1))
-        done
+        if [ "$action" = create ]; then
+            action_doing "Regenerating all accounts sudoers files from templates"
+            for account in $(getent passwd | grep ":$basedir/bin/shell/osh.pl$" | cut -d: -f1); do
+                manage_account_sudoers create "$account"|| nbfailed=$((nbfailed + 1))
+            done
+        elif [ "$action" = delete ]; then
+            echo "Cowardly refusing to delete all account sudoers, a man needs a name" >&2
+            die_usage
+        fi
     else
-        action_doing "Regenerating account '$name' sudoers file from templates"
-        generate_account_sudoers "$name"|| nbfailed=$((nbfailed + 1))
+        if [ "$action" = create ]; then
+            action_doing "Regenerating account '$name' sudoers file from templates"
+            manage_account_sudoers create "$name"|| nbfailed=$((nbfailed + 1))
+        elif [ "$action" = delete ]; then
+            action_doing "Deleting account '$name' sudoers file"
+            manage_account_sudoers delete "$name"
+        fi
     fi
     if [ "$nbfailed" != 0 ]; then
         action_error "Failed generating $nbfailed sudoers"
@@ -134,6 +191,9 @@ elif [ "$type" = account ]; then
         action_done
     fi
     exit $nbfailed
+else
+    echo "Invalid type specified" >&2
+    die_usage
 fi
 
 die_usage

tests/functional/docker/docker_build_and_run_tests.sh

@@ -79,7 +79,7 @@ docker run $privileged \
     --entrypoint=/opt/bastion/tests/functional/docker/target_role.sh \
     -e USER_PUBKEY_B64="$USER_PUBKEY_B64" \
     -e ROOT_PUBKEY_B64="$ROOT_PUBKEY_B64" \
-    -e TARGET_USER=user5000 \
+    -e TARGET_USER="user.5000" \
     -e TEST_QUICK="${TEST_QUICK:-0}" \
     $namespace:"$target"
 docker logs -f "bastion_${target}_target" | sed -u -e 's/^/target: /;s/$/\r/' &
@@ -126,7 +126,7 @@ docker run \
     --tty=$DOCKER_TTY \
     -e TARGET_IP="bastion_${target}_target" \
     -e TARGET_PORT=22 \
-    -e TARGET_USER=user5000 \
+    -e TARGET_USER="user.5000" \
     -e USER_PRIVKEY_B64="$USER_PRIVKEY_B64" \
     -e ROOT_PRIVKEY_B64="$ROOT_PRIVKEY_B64" \
     -e TARGET="$target " \