GitHub: Signing commits

Continuing the discussion from GitHub API Key Leak, May 16 2016:

After GitHub now supports git signed tags what about signing git commits locally to prevent such potentially attacks?
If all commits are signed and only new ones pushed by a user are not, this will be conspicuous.

What if there are people outside letsencrypt who want to contribute and can’t use gpg because of regulations or something or simple don’t want to use there private email to sign commits for a public repository? Think about the folk from the HackTeam where GitHub connected the repo with there private profiles :wink:

Basically you can’t enforce to commit and merge only signed. An attacker in this case had the ability to commit, open a PR, merge it to (master, release and staging branch) and to close it seconds before LE starts to upgrade there systems which is publicly announced.

But therefor a white hats like me where report such incidents to the people responsible for it. ISRG revoked the api key, committed the fix and merged it to the master branch within 2 hours and 18 minutes with https://github.com/letsencrypt/boulder/pull/1827 which is fast I think.

The merge commit can be signed, no?

There is no merge commit. You open a Pull Request which is basically a request to pull a commit from another git repository in the git repository where you open the request. If this commit isn’t signed then the commit which is merged into the specified branch is also not signed :slight_smile:
Answer this your question?

Merge commits (I.e., commits which have two parents) can be signed.

Having all people sign their own commits won’t help, as you also need their public keys (verified). It should be sufficient if the commits of the LE staff are signed (their regular commits or only their merge commits (after they carefully reviewed the PR) as the policy is to only accept merge request in a non-fast forward way).

Also, it is possible to sign tags. This way one does not need to review all old commits, but only since the last release/signed tag.

Thanks nice to know :slight_smile:

I agree with booth ideas. It should sufficient to sign the merge commit to verify the integrity of the code.
If the tags are signed with an official email from for example signing@letsencrypt.org everybody can verify the integrity from this release/tag. Also the staff during the pull on the servers before they restart the boulder process. This also prevent my worst case I explained above.

Any plans to sign the code in the main repository? @jsha @josh

We’ve talked about it and sketched out some designs. We may do it, but as the discussion above shows, it’s not trivial. For instance, right now we squash merge pull requests through GitHub’s UI. Since these commits can’t be signed by GitHub, we need to write a local tool to pull down a PR’s content, review it one last time, sign it locally, and push it up. Additionally, to avoid mistakes, we need some sort of tooling to prevent unsigned commits from landing in the repo. I still think it’s worthwhile, just hasn’t risen to the top of our priority list yet.

2 Likes