FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 07-04-2011, 02:36 PM
Allan McRae
 
Default makepkg: Add support for verifying pgp signatures

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")
" "gpg"
printf "$(gettext " --nocheck Do not run the %s function in the %s")
" "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")
" "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,hel p,holdver"
+OPT_LONG+=",noextract,force,forcever:,geninteg,he lp,holdver,skippgpcheck"
OPT_LONG+=",install,key:,log,nocolor,nobuild,noche ck,nosign,pkg:,rmdeps"
OPT_LONG+=",repackage,skipinteg,sign,source,syncde ps,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
 
Old 07-05-2011, 09:20 PM
Wieland Hoffmann
 
Default makepkg: Add support for verifying pgp signatures

Hallo, Allan McRae:
> On 04/07/11 22:13, Wieland Hoffmann wrote:
> 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 chose to implement it this way because checking the signature means
verifying that the data I downloaded is the data uploaded by the
project which is what data integrity is about. Personally, I would be
surprised if --skipinteg didn't imply --skippgpcheck, although it's kind
of doing the same thing twice. Maybe a switch like --skipchecksums would
be a good idea that doesn't imply skipping ALL integrity checks.

> >+ local file
> >+ local errors=0
>
> We should keep track of the number of non-error warnings too so a
> "==> WARNING:" message could be outputed.

The exact number/reason or just a simple "hey, there were some warnings"
so people scroll up to the actual warning(s)?

--
Wieland
 
Old 07-06-2011, 12:04 AM
Allan McRae
 
Default makepkg: Add support for verifying pgp signatures

On 06/07/11 07:20, Wieland Hoffmann wrote:

Hallo, Allan McRae:

On 04/07/11 22:13, Wieland Hoffmann wrote:
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 chose to implement it this way because checking the signature means
verifying that the data I downloaded is the data uploaded by the
project which is what data integrity is about. Personally, I would be
surprised if --skipinteg didn't imply --skippgpcheck, although it's kind
of doing the same thing twice. Maybe a switch like --skipchecksums would
be a good idea that doesn't imply skipping ALL integrity checks.


That sounds a good idea
--skipinteg - skips all
--skipchecksums - skips checksums only
--skippgpcheck - skips pgp sig check only

Leave the current patch as is with respect to how this is handled and
add that in a separate patch (if you want to do that work...)



+ local file
+ local errors=0


We should keep track of the number of non-error warnings too so a
"==> WARNING:" message could be outputed.


The exact number/reason or just a simple "hey, there were some warnings"
so people scroll up to the actual warning(s)?



Just a "hey there were some warnings" is enough.

Allan
 
Old 07-07-2011, 06:23 AM
Allan McRae
 
Default makepkg: Add support for verifying pgp signatures

On 06/07/11 21:02, 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.
---


Signed-off-by: Allan

Applied to my working branch with the minor changes mentioned below.

<snip>


