On 04/07/11 22:13, Wieland Hoffmann wrote:
Many projects provide signature files along with the source code
archives. It's good to check these, too, when verifying the integrity
of source code archives.
Not everybody is using gpg so the verification can be disabled with
--skippgpcheck.
Additionally, only a warning is displayed when the key that signed the
source file is unknown. Expired keys and signatures aren't considered an
error, too.
---
Looking good. Some general comments:
I saw that --skipinteg implies --skippgpcheck. I noticed this when I
copied a "bad" signature into my source directory and I did not update
the md5sums so used --skipinteg. I was quite surprised when the
signatures did not get checked. Should these be separated more?
I still wonder if --skippgpcheck is too long, but I can not think of a
better name. Suggestions from anyone?
doc/makepkg.8.txt | 3 ++
scripts/makepkg.sh.in | 72 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 74 insertions(+), 1 deletions(-)
diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
index e11e9b3..255fbca 100644
--- a/doc/makepkg.8.txt
+++ b/doc/makepkg.8.txt
@@ -169,6 +169,9 @@ Options
in linkman:makepkg.conf[5]. If not specified in either location, the
default key from the keyring will be used.
+*\--skippgpcheck*::
+ Verify PGP signatures of the source files if provided in the build
script.
+
*\--noconfirm*::
(Passed to pacman) Prevent pacman from waiting for user input before
proceeding with operations.
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index 1b132a9..0b7bed6 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -57,6 +57,7 @@ FORCE=0
INFAKEROOT=0
GENINTEG=0
SKIPINTEG=0
+NOPGPSIGS=0
I'd prefer renaming this to SKIPPGPCHECK=0 to keep the name like the
flag like with SKIPINTEG.
INSTALL=0
NOBUILD=0
NODEPS=0
@@ -674,6 +675,63 @@ check_checksums() {
fi
}
+check_pgpsigs() {
+ (( NOPGPSIGS ))&& return 0
+ (( ! ${#source[@]} ))&& return 0
We need a helper function here "source_has_signatures" that will allow
us to exit if the source array has not signatures in it. that way we
will not get the following message and then no checks. Is will also be
useful elsewhere (see below).
+ msg "$(gettext "Verifying source file signatures with gpg...")"
msg "$(gettext "Verifying source file signatures with %s...")" "gpg"
or just delete the "with gpg" part?
+
+ local file
+ local errors=0
We should keep track of the number of non-error warnings too so a "==>
WARNING:" message could be outputed.
+ local statusfile=$(mktemp)
+
+ for file in "${source[@]}"; do
+ local valid
+ local found=1
+
+ file="$(get_filename "$file")"
+ if [[ $file =~ .*(sig|asc) ]]; then
Lets reduce the nested ifs here with something like:
if [[ ! $file =~ .*(sig|asc) ]]; then
continue
fi
+ echo -n " $file ... ">&2
I'd use ${file%.*} to output the name of the file being checked rather
than the signature.
+ if ! file="$(get_filepath "$file")"; then
+ echo "$(gettext "NOT FOUND")">&2
+ errors=1
+ found=0
+ fi
We should check the source file is also present.
+ if (( found )); then
Lets get rid of this and just do a "continue" instead of setting found=0
above. I'm patching the check_checksums() function to do that too...
+ if ! gpg --quiet --batch --status-file "$statusfile"
--verify "$file" 2> /dev/null; then
+ if grep "NO_PUBKEY" "$statusfile">
/dev/null; then
+ echo "">&2
I think we should stick to plain output here and have a big warning at
the end (as mentioned above). See below for further comments.
+ warning "$(gettext "Unknown public key")
$(awk '/NO_PUBKEY/ {print $3}' $statusfile)">&2
+ else
+ echo "$(gettext "Verification
failed")">&2
+ errors=1
+ fi
+ else
+ if grep "REVKEYSIG" "$statusfile">
/dev/null; then
+ errors=1
+ echo "$(gettext "Verified, but the key that
signed it has been revoked.")">&2
+ elif grep "EXPSIG" "$statusfile">
/dev/null; then
+ echo "$(gettext "Verified, but the signature is
expired.")">&2
+ elif grep "EXPKEYSIG" "$statusfile">
/dev/null; then
+ echo "$(gettext "Verified, but the key that
signed it is expired.")">&2
+ else
+ echo $(gettext "Verified")>&2
+ fi
+ fi
Lets unify the output with the integrity checking as much as possible.
So "FAILED" on failure, "Passed" on success.
Now for the in between cases... maybe like this?
echo "$(gettext "Warning: unknown key '%s'")" $(awk...)
echo "$(gettext "Passed")" "-" "$(gettext "Warning: key has been revoked.")"
echo "$(gettext "Passed")" "-" "$(gettext "Warning: signature has
expired.")"
echo "$(gettext "Passed")" "-" "$(gettext "Warning: key has expired.")"
How does that seem?
+ fi
+ fi
+ done
+
+ rm -f "$statusfile"
+
+ if (( errors )); then
+ error "$(gettext "One or more PGP signatures could not be
verified!")"
+ exit 1
+ fi
if (( warnings )); then
warning "$....
fi
+}
+
extract_sources() {
msg "$(gettext "Extracting Sources...")"
local netfile
@@ -1478,6 +1536,14 @@ check_software() {
fi
fi
+ # gpg - source verification
+ if [[ ! NOPGPSIGS ]]; then
Use the helper function mentioned above so we only give this warning if
signatures are to be checked AND there are some to check.
+ if ! type -p gpg>/dev/null; then
+ error "$(gettext "Cannot find the %s binary required for verifying source
files.")" "gpg"
+ ret=1
+ fi
+ fi
+
# openssl - checksum operations
if (( ! SKIPINTEG )); then
if ! type -p openssl>/dev/null; then
@@ -1712,6 +1778,7 @@ usage() {
printf "$(gettext " --key<key> Specify a key to use for %s signing instead of the
default")\n" "gpg"
printf "$(gettext " --nocheck Do not run the %s function in the %s")\n"
"check()" "$BUILDSCRIPT"
echo "$(gettext " --nosign Do not create a signature for the
package")"
+ echo "$(gettext " --skippgpcheck Disable verification of source files with pgp
signatures")"
echo "$(gettext " --pkg<list> Only build listed packages from a split
package")"
printf "$(gettext " --sign Sign the resulting package with %s")\n"
"gpg"
echo "$(gettext " --skipinteg Do not fail when integrity checks are
missing")"
@@ -1749,7 +1816,7 @@ ARGLIST=("$@")
# Parse Command Line Options.
OPT_SHORT="AcCdefFghiLmop:rRsV"
OPT_LONG="allsource,asroot,ignorearch,check,clean,nodeps"
-OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver"
+OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver,skippgpcheck"
OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps"
OPT_LONG+=",repackage,skipinteg,sign,source,syncdeps,version,config:"
# Pacman Options
@@ -1791,6 +1858,7 @@ while true; do
--nosign) SIGNPKG='n' ;;
-o|--nobuild) NOBUILD=1 ;;
-p) shift; BUILDFILE=$1 ;;
+ --skippgpcheck) NOPGPSIGS=1;;
--pkg) shift; PKGLIST=($1) ;;
-r|--rmdeps) RMDEPS=1 ;;
-R|--repackage) REPKG=1 ;;
@@ -2122,6 +2190,7 @@ if (( SOURCEONLY )); then
if (( ! SKIPINTEG )); then
# We can only check checksums if we have all files.
check_checksums
+ check_pgpsigs
else
warning "$(gettext "Skipping integrity checks.")"
fi
See my comment above about splitting the handling --skipinteg and
--skippgpcheck up.
@@ -2200,6 +2269,7 @@ else
download_sources
if (( ! SKIPINTEG )); then
check_checksums
+ check_pgpsigs
else
warning "$(gettext "Skipping integrity checks.")"
fi
OK... that does seem a lot of comments, but they should be all quite
quick to deal with. I think this version of the patch is exactly how I
want this whole thing to be implemented, so we should be good to go
after those changes.
Allan
