[PATCH] stop using pwgen
Daniel Dörrhöfer
ddo at openmailbox.org
Sun Dec 18 16:15:40 CET 2016
So why not fix pwgen? Remember the unix philosophy »tools should *do one
thing*, and *do* it well«.
If you think you can do a better job, then you should fork it.
Since entropy is a combination of size and distribution, random number
generators play a major role for generating a good password, but
password generators in software are not perfect. So a user should
consider this and choose a large password x > 40. The entropy of pwgen
is way better than thinking of a password yourself. The distribution
generated by people is catastrophic compared to pwgen.
On 17.12.2016 23:02, Antoine Beaupré wrote:
> pwgen has a long history of generating insecure passphrases. up until
> 2014 (pwgen 2.07, shipped only in Debian jessie, and Ubuntu Vivid) it
> had two serious security vulnerabilities (CVE-2013-4440 and
> CVE-2013-4442) that specifically affect pass. it still defaults to an
> insecure "phoneme" password generation, although pass uses the more
> secure "-s" flag. more information about those issues and more can be
> found in those discussions:
>
> http://www.openwall.com/lists/oss-security/2012/01/22/6
> http://www.openwall.com/lists/oss-security/2013/05/24/7
>
> it is still unclear how actually secure the `--secure` flag is: the
> manpage doesn't say how much entropy is actually used to generate
> passwords. (according to a quick review of the source code: each
> character is chosen randomly based on a byte taken from the
> non-blocking /dev/urandom PRNG, and not all bytes are used in some
> cases, wasting possible entropy.)
>
> since we use pwgen only to generate passphrases, it seems reasonable
> we generate those ourselves. generating passphrases is simple: take a
> source of entropy (UNIX has /dev/random) and turn bytes into
> transportable strings (UNIX has base64):
>
> head -c 18 /dev/random | base64
>
> a 18 bytes password contains (naturally) 144 bits of entropy and
> base64 turns that in a 25 character password, the current default
> length in pass. 20 bytes may be better because we like to think in
> round numbers, which would give us 160 bits of entropy and 28
> character passwords. we could even cram an extra byte in there to get
> 168 bytes of entropy and keep 28 character passwords, but I chose to
> retain the same default to make this patch more acceptable.
>
> base64 passwords are more portable and incur only a ~13% size increase
> compared to original byte stream. we also have a more direct control
> over the entropy level than what pwgen provides: we don't actually
> know how much entropy a given pwgen password has, as it's
> implementation specific.
>
> we depend on GNU coreutils' base46 command because its CLI is
> different from the traditionnal BSD one (BSD requires -e or -d, GNU
> refuses -e) - I assume this can be fixed in the BSD ports.
>
> this removes a dependency and encourages stronger passwords.
>
> ideally, the password generation program would be made configurable
> instead of being hardcoded. that way people could use diceware, pwqgen
> or other software more easily.
> ---
> README | 4 ++--
> contrib/emacs/password-store.el | 1 -
> man/pass.1 | 12 +++++-------
> src/completion/pass.bash-completion | 2 +-
> src/completion/pass.fish-completion | 1 -
> src/completion/pass.zsh-completion | 4 +---
> src/password-store.sh | 21 ++++++++++++---------
> 7 files changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/README b/README
> index 1cc01b9..394d078 100644
> --- a/README
> +++ b/README
> @@ -21,8 +21,8 @@ Depends on:
> http://www.git-scm.com/
> - xclip
> http://sourceforge.net/projects/xclip/
> -- pwgen
> - http://sourceforge.net/projects/pwgen/
> +- base64 from GNU coreutils
> + http://www.gnu.org/software/coreutils/coreutils.html
> - tree >= 1.7.0
> http://mama.indstate.edu/users/ice/tree/
> - GNU getopt
> diff --git a/contrib/emacs/password-store.el b/contrib/emacs/password-store.el
> index a1be788..0d12595 100644
> --- a/contrib/emacs/password-store.el
> +++ b/contrib/emacs/password-store.el
> @@ -104,7 +104,6 @@ outputs error message on failure."
> (defun password-store--run-generate (entry password-length &optional force no-symbols)
> (password-store--run "generate"
> (if force "--force")
> - (if no-symbols "--no-symbols")
> entry
> (number-to-string password-length)))
>
> diff --git a/man/pass.1 b/man/pass.1
> index 33b6036..a47afc8 100644
> --- a/man/pass.1
> +++ b/man/pass.1
> @@ -111,12 +111,10 @@ ensure that temporary files are created in \fI/dev/shm\fP in order to avoid writ
> difficult-to-erase disk sectors. If \fI/dev/shm\fP is not accessible, fallback to
> the ordinary \fITMPDIR\fP location, and print a warning.
> .TP
> -\fBgenerate\fP [ \fI--no-symbols\fP, \fI-n\fP ] [ \fI--clip\fP, \fI-c\fP ] [ \fI--in-place\fP, \fI-i\fP | \fI--force\fP, \fI-f\fP ] \fIpass-name [pass-length]\fP
> -Generate a new password using
> -.BR pwgen (1)
> -of length \fIpass-length\fP (or \fIPASSWORD_STORE_GENERATED_LENGTH\fP if unspecified)
> -and insert into \fIpass-name\fP. If \fI--no-symbols\fP or \fI-n\fP
> -is specified, do not use any non-alphanumeric characters in the generated password.
> +\fBgenerate\fP [ \fI--clip\fP, \fI-c\fP ] [ \fI--in-place\fP, \fI-i\fP | \fI--force\fP, \fI-f\fP ] \fIpass-name [pass-entropy]\fP
> +Generate a new password of given entropy \fIpass-entropy\fP (or \fIPASSWORD_STORE_GENERATED_ENTROPY\fP if unspecified)
> +and insert into \fIpass-name\fP. Password is generated by extracting
> +\fIpass-entropy\P bytes from \fI/dev/random\fP en base64 encoding them.
> If \fI--clip\fP or \fI-c\fP is specified, do not print the password but instead copy
> it to the clipboard using
> .BR xclip (1)
> @@ -424,7 +422,7 @@ is unspecified.
> The location of the text editor used by \fBedit\fP.
> .SH SEE ALSO
> .BR gpg2 (1),
> -.BR pwgen (1),
> +.BR base64 (1),
> .BR git (1),
> .BR xclip (1).
>
> diff --git a/src/completion/pass.bash-completion b/src/completion/pass.bash-completion
> index 456485b..fdc3fe6 100644
> --- a/src/completion/pass.bash-completion
> +++ b/src/completion/pass.bash-completion
> @@ -106,7 +106,7 @@ _pass()
> _pass_complete_entries
> ;;
> generate)
> - COMPREPLY+=($(compgen -W "-n --no-symbols -c --clip -f --force -i --in-place" -- ${cur}))
> + COMPREPLY+=($(compgen -W "-c --clip -f --force -i --in-place" -- ${cur}))
> _pass_complete_entries
> ;;
> cp|copy|mv|rename)
> diff --git a/src/completion/pass.fish-completion b/src/completion/pass.fish-completion
> index c32a42c..25fa835 100644
> --- a/src/completion/pass.fish-completion
> +++ b/src/completion/pass.fish-completion
> @@ -74,7 +74,6 @@ complete -c $PROG -f -A -n '__fish_pass_uses_command insert' -s f -l force -d 'D
> complete -c $PROG -f -A -n '__fish_pass_uses_command insert' -a "(__fish_pass_print_entry_dirs)"
>
> complete -c $PROG -f -A -n '__fish_pass_needs_command' -a generate -d 'Command: generate new password'
> -complete -c $PROG -f -A -n '__fish_pass_uses_command generate' -s n -l no-symbols -d 'Do not use special symbols'
> complete -c $PROG -f -A -n '__fish_pass_uses_command generate' -s c -l clip -d 'Put the password in clipboard'
> complete -c $PROG -f -A -n '__fish_pass_uses_command generate' -s f -l force -d 'Do not prompt before overwritting'
> complete -c $PROG -f -A -n '__fish_pass_uses_command generate' -s i -l in-place -d 'Replace only the first line with the generated password'
> diff --git a/src/completion/pass.zsh-completion b/src/completion/pass.zsh-completion
> index 27ce15a..06659c5 100644
> --- a/src/completion/pass.zsh-completion
> +++ b/src/completion/pass.zsh-completion
> @@ -48,8 +48,6 @@ _pass () {
> ;;
> generate)
> _arguments : \
> - "-n[don't include symbols in password]" \
> - "--no-symbols[don't include symbols in password]" \
> "-c[copy password to the clipboard]" \
> "--clip[copy password to the clipboard]" \
> "-f[force overwrite]" \
> @@ -97,7 +95,7 @@ _pass () {
> "grep:Search inside decrypted password files for matching pattern"
> "show:Decrypt and print a password"
> "insert:Insert a new password"
> - "generate:Generate a new password using pwgen"
> + "generate:Generate a new password"
> "edit:Edit a password with \$EDITOR"
> "mv:Rename the password"
> "cp:Copy the password"
> diff --git a/src/password-store.sh b/src/password-store.sh
> index 63be840..8de4eac 100755
> --- a/src/password-store.sh
> +++ b/src/password-store.sh
> @@ -15,7 +15,9 @@ which gpg2 &>/dev/null && GPG="gpg2"
> PREFIX="${PASSWORD_STORE_DIR:-$HOME/.password-store}"
> X_SELECTION="${PASSWORD_STORE_X_SELECTION:-clipboard}"
> CLIP_TIME="${PASSWORD_STORE_CLIP_TIME:-45}"
> -GENERATED_LENGTH="${PASSWORD_STORE_GENERATED_LENGTH:-25}"
> +# backwards compatibility
> +PASSWORD_STORE_GENERATED_ENTROPY="${PASSWORD_STORE_GENERATED_LENGTH:-$PASSWORD_STORE_GENERATED_ENTROPY}"
> +GENERATED_ENTROPY="${PASSWORD_STORE_GENERATED_ENTROPY:-18}"
>
> export GIT_DIR="${PASSWORD_STORE_GIT:-$PREFIX}/.git"
> export GIT_WORK_TREE="${PASSWORD_STORE_GIT:-$PREFIX}"
> @@ -235,8 +237,8 @@ cmd_usage() {
> overwriting existing password unless forced.
> $PROGRAM edit pass-name
> Insert a new password or edit an existing password using ${EDITOR:-vi}.
> - $PROGRAM generate [--no-symbols,-n] [--clip,-c] [--in-place,-i | --force,-f] pass-name [pass-length]
> - Generate a new password of pass-length (or $GENERATED_LENGTH if unspecified) with optionally no symbols.
> + $PROGRAM generate [--clip,-c] [--in-place,-i | --force,-f] pass-name [entropy]
> + Generate a new password of given entropy (or $GENERATED_ENTROPY if unspecified).
> Optionally put it on the clipboard and clear board after $CLIP_TIME seconds.
> Prompt before overwriting existing password unless forced.
> Optionally replace only the first line of an existing file with a new password.
> @@ -431,30 +433,31 @@ cmd_edit() {
> }
>
> cmd_generate() {
> - local opts clip=0 force=0 symbols="-y" inplace=0
> + local opts clip=0 force=0 symbols="" inplace=0
> opts="$($GETOPT -o ncif -l no-symbols,clip,in-place,force -n "$PROGRAM" -- "$@")"
> local err=$?
> eval set -- "$opts"
> while true; do case $1 in
> - -n|--no-symbols) symbols=""; shift ;;
> + -n|--no-symbols) echo '--no-symbols deprecated'; shift ;;
> -c|--clip) clip=1; shift ;;
> -f|--force) force=1; shift ;;
> -i|--in-place) inplace=1; shift ;;
> --) shift; break ;;
> esac done
>
> - [[ $err -ne 0 || ( $# -ne 2 && $# -ne 1 ) || ( $force -eq 1 && $inplace -eq 1 ) ]] && die "Usage: $PROGRAM $COMMAND [--no-symbols,-n] [--clip,-c] [--in-place,-i | --force,-f] pass-name [pass-length]"
> + [[ $err -ne 0 || ( $# -ne 2 && $# -ne 1 ) || ( $force -eq 1 && $inplace -eq 1 ) ]] && die "Usage: $PROGRAM $COMMAND [--clip,-c] [--in-place,-i | --force,-f] pass-name [pass-length]"
> local path="$1"
> - local length="${2:-$GENERATED_LENGTH}"
> + local entropy="${2:-$GENERATED_ENTROPY}"
> check_sneaky_paths "$path"
> - [[ ! $length =~ ^[0-9]+$ ]] && die "Error: pass-length \"$length\" must be a number."
> + [[ ! $entropy =~ ^[0-9]+$ ]] && die "Error: pass-entropy \"$entropy\" must be a number."
> mkdir -p -v "$PREFIX/$(dirname "$path")"
> set_gpg_recipients "$(dirname "$path")"
> local passfile="$PREFIX/$path.gpg"
>
> [[ $inplace -eq 0 && $force -eq 0 && -e $passfile ]] && yesno "An entry already exists for $path. Overwrite it?"
>
> - local pass="$(pwgen -s $symbols $length 1)"
> + # strip possible newlines if output is wrapped and trailing = signs as they add nothing to the password's entropy
> + local pass="$(head -c $entropy /dev/random | base64 | tr -d '\n=')"
> [[ -n $pass ]] || exit 1
> if [[ $inplace -eq 0 ]]; then
> $GPG -e "${GPG_RECIPIENT_ARGS[@]}" -o "$passfile" "${GPG_OPTS[@]}" <<<"$pass" || die "Password encryption aborted."
More information about the Password-Store
mailing list