[Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in addition to tags

Wed Nov 1 16:16:11 UTC 2017

> -----Original Message-----
> From: dim-tools [mailto:dim-tools-bounces at lists.freedesktop.org] On Behalf
> Of Sean Paul
> Sent: Wednesday, November 01, 2017 8:52 AM
> To: Gustavo Padovan
> Cc: Daniel Vetter; Intel Graphics Development; dim-
> tools at lists.freedesktop.org; dri-devel; Daniel Vetter
> Subject: Re: [Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in
> addition to tags
> 
> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan <gustavo at padovan.org>
> wrote:
> > 2017-10-31 Sean Paul <seanpaul at chromium.org>:
> >
> >> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> >> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul <seanpaul at chromium.org>
> wrote:
> >> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
> >> >> <jani.nikula at linux.intel.com> wrote:
> >> >>>
> >> >>> Reminder, we have this new list dim-tools at lists.freedesktop.org for
> >> >>> maintainer tools patches. Cc'd.
> >> >>>
> >> >>
> >> >> Ahh, cool. I didn't realize dim grew up!
> >> >>
> >> >>> On Mon, 30 Oct 2017, Sean Paul <seanpaul at chromium.org> wrote:
> >> >>>> Expanding on Jani's work to sign tags, this patch adds signing for git
> >> >>>> commit/am.
> >> >>>
> >> >>> I guess I'd like more rationale here. Is this something we should be
> >> >>> doing? Is anyone else doing this?
> >> >>>
> >> >>
> >> >> Sure thing. Signing commits allows Dave to use --verify-signatures
> >> >> when pulling. If something is not signed, we'll know it was either not
> >> >> applied with dim, or was altered on fdo (both warrant investigation).
> >> >>
> >> >> I suspect no one else is doing this since most trees are single
> >> >> maintainer, and it's not possible to sign commits via git send-email.
> >> >> Since we have the committer model, and a bunch of people with
> access
> >> >> to fdo and the tree, I think it's important to add this. Especially
> >> >> since we can do it in dim without overhead.
> >> >>
> >> >>>> Signed-off-by: Sean Paul <seanpaul at chromium.org>
> >> >>>> ---
> >> >>>>
> >> >>>> This has been lightly tested with dim apply-branch/dim push-
> branch.
> >> >>>>
> >> >>>> Sean
> >> >>>>
> >> >>>>  dim | 78
> +++++++++++++++++++++++++++++++++++++++++++++----------------------
> --
> >> >>>>  1 file changed, 51 insertions(+), 27 deletions(-)
> >> >>>>
> >> >>>> diff --git a/dim b/dim
> >> >>>> index 527989aff9ad..cd5e41f89a3a 100755
> >> >>>> --- a/dim
> >> >>>> +++ b/dim
> >> >>>> @@ -67,9 +67,6 @@
> DIM_TEMPLATE_SIGNATURE=${DIM_TEMPLATE_SIGNATURE:-
> $HOME/.dim.template.signature}
> >> >>>>  # dim pull-request tag summary template
> >> >>>>
> DIM_TEMPLATE_TAG_SUMMARY=${DIM_TEMPLATE_TAG_SUMMARY:-
> $HOME/.dim.template.tagsummary}
> >> >>>>
> >> >>>> -# GPG key id for signing tags. If unset, don't sign.
> >> >>>> -DIM_GPG_KEYID=${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID}
> >> >>>> -
> >> >>>>  #
> >> >>>>  # Internal configuration.
> >> >>>>  #
> >> >>>> @@ -104,6 +101,20 @@ test_request_recipients=(
> >> >>>>  # integration configuration
> >> >>>>  integration_config=nightly.conf
> >> >>>>
> >> >>>> +# GPG key id for signing tags. If unset, don't sign.
> >> >>>> +function gpg_keyid_for_tag
> >> >>>> +{
> >> >>>> +     echo "${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID}"
> >> >>>> +     return 0
> >> >>>> +}
> >> >>>> +
> >> >>>> +# GPG key id for committing (git commit/am). If unset, don't sign.
> >> >>>> +function gpg_keyid_for_commit
> >> >>>> +{
> >> >>>> +     echo "${DIM_GPG_KEYID:+-S$DIM_GPG_KEYID}"
> >> >>>> +     return 0
> >> >>>> +}
> >> >>>
> >> >>> This seems like an overly complicated way to achieve what you want.
> >> >>>
> >> >>> Just put these under "Internal configuration." instead:
> >> >>>
> >> >>> dim_gpg_sign_tag=${DIM_GPG_KEYID:+-u $DIM_GPG_KEYID}
> >> >>> dim_gpg_sign_commit=${DIM_GPG_KEYID:+-S$DIM_GPG_KEYID}
> >> >>>
> >> >>> And use directly in git tag and commit, respectively?
> >> >>>
> >> >>
> >> >> Yep, sounds good.
> >> >>
> >> >>> Although... perhaps starting to sign tags should not force signing
> >> >>> commits?
> >> >>>
> >> >>
> >> >> Why would it be desirable to *not* sign tags?
> >> >
> >> > Again, what's the threat model you're trying to defend against? Atm
> >> > anyone with commit rights to fd.o can push whatever they want to. If
> >> > they want to be evil, they can also push whatever kind of garbage they
> >> > want to, including commit signature and and fake Link: and review
> >> > tags. With pull requests/tags signing them prevents a
> >> > man-in-the-midddle attack of the unprotected pull request in the mail,
> >> > but I still don't see what signing commits protects against.
> >>
> >> This is protecting against a bad actor (either through a committer's
> >> account, or some other fdo account) gaining access to the tree on fdo
> >> and either adding a malicious commit, or altering an existing commit.
> >
> > Yeah, but then we need to enforce it for all committer
> 
> My hope is that dim makes it easy enough to get everyone on board
> eventually. In the interim, the people with signing commits will be
> able to attest that those commits were applied by them.
> 
> > and we also need
> > a signing party to sign each others keys.
> 
> I feel like most of us see each other often enough to make this
> possible. Even without a signing party, we still get *some* amount of
> coverage by virtue of TOFU [1].
> 
> Is anyone against the idea of signing commits? Is there a reason that
> we shouldn't?

I feel like this raises the bar pretty high if we want to move to more committers.   Most of the work will fall on the few people willing to jump through the hoops or patches will just get dropped as people lose interest after fighting with gpg.  I can't say I've ever had a particularly straight forward interaction with gpg for anything.

Alex

> 
> Sean
> 
> [1]- https://lists.gnupg.org/pipermail/gnupg-users/2015-
> October/054608.html
> 
> >
> > Gustavo
> >
> _______________________________________________
> dim-tools mailing list
> dim-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools