Improve my password generation script

Question

I have created a little password generation script. I'm curious to what improvements can be made for it except input error handling, usage information etc. It's the core functionality I'm interested in seeing improvements upon.

This is what it does (and what I like it to do):

1. Keep it easy to change which Lowercase characters (L), Uppercase characters (U), Numbers (N) and Symbols (S) that are used in passwords.
2. I'd like it to find a new password of legnth 10 for me in max two seconds.
3. It should take a variable length of the password string as an argument.
4. Only a password containing at least one L, U, N and S should be accepted.

Here is the code:

#!/bin/bash

PASSWORDLENGTH=$1
RNDSOURCE=/dev/urandom
L="acdefghjkmnpqrtuvwxy"
U="ABDEFGHJLQRTY"
N="012345679"
S="\-/\\)?=+.%#"

until [ $(echo $password | grep [$L] | grep [$U] | grep [$N] | grep -c [$S] ) == 1 ]; do
    password=$(cat $RNDSOURCE | tr -cd "$L$U$N$S" | head -c $PASSWORDLENGTH)
    echo In progress: $password # It's simply for debug purposes, ignore it
done
echo Final password: $password

My questions are:

• Is there a nicer way of checking if the password is acceptable than the way I'm doing it?
• What about the actual password generation?
• Any coding style improvements? (The short variable names are temporary. Though I'm using uppercase names for "constants" [I know there formally are none] and lowercase for variables. Do you like it?)

Let's vote on the most improved version. :-)

For me it was just an exercise mostly for fun and as a learning experience, albeit I will start using it instead of the generation from KeepassX which I'm using now. It will be interesting to see which improvements and suggestions will come from more experienced Bashistas (I made that word up).

---

I created a little basic script to measure performance: (In case someone thinks it's fun)

#!/bin/bash

SAMPLES=100
SCALE=3

echo -e "PL\tMax\tMin\tAvg"
for p in $(seq 4 50); do
    bcstr=""; max=-98765; min=98765
    for s in $(seq 1 $SAMPLES); do
     gt=$(\time -f %e ./genpassw.sh $p 2>&1 1>/dev/null)
     bcstr="$gt + $bcstr"
     max=$(echo "if($max < $gt ) $gt else $max" | bc)
     min=$(echo "if($min > $gt ) $gt else $min" | bc)
    done
    bcstr="scale=$SCALE;($bcstr 0)/$SAMPLES"
    avg=$(echo $bcstr | bc)
    echo -e "$p\t$max\t$min\t$avg"
done

Answer 1

you could just use uuidgen or pwgen to generate your random passwords, maybe later shuffling some letters around or something of the sort

Answer 2

We had some discussion about some of these issues before

http://stackoverflow.com/questions/55556/password-generation-best-practice

Answer 3

You're throwing away a bunch of randomness in your input stream. Keep those bytes around and translate them into your character set. Replace the password=... statement in your loop with the following:

ALL="$L$U$N$S"
password=$(tr "\000-\377" "$ALL$ALL$ALL$ALL$ALL" < $RNDSOURCE | head -c $PASSWORDLENGTH)

The repetition of $ALL is to ensure that there are >=255 characters in the "map to" set.

I also removed the gratuitous use of cat.

(Edited to clarify that what appears above is not intended to replace the full script, just the inner loop.)

Edit: Here's a much faster strategy that doesn't call out to external programs:

#!/bin/bash

PASSWORDLENGTH=$1
RNDSOURCE=/dev/urandom
L="acdefghjkmnpqrtuvwxy"
U="ABDEFGHJLQRTY"
N="012345679"
# (Use this with tr.)
#S='\-/\\)?=+.%#'
# (Use this for bash.)
S='-/\)?=+.%#'

ALL="$L$U$N$S"

# This function echoes a random index into it's argument.
function rndindex() { echo $(($RANDOM % ${#1})); }

# Make sure the password contains at least one of each class.
password="${L:$(rndindex $L):1}${U:$(rndindex $U):1}${N:$(rndindex $N):1}${S:$(rndindex $S):1}"

# Add random other characters to the password until it is the desired length.
while [[ ${#password} -lt $PASSWORDLENGTH ]]
do
  password=$password${ALL:$(rndindex $ALL):1}
done

# Now shuffle it.
chars=$password
password=""
while [[ ${#password} -lt $PASSWORDLENGTH ]]
do
  n=$(rndindex $chars)
  ch=${chars:$n:1}
  password="$password$ch"
  if [[ $n == $(( ${#chars} - 1 )) ]]; then
      chars="${chars:0:$n}"
  elif [[ $n == 0 ]]; then
      chars="${chars:1}"
  else
      chars="${chars:0:$n}${chars:$((n+1))}"
  fi
done
echo $password

Timing tests show this runs 5-20x faster than the original script, and the time is more predictable from one run to the next.