[pacman-dev] [PATCH 6/6] pacman-key: add --import-trustdb option

[Dan McGee]

On Sat, May 28, 2011 at 9:37 AM, Pang Yan Han <pangyanhan at gmail.com> wrote:
> When pacman is installed, an empty trustdb is created if it is non-existent.
> The --import-trustdb option allows users to import their own trustdb into
> pacman's gpgdir to facilitate signature verification.

The name "import" leads me to believe it doesn't overwrite, but I
think this in fact does that. Is there any way we can actually do a
merge of the existing with the trust db one is pulling from?

> Signed-off-by: Pang Yan Han <pangyanhan at gmail.com>
> ---
>  doc/pacman-key.8.txt     |    4 ++
>  scripts/pacman-key.sh.in |   81 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+), 0 deletions(-)
>
> diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt
> index 8a08480..234e060 100644
> --- a/doc/pacman-key.8.txt
> +++ b/doc/pacman-key.8.txt
> @@ -59,6 +59,10 @@ Commands
>  *-h, \--help*::
>        Output syntax and command line options.
>
> +*\--import-trustdb* <db>::
> +       Overrides the trustdb with db. Confirmation from the user is required before
> +       the trustdb is overwritten, unless the trustdb is empty or non-existent.
> +
>  *-l, \--list*::
>        Equivalent to --list-sigs from GnuPG.
>
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index e795aad..7a1fa42 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -70,10 +70,26 @@ usage() {
>        echo "$(gettext "  -u, --updatedb            Update the trustdb of pacman")"
>        echo "$(gettext "  -V, --version             Show program version")"
>        echo "$(gettext "  --adv <params>            Use pacman's keyring with advanced gpg commands")"
> +       echo "$(gettext "  --import-trustdb <db>     Override pacman's trustdb")"
>        printf "$(gettext "  --reload                  Reload the default keys")"
>        echo
>  }
>
> +## From makepkg
> +#  usage: in_array( $needle, $haystack )
> +# return : 0 - found
> +#          1 - not found
> +##
> +in_array() {
> +       local needle=$1; shift
> +       [[ -z $1 ]] && return 1 # Not found
> +       local item
> +       for item in "$@"; do
> +               [[ $item = $needle ]] && return 0 # Found
> +       done
> +       return 1 # Not found
> +}
> +
>  version() {
>        printf "pacman-key (pacman) %s\n" "${myver}"
>        printf "$(gettext "\
> @@ -228,6 +244,56 @@ if [[ $1 != "--version" && $1 != "-V" && $1 != "--help" && $1 != "-h" && $1 != "
>        fi
>  fi
>
> +import_trustdb() {
> +       local choice=
> +       local valid_choices=('n' 'no' 'y' 'yes')
> +
> +       warning "$(gettext "This option will overwrite your existing trustdb at $PACMAN_KEYRING_DIR/trustdb.gpg with a new one.")"
> +
> +       while ! in_array "$choice" "${valid_choices[@]}"; do
> +               echo -n "$(gettext "==> Do you wish to continue (y/n) ")"
> +               read choice
> +               choice=$(echo "$choice" | tr '[:upper:]' '[:lower:]')
> +       done
> +
> +       if [[ $choice = 'n' || $choice = 'no' ]]; then
> +               msg "$(gettext "Your original trustdb at ${PACMAN_KEYRING_DIR}/trustdb.gpg is preserved.")"
> +               exit 0
> +       fi
Hmm. None of this works for translated messages. Please follow the
pattern established in makepkg in the CLEANCACHE code (line ~1810).
This way the same strings can be used for gettext, and "N" is the
obvious default, which I think it should be. You also didn't translate
'n' or 'no'; we should just use the Y/YES from makepkg over again.
Don't even worry about the valid choice stuff; if you don't type y or
yes, you lose.

I'd also ditch the unnecessary "is preserved" message unless other
people think it is required.

> +
> +       # Reset choice
> +       choice=
> +       echo
> +
> +       if [[ ! -e "${PACMAN_KEYRING_DIR}/trustdb.gpg" ]]; then
> +               msg "$(gettext "No trustdb found at ${PACMAN_KEYRING_DIR}/trustdb.gpg.")"
> +               msg "$(gettext "Importing $1...")"
> +               cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg
> +               msg "$(gettext "Successfully imported $1 to ${PACMAN_KEYRING_DIR}/trustdb.gpg")"
> +       elif [[ $(stat -c "%s" "${PACMAN_KEYRING_DIR}/trustdb.gpg") = "0" ]]; then
> +               msg "$(gettext "Empty trustdb at ${PACMAN_KEYRING_DIR}/trustdb.gpg.")"
> +               msg "$(gettext "Importing $1...")"
> +               cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg
> +               msg "$(gettext "Successfully imported $1 to ${PACMAN_KEYRING_DIR}/trustdb.gpg")"
I don't think this is the proper way to move owner trust values
around- shouldn't we be using a combo and pipe of --export-ownertrust
and --import-ownertrust?

> +       else
> +               warning "$(gettext "trustdb at \"${PACMAN_KEYRING_DIR}/trustdb.gpg\" is not empty.")"
> +               while ! in_array "$choice" "${valid_choices[@]}" ; do
> +                       echo -n "$(gettext "==> Do you wish to overwrite your pacman trustdb? (y/n) ")"
> +                       read choice
> +                       choice=$(echo "$choice" | tr '[:upper:]' '[:lower:]')
> +               done
Wait- why do we have two questions for this? Seems totally silly to
me. If I tell you to import and I have nothing, you should just do it.
If we determine we don't overwrite owner trust values via
export/import (but amend), then we shouldn't do questions at all.

> +
> +               echo
> +               if [[ $choice = 'y' || $choice = 'yes' ]]; then
> +                       cp $1 ${PACMAN_KEYRING_DIR}/trustdb.gpg
> +                       msg "$(gettext "Successfully imported $1 to ${PACMAN_KEYRING_DIR}/trustdb.gpg")"
> +               else
> +                       msg "$(gettext "$1 is not imported")"
> +                       msg "$(gettext "Your original trustdb at ${PACMAN_KEYRING_DIR}/trustdb.gpg is preserved.")"
> +               fi
> +       fi
> +}
> +
>  # Parse global options
>  CONFIG="@sysconfdir@/pacman.conf"
>  PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg"
> @@ -322,6 +388,21 @@ case "${command}" in
>                ;;
>        -h|--help)
>                usage; exit 0 ;;
> +       --import-trustdb)
> +               if (( $# != 1 )); then
> +                       error "$(gettext "You need to specify exactly one trustdb!")"
We don't use exclamation points on other messages in this section, so
we shouldn't start the trend here.

> +                       exit 1
> +               elif [[ ! -e $1 ]]; then
> +                       error "$(gettext "$1 does not exist!")"
> +                       exit 1
> +               elif [[ -d $1 ]]; then
> +                       error "$(gettext "$1 is a directory and cannot be imported!")"
> +                       exit 1
Why not just one -f check and a single "%s is not a trust DB file"
message? Also, don't use $1 directly in gettext, you need to use
substitution vars as is done everywhere else.

> +               fi
> +
> +               import_trustdb $1
> +
> +               ;;
>        -V|--version)
>                version; exit 0 ;;
>        *)
> --
> 1.7.5.rc0.101.g3d23c