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

31 Oct 2017

On Tue, Oct 31, 2017 at 1:31 PM, Daniel Vetter daniel@ffwll.ch wrote:
...
On Tue, Oct 31, 2017 at 5:14 PM, Sean Paul seanpaul@chromium.org wrote:
...
On Tue, Oct 31, 2017 at 4:27 AM, Jani Nikula
jani.nikula@linux.intel.com wrote:
...
Reminder, we have this new list dim-tools@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@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@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.
Sean
...
-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@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

    

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

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

Signed-off-by: Sean Paul seanpaul@chromium.org