On 06/07/17 03:48, 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`

For reference, I'd like to see some links in the commit message to where this is discussed with git developers and with them saying that this is not an issue to be fixed in git.

Signed-off-by: Eli Schwartz ---

v2: use git's built-in format specifier to obtain the real tagname with a single command. I didn't realize in v1 that this was possible.

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 5468f977..93d88006 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 "forged tag, you have been hacked!")" >&2

Just: "the git tag has been forged" It is not necessarily the person running makepkg that has been hacked.

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

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 ---

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