[pacman-dev] [PATCH v3] libmakepkg/integrity: check for invalid tags

[Allan McRae]

On 28/07/17 13:59, Eli Schwartz wrote:
> As per https://lists.archlinux.org/pipermail/arch-general/2017-July/043876.html
> git doesn't check that the tag name matches what an annotated tag object
> *thinks* it should be called. This is a bit of a theoretical attack and
> some would argue that we should always use commits since upstream can
> legitimately change a tag, but nevertheless this can result in a
> downgrade attack if the git download transport was manipulated or the
> upstream repository hacked.
> 
> So, check the tag blob to make sure the tag actually matches the name we
> used for `git checkout`.
> 
> This really should be fixed in git itself, rather than forcing all
> downstream users of git verify-tag to implement their own checks, but
> the git developers disagree, see the discussion surrounding
> https://public-inbox.org/git/xmqqk2hzldx8.fsf@gitster.mtv.corp.google.com/
> 
> Signed-off-by: Eli Schwartz <eschwartz at archlinux.org>
> ---
> 
> v3: Reference a discussion with the git developers. @sangy says that
> thread is where the initial decision to follow their current approach
> happened.
> 
> Use a better FAILED message.
> 
>  scripts/libmakepkg/integrity/verify_signature.sh.in | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in
> index 24519dbe..4f9f97ca 100644
> --- a/scripts/libmakepkg/integrity/verify_signature.sh.in
> +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in
> @@ -187,6 +187,13 @@ verify_git_signature() {
>  
>  	printf "    %s git repo ... " "${dir##*/}" >&2
>  
> +	tagname="$(git -C "$dir" tag -l --format='%(tag)' "$fragval")"
> +	if [[ $fragtype = tag && -n $tagname && $tagname != $fragval ]]; then
> +		printf "%s (%s)" "$(gettext "FAILED")" "$(gettext "the git tag has been forged")" >&2
> +		errors=1
> +		return 1
> +	fi
> +

Why is this in verify_signature?  As far as I can tell, it has nothing
to do with signatures and should just be down in the extract_git()
function when we are dealing with a tag.

>  	git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1
>  	if ! grep -qs NEWSIG "$statusfile"; then
>  		printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2
>