0

I'm attempting to pass the $PASSWORD variable contents into passwd using expect. This appears to work, adding the users but once you attempt to log in via ssh with one of the users, it doesnt work. If i set the password manually, it's then fine.

Had anyone encountered this issue before?

USERS=(user1 user2 user3)



generatePassword ()
{
        pwgen 16 -N 1
}

# Check if user is root
if [ $(whoami) != 'root' ]; then
        echo "Must be root to run $0"
        exit 1;
fi

# Check if pwgen is installed:
if [[ $(dpkg -s pwgen > /dev/null 2>&1; echo ${PIPESTATUS} ) != '0' ]]; then
        echo -e "pwgen is not installed, this script will not work without it\n\n'apt-get install pwgen'\n"
        exit 1;
    else
        echo -e "Starting Script...\n\n"
fi

# Iterate through users and add them with a password
for i in ${USERS[@]}; do
        PASSWORD=$(generatePassword)
        echo "$i $PASSWORD" >> passwords

        useradd -m "${i}"

        echo -e "Adding $i with a password of '$PASSWORD'\n"

        expect -c "
            spawn passwd ${i}

            expect \"Enter new UNIX password:\"
            send -- \"$PASSWORD\r\"
            send -- \"\r\"
            expect \"Retype new UNIX password:\"
            send -- \"$PASSWORD\r\"
            send -- \"\r\"
             "
            echo -e "\nADDED $i with a password of '$PASSWORD'\n"
done
Detnon
  • 21
  • 5
  • Ewww. You're performing string substitution on content you intend expect to parse as code? That's a significant code smell -- if users were ever allowed to set their own passwords, this could be used as an arbitrary command injection vulnerability. – Charles Duffy Jul 26 '16 at 16:05
  • BTW, see http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html for POSIX conventions around environment variable names (and thus names of shell variables, which share a namespace) -- all-caps names are used by variables with meaning to the shell or operating system, and names with at least one lower-case character are reserved for application use; thus, consider `password` instead of `PASSWORD` to avoid overwriting a variable with meaning to the OS by mistake. – Charles Duffy Jul 26 '16 at 16:17
  • @CharlesDuffy - I know, this is the result of a hour of fumbling in ignorance! Im trying to get better – Detnon Jul 26 '16 at 16:23
  • BTW -- consider getting in the habit of using `printf` instead of `echo -e`. See the POSIX standard for `echo`, particularly the APPLICATION USAGE section, at http://pubs.opengroup.org/onlinepubs/009604599/utilities/echo.html -- bash is actually *noncompliant* (not just extending the standard, but disobeying black-letter behavior) by implementing `-e` by default, and with some non-default shell options (such as `xpg_echo`), that behavior goes away (and `echo -e` prints `-e` on output). – Charles Duffy Jul 26 '16 at 16:27
  • Thanks! Will amend my script with printf – Detnon Jul 26 '16 at 16:29

2 Answers2

0

First: The most immediate problem is that you aren't escaping the backslash literals when you type \r, so these are just changed into rs on the expect side; the smallest possible change that would appear to fix the problem would be to change those to \\r.


However -- don't do it that way: In expect as with other languages, strings should be passed as literals, not substituted into code.

expect -f <(printf '%s\n' '
set username [lindex $argv 0];
set password [lindex $argv 1];
spawn passwd $username

expect "Enter new UNIX password:"
send -- "$password\r"
send -- "\r"
expect "Retype new UNIX password:"
send -- "$password\r"
send -- "\r"
') -- "$i" "$PASSWORD"

You could also save the literal text in question to a file, and run expect -f passwd.expect -- "$i" "$PASSWORD", which would work just as well (and avoid reliance on <() syntax, which is a ksh extension adopted by bash).

Charles Duffy
  • 280,126
  • 43
  • 390
  • 441
  • I may be using Expect in an Un _expect_ ed manner then... Am i correct in assuming that i need the [lindex $argv 0] parts? I thought this was needed if i was using arguments at the beginning of my script? – Detnon Jul 26 '16 at 16:25
  • So, re: the `[lindex $argv 0]` parts, what that's doing is pulling the variables off the command line into awk. (See at the end, after the `--`, where we're putting the shell variables on the awk command line; the `set username` and `set password` is where we take those command-line parameters and make awk variables of them). – Charles Duffy Jul 26 '16 at 20:00
  • The point of doing that rather than substituting the values directly into the awk script is to avoid injection attacks via a surprising or malicious username or password. See https://xkcd.com/327/ for a quick introduction to the SQL equivalent of the attack this protects you against; the best way to keep data from injecting content into code is to *pass your data out-of-band from code*, which this is doing here. – Charles Duffy Jul 26 '16 at 20:01
0

You con't need expect at all: use chpasswd instead of passwd

#!/bin/bash
users=(user1 user2 user3)

# Check if user is root
if [[ "$(id -un)" != 'root' ]]; then
    echo "Must be root to run $0"
    exit 1
fi

# Check if pwgen is installed:
if ! dpkg -s pwgen > /dev/null 2>&1; then
    printf "pwgen is not installed, this script will not work without it\n\n'apt-get install pwgen'\n"
    exit 1
else
    printf "Starting Script...\n\n"
fi

# Iterate through users and create a password
passwords=()
for user in "${users[@]}"; do
    useradd -m "$user"
    password="$user:$(pwgen 16 -N 1)"
    passwords+=("$password")
    echo "Adding user '$user' with '$password'"
done 

printf "%s\n" "${passwords[@]}" | chpasswd

I've added several quotes which are required.
I've simplified the dpkg checking.

Or, perhaps simpler, newusers to perform the "useradd" and "passwd" functions atomically.

for user in "${users[@]}"; do
    password="$user:$(pwgen 16 -N 1)"
    password=${password//:/-}         # replace all colon with hyphen
    printf "%s:%s::::/home/%s:/bin/bash\n" "$user" "${password//:/-}" "$user"
done | newusers

I don't think newusers populates the home directory from /etc/skel though.

glenn jackman
  • 238,783
  • 38
  • 220
  • 352