[pacman-dev] makepkg: verify git sources

older
[pacman-dev] [PATCH 1/5] makepkg:...

Jelle van der Waa

7 Dec 2016 7 Dec '16

9:48 p.m.

Hi, I would like to work on verifying git signed tags. So far I've actually managed to hack it into makepkg, but by missing a lot of edge cases and well writing an ugly implementation. [1] Some of this work has been borrowed from the systemd PKGBUILD. [2] Example with vte3-ng: ==> Validating source files with md5sums... vte-ng ... Skipped ==> Verifying source file signatures with gpg... vte-ng ... Passed Of course the hacks I've put in there are never going to be accepted even if I bribe Allan with a case of bourbon. I can think of the following issues, edge-cases which need to be handled: * git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case. * Not parsing or tested invalid signed tags, not sure how git verify-tag displays errors so that needs more work. * I would like to move the git verification into source/git.sh.in and then re-use the code which extracts #branch, #commit etc. It would also reduce the clutter in verify_signature.sh.in. Another idea is to move the verification into integrity/verify_git.sh.in. * Changing the directory is cumbersome. git offers git -C $path verify-tag $tag to resolve that. * Multiple sources, .tar.gz{,asc} and a git one. (Rare but should be handled) Or multiple git sources. So tl;dr, I would love to see a pointer where I should call my own verification function for git sources or any other edge-cases I've missed :) [1] https://github.com/jelly/pacman/commit/5172a74ed9de422429d18034841acf8025fd3... [2] https://git.archlinux.org/svntogit/packages.git/tree/trunk/PKGBUILD?h=packag... -- Jelle van der Waa

Attachments:

signature.asc (application/pgp-signature — 488 bytes)

Show replies by date

Eli Schwartz

8 Dec 8 Dec

3 a.m.

On 12/07/2016 03:48 PM, Jelle van der Waa wrote:

...

I would like to work on verifying git signed tags. So far I've actually managed to hack it into makepkg, but by missing a lot of edge cases and well writing an ugly implementation. [1] Some of this work has been borrowed from the systemd PKGBUILD. [2]

This is something I want to see also.

...

Of course the hacks I've put in there are never going to be accepted even if I bribe Allan with a case of bourbon.

I can think of the following issues, edge-cases which need to be handled:

* git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case.

I imagine that should be handled just like #commit= using verify-commit HEAD, why does it need to be special-cased?

...

* Not parsing or tested invalid signed tags, not sure how git verify-tag displays errors so that needs more work.

Non-signed tags return an "error: no signature found", non-signed commits just return an error.

...

* I would like to move the git verification into source/git.sh.in and then re-use the code which extracts #branch, #commit etc. It would also reduce the clutter in verify_signature.sh.in. Another idea is to move the verification into integrity/verify_git.sh.in.

Or extract the logic into a new function and reuse it in integrity/

...

* Changing the directory is cumbersome. git offers git -C $path verify-tag $tag to resolve that. * Multiple sources, .tar.gz{,asc} and a git one. (Rare but should be handled) Or multiple git sources.

Or put another way, how should a PKGBUILD declare that git GPG verification is demanded, for that particular source?

...

So tl;dr, I would love to see a pointer where I should call my own verification function for git sources or any other edge-cases I've missed :)

Well, aside from the confusing tab --> spaces thing... (It trips me up too, I keep having to remember to ":set noexpandtab") I have something similar-ish, but probably a lot uglier :p here: https://github.com/eli-schwartz/pacman/commit/edde351d919a5baf8c31764c5cfe9e... -- Eli Schwartz

Jelle van der Waa

9:14 a.m.

On 12/07/16 at 09:00pm, Eli Schwartz wrote:

...

On 12/07/2016 03:48 PM, Jelle van der Waa wrote:

...
* git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case.

I imagine that should be handled just like #commit= using verify-commit HEAD, why does it need to be special-cased?

Well with #commit you specify a certain commit, so I would say you want to verify that commit.

...

...
* Not parsing or tested invalid signed tags, not sure how git verify-tag displays errors so that needs more work.

Non-signed tags return an "error: no signature found", non-signed commits just return an error.

Yup, but what about other LOCALE's? Guess it needs a LOCALE=C git..

...

...
* I would like to move the git verification into source/git.sh.in and then re-use the code which extracts #branch, #commit etc. It would also reduce the clutter in verify_signature.sh.in. Another idea is to move the verification into integrity/verify_git.sh.in.

Or extract the logic into a new function and reuse it in integrity/

...
* Changing the directory is cumbersome. git offers git -C $path verify-tag $tag to resolve that. * Multiple sources, .tar.gz{,asc} and a git one. (Rare but should be handled) Or multiple git sources.

Or put another way, how should a PKGBUILD declare that git GPG verification is demanded, for that particular source?

I'd say if it has validpgpkeys=('234234') we verify the git tag. Which would require extracting the VALIDGSIG 23423 from git verify-tag --raw v12.

...

I have something similar-ish, but probably a lot uglier :p here: https://github.com/eli-schwartz/pacman/commit/edde351d919a5baf8c31764c5cfe9e...

Hmm looks less ugly somehow though ;-) -- Jelle van der Waa

Eli Schwartz

1:56 p.m.

