[pacman-dev] [PATCH 1/5] pacman-key: keyring management tool
Dan McGee
dpmcgee at gmail.com
Mon Jul 26 17:46:27 EDT 2010
On Mon, Jul 26, 2010 at 3:26 PM, Denis A. Altoé Falqueto
<denisfalqueto at gmail.com> wrote:
> The script pacman-key will manage pacman's keyring. It imports, exports,
> fetches from keyservers, helps in the process of trusting and updates
> the trust database.
>
> Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto at gmail.com>
> ---
> scripts/.gitignore | 1 +
> scripts/Makefile.am | 3 +
> scripts/pacman-key.sh.in | 296 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 300 insertions(+), 0 deletions(-)
> create mode 100644 scripts/pacman-key.sh.in
>
> diff --git a/scripts/.gitignore b/scripts/.gitignore
> index eafc493..1c662de 100644
> --- a/scripts/.gitignore
> +++ b/scripts/.gitignore
> @@ -4,3 +4,4 @@ rankmirrors
> repo-add
> repo-remove
> pkgdelta
> +pacman-key
> diff --git a/scripts/Makefile.am b/scripts/Makefile.am
> index 31e8fb5..c81f703 100644
> --- a/scripts/Makefile.am
> +++ b/scripts/Makefile.am
> @@ -7,6 +7,7 @@ bin_SCRIPTS = \
>
> OURSCRIPTS = \
> makepkg \
> + pacman-key \
> pacman-optimize \
> pkgdelta \
> rankmirrors \
> @@ -14,6 +15,7 @@ OURSCRIPTS = \
>
> EXTRA_DIST = \
> makepkg.sh.in \
> + pacman-key.sh.in \
> pacman-optimize.sh.in \
> pkgdelta.sh.in \
> rankmirrors.sh.in \
> @@ -60,6 +62,7 @@ $(OURSCRIPTS): Makefile
> @mv $@.tmp $@
>
> makepkg: $(srcdir)/makepkg.sh.in
> +pacman-key: ${srcdir}/pacman-key.sh.in
> pacman-optimize: $(srcdir)/pacman-optimize.sh.in
> pkgdelta: $(srcdir)/pkgdelta.sh.in
> rankmirrors: $(srcdir)/rankmirrors.sh.in
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> new file mode 100644
> index 0000000..5cb4066
> --- /dev/null
> +++ b/scripts/pacman-key.sh.in
> @@ -0,0 +1,296 @@
> +#!/bin/bash -e
> +#
> +# pacman-key - manages pacman's keyring
> +# @configure_input@
> +#
> +# Copyright (c) 2010 - Denis A. Altoé Falqueto <denisfalqueto at gmail.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +# gettext initialization
> +export TEXTDOMAIN='pacman'
> +export TEXTDOMAINDIR='@localedir@'
You don't end up actually using gettext anywhere. A script like this
actually should.
> +
> +# Based on apt-key, from Debian
> +# Author: Denis A. Altoé Falqueto <denisfalqueto at gmail dot com>
These quickly become outdated- I'm fine with the copyright, but if you
want this you also need to fix every single problem that comes up with
the script. git does a perfectly fine job at telling you who the
original author is.
> +PACMAN_KEY_VERSION="@PACKAGE_VERSION@"
> +
> +# According to apt-key, gpg doesn't like to be called without a secret keyring.
> +# We will not really need one, because pacman will not sign packages, just verify
> +# their integrities.
> +PACMAN_SHARE_DIR="/usr/share/pacman"
This needs to be autoconf-ed. See doc/Makefile.am and pkgdatadir and
use that like @sysconfdir@ in makepkg.sh.in.
> +
> +# Default parameters for the command gpg. Some more will be added when needed
> +GPG="gpg"
> +GPG_NOKEYRING="${GPG} --ignore-time-conflict --no-options --no-default-keyring"
> +SIG_EXT=".sig"
Would we ever change this? I'd rather have .sig hardcoded just like we
killed other things that will never really change because they are
convention outside of just this package.
> +
> +# Read-only keyring with keys to be added to the keyring
> +ADDED_KEYS="${PACMAN_SHARE_DIR}/addedkeys.gpg"
> +
> +# Read-only keyring with keys removed from the keyring. They need to be removed before
> +# the keys from the added keyring be really imported
> +REMOVED_KEYS="${PACMAN_SHARE_DIR}/removedkeys.gpg"
> +
> +usage() {
> + echo "pacman-key - Pacman's keyring management utility"
> + echo "Usage: $(basename $0) [options] command [arguments]"
> + echo
> + echo "Manage pacman's list of trusted keys"
> + echo
> + echo "Options must be placed before commands. The abailable options are:"
> + echo " --config - set an alternative configuraton file to use. "
> + echo " Default is @sysconfdir@/pacman.conf"
> + echo " --gpgdir - set an alternativ home directory for gnupg. "
> + echo " Default is @sysconfdir@/pacman.d/gnupg"
> + echo
> + echo "The available commands are:"
> + echo " pacman-key add <file> ... - add the key contained in <file> ('-' for stdin)"
> + echo " pacman-key del <keyid> ... - remove the key <keyid>"
> + echo " pacman-key export <keyid> ... - output the key <keyid>"
> + echo " pacman-key exportall - output all trusted keys"
> + echo " pacman-key receive <keyserver> <keyid> ... - fetch the keyids from the specified keyserver URL"
> + echo " pacman-key trust <keyid> ... - set the truslevel of the given key"
> + echo " pacman-key updatedb - update the trustdb of pacman"
> + echo " pacman-key reload - reloads the keys from the keyring package"
> + echo " pacman-key list - list keys"
> + echo " pacman-key finger <keyid> ... - list fingerprints"
> + echo " pacman-key adv <params> - pass advanced options to gpg"
> + echo " pacman-key help - displays this message"
> + echo " pacman-key version - displays the current version"
> +}
I forgot we did this in makepkg; I don't really like it there. This
stuff seems like it belongs in a manpage with a shorter usage() such
as the one in repo-add. Anyone else have thoughts?
Either way, for a essential tool, we will need a manpage.
> +
> +prepare_homedir() {
> + if [[ ! -d ${PACMAN_KEYRING_DIR} ]] ; then
> + mkdir -p "${PACMAN_KEYRING_DIR}"
> + [[ ! -f "${PACMAN_KEYRING_DIR}/secring.gpg" ]] && touch "${PACMAN_KEYRING_DIR}/secring.gpg"
> + [[ ! -f "${PACMAN_KEYRING_DIR}/pubring.gpg" ]] && touch "${PACMAN_KEYRING_DIR}/pubring.gpg"
If the dir didn't exist, why this -f check?
> + chmod 700 "${PACMAN_KEYRING_DIR}"
> + chmod 600 "${PACMAN_KEYRING_DIR}"/*
Why the glob? Can't we just explicitly list the two files previously
touched? Minor nitpick but seems more sensible to me.
> + fi
> +}
> +
> +add_key() {
> + ${GPG_PACMAN} --quiet --batch --import "$1"
> +}
> +
> +remove_key() {
> + ${GPG_PACMAN} --quiet --batch --delete-key --yes "$1"
> +}
> +
> +update_trustdb() {
> + ${GPG_PACMAN} --batch --check-trustdb
> +}
> +
> +list_sigs() {
> + ${GPG_PACMAN} --batch --list-sigs
> +}
> +
> +list_fingerprints() {
> + ${GPG_PACMAN} --batch --fingerprint $*
> +}
> +
> +export_key() {
> + ${GPG_PACMAN} --armor --export "$1"
> +}
> +
> +export_all() {
> + ${GPG_PACMAN} --armor --export
> +}
If these are all one-liners, it would seem to make more sense to
inline them if they are only called from one place. Ignore me if they
are not.
> +
> +trust_key() {
> + # Verify if the key exists in pacman's keyring
> + if ${GPG_PACMAN} --list-key "$1" > /dev/null 2>&1 ; then
> + ${GPG_PACMAN} --edit-key "$1"
> + else
> + echo >&2 "The key identified by $1 doesn't exist"
I'd rather find a way to reuse msg/msg2/warning/error from repo-add
then do this. We might want to move those into a sourced common
utilities file.
> + exit 1
> + fi
> +}
> +
> +reload_keyring() {
> + # Verify the signature of removed keys file
> + if [[ -f ${REMOVED_KEYS} ]] && ! ${GPG_PACMAN} --quiet --verify ${REMOVED_KEYS}${SIG_EXT} ; then
You quoted paths elsewhere but then didn't quote here or many of the
following places.
> + echo >&2 "The signature of file ${REMOVED_KEYS} is not valid."
> + exit 1
> + fi
> +
> + # Verify the signature of the added keys file
> + if [[ -f ${ADDED_KEYS} ]] && ! ${GPG_PACMAN} --quiet --verify ${ADDED_KEYS}${SIG_EXT} ; then
> + echo >&2 "The signature of file ${ADDED_KEYS} is not valid."
> + exit 1
> + fi
> +
> + # Remove the keys from REMOVED_KEYS keyring
> + [[ -r ${REMOVED_KEYS} ]] && cat "${REMOVED_KEYS}" | while read key ; do
> + ${GPG_PACMAN} --quiet --batch --yes --delete-keys ${key}
> + done
> +
> + # Add keys from the current set of keys from pacman-keyring package. The web of trust will
> + # be updated automatically.
> + if [[ -r ${ADDED_KEYS} ]] ; then
> + add_keys=$(${GPG_NOKEYRING} --keyring ${ADDED_KEYS} --with-colons --list-keys | grep ^pub | cut -d: -f5)
> + for add_key in $add_keys; do
> + ${GPG_NOKEYRING} --quiet --batch --keyring $ADDED_KEYS --export $add_key | ${GPG_PACMAN} --import
> + done
> + fi
> +
> + # Update trustdb, just to be sure
> + update_trustdb
> +}
> +
> +receive() {
> + keyserver="$1"
> + shift
> + ${GPG_PACMAN} --keyserver ${keyserver} $*
> +}
> +
> +# PROGRAM START
> +
> +if ! type gettext &>/dev/null; then
> + gettext() {
> + echo "$@"
> + }
> +fi
> +
> +if [[ "$command" != "version" && "$command" != "help" ]] && ! which "${GPG}" >/dev/null 2>&1; then
> + echo >&2 "Warning: gnupg does not seem to be installed."
> + echo >&2 "Warning: pacman-key requires gnupg for most operations."
> + echo >&2
> + exit 1
> +fi
> +
> +# Parse global options
> +CONFIG="@sysconfdir@/pacman.conf"
> +PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg"
> +while [[ $1 =~ ^- ]] ; do
> + case "$1" in
> + --config) shift; CONFIG="$1" ;;
> + --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;;
> + esac
> + shift
> +done
> +
> +if [[ ! -r "$CONFIG" ]] ; then
> + echo >&2 "It is not possible to read $CONFIG."
> + exit 1
> +fi
> +# Read GPGDIR from $CONFIG
> +GPGDIR=$(grep "^ *GPGDir *=" "$CONFIG" | cut -d= -f2)
> +if [[ "$GPGDIR" ]] ; then
> + PACMAN_KEYRING_DIR="$GPGDIR"
> +fi
Hmm, this is kinda ugly but not sure what to suggest just yet.
> +GPG_PACMAN="${GPG} --homedir ${PACMAN_KEYRING_DIR}"
> +
> +prepare_homedir
> +
> +# Parse and execute command
> +command="$1"
> +if [[ -z "$command" ]]; then
> + usage
> + exit 1
> +fi
> +shift
> +
> +case "$command" in
> + add)
> + if (( $# == 0 )) ; then
> + echo >&2 "You need to specify at least one key identifier"
> + usage
> + exit 1
> + fi
> + while (( $# > 0 )) ; do
> + add_key $1
> + shift
> + done
> + ;;
> + del|rm|remove)
> + if (( $# == 0 )) ; then
> + echo >&2 "You need to specify at least one key identifier"
> + usage
> + exit 1
> + fi
> + while (( $# > 0 )) ; do
> + remove_key $1
> + shift
> + done
> + ;;
> + updatedb)
> + update_trustdb
> + ;;
> + reload)
> + reload_keyring
> + ;;
> + list)
> + list_sigs
> + ;;
> + finger*)
> + if (( $# == 0 )) ; then
> + echo >&2 "You need to specify at least one key identifier"
> + usage
> + exit 1
> + fi
> + list_fingerprints $*
> + ;;
> + export)
> + if (( $# == 0 )) ; then
> + echo >&2 "You need to specify at least one key identifier"
> + usage
> + exit 1
> + fi
> + while (( $# > 0 )) ; do
> + export_key $1
> + shift
> + done
> + ;;
> + exportall)
> + export_all
> + ;;
> + receive)
> + if (( $# < 2 )) ; then
> + echo >&2 "You need to specify the keyserver and at least one key identifier"
> + usage
> + exit 1
> + fi
> + receive $*
> + ;;
> + trust)
> + if (( $# == 0 )) ; then
> + echo >&2 "You need to specify at least one key identifier"
> + usage
> + exit 1
> + fi
> + while (( $# > 0 )) ; do
> + trust_key $1
> + shift
> + done
> + ;;
> + adv*)
> + echo "Executing: ${GPG_PACMAN} $*"
> + ${GPG_PACMAN} $* || ret=$?
> + exit $ret
> + ;;
> + --help)
> + usage
> + ;;
> + --version)
> + echo "pacman-key v${PACMAN_KEY_VERSION}"
> + echo " This program can be freely distributed under the GPL v2"
> + ;;
Please use a similar version() function to makepkg/repo-add.
You also have help/version above, and --help/--version here.
> + *)
> + usage
> + exit 1
> + ;;
> +esac
> --
> 1.7.1.1
General thoughts:
1. We don't use the non-dashed option type commands in any other
utility. I'm not necessarily against it but thought it was worth
pointing out.
2. A lot of times our style in shell scripts has tended towards
omitting braces, e.g. ${foobar} would be just $foobar if possible.
3. This was a static code analysis, I need to actually run this and
give it a shot.
4. Don't get discouraged at my review here, I'm just trying to make
sure when we introduce something new we don't have to spend the next
few iterations making it a solid core piece of code. I'd rather get it
right the first time.
-Dan
-Dan
More information about the pacman-dev
mailing list