+check_pgpsigs() {
+ (( SKIPPGPCHECK ))&& return 0
+ (( ! ${#source[@]} ))&& return 0

> + [[ ! source_has_signatures ]]&& return 0

The ${#source[@]} size check is not needed given it is covered by the
source_has_signatures anyway.


<snip>


+
+ if ! gpg --quiet --batch --status-file "$statusfile" --verify "$file" "$sourcefile" 2> /dev/null; then
+ if grep "NO_PUBKEY" "$statusfile"> /dev/null; then
+ echo "$(gettext "Warning: Unknown public key") $(awk '/NO_PUBKEY/ {print $3}' $statusfile)">&2
+ warnings=1
+ else
+ echo "$(gettext "FAILED")">&2
+ errors=1
+ fi
+ else
+ if grep "REVKEYSIG" "$statusfile"> /dev/null; then
+ errors=1
+ echo "$(gettext "Passed")" "-" "$(gettext "Warning: the key has been revoked.")">&2


Just a style consistency change to have the message above the errors=1.

Allan
 
Old 07-19-2011, 01:59 AM
Dan McGee
 
Default makepkg: Add support for verifying pgp signatures

On Wed, Jul 6, 2011 at 6:02 AM, Wieland Hoffmann
<themineo@googlemail.com> 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.
> ---

This is really lacking some documentation, unless I missed something?

We need to mention in man PKGBUILD the special treatment of .sig or
.asc entries (which I think is how this works?)

Additionally, this has a rather silly requirement IMO of adding and
needing checksums of signature files to the checksums arrays. Do we
really want this?

> *doc/makepkg.8.txt * * | * *3 ++
> *scripts/makepkg.sh.in | * 92 ++++++++++++++++++++++++++++++++++++++++++++++++-
> *2 files changed, 94 insertions(+), 1 deletions(-)
>
> diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt
> index e11e9b3..bc1ffc1 100644
> --- a/doc/makepkg.8.txt
> +++ b/doc/makepkg.8.txt
> @@ -87,6 +87,9 @@ Options
> **--skipinteg*::
> * * * *Do not perform any integrity checks, just print a warning instead.
>
> +*--skippgpcheck*::
> + * * * Do not verify PGP signatures of the source files.
> +
> **-h, --help*::
> * * * *Output syntax and command line options.
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 1b132a9..20ba431 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -57,6 +57,7 @@ FORCE=0
> *INFAKEROOT=0
> *GENINTEG=0
> *SKIPINTEG=0
> +SKIPPGPCHECK=0
> *INSTALL=0
> *NOBUILD=0
> *NODEPS=0
> @@ -327,6 +328,15 @@ in_array() {
> * * * *return 1 # Not Found
> *}
>
> +source_has_signatures(){
> + * * * for file in "${source[@]}"; do
> + * * * * * * * if [[ $file =~ .*(sig|asc) ]]; then
> + * * * * * * * * * * * return 0
> + * * * * * * * fi
> + * * * done
> + * * * return 1
> +}
> +
> *get_downloadclient() {
> * * * *# $1 = URL with valid protocol prefix
> * * * *local url=$1
> @@ -674,6 +684,74 @@ check_checksums() {
> * * * *fi
> *}
>
> +check_pgpsigs() {
> + * * * (( SKIPPGPCHECK )) && return 0
> + * * * (( ! ${#source[@]} )) && return 0
> + * * * [[ ! source_has_signatures ]] && return 0
> +
> + * * * msg "$(gettext "Verifying source file signatures with %s...")" "gpg"
> +
> + * * * local file
> + * * * local warning=0
> + * * * local errors=0
> + * * * local statusfile=$(mktemp)
> +
> + * * * for file in "${source[@]}"; do
> + * * * * * * * file="$(get_filename "$file")"
> + * * * * * * * if [[ ! $file =~ .*(sig|asc) ]]; then
> + * * * * * * * * * * * continue
> + * * * * * * * fi
> +
> + * * * * * * * echo -n " * *${file%.*} ... " >&2
> +
> + * * * * * * * if ! file="$(get_filepath "$file")"; then
> + * * * * * * * * * * * echo "$(gettext "SIGNATURE NOT FOUND")" >&2
> + * * * * * * * * * * * errors=1
> + * * * * * * * * * * * continue
> + * * * * * * * fi
> +
> + * * * * * * * if ! sourcefile="$(get_filepath "${file%.*}")"; then
> + * * * * * * * * * * * echo "$(gettext "SOURCE FILE NOT FOUND")" >&2
> + * * * * * * * * * * * errors=1
> + * * * * * * * * * * * continue
> + * * * * * * * fi
> +
> + * * * * * * * if ! gpg --quiet --batch --status-file "$statusfile" --verify "$file" "$sourcefile" 2> /dev/null; then
> + * * * * * * * * * * * if grep "NO_PUBKEY" "$statusfile" > /dev/null; then
> + * * * * * * * * * * * * * * * echo "$(gettext "Warning: Unknown public key") $(awk '/NO_PUBKEY/ {print $3}' $statusfile)" >&2
> + * * * * * * * * * * * * * * * warnings=1
> + * * * * * * * * * * * else
> + * * * * * * * * * * * * * * * echo "$(gettext "FAILED")" >&2
> + * * * * * * * * * * * * * * * errors=1
> + * * * * * * * * * * * fi
> + * * * * * * * else
> + * * * * * * * * * * * if grep "REVKEYSIG" "$statusfile" > /dev/null; then
> + * * * * * * * * * * * * * * * errors=1
> + * * * * * * * * * * * * * * * echo "$(gettext "Passed")" "-" "$(gettext "Warning: the key has been revoked.")" >&2
> + * * * * * * * * * * * elif grep "EXPSIG" "$statusfile" > /dev/null; then
> + * * * * * * * * * * * * * * * warnings=1
> + * * * * * * * * * * * * * * * echo "$(gettext "Passed")" "-" "$(gettext "Warning: the signature has expired.")" >&2
> + * * * * * * * * * * * elif grep "EXPKEYSIG" "$statusfile" > /dev/null; then
> + * * * * * * * * * * * * * * * warnings=1
> + * * * * * * * * * * * * * * * echo "$(gettext "Passed")" "-" "$(gettext *"Warning: the key has expired.")" >&2
> + * * * * * * * * * * * else
> + * * * * * * * * * * * * * * * echo $(gettext "Passed") >&2
> + * * * * * * * * * * * 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 "$(gettext "Warnings have occurred while verifying the signatures. Please make sure you really trust them.")"
> + * * * fi
> +}
> +
> *extract_sources() {
> * * * *msg "$(gettext "Extracting Sources...")"
> * * * *local netfile
> @@ -1478,6 +1556,14 @@ check_software() {
> * * * * * * * *fi
> * * * *fi
>
> + * * * # gpg - source verification
> + * * * if (( ! SKIPPGPCHECK )) && [[ source_has_signatures ]]; then
> + * * * * * * * 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
> @@ -1715,6 +1801,7 @@ usage() {
> * * * *echo "$(gettext " *--pkg <list> * * Only build listed packages from a split package")"
> * * * *printf "$(gettext " *--sign * * * * * Sign the resulting package with %s")
" "gpg"
> * * * *echo "$(gettext " *--skipinteg * * *Do not fail when integrity checks are missing")"
> + * * * echo "$(gettext " *--skippgpcheck * Do not verify source files with pgp signatures")"
> * * * *echo "$(gettext " *--source * * * * Generate a source-only tarball without downloaded sources")"
> * * * *echo
> * * * *printf "$(gettext "These options can be passed to %s:")
" "pacman"
> @@ -1749,7 +1836,7 @@ ARGLIST=("$@")
> *# Parse Command Line Options.
> *OPT_SHORT="AcCdefFghiLmop:rRsV"
> *OPT_LONG="allsource,asroot,ignorearch,check,clean ,nodeps"
> -OPT_LONG+=",noextract,force,forcever:,geninteg,hel p,holdver"
> +OPT_LONG+=",noextract,force,forcever:,geninteg,he lp,holdver,skippgpcheck"
> *OPT_LONG+=",install,key:,log,nocolor,nobuild,noch eck,nosign,pkg:,rmdeps"
> *OPT_LONG+=",repackage,skipinteg,sign,source,syncd eps,version,config:"
> *# Pacman Options
> @@ -1791,6 +1878,7 @@ while true; do
> * * * * * * * *--nosign) * * * * SIGNPKG='n' ;;
> * * * * * * * *-o|--nobuild) * * NOBUILD=1 ;;
> * * * * * * * *-p) * * * * * * * shift; BUILDFILE=$1 ;;
> + * * * * * * * --skippgpcheck) * SKIPPGPCHECK=1;;
> * * * * * * * *--pkg) * * * * * *shift; PKGLIST=($1) ;;
> * * * * * * * *-r|--rmdeps) * * *RMDEPS=1 ;;
> * * * * * * * *-R|--repackage) * REPKG=1 ;;
> @@ -2122,6 +2210,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
> @@ -2200,6 +2289,7 @@ else
> * * * *download_sources
> * * * *if (( ! SKIPINTEG )); then
> * * * * * * * *check_checksums
> + * * * * * * * check_pgpsigs
> * * * *else
> * * * * * * * *warning "$(gettext "Skipping integrity checks.")"
> * * * *fi
> --
> 1.7.6
>
>
>
 

Thread Tools




All times are GMT. The time now is 06:26 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org