Sign the code available in master

[nbraud]

@myhro suggested signing the release tags in #278. I would like to suggest going one step further, and keeping the code in master signed.

There are several reasons to do so:

• users of the master branch can still be confident (to a large extent) that they obtained code you published (however, Github could serve older (but signed!) code);
• having the master branch signed gives you confidence, when tagging a release, that you are signing the code you authored (or reviewed).

One way to do it, that I use and some other projects adopted goes as follows:

• work as usual, without signing, in a non-master branch;
• when you want to merge your changes in master, do a signed merge: git merge -S --no-ff feature-branch;
• same thing after having reviewed and accepted a pull request;
• when updating your copy of master, use git pull [--ff-only] --verify-signatures to ensure that no untrusted code can be sneaked in.

Some important points:

• It's not strictly required to work on separate (non-master) branches, as long as the tip of the master branch (on Github) is signed.
• This ties together (by convention) signing and reviewing.
• This doesn't require to always have the key material available: when developing from an untrusted machine, the process for a regular PR from an external contributor can be followed.
• Signing code that is not meant to be merged in master can actually be a security problem:
  if somebody who is trusted for reviewing/merging branches publishes a development branch with signed commits, the adversary can serve any of the signed commit instead of the development branch.

[myhro]

Hi Nicolas,

I see that you are trying to find a reasonable approach between signing every commit and signing only tags/releases. Although I do agree that there are reasons to do this, it makes a simple process (merging new code) so complicated that it won't worth the effort. Not to mention that this would impact our current workflow, as it will prevent us from merging new code using the GitHub Web UI (something that I don't know if @andsens is used to, but I do).

Of course I'd like to hear Anders opinions about it, but to me this looks like a ton of bureaucracy (signing and verifying signatures) that I would prefer to avoid. The example that you gave us is about a privacy tool that have way tighter requirements about what is commited to their repository than ours.

Regards,
Tiago.

