[RFC PATCH] gpg: add support for gpgsm

Detect the gpgsm block header and run this command instead of gpg.
On the signing side, ask gpgsm if it knows the signing key we're trying
to use and fall back to gpg if it does not.

This lets the user more easily combine signing and verifying X509 and
PGP signatures without having to choose a default for a particular
repository that may need to be occasionally overridden.

Signed-off-by: Carlos Martín Nieto <redacted>

---

Out there in the so-called "real world", companies like using X509 to
sign things. Currently you can set 'gpg.program' to gpgsm to get
gpg-compatible verification, but if you're changing it to swap between
PGP and X509, it's an extra variable to keep in mind when working with
signed commits and tags.

While this does let us sign and verify, the probing is a bit
awkward. gpgsm returns 0 regardless of whether it found the key, and if
you pass in an id for which you have the public key, it'll still output the
filename as a heading, so we would consider it known. I'm not aware of a
way around that which doesn't involve parsing the output, which would
probably be even more fragile.

 Documentation/config.txt | 11 ++++++++
 gpg-interface.c          | 65 ++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2cd6bdd..40f3912 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1484,6 +1484,17 @@ gpg.program::
 	signed, and the program is expected to send the result to its
 	standard output.
 
+gpgsm.program::
+	Use this custom program instead of "gpgsm" found on $PATH when
+	making or verifying a gpsm signature. The program must support the
+	same command-line interface as GPG, namely, to verify a detached
+	signature, "gpgsm --verify $file - <$signature" is run, and the
+	program is expected to signal a good signature by exiting with
+	code 0, and to generate an ASCII-armored detached signature, the
+	standard input of "gpgsm -bsau $key" is fed with the contents to be
+	signed, and the program is expected to send the result to its
+	standard output.
+
 gui.commitMsgWidth::
 	Defines how wide the commit message window is in the
 	linkgit:git-gui[1]. "75" is the default.

diff --git a/gpg-interface.c b/gpg-interface.c
index 3dc2fe3..194a6c6 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c

@@ -6,10 +6,13 @@
 
 static char *configured_signing_key;
 static const char *gpg_program = "gpg";
+static const char *gpgsm_program = "gpgsm";
 
 #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
 #define PGP_MESSAGE "-----BEGIN PGP MESSAGE-----"
 
+#define GPGSM_MESSAGE "-----BEGIN SIGNED MESSAGE-----"
+
 void signature_check_clear(struct signature_check *sigc)
 {
 	free(sigc->payload);

@@ -24,6 +27,20 @@ void signature_check_clear(struct signature_check *sigc)
 	sigc->key = NULL;
 }
 
+/*
+ * Guess which program this signature was made with based on the block start.
+ * Right now we just detect a gpgsm block and fall back to gpg otherwise.
+ */
+static const char *guess_program(const char *message, size_t message_len)
+{
+	size_t gpgsm_len = strlen(GPGSM_MESSAGE);
+
+	if (message_len > gpgsm_len && !strncmp(message, GPGSM_MESSAGE, gpgsm_len))
+		return gpgsm_program;
+
+	return gpg_program;
+}
+
 static struct {
 	char result;
 	const char *check;

@@ -131,6 +148,11 @@ int git_gpg_config(const char *var, const char *value, void *cb)
 			return config_error_nonbool(var);
 		gpg_program = xstrdup(value);
 	}
+	if (!strcmp(var, "gpgsm.program")) {
+		if (!value)
+			return config_error_nonbool(var);
+		gpgsm_program = xstrdup(value);
+	}
 	return 0;
 }
 

