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

[Jani Nikula]

On Wed, 01 Nov 2017, Sean Paul  wrote:
> On Wed, Nov 1, 2017 at 1:00 PM, Eric Anholt  wrote:
>> Sean Paul  writes:
>>
>>> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan  wrote:
 2017-10-31 Sean Paul :

> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter  wrote:
> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul  
> > wrote:
> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
> >>  wrote:
> >>>
> >>> Reminder, we have this new list dim-to...@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  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 
>  ---
> 
>  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 

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

[Sean Paul]

On Wed, Nov 1, 2017 at 1:00 PM, Eric Anholt  wrote:
> Sean Paul  writes:
>
>> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan  wrote:
>>> 2017-10-31 Sean Paul :
>>>
 On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter  wrote:
 > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul  wrote:
 >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
 >>  wrote:
 >>>
 >>> Reminder, we have this new list dim-to...@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  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 
  ---
 
  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 

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

[Eric Anholt]

Sean Paul  writes:

> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan  wrote:
>> 2017-10-31 Sean Paul :
>>
>>> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter  wrote:
>>> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul  wrote:
>>> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
>>> >>  wrote:
>>> >>>
>>> >>> Reminder, we have this new list dim-to...@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  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 
>>>  ---
>>> 
>>>  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 

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

[Deucher, Alexander]

> -Original Message-
> From: dim-tools [mailto:dim-tools-boun...@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-
> to...@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 <gust...@padovan.org>
> wrote:
> > 2017-10-31 Sean Paul <seanp...@chromium.org>:
> >
> >> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter <dan...@ffwll.ch> wrote:
> >> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul <seanp...@chromium.org>
> wrote:
> >> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
> >> >> <jani.nik...@linux.intel.com> wrote:
> >> >>>
> >> >>> Reminder, we have this new list dim-to...@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 <seanp...@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 <seanp...@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 s

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

[Sean Paul]

On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan  wrote:
> 2017-10-31 Sean Paul :
>
>> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter  wrote:
>> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul  wrote:
>> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
>> >>  wrote:
>> >>>
>> >>> Reminder, we have this new list dim-to...@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  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 
>>  ---
>> 
>>  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?

Sean

[1]- https://lists.gnupg.org/pipermail/gnupg-users/2015-October/054608.html

>
> Gustavo
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

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

[Gustavo Padovan]

2017-10-31 Sean Paul :

> On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter  wrote:
> > On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul  wrote:
> >> On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
> >>  wrote:
> >>>
> >>> Reminder, we have this new list dim-to...@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  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 
>  ---
> 
>  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 and we also need
a signing party to sign each others keys.

Gustavo

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Daniel Vetter]

On Tue, Oct 31, 2017 at 10:27:24AM +0200, Jani Nikula wrote:
> 
> Reminder, we have this new list dim-to...@lists.freedesktop.org for
> maintainer tools patches. Cc'd.
> 
> On Mon, 30 Oct 2017, Sean Paul  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?

Same here. I get why signing tags makes sense, because email is an
entirely unsecured protocol. Signing commits otoh seems mildly silly.
What's the threat/attack model that prevents?

And if we go with signed commits, shouldn't we encourage contributors to
sign their mails and have some means to add the verification of the same
to the commit? This is what happens when you merge a signed patch at least
...

I'm confused about this.
-Daniel

> 
> > Signed-off-by: Sean Paul 
> > ---
> >
> > 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?
> 
> Although... perhaps starting to sign tags should not force signing
> commits?
> 
> BR,
> Jani.
> 
> 
> > +
> >  function read_integration_config
> >  {
> > # clear everything first to allow configuration reload
> > @@ -473,12 +484,14 @@ EOF
> >  # append all arguments as tags at the end of the commit message of HEAD
> >  function dim_commit_add_tag
> >  {
> > +   local gpg_keyid
> > +   gpg_keyid=$(gpg_keyid_for_commit)
> > for arg; do
> > # the first sed deletes all trailing blank lines at the end
> > git log -1 --pretty=%B | \
> > sed -e :a -e '/^\n*$/{$d;N;ba' -e '}' | \
> > sed "\$a${arg}" | \
> > -   git commit --amend -F-
> > +   git commit $gpg_keyid --amend -F-
> > done
> >  }
> >  
> > @@ -604,7 +617,7 @@ function update_rerere_cache
> >  
> >  function commit_rerere_cache
> >  {
> > -   local remote file commit_message
> > +   local remote file commit_message gpg_keyid
> >  
> > echo -n "Updating rerere cache... "
> >  
> > @@ -640,7 +653,8 @@ function commit_rerere_cache
> > $(git --version)
> > EOF
> >  
> > -   if git commit -F $commit_message >& /dev/null; then
> > +   gpg_keyid=$(gpg_keyid_for_commit)
> > +   if git commit $gpg_keyid -F $commit_message >& /dev/null; then
> > echo -n "New commit. "
> > else
> > echo -n "Nothing changed. "
> > @@ -653,13 +667,14 @@ function commit_rerere_cache
> >  
> >  function dim_rebuild_tip
> >  {
> > -   local integration_branch specfile first rerere repo remote
> > +   local integration_branch specfile first rerere repo remote gpg_keyid
> >  
> > integration_branch=drm-tip
> > specfile=$(mktemp)
> > first=1
> >  
> > rerere=$DIM_PREFIX/drm-rerere
> > +   gpg_keyid=$(gpg_keyid_for_commit)
> >  
> > cd $rerere
> > if git status --porcelain | grep -q -v "^[ ?][ ?]"; then
> > @@ -731,7 +746,7 @@ function dim_rebuild_tip
> >  
> > # because we filter out fast-forward merges there will
> > # always be something to commit
> > -   git commit --no-edit --quiet
> > +   git commit $gpg_keyid --no-edit --quiet
> > echo "Done."
> > fi
> >  
> > @@ -743,7 +758,7 @@ function dim_rebuild_tip
> > echo -n "Adding integration manifest $integration_branch: 
> > $dim_timestamp... "
> > mv $specfile integration-manifest
>