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

[Jani Nikula]

On Wed, 01 Nov 2017, Sean Paul <seanpaul at chromium.org> wrote:
> On Wed, Nov 1, 2017 at 1:00 PM, Eric Anholt <eric at anholt.net> wrote:
>> Sean Paul <seanpaul at chromium.org> writes:
>>
>>> 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?
>>
>> We've used GPG a bunch in fdo infrastructure, and my experience is that
>> it gets you basically no assurance, in exchange for a bunch of admin
>> overhead (since people lose keys and need to be able to say "Yes, this
>> is really me, here with a new key").
>>
>> I've been signing email for years, but I'm not a fan of tying
>> participation in open source development to using GPG.  It's just not
>> useful enough for its costs, particularly discouraging new developers.
>
> To be fair, this change only signs commits applied by committers who
> have already added their keyid to their dimrc (for signing tags). If
> there is no keyid, things work as normal.
>
> That said, it seems like the overall sentiment on signing commits is
> negative, so I'll move on from the idea.

I'm afraid my conclusion is the same. But we can revisit the idea once
we have everyone using signed tags!

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center