@@ -142,6 +164,41 @@ const char *get_signing_key(void)
 }
 
 /*
+ * Try to figure out if the given program contains given the key. Both
+ * gpg and gpgsm have keys in hex format, so we don't necessarily know
+ * which one to use.
+ */
+static int program_knows_key(const char *program, const char *signing_key)
+{
+	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct strbuf output = STRBUF_INIT;
+	const char *args[4];
+	size_t len;
+
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = program;
+	args[1] = "-K";
+	args[2] = signing_key;
+	args[3] = NULL;
+
+	if (start_command(&gpg))
+		return error(_("could not run '%s'"), program);
+
+	close(gpg.in);
+	len = strbuf_read(&output, gpg.out, 1024);
+	close(gpg.out);
+
+	/* If the command exits with an error, consider it as not found */
+	if (finish_command(&gpg))
+		return 0;
+
+	/* If the command showed the key we wanted, use it. */
+	return !!len;
+}
+
+/*
  * Create a detached signature for the contents of "buffer" and append
  * it after "signature"; "buffer" and "signature" can be the same
  * strbuf instance, which would cause the detached signature appended

@@ -154,10 +211,14 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	ssize_t len;
 	size_t i, j, bottom;
 
+	if (program_knows_key(gpgsm_program, signing_key))
+		args[0] = gpgsm_program;
+	else
+		args[0] = gpg_program;
+
 	gpg.argv = args;
 	gpg.in = -1;
 	gpg.out = -1;
-	args[0] = gpg_program;
 	args[1] = "-bsau";
 	args[2] = signing_key;
 	args[3] = NULL;

@@ -216,7 +277,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf *pbuf = &buf;
 
-	args_gpg[0] = gpg_program;
+	args_gpg[0] = guess_program(signature, signature_size);
 	fd = git_mkstemp(path, PATH_MAX, ".git_vtag_tmpXXXXXX");
 	if (fd < 0)
 		return error(_("could not create temporary file '%s': %s"),

-- 
2.8.0.rc3

On Thu, Mar 31, 2016 at 03:51:44PM +0200, Carlos Martín Nieto wrote:

Detect the gpgsm block header and run this command instead of gpg.

This part makes sense to me, and is a strict improvement (though
offhand, I wonder if any other systems use the generic "BEGIN SIGNED
MESSAGE" header. The obvious option would be PEM from "openssl smime",
but it is "BEGIN PKCS7").

On the signing side, ask gpgsm if it knows the signing key we're trying
to use and fall back to gpg if it does not.

This part looks like we incur an extra fork/exec each time we sign with
gpg, even if the user doesn't ever want to use gpgsm, or even have it
installed.

I wonder if there are any hints we can use from the key ident, but I
suppose not. In the default config, it comes straight from
$GIT_COMMITTER_*, and is just a name/email.

But maybe we could pull this out to a separate config option, like
"commit.defaultSignatureType", which could be either "gpg", "gpgsm", or
"auto" to enable the behavior you have here.  Then savvy users can pick
the type they plan to use.  We can have a discussion then about whether
to flip the default from "gpg" to "auto", but I'd vote to leave it at
gpg unless gpgsm gets a huge amount of traction, and it really is 50/50
what people would want.

And regardless of the default type for creating signatures, we'd still
automatically verify signatures from either type.

 /*
+ * Try to figure out if the given program contains given the key. Both
+ * gpg and gpgsm have keys in hex format, so we don't necessarily know
+ * which one to use.
+ */
+static int program_knows_key(const char *program, const char *signing_key)
+{
+	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct strbuf output = STRBUF_INIT;
+	const char *args[4];
+	size_t len;
+
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = program;
+	args[1] = "-K";
+	args[2] = signing_key;
+	args[3] = NULL;

I think you'd want to send stderr to /dev/null here, as this is for
speculatively seeing "does the user even have gpgsm set up?".

+
+	if (start_command(&gpg))
+		return error(_("could not run '%s'"), program);

Likewise, most users would start seeing "could not run 'gpgsm'" if they
do not even have it installed.

-Peff

On Thu, 2016-03-31 at 10:22 -0400, Jeff King wrote:

On Thu, Mar 31, 2016 at 03:51:44PM +0200, Carlos Martín Nieto wrote:

quoted

Detect the gpgsm block header and run this command instead of gpg.

This part makes sense to me, and is a strict improvement (though
offhand, I wonder if any other systems use the generic "BEGIN SIGNED
MESSAGE" header. The obvious option would be PEM from "openssl
smime",
but it is "BEGIN PKCS7").

Yep. It's useful when you can tell what generated the signatures.

quoted

On the signing side, ask gpgsm if it knows the signing key we're
trying
to use and fall back to gpg if it does not.

This part looks like we incur an extra fork/exec each time we sign
with
gpg, even if the user doesn't ever want to use gpgsm, or even have it
installed.

Yes, this is unfortunate as I don't know any other way to tell whether
gpgsm (or whatever else) knows about a key. We could try to find gpgsm
in PATH but I suspect this would be expensive as well.

I wonder if there are any hints we can use from the key ident, but I
suppose not. In the default config, it comes straight from
$GIT_COMMITTER_*, and is just a name/email.

Both gpg and gpgsm accept any string and try to match it against the
information in the keys. So even though gpgsm keys are shown by the
program itself with the "0x" prefix, 'gpgsm -k DigiCert' does show me
the public key I have for them.

But maybe we could pull this out to a separate config option, like
"commit.defaultSignatureType", which could be either "gpg", "gpgsm",
or
"auto" to enable the behavior you have here.  Then savvy users can
pick
the type they plan to use.  We can have a discussion then about
whether
to flip the default from "gpg" to "auto", but I'd vote to leave it at
gpg unless gpgsm gets a huge amount of traction, and it really is
50/50
what people would want.

And regardless of the default type for creating signatures, we'd
still
automatically verify signatures from either type.

This makes sense. It allows for the automatic detection if that's
wanted but it would stop us running gpgsm on each invocation.

quoted

 /*
+ * Try to figure out if the given program contains given the key.
Both
+ * gpg and gpgsm have keys in hex format, so we don't necessarily
know
+ * which one to use.
+ */
+static int program_knows_key(const char *program, const char
*signing_key)
+{
+	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct strbuf output = STRBUF_INIT;
+	const char *args[4];
+	size_t len;
+
+	gpg.argv = args;
+	gpg.in = -1;
+	gpg.out = -1;
+	args[0] = program;
+	args[1] = "-K";
+	args[2] = signing_key;
+	args[3] = NULL;

I think you'd want to send stderr to /dev/null here, as this is for
speculatively seeing "does the user even have gpgsm set up?".

quoted

+
+	if (start_command(&gpg))
+		return error(_("could not run '%s'"), program);

Likewise, most users would start seeing "could not run 'gpgsm'" if
they
do not even have it installed.

Ah yes, I completely forgot to take that into account.

   cmn