feat: support x509 commit signing (#2844)

* feat: support x509 commit signing

* fix changelog

* add e2e test

---------

Co-authored-by: extrawurst <776816+extrawurst@users.noreply.github.com>
Co-authored-by: rusticorn <mail@rusticorn.com>

Author: kaden-l-nelson

.github/workflows/ci.yml

@@ -53,6 +53,15 @@ jobs:
         run: |
           cargo build
 
+      # gpgsm: needed by the x509 signing e2e test (unix-only)
+      - name: Install commit-signing tools (linux)
+        if: matrix.os == 'ubuntu-latest'
+        run: sudo apt-get update && sudo apt-get install -y gpgsm
+
+      - name: Install commit-signing tools (macos)
+        if: matrix.os == 'macos-latest'
+        run: brew install gnupg
+
       - name: Run tests
         run: make test
 
@@ -128,6 +137,9 @@ jobs:
       - name: Setup MUSL
         run: |
           sudo apt-get -qq install musl-tools
+      # gpgsm: needed by the x509 signing e2e test
+      - name: Install commit-signing tools
+        run: sudo apt-get -qq install gpgsm
       - name: Build Debug
         run: |
           make build-linux-musl-debug

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Added
+* support x509 commit signing [[@kaden-l-nelson](https://github.com/kaden-l-nelson)] ([#2514](https://github.com/gitui-org/gitui/issues/2514))
+
 ### Changed
 * use [tombi](https://github.com/tombi-toml/tombi) for all toml file formatting
 * open the external editor from the status diff view [[@WaterWhisperer](https://github.com/WaterWhisperer)] ([#2805](https://github.com/gitui-org/gitui/issues/2805))

asyncgit/src/sync/sign.rs

@@ -145,16 +145,24 @@ impl SignBuilder {
 		// Variants are described in the git config documentation
 		// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgformat
 		match format.as_str() {
-			"openpgp" => {
+			"openpgp" | "x509" => {
 				// Try to retrieve the gpg program from the git configuration,
 				// moving from the least to the most specific config key,
 				// defaulting to "gpg" if nothing is explicitly defined (per git's implementation)
 				// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgprogram
-				// https://git-scm.com/docs/git-config#Documentation/git-config.txt-gpgprogram
 				let program = config
-					.get_string("gpg.openpgp.program")
+					.get_string(
+						format!("gpg.{format}.program").as_str(),
+					)
 					.or_else(|_| config.get_string("gpg.program"))
-					.unwrap_or_else(|_| "gpg".to_string());
+					.unwrap_or_else(|_| {
+						(if format == "x509" {
+							"gpgsm"
+						} else {
+							"gpg"
+						})
+						.to_string()
+					});
 
 				// Optional signing key.
 				// If 'user.signingKey' is not set, we'll use 'user.name' and 'user.email'
@@ -184,9 +192,6 @@ impl SignBuilder {
 					signing_key,
 				}))
 			}
-			"x509" => Err(SignBuilderError::MethodNotImplemented(
-				String::from("x509"),
-			)),
 			"ssh" => {
 				let ssh_signer = config
 					.get_string("user.signingKey")
@@ -374,6 +379,8 @@ mod tests {
 	use super::*;
 	use crate::error::Result;
 	use crate::sync::tests::repo_init_empty;
+	#[cfg(unix)]
+	use serial_test::serial;
 
 	#[test]
 	fn test_invalid_signing_format() -> Result<()> {
@@ -471,4 +478,269 @@ mod tests {
 
 		Ok(())
 	}
+
+	#[test]
+	fn test_x509_program_defaults() -> Result<()> {
+		let (_tmp_dir, repo) = repo_init_empty()?;
+
+		{
+			let mut config = repo.config()?;
+			config.set_str("gpg.format", "x509")?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		// default x509 program should be gpgsm
+		assert_eq!("gpgsm", sign.program());
+		// default signing key should be "name <email>" when not specified
+		assert_eq!("name <email>", sign.signing_key());
+
+		Ok(())
+	}
+
+	#[test]
+	fn test_x509_program_configs() -> Result<()> {
+		let (_tmp_dir, repo) = repo_init_empty()?;
+
+		{
+			let mut config = repo.config()?;
+			config.set_str("gpg.format", "x509")?;
+			config.set_str("gpg.program", "GPG_PROGRAM_TEST")?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		// we get gpg.program, because gpg.x509.program is not set
+		assert_eq!("GPG_PROGRAM_TEST", sign.program());
+
+		{
+			let mut config = repo.config()?;
+			config.set_str(
+				"gpg.x509.program",
+				"GPG_X509_PROGRAM_TEST",
+			)?;
+		}
+
+		let sign =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+
+		// since gpg.x509.program is now set as well, it is more specific than
+		// gpg.program and therefore takes precedence
+		assert_eq!("GPG_X509_PROGRAM_TEST", sign.program());
+
+		Ok(())
+	}
+
+	/// e2e x509 signing: set up a throwaway `gpgsm` identity, sign a real
+	/// commit and verify it. Serial + unix-only: uses a process-wide `GNUPGHOME`.
+	#[cfg(unix)]
+	#[test]
+	#[serial]
+	fn test_x509_sign_and_verify_e2e() -> Result<()> {
+		use std::os::unix::fs::PermissionsExt;
+		use std::process::Command;
+
+		// note: openssl wants `version`, not `--version`
+		fn tool_available(bin: &str, version_arg: &str) -> bool {
+			Command::new(bin)
+				.arg(version_arg)
+				.stdout(std::process::Stdio::null())
+				.stderr(std::process::Stdio::null())
+				.status()
+				.map(|s| s.success())
+				.unwrap_or(false)
+		}
+
+		assert!(
+			tool_available("gpgsm", "--version"),
+			"gpgsm is required for the x509 e2e test"
+		);
+		assert!(
+			tool_available("openssl", "version"),
+			"openssl is required for the x509 e2e test"
+		);
+
+		let email = "gitui-x509-test@example.com";
+		let gnupg = tempfile::tempdir()?;
+		let home = gnupg.path();
+		std::fs::set_permissions(
+			home,
+			std::fs::Permissions::from_mode(0o700),
+		)?;
+
+		// pinentry that OKs everything: empty passphrase + auto-trust, no tty.
+		let pinentry = home.join("fake-pinentry.sh");
+		std::fs::write(
+			&pinentry,
+			"#!/bin/sh\necho \"OK ready\"\nwhile read -r cmd; do\n  echo OK\n  [ \"$cmd\" = BYE ] && exit 0\ndone\n",
+		)?;
+		std::fs::set_permissions(
+			&pinentry,
+			std::fs::Permissions::from_mode(0o700),
+		)?;
+		std::fs::write(
+			home.join("gpg-agent.conf"),
+			format!(
+				"allow-loopback-pinentry\npinentry-program {}\n",
+				pinentry.display()
+			),
+		)?;
+
+		// GPGSign inherits env, so point the child gpgsm at our keyring.
+		std::env::set_var("GNUPGHOME", home);
+
+		let run = |program: &str, args: &[&str]| {
+			let out = Command::new(program)
+				.args(args)
+				.env("GNUPGHOME", home)
+				.output()
+				.unwrap_or_else(|e| {
+					panic!("failed to run {program}: {e}")
+				});
+			assert!(
+				out.status.success(),
+				"{program} {args:?} failed: {}",
+				String::from_utf8_lossy(&out.stderr)
+			);
+			out
+		};
+
+		let key = home.join("key.pem");
+		let cert = home.join("cert.pem");
+		let p12 = home.join("bundle.p12");
+		run(
+			"openssl",
+			&[
+				"req",
+				"-x509",
+				"-newkey",
+				"rsa:2048",
+				"-nodes",
+				"-keyout",
+				key.to_str().unwrap(),
+				"-out",
+				cert.to_str().unwrap(),
+				"-days",
+				"3650",
+				"-subj",
+				&format!("/CN=gitui test/emailAddress={email}"),
+			],
+		);
+		run(
+			"openssl",
+			&[
+				"pkcs12",
+				"-export",
+				"-inkey",
+				key.to_str().unwrap(),
+				"-in",
+				cert.to_str().unwrap(),
+				"-out",
+				p12.to_str().unwrap(),
+				"-passout",
+				"pass:",
+				// legacy PBE: gpgsm can't read OpenSSL 3's default PBES2/AES.
+				"-keypbe",
+				"PBE-SHA1-3DES",
+				"-certpbe",
+				"PBE-SHA1-3DES",
+				"-macalg",
+				"sha1",
+			],
+		);
+		run(
+			"gpgsm",
+			&[
+				"--batch",
+				"--pinentry-mode",
+				"loopback",
+				"--passphrase",
+				"",
+				"--import",
+				p12.to_str().unwrap(),
+			],
+		);
+
+		// trust our self-signed root ("S" relaxes CA checks) so gpgsm will sign.
+		let listing = run(
+			"gpgsm",
+			&["--batch", "--with-colons", "--list-secret-keys"],
+		);
+		let listing = String::from_utf8_lossy(&listing.stdout);
+		let fingerprint = listing
+			.lines()
+			.filter_map(|line| line.strip_prefix("fpr:"))
+			.find_map(|rest| {
+				rest.split(':').find(|field| {
+					field.len() == 40
+						&& field
+							.bytes()
+							.all(|b| b.is_ascii_hexdigit())
+				})
+			})
+			.expect("could not determine cert fingerprint");
+		std::fs::write(
+			home.join("trustlist.txt"),
+			format!("{fingerprint} S\n"),
+		)?;
+		// reload gpg-agent to read the new trustlist
+		run("gpgconf", &["--kill", "gpg-agent"]);
+
+		let (_tmp_dir, repo) = repo_init_empty()?;
+		{
+			let mut config = repo.config()?;
+			config.set_str("gpg.format", "x509")?;
+			config.set_str("user.signingKey", email)?;
+		}
+		let signer =
+			SignBuilder::from_gitconfig(&repo, &repo.config()?)?;
+		assert_eq!("gpgsm", signer.program());
+
+		let sig = git2::Signature::now("gitui test", email)?;
+		let tree = {
+			let mut index = repo.index()?;
+			let tree_id = index.write_tree()?;
+			repo.find_tree(tree_id)?
+		};
+		let commit_id = create_signed_commit(
+			&repo,
+			&*signer,
+			&sig,
+			&sig,
+			"x509 signed commit",
+			&tree,
+			&[],
+		)?;
+
+		let (signature, signed_data) =
+			repo.extract_signature(&commit_id, None)?;
+		let signature = std::str::from_utf8(&signature).unwrap();
+		assert!(
+			signature.contains("BEGIN SIGNED MESSAGE"),
+			"expected an armored CMS signature, got: {signature}"
+		);
+
+		let sig_file = home.join("commit.sig");
+		let data_file = home.join("commit.data");
+		std::fs::write(&sig_file, signature)?;
+		std::fs::write(&data_file, &*signed_data)?;
+		let verify = run(
+			"gpgsm",
+			&[
+				"--verify",
+				sig_file.to_str().unwrap(),
+				data_file.to_str().unwrap(),
+			],
+		);
+		let verify_err = String::from_utf8_lossy(&verify.stderr);
+		assert!(
+			verify_err.contains("Good signature"),
+			"gpgsm did not accept the signature: {verify_err}"
+		);
+
+		std::env::remove_var("GNUPGHOME");
+		Ok(())
+	}
 }