P.s.: of course any of us can be tricked to merge malicious code - and a signed branch won't prevent that. I'm much more worried about my GitHub account security (I've enabled two-factor authentication as soon as I got write access to bootstrap-vz repository, for instance) here than anything else.

[andsens]

I have been a bit lazy about getting my commits signed (sorry!). At the very least we should sign release tags, but it isn't very often that we tag anything, people usually just pull the latest master.

I love the ease with which we can merge new PRs right through the GH interface and am very hesitant to give up that convenience. I could definitely start signing my own commits, this would at least allow for verification up to that point. Any subsequent PR merges would of course still go unsigned.

There is of course stuff like this. I could make it more convenient to merge PRs through the CLI, then maybe it wont be as big of a hassle compared to pressing the "Merge" button in the WebUI.

[nbraud]

@andsens @myhro It's pretty easy to make Github PRs available as git refs:

# .git/config
[remote "origin"]
    url = https://github.com/andsens/bootstrap-vz.git
    fetch = +refs/heads/*:refs/remotes/origin/*
    fetch = +refs/pull/*/head:refs/remotes/origin/pull/*  # This is the extra line

Then, merging a given PR basically goes like this:

% git fetch

% git merge -S --no-ff origin/pull/335
Waiting for Emacs...
Merge made by the 'recursive' strategy.
 bootstrapvz/base/__init__.py                        |  6 ++++--
 bootstrapvz/base/manifest.py                        |  6 +++---
 bootstrapvz/common/tools.py                         |  9 +++++++++
 bootstrapvz/plugins/admin_user/__init__.py          | 18 ++++++++++--------
 bootstrapvz/plugins/admin_user/manifest-schema.yml  |  6 +++---
 bootstrapvz/plugins/admin_user/tasks.py             | 17 ++++++++++++-----
 bootstrapvz/plugins/ansible/__init__.py             |  4 ++--
 bootstrapvz/plugins/apt_proxy/__init__.py           |  4 ++--
 bootstrapvz/plugins/chef/__init__.py                |  4 ++--
 bootstrapvz/plugins/cloud_init/__init__.py          |  6 ++----
 bootstrapvz/plugins/commands/__init__.py            |  6 ++----
 bootstrapvz/plugins/debconf/__init__.py             |  6 +++---
 bootstrapvz/plugins/docker_daemon/__init__.py       | 11 ++++++-----
 bootstrapvz/plugins/ec2_launch/__init__.py          |  4 ++--
 bootstrapvz/plugins/ec2_publish/__init__.py         |  4 ++--
 bootstrapvz/plugins/file_copy/README.rst            |  2 +-
 bootstrapvz/plugins/file_copy/__init__.py           | 11 ++++++-----
 bootstrapvz/plugins/file_copy/tasks.py              |  9 ++++++---
 bootstrapvz/plugins/google_cloud_repo/__init__.py   |  4 ++--
 bootstrapvz/plugins/minimize_size/__init__.py       | 11 +++++++----
 bootstrapvz/plugins/ntp/__init__.py                 |  4 ++--
 bootstrapvz/plugins/pip_install/__init__.py         |  4 ++--
 bootstrapvz/plugins/prebootstrapped/__init__.py     |  4 ++--
 bootstrapvz/plugins/puppet/__init__.py              |  4 ++--
 bootstrapvz/plugins/root_password/__init__.py       |  6 ++----
 bootstrapvz/plugins/salt/__init__.py                |  4 ++--
 bootstrapvz/plugins/unattended_upgrades/__init__.py |  6 ++----
 bootstrapvz/plugins/vagrant/__init__.py             |  4 ++--
 bootstrapvz/providers/azure/__init__.py             |  4 ++--
 bootstrapvz/providers/docker/__init__.py            |  4 ++--
 bootstrapvz/providers/ec2/__init__.py               | 28 ++++++++++++++++------------
 bootstrapvz/providers/gce/__init__.py               |  4 ++--
 bootstrapvz/providers/kvm/__init__.py               |  4 ++--
 bootstrapvz/providers/oracle/__init__.py            | 16 +++++++++-------
 bootstrapvz/providers/virtualbox/__init__.py        |  4 ++--
 docs/developers/plugins.rst                         |  4 ++--
 36 files changed, 139 insertions(+), 113 deletions(-)

% git show --show-signature -q
commit a8922a2207ecfb765c86245dcc49872133370975
gpg: Signature made Sun 14 Aug 2016 02:33:46 PM CEST
gpg:                using RSA key 0x9D4F88010CFE19E3
gpg: please do a --check-trustdb
gpg: Good signature from "Nicolas Braud-Santoni <[email protected]>" [ultimate]
gpg:                 aka "Nicolas Braud-Santoni <[email protected]>" [ultimate]
gpg:                 aka "Nicolas Braud-Santoni <[email protected]>" [ultimate]
gpg:                 aka "Nicolas Braud-Santoni <[email protected]>" [ultimate]
Primary key fingerprint: 772B 11B4 F2DC 80E1 212B  3F41 B073 9AAD 91B7 CDC0
     Subkey fingerprint: 8961 1B14 A136 87FB 354A  924F 9D4F 8801 0CFE 19E3
Merge: e0d9238 1adfd46
Author: Nicolas Braud-Santoni <[email protected]>
Date:   Sun Aug 14 14:32:41 2016 +0200

    Merge PR 335

    admin_user & file_copy: Make paths relative to the manifest

% git push

My main annoyances with this are:

• The default merge message isn't super useful;
• You end up with bazillions of remote refs (since the PR refs stay forever in that location), but since they don't show up on git branch and friends it's a very minor thing.
  It's possible, instead of making all PRs available, to fetch a single PR with git fetch origin refs/pull/335/head and then git merge --no-ff -S FETCH_HEAD, but that's a tad cryptic (and shell autocompletion cannot cope with remote refs, obviously).

[nbraud]

@myhro I'm sorry I gave the impression this is complicated.
As you pointed out, this makes it impossible to merge from Github's UI, but I would argue that the alternative workflow isn't too cumbersome.
Obviously, since we are talking about the workflow used by @andsens and you, this is only a suggestion (though I hope it's an helpful one).

[andsens]

(though I hope it's an helpful one).

Indeed it is! I'm in the process of getting myself a YUBI key, one way to try it out would definitely be git signing :-)