[pacman-dev] [PATCH 0/2] Add support for verifying pgp signatures to makepkg
Hi, this adds support for verifying pgp signatures provided by upstream to makepkg. A new array pgpsigs is defined holding the URLs to all the signature files. However, there're still a few quirks: * You have to manually import the key which signed the source. Actually that's good, but: * You don't know why the verification failed. It's either a wrong signature or the key is simply not known to gnupg. This is really bad, so I've chosen to make pgp verification optional for now. makepkg --pgp enables it. Wieland Hoffmann (2): Add support for verifying pgp signatures to makepkg And update the manpages accordingly doc/PKGBUILD.5.txt | 5 ++++ doc/makepkg.8.txt | 3 ++ scripts/makepkg.sh.in | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 58 insertions(+), 2 deletions(-) -- 1.7.5.4
--- scripts/makepkg.sh.in | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 50 insertions(+), 2 deletions(-) diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 78cd4cf..cc4f152 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -516,7 +516,7 @@ download_sources() { pushd "$SRCDEST" &>/dev/null local netfile - for netfile in "${source[@]}"; do + for netfile in "${source[@]}" "${pgpsigs[@]}"; do local file=$(get_filepath "$netfile" || true) if [[ -n "$file" ]]; then msg2 "$(gettext "Found %s")" "${file##*/}" @@ -680,6 +680,49 @@ check_checksums() { fi } +check_pgpsigs() { + (( ! ${#source[@]} )) && return 0 + (( ! ${#pgpsigs[@]})) && return 0 + + if ! type -p gpg >/dev/null; then + error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" + exit 1 # $E_MISSING_PROGRAM + fi + + msg "$(gettext "Validating source files with gpg...")" + + local file + local errors=0 + + for file in "${pgpsigs[@]}"; do + local valid + local found=1 + + file="$(get_filename "$file")" + echo -n " ${file%.sig} ... " >&2 + + if ! file="$(get_filepath "$file")"; then + echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 + errors=1 + else + echo $(gettext "Verified") >&2 + fi + fi + done + + if (( errors )); then + error "$(gettext "One or more pgp signatures could not be verified!")" + exit 1 + fi +} + extract_sources() { msg "$(gettext "Extracting Sources...")" local netfile @@ -1614,6 +1657,7 @@ usage() { echo "$(gettext " --key <key> Specify a key to use for gpg signing instead of the default")" printf "$(gettext " --nocheck Do not run the check() function in the %s")\n" "$BUILDSCRIPT" echo "$(gettext " --nosign Do not create a signature for the package")" + echo "$(gettext " --pgp Enable verification of source files with pgp signatures")" echo "$(gettext " --pkg <list> Only build listed packages from a split package")" echo "$(gettext " --sign Sign the resulting package with gpg")" echo "$(gettext " --skipinteg Do not fail when integrity checks are missing")" @@ -1651,7 +1695,7 @@ ARGLIST=("$@") # Parse Command Line Options. OPT_SHORT="AcCdefFghiLmop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" -OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" +OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver,pgp" OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps" OPT_LONG+=",repackage,skipinteg,sign,source,syncdeps,version,config:" # Pacman Options @@ -1694,6 +1738,7 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; + --pgp) PGPSIGS=1;; --pkg) shift; PKGLIST=($1) ;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; @@ -2129,6 +2174,9 @@ else download_sources if (( ! SKIPINTEG )); then check_checksums + if (( PGPSIGS )); then + check_pgpsigs + fi else warning "$(gettext "Skipping integrity checks.")" fi -- 1.7.5.4
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote: First, thanks for giving this a try. Commit descriptions are nice to have in permanent history, but all that stuff you wrote in the cover letter won't show up. Can you instead include some of that right here in the patch in commit message-style writing?
--- scripts/makepkg.sh.in | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 78cd4cf..cc4f152 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -516,7 +516,7 @@ download_sources() { pushd "$SRCDEST" &>/dev/null
local netfile - for netfile in "${source[@]}"; do + for netfile in "${source[@]}" "${pgpsigs[@]}"; do local file=$(get_filepath "$netfile" || true) if [[ -n "$file" ]]; then msg2 "$(gettext "Found %s")" "${file##*/}" @@ -680,6 +680,49 @@ check_checksums() { fi }
+check_pgpsigs() { + (( ! ${#source[@]} )) && return 0 + (( ! ${#pgpsigs[@]})) && return 0 + + if ! type -p gpg >/dev/null; then + error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" + exit 1 # $E_MISSING_PROGRAM + fi Please see the check_software patch (http://projects.archlinux.org/pacman.git/commit/?id=7468956236), this will need to be updated to work that way instead of how we used to do it.
+ + msg "$(gettext "Validating source files with gpg...")" + + local file + local errors=0 + + for file in "${pgpsigs[@]}"; do + local valid + local found=1 + + file="$(get_filename "$file")" + echo -n " ${file%.sig} ... " >&2 + + if ! file="$(get_filepath "$file")"; then What are you doing here? Assignment or comparison? Do this in two statements rather than trying to be cute, and use an actual [[ -z ]] block please.
+ echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 Any need to eat stderr? If things only show up in exceptional cases, I'd rather it come through.
+ errors=1 + else + echo $(gettext "Verified") >&2 + fi + fi + done + + if (( errors )); then + error "$(gettext "One or more pgp signatures could not be verified!")" + exit 1 + fi +} + extract_sources() { msg "$(gettext "Extracting Sources...")" local netfile @@ -1614,6 +1657,7 @@ usage() { echo "$(gettext " --key <key> Specify a key to use for gpg signing instead of the default")" printf "$(gettext " --nocheck Do not run the check() function in the %s")\n" "$BUILDSCRIPT" echo "$(gettext " --nosign Do not create a signature for the package")" + echo "$(gettext " --pgp Enable verification of source files with pgp signatures")" I'm not real keen on this option as it isn't clear whether it deals with signing or verification. I think --verify would be better, but I'll leave that up to Allan.
echo "$(gettext " --pkg <list> Only build listed packages from a split package")" echo "$(gettext " --sign Sign the resulting package with gpg")" echo "$(gettext " --skipinteg Do not fail when integrity checks are missing")" @@ -1651,7 +1695,7 @@ ARGLIST=("$@") # Parse Command Line Options. OPT_SHORT="AcCdefFghiLmop:rRsV" OPT_LONG="allsource,asroot,ignorearch,check,clean,cleancache,nodeps" -OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver" +OPT_LONG+=",noextract,force,forcever:,geninteg,help,holdver,pgp" OPT_LONG+=",install,key:,log,nocolor,nobuild,nocheck,nosign,pkg:,rmdeps" OPT_LONG+=",repackage,skipinteg,sign,source,syncdeps,version,config:" # Pacman Options @@ -1694,6 +1738,7 @@ while true; do --nosign) SIGNPKG='n' ;; -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; + --pgp) PGPSIGS=1;; --pkg) shift; PKGLIST=($1) ;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; @@ -2129,6 +2174,9 @@ else download_sources if (( ! SKIPINTEG )); then check_checksums + if (( PGPSIGS )); then + check_pgpsigs + fi This check should probably match how we do it elsewhere; e.g., check_signature() the first two lines. I'd move it inside check_pgpsigs itself. Also declare it upfront where we set the rest of these.
else warning "$(gettext "Skipping integrity checks.")" fi -- 1.7.5.4
Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
First, thanks for giving this a try.
Commit descriptions are nice to have in permanent history, but all that stuff you wrote in the cover letter won't show up. Can you instead include some of that right here in the patch in commit message-style writing?
Will do (looks like I have to resubmit these anyway).
+ + msg "$(gettext "Validating source files with gpg...")" + + local file + local errors=0 + + for file in "${pgpsigs[@]}"; do + local valid + local found=1 + + file="$(get_filename "$file")" + echo -n " ${file%.sig} ... " >&2 + + if ! file="$(get_filepath "$file")"; then What are you doing here? Assignment or comparison? Do this in two statements rather than trying to be cute, and use an actual [[ -z ]] block please.
That's just a copy of http://projects.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n640 get_filepath() already checks if the file exists.
+ echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 Any need to eat stderr? If things only show up in exceptional cases, I'd rather it come through.
Oh, that's supposed to be a 1.
@@ -2129,6 +2174,9 @@ else download_sources if (( ! SKIPINTEG )); then check_checksums + if (( PGPSIGS )); then + check_pgpsigs + fi This check should probably match how we do it elsewhere; e.g., check_signature() the first two lines.
Hm, check_signature()? Either grep is lying to me or that doesn't exist anywhere.
I'd move it inside check_pgpsigs itself. Also declare it upfront where we set the rest of these.
I'm not exactly sure how I missed that, thanks! -- Wieland
On Fri, Jun 24, 2011 at 3:53 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
First, thanks for giving this a try.
Commit descriptions are nice to have in permanent history, but all that stuff you wrote in the cover letter won't show up. Can you instead include some of that right here in the patch in commit message-style writing?
Will do (looks like I have to resubmit these anyway).
+ + msg "$(gettext "Validating source files with gpg...")" + + local file + local errors=0 + + for file in "${pgpsigs[@]}"; do + local valid + local found=1 + + file="$(get_filename "$file")" + echo -n " ${file%.sig} ... " >&2 + + if ! file="$(get_filepath "$file")"; then What are you doing here? Assignment or comparison? Do this in two statements rather than trying to be cute, and use an actual [[ -z ]] block please.
That's just a copy of http://projects.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n640 get_filepath() already checks if the file exists. Aha, OK. It's fine, it just is a bit hairy in what it is doing.
+ echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 Any need to eat stderr? If things only show up in exceptional cases, I'd rather it come through.
Oh, that's supposed to be a 1.
I was suggesting no redirection at all.
This check should probably match how we do it elsewhere; e.g., check_signature() the first two lines.
Hm, check_signature()? Either grep is lying to me or that doesn't exist anywhere.
I'd move it inside check_pgpsigs itself. Also declare it upfront where we set the rest of these.
create_signature, whoops!
Hallo, Dan McGee:
+ echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 Any need to eat stderr? If things only show up in exceptional cases, I'd rather it come through.
Oh, that's supposed to be a 1.
I was suggesting no redirection at all.
That redirect is taken from pacman-key (the patch introducing it is http://mailman.archlinux.org/pipermail/pacman-dev/2010-October/011724.html). Maybe Denis (if he's still reading this list) can elaborate on why it's there.
This check should probably match how we do it elsewhere; e.g., check_signature() the first two lines.
Hm, check_signature()? Either grep is lying to me or that doesn't exist anywhere.
I'd move it inside check_pgpsigs itself. Also declare it upfront where we set the rest of these.
create_signature, whoops!
Will do. -- Wieland
Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
First, thanks for giving this a try.
Commit descriptions are nice to have in permanent history, but all that stuff you wrote in the cover letter won't show up. Can you instead include some of that right here in the patch in commit message-style writing?
--- scripts/makepkg.sh.in | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 78cd4cf..cc4f152 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in +check_pgpsigs() { + (( ! ${#source[@]} )) && return 0 + (( ! ${#pgpsigs[@]})) && return 0 + + if ! type -p gpg >/dev/null; then + error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" + exit 1 # $E_MISSING_PROGRAM + fi Please see the check_software patch (http://projects.archlinux.org/pacman.git/commit/?id=7468956236), this will need to be updated to work that way instead of how we used to do it.
Is it better to add a separate check or should I extend the existing one and its error message? -- Wieland
On Fri, Jun 24, 2011 at 4:57 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
First, thanks for giving this a try.
Commit descriptions are nice to have in permanent history, but all that stuff you wrote in the cover letter won't show up. Can you instead include some of that right here in the patch in commit message-style writing?
--- scripts/makepkg.sh.in | 52 +++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 78cd4cf..cc4f152 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in +check_pgpsigs() { + (( ! ${#source[@]} )) && return 0 + (( ! ${#pgpsigs[@]})) && return 0 + + if ! type -p gpg >/dev/null; then + error "$(gettext "Cannot find the gpg binary! Is gnupg installed?")" + exit 1 # $E_MISSING_PROGRAM + fi Please see the check_software patch (http://projects.archlinux.org/pacman.git/commit/?id=7468956236), this will need to be updated to work that way instead of how we used to do it.
Is it better to add a separate check or should I extend the existing one and its error message?
Probably just add a new block- checking it twice isn't that big of deal, and that way the error message can say "required for verifying source files" or whatever. -Dan
Hallo, Dan McGee:
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote:
+ echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --verify "$file" 2> /dev/null; then + echo "$(gettext "Verification failed")" >&2 Any need to eat stderr? If things only show up in exceptional cases, I'd rather it come through.
After looking at this more thorougly it seems like ALL output will appear on stderr [0]. I think it's a good idea to eat stderr here and instead use --status-file to save status messages in a temporary file and then grep for one of EXPSIG, EXPKEYSIG or REVKEYSIG (yes, gpg exits with status 0 even if the key that signed something has been revoked)[1] and exit immediately with an error message. Any objections? [0] http://lists.gnupg.org/pipermail/gnupg-users/2010-November/039821.html [1] http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS;h=2... -- Wieland / Mineo
--- doc/PKGBUILD.5.txt | 5 +++++ doc/makepkg.8.txt | 3 +++ 2 files changed, 8 insertions(+), 0 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 24e1a12..e41b5b1 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -108,6 +108,11 @@ It is also possible to change the name of the downloaded file, which is helpful with weird URLs and for handling multiple source files with the same name. The syntax is: `source=('filename::url')`. +*pgpsigs (array)*:: + An array containing pgp signature files. Behaves like the source array above. Please + note that for this to work, you need to manually retrieve the key a source file is + signed with from a keyserver. + *noextract (array)*:: An array of filenames corresponding to those from the source array. Files listed here will not be extracted with the rest of the source files. This diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index e61f7ab..2934865 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -173,6 +173,9 @@ Options in linkman:makepkg.conf[5]. If not specified in either location, the default key from the keyring will be used. +*\--pgp*:: + Verify pgp signatures of the source files, if there are any. + *\--noconfirm*:: (Passed to pacman) Prevent pacman from waiting for user input before proceeding with operations. -- 1.7.5.4
--- doc/PKGBUILD.5.txt | 5 +++++ doc/makepkg.8.txt | 3 +++ 2 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 24e1a12..e41b5b1 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -108,6 +108,11 @@ It is also possible to change the name of the downloaded file, which is helpful with weird URLs and for handling multiple source files with the same name. The syntax is: `source=('filename::url')`.
+*pgpsigs (array)*:: + An array containing pgp signature files. Behaves like the source array above. Please "It behaves...". subject/verb/object please. I also believe you want to capitalize PGP. + note that for this to work, you need to manually retrieve the key a source file is + signed with from a keyserver. "for this to work, your GnuPG keyring must contain the keys used in
On Thu, Jun 23, 2011 at 2:36 AM, Wieland Hoffmann <themineo@googlemail.com> wrote: Squash this all into one patch- no need for it to be separate, especially since the commit message is quite useless without context of that other commit. the signatures."
+ *noextract (array)*:: An array of filenames corresponding to those from the source array. Files listed here will not be extracted with the rest of the source files. This diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index e61f7ab..2934865 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -173,6 +173,9 @@ Options in linkman:makepkg.conf[5]. If not specified in either location, the default key from the keyring will be used.
+*\--pgp*:: + Verify pgp signatures of the source files, if there are any. + "Verify PGP signatures of the source files if provided in the package build script."
*\--noconfirm*:: (Passed to pacman) Prevent pacman from waiting for user input before proceeding with operations. -- 1.7.5.4
On 23/06/11 17:36, Wieland Hoffmann wrote:
Hi,
this adds support for verifying pgp signatures provided by upstream to makepkg. A new array pgpsigs is defined holding the URLs to all the signature files.
However, there're still a few quirks:
* You have to manually import the key which signed the source. Actually that's good, but:
* You don't know why the verification failed. It's either a wrong signature or the key is simply not known to gnupg. This is really bad, so I've chosen to make pgp verification optional for now. makepkg --pgp enables it.
I'm not going to review the actual patches yet because I think there are a few things that need discussed about how best to handle this first. Firstly I want to note that there is another patch implementing this in a slightly different way on the bug tracker: https://bugs.archlinux.org/task/20448 https://github.com/str1ngs/pacman/compare/sigs So we might be able to combine some ideas here. So, onto the implementation: 1) Do we need a separate array for signatures, or should they be just added in the source=() array? If it was in the source array, I can just use bash expansion like: source=(http://ftp.gnu.org/gnu/bash/bash-4.2.tar.gz{,.sig}) and it is fairly clear which files have signatures. It is also flexible enough to have a different source line if the signature is hosted elsewhere. As a "bonus" we would get md5sum checks on the signature file... I find a separate array that is not aligned with the source array (as in element x in the source array is not going to always be element x in the sig array) to be a bit confusing. We can detect signatures to check in the source array by extension (note comment #4 below) so I really think a separate array is overkill. 2) How much control do we need on when this checking is done? Both implementation so far have provided some way to enable/disable this checking. I think it should run by default and a --skippgpcheck (name needs work...) analog to --skipinteg is all that is needed. 3) Can we use some return values from gpg to distinguish the failure cases? Then we could give some granularity in our output - e.g. Pass, FAIL, Unknown Key, (others???). I would be fine if the "Unknown Key" case was just a warning. I would also tend to hide the gpg output here as a failure will need manually investigated by the user anyway. 4) Note many projects distribute ascii armored signatures, so the extensions that need to be detected are .sig and .asc (is that all?) Allan
On 29/06/11 09:21, Allan McRae wrote:
On 23/06/11 17:36, Wieland Hoffmann wrote:
Hi,
this adds support for verifying pgp signatures provided by upstream to makepkg. A new array pgpsigs is defined holding the URLs to all the signature files.
However, there're still a few quirks:
* You have to manually import the key which signed the source. Actually that's good, but:
* You don't know why the verification failed. It's either a wrong signature or the key is simply not known to gnupg. This is really bad, so I've chosen to make pgp verification optional for now. makepkg --pgp enables it.
I'm not going to review the actual patches yet because I think there are a few things that need discussed about how best to handle this first.
Firstly I want to note that there is another patch implementing this in a slightly different way on the bug tracker: https://bugs.archlinux.org/task/20448 https://github.com/str1ngs/pacman/compare/sigs So we might be able to combine some ideas here.
So, onto the implementation:
1) Do we need a separate array for signatures, or should they be just added in the source=() array? If it was in the source array, I can just use bash expansion like:
source=(http://ftp.gnu.org/gnu/bash/bash-4.2.tar.gz{,.sig})
and it is fairly clear which files have signatures. It is also flexible enough to have a different source line if the signature is hosted elsewhere. As a "bonus" we would get md5sum checks on the signature file...
I find a separate array that is not aligned with the source array (as in element x in the source array is not going to always be element x in the sig array) to be a bit confusing. We can detect signatures to check in the source array by extension (note comment #4 below) so I really think a separate array is overkill.
Another advantage of using the source array is that the gpg keys get included with makepkg --allsource without any modifiations to makepkg.
2) How much control do we need on when this checking is done? Both implementation so far have provided some way to enable/disable this checking. I think it should run by default and a --skippgpcheck (name needs work...) analog to --skipinteg is all that is needed.
3) Can we use some return values from gpg to distinguish the failure cases? Then we could give some granularity in our output - e.g. Pass, FAIL, Unknown Key, (others???). I would be fine if the "Unknown Key" case was just a warning. I would also tend to hide the gpg output here as a failure will need manually investigated by the user anyway.
I see in another message to this thread the mention of using --status-file and grepping the output given gpg is crap with its return codes. That seems fine, but before that is implemented we should get a list of possible values and decide what makepkg will do with them. i.e., success, error or warning for the various cases. I'd lean to more warnings than failures...
4) Note many projects distribute ascii armored signatures, so the extensions that need to be detected are .sig and .asc (is that all?)
Allan
Hallo, Allan McRae:
On 29/06/11 09:21, Allan McRae wrote:
Firstly I want to note that there is another patch implementing this in a slightly different way on the bug tracker: https://bugs.archlinux.org/task/20448 https://github.com/str1ngs/pacman/compare/sigs So we might be able to combine some ideas here.
I'm a bit short on time until next week, but I'll add a comment on Flyspray so we're not duplicating our efforts :)
So, onto the implementation:
1) Do we need a separate array for signatures, or should they be just added in the source=() array? If it was in the source array, I can just use bash expansion like:
source=(http://ftp.gnu.org/gnu/bash/bash-4.2.tar.gz{,.sig})
and it is fairly clear which files have signatures. It is also flexible enough to have a different source line if the signature is hosted elsewhere. As a "bonus" we would get md5sum checks on the signature file...
I find a separate array that is not aligned with the source array (as in element x in the source array is not going to always be element x in the sig array) to be a bit confusing. We can detect signatures to check in the source array by extension (note comment #4 below) so I really think a separate array is overkill.
It should be possible to implement this without adding a separate array.
2) How much control do we need on when this checking is done? Both implementation so far have provided some way to enable/disable this checking. I think it should run by default and a --skippgpcheck (name needs work...) analog to --skipinteg is all that is needed.
The reason for disabling this by default were that is was unclear why the verification process failed.
3) Can we use some return values from gpg to distinguish the failure cases? Then we could give some granularity in our output - e.g. Pass, FAIL, Unknown Key, (others???). I would be fine if the "Unknown Key" case was just a warning. I would also tend to hide the gpg output here as a failure will need manually investigated by the user anyway.
I see in another message to this thread the mention of using --status-file and grepping the output given gpg is crap with its return codes. That seems fine, but before that is implemented we should get a list of possible values and decide what makepkg will do with them. i.e., success, error or warning for the various cases. I'd lean to more warnings than failures...
To better illustrate this, I've uploaded my patch to github, too: https://github.com/mineo/pacman/compare/makepkg-pgp . -- Wieland
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. --- 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 INSTALL=0 NOBUILD=0 NODEPS=0 @@ -674,6 +675,63 @@ check_checksums() { fi } +check_pgpsigs() { + (( NOPGPSIGS )) && return 0 + (( ! ${#source[@]} )) && return 0 + + msg "$(gettext "Verifying source file signatures with gpg...")" + + local file + local errors=0 + local statusfile=$(mktemp) + + for file in "${source[@]}"; do + local valid + local found=1 + + file="$(get_filename "$file")" + if [[ $file =~ .*(sig|asc) ]]; then + echo -n " $file ... " >&2 + + if ! file="$(get_filepath "$file")"; then + echo "$(gettext "NOT FOUND")" >&2 + errors=1 + found=0 + fi + + if (( found )); then + if ! gpg --quiet --batch --status-file "$statusfile" --verify "$file" 2> /dev/null; then + if grep "NO_PUBKEY" "$statusfile" > /dev/null; then + echo "" >&2 + 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 + fi + fi + done + + rm -f "$statusfile" + + if (( errors )); then + error "$(gettext "One or more PGP signatures could not be verified!")" + exit 1 + fi +} + extract_sources() { msg "$(gettext "Extracting Sources...")" local netfile @@ -1478,6 +1536,14 @@ check_software() { fi fi + # gpg - source verification + if [[ ! NOPGPSIGS ]]; 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 @@ -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 @@ -2200,6 +2269,7 @@ else download_sources if (( ! SKIPINTEG )); then check_checksums + check_pgpsigs else warning "$(gettext "Skipping integrity checks.")" fi -- 1.7.6
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
On Mon, Jul 4, 2011 at 4:36 PM, Allan McRae <allan@archlinux.org> wrote:
I still wonder if --skippgpcheck is too long, but I can not think of a better name. Suggestions from anyone?
--skipinteg / --skipsig or --skippgp ?
+*\--skippgpcheck*:: + Verify PGP signatures of the source files if provided in the build script. +
Skip verification of 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
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
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. --- 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")\n" "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:")\n" "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,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 +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
With two integrity check methods available (checksums and pgp signatures) it should be possible to skip only one of them. When checksums are disabled and the public key of one source file signature is unknown, stop the build process. --- doc/makepkg.8.txt | 3 +++ scripts/makepkg.sh.in | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/makepkg.8.txt b/doc/makepkg.8.txt index bc1ffc1..b032fd4 100644 --- a/doc/makepkg.8.txt +++ b/doc/makepkg.8.txt @@ -90,6 +90,9 @@ Options *\--skippgpcheck*:: Do not verify PGP signatures of the source files. +*\--skipchecksums*:: + Do not verify source files with checksums. + *-h, \--help*:: Output syntax and command line options. diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 20ba431..b28c702 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -58,6 +58,7 @@ INFAKEROOT=0 GENINTEG=0 SKIPINTEG=0 SKIPPGPCHECK=0 +SKIPCHECKSUMS=0 INSTALL=0 NOBUILD=0 NODEPS=0 @@ -631,6 +632,7 @@ generate_checksums() { check_checksums() { (( ! ${#source[@]} )) && return 0 + (( SKIPCHECKSUMS )) && return 0 local correlation=0 local integ required @@ -719,7 +721,7 @@ check_pgpsigs() { 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 + (( SKIPCHECKSUMS )) && errors=1 || warnings=1 else echo "$(gettext "FAILED")" >&2 errors=1 @@ -1802,6 +1804,7 @@ usage() { printf "$(gettext " --sign Sign the resulting package with %s")\n" "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 " --skipchecksums Do not verify source files with checksums")" echo "$(gettext " --source Generate a source-only tarball without downloaded sources")" echo printf "$(gettext "These options can be passed to %s:")\n" "pacman" @@ -1840,7 +1843,7 @@ 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 -OPT_LONG+=",noconfirm,noprogressbar" +OPT_LONG+=",noconfirm,noprogressbar,skipchecksums" OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@" || echo 'PARSE_OPTIONS FAILED')" if [[ $OPT_TEMP = *'PARSE_OPTIONS FAILED'* ]]; then # This is a small hack to stop the script bailing with 'set -e' @@ -1879,6 +1882,7 @@ while true; do -o|--nobuild) NOBUILD=1 ;; -p) shift; BUILDFILE=$1 ;; --skippgpcheck) SKIPPGPCHECK=1;; + --skipchecksums) SKIPCHECKSUMS=1;; --pkg) shift; PKGLIST=($1) ;; -r|--rmdeps) RMDEPS=1 ;; -R|--repackage) REPKG=1 ;; -- 1.7.6
On 06/07/11 21:02, Wieland Hoffmann wrote:
With two integrity check methods available (checksums and pgp signatures) it should be possible to skip only one of them.
There were a few more places in makepkg where this needed some more attention so I took this patch and added to it. It will soon follow to the list.
When checksums are disabled and the public key of one source file signature is unknown, stop the build process.
I removed this as I do not like the behaviour of signature checking to change based on a flag for another check type. Allan
Hallo, Allan McRae:
I removed this as I do not like the behaviour of signature checking to change based on a flag for another check type.
Then it should be mentioned in the manpage that disabling the checksum checks leads to disabling integrity checks completely if the user does not have the public key that signed the sources in his keyring.
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
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")\n" "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:")\n" "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,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 +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
participants (4)
-
Allan McRae
-
Dan McGee
-
Wieland Hoffmann
-
Xavier Chantry