diff mbox

[pacman-dev] pacman-key: just accept one file to verify, and enforce detached sigs

Message ID 20180529170040.20398-1-eschwartz@archlinux.org
State Superseded, archived
Headers show

Commit Message

Eli Schwartz May 29, 2018, 5 p.m. UTC
Simply pass options on to gpg the same way gpg uses them -- no looping
through and checking lots of signatures.

This prevents a situation where the signature file to be verified is
manipulated to contain a complete signature which is valid, but not a
detached signature for the file you are actually trying to verify.

gpg does not offer an option to verify many files at once by naming each
signature/file pair, and there's no reason for us to do so either, since
it would be quite tiresome to do so.

Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
---
 scripts/pacman-key.sh.in | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Comments

Allan McRae June 4, 2018, 7:09 a.m. UTC | #1
On 30/05/18 03:00, Eli Schwartz wrote:
> Simply pass options on to gpg the same way gpg uses them -- no looping
> through and checking lots of signatures.
> 
> This prevents a situation where the signature file to be verified is
> manipulated to contain a complete signature which is valid, but not a
> detached signature for the file you are actually trying to verify.
> 
> gpg does not offer an option to verify many files at once by naming each
> signature/file pair, and there's no reason for us to do so either, since
> it would be quite tiresome to do so.
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
>  scripts/pacman-key.sh.in | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index 0f1630a9..0573e92f 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -486,18 +486,19 @@ refresh_keys() {
>  }
>  
>  verify_sig() {
> -	local ret=0
> -	for sig; do
> -		msg "Checking %s..." "$sig"
> -		if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
> -			error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
> -			return 1
> -		fi
> -		if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
> -			error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
> -			ret=1
> -		fi
> -	done
> +	local ret=0 sig=$1 file=$2
> +	if [[ -z $file ]]; then
> +		file=${sig%.*}
> +	fi

Opinions on if we should we do this?  All pacman's infrastructure
assumes detached signatures, but this is a difference from how gpg does
things when only one argument is given.


> +	msg "Checking %s..." "$sig"
> +	if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
> +		error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
> +		exit 1
> +	fi
> +	if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" "$file" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
> +		error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
> +		ret=1
> +	fi
>  	exit $ret
>  }
>  
>
Allan McRae Oct. 21, 2018, 11:22 a.m. UTC | #2
On 30/5/18 3:00 am, Eli Schwartz wrote:
> Simply pass options on to gpg the same way gpg uses them -- no looping
> through and checking lots of signatures.
> 
> This prevents a situation where the signature file to be verified is
> manipulated to contain a complete signature which is valid, but not a
> detached signature for the file you are actually trying to verify.
> 
> gpg does not offer an option to verify many files at once by naming each
> signature/file pair, and there's no reason for us to do so either, since
> it would be quite tiresome to do so.
> 
> Signed-off-by: Eli Schwartz <eschwartz@archlinux.org>
> ---
>  scripts/pacman-key.sh.in | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index 0f1630a9..0573e92f 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -486,18 +486,19 @@ refresh_keys() {
>  }
>  
>  verify_sig() {
> -	local ret=0
> -	for sig; do
> -		msg "Checking %s..." "$sig"
> -		if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
> -			error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
> -			return 1
> -		fi
> -		if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
> -			error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
> -			ret=1
> -		fi
> -	done
> +	local ret=0 sig=$1 file=$2
> +	if [[ -z $file ]]; then
> +		file=${sig%.*}
> +	fi

Only do this if $file exists.  Otherwise we can assume it is an embedded
signature.

> +	msg "Checking %s..." "$sig"

Can we add a (detached) at the end here if $file exists?

Also, docs will need updated.

> +	if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
> +		error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
> +		exit 1
> +	fi
> +	if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" "$file" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
> +		error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
> +		ret=1
> +	fi
>  	exit $ret
>  }
>  
>
diff mbox

Patch

diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
index 0f1630a9..0573e92f 100644
--- a/scripts/pacman-key.sh.in
+++ b/scripts/pacman-key.sh.in
@@ -486,18 +486,19 @@  refresh_keys() {
 }
 
 verify_sig() {
-	local ret=0
-	for sig; do
-		msg "Checking %s..." "$sig"
-		if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
-			error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
-			return 1
-		fi
-		if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
-			error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
-			ret=1
-		fi
-	done
+	local ret=0 sig=$1 file=$2
+	if [[ -z $file ]]; then
+		file=${sig%.*}
+	fi
+	msg "Checking %s..." "$sig"
+	if grep -q 'BEGIN PGP SIGNATURE' "$sig"; then
+		error "$(gettext "Cannot use armored signatures for packages: %s")" "$sig"
+		exit 1
+	fi
+	if ! "${GPG_PACMAN[@]}" --status-fd 1 --verify "$sig" "$file" | grep -qE '^\[GNUPG:\] TRUST_(FULLY|ULTIMATE).*$'; then
+		error "$(gettext "The signature identified by %s could not be verified.")" "$sig"
+		ret=1
+	fi
 	exit $ret
 }