[Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in addition to tags
Daniel Vetter
daniel at ffwll.ch
Tue Oct 31 17:31:59 UTC 2017
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.
-Daniel
>
> Sean
>
>
>> 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
>>> git add integration-manifest
>>> - git commit --quiet -m "$integration_branch: $dim_timestamp integration manifest"
>>> + git commit $gpg_keyid --quiet -m "$integration_branch: $dim_timestamp integration manifest"
>>> echo "Done."
>>>
>>> remote=$(repo_to_remote drm-tip)
>>> @@ -848,7 +863,7 @@ function dim_push
>>>
>>> function apply_patch #patch_file
>>> {
>>> - local patch message_id committer_email patch_from sob rv
>>> + local patch message_id committer_email patch_from sob rv gpg_keyid
>>>
>>> patch="$1"
>>> shift
>>> @@ -860,7 +875,8 @@ function apply_patch #patch_file
>>> sob=-s
>>> fi
>>>
>>> - git am --scissors -3 $sob "$@" $patch
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> + git am --scissors -3 $sob $gpg_keyid "$@" $patch
>>>
>>> if [ -n "$message_id" ]; then
>>> dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
>>> @@ -911,7 +927,7 @@ function dim_apply_branch
>>>
>>> function dim_apply_pull
>>> {
>>> - local branch file message_id pull_branch rv
>>> + local branch file message_id pull_branch rv gpg_keyid
>>>
>>> branch=${1:?$usage}
>>> file=$(mktemp)
>>> @@ -929,7 +945,8 @@ function dim_apply_pull
>>>
>>> message_id=$(message_get_id $file)
>>>
>>> - git commit --amend -s --no-edit
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> + git commit $gpg_keyid --amend -s --no-edit
>>> if [ -n "$message_id" ]; then
>>> dim_commit_add_tag "Link: https://patchwork.freedesktop.org/patch/msgid/$message_id"
>>> else
>>> @@ -945,7 +962,7 @@ function dim_apply_pull
>>>
>>> function dim_backmerge
>>> {
>>> - local branch upstream patch_file
>>> + local branch upstream patch_file gpg_keyid
>>>
>>> branch=${1:?$usage}
>>> upstream=${2:?$usage}
>>> @@ -990,8 +1007,9 @@ function dim_backmerge
>>> echoerr " git commit -a"
>>> fi
>>>
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> git add -u
>>> - git commit -s
>>> + git commit $gpg_keyid -s
>>> }
>>>
>>> function dim_add_link
>>> @@ -1227,7 +1245,7 @@ function dim_magic_patch
>>>
>>> function dim_create_branch
>>> {
>>> - local branch repo remote
>>> + local branch repo remote gpg_keyid
>>>
>>> branch=${1:?$usage}
>>> start=${2:-HEAD}
>>> @@ -1250,13 +1268,14 @@ function dim_create_branch
>>> cd $DIM_PREFIX/drm-rerere
>>> $DRY sed -i "s/^\() # DO NOT CHANGE THIS LINE\)$/\t\"$repo\t\t${branch//\//\\\/}\"\n\1/" $integration_config
>>>
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> $DRY git add $integration_config
>>> - $DRY git commit --quiet -m "Add $repo $branch to $integration_config"
>>> + $DRY git commit $gpg_keyid --quiet -m "Add $repo $branch to $integration_config"
>>> }
>>>
>>> function dim_remove_branch
>>> {
>>> - local branch repo remote
>>> + local branch repo remote gpg_keyid
>>>
>>> branch=${1:?$usage}
>>>
>>> @@ -1288,8 +1307,9 @@ function dim_remove_branch
>>> cd $DIM_PREFIX/drm-rerere
>>> $DRY sed -i "/^[[:space:]]*\"${repo}[[:space:]]\+${branch//\//\\\/}.*$/d" $integration_config
>>>
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> $DRY git add $integration_config
>>> - $DRY git commit --quiet -m "Remove $repo $branch from $integration_config"
>>> + $DRY git commit $gpg_keyid --quiet -m "Remove $repo $branch from $integration_config"
>>>
>>> dim_rebuild_tip
>>> }
>>> @@ -1579,7 +1599,7 @@ function dim_for_each_workdir
>>>
>>> function dim_update_next
>>> {
>>> - local remote
>>> + local remote gpg_keyid
>>>
>>> assert_branch drm-intel-next-queued
>>>
>>> @@ -1597,12 +1617,13 @@ function dim_update_next
>>> exit 2
>>> fi
>>>
>>> + gpg_keyid=$(gpg_keyid_for_commit)
>>> driver_date=$(date +%Y%m%d)
>>> driver_timestamp=$(date +%s)
>>> $DRY sed -i -e "s/^#define DRIVER_DATE.*\"[0-9]*\"$/#define DRIVER_DATE\t\t\"$driver_date\"/; s/^#define DRIVER_TIMESTAMP.*/#define DRIVER_TIMESTAMP\t$driver_timestamp/" \
>>> drivers/gpu/drm/i915/i915_drv.h
>>> $DRY git add drivers/gpu/drm/i915/i915_drv.h
>>> - git commit $DRY_RUN -sm "drm/i915: Update DRIVER_DATE to $driver_date"
>>> + git commit $DRY_RUN $gpg_keyid -sm "drm/i915: Update DRIVER_DATE to $driver_date"
>>>
>>> gitk drm-intel-next-queued ^$(repo_to_remote drm-upstream)/drm-next &
>>>
>>> @@ -1614,7 +1635,7 @@ function dim_update_next
>>>
>>> function dim_update_next_continue
>>> {
>>> - local remote intel_remote req_file suffix tag tag_testing
>>> + local remote intel_remote req_file suffix tag tag_testing gpg_keyid
>>>
>>> assert_branch drm-intel-next-queued
>>>
>>> @@ -1630,7 +1651,8 @@ function dim_update_next_continue
>>> tag_testing="drm-intel-testing-$dim_today-$((++suffix))"
>>> done
>>>
>>> - $DRY git tag -a $DIM_GPG_KEYID $tag $intel_remote/drm-intel-next
>>> + gpg_keyid=$(gpg_keyid_for_tag)
>>> + $DRY git tag -a $gpg_keyid $tag $intel_remote/drm-intel-next
>>> git push $DRY_RUN $intel_remote $tag
>>>
>>> echo "Updating drm-intel-testing to latest drm-tip"
>>> @@ -1655,7 +1677,7 @@ function dim_update_next_continue
>>>
>>> function dim_tag_next
>>> {
>>> - local intel_remote tag suffix
>>> + local intel_remote tag suffix gpg_keyid
>>>
>>> cd $DIM_PREFIX/$DIM_REPO
>>>
>>> @@ -1670,7 +1692,8 @@ function dim_tag_next
>>> tag="drm-intel-next-$dim_today-$((++suffix))"
>>> done
>>>
>>> - $DRY git tag -a $DIM_GPG_KEYID $tag $intel_remote/drm-intel-next
>>> + gpg_keyid=$(gpg_keyid_for_tag)
>>> + $DRY git tag -a $gpg_keyid $tag $intel_remote/drm-intel-next
>>> git push $DRY_RUN $intel_remote $tag
>>> else
>>> echo "drm-intel-next not up-to-date, aborting"
>>> @@ -1700,7 +1723,7 @@ function prep_pull_tag_summary
>>> # dim_pull_request branch upstream
>>> function dim_pull_request
>>> {
>>> - local branch upstream remote repo req_file url_list git_url suffix tag
>>> + local branch upstream remote repo req_file url_list git_url suffix tag gpg_keyid
>>>
>>> branch=${1:?$usage}
>>> upstream=${2:?$usage}
>>> @@ -1731,7 +1754,8 @@ function dim_pull_request
>>> done
>>> gitk "$branch@{upstream}" ^$upstream &
>>> prep_pull_tag_summary | $DRY git tag -F- $tag "$branch@{upstream}"
>>> - $DRY git tag -a $DIM_GPG_KEYID -f $tag
>>> + gpg_keyid=$(gpg_keyid_for_tag)
>>> + $DRY git tag -a $gpg_keyid -f $tag
>>> $DRY git push $remote $tag
>>> prep_pull_mail $req_file $tag
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> dim-tools mailing list
> dim-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list