RFC: update hook for GPG signed submission on secured branches

RFC: update hook for GPG signed submission on secured branches

[Jason Pyeron]

Dev & users:

I would like your feedback.

I am working on a continuous integration (CI) system for CipherShed.org and we want to allow any fork to submit their patch to our CI server farm which will do integration testing. We want it to be easy to submit code, but to not allow interference with other submitters branches or fetching of other people's (any for bandwidth and simplicity) branches.

What would you change? Any bugs that you see?

-Jason (no cc please, I am on the list)

Here are the 2 configs.

$ cat config
[core]
        repositoryformatversion = 0
        filemode = true
        bare = true
[http]
        receivepack = true
        uploadpack = false
        getanyfile = false

$ cat hooks/update
#!/bin/bash

# (c) 2015 PD Inc. License found at http://www.apache.org/licenses/LICENSE-2.0 .

# --- Command line
refname="$1"
oldrev="$2"
newrev="$3"

# --- Safety check
if [ -z "$GIT_DIR" ]; then
        echo "Don't run this script from the command line." >&2
        echo " (if you want, you could supply GIT_DIR then run" >&2
        echo "  $0 <ref> <oldrev> <newrev>)" >&2
        exit 1
fi

if [ -z "$refname" -o -z "$oldrev" -o -z "$newrev" ]; then
        echo "usage: $0 <ref> <oldrev> <newrev>" >&2
        exit 1
fi

sigkey=$(\
 git cat-file $newrev -p |\
 sed -e '/^ /{H;$!d;}; x;/^gpgsig /!d; s/^gpgsig//;' |\
 cut -c 2- |\
 gpg --list-packets --textmode |\
 sed '/keyid/!d; s/.*keyid \([0-9A-F]\{16\}\).*/\1/I' \
)

if [ -z "$sigkey" ]; then
        echo no GPG signature on commit $newrev
        exit 1
fi

if ! gpg -k "$sigkey" 2> /dev/null > /dev/null; then
        # "$sigkey" not known
        RES="$(gpg --keyserver hkp://pgp.mit.edu --recv-keys "$sigkey" 2>&1)"
        if [ $? -ne 0 ]; then
                echo "$RES"
                exit 1
        fi
fi

sigstatus=$(git log $newrev --pretty=format:%G? -n 1)

case "$sigstatus" in
        G)
                #ok, trusted
                ;;
        U)
                #ok, untrusted
                ;;
        *)
                #not ok
                echo sigstatus: $sigstatus
                git log $newrev --pretty=format:%GG -n 1
                exit 1;
                ;;
esac

