* [maintainer-tools PATCH] dim: Sign commits in addition to tags
@ 2017-10-30 21:06 Sean Paul
2017-10-31 8:27 ` Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Sean Paul @ 2017-10-30 21:06 UTC (permalink / raw)
To: daniel.vetter, jani.nikula; +Cc: intel-gfx, dri-devel
Expanding on Jani's work to sign tags, this patch adds signing for git
commit/am.
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
+}
+
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
--
2.15.0.rc2.357.g7e34df9404-goog
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-30 21:06 [maintainer-tools PATCH] dim: Sign commits in addition to tags Sean Paul
@ 2017-10-31 8:27 ` Jani Nikula
2017-10-31 9:02 ` Daniel Vetter
2017-10-31 16:14 ` Sean Paul
0 siblings, 2 replies; 12+ messages in thread
From: Jani Nikula @ 2017-10-31 8:27 UTC (permalink / raw)
To: Sean Paul, daniel.vetter; +Cc: dim-tools, intel-gfx, dri-devel
Reminder, we have this new list dim-tools@lists.freedesktop.org for
maintainer tools patches. Cc'd.
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?
> 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?
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
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-31 8:27 ` Jani Nikula
@ 2017-10-31 9:02 ` Daniel Vetter
2017-10-31 16:14 ` Sean Paul
1 sibling, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-10-31 9:02 UTC (permalink / raw)
To: Jani Nikula; +Cc: daniel.vetter, intel-gfx, dim-tools, dri-devel
On Tue, Oct 31, 2017 at 10:27:24AM +0200, Jani Nikula wrote:
>
> Reminder, we have this new list dim-tools@lists.freedesktop.org for
> maintainer tools patches. Cc'd.
>
> 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?
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 <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?
>
> 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
> > 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-31 8:27 ` Jani Nikula
2017-10-31 9:02 ` Daniel Vetter
@ 2017-10-31 16:14 ` Sean Paul
2017-10-31 17:31 ` Daniel Vetter
1 sibling, 1 reply; 12+ messages in thread
From: Sean Paul @ 2017-10-31 16:14 UTC (permalink / raw)
To: Jani Nikula
Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
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?
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-31 16:14 ` Sean Paul
@ 2017-10-31 17:31 ` Daniel Vetter
2017-10-31 17:45 ` Sean Paul
0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2017-10-31 17:31 UTC (permalink / raw)
To: Sean Paul; +Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
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.
-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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-31 17:31 ` Daniel Vetter
@ 2017-10-31 17:45 ` Sean Paul
2017-11-01 11:12 ` Gustavo Padovan
0 siblings, 1 reply; 12+ messages in thread
From: Sean Paul @ 2017-10-31 17:45 UTC (permalink / raw)
To: Daniel Vetter
Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-10-31 17:45 ` Sean Paul
@ 2017-11-01 11:12 ` Gustavo Padovan
2017-11-01 12:52 ` Sean Paul
0 siblings, 1 reply; 12+ messages in thread
From: Gustavo Padovan @ 2017-11-01 11:12 UTC (permalink / raw)
To: Sean Paul; +Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
2017-10-31 Sean Paul <seanpaul@chromium.org>:
> 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.
Yeah, but then we need to enforce it for all committer and we also need
a signing party to sign each others keys.
Gustavo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-11-01 11:12 ` Gustavo Padovan
@ 2017-11-01 12:52 ` Sean Paul
2017-11-01 16:16 ` [Intel-gfx] " Deucher, Alexander
2017-11-01 17:00 ` Eric Anholt
0 siblings, 2 replies; 12+ messages in thread
From: Sean Paul @ 2017-11-01 12:52 UTC (permalink / raw)
To: Gustavo Padovan
Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
> 2017-10-31 Sean Paul <seanpaul@chromium.org>:
>
>> 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.
>
> 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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-11-01 12:52 ` Sean Paul
@ 2017-11-01 16:16 ` Deucher, Alexander
2017-11-01 17:00 ` Eric Anholt
1 sibling, 0 replies; 12+ messages in thread
From: Deucher, Alexander @ 2017-11-01 16:16 UTC (permalink / raw)
To: 'Sean Paul', Gustavo Padovan
Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
> -----Original Message-----
> From: dim-tools [mailto:dim-tools-bounces@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@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@padovan.org>
> wrote:
> > 2017-10-31 Sean Paul <seanpaul@chromium.org>:
> >
> >> 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.
> >
> > 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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-11-01 12:52 ` Sean Paul
2017-11-01 16:16 ` [Intel-gfx] " Deucher, Alexander
@ 2017-11-01 17:00 ` Eric Anholt
2017-11-01 17:31 ` Sean Paul
1 sibling, 1 reply; 12+ messages in thread
From: Eric Anholt @ 2017-11-01 17:00 UTC (permalink / raw)
To: Sean Paul, Gustavo Padovan
Cc: Daniel Vetter, Intel Graphics Development, dim-tools, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 5372 bytes --]
Sean Paul <seanpaul@chromium.org> writes:
> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
>> 2017-10-31 Sean Paul <seanpaul@chromium.org>:
>>
>>> 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.
>>
>> 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.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-11-01 17:00 ` Eric Anholt
@ 2017-11-01 17:31 ` Sean Paul
2017-11-02 8:08 ` [Intel-gfx] " Jani Nikula
0 siblings, 1 reply; 12+ messages in thread
From: Sean Paul @ 2017-11-01 17:31 UTC (permalink / raw)
To: Eric Anholt
Cc: dim-tools, Intel Graphics Development, dri-devel, Alex Deucher,
Daniel Vetter
On Wed, Nov 1, 2017 at 1:00 PM, Eric Anholt <eric@anholt.net> wrote:
> Sean Paul <seanpaul@chromium.org> writes:
>
>> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
>>> 2017-10-31 Sean Paul <seanpaul@chromium.org>:
>>>
>>>> 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.
>>>
>>> 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.
Thanks for the feedback, all!
Sean
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Intel-gfx] [maintainer-tools PATCH] dim: Sign commits in addition to tags
2017-11-01 17:31 ` Sean Paul
@ 2017-11-02 8:08 ` Jani Nikula
0 siblings, 0 replies; 12+ messages in thread
From: Jani Nikula @ 2017-11-02 8:08 UTC (permalink / raw)
To: Sean Paul, Eric Anholt
Cc: dim-tools, Intel Graphics Development, dri-devel, Alex Deucher,
Daniel Vetter
On Wed, 01 Nov 2017, Sean Paul <seanpaul@chromium.org> wrote:
> On Wed, Nov 1, 2017 at 1:00 PM, Eric Anholt <eric@anholt.net> wrote:
>> Sean Paul <seanpaul@chromium.org> writes:
>>
>>> On Wed, Nov 1, 2017 at 7:12 AM, Gustavo Padovan <gustavo@padovan.org> wrote:
>>>> 2017-10-31 Sean Paul <seanpaul@chromium.org>:
>>>>
>>>>> 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.
>>>>
>>>> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-11-02 8:08 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-30 21:06 [maintainer-tools PATCH] dim: Sign commits in addition to tags Sean Paul
2017-10-31 8:27 ` Jani Nikula
2017-10-31 9:02 ` Daniel Vetter
2017-10-31 16:14 ` Sean Paul
2017-10-31 17:31 ` Daniel Vetter
2017-10-31 17:45 ` Sean Paul
2017-11-01 11:12 ` Gustavo Padovan
2017-11-01 12:52 ` Sean Paul
2017-11-01 16:16 ` [Intel-gfx] " Deucher, Alexander
2017-11-01 17:00 ` Eric Anholt
2017-11-01 17:31 ` Sean Paul
2017-11-02 8:08 ` [Intel-gfx] " Jani Nikula
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.