[pacman-dev] [PATCH 1/2] libmakepkg/integrity: fix regression that broke invalid file sigs
In 42e7020281d3ae260e1e9693495f527b7f476625 creating the gpg statusfile for a source file was split into a separate function, which used the return code to indicate unsigned files and proto-specific errors. However, the fallback return code was set by the final gpg invocation, which would be 1 if the signature was somehow broken (for example, the key was not available in the gpg keyring). As a result makepkg thought that file did not have a signature and skipped over it rather than erroring out. Fix this by explicitly setting the return code for all verify_*_signature() functions. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- I noticed this when building a package with pacman-git on a new laptop that did not yet have "auto-key-retrieve" in gpg.conf, fun stuff. scripts/libmakepkg/integrity/verify_signature.sh.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 24519dbe..add7f75d 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -157,6 +157,7 @@ verify_file_signature() { esac $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + return 0 } verify_git_signature() { @@ -193,6 +194,7 @@ verify_git_signature() { errors=1 return 1 fi + return 0 } parse_gpg_statusfile() { -- 2.15.0
In eaa82b4d0775252856a4e54a6f2a9ea191cf0b8f source_has_signature() was modified to check if git repositories are marked as signed. However, due to a typo the unused variable $netfile was checked. This worked as long as the last source element was marked as signed, due to $netfile being mistakenly set as a global in check_vcs_software(), but usually failed with multiple sources. Break this more consistently by properly declaring $netfile as a local variable in check_vcs_software() which it should be regardless. Fix it again by completely moving over to $netfile in source_has_signature() as netfile is more descriptive of the current state. Signed-off-by: Eli Schwartz <eschwartz@archlinux.org> --- I have no idea how I could have made this mistake. I'm also somewhat disappointed that no one else spotted the typo while looking at my patch, as I added mismatched proto and query lines... But on the other hand, I uncovered a preexisting bug, so yay! scripts/libmakepkg/integrity/verify_signature.sh.in | 8 ++++---- scripts/makepkg.sh.in | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index add7f75d..640b27f6 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -256,14 +256,14 @@ parse_gpg_statusfile() { } source_has_signatures() { - local file all_sources proto + local netfile all_sources proto get_all_sources_for_arch 'all_sources' - for file in "${all_sources[@]}"; do - proto="$(get_protocol "$file")" + for netfile in "${all_sources[@]}"; do + proto="$(get_protocol "$netfile")" query=$(get_uri_query "$netfile") - if [[ ${file%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then + if [[ ${netfile%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then return 0 fi done diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in index 9a434e27..a5590e07 100644 --- a/scripts/makepkg.sh.in +++ b/scripts/makepkg.sh.in @@ -915,7 +915,7 @@ get_vcsclient() { } check_vcs_software() { - local all_sources all_deps deps ret=0 + local netfile all_sources all_deps deps ret=0 if (( SOURCEONLY == 1 )); then # we will not download VCS sources -- 2.15.0
On 22/11/17 14:34, Eli Schwartz wrote:
In 42e7020281d3ae260e1e9693495f527b7f476625 creating the gpg statusfile for a source file was split into a separate function, which used the return code to indicate unsigned files and proto-specific errors. However, the fallback return code was set by the final gpg invocation, which would be 1 if the signature was somehow broken (for example, the key was not available in the gpg keyring). As a result makepkg thought that file did not have a signature and skipped over it rather than erroring out.
Fix this by explicitly setting the return code for all verify_*_signature() functions.
OK
participants (2)
-
Allan McRae
-
Eli Schwartz