if [[ $refname != refs/heads/* ]]; then
        echo only heads may be pushed, illegal ref: $refname
        exit 1;
fi

head="${refname:11}"

shopt -s nocasematch

case "$head" in
        ${sigkey}-*)
                #ok
                ;;
        ${sigkey}/*)
                #ok
                ;;
        ${sigkey:(-8)}-*)
                #ok
                ;;
        ${sigkey:(-8)}/*)
                #ok
                ;;
        *)
                #not your branch
                echo "you (a.k.a. $sigkey) are not authorized to push to branch: $head"
                echo "try making a branch like: $sigkey-... or $sigkey/*"
                echo "you can use a less secure ${sigkey:(-8)} too"
                exit 1
                ;;
esac

exit 0

--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Jason Pyeron                      PD Inc. http://www.pdinc.us -
- Principal Consultant              10 West 24th Street #100    -
- +1 (443) 269-1555 x333            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00.

Re: RFC: update hook for GPG signed submission on secured branches

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

> What would you change? Any bugs that you see?

> sigkey=$(\
>  git cat-file $newrev -p |\

"-p" being a command line option should come before revision, but
more importantly, because you accept pushes only to refs/heads/, you
would want to explicitly require commit objects, no?  i.e.

	git cat-file commit "$newrev" |

I am not sure if you need these unsightly backslashes.  When you
stop talking to it after saying "$(", or "$( git cat-file ... |",
the shell _knows_ that you haven't stopped what you want to tell
it.

>  sed -e '/^ /{H;$!d;}; x;/^gpgsig /!d; s/^gpgsig//;' |\
>  cut -c 2- |\

It always makes me feel nervous to see people pipe sed output to
another filter that is a mere s/.//;

Is this complex pipeline the same as this (I didn't understand the
trailing I at the end)?

	git cat-file commit "$newrev" |
        sed -ne '/^gpgsig /,/^ -----END/{
        	s/^gpgsig //
                s/^ //p
	}' |
	gpg --list-packets --textmode |
        sed -ne '/^:signature packet:/s/.*keyid \([0-9A-F]*\).*/\1/p'

>  gpg --list-packets --textmode |\
>  sed '/keyid/!d; s/.*keyid \([0-9A-F]\{16\}\).*/\1/I' \
> )

> if [ -z "$sigkey" ]; then
>         echo no GPG signature on commit $newrev
>         exit 1
> fi

I am not sure if the design of this, to require signature only on
the tip commit, is sound.  That is not a -bug- in the script,
though.

> if [[ $refname != refs/heads/* ]]; then
>         echo only heads may be pushed, illegal ref: $refname
>         exit 1;
> fi
>
> head="${refname:11}"

It is hard to tell where the magic number 11 comes from.  Perhaps

    head="${refname#refs/heads/}"

reads easier?

Re: RFC: update hook for GPG signed submission on secured branches

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> I am not sure if you need these unsightly backslashes.  When you
> stop talking to it after saying "$(", or "$( git cat-file ... |",
> the shell _knows_ that you haven't stopped what you want to tell
> it.

Ehh, s/stopped/finished/, that is.

RE: RFC: update hook for GPG signed submission on secured branches

[Jason Pyeron]

> -----Original Message-----
> From: Junio C Hamano
> Sent: Friday, January 16, 2015 14:33
> 
> "Jason Pyeron" <jpyeron@pdinc.us> writes:
> 
> > What would you change? Any bugs that you see?
> 
> > sigkey=$(\
> >  git cat-file $newrev -p |\
> 
> "-p" being a command line option should come before revision, but
> more importantly, because you accept pushes only to refs/heads/, you
> would want to explicitly require commit objects, no?  i.e.
> 
> 	git cat-file commit "$newrev" |

True.

> 
> I am not sure if you need these unsightly backslashes.  When you
> stop talking to it after saying "$(", or "$( git cat-file ... |",
> the shell _knows_ that you haven't stopped what you want to tell
> it.
> 
> >  sed -e '/^ /{H;$!d;}; x;/^gpgsig /!d; s/^gpgsig//;' |\
> >  cut -c 2- |\
> 
> It always makes me feel nervous to see people pipe sed output to
> another filter that is a mere s/.//;

It was a very quick Lego block build.

> 
> Is this complex pipeline the same as this (I didn't understand the
> trailing I at the end)?

Case insensitive, could have used [0-9a-fA-F].

> 
> 	git cat-file commit "$newrev" |
>         sed -ne '/^gpgsig /,/^ -----END/{
>         	s/^gpgsig //
>                 s/^ //p
> 	}' |

Will all future signature values end with a "^ -----END"? I was only going on the assumption that continuation lines start with a single space.

> 	gpg --list-packets --textmode |
>         sed -ne '/^:signature packet:/s/.*keyid \([0-9A-F]*\).*/\1/p'
> 
> >  gpg --list-packets --textmode |\
> >  sed '/keyid/!d; s/.*keyid \([0-9A-F]\{16\}\).*/\1/I' \
> > )
> 
> > if [ -z "$sigkey" ]; then
> >         echo no GPG signature on commit $newrev
> >         exit 1
> > fi
> 
> I am not sure if the design of this, to require signature only on
> the tip commit, is sound.  That is not a -bug- in the script,
> though.

It is to handle the "all my devs worked on this, they do ________ GPG", so as long as the tip os signed, it is an implicit I am responsible for what is submitted.

> 
> > if [[ $refname != refs/heads/* ]]; then
> >         echo only heads may be pushed, illegal ref: $refname
> >         exit 1;
> > fi
> >
> > head="${refname:11}"
> 
> It is hard to tell where the magic number 11 comes from.  Perhaps
> 
>     head="${refname#refs/heads/}"
> 
> reads easier?

Much.

Thanks!

-Jason

--
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
-                                                               -
- Jason Pyeron                      PD Inc. http://www.pdinc.us -
- Principal Consultant              10 West 24th Street #100    -
- +1 (443) 269-1555 x333            Baltimore, Maryland 21218   -
-                                                               -
-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
This message is copyright PD Inc, subject to license 20080407P00. 

Re: RFC: update hook for GPG signed submission on secured branches

[John Keeping]

On Fri, Jan 16, 2015 at 02:47:25PM -0500, Jason Pyeron wrote:
> > I am not sure if the design of this, to require signature only on
> > the tip commit, is sound.  That is not a -bug- in the script,
> > though.
> 
> It is to handle the "all my devs worked on this, they do ________
> GPG", so as long as the tip os signed, it is an implicit I am
> responsible for what is submitted.

Isn't this an ideal scenario for using the signed pushes introduced in
Git 2.2.0?

Re: RFC: update hook for GPG signed submission on secured branches

[Junio C Hamano]

"Jason Pyeron" <jpyeron@pdinc.us> writes:

>> Is this complex pipeline the same as this (I didn't understand the
>> trailing I at the end)?
>
> Case insensitive, could have used [0-9a-fA-F].

Ahh, a GNU extension.

>> 	git cat-file commit "$newrev" |
>>         sed -ne '/^gpgsig /,/^ -----END/{
>>         	s/^gpgsig //
>>                 s/^ //p
>> 	}' |
>
> Will all future signature values end with a "^ -----END"?

If it bothers you, alternatively, you can stop at the end of the
commit object header, i.e.

	/^gpgsig /,/^$/{
        	s/^gigsig//
                s/^ //p
	}

but I do not think it matters that much ;-)