From 1e82272487495b08261093f751b3f460053d3af2 Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Wed, 7 Nov 2018 17:25:12 -0500 Subject: [PATCH 1/8] Clean log_msg() in ftpadmin.sh Made the code a bit cleaner and added proper variable quoting. --- scripts/ftpadmin.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 02c4dcf..25b8725 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -48,8 +48,8 @@ EOT } log_msg() { - curdate=`date +"%Y/%m/%d %H:%M:%S"` - echo "$curdate $1" >>$FTPLOG_PATH + curdate="$(date +"%Y/%m/%d %H:%M:%S")" + echo "$curdate $1" >> "$FTPLOG_PATH" } get_user_login_by_UID() { From e3a50177c8bde996895cc8754cce07171ae79a8e Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Wed, 7 Nov 2018 17:27:03 -0500 Subject: [PATCH 2/8] Cleanup get_user_login_by_UID() in ftpadmin.sh Adds proper variable quoting --- scripts/ftpadmin.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 25b8725..6973875 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -54,7 +54,7 @@ log_msg() { get_user_login_by_UID() { uid=$1 - grep $uid /etc/passwd | awk -F : "{if (\$3==$uid) print \$1}" + grep "$uid" /etc/passwd | awk -F : "{if (\$3==$uid) print \$1}" } list_accounts_by_UID() { From e97ddd8be0f2d6e29dd3587d5ad1ae1a86d46f0b Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:33:44 -0500 Subject: [PATCH 3/8] Simplification of list_account_by_UID in ftpadmin Setting / unsetting IFS variables can be removed by setting it only in the loop context. The for cat can be replaced by a simpler while read loop. Proper variable quoting was added. Changed the way modif was optionally passed, this removes the extraneous ':' at the end. echo(1) the lines as we go instead of building an array, this removes the possibility of sub-shell screwups. --- scripts/ftpadmin.sh | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 6973875..6b09bf6 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -60,36 +60,24 @@ get_user_login_by_UID() { list_accounts_by_UID() { uid=$1 - account_list='' - oldIFS=IFS - IFS=$'\n' - - for line in `cat $VPASSWD_PATH` + while IFS=$'\n' read -r line; do - line_uid=`echo $line | cut -d":" -f3` + line_uid="$(echo "$line" | cut -d":" -f3)" - if [ ! "$uid" ] || [ "$line_uid" == "$uid" ]; then - username=`get_user_login_by_UID $line_uid` - account=`echo $line | cut -d":" -f1` - path=`echo $line | cut -d":" -f6` - if [ -r $path/.size ]; then - size=`cat $path/.size` - else - size=0 - fi - #modif=`cat $path/.lastmodified` + if [[ ! "$uid" ]] || [[ "$line_uid" == "$uid" ]]; then + username="$(get_user_login_by_UID "$line_uid")" + account="$(echo "$line" | cut -d":" -f1)" + path="$(echo "$line" | cut -d":" -f6)" + size="$(du -s "$path" | cut -f 1)" + #modif="$(cat $path/.lastmodified)" # Passage en minuscule ? - #account=`echo $account | tr '[A-Z]' '[a-z]'` - #path=`echo $path | tr '[A-Z]' '[a-z]'` + #account="$(echo $account | tr '[A-Z]' '[a-z]')" + #path="$(echo $path | tr '[A-Z]' '[a-z]')" - account_list="${account_list}$username:$account:$path:$size:$modif\n" + echo "$username:$account:$path:$size${modif:+:$modif}" fi - done - - echo "$account_list" - - IFS=$oldIFS + done < "$VPASSWD_PATH" } add_account() { From 66c2c8ab35dea6024fa10bee4bbb959baa6d961a Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:41:32 -0500 Subject: [PATCH 4/8] Shellcheck pass on add_account in ftpadmin Simple variable quoting and more modern shell constructs. --- scripts/ftpadmin.sh | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 6b09bf6..76b43c5 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -87,18 +87,17 @@ add_account() { passwd=$4 cmd="{if (\$3==$user_id) print \$4}" - user_gid=`awk -F : "$cmd" /etc/passwd` + user_gid="$(awk -F : "$cmd" /etc/passwd)" - # Si le répoertoire de travail du compte FTP n'existe pas, on le crée - if [ ! -d "$path" ]; then - mkdir -p $path - chown $user_id:$user_gid $path + # Si le répertoire de travail du compte FTP n'existe pas, on le crée + if [[ ! -d "$path" ]]; then + mkdir -p "$path" + chown "$user_id":"$user_gid" "$path" # fix by tmartin : s/655/755/ - chmod 755 $path - setfacl -R -d -m 'o:rX' $path + chmod 755 "$path" fi - echo `echo $passwd | ftpasswd --passwd --file=$VPASSWD_PATH --name=$account_name --uid=$user_id --gid=$user_gid --home=$path --shell=/bin/false --stdin` + echo "$passwd" | ftpasswd --passwd --file=$VPASSWD_PATH --name="$account_name" --uid="$user_id" --gid="$user_gid" --home="$path" --shell=/bin/false --stdin log_msg "Creation du compte $account_name (uid=$user_id, gid=$user_gid, home=$path)" } From 2ab5a609068074e70e7e48d6f6519baf65ac8150 Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:42:47 -0500 Subject: [PATCH 5/8] Shellcheck pass on edit_password in ftpadmin Removed useless echo and used proper variable quoting. --- scripts/ftpadmin.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 76b43c5..e705810 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -105,7 +105,7 @@ edit_password() { account_name=$1 passwd=$2 - echo `echo $passwd | ftpasswd --passwd --file=$VPASSWD_PATH --name=$account_name --uid=9999 --gid=9999 --home=/dev/null --shell=/dev/null --change-password --stdin` + echo "$passwd" | ftpasswd --passwd --file="$VPASSWD_PATH" --name="$account_name" --uid=9999 --gid=9999 --home=/dev/null --shell=/dev/null --change-password --stdin } From 58642ec0aca4aa7d7bd96b1dd29cbee5be7b126f Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:45:01 -0500 Subject: [PATCH 6/8] Made ftpadmin more usable from the commandline Adds more portable bash invocation. Adds more severe bash evaluation. Added h flag and made improper use print the usage function. Added checks that make sure the parameters are okay. Proper variable quoting, tests and $() use. --- scripts/ftpadmin.sh | 65 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 55 insertions(+), 10 deletions(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index e705810..9b9764e 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash ############################################################ # # @@ -13,6 +13,11 @@ # vim: expandtab softtabstop=4 tabstop=4 shiftwidth=4 showtabline=2 +set -o errexit +set -o pipefail +set -o nounset +#set -x + VPASSWD_PATH="/etc/proftpd/vpasswd" FTPLOG_PATH="/var/log/evolix-ftp.log" @@ -118,8 +123,7 @@ delete_account() { log_msg "Suppression du compte $account_name" } - -while getopts a:u:n:f:p: opt; do +while getopts ha:u:n:f:p: opt; do case "$opt" in a) in_action=$OPTARG @@ -136,26 +140,67 @@ while getopts a:u:n:f:p: opt; do p) in_password=$OPTARG ;; + h) + usage + exit 1 + ;; + *) + usage + exit 1 + ;; esac done -case "$in_action" in +case "${in_action-}" in l) - account_list=`list_accounts_by_UID $in_userid` - echo -e -n $account_list + echo -e "$(list_accounts_by_UID "${in_userid-}")" exit 1 ;; a) - echo -e -n `add_account $in_userid $in_accountname $in_workpath $in_password` + if [[ -z "${in_userid-}" ]]; then + echo "User ID not specified" + elif [[ $in_userid = *[!0-9]* ]]; then + echo "User ID must be a non negative integer" + elif [[ -z "${in_accountname-}" ]]; then + echo "Account name not specified" + elif [[ -z "${in_workpath-}" ]]; then + echo "A directory was not specified" + elif [[ -z "${in_password-}" ]]; then + echo "A password was not specified" + else + echo -e -n \ + "$(add_account \ + "$in_userid" \ + "$in_accountname" \ + "$in_workpath" \ + "$in_password")" + fi exit 1 ;; m) - echo -e -n `edit_password $in_accountname $in_password` + if [[ -z "${in_accountname-}" ]]; then + echo "Account name not specified" + elif [[ -z "${in_password-}" ]]; then + echo "A password was not specified" + else + echo -e -n \ + "$(edit_password \ + "$in_accountname" \ + "$in_password")" + fi exit 1; ;; d) - echo -e -n `delete_account $in_accountname` + if [[ -z "${in_accountname-}" ]]; then + echo "Account name not specified" + else + echo -e -n \ + "$(delete_account "$in_accountname")" + fi exit 1; ;; + *) + usage + exit 1 + ;; esac - From d3be332ba479ce842bc947c2f7152798a81a9fb9 Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:48:35 -0500 Subject: [PATCH 7/8] Shellcheck on delete_account in ftpadmin Removed useless echo and added variable quoting. --- scripts/ftpadmin.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/ftpadmin.sh b/scripts/ftpadmin.sh index 9b9764e..064a00f 100755 --- a/scripts/ftpadmin.sh +++ b/scripts/ftpadmin.sh @@ -119,7 +119,7 @@ delete_account() { account_name=$1 - echo `ftpasswd --passwd --file=$VPASSWD_PATH --name=$account_name --uid=9999 --gid=9999 --home=/dev/null --shell=/dev/null --delete-user` + ftpasswd --passwd --file=$VPASSWD_PATH --name="$account_name" --uid=9999 --gid=9999 --home=/dev/null --shell=/dev/null --delete-user log_msg "Suppression du compte $account_name" } From 393851d4c6386ffae961eb39d2632add0fd561ee Mon Sep 17 00:00:00 2001 From: Patrick Marchand Date: Fri, 9 Nov 2018 17:50:25 -0500 Subject: [PATCH 8/8] Removed unecessary argument in ftpadmin.php delete It caused errors because it wasnt set in some cases, but was never even checked by the invoked shell script. --- inc/ftpadmin.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/inc/ftpadmin.php b/inc/ftpadmin.php index f528318..283398c 100644 --- a/inc/ftpadmin.php +++ b/inc/ftpadmin.php @@ -155,7 +155,7 @@ if ($action=="add") { } elseif ($action=="delete") { - sudoexec("ftpadmin.sh -a d -u $user_id -n $account -f /dev/null -p azertyuiop", $standard_output, $function_output); + sudoexec("ftpadmin.sh -a d -n $account -f /dev/null -p azertyuiop", $standard_output, $function_output); $_SESSION['error'] = null;