diff --git a/config/test.exs b/config/test.exs index 6ff8afa4d..3ddb002cb 100644 --- a/config/test.exs +++ b/config/test.exs @@ -28,4 +28,6 @@ config :bors, :server, BorsNG.GitHub.ServerMock config :bors, :oauth2, BorsNG.GitHub.OAuth2Mock config :bors, :is_test, true +config :bors, :test_gpg_key_id, {:system, "BORS_TEST_GPG_KEY_ID", ""} + config :bors, :celebrate_new_year, false diff --git a/lib/github/github.ex b/lib/github/github.ex index e913d5fa6..6f74fd00b 100644 --- a/lib/github/github.ex +++ b/lib/github/github.ex @@ -198,12 +198,12 @@ defmodule BorsNG.GitHub do parents: [bitstring], commit_message: bitstring, committer: tcommitter | nil - }) :: binary - def synthesize_commit!(repo_conn, info) do + }, bitstring | nil) :: binary + def synthesize_commit!(repo_conn, info, signing_key \\ nil) do {:ok, sha} = GenServer.call( BorsNG.GitHub, - {:synthesize_commit, repo_conn, {info}}, + {:synthesize_commit, repo_conn, {info, signing_key}}, Confex.fetch_env!(:bors, :api_github_timeout) ) @@ -214,13 +214,14 @@ defmodule BorsNG.GitHub do tree: bitstring, parents: [bitstring], commit_message: bitstring, + author: tcommitter | nil, committer: tcommitter | nil - }) :: binary - def create_commit!(repo_conn, info) do + }, bitstring | nil) :: binary + def create_commit!(repo_conn, info, signing_key \\ nil) do {:ok, sha} = GenServer.call( BorsNG.GitHub, - {:create_commit, repo_conn, {info}}, + {:create_commit, repo_conn, {info, signing_key}}, Confex.fetch_env!(:bors, :api_github_timeout) ) diff --git a/lib/github/github/server.ex b/lib/github/github/server.ex index 6b38d5841..5e93d1181 100644 --- a/lib/github/github/server.ex +++ b/lib/github/github/server.ex @@ -282,8 +282,9 @@ defmodule BorsNG.GitHub.Server do tree: tree, parents: parents, commit_message: commit_message, + author: author, committer: committer - }} + }, signing_key} ) do msg = %{parents: parents, tree: tree, message: commit_message} @@ -291,12 +292,36 @@ defmodule BorsNG.GitHub.Server do if is_nil(committer) do msg else - Map.put(msg, "author", %{ + Map.put(msg, :committer, %{ name: committer.name, email: committer.email }) end + msg = + if is_nil(author) do + msg + else + Map.put(msg, :author, author) + end + + msg = + cond do + is_nil(signing_key) -> + Logger.debug("Not signing commit in create_commit because signing_key is nil") + msg + is_nil(author) -> + Logger.debug("Not signing commit in create_commit because author is nil") + msg + is_nil(committer) -> + Logger.debug("Not signing commit in create_commit because committer is nil") + msg + true -> + msg + |> GitHub.Signature.add_timestamp(DateTime.utc_now()) + |> GitHub.Signature.sign!(signing_key) + end + resp = repo_conn |> post!("git/commits", Poison.encode!(msg)) @@ -333,7 +358,7 @@ defmodule BorsNG.GitHub.Server do parents: parents, commit_message: commit_message, committer: committer - }} + }, signing_key} ) do msg = %{parents: parents, tree: tree, message: commit_message} @@ -341,10 +366,26 @@ defmodule BorsNG.GitHub.Server do if is_nil(committer) do msg else - Map.put(msg, "author", %{ + msg + |> Map.put(:author, %{ name: committer.name, email: committer.email }) + |> Map.put(:committer, committer) + end + + msg = + cond do + is_nil(signing_key) -> + Logger.debug("Not signing commit in synthesize_commit because signing_key is nil") + msg + is_nil(committer) -> + Logger.debug("Not signing commit in synthesize_commit because committer is nil") + msg + true -> + msg + |> GitHub.Signature.add_timestamp(DateTime.utc_now()) + |> GitHub.Signature.sign!(signing_key) end repo_conn diff --git a/lib/github/github/signature.ex b/lib/github/github/signature.ex new file mode 100644 index 000000000..d9af10f10 --- /dev/null +++ b/lib/github/github/signature.ex @@ -0,0 +1,98 @@ +require Logger + +defmodule BorsNG.GitHub.Signature do + @moduledoc """ + Provides the ability to sign commits using gpg2. + """ + + @type tcommit_no_ts :: %{ + parents: [binary], + tree: binary, + message: binary, + author: %{name: binary, email: binary}, + committer: %{name: binary, email: binary} + } + + @type tcommit :: %{ + parents: [binary], + tree: binary, + message: binary, + author: %{name: binary, email: binary, date: binary}, + committer: %{name: binary, email: binary, date: binary} + } + + @type tcommit_sig :: %{ + parents: [binary], + tree: binary, + message: binary, + author: %{name: binary, email: binary, date: binary}, + committer: %{name: binary, email: binary, date: binary}, + signature: binary + } + + @spec add_timestamp(tcommit_no_ts, DateTime.t) :: tcommit + def add_timestamp(commit, dt) do + ts = DateTime.to_iso8601(dt) + commit + |> Map.put(:author, Map.put(commit[:author], :date, ts)) + |> Map.put(:committer, Map.put(commit[:committer], :date, ts)) + end + + @spec format_commit(tcommit) :: binary + def format_commit(commit) do + {:ok, author_date, _} = DateTime.from_iso8601(commit[:author][:date]) + author_ts = DateTime.to_unix(author_date, :second) |> Integer.to_string() + {:ok, committer_date, _} = DateTime.from_iso8601(commit[:committer][:date]) + committer_ts = DateTime.to_unix(committer_date, :second) |> Integer.to_string() + gpg_sig = + case Map.fetch(commit, :signature) do + {:ok, sig} -> + lines = String.split(sig, "\n") + |> Enum.intersperse("\n ") + |> Enum.to_list() + ["gpgsig ", lines, "\n"] + :error -> + [] + end + + IO.iodata_to_binary([ + ["tree ", commit[:tree], "\n"], + Enum.map(commit[:parents], fn parent -> ["parent ", parent, "\n"] end), + ["author ", commit[:author][:name], " <", commit[:author][:email], "> ", author_ts, " +0000\n"], + ["committer ", commit[:committer][:name], " <", commit[:committer][:email], "> ", committer_ts, " +0000\n"], + gpg_sig, + "\n", + commit[:message], + ]) + end + + @spec sign!(tcommit, binary) :: tcommit_sig + def sign!(commit, key_id) do + Logger.info("Signing commit #{inspect(commit)} with key #{inspect(key_id)}") + + path = System.find_executable("gpg") + if is_nil(path) do + throw :missing_gpg + end + + commit_to_sign = format_commit(commit) + Logger.debug("Commit to sign: #{inspect commit_to_sign}") + + tmp_dir = System.tmp_dir!() + tmp_filename = Path.join(tmp_dir, "bors_commit_signing.#{commit[:tree]}.txt") + sig_filename = Path.join(tmp_dir, "bors_commit_signing.#{commit[:tree]}.txt.asc") + try do + _ = File.rm(sig_filename) + File.write!(tmp_filename, commit_to_sign, [:write, :binary, :sync]) + args = ["--batch", "--with-colons", "--status-fd", "2", "--armor", "--local-user", key_id, "--detach-sign", tmp_filename] + Logger.debug("Calling #{path} #{Enum.join(args, " ")}") + {output, 0} = System.cmd(path, args) + Logger.debug("Output from gpg: #{inspect output}") + sig = File.read!(sig_filename) + Map.put(commit, :signature, sig) + after + _ = File.rm(tmp_filename) + _ = File.rm(sig_filename) + end + end +end diff --git a/lib/worker/batcher.ex b/lib/worker/batcher.ex index 350f0b367..bada0e7ef 100644 --- a/lib/worker/batcher.ex +++ b/lib/worker/batcher.ex @@ -395,6 +395,17 @@ defmodule BorsNG.Worker.Batcher do batch.into_branch ) + toml = repo_conn + |> Batcher.GetBorsToml.get(base.commit) + |> case do + {:ok, toml} -> toml + {:error, message} -> + message = Batcher.Message.generate_bors_toml_error(message) + patches = Enum.map(patch_links, & &1.patch) + send_message(repo_conn, patches, {:config, message}) + nil + end + tbase = %{ tree: base.tree, commit: @@ -406,7 +417,12 @@ defmodule BorsNG.Worker.Batcher do parents: [base.commit], commit_message: "[ci skip][skip ci][skip netlify]", committer: nil - } + }, + if is_nil(toml) do + nil + else + toml.signing_key + end ) } @@ -535,8 +551,10 @@ defmodule BorsNG.Worker.Batcher do tree: merge_commit.tree, parents: [prev_head], commit_message: commit_message, - committer: %{name: user.name || user.login, email: user_email} - } + author: %{name: user.name || user.login, email: user_email}, + committer: toml.committer + }, + toml.signing_key ) Logger.info("Commit Sha #{inspect(cpt)}") @@ -574,7 +592,8 @@ defmodule BorsNG.Worker.Batcher do parents: parents, commit_message: commit_message, committer: toml.committer - } + }, + toml.signing_key ) end diff --git a/lib/worker/batcher/bors_toml.ex b/lib/worker/batcher/bors_toml.ex index bce94125b..29a9d68ee 100644 --- a/lib/worker/batcher/bors_toml.ex +++ b/lib/worker/batcher/bors_toml.ex @@ -30,7 +30,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do use_codeowners: false, committer: nil, commit_title: "Merge ${PR_REFS}", - update_base_for_deletes: false + update_base_for_deletes: false, + signing_key: nil @type tcommitter :: %{ name: binary, @@ -51,7 +52,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do use_codeowners: boolean, committer: tcommitter, commit_title: binary, - update_base_for_deletes: boolean + update_base_for_deletes: boolean, + signing_key: binary | nil } @type err :: @@ -81,18 +83,18 @@ defmodule BorsNG.Worker.Batcher.BorsToml do committer = Map.get(toml, "committer", nil) - committer = + {committer, signing_key} = case committer do nil -> - nil + {nil, nil} _ -> c = to_map(committer) - %{ + {%{ name: Map.get(c, "name", nil), email: Map.get(c, "email", nil) - } + }, Map.get(c, "signing_key", nil)} end toml = %BorsNG.Worker.Batcher.BorsToml{ @@ -124,7 +126,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do ), committer: committer, commit_title: Map.get(toml, "commit_title", "Merge ${PR_REFS}"), - update_base_for_deletes: Map.get(toml, "update_base_for_deletes", false) + update_base_for_deletes: Map.get(toml, "update_base_for_deletes", false), + signing_key: signing_key, } case toml do @@ -159,6 +162,9 @@ defmodule BorsNG.Worker.Batcher.BorsToml do %{commit_title: msg} when not is_binary(msg) and not is_nil(msg) -> {:error, :commit_title} + %{signing_key: k} when not is_binary(k) and not is_nil(k) -> + {:error, :signing_key} + toml -> status = toml.status diff --git a/test/github_signature_test.exs b/test/github_signature_test.exs new file mode 100644 index 000000000..60bc32b87 --- /dev/null +++ b/test/github_signature_test.exs @@ -0,0 +1,61 @@ +require Logger + +defmodule BorsNG.GitHub.GitHubSignatureTest do + use BorsNG.ConnCase + alias BorsNG.GitHub.Signature + + test "can gpg-sign a commit" do + key_id = Confex.fetch_env!(:bors, :test_gpg_key_id) + cond do + is_nil(System.find_executable("gpg")) -> + Logger.info("Skipping GPG signing test because gpg is not installed") + key_id == "" -> + Logger.info("Skipping GPG signing test because test key was not set (see `config/test.exs`)") + true -> + date = DateTime.utc_now() + commit_to_sign = %{ + tree: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + parents: ["bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"], + author: %{ + name: "Author Name", + email: "author@example.com", + date: DateTime.to_iso8601(date), + }, + committer: %{ + name: "Committer Name", + email: "committer@example.com", + date: DateTime.to_iso8601(date), + }, + message: "Example commit" + } + + commit_with_sig = Signature.sign!(commit_to_sign, key_id) + raw_with_sig = Signature.format_commit(commit_with_sig) + + # Create a temporary git repo, insert the raw commit, and verify it with + # `git-verify-commit`. + git = System.find_executable("git") + tmp_dir = Path.join(System.tmp_dir!(), "git-signature-test.#{Enum.random(10000..99999)}") + :ok = File.mkdir!(tmp_dir) + try do + # create empty git repo + Logger.debug("Creating temporary git repository at #{tmp_dir}") + {_, 0} = System.cmd(git, ["init"], cd: tmp_dir, stderr_to_stdout: true) + # insert raw commit object + Logger.debug("Inserting raw commit object") + raw_obj_path = Path.join(tmp_dir, "raw-commit.txt") + :ok = File.write!(raw_obj_path, raw_with_sig, [:write, :binary, :sync]) + {raw_hash, 0} = System.cmd( + git, ["hash-object", "-t", "commit", "-w", raw_obj_path], + cd: tmp_dir, stderr_to_stdout: true + ) + hash = String.trim(raw_hash) + # verify commit + Logger.debug("Verifying commit #{inspect hash}") + {_, 0} = System.cmd(git, ["verify-commit", "--verbose", hash], cd: tmp_dir) + after + File.rm_rf!(tmp_dir) + end + end + end +end