On 12/08/2016 03:14 AM, Jelle van der Waa wrote:

...

On 12/07/16 at 09:00pm, Eli Schwartz wrote:

...
On 12/07/2016 03:48 PM, Jelle van der Waa wrote:

...
* git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case.

I imagine that should be handled just like #commit= using verify-commit HEAD, why does it need to be special-cased?

Well with #commit you specify a certain commit, so I would say you want to verify that commit.

Huhhhh... right. We're checking the bare source repo, not the copy in $srcdir which is checked out to the correct $commit. Too true. :o

...

...
Or put another way, how should a PKGBUILD declare that git GPG verification is demanded, for that particular source?

I'd say if it has validpgpkeys=('234234') we verify the git tag. Which would require extracting the VALIDGSIG 23423 from git verify-tag --raw v12.

What happens when you have validpgpkeys and want to check a file but the repository is not signed? What happens when you have two repositories and only one is signed?

...

...
I have something similar-ish, but probably a lot uglier :p here: https://github.com/eli-schwartz/pacman/commit/edde351d919a5baf8c31764c5cfe9e...

Hmm looks less ugly somehow though ;-)

Well, the more you do the uglier it looks... Anyway, that patch suffers from critical existence failure, see the next commit: https://github.com/eli-schwartz/pacman/tree/makepkg-git-verification -- Eli Schwartz

Jelle van der Waa

3:28 p.m.

On 12/08/16 at 07:56am, Eli Schwartz wrote:

...

On 12/08/2016 03:14 AM, Jelle van der Waa wrote:

...
On 12/07/16 at 09:00pm, Eli Schwartz wrote:

...
On 12/07/2016 03:48 PM, Jelle van der Waa wrote:

...
* git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case.

I imagine that should be handled just like #commit= using verify-commit HEAD, why does it need to be special-cased?

Well with #commit you specify a certain commit, so I would say you want to verify that commit.

Huhhhh... right. We're checking the bare source repo, not the copy in $srcdir which is checked out to the correct $commit. Too true. :o

...
...
Or put another way, how should a PKGBUILD declare that git GPG verification is demanded, for that particular source?

I'd say if it has validpgpkeys=('234234') we verify the git tag. Which would require extracting the VALIDGSIG 23423 from git verify-tag --raw v12.

What happens when you have validpgpkeys and want to check a file but the repository is not signed? What happens when you have two repositories and only one is signed?

Yes that's tricky, and exactly why I wanted to start a discussion here :) -- Jelle van der Waa

Eli Schwartz

6:26 p.m.

On 12/08/2016 09:28 AM, Jelle van der Waa wrote:

...

On 12/08/16 at 07:56am, Eli Schwartz wrote:

...
What happens when you have validpgpkeys and want to check a file but the repository is not signed? What happens when you have two repositories and only one is signed?

Yes that's tricky, and exactly why I wanted to start a discussion here :)

