[pacman-dev] [package signing] Some new patches
Hi, These pathces are kind of sitting here in my drive. So I would like to send it, so we can discuss them. They are named after some items in the TODO list for package signing: https://wiki.archlinux.org/index.php/User:Allan/Package_Signing Just a little fix to make it the way other pacman related scripts are doing. This is a merge of the two first TODO items for makepkg. They are very interdependent, so one patch is better than two, IMHO. When I was implementing the changes in makepkg, I discovered a little problem with some options handling, so this fixes it. Also implements the first TODO item for pacman-key. I have some questions about repo-add, but I'll put it in another email. So, comments very much appreciated, as always. -- Denis Falqueto
Minor change to use macro to substitute the shebang with the correct shell binary, as is done in other scripts. Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> --- scripts/pacman-key.sh.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index b2bfb23..ccaf4b2 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -1,4 +1,4 @@ -#!/bin/bash -e +#!@BASH_SHELL@ -e # # pacman-key - manages pacman's keyring # Based on apt-key, from Debian -- 1.7.4.1
On 19/02/11 11:30, Denis A. Altoé Falqueto wrote:
Minor change to use macro to substitute the shebang with the correct shell binary, as is done in other scripts.
Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto@gmail.com> ---
Signed-off-by: Allan
Two new command line options were added: -n, --sign: forces the generation of a signature for the resulting package, even if not configured in makepkg.conf. The command line has precedence over the option in makepkg.conf. So, even if makepkg.conf has !sign in BUILDENV, passing --sign to makepkg will make it sign the package. --signwithkey <key>: there is a possibility of another key being used, instead of the user's default. For exemple, pacman-keyring package could be signed by a master key, because it needs to be trusted explicitly by the user before the installation of that package. So, this parameter will be used to supply an id for a key to be used to sign the package. Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> --- scripts/makepkg.sh.in | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8381a78..dc71ffd 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -28,7 +28,7 @@ # makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, file, find (findutils), -# gettext, grep, gzip, openssl, sed, tput (ncurses), xz +# gettext, gpg, grep, gzip, openssl, sed, tput (ncurses), xz # gettext initialization export TEXTDOMAIN='pacman' @@ -74,6 +74,8 @@ BUILDFUNC=0 CHECKFUNC=0 PKGFUNC=0 SPLITPKG=0 +SIGN=0 +SIGNKEY="" PKGLIST=() # Forces the pkgver of the current PKGBUILD. Used by the fakeroot call @@ -1106,7 +1108,7 @@ create_package() { } create_signature() { - if [[ $(check_buildenv sign) != "y" ]]; then + if [[ $(check_buildenv sign) != "y" && $SIGN != 1 ]]; then return fi local ret=0 @@ -1116,7 +1118,18 @@ create_signature() { error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" exit 1 # $E_MISSING_PROGRAM fi - gpg --detach-sign --use-agent "$filename" || ret=$? + + # Check if SIGNKEY is valid. + local SIGNWITHKEY="" + if [[ "${SIGNKEY}" ]]; then + if ! gpg --list-key "${SIGNKEY}" 1>/dev/null 2>&1; then + error "$(gettext "The key ${SIGNKEY} doesn\'t exist.")" + exit 1 + fi + SIGNWITHKEY="-u ${SIGNKEY}" + fi + # The signature will be generated directly in ascii-friendly format + gpg --detach-sign --quiet --batch --use-agent ${SIGNWITHKEY} "$filename" 1>/dev/null || ret=$? if (( ! ret )); then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else @@ -1614,6 +1627,9 @@ usage() { echo "$(gettext " --pkg <list> Only build listed packages from a split package")" echo "$(gettext " --skipinteg Do not fail when integrity checks are missing")" echo "$(gettext " --source Generate a source-only tarball without downloaded sources")" + echo "$(gettext " -n, --sign Sign the resulting package with gpg")" + printf "$(gettext " --signwithkey <key>\n\ + Selects an specific key to use for signing, instead of user's default")" echo echo "$(gettext "These options can be passed to pacman:")" echo @@ -1645,11 +1661,11 @@ fi ARGLIST=("$@") # Parse Command Line Options. -OPT_SHORT="AcCdefFghiLmop:rRsV" +OPT_SHORT="AcCdefFghiLmnop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" OPT_LONG+=",install,log,nocolor,nobuild,nocheck,pkg:,rmdeps" -OPT_LONG+=",repackage,skipinteg,source,syncdeps,version,config:" +OPT_LONG+=",repackage,sign,signwithkey:,skipinteg,source,syncdeps,version,config:" # Pacman Options OPT_LONG+=",noconfirm,noprogressbar" OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@" || echo 'PARSE_OPTIONS FAILED')" @@ -1693,6 +1709,8 @@ while true; do -R|--repackage) REPKG=1 ;; --skipinteg) SKIPINTEG=1 ;; --source) SOURCEONLY=1 ;; + --sign) SIGN=1 ;; + --signwithkey) shift; SIGNKEY=$1 ;; -s|--syncdeps) DEP_BIN=1 ;; -h|--help) usage; exit 0 ;; # E_OK -- 1.7.4.1
On Fri, 18 Feb 2011 23:30:22 -0200 Denis A. Altoé Falqueto@256.com wrote:
Two new command line options were added:
Nice to see your work with makepkg in this area Denis - that's key (pun). >From what I've reviewed of what you're doing, I would say you're working in an area that needs it for this to gain usage. So thanks! As for laziness, it's hard to get motivated in an area where your work isn't pushed through to actual use (that's what I meant by politics in this). But from what I'm reading it does sound like some of the devs here do 'get it' with regard to the gaping hole in Arch's package security, which is reassuring. I'm amazed there is so much contention on this issue, though. What Sourceforge had to say after they got caught with their pants down on security: Sourceforge.net has been around a long time, and security decisions made a decade ago are now being reassessed. In most cases past decisions were made around the general principle that we trust open source developers to work together, play nice, and generally do the right thing. Services were rolled out based on widespread trust for the developer community. And that philosophy served us well. But in the years since then, we’ve evolved from hundreds of sf.net users to millions, and in many cases it’s time to re-assess the balance between widespread trust and security. http://sourceforge.net/blog/sourceforge-attack-full-report/ I think Arch is facing a similar transition. Due the quality work of its dev its coming of age, and part of that means more exposure and interest from a security perspective.
On 19/02/11 11:30, Denis A. Altoé Falqueto wrote:
Two new command line options were added:
-n, --sign: forces the generation of a signature for the resulting package, even if not configured in makepkg.conf. The command line has precedence over the option in makepkg.conf. So, even if makepkg.conf has !sign in BUILDENV, passing --sign to makepkg will make it sign the package.
I think we should have a --nosign option to which would negate 'sign' in makepkg.conf. See the --check/--nocheck pair to see how that is achieved.
--signwithkey<key>: there is a possibility of another key being used, instead of the user's default. For exemple, pacman-keyring package could be signed by a master key, because it needs to be trusted explicitly by the user before the installation of that package. So, this parameter will be used to supply an id for a key to be used to sign the package.
Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto@gmail.com> --- scripts/makepkg.sh.in | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8381a78..dc71ffd 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -28,7 +28,7 @@ # makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, file, find (findutils), -# gettext, grep, gzip, openssl, sed, tput (ncurses), xz +# gettext, gpg, grep, gzip, openssl, sed, tput (ncurses), xz
# gettext initialization export TEXTDOMAIN='pacman' @@ -74,6 +74,8 @@ BUILDFUNC=0 CHECKFUNC=0 PKGFUNC=0 SPLITPKG=0 +SIGN=0 +SIGNKEY="" PKGLIST=()
# Forces the pkgver of the current PKGBUILD. Used by the fakeroot call @@ -1106,7 +1108,7 @@ create_package() { }
create_signature() { - if [[ $(check_buildenv sign) != "y" ]]; then + if [[ $(check_buildenv sign) != "y"&& $SIGN != 1 ]]; then return fi local ret=0 @@ -1116,7 +1118,18 @@ create_signature() { error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" exit 1 # $E_MISSING_PROGRAM fi - gpg --detach-sign --use-agent "$filename" || ret=$? + + # Check if SIGNKEY is valid. + local SIGNWITHKEY="" + if [[ "${SIGNKEY}" ]]; then + if ! gpg --list-key "${SIGNKEY}" 1>/dev/null 2>&1; then + error "$(gettext "The key ${SIGNKEY} doesn\'t exist.")" + exit 1 + fi + SIGNWITHKEY="-u ${SIGNKEY}" + fi
I wonder if this is checked too late. I suppose with a package() function in a PKGBUILD, we can not rebuild by using "makepkg -R" but this still seems quite late to abort.
+ # The signature will be generated directly in ascii-friendly format + gpg --detach-sign --quiet --batch --use-agent ${SIGNWITHKEY} "$filename" 1>/dev/null || ret=$?
--batch is bad here. It forces the use of a gpg-agent.
if (( ! ret )); then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else @@ -1614,6 +1627,9 @@ usage() { echo "$(gettext " --pkg<list> Only build listed packages from a split package")" echo "$(gettext " --skipinteg Do not fail when integrity checks are missing")" echo "$(gettext " --source Generate a source-only tarball without downloaded sources")" + echo "$(gettext " -n, --sign Sign the resulting package with gpg")" + printf "$(gettext " --signwithkey<key>\n\ + Selects an specific key to use for signing, instead of user's default")" echo echo "$(gettext "These options can be passed to pacman:")" echo @@ -1645,11 +1661,11 @@ fi ARGLIST=("$@")
# Parse Command Line Options. -OPT_SHORT="AcCdefFghiLmop:rRsV" +OPT_SHORT="AcCdefFghiLmnop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" OPT_LONG+=",install,log,nocolor,nobuild,nocheck,pkg:,rmdeps" -OPT_LONG+=",repackage,skipinteg,source,syncdeps,version,config:" +OPT_LONG+=",repackage,sign,signwithkey:,skipinteg,source,syncdeps,version,config:" # Pacman Options OPT_LONG+=",noconfirm,noprogressbar" OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@" || echo 'PARSE_OPTIONS FAILED')" @@ -1693,6 +1709,8 @@ while true; do -R|--repackage) REPKG=1 ;; --skipinteg) SKIPINTEG=1 ;; --source) SOURCEONLY=1 ;; + --sign) SIGN=1 ;; + --signwithkey) shift; SIGNKEY=$1 ;; -s|--syncdeps) DEP_BIN=1 ;;
-h|--help) usage; exit 0 ;; # E_OK
On Sat, Feb 19, 2011 at 12:51 PM, Allan McRae <allan@archlinux.org> wrote:
+ if [[ "${SIGNKEY}" ]]; then + if ! gpg --list-key "${SIGNKEY}" 1>/dev/null 2>&1; then + error "$(gettext "The key ${SIGNKEY} doesn\'t exist.")" + exit 1 + fi + SIGNWITHKEY="-u ${SIGNKEY}" + fi
I wonder if this is checked too late. I suppose with a package() function in a PKGBUILD, we can not rebuild by using "makepkg -R" but this still seems quite late to abort.
I've changed that test to happen just after check_sanity. Of course, the signature is only tested if there is need to sign. The new patch will go in a second. -- A: Because it obfuscates the reading. Q: Why is top posting so bad? ------------------------------------------- Denis A. Altoe Falqueto Linux user #524555 -------------------------------------------
Three new command line options were added: -n, --sign: forces the generation of a signature for the resulting package, even if not configured in makepkg.conf. The command line has precedence over the option in makepkg.conf. So, even if makepkg.conf has !sign in BUILDENV, passing --sign to makepkg will make it sign the package. --nosign: doesn't sign the resulting package, even if option sign is present in $BUILDENV --signwithkey <key>: there is a possibility of another key being used, instead of the user's default. For exemple, pacman-keyring package could be signed by a master key, because it needs to be trusted explicitly by the user before the installation of that package. So, this parameter will be used to supply an id for a key to be used to sign the package. The verification of validity of key is made just after check_sanity, because if there is a problem, the script will die as soon as possible. Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> --- scripts/makepkg.sh.in | 56 ++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 51 insertions(+), 5 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 8381a78..c7612cd 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -28,7 +28,7 @@ # makepkg uses quite a few external programs during its execution. You # need to have at least the following installed for makepkg to function: # awk, bsdtar (libarchive), bzip2, coreutils, fakeroot, file, find (findutils), -# gettext, grep, gzip, openssl, sed, tput (ncurses), xz +# gettext, gpg, grep, gzip, openssl, sed, tput (ncurses), xz # gettext initialization export TEXTDOMAIN='pacman' @@ -74,6 +74,8 @@ BUILDFUNC=0 CHECKFUNC=0 PKGFUNC=0 SPLITPKG=0 +RUN_SIGN='?' +SIGNKEY="" PKGLIST=() # Forces the pkgver of the current PKGBUILD. Used by the fakeroot call @@ -280,6 +282,23 @@ check_buildenv() { ## +# Check if a signature must be made. +# +# usage : must_sign +# return : y - must sign +# n - must not +## +must_sign() { + if [[ $RUN_SIGN = 'n' || ( $RUN_SIGN != 'y' && $(check_buildenv sign) != 'y' ) ]]; then + echo 'n' + return + fi + + echo 'y' +} + + +## # usage : in_opt_array( $needle, $haystack ) # return : y - enabled # n - disabled @@ -1106,7 +1125,7 @@ create_package() { } create_signature() { - if [[ $(check_buildenv sign) != "y" ]]; then + if [[ $(must_sign) = 'n' ]]; then return fi local ret=0 @@ -1116,7 +1135,14 @@ create_signature() { error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" exit 1 # $E_MISSING_PROGRAM fi - gpg --detach-sign --use-agent "$filename" || ret=$? + + # Check if SIGNKEY is valid. + local SIGNWITHKEY="" + if [[ "${SIGNKEY}" ]]; then + SIGNWITHKEY="-u ${SIGNKEY}" + fi + # The signature will be generated directly in ascii-friendly format + gpg --detach-sign --use-agent ${SIGNWITHKEY} "$filename" &>/dev/null || ret=$? if (( ! ret )); then msg2 "$(gettext "Created signature file %s.")" "$filename.sig" else @@ -1614,6 +1640,10 @@ usage() { echo "$(gettext " --pkg <list> Only build listed packages from a split package")" echo "$(gettext " --skipinteg Do not fail when integrity checks are missing")" echo "$(gettext " --source Generate a source-only tarball without downloaded sources")" + echo "$(gettext " -n, --sign Sign the resulting package with gpg")" + echo "$(gettext " --nosign Do not sign the package")" + printf "$(gettext " --signwithkey <key>\n\ + Selects an specific key to use for signing, instead of user's default")" echo echo "$(gettext "These options can be passed to pacman:")" echo @@ -1645,11 +1675,12 @@ fi ARGLIST=("$@") # Parse Command Line Options. -OPT_SHORT="AcCdefFghiLmop:rRsV" +OPT_SHORT="AcCdefFghiLmnop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" OPT_LONG+=",install,log,nocolor,nobuild,nocheck,pkg:,rmdeps" -OPT_LONG+=",repackage,skipinteg,source,syncdeps,version,config:" +OPT_LONG+=",repackage,sign,nosign,signwithkey:,skipinteg," +OPT_LONG+="source,syncdeps,version,config:" # Pacman Options OPT_LONG+=",noconfirm,noprogressbar" OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@" || echo 'PARSE_OPTIONS FAILED')" @@ -1693,6 +1724,9 @@ while true; do -R|--repackage) REPKG=1 ;; --skipinteg) SKIPINTEG=1 ;; --source) SOURCEONLY=1 ;; + --sign) RUN_SIGN='y' ;; + --nosign) RUN_SIGN='n' ;; + --signwithkey) shift; SIGNKEY=$1 ;; -s|--syncdeps) DEP_BIN=1 ;; -h|--help) usage; exit 0 ;; # E_OK @@ -1889,6 +1923,18 @@ epoch=${epoch:-0} # check the PKGBUILD for some basic requirements check_sanity || exit 1 +# If the package must be signed, verify if the key is valid +if [[ $(must_sign) = 'y' ]]; then + if ! gpg --list-key "${SIGNKEY}" &>/dev/null; then + if [[ ! -z $SIGNKEY ]]; then + error "$(gettext "The key ${SIGNKEY} doesn\'t exist in your keyring.")" + else + error "$(gettext "There is no key in your keyring.")" + fi + exit 1 + fi +fi + # We need to run devel_update regardless of whether we are in the fakeroot # build process so that if the user runs makepkg --forcever manually, we # 1) output the correct pkgver, and 2) use the correct filename when -- 1.7.4.1
The option --trus was changed to --edit-key, for better alignment with the underlying --edit-key of gnupg. The options --config and --gpgdir were not being handled correctly. They would not work if were not used as first arguments always. Now the handling is more flexible. The use of gpg for verification purposes was leaking inconvenient messages to the output, so they were quieted with --quiet, 1>/dev/null and 2>&1. Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> --- doc/pacman-key.8.txt | 4 +- scripts/pacman-key.sh.in | 55 ++++++++++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 5ebbd0a..ba97b82 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -59,8 +59,8 @@ Commands *\--reload*:: Reloads the keys from the keyring package -*-t*, *\--trust* 'keyid':: - Set the trust level of the given key +*-t*, *\--edit-key* 'keyid ...':: + Edit trust properties for the given keys *-u*, *\--updatedb*:: Equivalent to \--check-trustdb in GnuPG diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index ccaf4b2..d97b071 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -71,7 +71,7 @@ usage() { echo "$(gettext " -l | --list - list keys")" echo "$(gettext " -r | --receive <keyserver> <keyid> ... - fetch the keyids from the specified")" echo "$(gettext " keyserver URL")" - echo "$(gettext " -t | --trust <keyid> ... - set the trust level of the given key")" + echo "$(gettext " -t | --edit-key <keyid> ... - edit trust properties for the given keys")" echo "$(gettext " -u | --updatedb - update the trustdb of pacman")" echo "$(gettext " -v | --version - displays the current version")" echo "$(gettext " --adv <params> - use pacman's keyring as target for")" @@ -117,7 +117,7 @@ reload_keyring() { # Verify signatures of related files, if they exist if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Verifying official keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${ADDED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --quiet --verify "${ADDED_KEYS}.sig" 1>/dev/null 2>&1; then error "$(gettext "The signature of file %s is not valid.")" "${ADDED_KEYS}" exit 1 fi @@ -125,7 +125,7 @@ reload_keyring() { if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Verifying deprecated keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --quiet --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null 2>&1; then error "$(gettext "The signature of file %s is not valid.")" "${DEPRECATED_KEYS}" exit 1 fi @@ -133,7 +133,7 @@ reload_keyring() { if [[ -r "${REMOVED_KEYS}" ]]; then msg "$(gettext "Verifying deleted keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${REMOVED_KEYS}.sig"; then + if ! ${GPG_PACMAN} --quiet --verify "${REMOVED_KEYS}.sig" 1>/dev/null 2>&1; then error "$(gettext "The signature of file %s is not valid.")" "${REMOVED_KEYS}" exit 1 fi @@ -229,15 +229,27 @@ if [[ $1 != "--version" && $1 != "-v" && $1 != "--help" && $1 != "-h" && $1 != " fi fi -# Parse global options +# Iterate over the parameters to get --config and --gpgdir +# This time, the parameters will not be consumed. This is needed +# because the code needs to know where is pacman's keyring before +# signing or verifying any files. CONFIG="@sysconfdir@/pacman.conf" -PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" -while [[ $1 =~ ^--(config|gpgdir)$ ]]; do - case "$1" in - --config) shift; CONFIG="$1" ;; - --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;; +GPGDIR="" +isconfig=0 +isgpgdir=0 +for arg in "$@"; do + if (( isconfig )); then + isconfig=0 + CONFIG="$arg" + fi + if (( isgpgdir )); then + isgpgdir=0 + GPGDIR="$arg" + fi + case "$arg" in + --config) isconfig=1;; + --gpgdir) isgpgdir=1;; esac - shift done if [[ ! -r "${CONFIG}" ]]; then @@ -246,11 +258,13 @@ if [[ ! -r "${CONFIG}" ]]; then fi # Read GPGDIR from $CONFIG. -# The pattern is: any spaces or tabs, GPGDir, any spaces or tabs, equal sign -# and the rest of the line. The string is splitted after the first occurrence of = -if [[ GPGDIR=$(find_config "GPGDir") == 0 ]]; then - PACMAN_KEYRING_DIR="${GPGDIR}" -fi +# The precedence for GPGDIR is: +# 1st: command line +# 2nd: pacman.conf +# 3rd: default value +[[ -z "$GPGDIR" ]] && GPGDIR=$(find_config "GPGDir") +[[ -z "$GPGDIR" ]] && GPGDIR="@sysconfdir@/pacman.d/gnupg" +PACMAN_KEYRING_DIR="${GPGDIR}" GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR}" # Parse and execute command @@ -281,10 +295,10 @@ case "${command}" in reload_keyring ;; -l|--list) - ${GPG_PACMAN} --batch --list-sigs "$@" + ${GPG_PACMAN} --list-sigs "$@" ;; -f|--finger) - ${GPG_PACMAN} --batch --fingerprint $* + ${GPG_PACMAN} --fingerprint $* ;; -e|--export) ${GPG_PACMAN} --armor --export "$@" @@ -299,7 +313,7 @@ case "${command}" in shift ${GPG_PACMAN} --keyserver "${keyserver}" --recv-keys "$@" ;; - -t|--trust) + -t|--edit-key) if (( $# == 0 )); then error "$(gettext "You need to specify at least one key identifier")" usage @@ -328,6 +342,9 @@ case "${command}" in version exit 0 ;; + # Parameters already handled + --config) shift ;; + --gpgdir) shift ;; *) usage exit 1 -- 1.7.4.1
On 19/02/11 11:30, Denis A. Altoé Falqueto wrote:
The option --trus was changed to --edit-key, for better alignment with the underlying --edit-key of gnupg.
The options --config and --gpgdir were not being handled correctly. They would not work if were not used as first arguments always. Now the handling is more flexible.
The use of gpg for verification purposes was leaking inconvenient messages to the output, so they were quieted with --quiet, 1>/dev/null and 2>&1.
Signed-off-by: Denis A. Altoé Falqueto<denisfalqueto@gmail.com> --- doc/pacman-key.8.txt | 4 +- scripts/pacman-key.sh.in | 55 ++++++++++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 21 deletions(-)
diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 5ebbd0a..ba97b82 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -59,8 +59,8 @@ Commands *\--reload*:: Reloads the keys from the keyring package
-*-t*, *\--trust* 'keyid':: - Set the trust level of the given key +*-t*, *\--edit-key* 'keyid ...':: + Edit trust properties for the given keys
*-u*, *\--updatedb*:: Equivalent to \--check-trustdb in GnuPG diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index ccaf4b2..d97b071 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -71,7 +71,7 @@ usage() { echo "$(gettext " -l | --list - list keys")" echo "$(gettext " -r | --receive<keyserver> <keyid> ... - fetch the keyids from the specified")" echo "$(gettext " keyserver URL")" - echo "$(gettext " -t | --trust<keyid> ... - set the trust level of the given key")" + echo "$(gettext " -t | --edit-key<keyid> ... - edit trust properties for the given keys")" echo "$(gettext " -u | --updatedb - update the trustdb of pacman")" echo "$(gettext " -v | --version - displays the current version")" echo "$(gettext " --adv<params> - use pacman's keyring as target for")" @@ -117,7 +117,7 @@ reload_keyring() { # Verify signatures of related files, if they exist if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Verifying official keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${ADDED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --quiet --verify "${ADDED_KEYS}.sig" 1>/dev/null 2>&1; then
using "&>/dev/null" would be cleaner. And given --quiet is obviously not doing much, should we just remove it? As an aside, the man page for gpg says --verify should "verify it without generating any output". Clearly there is output...
error "$(gettext "The signature of file %s is not valid.")" "${ADDED_KEYS}" exit 1 fi @@ -125,7 +125,7 @@ reload_keyring() {
if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Verifying deprecated keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --quiet --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null 2>&1; then error "$(gettext "The signature of file %s is not valid.")" "${DEPRECATED_KEYS}" exit 1 fi @@ -133,7 +133,7 @@ reload_keyring() {
if [[ -r "${REMOVED_KEYS}" ]]; then msg "$(gettext "Verifying deleted keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${REMOVED_KEYS}.sig"; then + if ! ${GPG_PACMAN} --quiet --verify "${REMOVED_KEYS}.sig" 1>/dev/null 2>&1; then error "$(gettext "The signature of file %s is not valid.")" "${REMOVED_KEYS}" exit 1 fi @@ -229,15 +229,27 @@ if [[ $1 != "--version"&& $1 != "-v"&& $1 != "--help"&& $1 != "-h"&& $1 != " fi fi
-# Parse global options +# Iterate over the parameters to get --config and --gpgdir +# This time, the parameters will not be consumed. This is needed +# because the code needs to know where is pacman's keyring before +# signing or verifying any files. CONFIG="@sysconfdir@/pacman.conf" -PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" -while [[ $1 =~ ^--(config|gpgdir)$ ]]; do - case "$1" in - --config) shift; CONFIG="$1" ;; - --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;; +GPGDIR="" +isconfig=0 +isgpgdir=0 +for arg in "$@"; do + if (( isconfig )); then + isconfig=0 + CONFIG="$arg" + fi + if (( isgpgdir )); then + isgpgdir=0 + GPGDIR="$arg" + fi + case "$arg" in + --config) isconfig=1;; + --gpgdir) isgpgdir=1;;
This leaves --config and --gpgdir in "$@". So if I run (e.g.) "pacman-key --delete <keyid> --config <file>" Then the command run will be: ${GPG_PACMAN} --quiet --batch --delete-key --yes "$@" where "$@" is expanded to "<keyid> --config <file>", which clearly is bad... So this needs to be slightly more clever. Allan
On Sat, Feb 19, 2011 at 12:33 PM, Allan McRae <allan@archlinux.org> wrote:
+ if ! ${GPG_PACMAN} --quiet --verify "${ADDED_KEYS}.sig" 1>/dev/null 2>&1; then
using "&>/dev/null" would be cleaner. And given --quiet is obviously not doing much, should we just remove it?
As an aside, the man page for gpg says --verify should "verify it without generating any output". Clearly there is output...
Yes, really weird...
+ case "$arg" in + --config) isconfig=1;; + --gpgdir) isgpgdir=1;;
This leaves --config and --gpgdir in "$@". So if I run (e.g.) "pacman-key --delete <keyid> --config <file>"
Then the command run will be:
${GPG_PACMAN} --quiet --batch --delete-key --yes "$@"
where "$@" is expanded to "<keyid> --config <file>", which clearly is bad... So this needs to be slightly more clever.
I've remade the code. Now, it iterates over the parameters and creates another vector for the values that are not related to --config and --gpgdir. The real processing is made over the vector, not over $@. It is a little uncommon, but pacman-key must know beforehand what is the configuration and gpg home. I'll resend the patches as a reply just after this mail. -- A: Because it obfuscates the reading. Q: Why is top posting so bad? ------------------------------------------- Denis A. Altoe Falqueto Linux user #524555 -------------------------------------------
The option --trus was changed to --edit-key, for better alignment with the underlying --edit-key of gnupg. The options --config and --gpgdir were not being handled correctly. They would not work if were not used as first arguments always. Now the handling is more flexible. The use of gpg for verification purposes was leaking inconvenient messages to the output, so they were quieted with --quiet, 1>/dev/null and 2>&1. Signed-off-by: Denis A. Altoé Falqueto <denisfalqueto@gmail.com> --- doc/pacman-key.8.txt | 4 +- scripts/pacman-key.sh.in | 100 ++++++++++++++++++++++++++++++---------------- 2 files changed, 67 insertions(+), 37 deletions(-) diff --git a/doc/pacman-key.8.txt b/doc/pacman-key.8.txt index 5ebbd0a..ba97b82 100644 --- a/doc/pacman-key.8.txt +++ b/doc/pacman-key.8.txt @@ -59,8 +59,8 @@ Commands *\--reload*:: Reloads the keys from the keyring package -*-t*, *\--trust* 'keyid':: - Set the trust level of the given key +*-t*, *\--edit-key* 'keyid ...':: + Edit trust properties for the given keys *-u*, *\--updatedb*:: Equivalent to \--check-trustdb in GnuPG diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in index ccaf4b2..dd20172 100644 --- a/scripts/pacman-key.sh.in +++ b/scripts/pacman-key.sh.in @@ -71,7 +71,7 @@ usage() { echo "$(gettext " -l | --list - list keys")" echo "$(gettext " -r | --receive <keyserver> <keyid> ... - fetch the keyids from the specified")" echo "$(gettext " keyserver URL")" - echo "$(gettext " -t | --trust <keyid> ... - set the trust level of the given key")" + echo "$(gettext " -t | --edit-key <keyid> ... - edit trust properties for the given keys")" echo "$(gettext " -u | --updatedb - update the trustdb of pacman")" echo "$(gettext " -v | --version - displays the current version")" echo "$(gettext " --adv <params> - use pacman's keyring as target for")" @@ -117,7 +117,7 @@ reload_keyring() { # Verify signatures of related files, if they exist if [[ -r "${ADDED_KEYS}" ]]; then msg "$(gettext "Verifying official keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${ADDED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --verify "${ADDED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${ADDED_KEYS}" exit 1 fi @@ -125,7 +125,7 @@ reload_keyring() { if [[ -r "${DEPRECATED_KEYS}" ]]; then msg "$(gettext "Verifying deprecated keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${DEPRECATED_KEYS}.sig" 1>/dev/null; then + if ! ${GPG_PACMAN} --verify "${DEPRECATED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${DEPRECATED_KEYS}" exit 1 fi @@ -133,7 +133,7 @@ reload_keyring() { if [[ -r "${REMOVED_KEYS}" ]]; then msg "$(gettext "Verifying deleted keys file signature...")" - if ! ${GPG_PACMAN} --quiet --batch --verify "${REMOVED_KEYS}.sig"; then + if ! ${GPG_PACMAN} --verify "${REMOVED_KEYS}.sig" &>/dev/null; then error "$(gettext "The signature of file %s is not valid.")" "${REMOVED_KEYS}" exit 1 fi @@ -229,15 +229,40 @@ if [[ $1 != "--version" && $1 != "-v" && $1 != "--help" && $1 != "-h" && $1 != " fi fi -# Parse global options +# Iterate over the parameters to get --config and --gpgdir +# The other parameters will be filtered to another array, +# so --config and --gpgdir don't interfere with other options. CONFIG="@sysconfdir@/pacman.conf" -PACMAN_KEYRING_DIR="@sysconfdir@/pacman.d/gnupg" -while [[ $1 =~ ^--(config|gpgdir)$ ]]; do - case "$1" in - --config) shift; CONFIG="$1" ;; - --gpgdir) shift; PACMAN_KEYRING_DIR="$1" ;; +declare -a PARAMS +GPGDIR="" +isconfig=0 +isgpgdir=0 +for arg in "$@"; do + if (( isconfig )); then + isconfig=0 + CONFIG="$arg" + if [[ ! -f "$CONFIG" ]]; then + error "$(gettext "The configuration file is not a valid file.")" + usage + exit 1 + fi + continue + fi + if (( isgpgdir )); then + isgpgdir=0 + GPGDIR="$arg" + if [[ ! -d "$GPGDIR" ]]; then + error "$(gettext "The home directory for GnuPG is not valid.")" + usage + exit 1 + fi + continue + fi + case "$arg" in + --config) isconfig=1;; + --gpgdir) isgpgdir=1;; + *) PARAMS[${#PARAMS[@]}]="$arg" esac - shift done if [[ ! -r "${CONFIG}" ]]; then @@ -246,33 +271,35 @@ if [[ ! -r "${CONFIG}" ]]; then fi # Read GPGDIR from $CONFIG. -# The pattern is: any spaces or tabs, GPGDir, any spaces or tabs, equal sign -# and the rest of the line. The string is splitted after the first occurrence of = -if [[ GPGDIR=$(find_config "GPGDir") == 0 ]]; then - PACMAN_KEYRING_DIR="${GPGDIR}" -fi +# The precedence for GPGDIR is: +# 1st: command line +# 2nd: pacman.conf +# 3rd: default value +[[ -z "$GPGDIR" ]] && GPGDIR=$(find_config "GPGDir") +[[ -z "$GPGDIR" ]] && GPGDIR="@sysconfdir@/pacman.d/gnupg" +PACMAN_KEYRING_DIR="${GPGDIR}" GPG_PACMAN="gpg --homedir ${PACMAN_KEYRING_DIR}" # Parse and execute command -command="$1" +command="${PARAMS[0]}" if [[ -z "${command}" ]]; then usage exit 1 fi -shift +unset PARAMS[0] case "${command}" in -a|--add) # If there is no extra parameter, gpg will read stdin - ${GPG_PACMAN} --quiet --batch --import "$@" + ${GPG_PACMAN} --quiet --batch --import "${PARAMS[@]}" ;; -d|--del) - if (( $# == 0 )); then + if (( ${#PARAMS[@]} == 0 )); then error "$(gettext "You need to specify at least one key identifier")" usage exit 1 fi - ${GPG_PACMAN} --quiet --batch --delete-key --yes "$@" + ${GPG_PACMAN} --quiet --batch --delete-key --yes "${PARAMS[@]}" ;; -u|--updatedb) ${GPG_PACMAN} --batch --check-trustdb @@ -281,39 +308,39 @@ case "${command}" in reload_keyring ;; -l|--list) - ${GPG_PACMAN} --batch --list-sigs "$@" + ${GPG_PACMAN} --list-sigs "${PARAMS[@]}" ;; -f|--finger) - ${GPG_PACMAN} --batch --fingerprint $* + ${GPG_PACMAN} --fingerprint "${PARAMS[@]}" ;; -e|--export) - ${GPG_PACMAN} --armor --export "$@" + ${GPG_PACMAN} --armor --export "${PARAMS[@]}" ;; -r|--receive) - if (( $# < 2 )); then + if (( ${#PARAMS[@]} < 2 )); then error "$(gettext "You need to specify the keyserver and at least one key identifier")" usage exit 1 fi - keyserver="$1" - shift - ${GPG_PACMAN} --keyserver "${keyserver}" --recv-keys "$@" + keyserver="${PARAMS[0]}" + unset PARAMS[0] + ${GPG_PACMAN} --keyserver "${keyserver}" --recv-keys "${PARAMS[@]}" ;; - -t|--trust) - if (( $# == 0 )); then + -t|--edit-key) + if (( ${#PARAMS[@]} == 0 )); then error "$(gettext "You need to specify at least one key identifier")" usage exit 1 fi - while (( $# > 0 )); do + while (( ${#PARAMS[@]} > 0 )); do # Verify if the key exists in pacman's keyring - if ${GPG_PACMAN} --list-keys "$1" > /dev/null 2>&1; then - ${GPG_PACMAN} --edit-key "$1" + if ${GPG_PACMAN} --list-keys "${PARAMS[0]}" &>/dev/null; then + ${GPG_PACMAN} --edit-key "${PARAMS[0]}" else - error "$(gettext "The key identified by %s doesn't exist")" "$1" + error "$(gettext "The key identified by %s doesn't exist")" "${PARAMS[0]}" exit 1 fi - shift + unset PARAMS[0] done ;; --adv) @@ -328,6 +355,9 @@ case "${command}" in version exit 0 ;; + # Parameters already handled + --config) shift ;; + --gpgdir) shift ;; *) usage exit 1 -- 1.7.4.1
participants (3)
-
Allan McRae
-
Denis A. Altoé Falqueto
-
IgnorantGuru