So currently everything works correctly, absent this^^. Checked with signed tags, branches, commits. Actually, there is no validpgpkeys check, since file signatures still fail when pgpsigs are checked but no validpgpkeys are declared (so why change that if we don't have to), but other than that... Way too many if statements, hopefully that can be reworked somehow, but the important thing is it doesn't fall over. Possibilities for switching on repository signature checking: - Add something to the url fragment to indicate it is signed. - All-or-nothing (distasteful) check on ``` (( ${#validpgpkeys[@]} > 0 )) ``` - Warn and continue (distasteful)? We should hope for something better than second-class citizenship. Anyone else have ideas on this? -- Eli Schwartz

Eli Schwartz

15 Dec 15 Dec

8:44 a.m.

After discussion on IRC, I have implemented checking for git source signatures via URI queries[1] so this is finally done. Test the changes at https://github.com/eli-schwartz/pacman/tree/makepkg-git-verification (or just look at it to see what on earth was going through my mind throughout). In the morning or something, I will try to clean it up a bit, then squash it and post a patch here. Side note: dealing with the tab formatting got a lot easier and saved me a lot of frustration when I wrote a .editorconfig and installed the associated vim plugin. I think I now love that project! -- Eli Schwartz [1] https://tools.ietf.org/html/rfc3986#section-3.4

Eli Schwartz

16 Dec 16 Dec

7:56 p.m.

So, I think this is done now. I had to fix a few other embarrassing mistakes (see https://github.com/eli-schwartz/pacman/tree/makepkg-git-verify ) but everything should work now. I've tested it on git and http sources, with and without signatures, and it consistently does the right thing (at last). I decided to check for the existence of git signed objects later rather than earlier. It is possible for e.g. a (weird) commit message to trigger a match with grep, so better to be explicit even if the flow is a bit odd. Too bad git doesn't have a way to dump a signature directly. -- Eli Schwartz

Eli Schwartz

7:56 p.m.

New subject: [pacman-dev] [PATCH] makepkg: Verify git signatures

A git repository is marked as signed if it contains the query "signed" as defined by https://tools.ietf.org/html/rfc3986 Adds two utility functions in util/source.sh.in to extract fragments and queries, and modifies source/git.sh.in to use them. Signed-off-by: Eli Schwartz --- .../libmakepkg/integrity/verify_signature.sh.in | 56 ++++++++++++++++++---- scripts/libmakepkg/source/git.sh.in | 11 ++--- scripts/libmakepkg/util/source.sh.in | 27 +++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6df62727..634958f9 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,11 +32,12 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local file ext decompress found pubkey success status fingerprint trusted + local netfile file ext decompress found pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) local all_sources + local proto dir fragment query fragtype fragval case $1 in all) @@ -46,15 +47,38 @@ check_pgpsigs() { get_all_sources_for_arch 'all_sources' ;; esac - for file in "${all_sources[@]}"; do - file="$(get_filename "$file")" - if [[ $file != *.@(sig?(n)|asc) ]]; then + for netfile in "${all_sources[@]}"; do + file="$(get_filename "$netfile")" + proto="$(get_protocol "$netfile")" + dir=$(get_filepath "$netfile") + fragment=$(get_uri_fragment "$netfile") + query=$(get_uri_query "$netfile") + + if [[ $proto = git* && $query = signed ]]; then + case ${fragment%%=*} in + tag) + fragtype=tag + fragval=${fragment##*=} + ;; + commit|branch) + fragtype=commit + fragval=${fragment##*=} + ;; + '') + fragtype=commit + fragval=HEAD + esac + elif [[ $file != *.@(sig?(n)|asc) ]]; then continue fi - printf " %s ... " "${file%.*}" >&2 + if [[ $proto = git* ]]; then + printf " %s git repo ... " "${file%.*}" >&2 + else + printf " %s ... " "${file%.*}" >&2 + fi - if ! file="$(get_filepath "$file")"; then + if ! file="$(get_filepath "$netfile")"; then printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 errors=1 continue @@ -67,7 +91,7 @@ check_pgpsigs() { break; fi done - if (( ! found )); then + if [[ $proto != git* ]] && (( ! found )); then printf '%s\n' "$(gettext "SOURCE FILE NOT FOUND")" >&2 errors=1 continue @@ -83,7 +107,16 @@ check_pgpsigs() { "") decompress="cat" ;; esac - $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + if [[ $proto = git* ]]; then + git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1 + if ! grep -qs NEWSIG "$statusfile"; then + printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 + errors=1 + continue + fi + else + $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + fi # these variables are assigned values in parse_gpg_statusfile success=0 status= @@ -204,11 +237,14 @@ parse_gpg_statusfile() { } source_has_signatures() { - local file all_sources + local file all_sources proto get_all_sources_for_arch 'all_sources' for file in "${all_sources[@]}"; do - if [[ ${file%%::*} = *.@(sig?(n)|asc) ]]; then + proto="$(get_protocol "$file")" + query=$(get_uri_query "$netfile") + + if [[ ${file%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then return 0 fi done diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in index cc27663d..3d370dbd 100644 --- a/scripts/libmakepkg/source/git.sh.in +++ b/scripts/libmakepkg/source/git.sh.in @@ -39,6 +39,7 @@ download_git() { local url=$(get_url "$netfile") url=${url#git+} url=${url%%#*} + url=${url%%\?*} if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" @@ -66,14 +67,8 @@ download_git() { extract_git() { local netfile=$1 - local fragment=${netfile#*#} - if [[ $fragment = "$netfile" ]]; then - unset fragment - fi - - local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local fragment=$(get_uri_fragment "$netfile") + local repo=$(get_filename "$netfile") local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" diff --git a/scripts/libmakepkg/util/source.sh.in b/scripts/libmakepkg/util/source.sh.in index 9d4ba4a6..890e92ba 100644 --- a/scripts/libmakepkg/util/source.sh.in +++ b/scripts/libmakepkg/util/source.sh.in @@ -65,6 +65,7 @@ get_filename() { case $proto in bzr*|git*|hg*|svn*) filename=${netfile%%#*} + filename=${netfile%%\?*} filename=${filename%/} filename=${filename##*/} if [[ $proto = bzr* ]]; then @@ -111,6 +112,32 @@ get_filepath() { printf "%s\n" "$file" } +# extract the VCS revision/branch specifier from a source entry +get_uri_fragment() { + local netfile=$1 + + local fragment=${netfile#*#} + if [[ $fragment = "$netfile" ]]; then + unset fragment + fi + fragment=${fragment%\?*} + + printf "%s\n" "$fragment" +} + +# extract the "signed" status of a source entry - defined as a trailing URI data component +get_uri_query() { + local netfile=$1 + + local query=${netfile#*\?} + if [[ $query = "$netfile" ]]; then + unset query + fi + query=${query%#*} + + printf "%s\n" "$query" +} + get_downloadclient() { local proto=$1 -- 2.11.0

Eli Schwartz

19 Dec 19 Dec

4:23 a.m.

New subject: [pacman-dev] [PATCH v2] makepkg: Verify git signatures

A git repository is marked as signed if it contains the query "signed" as defined by https://tools.ietf.org/html/rfc3986 Adds two utility functions in util/source.sh.in to extract fragments and queries, and modifies source/git.sh.in to use them. Signed-off-by: Eli Schwartz --- Stupid copy-paste-of-the-previous-line typo. In get_filename() it would be much much much more helpful if I didn't *reset* the filename back to "${netfile%%\?*}". That's all for now... .../libmakepkg/integrity/verify_signature.sh.in | 56 ++++++++++++++++++---- scripts/libmakepkg/source/git.sh.in | 11 ++--- scripts/libmakepkg/util/source.sh.in | 27 +++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6df62727..634958f9 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,11 +32,12 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local file ext decompress found pubkey success status fingerprint trusted + local netfile file ext decompress found pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) local all_sources + local proto dir fragment query fragtype fragval case $1 in all) @@ -46,15 +47,38 @@ check_pgpsigs() { get_all_sources_for_arch 'all_sources' ;; esac - for file in "${all_sources[@]}"; do - file="$(get_filename "$file")" - if [[ $file != *.@(sig?(n)|asc) ]]; then + for netfile in "${all_sources[@]}"; do + file="$(get_filename "$netfile")" + proto="$(get_protocol "$netfile")" + dir=$(get_filepath "$netfile") + fragment=$(get_uri_fragment "$netfile") + query=$(get_uri_query "$netfile") + + if [[ $proto = git* && $query = signed ]]; then + case ${fragment%%=*} in + tag) + fragtype=tag + fragval=${fragment##*=} + ;; + commit|branch) + fragtype=commit + fragval=${fragment##*=} + ;; + '') + fragtype=commit + fragval=HEAD + esac + elif [[ $file != *.@(sig?(n)|asc) ]]; then continue fi - printf " %s ... " "${file%.*}" >&2 + if [[ $proto = git* ]]; then + printf " %s git repo ... " "${file%.*}" >&2 + else + printf " %s ... " "${file%.*}" >&2 + fi - if ! file="$(get_filepath "$file")"; then + if ! file="$(get_filepath "$netfile")"; then printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 errors=1 continue @@ -67,7 +91,7 @@ check_pgpsigs() { break; fi done - if (( ! found )); then + if [[ $proto != git* ]] && (( ! found )); then printf '%s\n' "$(gettext "SOURCE FILE NOT FOUND")" >&2 errors=1 continue @@ -83,7 +107,16 @@ check_pgpsigs() { "") decompress="cat" ;; esac - $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + if [[ $proto = git* ]]; then + git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1 + if ! grep -qs NEWSIG "$statusfile"; then + printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 + errors=1 + continue + fi + else + $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + fi # these variables are assigned values in parse_gpg_statusfile success=0 status= @@ -204,11 +237,14 @@ parse_gpg_statusfile() { } source_has_signatures() { - local file all_sources + local file all_sources proto get_all_sources_for_arch 'all_sources' for file in "${all_sources[@]}"; do - if [[ ${file%%::*} = *.@(sig?(n)|asc) ]]; then + proto="$(get_protocol "$file")" + query=$(get_uri_query "$netfile") + + if [[ ${file%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then return 0 fi done diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in index cc27663d..3d370dbd 100644 --- a/scripts/libmakepkg/source/git.sh.in +++ b/scripts/libmakepkg/source/git.sh.in @@ -39,6 +39,7 @@ download_git() { local url=$(get_url "$netfile") url=${url#git+} url=${url%%#*} + url=${url%%\?*} if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" @@ -66,14 +67,8 @@ download_git() { extract_git() { local netfile=$1 - local fragment=${netfile#*#} - if [[ $fragment = "$netfile" ]]; then - unset fragment - fi - - local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local fragment=$(get_uri_fragment "$netfile") + local repo=$(get_filename "$netfile") local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" diff --git a/scripts/libmakepkg/util/source.sh.in b/scripts/libmakepkg/util/source.sh.in index 9d4ba4a6..485ebe3a 100644 --- a/scripts/libmakepkg/util/source.sh.in +++ b/scripts/libmakepkg/util/source.sh.in @@ -65,6 +65,7 @@ get_filename() { case $proto in bzr*|git*|hg*|svn*) filename=${netfile%%#*} + filename=${filename%%\?*} filename=${filename%/} filename=${filename##*/} if [[ $proto = bzr* ]]; then @@ -111,6 +112,32 @@ get_filepath() { printf "%s\n" "$file" } +# extract the VCS revision/branch specifier from a source entry +get_uri_fragment() { + local netfile=$1 + + local fragment=${netfile#*#} + if [[ $fragment = "$netfile" ]]; then + unset fragment + fi + fragment=${fragment%\?*} + + printf "%s\n" "$fragment" +} + +# extract the "signed" status of a source entry - defined as a trailing URI data component +get_uri_query() { + local netfile=$1 + + local query=${netfile#*\?} + if [[ $query = "$netfile" ]]; then + unset query + fi + query=${query%#*} + + printf "%s\n" "$query" +} + get_downloadclient() { local proto=$1 -- 2.11.0

Allan McRae

3 Jan 3 Jan

6:22 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Verify git signatures

On 17/12/16 04:56, Eli Schwartz wrote:

...

A git repository is marked as signed if it contains the query "signed" as defined by https://tools.ietf.org/html/rfc3986

Adds two utility functions in util/source.sh.in to extract fragments and queries, and modifies source/git.sh.in to use them.

Needs documentation added. e.g. can the query string occur anywhere relative to the fragment?

...

Signed-off-by: Eli Schwartz --- .../libmakepkg/integrity/verify_signature.sh.in | 56 ++++++++++++++++++---- scripts/libmakepkg/source/git.sh.in | 11 ++--- scripts/libmakepkg/util/source.sh.in | 27 +++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6df62727..634958f9 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,11 +32,12 @@ check_pgpsigs() {

msg "$(gettext "Verifying source file signatures with %s...")" "gpg"

- local file ext decompress found pubkey success status fingerprint trusted + local netfile file ext decompress found pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) local all_sources + local proto dir fragment query fragtype fragval

case $1 in all) @@ -46,15 +47,38 @@ check_pgpsigs() { get_all_sources_for_arch 'all_sources' ;; esac - for file in "${all_sources[@]}"; do - file="$(get_filename "$file")" - if [[ $file != *.@(sig?(n)|asc) ]]; then + for netfile in "${all_sources[@]}"; do + file="$(get_filename "$netfile")" + proto="$(get_protocol "$netfile")" + dir=$(get_filepath "$netfile") + fragment=$(get_uri_fragment "$netfile") + query=$(get_uri_query "$netfile") + + if [[ $proto = git* && $query = signed ]]; then + case ${fragment%%=*} in + tag) + fragtype=tag + fragval=${fragment##*=} + ;; + commit|branch) + fragtype=commit + fragval=${fragment##*=} + ;; + '') + fragtype=commit + fragval=HEAD + esac

I'm guessing other modern VCS tools can have signatures verified too? This function will become a mess when they are included. Please split out git and standard file verification to their own functions called within this one.

...

+ elif [[ $file != *.@(sig?(n)|asc) ]]; then continue fi

- printf " %s ... " "${file%.*}" >&2 + if [[ $proto = git* ]]; then + printf " %s git repo ... " "${file%.*}" >&2 + else + printf " %s ... " "${file%.*}" >&2 + fi

- if ! file="$(get_filepath "$file")"; then + if ! file="$(get_filepath "$netfile")"; then printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 errors=1 continue @@ -67,7 +91,7 @@ check_pgpsigs() { break; fi done - if (( ! found )); then + if [[ $proto != git* ]] && (( ! found )); then printf '%s\n' "$(gettext "SOURCE FILE NOT FOUND")" >&2 errors=1 continue @@ -83,7 +107,16 @@ check_pgpsigs() { "") decompress="cat" ;; esac

- $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + if [[ $proto = git* ]]; then + git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1 + if ! grep -qs NEWSIG "$statusfile"; then + printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 + errors=1 + continue + fi + else + $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null + fi # these variables are assigned values in parse_gpg_statusfile success=0 status= @@ -204,11 +237,14 @@ parse_gpg_statusfile() { }

source_has_signatures() { - local file all_sources + local file all_sources proto

get_all_sources_for_arch 'all_sources' for file in "${all_sources[@]}"; do - if [[ ${file%%::*} = *.@(sig?(n)|asc) ]]; then + proto="$(get_protocol "$file")" + query=$(get_uri_query "$netfile") + + if [[ ${file%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then return 0 fi done diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in index cc27663d..3d370dbd 100644 --- a/scripts/libmakepkg/source/git.sh.in +++ b/scripts/libmakepkg/source/git.sh.in @@ -39,6 +39,7 @@ download_git() { local url=$(get_url "$netfile") url=${url#git+} url=${url%%#*} + url=${url%%\?*}

if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" @@ -66,14 +67,8 @@ download_git() { extract_git() { local netfile=$1

- local fragment=${netfile#*#} - if [[ $fragment = "$netfile" ]]; then - unset fragment - fi - - local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local fragment=$(get_uri_fragment "$netfile") + local repo=$(get_filename "$netfile")

local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" diff --git a/scripts/libmakepkg/util/source.sh.in b/scripts/libmakepkg/util/source.sh.in index 9d4ba4a6..890e92ba 100644 --- a/scripts/libmakepkg/util/source.sh.in +++ b/scripts/libmakepkg/util/source.sh.in @@ -65,6 +65,7 @@ get_filename() { case $proto in bzr*|git*|hg*|svn*) filename=${netfile%%#*} + filename=${netfile%%\?*} filename=${filename%/} filename=${filename##*/} if [[ $proto = bzr* ]]; then @@ -111,6 +112,32 @@ get_filepath() { printf "%s\n" "$file" }

+# extract the VCS revision/branch specifier from a source entry +get_uri_fragment() { + local netfile=$1 + + local fragment=${netfile#*#} + if [[ $fragment = "$netfile" ]]; then + unset fragment + fi + fragment=${fragment%\?*} + + printf "%s\n" "$fragment" +} + +# extract the "signed" status of a source entry - defined as a trailing URI data component +get_uri_query() { + local netfile=$1 + + local query=${netfile#*\?} + if [[ $query = "$netfile" ]]; then + unset query + fi + query=${query%#*} + + printf "%s\n" "$query" +} + get_downloadclient() { local proto=$1

Eli Schwartz

8:41 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Verify git signatures

On 01/03/2017 12:22 AM, Allan McRae wrote:

...

Needs documentation added. e.g. can the query string occur anywhere relative to the fragment?

I wrote it so it should work either way, #fragment?query or ?query#fragment -- should we prefer one over the other? For documentation: just add a new paragraph in PKGBUILD.5 under "USING VCS SOURCES" (and tweak the wording to fit)?

...

I'm guessing other modern VCS tools can have signatures verified too?

I would be pleasantly surprised if that were true. AFAIK only git and mercurial can really be considered a "modern VCS", and it seems mercurial can only do this via an optional thirdparty plugin (commitsigs) or separately track a file containing signed hashes -- one extra commit per signature -- via an optional builtin plugin (gpg). Either one requires, in true mercurial fashion, explicitly enabling via .hgrc. And using the non-thirdparty plugin is apparently recommended against for what I imagine are obvious reasons.

...

This function will become a mess when they are included. Please split out git and standard file verification to their own functions called within this one.

When? Or if? Would it even be reasonable to try implementing Mercurial signature verification? If not, does it still make sense to split out the git verification from file verification? Anyway, that case statement is VCS-agnostic, except for the check to make sure we are using a (supported) VCS, and the fallthrough. Although maybe the fallthrough should be handled when expanding the variable later on? I'll look at splitting each sourcetype into functions to generate the statusfile though, since there is already a bit of unpleasantly convoluted logic there. -- Eli Schwartz

Jelle van der Waa

8:51 a.m.

New subject: [pacman-dev] [PATCH] makepkg: Verify git signatures

On 01/03/17 at 03:22pm, Allan McRae wrote:

...

On 17/12/16 04:56, Eli Schwartz wrote:

...
A git repository is marked as signed if it contains the query "signed" as defined by https://tools.ietf.org/html/rfc3986

Adds two utility functions in util/source.sh.in to extract fragments and queries, and modifies source/git.sh.in to use them.

Needs documentation added. e.g. can the query string occur anywhere relative to the fragment?

...
Signed-off-by: Eli Schwartz --- .../libmakepkg/integrity/verify_signature.sh.in | 56 ++++++++++++++++++---- scripts/libmakepkg/source/git.sh.in | 11 ++--- scripts/libmakepkg/util/source.sh.in | 27 +++++++++++ 3 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6df62727..634958f9 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,11 +32,12 @@ check_pgpsigs() {

msg "$(gettext "Verifying source file signatures with %s...")" "gpg"

- local file ext decompress found pubkey success status fingerprint trusted + local netfile file ext decompress found pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) local all_sources + local proto dir fragment query fragtype fragval

case $1 in all) @@ -46,15 +47,38 @@ check_pgpsigs() { get_all_sources_for_arch 'all_sources' ;; esac - for file in "${all_sources[@]}"; do - file="$(get_filename "$file")" - if [[ $file != *.@(sig?(n)|asc) ]]; then + for netfile in "${all_sources[@]}"; do + file="$(get_filename "$netfile")" + proto="$(get_protocol "$netfile")" + dir=$(get_filepath "$netfile") + fragment=$(get_uri_fragment "$netfile") + query=$(get_uri_query "$netfile") + + if [[ $proto = git* && $query = signed ]]; then + case ${fragment%%=*} in + tag) + fragtype=tag + fragval=${fragment##*=} + ;; + commit|branch) + fragtype=commit + fragval=${fragment##*=} + ;; + '') + fragtype=commit + fragval=HEAD + esac

I'm guessing other modern VCS tools can have signatures verified too? This function will become a mess when they are included. Please split out git and standard file verification to their own functions called within this one.

It seems that SVN does not support signing commits, CVS has no support for it either and mercurial has an extension for it. [1] [1] https://www.mercurial-scm.org/wiki/GpgExtension -- Jelle van der Waa

Eli Schwartz

9:10 p.m.

New subject: [pacman-dev] [PATCH] makepkg: Verify git signatures

Added documentation, if anyone has suggestions for wording it better I am all ears ;) -- I make no promises. Factoring out the per-proto signature-checking-and-creation-of-statusfiles really did get it to look a lot nicer, even if we never add any other VCS types. :)

Eli Schwartz

9:10 p.m.

New subject: [pacman-dev] [PATCH v3 1/3] libmakepkg/integrity: Verify file signatures in a separate function

This makes it easier to add signature verification for new protos. Signed-off-by: Eli Schwartz --- .../libmakepkg/integrity/verify_signature.sh.in | 84 ++++++++++++---------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6df62727..6ffc6df4 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,7 +32,7 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local file ext decompress found pubkey success status fingerprint trusted + local netfile pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) @@ -46,44 +46,9 @@ check_pgpsigs() { get_all_sources_for_arch 'all_sources' ;; esac - for file in "${all_sources[@]}"; do - file="$(get_filename "$file")" - if [[ $file != *.@(sig?(n)|asc) ]]; then - continue - fi + for netfile in "${all_sources[@]}"; do + verify_file_signature "$netfile" "$statusfile" || continue - printf " %s ... " "${file%.*}" >&2 - - if ! file="$(get_filepath "$file")"; then - printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 - errors=1 - continue - fi - - found=0 - for ext in "" gz bz2 xz lrz lzo Z; do - if sourcefile="$(get_filepath "${file%.*}${ext:+.$ext}")"; then - found=1 - break; - fi - done - if (( ! found )); then - printf '%s\n' "$(gettext "SOURCE FILE NOT FOUND")" >&2 - errors=1 - continue - fi - - case "$ext" in - gz) decompress="gzip -c -d -f" ;; - bz2) decompress="bzip2 -c -d -f" ;; - xz) decompress="xz -c -d" ;; - lrz) decompress="lrzip -q -d" ;; - lzo) decompress="lzop -c -d -q" ;; - Z) decompress="uncompress -c -f" ;; - "") decompress="cat" ;; - esac - - $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null # these variables are assigned values in parse_gpg_statusfile success=0 status= @@ -145,6 +110,49 @@ check_pgpsigs() { fi } +verify_file_signature() { + local netfile="$1" statusfile="$2" + local file ext decompress found sourcefile + + file="$(get_filename "$netfile")" + if [[ $file != *.@(sig?(n)|asc) ]]; then + return 1 + fi + + printf " %s ... " "${file%.*}" >&2 + + if ! file="$(get_filepath "$netfile")"; then + printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 + errors=1 + return 1 + fi + + found=0 + for ext in "" gz bz2 xz lrz lzo Z; do + if sourcefile="$(get_filepath "${file%.*}${ext:+.$ext}")"; then + found=1 + break; + fi + done + if (( ! found )); then + printf '%s\n' "$(gettext "SOURCE FILE NOT FOUND")" >&2 + errors=1 + return 1 + fi + + case "$ext" in + gz) decompress="gzip -c -d -f" ;; + bz2) decompress="bzip2 -c -d -f" ;; + xz) decompress="xz -c -d" ;; + lrz) decompress="lrzip -q -d" ;; + lzo) decompress="lzop -c -d -q" ;; + Z) decompress="uncompress -c -f" ;; + "") decompress="cat" ;; + esac + + $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null +} + parse_gpg_statusfile() { local type arg1 arg6 arg10 -- 2.11.0

Eli Schwartz

9:10 p.m.

New subject: [pacman-dev] [PATCH v3 2/3] makepkg: Verify git signatures

A git repository is marked as signed if it contains the query "signed" as defined by https://tools.ietf.org/html/rfc3986 Adds two utility functions in util/source.sh.in to extract fragments and queries, and modifies source/git.sh.in to use them. Signed-off-by: Eli Schwartz --- .../libmakepkg/integrity/verify_signature.sh.in | 53 ++++++++++++++++++++-- scripts/libmakepkg/source/git.sh.in | 11 ++--- scripts/libmakepkg/util/source.sh.in | 27 +++++++++++ 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/scripts/libmakepkg/integrity/verify_signature.sh.in b/scripts/libmakepkg/integrity/verify_signature.sh.in index 6ffc6df4..7cce58ee 100644 --- a/scripts/libmakepkg/integrity/verify_signature.sh.in +++ b/scripts/libmakepkg/integrity/verify_signature.sh.in @@ -32,7 +32,7 @@ check_pgpsigs() { msg "$(gettext "Verifying source file signatures with %s...")" "gpg" - local netfile pubkey success status fingerprint trusted + local netfile proto pubkey success status fingerprint trusted local warning=0 local errors=0 local statusfile=$(mktemp) @@ -47,7 +47,13 @@ check_pgpsigs() { ;; esac for netfile in "${all_sources[@]}"; do - verify_file_signature "$netfile" "$statusfile" || continue + proto="$(get_protocol "$netfile")" + + if [[ $proto = git* ]]; then + verify_git_signature "$netfile" "$statusfile" || continue + else + verify_file_signature "$netfile" "$statusfile" || continue + fi # these variables are assigned values in parse_gpg_statusfile success=0 @@ -153,6 +159,42 @@ verify_file_signature() { $decompress < "$sourcefile" | gpg --quiet --batch --status-file "$statusfile" --verify "$file" - 2> /dev/null } +verify_git_signature() { + local netfile=$1 statusfile=$2 + local dir fragment query fragtype fragval + + dir=$(get_filepath "$netfile") + fragment=$(get_uri_fragment "$netfile") + query=$(get_uri_query "$netfile") + + if [[ $query != signed ]]; then + return 1 + fi + + case ${fragment%%=*} in + tag) + fragtype=tag + fragval=${fragment##*=} + ;; + commit|branch) + fragtype=commit + fragval=${fragment##*=} + ;; + '') + fragtype=commit + fragval=HEAD + esac + + printf " %s git repo ... " "${dir##*/}" >&2 + + git -C "$dir" verify-$fragtype --raw "$fragval" > "$statusfile" 2>&1 + if ! grep -qs NEWSIG "$statusfile"; then + printf '%s\n' "$(gettext "SIGNATURE NOT FOUND")" >&2 + errors=1 + return 1 + fi +} + parse_gpg_statusfile() { local type arg1 arg6 arg10 @@ -212,11 +254,14 @@ parse_gpg_statusfile() { } source_has_signatures() { - local file all_sources + local file all_sources proto get_all_sources_for_arch 'all_sources' for file in "${all_sources[@]}"; do - if [[ ${file%%::*} = *.@(sig?(n)|asc) ]]; then + proto="$(get_protocol "$file")" + query=$(get_uri_query "$netfile") + + if [[ ${file%%::*} = *.@(sig?(n)|asc) || ( $proto = git* && $query = signed ) ]]; then return 0 fi done diff --git a/scripts/libmakepkg/source/git.sh.in b/scripts/libmakepkg/source/git.sh.in index cc27663d..3d370dbd 100644 --- a/scripts/libmakepkg/source/git.sh.in +++ b/scripts/libmakepkg/source/git.sh.in @@ -39,6 +39,7 @@ download_git() { local url=$(get_url "$netfile") url=${url#git+} url=${url%%#*} + url=${url%%\?*} if [[ ! -d "$dir" ]] || dir_is_empty "$dir" ; then msg2 "$(gettext "Cloning %s %s repo...")" "${repo}" "git" @@ -66,14 +67,8 @@ download_git() { extract_git() { local netfile=$1 - local fragment=${netfile#*#} - if [[ $fragment = "$netfile" ]]; then - unset fragment - fi - - local repo=${netfile##*/} - repo=${repo%%#*} - repo=${repo%%.git*} + local fragment=$(get_uri_fragment "$netfile") + local repo=$(get_filename "$netfile") local dir=$(get_filepath "$netfile") [[ -z "$dir" ]] && dir="$SRCDEST/$(get_filename "$netfile")" diff --git a/scripts/libmakepkg/util/source.sh.in b/scripts/libmakepkg/util/source.sh.in index 9d4ba4a6..bbe3e927 100644 --- a/scripts/libmakepkg/util/source.sh.in +++ b/scripts/libmakepkg/util/source.sh.in @@ -65,6 +65,7 @@ get_filename() { case $proto in bzr*|git*|hg*|svn*) filename=${netfile%%#*} + filename=${filename%%\?*} filename=${filename%/} filename=${filename##*/} if [[ $proto = bzr* ]]; then @@ -111,6 +112,32 @@ get_filepath() { printf "%s\n" "$file" } +# extract the VCS revision/branch specifier from a source entry +get_uri_fragment() { + local netfile=$1 + + local fragment=${netfile#*#} + if [[ $fragment = "$netfile" ]]; then + unset fragment + fi + fragment=${fragment%\?*} + + printf "%s\n" "$fragment" +} + +# extract the VCS "signed" status from a source entry +get_uri_query() { + local netfile=$1 + + local query=${netfile#*\?} + if [[ $query = "$netfile" ]]; then + unset query + fi + query=${query%#*} + + printf "%s\n" "$query" +} + get_downloadclient() { local proto=$1 -- 2.11.0

Eli Schwartz

9:10 p.m.

New subject: [pacman-dev] [PATCH v3 3/3] Document the new query parameter for VCS sources

Signed-off-by: Eli Schwartz --- doc/PKGBUILD.5.txt | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt index 08ef3e4d..da052b0b 100644 --- a/doc/PKGBUILD.5.txt +++ b/doc/PKGBUILD.5.txt @@ -464,12 +464,12 @@ Using VCS Sources[[VCS]] ------------------------ Building a developmental version of a package using sources from a version control system (VCS) is enabled by specifying the source in the form -`source=('directory::url#fragment')`. Currently makepkg supports the Bazaar, Git, -Subversion, and Mercurial version control systems. For other version control -systems, manual cloning of upstream repositories must be done in the `prepare()` -function. +`source=('directory::url#fragment?query')`. Currently makepkg supports the +Bazaar, Git, Subversion, and Mercurial version control systems. For other +version control systems, manual cloning of upstream repositories must be done +in the `prepare()` function. -The source URL is divided into three components: +The source URL is divided into four components: *directory*:: (optional) Specifies an alternate directory name for makepkg to download @@ -501,6 +501,11 @@ The source URL is divided into three components: *svn*;; revision +*query*:: + (optional) Allows specifying whether a VCS checkout should be checked for + PGP-signed revisions. The source line should have the format + `source=(url#fragment?signed)` or `source=(url?signed#fragment)`. Currently + only supported by Git. Example ------- -- 2.11.0

Travis Burtrum

8 Dec 8 Dec

3:45 p.m.

Hello, Just a small comment. On 12/07/2016 03:48 PM, Jelle van der Waa wrote:

...

* git url, but no #tag= or #commit= specified, should verify HEAD on the #branch or no tag, commit, branch case.

For a #commit=hash you shouldn't have to verify anything, since git itself guarantees that the code under a specific commit hash cannot change. Everything else can change, including tags, so those are suitable for pgp verification. Thanks, Travis

Eli Schwartz

4:01 p.m.

On 12/08/2016 09:45 AM, Travis Burtrum wrote:

...

For a #commit=hash you shouldn't have to verify anything, since git itself guarantees that the code under a specific commit hash cannot change.

Everything else can change, including tags, so those are suitable for pgp verification.

Well, that would be just as good as having checksums, which is certainly something. But it is also completely missing the fundamental point of "PGP" verification. ... If users want to assume the maintainer has already checked the PGP signatures for proof of authorship, and simply rely on the checksums being accurate, they can use --skippgpcheck. Personally, I will continue on with checking pgp signatures... I don't see why signed git commits should be different from files with sha256sums in that respect. -- Eli Schwartz

2663

Age (days ago)

2690

Last active (days ago)

List overview

Download

18 comments

4 participants

participants (4)

Allan McRae
Eli Schwartz
Jelle van der Waa
Travis Burtrum