{"thread":{"id":"48253","subject":"[PATCH 0/8] gpg-interface: Multiple signing tools","startedAt":"2018-04-09T20:42:18Z","lastAt":"2018-05-09T08:03:54Z","messageCount":46,"participants":["Ben Toews","Eric Sunshine","Stefan Beller","Junio C Hamano","Jeff King","SZEDER Gábor","brian m. carlson","Taylor Blau"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"344234","messageId":"20180409204129.43537-1-mastahyeti@gmail.com","threadId":"48253","inReplyTo":null,"subject":"[PATCH 0/8] gpg-interface: Multiple signing tools","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:21Z","receivedAt":"2018-04-09T20:42:18Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"This series extends the configuration to allow Git to better work with multiple signing tools.\n\nBen Toews (1):\n gpg-interface: handle alternative signature types\n\nJeff King (7):\n gpg-interface: handle bool user.signingkey\n gpg-interface: modernize function declarations\n gpg-interface: use size_t for signature buffer size\n gpg-interface: fix const-correctness of \"eol\" pointer\n gpg-interface: extract gpg line matching helper\n gpg-interface: find the last gpg signature line\n gpg-interface: prepare for parsing arbitrary PEM blocks\n\n Documentation/config.txt | 40 +++++++---\n builtin/fmt-merge-msg.c | 6 +-\n builtin/receive-pack.c | 7 +-\n builtin/tag.c | 2 +-\n commit.c | 2 +-\n gpg-interface.c | 198 +++++++++++++++++++++++++++++++++++++++--------\n gpg-interface.h | 67 +++++++++++++---\n log-tree.c | 7 +-\n ref-filter.c | 2 +-\n t/lib-gpg.sh | 26 +++++++\n t/t7004-tag.sh | 11 +++\n t/t7510-signed-commit.sh | 32 +++++++-\n tag.c | 2 +-\n 13 files changed, 336 insertions(+), 66 deletions(-)\n\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344235","messageId":"20180409204129.43537-2-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 1/8] gpg-interface: handle bool user.signingkey","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:22Z","receivedAt":"2018-04-09T20:42:20Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nThe config handler for user.signingkey does not check for a\nboolean value, and thus:\n\n git -c user.signingkey tag\n\nwill segfault. We could fix this and even shorten the code\nby using git_config_string(). But our set_signing_key()\nhelper is used by other code outside of gpg-interface.c, so\nwe must keep it (and we may as well use it, because unlike\ngit_config_string() it does not leak when we overwrite an\nold value).\n\nIronically, the handler for gpg.program just below _could_\nuse git_config_string() but doesn't. But since we're going\nto touch that in a future patch, we'll leave it alone for\nnow. We will add some whitespace and returns in preparation\nfor adding more config keys, though.\n---\n gpg-interface.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 4feacf16e5..61c0690e12 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -128,13 +128,19 @@ void set_signing_key(const char *key)\n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"user.signingkey\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n \t\tset_signing_key(value);\n+\t\treturn 0;\n \t}\n+\n \tif (!strcmp(var, \"gpg.program\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tgpg_program = xstrdup(value);\n+\t\treturn 0;\n \t}\n+\n \treturn 0;\n }\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344236","messageId":"20180409204129.43537-4-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 3/8] gpg-interface: use size_t for signature buffer size","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:24Z","receivedAt":"2018-04-09T20:42:23Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nEven though our object sizes (from which these buffers would\ncome) are typically \"unsigned long\", this is something we'd\nlike to eventually fix (since it's only 32-bits even on\n64-bit Windows). It makes more sense to use size_t when\ntaking an in-memory buffer.\n---\n gpg-interface.c | 2 +-\n gpg-interface.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 08de0daa41..ac852ad4b9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,7 +101,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-size_t parse_signature(const char *buf, unsigned long size)\n+size_t parse_signature(const char *buf, size_t size)\n {\n \tchar *eol;\n \tsize_t len = 0;\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 2c40a9175f..a5e6517ae6 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -31,7 +31,7 @@ void signature_check_clear(struct signature_check *sigc);\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, unsigned long size);\n+size_t parse_signature(const char *buf, size_t size);\n \n void parse_gpg_output(struct signature_check *);\n \n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344237","messageId":"20180409204129.43537-3-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 2/8] gpg-interface: modernize function declarations","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:23Z","receivedAt":"2018-04-09T20:42:25Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nLet's drop \"extern\" from our declarations, which brings us\nin line with our modern style guidelines. While we're\nhere, let's wrap some of the overly long lines, and move\ndocstrings for public functions to their declarations, since\nthey document the interface.\n---\n gpg-interface.c | 17 -----------------\n gpg-interface.h | 49 ++++++++++++++++++++++++++++++++++++++-----------\n 2 files changed, 38 insertions(+), 28 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 61c0690e12..08de0daa41 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,12 +101,6 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-/*\n- * Look at GPG signed content (e.g. a signed tag object), whose\n- * payload is followed by a detached signature on it. Return the\n- * offset where the embedded detached signature begins, or the end of\n- * the data when there is no such signature.\n- */\n size_t parse_signature(const char *buf, unsigned long size)\n {\n \tchar *eol;\n@@ -151,12 +145,6 @@ const char *get_signing_key(void)\n \treturn git_committer_info(IDENT_STRICT|IDENT_NO_DATE);\n }\n \n-/*\n- * Create a detached signature for the contents of \"buffer\" and append\n- * it after \"signature\"; \"buffer\" and \"signature\" can be the same\n- * strbuf instance, which would cause the detached signature appended\n- * at the end.\n- */\n int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n@@ -198,11 +186,6 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \treturn 0;\n }\n \n-/*\n- * Run \"gpg\" to see if the payload matches the detached signature.\n- * gpg_output, when set, receives the diagnostic output from GPG.\n- * gpg_status, when set, receives the status output from GPG.\n- */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n \t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex d2d4fd3a65..2c40a9175f 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -23,16 +23,43 @@ struct signature_check {\n \tchar *key;\n };\n \n-extern void signature_check_clear(struct signature_check *sigc);\n-extern size_t parse_signature(const char *buf, unsigned long size);\n-extern void parse_gpg_output(struct signature_check *);\n-extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);\n-extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status);\n-extern int git_gpg_config(const char *, const char *, void *);\n-extern void set_signing_key(const char *);\n-extern const char *get_signing_key(void);\n-extern int check_signature(const char *payload, size_t plen,\n-\tconst char *signature, size_t slen, struct signature_check *sigc);\n-void print_signature_buffer(const struct signature_check *sigc, unsigned flags);\n+void signature_check_clear(struct signature_check *sigc);\n+\n+/*\n+ * Look at GPG signed content (e.g. a signed tag object), whose\n+ * payload is followed by a detached signature on it. Return the\n+ * offset where the embedded detached signature begins, or the end of\n+ * the data when there is no such signature.\n+ */\n+size_t parse_signature(const char *buf, unsigned long size);\n+\n+void parse_gpg_output(struct signature_check *);\n+\n+/*\n+ * Create a detached signature for the contents of \"buffer\" and append\n+ * it after \"signature\"; \"buffer\" and \"signature\" can be the same\n+ * strbuf instance, which would cause the detached signature appended\n+ * at the end.\n+ */\n+int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n+\t\tconst char *signing_key);\n+\n+/*\n+ * Run \"gpg\" to see if the payload matches the detached signature.\n+ * gpg_output, when set, receives the diagnostic output from GPG.\n+ * gpg_status, when set, receives the status output from GPG.\n+ */\n+int verify_signed_buffer(const char *payload, size_t payload_size,\n+\t\t\t const char *signature, size_t signature_size,\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status);\n+\n+int git_gpg_config(const char *, const char *, void *);\n+void set_signing_key(const char *);\n+const char *get_signing_key(void);\n+int check_signature(const char *payload, size_t plen,\n+\t\t const char *signature, size_t slen,\n+\t\t struct signature_check *sigc);\n+void print_signature_buffer(const struct signature_check *sigc,\n+\t\t\t unsigned flags);\n \n #endif\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344238","messageId":"20180409204129.43537-5-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 4/8] gpg-interface: fix const-correctness of \"eol\" pointer","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:25Z","receivedAt":"2018-04-09T20:42:28Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nWe accidentally shed the \"const\" of our buffer by passing it\nthrough memchr. Let's fix that, and while we're at it, move\nour variable declaration inside the loop, which is the only\nplace that uses it.\n---\n gpg-interface.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex ac852ad4b9..3414af38b9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -103,11 +103,10 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \n size_t parse_signature(const char *buf, size_t size)\n {\n-\tchar *eol;\n \tsize_t len = 0;\n \twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n \t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n-\t\teol = memchr(buf + len, '\\n', size - len);\n+\t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n \treturn len;\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344239","messageId":"20180409204129.43537-6-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 5/8] gpg-interface: extract gpg line matching helper","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:26Z","receivedAt":"2018-04-09T20:42:30Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nLet's separate the actual line-by-line parsing of signatures\nfrom the notion of \"is this a gpg signature line\". That will\nmake it easier to do more refactoring of this loop in future\npatches.\n---\n gpg-interface.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3414af38b9..79333c1ee8 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,11 +101,16 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n+static int is_gpg_start(const char *line)\n+{\n+\treturn starts_with(line, PGP_SIGNATURE) ||\n+\t\tstarts_with(line, PGP_MESSAGE);\n+}\n+\n size_t parse_signature(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n-\twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n-\t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n+\twhile (len < size && !is_gpg_start(buf + len)) {\n \t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344240","messageId":"20180409204129.43537-8-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 7/8] gpg-interface: prepare for parsing arbitrary PEM blocks","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:28Z","receivedAt":"2018-04-09T20:42:33Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nIn preparation for handling more PEM blocks besides \"PGP\nSIGNATURE\" and \"PGP MESSAGE', let's break up the parsing to\nparameterize the actual block type.\n---\n gpg-interface.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 0647bd6348..0ba4a8ac3b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -9,9 +9,6 @@\n static char *configured_signing_key;\n static const char *gpg_program = \"gpg\";\n \n-#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n-#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n-\n void signature_check_clear(struct signature_check *sigc)\n {\n \tFREE_AND_NULL(sigc->payload);\n@@ -101,10 +98,17 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-static int is_gpg_start(const char *line)\n+static int is_pem_start(const char *line)\n {\n-\treturn starts_with(line, PGP_SIGNATURE) ||\n-\t\tstarts_with(line, PGP_MESSAGE);\n+\tif (!skip_prefix(line, \"-----BEGIN \", &line))\n+\t\treturn 0;\n+\tif (!skip_prefix(line, \"PGP SIGNATURE\", &line) &&\n+\t !skip_prefix(line, \"PGP MESSAGE\", &line))\n+\t\treturn 0;\n+\tif (!starts_with(line, \"-----\"))\n+\t\treturn 0;\n+\n+\treturn 1;\n }\n \n size_t parse_signature(const char *buf, size_t size)\n@@ -114,7 +118,7 @@ size_t parse_signature(const char *buf, size_t size)\n \twhile (len < size) {\n \t\tconst char *eol;\n \n-\t\tif (is_gpg_start(buf + len))\n+\t\tif (is_pem_start(buf + len))\n \t\t\tmatch = len;\n \n \t\teol = memchr(buf + len, '\\n', size - len);\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344241","messageId":"20180409204129.43537-7-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-09T20:41:27Z","receivedAt":"2018-04-09T20:42:34Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nA signed tag has a detached signature like this:\n\n object ...\n [...more header...]\n\n This is the tag body.\n\n -----BEGIN PGP SIGNATURE-----\n [opaque gpg data]\n -----END PGP SIGNATURE-----\n\nOur parser finds the _first_ line that appears to start a\nPGP signature block, meaning we may be confused by a\nsignature (or a signature-like line) in the actual body.\nLet's keep parsing and always find the final block, which\nshould be the detached signature over all of the preceding\ncontent.\n---\n gpg-interface.c | 12 +++++++++---\n t/t7004-tag.sh | 11 +++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 79333c1ee8..0647bd6348 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -110,11 +110,17 @@ static int is_gpg_start(const char *line)\n size_t parse_signature(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n-\twhile (len < size && !is_gpg_start(buf + len)) {\n-\t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n+\tsize_t match = size;\n+\twhile (len < size) {\n+\t\tconst char *eol;\n+\n+\t\tif (is_gpg_start(buf + len))\n+\t\t\tmatch = len;\n+\n+\t\teol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n-\treturn len;\n+\treturn match;\n }\n \n void set_signing_key(const char *key)\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex ee093b393d..e3f1e014aa 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1059,6 +1059,17 @@ test_expect_success GPG \\\n \tgit tag -v blanknonlfile-signed-tag\n '\n \n+test_expect_success GPG 'signed tag with embedded PGP message' '\n+\tcat >msg <<-\\EOF &&\n+\t-----BEGIN PGP MESSAGE-----\n+\n+\tthis is not a real PGP message\n+\t-----END PGP MESSAGE-----\n+\tEOF\n+\tgit tag -s -F msg confusing-pgp-message &&\n+\tgit tag -v confusing-pgp-message\n+'\n+\n # messages with commented lines for signed tags:\n \n cat >sigcommentsfile <\n\nCurrently you can only sign commits and tags using \"gpg\".\nYou can _almost_ plug in a related tool like \"gpgsm\" (which\nuses S/MIME-style signatures instead of PGP) using\ngpg.program, as it has command-line compatibility. But there\nare a few rough edges:\n\n 1. gpgsm generates a slightly different PEM format than\n gpg. It says:\n\n -----BEGIN SIGNED MESSAGE-----\n\n instead of:\n\n -----BEGIN PGP SIGNATURE-----\n\n This actually works OK for signed commits, where we\n just dump the gpgsig header to gpg.program regardless.\n\n But for tags, we actually have to parse out the\n detached signature, and we fail to recognize the gpgsm\n version.\n\n 2. You can't mix the two types of signatures easily, as\n we'd send the output to whatever tool you've\n configured. For verification, we'd ideally dispatch gpg\n signatures to gpg, gpgsm ones to gpgsm, and so on. For\n signing, you'd obviously have to pick a tool to use.\n\nThis patch introduces a set of configuration options for\ndefining a \"signing tool\", of which gpg may be just one.\nWith this patch you can:\n\n - define a new tool \"foo\" with signingtool.foo.program\n\n - map PEM strings to it for verification using\n signingtool.foo.pemtype\n\n - set it as your default tool for creating signatures\n using using signingtool.default\n\nThis subsumes the existing gpg config, as we have baked-in\nconfig for signingtool.gpg that works just like the current\nhard-coded behavior. And setting gpg.program becomes an\nalias for signingtool.gpg.program.\n\nThis is enough to plug in gpgsm like:\n\n [signingtool \"gpgsm\"]\n program = gpgsm\n pemtype = \"SIGNED MESSAGE\"\n\nIn the future, this config scheme gives us options for\nextending to other tools:\n\n - the tools all have to accept gpg-like options for now,\n but we could add signingtool.interface to meet other\n standards\n\n - we only match PEM headers now, but we could add other\n config for matching non-PEM types\n\nThe implementation is still in gpg-interface.c, and most of\nthe code internally refers to this as \"gpg\". I've left it\nthis way to keep the diff sane, and to make it clear that\nwe're not breaking anything gpg-related. This is probably OK\nfor now, as our tools must be gpg-like anyway. But renaming\neverything would be an obvious next-step refactoring if we\nwant to offer support for more exotic tools (e.g., people\nhave asked before on the list about using OpenBSD signify).\n---\n Documentation/config.txt | 40 +++++++++---\n builtin/fmt-merge-msg.c | 6 +-\n builtin/receive-pack.c | 7 +-\n builtin/tag.c | 2 +-\n commit.c | 2 +-\n gpg-interface.c | 165 ++++++++++++++++++++++++++++++++++++++++++-----\n gpg-interface.h | 24 ++++++-\n log-tree.c | 7 +-\n ref-filter.c | 2 +-\n t/lib-gpg.sh | 26 ++++++++\n t/t7510-signed-commit.sh | 32 ++++++++-\n tag.c | 2 +-\n 12 files changed, 272 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4e0cff87f6..7906123a59 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,16 +1727,38 @@ grep.fallbackToNoIndex::\n \tIf set to true, fall back to git grep --no-index if git grep\n \tis executed outside of a git repository. Defaults to false.\n \n-gpg.program::\n-\tUse this custom program instead of \"`gpg`\" found on `$PATH` when\n-\tmaking or verifying a PGP signature. The program must support the\n-\tsame command-line interface as GPG, namely, to verify a detached\n-\tsignature, \"`gpg --verify $file - <$signature`\" is run, and the\n-\tprogram is expected to signal a good signature by exiting with\n-\tcode 0, and to generate an ASCII-armored detached signature, the\n-\tstandard input of \"`gpg -bsau $key`\" is fed with the contents to be\n+signingtool..program::\n+\tThe name of the program on `$PATH` to execute when making or\n+\tverifying a signature. This program will be used for making\n+\tsignatures if `` is configured as `signingtool.default`.\n+\tThis program will be used for verifying signatures whose PEM\n+\tblock type matches `signingtool..pemtype` (see below). The\n+\tprogram must support the same command-line interface as GPG.\n+\tTo verify a detached signature,\n+\t\"`gpg --verify $file - <$signature`\" is run, and the program is\n+\texpected to signal a good signature by exiting with code 0.\n+\tTo generate an ASCII-armored detached signature, the standard\n+\tinput of \"`gpg -bsau $key`\" is fed with the contents to be\n \tsigned, and the program is expected to send the result to its\n-\tstandard output.\n+\tstandard output. By default, `signingtool.gpg.program` is set to\n+\t`gpg`.\n+\n+signingtool..pemtype::\n+\tThe PEM block type associated with the signing tool named\n+\t``. For example, the block type of a GPG signature\n+\tstarting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n+\tSIGNATURE`. When verifying a signature with this PEM block type\n+\tthe program specified in `signingtool..program` will be\n+\tused. By default `signingtool.gpg.pemtype` contains `PGP\n+\tSIGNATURE` and `PGP MESSAGE`.\n+\n+signingtool.default::\n+\tThe `` of the signing tool to use when creating\n+\tsignatures (e.g., setting it to \"foo\" will use use the program\n+\tspecified by `signingtool.foo.program`). Defaults to `gpg`.\n+\n+gpg.program::\n+\tHistorical alias for `signingtool.gpg.program`.\n \n gui.commitMsgWidth::\n \tDefines how wide the commit message window is in the\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 8e8a15ea4a..6aa8226308 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -490,14 +490,16 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\tunsigned long size, len;\n \t\tchar *buf = read_sha1_file(sha1, &type, &size);\n \t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tconst struct signing_tool *tool;\n \n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n+\t\tlen = parse_signature(buf, size, &tool);\n \n \t\tif (size == len)\n \t\t\t; /* merely annotated */\n-\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {\n+\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len,\n+\t\t\t\t\t &sig, NULL, tool)) {\n \t\t\tif (!sig.len)\n \t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\t}\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 75e7f18ace..be17dcaa78 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -631,6 +631,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \t\tstruct strbuf gpg_output = STRBUF_INIT;\n \t\tstruct strbuf gpg_status = STRBUF_INIT;\n \t\tint bogs /* beginning_of_gpg_sig */;\n+\t\tconst struct signing_tool *tool;\n \n \t\talready_done = 1;\n \t\tif (write_object_file(push_cert.buf, push_cert.len, \"blob\",\n@@ -640,10 +641,10 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \t\tsigcheck.result = 'N';\n \n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signature(push_cert.buf, push_cert.len, &tool);\n \t\tif (verify_signed_buffer(push_cert.buf, bogs,\n \t\t\t\t\t push_cert.buf + bogs, push_cert.len - bogs,\n-\t\t\t\t\t &gpg_output, &gpg_status) < 0) {\n+\t\t\t\t\t &gpg_output, &gpg_status, tool) < 0) {\n \t\t\t; /* error running gpg */\n \t\t} else {\n \t\t\tsigcheck.payload = push_cert.buf;\n@@ -1565,7 +1566,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len, NULL);\n \n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex da186691ed..adc37a28c5 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -178,7 +178,7 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp, NULL));\n \n \tfree(buf);\n }\ndiff --git a/commit.c b/commit.c\nindex 00c99c7272..056942df7f 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1209,7 +1209,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_sha1_file(desc->obj->oid.hash, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n+\tlen = parse_signature(buf, size, NULL);\n \tif (size == len)\n \t\tgoto free_return;\n \t/*\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 0ba4a8ac3b..0e2a82e8e5 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -7,7 +7,60 @@\n #include \"tempfile.h\"\n \n static char *configured_signing_key;\n-static const char *gpg_program = \"gpg\";\n+static const char *default_signing_tool = \"gpg\";\n+static struct signing_tool *signing_tool_config;\n+\n+static struct signing_tool *alloc_signing_tool(void)\n+{\n+\tstruct signing_tool *ret;\n+\tret = xcalloc(1, sizeof(*ret));\n+\tret->pemtype.strdup_strings = 1;\n+\treturn ret;\n+}\n+\n+/*\n+ * Our default tool config is too complicated to specify as a constant\n+ * initializer, so we lazily create it as needed.\n+ */\n+static void init_signing_tool_defaults(void) {\n+\tstruct signing_tool *tool;\n+\n+\tif (signing_tool_config)\n+\t\treturn;\n+\n+\ttool = alloc_signing_tool();\n+\ttool->name = xstrdup(\"gpg\");\n+\ttool->program = xstrdup(\"gpg\");\n+\tstring_list_append(&tool->pemtype, \"PGP SIGNATURE\");\n+\tstring_list_append(&tool->pemtype, \"PGP MESSAGE\");\n+\n+\ttool->next = signing_tool_config;\n+\tsigning_tool_config = tool;\n+}\n+\n+static struct signing_tool *get_signing_tool(const char *name) {\n+\tstruct signing_tool *tool;\n+\n+\tinit_signing_tool_defaults();\n+\n+\tfor (tool = signing_tool_config; tool; tool = tool->next) {\n+\t\tif (!strcmp(name, tool->name))\n+\t\t\treturn tool;\n+\t}\n+\treturn NULL;\n+}\n+\n+static struct signing_tool *get_or_create_signing_tool(const char *name)\n+{\n+\tstruct signing_tool *tool = get_signing_tool(name);\n+\tif (!tool) {\n+\t\ttool = alloc_signing_tool();\n+\t\ttool->name = xstrdup(name);\n+\t\ttool->next = signing_tool_config;\n+\t\tsigning_tool_config = tool;\n+\t}\n+\treturn tool;\n+}\n \n void signature_check_clear(struct signature_check *sigc)\n {\n@@ -71,7 +124,7 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n \tsigc->result = 'N';\n \n \tstatus = verify_signed_buffer(payload, plen, signature, slen,\n-\t\t\t\t &gpg_output, &gpg_status);\n+\t\t\t\t &gpg_output, &gpg_status, NULL);\n \tif (status && !gpg_output.len)\n \t\tgoto out;\n \tsigc->payload = xmemdupz(payload, plen);\n@@ -98,32 +151,49 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n \n-static int is_pem_start(const char *line)\n+static int is_pem_start(const char *line, struct signing_tool **out_tool)\n {\n+\tstruct signing_tool *tool;\n+\n \tif (!skip_prefix(line, \"-----BEGIN \", &line))\n \t\treturn 0;\n-\tif (!skip_prefix(line, \"PGP SIGNATURE\", &line) &&\n-\t !skip_prefix(line, \"PGP MESSAGE\", &line))\n-\t\treturn 0;\n-\tif (!starts_with(line, \"-----\"))\n-\t\treturn 0;\n \n-\treturn 1;\n+\tinit_signing_tool_defaults();\n+\n+\tfor (tool = signing_tool_config; tool; tool = tool->next) {\n+\t\tint i;\n+\t\tfor (i = 0; i < tool->pemtype.nr; i++) {\n+\t\t\tconst char *match = tool->pemtype.items[i].string;\n+\t\t\tconst char *end;\n+\t\t\tif (skip_prefix(line, match, &end) &&\n+\t\t\t starts_with(end, \"-----\")) {\n+\t\t\t\t*out_tool = tool;\n+\t\t\t\treturn 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 0;\n }\n \n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signature(const char *buf, size_t size,\n+\t\t const struct signing_tool **out_tool)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n+\tstruct signing_tool *tool = NULL;\n+\n \twhile (len < size) {\n \t\tconst char *eol;\n \n-\t\tif (is_pem_start(buf + len))\n+\t\tif (is_pem_start(buf + len, &tool))\n \t\t\tmatch = len;\n \n \t\teol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n+\n+\tif (out_tool)\n+\t\t*out_tool = tool;\n \treturn match;\n }\n \n@@ -135,6 +205,9 @@ void set_signing_key(const char *key)\n \n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n+\tconst char *key, *name = NULL;\n+\tint name_len;\n+\n \tif (!strcmp(var, \"user.signingkey\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -143,12 +216,43 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t}\n \n \tif (!strcmp(var, \"gpg.program\")) {\n+\t\tstruct signing_tool *tool = get_or_create_signing_tool(\"gpg\");\n+\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tgpg_program = xstrdup(value);\n+\n+\t\tfree(tool->program);\n+\t\ttool->program = xstrdup(value);\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"signingtool.default\"))\n+\t\treturn git_config_string(&default_signing_tool, var, value);\n+\n+\tif (!parse_config_key(var, \"signingtool\", &name, &name_len, &key) && name) {\n+\t\tchar *tmpname = xmemdupz(name, name_len);\n+\t\tstruct signing_tool *tool = get_or_create_signing_tool(tmpname);\n+\n+\t\tfree(tmpname);\n+\n+\t\tif (!strcmp(key, \"program\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\n+\t\t\tfree(tool->program);\n+\t\t\ttool->program = xstrdup(value);\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tif (!strcmp(key, \"pemtype\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\n+\t\t\tstring_list_append(&tool->pemtype, value);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n \treturn 0;\n }\n \n@@ -159,7 +263,9 @@ const char *get_signing_key(void)\n \treturn git_committer_info(IDENT_STRICT|IDENT_NO_DATE);\n }\n \n-int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n+static int sign_buffer_with(struct strbuf *buffer, struct strbuf *signature,\n+\t\t\t const char *signing_key,\n+\t\t\t const struct signing_tool *tool)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n@@ -167,7 +273,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tstruct strbuf gpg_status = STRBUF_INIT;\n \n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n+\t\t\t tool->program,\n \t\t\t \"--status-fd=2\",\n \t\t\t \"-bsau\", signing_key,\n \t\t\t NULL);\n@@ -200,15 +306,42 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \treturn 0;\n }\n \n+int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n+{\n+\tstruct signing_tool *tool = get_signing_tool(default_signing_tool);\n+\tif (!tool || !tool->program)\n+\t\treturn error(_(\"default signing tool '%s' has no program configured\"),\n+\t\t\t default_signing_tool);\n+\treturn sign_buffer_with(buffer, signature, signing_key, tool);\n+}\n+\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n-\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status,\n+\t\t\t const struct signing_tool *tool)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tstruct tempfile *temp;\n \tint ret;\n \tstruct strbuf buf = STRBUF_INIT;\n \n+\tif (!tool) {\n+\t\tparse_signature(signature, signature_size, &tool);\n+\t\tif (!tool) {\n+\t\t\t/*\n+\t\t\t * The caller didn't tell us which tool to use, and we\n+\t\t\t * didn't recognize the format. Historically we've fed\n+\t\t\t * these cases to blindly to gpg, so let's continue to\n+\t\t\t * do so.\n+\t\t\t */\n+\t\t\ttool = get_signing_tool(\"gpg\");\n+\t\t}\n+\t}\n+\n+\tif (!tool->program)\n+\t\treturn error(_(\"signing tool '%s' has no program configured\"),\n+\t\t\t tool->name);\n+\n \ttemp = mks_tempfile_t(\".git_vtag_tmpXXXXXX\");\n \tif (!temp)\n \t\treturn error_errno(_(\"could not create temporary file\"));\n@@ -221,7 +354,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t}\n \n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n+\t\t\t tool->program,\n \t\t\t \"--status-fd=1\",\n \t\t\t \"--keyid-format=long\",\n \t\t\t \"--verify\", temp->filename.buf, \"-\",\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex a5e6517ae6..cee0dfe401 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -23,15 +23,27 @@ struct signature_check {\n \tchar *key;\n };\n \n+struct signing_tool {\n+\tchar *name;\n+\tchar *program;\n+\tstruct string_list pemtype;\n+\tstruct signing_tool *next;\n+};\n+\n void signature_check_clear(struct signature_check *sigc);\n \n /*\n- * Look at GPG signed content (e.g. a signed tag object), whose\n+ * Look for signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n+ *\n+ * If out_tool is non-NULL and a signature is found, it will be\n+ * pointed at the signing_tool that corresponds to the found\n+ * signature type.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signature(const char *buf, unsigned long size,\n+\t\t const struct signing_tool **out_tool);\n \n void parse_gpg_output(struct signature_check *);\n \n@@ -48,10 +60,16 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n * Run \"gpg\" to see if the payload matches the detached signature.\n * gpg_output, when set, receives the diagnostic output from GPG.\n * gpg_status, when set, receives the status output from GPG.\n+ *\n+ * Typically the \"tool\" argument should come from a previous call to\n+ * parse_signature(). If it's NULL, then verify_signed_buffer() will\n+ * try to choose the appropriate tool based on the contents of the\n+ * \"signature\" buffer.\n */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n-\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status);\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status,\n+\t\t\t const struct signing_tool *tool);\n \n int git_gpg_config(const char *, const char *, void *);\n void set_signing_key(const char *);\ndiff --git a/log-tree.c b/log-tree.c\nindex bdf23c5f7b..f8fdff6e65 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -456,7 +456,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n \n \tstatus = verify_signed_buffer(payload.buf, payload.len,\n \t\t\t\t signature.buf, signature.len,\n-\t\t\t\t &gpg_output, NULL);\n+\t\t\t\t &gpg_output, NULL, NULL);\n \tif (status && !gpg_output.len)\n \t\tstrbuf_addstr(&gpg_output, \"No signature\\n\");\n \n@@ -498,6 +498,7 @@ static void show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tint status, nth;\n \tsize_t payload_size, gpg_message_offset;\n+\tconst struct signing_tool *tool;\n \n \thash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);\n \ttag = lookup_tag(&oid);\n@@ -520,14 +521,14 @@ static void show_one_mergetag(struct commit *commit,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \tgpg_message_offset = verify_message.len;\n \n-\tpayload_size = parse_signature(extra->value, extra->len);\n+\tpayload_size = parse_signature(extra->value, extra->len, &tool);\n \tstatus = -1;\n \tif (extra->len > payload_size) {\n \t\t/* could have a good signature */\n \t\tif (!verify_signed_buffer(extra->value, payload_size,\n \t\t\t\t\t extra->value + payload_size,\n \t\t\t\t\t extra->len - payload_size,\n-\t\t\t\t\t &verify_message, NULL))\n+\t\t\t\t\t &verify_message, NULL, tool))\n \t\t\tstatus = 0; /* good */\n \t\telse if (verify_message.len <= gpg_message_offset)\n \t\t\tstrbuf_addstr(&verify_message, \"No signature\\n\");\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 45fc56216a..6ab66b9a7c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1009,7 +1009,7 @@ static void find_subpos(const char *buf, unsigned long sz,\n \t\tbuf++;\n \n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n+\t*sig = buf + parse_signature(buf, strlen(buf), NULL);\n \t*siglen = strlen(*sig);\n \n \t/* subject is first non-empty line */\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex a5d3b2cbaa..08d23b0cf5 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -56,3 +56,29 @@ sanitize_pgp() {\n \t\t/^-----BEGIN PGP/ and $in_pgp = 1;\n \t'\n }\n+\n+create_fake_signer () {\n+\twrite_script fake-signer <<-\\EOF\n+\tif [ \"$1 $2\" = \"--status-fd=2 -bsau\" ]; then\n+\t\techo >&2 \"[GNUPG:] BEGIN_SIGNING\"\n+\t\techo >&2 \"[GNUPG:] SIG_CREATED D 1 SHA256 0 1513792449 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70\"\n+\t\t# avoid \"-\" in echo arguments\n+\t\tprintf \"%s\\n\" \\\n+\t\t \"-----BEGIN FAKE SIGNER SIGNATURE-----\" \\\n+\t\t \"fake-signature\" \\\n+\t\t \"-----END FAKE SIGNER SIGNATURE-----\"\n+\t\texit 0\n+\n+\telif [ \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\" ]; then\n+\t\techo \"[GNUPG:] NEWSIG\"\n+\t\techo \"[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\"\n+\t\techo \"[GNUPG:] TRUST_FULLY 0 shell\"\n+\t\techo >&2 \"Good signature from /CN=Some User/EMail=some@user.email\"\n+\t\texit 0\n+\n+\telse\n+\t\techo \"bad arguments\" 1>&2\n+\t\texit 1\n+\tfi\n+\tEOF\n+}\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 762135adea..848a823302 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -5,6 +5,10 @@ test_description='signed commit tests'\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n+test_expect_success 'create fake signing program' '\n+\tcreate_fake_signer\n+'\n+\n test_expect_success GPG 'create signed commits' '\n \ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n \n@@ -51,13 +55,33 @@ test_expect_success GPG 'create signed commits' '\n \t# commit.gpgsign is still on but this must not be signed\n \tgit tag ninth-unsigned $(echo 9 | git commit-tree HEAD^{tree}) &&\n \t# explicit -S of course must sign.\n-\tgit tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree})\n+\tgit tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree}) &&\n+\n+\tgit config signingtool.fake.program \"$TRASH_DIRECTORY/fake-signer\" &&\n+\tgit config signingtool.fake.pemtype \"FAKE SIGNER SIGNATURE\" &&\n+\techo 11 >file && test_tick && git commit -a -m eleventh &&\n+\tgit tag eleventh-pgp-signed &&\n+\tgit cat-file -p eleventh-pgp-signed >actual &&\n+\tgrep \"PGP SIGNATURE\" actual &&\n+\n+\tgit config gpg.program \"$TRASH_DIRECTORY/fake-signer\" &&\n+\techo 12 >file && test_tick && git commit -a -m twelfth && test_unconfig gpg.program &&\n+\tgit tag twelfth-fake-signed &&\n+\tgit cat-file -p twelfth-fake-signed >actual &&\n+\tgrep \"FAKE SIGNER SIGNATURE\" actual &&\n+\n+\tgit config signingtool.default fake &&\n+\techo 13 >file && test_tick && git commit -a -m thirteenth && test_unconfig signingtool.default &&\n+\tgit tag thirteenth-fake-signed &&\n+\tgit cat-file -p thirteenth-fake-signed >actual &&\n+\tgrep \"FAKE SIGNER SIGNATURE\" actual\n '\n \n test_expect_success GPG 'verify and show signatures' '\n \t(\n \t\tfor commit in initial second merge fourth-signed \\\n-\t\t\tfifth-signed sixth-signed seventh-signed tenth-signed\n+\t\t\tfifth-signed sixth-signed seventh-signed tenth-signed \\\n+\t\t\televenth-pgp-signed twelfth-fake-signed thirteenth-fake-signed\n \t\tdo\n \t\t\tgit verify-commit $commit &&\n \t\t\tgit show --pretty=short --show-signature $commit >actual &&\n@@ -98,7 +122,9 @@ test_expect_success GPG 'verify-commit exits success on untrusted signature' '\n \n test_expect_success GPG 'verify signatures with --raw' '\n \t(\n-\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed\n+\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed \\\n+\t\t\tseventh-signed eleventh-pgp-signed twelfth-fake-signed \\\n+\t\t\tthirteenth-fake-signed\n \t\tdo\n \t\t\tgit verify-commit --raw $commit 2>actual &&\n \t\t\tgrep \"GOODSIG\" actual &&\ndiff --git a/tag.c b/tag.c\nindex 66210fd477..2d557e1f5e 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -15,7 +15,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \n \tmemset(&sigc, 0, sizeof(sigc));\n \n-\tpayload_size = parse_signature(buf, size);\n+\tpayload_size = parse_signature(buf, size, NULL);\n \n \tif (size == payload_size) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n-- \n2.15.1 (Apple Git-101)\n\n"},{"id":"344243","messageId":"CAPig+cQb+ZDWgCNVPX8_eFqUy-_crQ4wtrYhv-6kwasGwLTWuA@mail.gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-2-mastahyeti@gmail.com","subject":"Re: [PATCH 1/8] gpg-interface: handle bool user.signingkey","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-09T20:55:26Z","receivedAt":"2018-04-09T20:55:31Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews wrote:\n> From: Jeff King \n>\n> The config handler for user.signingkey does not check for a\n> boolean value, and thus:\n>\n> git -c user.signingkey tag\n>\n> will segfault. We could fix this and even shorten the code\n> by using git_config_string(). But our set_signing_key()\n> helper is used by other code outside of gpg-interface.c, so\n> we must keep it (and we may as well use it, because unlike\n> git_config_string() it does not leak when we overwrite an\n> old value).\n>\n> Ironically, the handler for gpg.program just below _could_\n> use git_config_string() but doesn't. But since we're going\n> to touch that in a future patch, we'll leave it alone for\n> now. We will add some whitespace and returns in preparation\n> for adding more config keys, though.\n> ---\n\nPeff's Signed-off-by: is missing. Also, since you're forwarding this\npatch on Peff's behalf, your Signed-off-by: should follow his. Same\ncomment applies to all patches by Peff in this series.\n\nThe patch itself looks reasonable.\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 4feacf16e5..61c0690e12 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -128,13 +128,19 @@ void set_signing_key(const char *key)\n> int git_gpg_config(const char *var, const char *value, void *cb)\n> {\n> if (!strcmp(var, \"user.signingkey\")) {\n> + if (!value)\n> + return config_error_nonbool(var);\n> set_signing_key(value);\n> + return 0;\n> }\n> +\n> if (!strcmp(var, \"gpg.program\")) {\n> if (!value)\n> return config_error_nonbool(var);\n> gpg_program = xstrdup(value);\n> + return 0;\n> }\n> +\n> return 0;\n> }\n"},{"id":"344244","messageId":"CAGZ79kYQ0AO=Lp6hpsMfQK0PFt7veEwc+uK-0+JLrudAQ1iAXw@mail.gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-9-mastahyeti@gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-04-09T21:01:27Z","receivedAt":"2018-04-09T21:01:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi Ben,\n\nOn Mon, Apr 9, 2018 at 1:41 PM, Ben Toews wrote:\n> From: Ben Toews \n>\n> Currently you can only sign commits and tags using \"gpg\".\n> You can _almost_ plug in a related tool like \"gpgsm\" (which\n> uses S/MIME-style signatures instead of PGP) using\n> gpg.program, as it has command-line compatibility. But there\n> are a few rough edges:\n>\n> 1. gpgsm generates a slightly different PEM format than\n> gpg. It says:\n>\n> -----BEGIN SIGNED MESSAGE-----\n>\n> instead of:\n>\n> -----BEGIN PGP SIGNATURE-----\n>\n> This actually works OK for signed commits, where we\n> just dump the gpgsig header to gpg.program regardless.\n>\n> But for tags, we actually have to parse out the\n> detached signature, and we fail to recognize the gpgsm\n> version.\n>\n> 2. You can't mix the two types of signatures easily, as\n> we'd send the output to whatever tool you've\n> configured. For verification, we'd ideally dispatch gpg\n> signatures to gpg, gpgsm ones to gpgsm, and so on. For\n> signing, you'd obviously have to pick a tool to use.\n>\n> This patch introduces a set of configuration options for\n> defining a \"signing tool\", of which gpg may be just one.\n> With this patch you can:\n>\n> - define a new tool \"foo\" with signingtool.foo.program\n>\n> - map PEM strings to it for verification using\n> signingtool.foo.pemtype\n>\n> - set it as your default tool for creating signatures\n> using using signingtool.default\n>\n> This subsumes the existing gpg config, as we have baked-in\n> config for signingtool.gpg that works just like the current\n> hard-coded behavior. And setting gpg.program becomes an\n> alias for signingtool.gpg.program.\n>\n> This is enough to plug in gpgsm like:\n>\n> [signingtool \"gpgsm\"]\n> program = gpgsm\n> pemtype = \"SIGNED MESSAGE\"\n>\n> In the future, this config scheme gives us options for\n> extending to other tools:\n>\n> - the tools all have to accept gpg-like options for now,\n> but we could add signingtool.interface to meet other\n> standards\n>\n> - we only match PEM headers now, but we could add other\n> config for matching non-PEM types\n>\n> The implementation is still in gpg-interface.c, and most of\n> the code internally refers to this as \"gpg\". I've left it\n> this way to keep the diff sane, and to make it clear that\n> we're not breaking anything gpg-related. This is probably OK\n> for now, as our tools must be gpg-like anyway. But renaming\n> everything would be an obvious next-step refactoring if we\n> want to offer support for more exotic tools (e.g., people\n> have asked before on the list about using OpenBSD signify).\n\nPlease sign off your patch, see Documentation/SubmittingPatches\n\n> ---\n> Documentation/config.txt | 40 +++++++++---\n> builtin/fmt-merge-msg.c | 6 +-\n> builtin/receive-pack.c | 7 +-\n> builtin/tag.c | 2 +-\n> commit.c | 2 +-\n> gpg-interface.c | 165 ++++++++++++++++++++++++++++++++++++++++++-----\n> gpg-interface.h | 24 ++++++-\n> log-tree.c | 7 +-\n> ref-filter.c | 2 +-\n> t/lib-gpg.sh | 26 ++++++++\n> t/t7510-signed-commit.sh | 32 ++++++++-\n> tag.c | 2 +-\n> 12 files changed, 272 insertions(+), 43 deletions(-)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 4e0cff87f6..7906123a59 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -1727,16 +1727,38 @@ grep.fallbackToNoIndex::\n> If set to true, fall back to git grep --no-index if git grep\n> is executed outside of a git repository. Defaults to false.\n>\n> -gpg.program::\n> - Use this custom program instead of \"`gpg`\" found on `$PATH` when\n> - making or verifying a PGP signature. The program must support the\n> - same command-line interface as GPG, namely, to verify a detached\n> - signature, \"`gpg --verify $file - <$signature`\" is run, and the\n> - program is expected to signal a good signature by exiting with\n> - code 0, and to generate an ASCII-armored detached signature, the\n> - standard input of \"`gpg -bsau $key`\" is fed with the contents to be\n> +signingtool..program::\n> + The name of the program on `$PATH` to execute when making or\n> + verifying a signature. This program will be used for making\n> + signatures if `` is configured as `signingtool.default`.\n> + This program will be used for verifying signatures whose PEM\n> + block type matches `signingtool..pemtype` (see below). The\n> + program must support the same command-line interface as GPG.\n> + To verify a detached signature,\n> + \"`gpg --verify $file - <$signature`\" is run, and the program is\n> + expected to signal a good signature by exiting with code 0.\n> + To generate an ASCII-armored detached signature, the standard\n> + input of \"`gpg -bsau $key`\" is fed with the contents to be\n> signed, and the program is expected to send the result to its\n> - standard output.\n> + standard output. By default, `signingtool.gpg.program` is set to\n> + `gpg`.\n\nI wonder if the mention of the default is best put at the pgp.program\ninstead of here, although the mention of it being an alias strongly suggest\nwe'd want to deprecate it eventually.\n\n> +\n> +signingtool..pemtype::\n> + The PEM block type associated with the signing tool named\n> + ``. For example, the block type of a GPG signature\n> + starting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n> + SIGNATURE`. When verifying a signature with this PEM block type\n> + the program specified in `signingtool..program` will be\n> + used. By default `signingtool.gpg.pemtype` contains `PGP\n> + SIGNATURE` and `PGP MESSAGE`.\n> +\n> +signingtool.default::\n> + The `` of the signing tool to use when creating\n> + signatures (e.g., setting it to \"foo\" will use use the program\n> + specified by `signingtool.foo.program`). Defaults to `gpg`.\n> +\n> +gpg.program::\n> + Historical alias for `signingtool.gpg.program`.\n>\n> gui.commitMsgWidth::\n> Defines how wide the commit message window is in the\n\n\n> +/*\n> + * Our default tool config is too complicated to specify as a constant\n> + * initializer, so we lazily create it as needed.\n> + */\n> +static void init_signing_tool_defaults(void) {\n\nCoding Style: please put the opening brace in a new\nline when staring a function (but not for statements).\n"},{"id":"344245","messageId":"CAPig+cQQuvr5hOXgLM7rwHDK-q4eJrNLR==F+mQfHrBRjj0vWg@mail.gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-7-mastahyeti@gmail.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-09T21:13:46Z","receivedAt":"2018-04-09T21:13:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews wrote:\n> From: Jeff King \n>\n> A signed tag has a detached signature like this:\n>\n> object ...\n> [...more header...]\n>\n> This is the tag body.\n>\n> -----BEGIN PGP SIGNATURE-----\n> [opaque gpg data]\n> -----END PGP SIGNATURE-----\n>\n> Our parser finds the _first_ line that appears to start a\n> PGP signature block, meaning we may be confused by a\n> signature (or a signature-like line) in the actual body.\n> Let's keep parsing and always find the final block, which\n> should be the detached signature over all of the preceding\n> content.\n> ---\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> @@ -110,11 +110,17 @@ static int is_gpg_start(const char *line)\n> size_t parse_signature(const char *buf, size_t size)\n> {\n> size_t len = 0;\n> - while (len < size && !is_gpg_start(buf + len)) {\n> - const char *eol = memchr(buf + len, '\\n', size - len);\n> + size_t match = size;\n\nIf no GPG-start line is found then 'size' will be returned, which\nmatches the logic before this change. Okay.\n\n> + while (len < size) {\n> + const char *eol;\n> +\n> + if (is_gpg_start(buf + len))\n> + match = len;\n\nOtherwise, the position of the final GPG-start line will be remembered\nand returned. Makes sense.\n\n> + eol = memchr(buf + len, '\\n', size - len);\n> len += eol ? eol - (buf + len) + 1 : size - len;\n> }\n> - return len;\n> + return match;\n> }\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> @@ -1059,6 +1059,17 @@ test_expect_success GPG \\\n> +test_expect_success GPG 'signed tag with embedded PGP message' '\n> + cat >msg <<-\\EOF &&\n> + -----BEGIN PGP MESSAGE-----\n> +\n> + this is not a real PGP message\n> + -----END PGP MESSAGE-----\n> + EOF\n\nThis bogus PGP message is just in the body...\n\n> + git tag -s -F msg confusing-pgp-message &&\n\nand \"git tag -s\" adds the real PGP message at the end...\n\n> + git tag -v confusing-pgp-message\n\nand the new logic finds the real PGP message rather than the bogus\none, so \"git tag -v\" exits successfully. Looks good.\n\n> +'\n"},{"id":"344303","messageId":"CAPig+cT3AobThgZ15iquyRQG0Qes1ZzQxycXcgHYuwQCuDEDBQ@mail.gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-9-mastahyeti@gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-10T08:24:27Z","receivedAt":"2018-04-10T08:24:34Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews wrote:\n> [...]\n> This patch introduces a set of configuration options for\n> defining a \"signing tool\", of which gpg may be just one.\n> With this patch you can:\n>\n> - define a new tool \"foo\" with signingtool.foo.program\n>\n> - map PEM strings to it for verification using\n> signingtool.foo.pemtype\n>\n> - set it as your default tool for creating signatures\n> using using signingtool.default\n\ns/using using/using/\n\n> This subsumes the existing gpg config, as we have baked-in\n> config for signingtool.gpg that works just like the current\n> hard-coded behavior. And setting gpg.program becomes an\n> alias for signingtool.gpg.program.\n>\n> This is enough to plug in gpgsm like:\n>\n> [signingtool \"gpgsm\"]\n> program = gpgsm\n> pemtype = \"SIGNED MESSAGE\"\n\nHow confident are we that _all_ possible signing programs will conform\nto the \"-----BEGIN %s-----\" pattern? If we're not confident, then\nperhaps the user should be providing the full string here, not just\nthe '%s' part?\n\nFurther, I can infer from PGP itself, as well as from reading the code\nthat the 'pemtype' key can be specified multiple times; it would be\nnice to mention that in the commit message.\n\n> [...]\n> The implementation is still in gpg-interface.c, and most of\n> the code internally refers to this as \"gpg\". I've left it\n> this way to keep the diff sane, and to make it clear that\n> we're not breaking anything gpg-related. This is probably OK\n> for now, as our tools must be gpg-like anyway. But renaming\n> everything would be an obvious next-step refactoring if we\n> want to offer support for more exotic tools (e.g., people\n> have asked before on the list about using OpenBSD signify).\n\nI can't decide if this paragraph belongs in the commit message proper\n(as it currently is) or if it is commentary which should be placed\nbelow the \"---\" line just above the diffstat. It feels more like\ncommentary, but not a big deal, and not itself worth a re-roll.\n\n> ---\n> Documentation/config.txt | 40 +++++++++---\n> builtin/fmt-merge-msg.c | 6 +-\n> builtin/receive-pack.c | 7 +-\n> builtin/tag.c | 2 +-\n> commit.c | 2 +-\n> gpg-interface.c | 165 ++++++++++++++++++++++++++++++++++++++++++-----\n> gpg-interface.h | 24 ++++++-\n> log-tree.c | 7 +-\n> ref-filter.c | 2 +-\n> t/lib-gpg.sh | 26 ++++++++\n> t/t7510-signed-commit.sh | 32 ++++++++-\n> tag.c | 2 +-\n> 12 files changed, 272 insertions(+), 43 deletions(-)\n\nDue to its length, this patch is painfully time-consuming to review,\nand asks reviewers to keep track of too many details at one time. As a\nconsequence, it's likely to scare away potential reviewers and limit\nthe quality of those reviews it does receive. If you break the changes\ndown into a series of much smaller patches which hold the hands of\nreviewers, then you are likely to attract more and better reviews.\nPlease consider doing so.\n\nEach patch should be reasonably small and easy to digest. Each patch\nshould build logically upon the patch or patches preceding it, thus\nincrementally building up a picture of the complete change. A (very)\nrough idea of a series of smaller patches corresponding to this\nfeature might be:\n\n1. introduce 'struct signing_tool' and the bare machinery for managing them\n\n2. convert PGP from a hard-coded signer to a 'signing_tool' signer\n\n3. add support for the new configuration variables\n\nIt's likely that these steps can be broken into even smaller, more\nreviewer-friendly ones; exactly how to do so may become apparent as\nyou think about how the patch series should be structured. For\ninstance, perhaps step 3 could be divided into:\n\n3.1. add support for \"signingtool.foo\" variables\n3.2. retrofit \"gpg.program\" to be alias of \"signingtool.gpg.program\"\n\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> @@ -1727,16 +1727,38 @@ grep.fallbackToNoIndex::\n> -gpg.program::\n> - Use this custom program instead of \"`gpg`\" found on `$PATH` when\n> - making or verifying a PGP signature. The program must support the\n> - same command-line interface as GPG, namely, to verify a detached\n> - signature, \"`gpg --verify $file - <$signature`\" is run, and the\n> - program is expected to signal a good signature by exiting with\n> - code 0, and to generate an ASCII-armored detached signature, the\n> - standard input of \"`gpg -bsau $key`\" is fed with the contents to be\n> +signingtool..program::\n> + The name of the program on `$PATH` to execute when making or\n\nWhy does the program need to be on $PATH? That seems like an\nunnecessarily harsh limitation, one which wasn't shared by\ngpg.program. (Reading the code, it looks like 'program' does not in\nfact need to be on $PATH, so this documentation is wrong.)\n\n> + verifying a signature. This program will be used for making\n> + signatures if `` is configured as `signingtool.default`.\n> + This program will be used for verifying signatures whose PEM\n> + block type matches `signingtool..pemtype` (see below). The\n> + program must support the same command-line interface as GPG.\n> [...]\n> +signingtool..pemtype::\n> + The PEM block type associated with the signing tool named\n> + ``. For example, the block type of a GPG signature\n> + starting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n> + SIGNATURE`. When verifying a signature with this PEM block type\n> + the program specified in `signingtool..program` will be\n> + used. By default `signingtool.gpg.pemtype` contains `PGP\n> + SIGNATURE` and `PGP MESSAGE`.\n\nAlthough it's somewhat inferred for the PGP case, the documentation\nreally needs to state explicitly that multiple 'pemtype's can be\nspecified, and it needs to explain how to do so. Reading the code, I\nsee that one does so by specifying 'pemtype' key multiple times, but\nthe documentation should say this.\n\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> @@ -143,12 +216,43 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n> if (!strcmp(var, \"gpg.program\")) {\n> + struct signing_tool *tool = get_or_create_signing_tool(\"gpg\");\n\nNot at all worth a re-roll, but the get_or_create_signing_tool()\ninvocation could be moved below the \"if (!value)\" conditional. (The\ndeclaration of 'tool', of course, would remain here.)\n\n> if (!value)\n> return config_error_nonbool(var);\n> - gpg_program = xstrdup(value);\n> +\n> + free(tool->program);\n> + tool->program = xstrdup(value);\n> return 0;\n> }\n> @@ -200,15 +306,42 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n> int verify_signed_buffer(const char *payload, size_t payload_size,\n> const char *signature, size_t signature_size,\n> - struct strbuf *gpg_output, struct strbuf *gpg_status)\n> + struct strbuf *gpg_output, struct strbuf *gpg_status,\n> + const struct signing_tool *tool)\n> {\n> [...]\n> + if (!tool) {\n> + parse_signature(signature, signature_size, &tool);\n> + if (!tool) {\n> + /*\n> + * The caller didn't tell us which tool to use, and we\n> + * didn't recognize the format. Historically we've fed\n> + * these cases to blindly to gpg, so let's continue to\n\ns/to blindly/blindly/\n\n> + * do so.\n> + */\n> + tool = get_signing_tool(\"gpg\");\n> + }\n> + }\n> diff --git a/gpg-interface.h b/gpg-interface.h\n> @@ -48,10 +60,16 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n> * Run \"gpg\" to see if the payload matches the detached signature.\n> * gpg_output, when set, receives the diagnostic output from GPG.\n> * gpg_status, when set, receives the status output from GPG.\n> + *\n> + * Typically the \"tool\" argument should come from a previous call to\n\ns/Typically/&,/\n\n> + * parse_signature(). If it's NULL, then verify_signed_buffer() will\n> + * try to choose the appropriate tool based on the contents of the\n> + * \"signature\" buffer.\n> */\n> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n> @@ -56,3 +56,29 @@ sanitize_pgp() {\n> +create_fake_signer () {\n> + write_script fake-signer <<-\\EOF\n> + if [ \"$1 $2\" = \"--status-fd=2 -bsau\" ]; then\n\nStyle: This codebase uses 'test' rather than '['. Also, 'then' is\nplaced on its own line and the semicolon omitted.\n\n> + echo >&2 \"[GNUPG:] BEGIN_SIGNING\"\n> + echo >&2 \"[GNUPG:] SIG_CREATED D 1 SHA256 0 1513792449 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70\"\n> + # avoid \"-\" in echo arguments\n> + printf \"%s\\n\" \\\n> + \"-----BEGIN FAKE SIGNER SIGNATURE-----\" \\\n> + \"fake-signature\" \\\n> + \"-----END FAKE SIGNER SIGNATURE-----\"\n\nIf you use 'cat' with a here-doc, then you don't need the comment\nabout avoiding \"-\".\n\n cat <<-\\END\n -----BEGIN FAKE SIGNER SIGNATURE-----\n fake-signature\n -----END FAKE SIGNER SIGNATURE-----\n END\n\n> + exit 0\n> +\n> + elif [ \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\" ]; then\n> + echo \"[GNUPG:] NEWSIG\"\n> + echo \"[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\"\n> + echo \"[GNUPG:] TRUST_FULLY 0 shell\"\n\nAnother good place to use 'cat' with here-doc.\n\n> + echo >&2 \"Good signature from /CN=Some User/EMail=some@user.email\"\n> + exit 0\n> +\n> + else\n> + echo \"bad arguments\" 1>&2\n> + exit 1\n> + fi\n> + EOF\n> +}\n> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\n> @@ -51,13 +55,33 @@ test_expect_success GPG 'create signed commits' '\n> + git tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree}) &&\n> +\n> + git config signingtool.fake.program \"$TRASH_DIRECTORY/fake-signer\" &&\n> + git config signingtool.fake.pemtype \"FAKE SIGNER SIGNATURE\" &&\n> + echo 11 >file && test_tick && git commit -a -m eleventh &&\n\nIt's normally frowned upon in tests to string together a bunch of\n&&-chained commands on a single line, but since this script is already\nfull of this stylized 'commit' invocation, it may be okay. However...\n\n> + git tag eleventh-pgp-signed &&\n> + git cat-file -p eleventh-pgp-signed >actual &&\n> + grep \"PGP SIGNATURE\" actual &&\n> +\n> + git config gpg.program \"$TRASH_DIRECTORY/fake-signer\" &&\n> + echo 12 >file && test_tick && git commit -a -m twelfth && test_unconfig gpg.program &&\n\nPlace the test_unconfig() on its own line so it can be clearly seen by\nsomeone scanning an eye over the test; otherwise, the test_unconfig()\nis easily overlooked, with the result that the reader may get a false\nimpression of what's going on.\n\n> + git tag twelfth-fake-signed &&\n> + git cat-file -p twelfth-fake-signed >actual &&\n> + grep \"FAKE SIGNER SIGNATURE\" actual &&\n> +\n> + git config signingtool.default fake &&\n> + echo 13 >file && test_tick && git commit -a -m thirteenth && test_unconfig signingtool.default &&\n\nDitto.\n\n> + git tag thirteenth-fake-signed &&\n> + git cat-file -p thirteenth-fake-signed >actual &&\n> + grep \"FAKE SIGNER SIGNATURE\" actual\n> '\n"},{"id":"344306","messageId":"xmqqr2nn2ylz.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"20180409204129.43537-9-mastahyeti@gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-10T09:35:04Z","receivedAt":"2018-04-10T09:35:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Toews writes:\n\n> From: Ben Toews \n>\n> Currently you can only sign commits and tags using \"gpg\".\n> ...\n> have asked before on the list about using OpenBSD signify).\n> ---\n\nMissing sign-off.\n\n> -gpg.program::\n> -\tUse this custom program instead of \"`gpg`\" found on `$PATH` when\n> -\tmaking or verifying a PGP signature. The program must support the\n> -\tsame command-line interface as GPG, namely, to verify a detached\n> -\tsignature, \"`gpg --verify $file - <$signature`\" is run, and the\n> -\tprogram is expected to signal a good signature by exiting with\n> -\tcode 0, and to generate an ASCII-armored detached signature, the\n> -\tstandard input of \"`gpg -bsau $key`\" is fed with the contents to be\n> +signingtool..program::\n> +\tThe name of the program on `$PATH` to execute when making or\n> +\tverifying a signature.\n\nI think you do not want \"on `$PATH`\", as you should be able to\nspecify a full path /opt/some/where/not/on/my/path/pgp and have it\nwork just fine. The mention of \"found on `$PATH`\" in the original\nis talking about the behaviour _WITHOUT_ the configuration, i.e. by\ndefault we just invoke \"gpg\" and expect that it is found in the\nusual measure, i.e. being on user's $PATH. What you are describing\nin this updated explanation is what happens _WITH_ the configuration.\n\n> +\tThis program will be used for making\n> +\tsignatures if `` is configured as `signingtool.default`.\n> +\tThis program will be used for verifying signatures whose PEM\n> +\tblock type matches `signingtool..pemtype` (see below). The\n> +\tprogram must support the same command-line interface as GPG.\n> +\tTo verify a detached signature,\n> +\t\"`gpg --verify $file - <$signature`\" is run, and the program is\n> +\texpected to signal a good signature by exiting with code 0.\n> +\tTo generate an ASCII-armored detached signature, the standard\n> +\tinput of \"`gpg -bsau $key`\" is fed with the contents to be\n> \tsigned, and the program is expected to send the result to its\n> -\tstandard output.\n> +\tstandard output. By default, `signingtool.gpg.program` is set to\n> +\t`gpg`.\n\nI do not think the description is wrong per-se, but reading it made\nme realize that with this \"custom\" program, you still require that\nthe \"custom\" program MUST accept the command line options as if it\nwere an implementation of GPG. Most likely you'd write a thin\nwrapper to call your custom program with whatever options that are\nappropriate when asked to --verify or -bsau (aka \"sign\")? If that\nis the case, I have to wonder if such a wrappper program can also\ntrivially reformat the --- BEGIN WHATEVER --- block and behave as if\nit were an implementation of GPG. That makes the primary point of\nthis long series somewhat moot, as we won't need that pemtype thing\nat all, no?\n\n> +signingtool..pemtype::\n> +\tThe PEM block type associated with the signing tool named\n> +\t``. For example, the block type of a GPG signature\n> +\tstarting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n> +\tSIGNATURE`. When verifying a signature with this PEM block type\n> +\tthe program specified in `signingtool..program` will be\n> +\tused. By default `signingtool.gpg.pemtype` contains `PGP\n> +\tSIGNATURE` and `PGP MESSAGE`.\n\nAs Eric noted elsewhere, I suspect that it is cleaner and more\nuseful if this were *NOT* \"pemtype\" but were \"boundary\", i.e.\nletting \"-----BEGIN PGP SIGNATURE-----\\n\" string be specified.\n\n> +signingtool.default::\n> +\tThe `` of the signing tool to use when creating\n> +\tsignatures (e.g., setting it to \"foo\" will use use the program\n> +\tspecified by `signingtool.foo.program`). Defaults to `gpg`.\n\nWill there be a command line option to say \"I may usually be using\nwhatever I configured with signingtool.default, but for this single\ninvocation only, let me use something else\"? Without such a command\nline option that overrides such a default, I do not quite get the\npoint of adding this configuration variable.\n\nThanks.\n"},{"id":"344308","messageId":"xmqqlgdv2y66.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"20180409204129.43537-7-mastahyeti@gmail.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-10T09:44:33Z","receivedAt":"2018-04-10T09:44:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Toews writes:\n\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index ee093b393d..e3f1e014aa 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1059,6 +1059,17 @@ test_expect_success GPG \\\n> \tgit tag -v blanknonlfile-signed-tag\n> '\n> \n> +test_expect_success GPG 'signed tag with embedded PGP message' '\n> +\tcat >msg <<-\\EOF &&\n> +\t-----BEGIN PGP MESSAGE-----\n> +\n> +\tthis is not a real PGP message\n> +\t-----END PGP MESSAGE-----\n> +\tEOF\n> +\tgit tag -s -F msg confusing-pgp-message &&\n> +\tgit tag -v confusing-pgp-message\n> +'\n> +\n> # messages with commented lines for signed tags:\n> \n> cat >sigcommentsfile <sigblanknonlfile\n get_tag_header blanknonlfile-signed-tag $commit commit $time >expect\n echo '-----BEGIN PGP SIGNATURE-----' >>expect\n test_expect_success GPG \\\n 'creating a signed tag with spaces and no newline should succeed' '\n git tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n get_tag_msg blanknonlfile-signed-tag >actual &&\n test_cmp expect actual &&\n git tag -v signed-tag\n '\n\nwhich appear between the pre- and post- context of the lines you are\ninserting? They date back to 2007-2009.\n\n"},{"id":"344346","messageId":"20180410143234.GA15346@sigill.intra.peff.net","threadId":"48253","inReplyTo":"CAPig+cQb+ZDWgCNVPX8_eFqUy-_crQ4wtrYhv-6kwasGwLTWuA@mail.gmail.com","subject":"Re: [PATCH 1/8] gpg-interface: handle bool user.signingkey","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-04-10T14:32:35Z","receivedAt":"2018-04-10T14:32:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 09, 2018 at 04:55:26PM -0400, Eric Sunshine wrote:\n\n> Peff's Signed-off-by: is missing. Also, since you're forwarding this\n> patch on Peff's behalf, your Signed-off-by: should follow his. Same\n> comment applies to all patches by Peff in this series.\n\nI usually sign-off as I send to the list, but I passed these patches to\nBen via a repository. For the record, it's OK to add my s-o-b to the\nwhole series.\n\n-Peff\n"},{"id":"344348","messageId":"CAE=pOyFj+WZBB6Zp35d=mYNsxcJK-LwOxseWXSmY_hs+Lwugpg@mail.gmail.com","threadId":"48253","inReplyTo":"xmqqlgdv2y66.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-10T14:47:39Z","receivedAt":"2018-04-10T14:48:05Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Tue, Apr 10, 2018 at 3:44 AM, Junio C Hamano wrote:\n> Ben Toews writes:\n>\n>> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n>> index ee093b393d..e3f1e014aa 100755\n>> --- a/t/t7004-tag.sh\n>> +++ b/t/t7004-tag.sh\n>> @@ -1059,6 +1059,17 @@ test_expect_success GPG \\\n>> git tag -v blanknonlfile-signed-tag\n>> '\n>>\n>> +test_expect_success GPG 'signed tag with embedded PGP message' '\n>> + cat >msg <<-\\EOF &&\n>> + -----BEGIN PGP MESSAGE-----\n>> +\n>> + this is not a real PGP message\n>> + -----END PGP MESSAGE-----\n>> + EOF\n>> + git tag -s -F msg confusing-pgp-message &&\n>> + git tag -v confusing-pgp-message\n>> +'\n>> +\n>> # messages with commented lines for signed tags:\n>>\n>> cat >sigcommentsfile <\n> Hmmmm, what vintage of our codebase is this patch based on? Did I\n> miss a patch that removes these lines\n>\n>\n> printf ' ' >sigblanknonlfile\n> get_tag_header blanknonlfile-signed-tag $commit commit $time >expect\n> echo '-----BEGIN PGP SIGNATURE-----' >>expect\n> test_expect_success GPG \\\n> 'creating a signed tag with spaces and no newline should succeed' '\n> git tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n> get_tag_msg blanknonlfile-signed-tag >actual &&\n> test_cmp expect actual &&\n> git tag -v signed-tag\n> '\n>\n> which appear between the pre- and post- context of the lines you are\n> inserting? They date back to 2007-2009.\n>\n\nThat test was fixed a week ago:\nhttps://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd\n\n\n\n-- \n-Ben Toews\n"},{"id":"344350","messageId":"CAE=pOyGnLBTJ7goduOu-bqTuqLxvwECVgaVGzFZRCx3RcuAWcQ@mail.gmail.com","threadId":"48253","inReplyTo":"CAPig+cT3AobThgZ15iquyRQG0Qes1ZzQxycXcgHYuwQCuDEDBQ@mail.gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-10T15:00:00Z","receivedAt":"2018-04-10T15:00:28Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Tue, Apr 10, 2018 at 2:24 AM, Eric Sunshine wrote:\n> On Mon, Apr 9, 2018 at 4:41 PM, Ben Toews wrote:\n>> [...]\n>> This patch introduces a set of configuration options for\n>> defining a \"signing tool\", of which gpg may be just one.\n>> With this patch you can:\n>>\n>> - define a new tool \"foo\" with signingtool.foo.program\n>>\n>> - map PEM strings to it for verification using\n>> signingtool.foo.pemtype\n>>\n>> - set it as your default tool for creating signatures\n>> using using signingtool.default\n>\n> s/using using/using/\n>\n>> This subsumes the existing gpg config, as we have baked-in\n>> config for signingtool.gpg that works just like the current\n>> hard-coded behavior. And setting gpg.program becomes an\n>> alias for signingtool.gpg.program.\n>>\n>> This is enough to plug in gpgsm like:\n>>\n>> [signingtool \"gpgsm\"]\n>> program = gpgsm\n>> pemtype = \"SIGNED MESSAGE\"\n>\n> How confident are we that _all_ possible signing programs will conform\n> to the \"-----BEGIN %s-----\" pattern? If we're not confident, then\n> perhaps the user should be providing the full string here, not just\n> the '%s' part?\n\nThese changes are only intended to work with PEM encoded signatures.\nThe new config format leaves room for working with other signature\nformats in the future, though this would require further code changes.\nRequiring the user to specify the whole PEM start/end markers in the\nconfig doesn't make sense to me, since it assumes that non-PEM\nencodings would have similarly simple start/end delimiters.\n\n>\n> Further, I can infer from PGP itself, as well as from reading the code\n> that the 'pemtype' key can be specified multiple times; it would be\n> nice to mention that in the commit message.\n>\n>> [...]\n>> The implementation is still in gpg-interface.c, and most of\n>> the code internally refers to this as \"gpg\". I've left it\n>> this way to keep the diff sane, and to make it clear that\n>> we're not breaking anything gpg-related. This is probably OK\n>> for now, as our tools must be gpg-like anyway. But renaming\n>> everything would be an obvious next-step refactoring if we\n>> want to offer support for more exotic tools (e.g., people\n>> have asked before on the list about using OpenBSD signify).\n>\n> I can't decide if this paragraph belongs in the commit message proper\n> (as it currently is) or if it is commentary which should be placed\n> below the \"---\" line just above the diffstat. It feels more like\n> commentary, but not a big deal, and not itself worth a re-roll.\n>\n>> ---\n>> Documentation/config.txt | 40 +++++++++---\n>> builtin/fmt-merge-msg.c | 6 +-\n>> builtin/receive-pack.c | 7 +-\n>> builtin/tag.c | 2 +-\n>> commit.c | 2 +-\n>> gpg-interface.c | 165 ++++++++++++++++++++++++++++++++++++++++++-----\n>> gpg-interface.h | 24 ++++++-\n>> log-tree.c | 7 +-\n>> ref-filter.c | 2 +-\n>> t/lib-gpg.sh | 26 ++++++++\n>> t/t7510-signed-commit.sh | 32 ++++++++-\n>> tag.c | 2 +-\n>> 12 files changed, 272 insertions(+), 43 deletions(-)\n>\n> Due to its length, this patch is painfully time-consuming to review,\n> and asks reviewers to keep track of too many details at one time. As a\n> consequence, it's likely to scare away potential reviewers and limit\n> the quality of those reviews it does receive. If you break the changes\n> down into a series of much smaller patches which hold the hands of\n> reviewers, then you are likely to attract more and better reviews.\n> Please consider doing so.\n>\n> Each patch should be reasonably small and easy to digest. Each patch\n> should build logically upon the patch or patches preceding it, thus\n> incrementally building up a picture of the complete change. A (very)\n> rough idea of a series of smaller patches corresponding to this\n> feature might be:\n>\n> 1. introduce 'struct signing_tool' and the bare machinery for managing them\n>\n> 2. convert PGP from a hard-coded signer to a 'signing_tool' signer\n>\n> 3. add support for the new configuration variables\n>\n> It's likely that these steps can be broken into even smaller, more\n> reviewer-friendly ones; exactly how to do so may become apparent as\n> you think about how the patch series should be structured. For\n> instance, perhaps step 3 could be divided into:\n>\n> 3.1. add support for \"signingtool.foo\" variables\n> 3.2. retrofit \"gpg.program\" to be alias of \"signingtool.gpg.program\"\n>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> @@ -1727,16 +1727,38 @@ grep.fallbackToNoIndex::\n>> -gpg.program::\n>> - Use this custom program instead of \"`gpg`\" found on `$PATH` when\n>> - making or verifying a PGP signature. The program must support the\n>> - same command-line interface as GPG, namely, to verify a detached\n>> - signature, \"`gpg --verify $file - <$signature`\" is run, and the\n>> - program is expected to signal a good signature by exiting with\n>> - code 0, and to generate an ASCII-armored detached signature, the\n>> - standard input of \"`gpg -bsau $key`\" is fed with the contents to be\n>> +signingtool..program::\n>> + The name of the program on `$PATH` to execute when making or\n>\n> Why does the program need to be on $PATH? That seems like an\n> unnecessarily harsh limitation, one which wasn't shared by\n> gpg.program. (Reading the code, it looks like 'program' does not in\n> fact need to be on $PATH, so this documentation is wrong.)\n>\n\nYep. It looks like this inaccuracy was carried over from the existing docs.\n\n>> + verifying a signature. This program will be used for making\n>> + signatures if `` is configured as `signingtool.default`.\n>> + This program will be used for verifying signatures whose PEM\n>> + block type matches `signingtool..pemtype` (see below). The\n>> + program must support the same command-line interface as GPG.\n>> [...]\n>> +signingtool..pemtype::\n>> + The PEM block type associated with the signing tool named\n>> + ``. For example, the block type of a GPG signature\n>> + starting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n>> + SIGNATURE`. When verifying a signature with this PEM block type\n>> + the program specified in `signingtool..program` will be\n>> + used. By default `signingtool.gpg.pemtype` contains `PGP\n>> + SIGNATURE` and `PGP MESSAGE`.\n>\n> Although it's somewhat inferred for the PGP case, the documentation\n> really needs to state explicitly that multiple 'pemtype's can be\n> specified, and it needs to explain how to do so. Reading the code, I\n> see that one does so by specifying 'pemtype' key multiple times, but\n> the documentation should say this.\n>\n>> diff --git a/gpg-interface.c b/gpg-interface.c\n>> @@ -143,12 +216,43 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n>> if (!strcmp(var, \"gpg.program\")) {\n>> + struct signing_tool *tool = get_or_create_signing_tool(\"gpg\");\n>\n> Not at all worth a re-roll, but the get_or_create_signing_tool()\n> invocation could be moved below the \"if (!value)\" conditional. (The\n> declaration of 'tool', of course, would remain here.)\n>\n>> if (!value)\n>> return config_error_nonbool(var);\n>> - gpg_program = xstrdup(value);\n>> +\n>> + free(tool->program);\n>> + tool->program = xstrdup(value);\n>> return 0;\n>> }\n>> @@ -200,15 +306,42 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n>> int verify_signed_buffer(const char *payload, size_t payload_size,\n>> const char *signature, size_t signature_size,\n>> - struct strbuf *gpg_output, struct strbuf *gpg_status)\n>> + struct strbuf *gpg_output, struct strbuf *gpg_status,\n>> + const struct signing_tool *tool)\n>> {\n>> [...]\n>> + if (!tool) {\n>> + parse_signature(signature, signature_size, &tool);\n>> + if (!tool) {\n>> + /*\n>> + * The caller didn't tell us which tool to use, and we\n>> + * didn't recognize the format. Historically we've fed\n>> + * these cases to blindly to gpg, so let's continue to\n>\n> s/to blindly/blindly/\n>\n>> + * do so.\n>> + */\n>> + tool = get_signing_tool(\"gpg\");\n>> + }\n>> + }\n>> diff --git a/gpg-interface.h b/gpg-interface.h\n>> @@ -48,10 +60,16 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n>> * Run \"gpg\" to see if the payload matches the detached signature.\n>> * gpg_output, when set, receives the diagnostic output from GPG.\n>> * gpg_status, when set, receives the status output from GPG.\n>> + *\n>> + * Typically the \"tool\" argument should come from a previous call to\n>\n> s/Typically/&,/\n>\n>> + * parse_signature(). If it's NULL, then verify_signed_buffer() will\n>> + * try to choose the appropriate tool based on the contents of the\n>> + * \"signature\" buffer.\n>> */\n>> diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n>> @@ -56,3 +56,29 @@ sanitize_pgp() {\n>> +create_fake_signer () {\n>> + write_script fake-signer <<-\\EOF\n>> + if [ \"$1 $2\" = \"--status-fd=2 -bsau\" ]; then\n>\n> Style: This codebase uses 'test' rather than '['. Also, 'then' is\n> placed on its own line and the semicolon omitted.\n>\n>> + echo >&2 \"[GNUPG:] BEGIN_SIGNING\"\n>> + echo >&2 \"[GNUPG:] SIG_CREATED D 1 SHA256 0 1513792449 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70\"\n>> + # avoid \"-\" in echo arguments\n>> + printf \"%s\\n\" \\\n>> + \"-----BEGIN FAKE SIGNER SIGNATURE-----\" \\\n>> + \"fake-signature\" \\\n>> + \"-----END FAKE SIGNER SIGNATURE-----\"\n>\n> If you use 'cat' with a here-doc, then you don't need the comment\n> about avoiding \"-\".\n>\n> cat <<-\\END\n> -----BEGIN FAKE SIGNER SIGNATURE-----\n> fake-signature\n> -----END FAKE SIGNER SIGNATURE-----\n> END\n>\n>> + exit 0\n>> +\n>> + elif [ \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\" ]; then\n>> + echo \"[GNUPG:] NEWSIG\"\n>> + echo \"[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\"\n>> + echo \"[GNUPG:] TRUST_FULLY 0 shell\"\n>\n> Another good place to use 'cat' with here-doc.\n>\n>> + echo >&2 \"Good signature from /CN=Some User/EMail=some@user.email\"\n>> + exit 0\n>> +\n>> + else\n>> + echo \"bad arguments\" 1>&2\n>> + exit 1\n>> + fi\n>> + EOF\n>> +}\n>> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\n>> @@ -51,13 +55,33 @@ test_expect_success GPG 'create signed commits' '\n>> + git tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree}) &&\n>> +\n>> + git config signingtool.fake.program \"$TRASH_DIRECTORY/fake-signer\" &&\n>> + git config signingtool.fake.pemtype \"FAKE SIGNER SIGNATURE\" &&\n>> + echo 11 >file && test_tick && git commit -a -m eleventh &&\n>\n> It's normally frowned upon in tests to string together a bunch of\n> &&-chained commands on a single line, but since this script is already\n> full of this stylized 'commit' invocation, it may be okay. However...\n>\n>> + git tag eleventh-pgp-signed &&\n>> + git cat-file -p eleventh-pgp-signed >actual &&\n>> + grep \"PGP SIGNATURE\" actual &&\n>> +\n>> + git config gpg.program \"$TRASH_DIRECTORY/fake-signer\" &&\n>> + echo 12 >file && test_tick && git commit -a -m twelfth && test_unconfig gpg.program &&\n>\n> Place the test_unconfig() on its own line so it can be clearly seen by\n> someone scanning an eye over the test; otherwise, the test_unconfig()\n> is easily overlooked, with the result that the reader may get a false\n> impression of what's going on.\n>\n>> + git tag twelfth-fake-signed &&\n>> + git cat-file -p twelfth-fake-signed >actual &&\n>> + grep \"FAKE SIGNER SIGNATURE\" actual &&\n>> +\n>> + git config signingtool.default fake &&\n>> + echo 13 >file && test_tick && git commit -a -m thirteenth && test_unconfig signingtool.default &&\n>\n> Ditto.\n>\n>> + git tag thirteenth-fake-signed &&\n>> + git cat-file -p thirteenth-fake-signed >actual &&\n>> + grep \"FAKE SIGNER SIGNATURE\" actual\n>> '\n\n\n\n-- \n-Ben Toews\n"},{"id":"344358","messageId":"CAE=pOyHey_8kJ0JsA3ROhpE9qn+HivVQgwd+Wdonw8L6D+9MUg@mail.gmail.com","threadId":"48253","inReplyTo":"xmqqr2nn2ylz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-10T16:01:36Z","receivedAt":"2018-04-10T16:02:03Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Tue, Apr 10, 2018 at 3:35 AM, Junio C Hamano wrote:\n> Ben Toews writes:\n>\n>> From: Ben Toews \n>>\n>> Currently you can only sign commits and tags using \"gpg\".\n>> ...\n>> have asked before on the list about using OpenBSD signify).\n>> ---\n>\n> Missing sign-off.\n>\n>> -gpg.program::\n>> - Use this custom program instead of \"`gpg`\" found on `$PATH` when\n>> - making or verifying a PGP signature. The program must support the\n>> - same command-line interface as GPG, namely, to verify a detached\n>> - signature, \"`gpg --verify $file - <$signature`\" is run, and the\n>> - program is expected to signal a good signature by exiting with\n>> - code 0, and to generate an ASCII-armored detached signature, the\n>> - standard input of \"`gpg -bsau $key`\" is fed with the contents to be\n>> +signingtool..program::\n>> + The name of the program on `$PATH` to execute when making or\n>> + verifying a signature.\n>\n> I think you do not want \"on `$PATH`\", as you should be able to\n> specify a full path /opt/some/where/not/on/my/path/pgp and have it\n> work just fine. The mention of \"found on `$PATH`\" in the original\n> is talking about the behaviour _WITHOUT_ the configuration, i.e. by\n> default we just invoke \"gpg\" and expect that it is found in the\n> usual measure, i.e. being on user's $PATH. What you are describing\n> in this updated explanation is what happens _WITH_ the configuration.\n>\n>> + This program will be used for making\n>> + signatures if `` is configured as `signingtool.default`.\n>> + This program will be used for verifying signatures whose PEM\n>> + block type matches `signingtool..pemtype` (see below). The\n>> + program must support the same command-line interface as GPG.\n>> + To verify a detached signature,\n>> + \"`gpg --verify $file - <$signature`\" is run, and the program is\n>> + expected to signal a good signature by exiting with code 0.\n>> + To generate an ASCII-armored detached signature, the standard\n>> + input of \"`gpg -bsau $key`\" is fed with the contents to be\n>> signed, and the program is expected to send the result to its\n>> - standard output.\n>> + standard output. By default, `signingtool.gpg.program` is set to\n>> + `gpg`.\n>\n> I do not think the description is wrong per-se, but reading it made\n> me realize that with this \"custom\" program, you still require that\n> the \"custom\" program MUST accept the command line options as if it\n> were an implementation of GPG. Most likely you'd write a thin\n> wrapper to call your custom program with whatever options that are\n> appropriate when asked to --verify or -bsau (aka \"sign\")? If that\n> is the case, I have to wonder if such a wrappper program can also\n> trivially reformat the --- BEGIN WHATEVER --- block and behave as if\n> it were an implementation of GPG. That makes the primary point of\n> this long series somewhat moot, as we won't need that pemtype thing\n> at all, no?\n>\n\nJust because a signature is PEM encoded and claims to be a \"PGP\nSIGNATURE\", doesn't mean it can be understood or verified by a PGP\nimplementation. Without different tools specifying different PEM types\nwe would have no way of knowing which tool to route the signature to\nfor verification.\n\n>> +signingtool..pemtype::\n>> + The PEM block type associated with the signing tool named\n>> + ``. For example, the block type of a GPG signature\n>> + starting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n>> + SIGNATURE`. When verifying a signature with this PEM block type\n>> + the program specified in `signingtool..program` will be\n>> + used. By default `signingtool.gpg.pemtype` contains `PGP\n>> + SIGNATURE` and `PGP MESSAGE`.\n>\n> As Eric noted elsewhere, I suspect that it is cleaner and more\n> useful if this were *NOT* \"pemtype\" but were \"boundary\", i.e.\n> letting \"-----BEGIN PGP SIGNATURE-----\\n\" string be specified.\n>\n>> +signingtool.default::\n>> + The `` of the signing tool to use when creating\n>> + signatures (e.g., setting it to \"foo\" will use use the program\n>> + specified by `signingtool.foo.program`). Defaults to `gpg`.\n>\n> Will there be a command line option to say \"I may usually be using\n> whatever I configured with signingtool.default, but for this single\n> invocation only, let me use something else\"? Without such a command\n> line option that overrides such a default, I do not quite get the\n> point of adding this configuration variable.\n>\n> Thanks.\n\n\n\n-- \n-Ben Toews\n"},{"id":"344396","messageId":"xmqqbmeq3h8o.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"CAE=pOyFj+WZBB6Zp35d=mYNsxcJK-LwOxseWXSmY_hs+Lwugpg@mail.gmail.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-10T21:04:55Z","receivedAt":"2018-04-10T21:05:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Toews writes:\n\n>> Hmmmm, what vintage of our codebase is this patch based on? Did I\n>> miss a patch that removes these lines\n>>\n>>\n>> printf ' ' >sigblanknonlfile\n>> get_tag_header blanknonlfile-signed-tag $commit commit $time >expect\n>> echo '-----BEGIN PGP SIGNATURE-----' >>expect\n>> test_expect_success GPG \\\n>> 'creating a signed tag with spaces and no newline should succeed' '\n>> git tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n>> get_tag_msg blanknonlfile-signed-tag >actual &&\n>> test_cmp expect actual &&\n>> git tag -v signed-tag\n>> '\n>>\n>> which appear between the pre- and post- context of the lines you are\n>> inserting? They date back to 2007-2009.\n>>\n>\n> That test was fixed a week ago:\n> https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd\n\nWell, you cannot expect any reviewer to know about a change that has\nnever been sent to the list and has never been part of even 'pu'\nbranch, no matter how old such a private \"fix\" is. \n\nWhat other unpublished things that are not even on 'pu' do these\npatches depend on?\n"},{"id":"344416","messageId":"xmqqh8oi1zai.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"xmqqbmeq3h8o.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-10T22:17:57Z","receivedAt":"2018-04-10T22:18:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> That test was fixed a week ago:\n>> https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd\n>\n> Well, you cannot expect any reviewer to know about a change that has\n> never been sent to the list and has never been part of even 'pu'\n> branch, no matter how old such a private \"fix\" is. \n>\n> What other unpublished things that are not even on 'pu' do these\n> patches depend on?\n\nAnswering my own question after digging a bit more, now I know that\na99d903 comes from the 'private' branch in peff/git/ repository\nhosted at GitHub [*1*]. The branch builds on 'next' by merging many\nprivate branches, and 'jk/non-pgp-signatures' branch has that commit.\n\n peff.git/private$ git log --oneline --boundary 0c8726318..7a526834e^2\n c9ce7c5b7 gpg-interface: handle multiple signing tools\n 914951682 gpg-interface: handle bool user.signingkey\n 1f2ea84b3 gpg-interface: return signature type from parse_signature()\n 6d2ce6547 gpg-interface: prepare for parsing arbitrary PEM blocks\n fb1d173db gpg-interface: find the last gpg signature line\n 6bc4e7e17 gpg-interface: extract gpg line matching helper\n 4f883ac49 gpg-interface: fix const-correctness of \"eol\" pointer\n ae6529fdb gpg-interface: use size_t for signature buffer size\n 1bca4296b gpg-interface: modernize function declarations\n a99d903f2 t7004: fix mistaken tag name\n - 468165c1d Git 2.17\n\nIt seems to me that Peff did the t7004 change as the earliest\npreliminary step of the series, but it caused confusion when it was\nnot sent as part of the series by mistake. Judging from the shape\nof the topic, I do not think this topic has any other hidden\ndependencies as it builds directly on top of v2.17.0.\n\nFor those who are reading and reviewing from the sideline, I've\nattached that missing 0.9/8 patch at the end of this message.\n\n[Footnote]\n\n*1* I do not know if it deserves to be called a bug, but it\n certainly is an anomaly GitHub exhibits that a99d903f can be\n viewed at https://github.com/git/git/commit/a99d903f..., as if\n it is part of the official git/git history, when it only exists\n in a fork of that repository. I can understand why it happens\n but it certainly is unexpected.\n\n-- >8 --\nFrom: Jeff King \nDate: Tue, 3 Apr 2018 17:10:30 -0400\nSubject: [PATCH 0.9/8] t7004: fix mistaken tag name\n\nWe have a series of tests which create signed tags with\nvarious properties, but one test accidentally verifies a tag\nfrom much earlier in the series.\n---\n t/t7004-tag.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 2aac77af7..ee093b393 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1056,7 +1056,7 @@ test_expect_success GPG \\\n \tgit tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n \tget_tag_msg blanknonlfile-signed-tag >actual &&\n \ttest_cmp expect actual &&\n-\tgit tag -v signed-tag\n+\tgit tag -v blanknonlfile-signed-tag\n '\n \n # messages with commented lines for signed tags:\n-- \n2.17.0-140-g0b0cc9f867\n\n"},{"id":"344451","messageId":"20180411101141.7292-1-szeder.dev@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-9-mastahyeti@gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-04-11T10:11:41Z","receivedAt":"2018-04-11T10:12:00Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\n> diff --git a/gpg-interface.h b/gpg-interface.h\n> index a5e6517ae6..cee0dfe401 100644\n> --- a/gpg-interface.h\n> +++ b/gpg-interface.h\n> @@ -23,15 +23,27 @@ struct signature_check {\n> \tchar *key;\n> };\n> \n> +struct signing_tool {\n> +\tchar *name;\n> +\tchar *program;\n> +\tstruct string_list pemtype;\n> +\tstruct signing_tool *next;\n> +};\n> +\n> void signature_check_clear(struct signature_check *sigc);\n> \n> /*\n> - * Look at GPG signed content (e.g. a signed tag object), whose\n> + * Look for signed content (e.g. a signed tag object), whose\n> * payload is followed by a detached signature on it. Return the\n> * offset where the embedded detached signature begins, or the end of\n> * the data when there is no such signature.\n> + *\n> + * If out_tool is non-NULL and a signature is found, it will be\n> + * pointed at the signing_tool that corresponds to the found\n> + * signature type.\n> */\n> -size_t parse_signature(const char *buf, size_t size);\n> +size_t parse_signature(const char *buf, unsigned long size,\n> +\t\t const struct signing_tool **out_tool);\n\nThis hunk changes the type of the 'size' argument from size_t to\nunsigned long, but leaves the function's signature in\n'gpg-interface.c' unchanged. This breaks the build on 32 bit systems\n(and Windows?), where unsigned long is only 32 bits, and I presume is\nunintended, as it goes against the earlier patch 3/8 \"gpg-interface:\nuse size_t for signature buffer size\".\n\n"},{"id":"344460","messageId":"CAE=pOyE29KwsMOt4CepDXHa=rzz2kT45sMKtUrbQT1TAPEuJQQ@mail.gmail.com","threadId":"48253","inReplyTo":"xmqqh8oi1zai.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 6/8] gpg-interface: find the last gpg signature line","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-11T15:19:48Z","receivedAt":"2018-04-11T15:20:15Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Tue, Apr 10, 2018 at 4:17 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>>> That test was fixed a week ago:\n>>> https://github.com/git/git/commit/a99d903f21d102a5768f19157085a9733aeb68dd\n>>\n>> Well, you cannot expect any reviewer to know about a change that has\n>> never been sent to the list and has never been part of even 'pu'\n>> branch, no matter how old such a private \"fix\" is.\n>>\n>> What other unpublished things that are not even on 'pu' do these\n>> patches depend on?\n>\n> Answering my own question after digging a bit more, now I know that\n> a99d903 comes from the 'private' branch in peff/git/ repository\n> hosted at GitHub [*1*]. The branch builds on 'next' by merging many\n> private branches, and 'jk/non-pgp-signatures' branch has that commit.\n>\n> peff.git/private$ git log --oneline --boundary 0c8726318..7a526834e^2\n> c9ce7c5b7 gpg-interface: handle multiple signing tools\n> 914951682 gpg-interface: handle bool user.signingkey\n> 1f2ea84b3 gpg-interface: return signature type from parse_signature()\n> 6d2ce6547 gpg-interface: prepare for parsing arbitrary PEM blocks\n> fb1d173db gpg-interface: find the last gpg signature line\n> 6bc4e7e17 gpg-interface: extract gpg line matching helper\n> 4f883ac49 gpg-interface: fix const-correctness of \"eol\" pointer\n> ae6529fdb gpg-interface: use size_t for signature buffer size\n> 1bca4296b gpg-interface: modernize function declarations\n> a99d903f2 t7004: fix mistaken tag name\n> - 468165c1d Git 2.17\n>\n> It seems to me that Peff did the t7004 change as the earliest\n> preliminary step of the series, but it caused confusion when it was\n> not sent as part of the series by mistake. Judging from the shape\n> of the topic, I do not think this topic has any other hidden\n> dependencies as it builds directly on top of v2.17.0.\n>\n> For those who are reading and reviewing from the sideline, I've\n> attached that missing 0.9/8 patch at the end of this message.\n\nSorry for the confusion Junio. I'm not used to the mailing list\nworkflow and it seems that I missed a patch. I'll make sure to include\nthat patch in v2.\n\n>\n> [Footnote]\n>\n> *1* I do not know if it deserves to be called a bug, but it\n> certainly is an anomaly GitHub exhibits that a99d903f can be\n> viewed at https://github.com/git/git/commit/a99d903f..., as if\n> it is part of the official git/git history, when it only exists\n> in a fork of that repository. I can understand why it happens\n> but it certainly is unexpected.\n>\n> -- >8 --\n> From: Jeff King \n> Date: Tue, 3 Apr 2018 17:10:30 -0400\n> Subject: [PATCH 0.9/8] t7004: fix mistaken tag name\n>\n> We have a series of tests which create signed tags with\n> various properties, but one test accidentally verifies a tag\n> from much earlier in the series.\n> ---\n> t/t7004-tag.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 2aac77af7..ee093b393 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -1056,7 +1056,7 @@ test_expect_success GPG \\\n> git tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n> get_tag_msg blanknonlfile-signed-tag >actual &&\n> test_cmp expect actual &&\n> - git tag -v signed-tag\n> + git tag -v blanknonlfile-signed-tag\n> '\n>\n> # messages with commented lines for signed tags:\n> --\n> 2.17.0-140-g0b0cc9f867\n>\n\n\n\n-- \n-Ben Toews\n"},{"id":"344639","messageId":"20180413211837.99415-1-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180409204129.43537-1-mastahyeti@gmail.com","subject":"[PATCH v2 0/9] gpg-interface: Multiple signing tools","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:28Z","receivedAt":"2018-04-13T21:19:14Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"Updated to incorporate feedback from v1. In addition to changes to the patches\nfrom v1, I added the missing `t7004: fix mistaken tag name` patch, which had\ncaused some confusion (sorry about that). Thanks for everyone's feedback on v1.\n\n### Interdiff (v1..v2):\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 7906123a59..691b309306 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1728,7 +1728,7 @@ grep.fallbackToNoIndex::\n \tis executed outside of a git repository. Defaults to false.\n\n signingtool..program::\n-\tThe name of the program on `$PATH` to execute when making or\n+\tThe name or path of the program to execute when making or\n \tverifying a signature. This program will be used for making\n \tsignatures if `` is configured as `signingtool.default`.\n \tThis program will be used for verifying signatures whose PEM\n@@ -1750,7 +1750,9 @@ signingtool..pemtype::\n \tSIGNATURE`. When verifying a signature with this PEM block type\n \tthe program specified in `signingtool..program` will be\n \tused. By default `signingtool.gpg.pemtype` contains `PGP\n-\tSIGNATURE` and `PGP MESSAGE`.\n+\tSIGNATURE` and `PGP MESSAGE`. Multiple PEM types may be specified\n+\tfor a single signing tool by including the `pemtype` directive\n+\tmultiple times within the `signingtool` configuration.\n\n signingtool.default::\n \tThe `` of the signing tool to use when creating\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 0e2a82e8e5..5d4ae2a7ed 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -22,7 +22,8 @@ static struct signing_tool *alloc_signing_tool(void)\n * Our default tool config is too complicated to specify as a constant\n * initializer, so we lazily create it as needed.\n */\n-static void init_signing_tool_defaults(void) {\n+static void init_signing_tool_defaults(void)\n+{\n \tstruct signing_tool *tool;\n\n \tif (signing_tool_config)\n@@ -38,7 +39,8 @@ static void init_signing_tool_defaults(void) {\n \tsigning_tool_config = tool;\n }\n\n-static struct signing_tool *get_signing_tool(const char *name) {\n+static struct signing_tool *get_signing_tool(const char *name)\n+{\n \tstruct signing_tool *tool;\n\n \tinit_signing_tool_defaults();\n@@ -216,11 +218,12 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t}\n\n \tif (!strcmp(var, \"gpg.program\")) {\n-\t\tstruct signing_tool *tool = get_or_create_signing_tool(\"gpg\");\n+\t\tstruct signing_tool *tool;\n\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n\n+\t\ttool = get_or_create_signing_tool(\"gpg\");\n \t\tfree(tool->program);\n \t\ttool->program = xstrdup(value);\n \t\treturn 0;\n@@ -331,7 +334,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t/*\n \t\t\t * The caller didn't tell us which tool to use, and we\n \t\t\t * didn't recognize the format. Historically we've fed\n-\t\t\t * these cases to blindly to gpg, so let's continue to\n+\t\t\t * these cases blindly to gpg, so let's continue to\n \t\t\t * do so.\n \t\t\t */\n \t\t\ttool = get_signing_tool(\"gpg\");\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex cee0dfe401..8e22e67b6f 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -42,7 +42,7 @@ void signature_check_clear(struct signature_check *sigc);\n * pointed at the signing_tool that corresponds to the found\n * signature type.\n */\n-size_t parse_signature(const char *buf, unsigned long size,\n+size_t parse_signature(const char *buf, size_t size,\n \t\t const struct signing_tool **out_tool);\n\n void parse_gpg_output(struct signature_check *);\n@@ -61,7 +61,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n * gpg_output, when set, receives the diagnostic output from GPG.\n * gpg_status, when set, receives the status output from GPG.\n *\n- * Typically the \"tool\" argument should come from a previous call to\n+ * Typically, the \"tool\" argument should come from a previous call to\n * parse_signature(). If it's NULL, then verify_signed_buffer() will\n * try to choose the appropriate tool based on the contents of the\n * \"signature\" buffer.\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 08d23b0cf5..b9ba47057f 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -59,20 +59,24 @@ sanitize_pgp() {\n\n create_fake_signer () {\n \twrite_script fake-signer <<-\\EOF\n-\tif [ \"$1 $2\" = \"--status-fd=2 -bsau\" ]; then\n+\tif test \"$1 $2\" = \"--status-fd=2 -bsau\"\n+\tthen\n \t\techo >&2 \"[GNUPG:] BEGIN_SIGNING\"\n \t\techo >&2 \"[GNUPG:] SIG_CREATED D 1 SHA256 0 1513792449 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70\"\n-\t\t# avoid \"-\" in echo arguments\n-\t\tprintf \"%s\\n\" \\\n-\t\t \"-----BEGIN FAKE SIGNER SIGNATURE-----\" \\\n-\t\t \"fake-signature\" \\\n-\t\t \"-----END FAKE SIGNER SIGNATURE-----\"\n+\t\tcat <<-\\END\n+\t\t-----BEGIN FAKE SIGNER SIGNATURE-----\n+\t\tfake-signature\n+\t\t-----END FAKE SIGNER SIGNATURE-----\n+\t\tEND\n \t\texit 0\n\n-\telif [ \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\" ]; then\n-\t\techo \"[GNUPG:] NEWSIG\"\n-\t\techo \"[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\"\n-\t\techo \"[GNUPG:] TRUST_FULLY 0 shell\"\n+\telif test \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\"\n+\tthen\n+\t\tcat <<-\\END\n+\t\t[GNUPG:] NEWSIG\n+\t\t[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\n+\t\t[GNUPG:] TRUST_FULLY 0 shell\n+\t\tEND\n \t\techo >&2 \"Good signature from /CN=Some User/EMail=some@user.email\"\n \t\texit 0\n\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 848a823302..fb41f98ca6 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -65,13 +65,15 @@ test_expect_success GPG 'create signed commits' '\n \tgrep \"PGP SIGNATURE\" actual &&\n\n \tgit config gpg.program \"$TRASH_DIRECTORY/fake-signer\" &&\n-\techo 12 >file && test_tick && git commit -a -m twelfth && test_unconfig gpg.program &&\n+\techo 12 >file && test_tick && git commit -a -m twelfth &&\n+\ttest_unconfig gpg.program &&\n \tgit tag twelfth-fake-signed &&\n \tgit cat-file -p twelfth-fake-signed >actual &&\n \tgrep \"FAKE SIGNER SIGNATURE\" actual &&\n\n \tgit config signingtool.default fake &&\n-\techo 13 >file && test_tick && git commit -a -m thirteenth && test_unconfig signingtool.default &&\n+\techo 13 >file && test_tick && git commit -a -m thirteenth &&\n+\ttest_unconfig signingtool.default &&\n \tgit tag thirteenth-fake-signed &&\n \tgit cat-file -p thirteenth-fake-signed >actual &&\n \tgrep \"FAKE SIGNER SIGNATURE\" actual\n\n### Patches\n\nBen Toews (1):\n gpg-interface: handle alternative signature types\n\nJeff King (8):\n t7004: fix mistaken tag name\n gpg-interface: handle bool user.signingkey\n gpg-interface: modernize function declarations\n gpg-interface: use size_t for signature buffer size\n gpg-interface: fix const-correctness of \"eol\" pointer\n gpg-interface: extract gpg line matching helper\n gpg-interface: find the last gpg signature line\n gpg-interface: prepare for parsing arbitrary PEM blocks\n\n Documentation/config.txt | 42 +++++++---\n builtin/fmt-merge-msg.c | 6 +-\n builtin/receive-pack.c | 7 +-\n builtin/tag.c | 2 +-\n commit.c | 2 +-\n gpg-interface.c | 201 +++++++++++++++++++++++++++++++++++++++--------\n gpg-interface.h | 67 +++++++++++++---\n log-tree.c | 7 +-\n ref-filter.c | 2 +-\n t/lib-gpg.sh | 30 +++++++\n t/t7004-tag.sh | 13 ++-\n t/t7510-signed-commit.sh | 34 +++++++-\n tag.c | 2 +-\n 13 files changed, 348 insertions(+), 67 deletions(-)\n\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344640","messageId":"20180413211837.99415-3-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 2/9] gpg-interface: handle bool user.signingkey","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:30Z","receivedAt":"2018-04-13T21:19:16Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nThe config handler for user.signingkey does not check for a\nboolean value, and thus:\n\n git -c user.signingkey tag\n\nwill segfault. We could fix this and even shorten the code\nby using git_config_string(). But our set_signing_key()\nhelper is used by other code outside of gpg-interface.c, so\nwe must keep it (and we may as well use it, because unlike\ngit_config_string() it does not leak when we overwrite an\nold value).\n\nIronically, the handler for gpg.program just below _could_\nuse git_config_string() but doesn't. But since we're going\nto touch that in a future patch, we'll leave it alone for\nnow. We will add some whitespace and returns in preparation\nfor adding more config keys, though.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 4feacf16e5..61c0690e12 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -128,13 +128,19 @@ void set_signing_key(const char *key)\n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"user.signingkey\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n \t\tset_signing_key(value);\n+\t\treturn 0;\n \t}\n+\n \tif (!strcmp(var, \"gpg.program\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \t\tgpg_program = xstrdup(value);\n+\t\treturn 0;\n \t}\n+\n \treturn 0;\n }\n\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344641","messageId":"20180413211837.99415-4-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 3/9] gpg-interface: modernize function declarations","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:31Z","receivedAt":"2018-04-13T21:19:19Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nLet's drop \"extern\" from our declarations, which brings us\nin line with our modern style guidelines. While we're\nhere, let's wrap some of the overly long lines, and move\ndocstrings for public functions to their declarations, since\nthey document the interface.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 17 -----------------\n gpg-interface.h | 49 ++++++++++++++++++++++++++++++++++++++-----------\n 2 files changed, 38 insertions(+), 28 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 61c0690e12..08de0daa41 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,12 +101,6 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n\n-/*\n- * Look at GPG signed content (e.g. a signed tag object), whose\n- * payload is followed by a detached signature on it. Return the\n- * offset where the embedded detached signature begins, or the end of\n- * the data when there is no such signature.\n- */\n size_t parse_signature(const char *buf, unsigned long size)\n {\n \tchar *eol;\n@@ -151,12 +145,6 @@ const char *get_signing_key(void)\n \treturn git_committer_info(IDENT_STRICT|IDENT_NO_DATE);\n }\n\n-/*\n- * Create a detached signature for the contents of \"buffer\" and append\n- * it after \"signature\"; \"buffer\" and \"signature\" can be the same\n- * strbuf instance, which would cause the detached signature appended\n- * at the end.\n- */\n int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n@@ -198,11 +186,6 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \treturn 0;\n }\n\n-/*\n- * Run \"gpg\" to see if the payload matches the detached signature.\n- * gpg_output, when set, receives the diagnostic output from GPG.\n- * gpg_status, when set, receives the status output from GPG.\n- */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n \t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex d2d4fd3a65..2c40a9175f 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -23,16 +23,43 @@ struct signature_check {\n \tchar *key;\n };\n\n-extern void signature_check_clear(struct signature_check *sigc);\n-extern size_t parse_signature(const char *buf, unsigned long size);\n-extern void parse_gpg_output(struct signature_check *);\n-extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);\n-extern int verify_signed_buffer(const char *payload, size_t payload_size, const char *signature, size_t signature_size, struct strbuf *gpg_output, struct strbuf *gpg_status);\n-extern int git_gpg_config(const char *, const char *, void *);\n-extern void set_signing_key(const char *);\n-extern const char *get_signing_key(void);\n-extern int check_signature(const char *payload, size_t plen,\n-\tconst char *signature, size_t slen, struct signature_check *sigc);\n-void print_signature_buffer(const struct signature_check *sigc, unsigned flags);\n+void signature_check_clear(struct signature_check *sigc);\n+\n+/*\n+ * Look at GPG signed content (e.g. a signed tag object), whose\n+ * payload is followed by a detached signature on it. Return the\n+ * offset where the embedded detached signature begins, or the end of\n+ * the data when there is no such signature.\n+ */\n+size_t parse_signature(const char *buf, unsigned long size);\n+\n+void parse_gpg_output(struct signature_check *);\n+\n+/*\n+ * Create a detached signature for the contents of \"buffer\" and append\n+ * it after \"signature\"; \"buffer\" and \"signature\" can be the same\n+ * strbuf instance, which would cause the detached signature appended\n+ * at the end.\n+ */\n+int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n+\t\tconst char *signing_key);\n+\n+/*\n+ * Run \"gpg\" to see if the payload matches the detached signature.\n+ * gpg_output, when set, receives the diagnostic output from GPG.\n+ * gpg_status, when set, receives the status output from GPG.\n+ */\n+int verify_signed_buffer(const char *payload, size_t payload_size,\n+\t\t\t const char *signature, size_t signature_size,\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status);\n+\n+int git_gpg_config(const char *, const char *, void *);\n+void set_signing_key(const char *);\n+const char *get_signing_key(void);\n+int check_signature(const char *payload, size_t plen,\n+\t\t const char *signature, size_t slen,\n+\t\t struct signature_check *sigc);\n+void print_signature_buffer(const struct signature_check *sigc,\n+\t\t\t unsigned flags);\n\n #endif\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344642","messageId":"20180413211837.99415-2-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 1/9] t7004: fix mistaken tag name","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:29Z","receivedAt":"2018-04-13T21:19:22Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nWe have a series of tests which create signed tags with\nvarious properties, but one test accidentally verifies a tag\nfrom much earlier in the series.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n t/t7004-tag.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 2aac77af70..ee093b393d 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1056,7 +1056,7 @@ test_expect_success GPG \\\n \tgit tag -s -F sigblanknonlfile blanknonlfile-signed-tag &&\n \tget_tag_msg blanknonlfile-signed-tag >actual &&\n \ttest_cmp expect actual &&\n-\tgit tag -v signed-tag\n+\tgit tag -v blanknonlfile-signed-tag\n '\n\n # messages with commented lines for signed tags:\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344643","messageId":"20180413211837.99415-9-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 8/9] gpg-interface: prepare for parsing arbitrary PEM blocks","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:36Z","receivedAt":"2018-04-13T21:19:24Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nIn preparation for handling more PEM blocks besides \"PGP\nSIGNATURE\" and \"PGP MESSAGE', let's break up the parsing to\nparameterize the actual block type.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 0647bd6348..0ba4a8ac3b 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -9,9 +9,6 @@\n static char *configured_signing_key;\n static const char *gpg_program = \"gpg\";\n\n-#define PGP_SIGNATURE \"-----BEGIN PGP SIGNATURE-----\"\n-#define PGP_MESSAGE \"-----BEGIN PGP MESSAGE-----\"\n-\n void signature_check_clear(struct signature_check *sigc)\n {\n \tFREE_AND_NULL(sigc->payload);\n@@ -101,10 +98,17 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n\n-static int is_gpg_start(const char *line)\n+static int is_pem_start(const char *line)\n {\n-\treturn starts_with(line, PGP_SIGNATURE) ||\n-\t\tstarts_with(line, PGP_MESSAGE);\n+\tif (!skip_prefix(line, \"-----BEGIN \", &line))\n+\t\treturn 0;\n+\tif (!skip_prefix(line, \"PGP SIGNATURE\", &line) &&\n+\t !skip_prefix(line, \"PGP MESSAGE\", &line))\n+\t\treturn 0;\n+\tif (!starts_with(line, \"-----\"))\n+\t\treturn 0;\n+\n+\treturn 1;\n }\n\n size_t parse_signature(const char *buf, size_t size)\n@@ -114,7 +118,7 @@ size_t parse_signature(const char *buf, size_t size)\n \twhile (len < size) {\n \t\tconst char *eol;\n\n-\t\tif (is_gpg_start(buf + len))\n+\t\tif (is_pem_start(buf + len))\n \t\t\tmatch = len;\n\n \t\teol = memchr(buf + len, '\\n', size - len);\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344644","messageId":"20180413211837.99415-5-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 4/9] gpg-interface: use size_t for signature buffer size","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:32Z","receivedAt":"2018-04-13T21:19:28Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nEven though our object sizes (from which these buffers would\ncome) are typically \"unsigned long\", this is something we'd\nlike to eventually fix (since it's only 32-bits even on\n64-bit Windows). It makes more sense to use size_t when\ntaking an in-memory buffer.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 2 +-\n gpg-interface.h | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 08de0daa41..ac852ad4b9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,7 +101,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n\n-size_t parse_signature(const char *buf, unsigned long size)\n+size_t parse_signature(const char *buf, size_t size)\n {\n \tchar *eol;\n \tsize_t len = 0;\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 2c40a9175f..a5e6517ae6 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -31,7 +31,7 @@ void signature_check_clear(struct signature_check *sigc);\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n */\n-size_t parse_signature(const char *buf, unsigned long size);\n+size_t parse_signature(const char *buf, size_t size);\n\n void parse_gpg_output(struct signature_check *);\n\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344645","messageId":"20180413211837.99415-6-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 5/9] gpg-interface: fix const-correctness of \"eol\" pointer","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:33Z","receivedAt":"2018-04-13T21:19:30Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nWe accidentally shed the \"const\" of our buffer by passing it\nthrough memchr. Let's fix that, and while we're at it, move\nour variable declaration inside the loop, which is the only\nplace that uses it.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex ac852ad4b9..3414af38b9 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -103,11 +103,10 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n\n size_t parse_signature(const char *buf, size_t size)\n {\n-\tchar *eol;\n \tsize_t len = 0;\n \twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n \t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n-\t\teol = memchr(buf + len, '\\n', size - len);\n+\t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n \treturn len;\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344646","messageId":"20180413211837.99415-7-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 6/9] gpg-interface: extract gpg line matching helper","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:34Z","receivedAt":"2018-04-13T21:19:32Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nLet's separate the actual line-by-line parsing of signatures\nfrom the notion of \"is this a gpg signature line\". That will\nmake it easier to do more refactoring of this loop in future\npatches.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 9 +++++++--\n 1 file changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 3414af38b9..79333c1ee8 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -101,11 +101,16 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n\n+static int is_gpg_start(const char *line)\n+{\n+\treturn starts_with(line, PGP_SIGNATURE) ||\n+\t\tstarts_with(line, PGP_MESSAGE);\n+}\n+\n size_t parse_signature(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n-\twhile (len < size && !starts_with(buf + len, PGP_SIGNATURE) &&\n-\t\t\t!starts_with(buf + len, PGP_MESSAGE)) {\n+\twhile (len < size && !is_gpg_start(buf + len)) {\n \t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344647","messageId":"20180413211837.99415-8-mastahyeti@gmail.com","threadId":"48253","inReplyTo":"20180413211837.99415-1-mastahyeti@gmail.com","subject":"[PATCH v2 7/9] gpg-interface: find the last gpg signature line","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-13T21:18:35Z","receivedAt":"2018-04-13T21:19:34Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"From: Jeff King \n\nA signed tag has a detached signature like this:\n\n object ...\n [...more header...]\n\n This is the tag body.\n\n -----BEGIN PGP SIGNATURE-----\n [opaque gpg data]\n -----END PGP SIGNATURE-----\n\nOur parser finds the _first_ line that appears to start a\nPGP signature block, meaning we may be confused by a\nsignature (or a signature-like line) in the actual body.\nLet's keep parsing and always find the final block, which\nshould be the detached signature over all of the preceding\ncontent.\n\nSigned-off-by: Jeff King \nSigned-off-by: Ben Toews \n---\n gpg-interface.c | 12 +++++++++---\n t/t7004-tag.sh | 11 +++++++++++\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 79333c1ee8..0647bd6348 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -110,11 +110,17 @@ static int is_gpg_start(const char *line)\n size_t parse_signature(const char *buf, size_t size)\n {\n \tsize_t len = 0;\n-\twhile (len < size && !is_gpg_start(buf + len)) {\n-\t\tconst char *eol = memchr(buf + len, '\\n', size - len);\n+\tsize_t match = size;\n+\twhile (len < size) {\n+\t\tconst char *eol;\n+\n+\t\tif (is_gpg_start(buf + len))\n+\t\t\tmatch = len;\n+\n+\t\teol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n-\treturn len;\n+\treturn match;\n }\n\n void set_signing_key(const char *key)\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex ee093b393d..e3f1e014aa 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -1059,6 +1059,17 @@ test_expect_success GPG \\\n \tgit tag -v blanknonlfile-signed-tag\n '\n\n+test_expect_success GPG 'signed tag with embedded PGP message' '\n+\tcat >msg <<-\\EOF &&\n+\t-----BEGIN PGP MESSAGE-----\n+\n+\tthis is not a real PGP message\n+\t-----END PGP MESSAGE-----\n+\tEOF\n+\tgit tag -s -F msg confusing-pgp-message &&\n+\tgit tag -v confusing-pgp-message\n+'\n+\n # messages with commented lines for signed tags:\n\n cat >sigcommentsfile <\n---\n Documentation/config.txt | 42 +++++++++---\n builtin/fmt-merge-msg.c | 6 +-\n builtin/receive-pack.c | 7 +-\n builtin/tag.c | 2 +-\n commit.c | 2 +-\n gpg-interface.c | 168 ++++++++++++++++++++++++++++++++++++++++++-----\n gpg-interface.h | 24 ++++++-\n log-tree.c | 7 +-\n ref-filter.c | 2 +-\n t/lib-gpg.sh | 30 +++++++++\n t/t7510-signed-commit.sh | 34 +++++++++-\n tag.c | 2 +-\n 12 files changed, 283 insertions(+), 43 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4e0cff87f6..691b309306 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1727,16 +1727,40 @@ grep.fallbackToNoIndex::\n \tIf set to true, fall back to git grep --no-index if git grep\n \tis executed outside of a git repository. Defaults to false.\n\n-gpg.program::\n-\tUse this custom program instead of \"`gpg`\" found on `$PATH` when\n-\tmaking or verifying a PGP signature. The program must support the\n-\tsame command-line interface as GPG, namely, to verify a detached\n-\tsignature, \"`gpg --verify $file - <$signature`\" is run, and the\n-\tprogram is expected to signal a good signature by exiting with\n-\tcode 0, and to generate an ASCII-armored detached signature, the\n-\tstandard input of \"`gpg -bsau $key`\" is fed with the contents to be\n+signingtool..program::\n+\tThe name or path of the program to execute when making or\n+\tverifying a signature. This program will be used for making\n+\tsignatures if `` is configured as `signingtool.default`.\n+\tThis program will be used for verifying signatures whose PEM\n+\tblock type matches `signingtool..pemtype` (see below). The\n+\tprogram must support the same command-line interface as GPG.\n+\tTo verify a detached signature,\n+\t\"`gpg --verify $file - <$signature`\" is run, and the program is\n+\texpected to signal a good signature by exiting with code 0.\n+\tTo generate an ASCII-armored detached signature, the standard\n+\tinput of \"`gpg -bsau $key`\" is fed with the contents to be\n \tsigned, and the program is expected to send the result to its\n-\tstandard output.\n+\tstandard output. By default, `signingtool.gpg.program` is set to\n+\t`gpg`.\n+\n+signingtool..pemtype::\n+\tThe PEM block type associated with the signing tool named\n+\t``. For example, the block type of a GPG signature\n+\tstarting with `-----BEGIN PGP SIGNATURE-----` is `PGP\n+\tSIGNATURE`. When verifying a signature with this PEM block type\n+\tthe program specified in `signingtool..program` will be\n+\tused. By default `signingtool.gpg.pemtype` contains `PGP\n+\tSIGNATURE` and `PGP MESSAGE`. Multiple PEM types may be specified\n+\tfor a single signing tool by including the `pemtype` directive\n+\tmultiple times within the `signingtool` configuration.\n+\n+signingtool.default::\n+\tThe `` of the signing tool to use when creating\n+\tsignatures (e.g., setting it to \"foo\" will use use the program\n+\tspecified by `signingtool.foo.program`). Defaults to `gpg`.\n+\n+gpg.program::\n+\tHistorical alias for `signingtool.gpg.program`.\n\n gui.commitMsgWidth::\n \tDefines how wide the commit message window is in the\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 8e8a15ea4a..6aa8226308 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -490,14 +490,16 @@ static void fmt_merge_msg_sigs(struct strbuf *out)\n \t\tunsigned long size, len;\n \t\tchar *buf = read_sha1_file(sha1, &type, &size);\n \t\tstruct strbuf sig = STRBUF_INIT;\n+\t\tconst struct signing_tool *tool;\n\n \t\tif (!buf || type != OBJ_TAG)\n \t\t\tgoto next;\n-\t\tlen = parse_signature(buf, size);\n+\t\tlen = parse_signature(buf, size, &tool);\n\n \t\tif (size == len)\n \t\t\t; /* merely annotated */\n-\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len, &sig, NULL)) {\n+\t\telse if (verify_signed_buffer(buf, len, buf + len, size - len,\n+\t\t\t\t\t &sig, NULL, tool)) {\n \t\t\tif (!sig.len)\n \t\t\t\tstrbuf_addstr(&sig, \"gpg verification failed.\\n\");\n \t\t}\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 75e7f18ace..be17dcaa78 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -631,6 +631,7 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \t\tstruct strbuf gpg_output = STRBUF_INIT;\n \t\tstruct strbuf gpg_status = STRBUF_INIT;\n \t\tint bogs /* beginning_of_gpg_sig */;\n+\t\tconst struct signing_tool *tool;\n\n \t\talready_done = 1;\n \t\tif (write_object_file(push_cert.buf, push_cert.len, \"blob\",\n@@ -640,10 +641,10 @@ static void prepare_push_cert_sha1(struct child_process *proc)\n \t\tmemset(&sigcheck, '\\0', sizeof(sigcheck));\n \t\tsigcheck.result = 'N';\n\n-\t\tbogs = parse_signature(push_cert.buf, push_cert.len);\n+\t\tbogs = parse_signature(push_cert.buf, push_cert.len, &tool);\n \t\tif (verify_signed_buffer(push_cert.buf, bogs,\n \t\t\t\t\t push_cert.buf + bogs, push_cert.len - bogs,\n-\t\t\t\t\t &gpg_output, &gpg_status) < 0) {\n+\t\t\t\t\t &gpg_output, &gpg_status, tool) < 0) {\n \t\t\t; /* error running gpg */\n \t\t} else {\n \t\t\tsigcheck.payload = push_cert.buf;\n@@ -1565,7 +1566,7 @@ static void queue_commands_from_cert(struct command **tail,\n \t\tdie(\"malformed push certificate %.*s\", 100, push_cert->buf);\n \telse\n \t\tboc += 2;\n-\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);\n+\teoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len, NULL);\n\n \twhile (boc < eoc) {\n \t\tconst char *eol = memchr(boc, '\\n', eoc - boc);\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex da186691ed..adc37a28c5 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -178,7 +178,7 @@ static void write_tag_body(int fd, const struct object_id *oid)\n \t\treturn;\n \t}\n \tsp += 2; /* skip the 2 LFs */\n-\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp));\n+\twrite_or_die(fd, sp, parse_signature(sp, buf + size - sp, NULL));\n\n \tfree(buf);\n }\ndiff --git a/commit.c b/commit.c\nindex 00c99c7272..056942df7f 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1209,7 +1209,7 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tbuf = read_sha1_file(desc->obj->oid.hash, &type, &size);\n \tif (!buf || type != OBJ_TAG)\n \t\tgoto free_return;\n-\tlen = parse_signature(buf, size);\n+\tlen = parse_signature(buf, size, NULL);\n \tif (size == len)\n \t\tgoto free_return;\n \t/*\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 0ba4a8ac3b..5d4ae2a7ed 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -7,7 +7,62 @@\n #include \"tempfile.h\"\n\n static char *configured_signing_key;\n-static const char *gpg_program = \"gpg\";\n+static const char *default_signing_tool = \"gpg\";\n+static struct signing_tool *signing_tool_config;\n+\n+static struct signing_tool *alloc_signing_tool(void)\n+{\n+\tstruct signing_tool *ret;\n+\tret = xcalloc(1, sizeof(*ret));\n+\tret->pemtype.strdup_strings = 1;\n+\treturn ret;\n+}\n+\n+/*\n+ * Our default tool config is too complicated to specify as a constant\n+ * initializer, so we lazily create it as needed.\n+ */\n+static void init_signing_tool_defaults(void)\n+{\n+\tstruct signing_tool *tool;\n+\n+\tif (signing_tool_config)\n+\t\treturn;\n+\n+\ttool = alloc_signing_tool();\n+\ttool->name = xstrdup(\"gpg\");\n+\ttool->program = xstrdup(\"gpg\");\n+\tstring_list_append(&tool->pemtype, \"PGP SIGNATURE\");\n+\tstring_list_append(&tool->pemtype, \"PGP MESSAGE\");\n+\n+\ttool->next = signing_tool_config;\n+\tsigning_tool_config = tool;\n+}\n+\n+static struct signing_tool *get_signing_tool(const char *name)\n+{\n+\tstruct signing_tool *tool;\n+\n+\tinit_signing_tool_defaults();\n+\n+\tfor (tool = signing_tool_config; tool; tool = tool->next) {\n+\t\tif (!strcmp(name, tool->name))\n+\t\t\treturn tool;\n+\t}\n+\treturn NULL;\n+}\n+\n+static struct signing_tool *get_or_create_signing_tool(const char *name)\n+{\n+\tstruct signing_tool *tool = get_signing_tool(name);\n+\tif (!tool) {\n+\t\ttool = alloc_signing_tool();\n+\t\ttool->name = xstrdup(name);\n+\t\ttool->next = signing_tool_config;\n+\t\tsigning_tool_config = tool;\n+\t}\n+\treturn tool;\n+}\n\n void signature_check_clear(struct signature_check *sigc)\n {\n@@ -71,7 +126,7 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n \tsigc->result = 'N';\n\n \tstatus = verify_signed_buffer(payload, plen, signature, slen,\n-\t\t\t\t &gpg_output, &gpg_status);\n+\t\t\t\t &gpg_output, &gpg_status, NULL);\n \tif (status && !gpg_output.len)\n \t\tgoto out;\n \tsigc->payload = xmemdupz(payload, plen);\n@@ -98,32 +153,49 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \t\tfputs(output, stderr);\n }\n\n-static int is_pem_start(const char *line)\n+static int is_pem_start(const char *line, struct signing_tool **out_tool)\n {\n+\tstruct signing_tool *tool;\n+\n \tif (!skip_prefix(line, \"-----BEGIN \", &line))\n \t\treturn 0;\n-\tif (!skip_prefix(line, \"PGP SIGNATURE\", &line) &&\n-\t !skip_prefix(line, \"PGP MESSAGE\", &line))\n-\t\treturn 0;\n-\tif (!starts_with(line, \"-----\"))\n-\t\treturn 0;\n\n-\treturn 1;\n+\tinit_signing_tool_defaults();\n+\n+\tfor (tool = signing_tool_config; tool; tool = tool->next) {\n+\t\tint i;\n+\t\tfor (i = 0; i < tool->pemtype.nr; i++) {\n+\t\t\tconst char *match = tool->pemtype.items[i].string;\n+\t\t\tconst char *end;\n+\t\t\tif (skip_prefix(line, match, &end) &&\n+\t\t\t starts_with(end, \"-----\")) {\n+\t\t\t\t*out_tool = tool;\n+\t\t\t\treturn 1;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 0;\n }\n\n-size_t parse_signature(const char *buf, size_t size)\n+size_t parse_signature(const char *buf, size_t size,\n+\t\t const struct signing_tool **out_tool)\n {\n \tsize_t len = 0;\n \tsize_t match = size;\n+\tstruct signing_tool *tool = NULL;\n+\n \twhile (len < size) {\n \t\tconst char *eol;\n\n-\t\tif (is_pem_start(buf + len))\n+\t\tif (is_pem_start(buf + len, &tool))\n \t\t\tmatch = len;\n\n \t\teol = memchr(buf + len, '\\n', size - len);\n \t\tlen += eol ? eol - (buf + len) + 1 : size - len;\n \t}\n+\n+\tif (out_tool)\n+\t\t*out_tool = tool;\n \treturn match;\n }\n\n@@ -135,6 +207,9 @@ void set_signing_key(const char *key)\n\n int git_gpg_config(const char *var, const char *value, void *cb)\n {\n+\tconst char *key, *name = NULL;\n+\tint name_len;\n+\n \tif (!strcmp(var, \"user.signingkey\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -143,12 +218,44 @@ int git_gpg_config(const char *var, const char *value, void *cb)\n \t}\n\n \tif (!strcmp(var, \"gpg.program\")) {\n+\t\tstruct signing_tool *tool;\n+\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tgpg_program = xstrdup(value);\n+\n+\t\ttool = get_or_create_signing_tool(\"gpg\");\n+\t\tfree(tool->program);\n+\t\ttool->program = xstrdup(value);\n \t\treturn 0;\n \t}\n\n+\tif (!strcmp(var, \"signingtool.default\"))\n+\t\treturn git_config_string(&default_signing_tool, var, value);\n+\n+\tif (!parse_config_key(var, \"signingtool\", &name, &name_len, &key) && name) {\n+\t\tchar *tmpname = xmemdupz(name, name_len);\n+\t\tstruct signing_tool *tool = get_or_create_signing_tool(tmpname);\n+\n+\t\tfree(tmpname);\n+\n+\t\tif (!strcmp(key, \"program\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\n+\t\t\tfree(tool->program);\n+\t\t\ttool->program = xstrdup(value);\n+\t\t\treturn 0;\n+\t\t}\n+\n+\t\tif (!strcmp(key, \"pemtype\")) {\n+\t\t\tif (!value)\n+\t\t\t\treturn config_error_nonbool(var);\n+\n+\t\t\tstring_list_append(&tool->pemtype, value);\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n \treturn 0;\n }\n\n@@ -159,7 +266,9 @@ const char *get_signing_key(void)\n \treturn git_committer_info(IDENT_STRICT|IDENT_NO_DATE);\n }\n\n-int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n+static int sign_buffer_with(struct strbuf *buffer, struct strbuf *signature,\n+\t\t\t const char *signing_key,\n+\t\t\t const struct signing_tool *tool)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tint ret;\n@@ -167,7 +276,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \tstruct strbuf gpg_status = STRBUF_INIT;\n\n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n+\t\t\t tool->program,\n \t\t\t \"--status-fd=2\",\n \t\t\t \"-bsau\", signing_key,\n \t\t\t NULL);\n@@ -200,15 +309,42 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig\n \treturn 0;\n }\n\n+int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)\n+{\n+\tstruct signing_tool *tool = get_signing_tool(default_signing_tool);\n+\tif (!tool || !tool->program)\n+\t\treturn error(_(\"default signing tool '%s' has no program configured\"),\n+\t\t\t default_signing_tool);\n+\treturn sign_buffer_with(buffer, signature, signing_key, tool);\n+}\n+\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n-\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status)\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status,\n+\t\t\t const struct signing_tool *tool)\n {\n \tstruct child_process gpg = CHILD_PROCESS_INIT;\n \tstruct tempfile *temp;\n \tint ret;\n \tstruct strbuf buf = STRBUF_INIT;\n\n+\tif (!tool) {\n+\t\tparse_signature(signature, signature_size, &tool);\n+\t\tif (!tool) {\n+\t\t\t/*\n+\t\t\t * The caller didn't tell us which tool to use, and we\n+\t\t\t * didn't recognize the format. Historically we've fed\n+\t\t\t * these cases blindly to gpg, so let's continue to\n+\t\t\t * do so.\n+\t\t\t */\n+\t\t\ttool = get_signing_tool(\"gpg\");\n+\t\t}\n+\t}\n+\n+\tif (!tool->program)\n+\t\treturn error(_(\"signing tool '%s' has no program configured\"),\n+\t\t\t tool->name);\n+\n \ttemp = mks_tempfile_t(\".git_vtag_tmpXXXXXX\");\n \tif (!temp)\n \t\treturn error_errno(_(\"could not create temporary file\"));\n@@ -221,7 +357,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,\n \t}\n\n \targv_array_pushl(&gpg.args,\n-\t\t\t gpg_program,\n+\t\t\t tool->program,\n \t\t\t \"--status-fd=1\",\n \t\t\t \"--keyid-format=long\",\n \t\t\t \"--verify\", temp->filename.buf, \"-\",\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex a5e6517ae6..8e22e67b6f 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -23,15 +23,27 @@ struct signature_check {\n \tchar *key;\n };\n\n+struct signing_tool {\n+\tchar *name;\n+\tchar *program;\n+\tstruct string_list pemtype;\n+\tstruct signing_tool *next;\n+};\n+\n void signature_check_clear(struct signature_check *sigc);\n\n /*\n- * Look at GPG signed content (e.g. a signed tag object), whose\n+ * Look for signed content (e.g. a signed tag object), whose\n * payload is followed by a detached signature on it. Return the\n * offset where the embedded detached signature begins, or the end of\n * the data when there is no such signature.\n+ *\n+ * If out_tool is non-NULL and a signature is found, it will be\n+ * pointed at the signing_tool that corresponds to the found\n+ * signature type.\n */\n-size_t parse_signature(const char *buf, size_t size);\n+size_t parse_signature(const char *buf, size_t size,\n+\t\t const struct signing_tool **out_tool);\n\n void parse_gpg_output(struct signature_check *);\n\n@@ -48,10 +60,16 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,\n * Run \"gpg\" to see if the payload matches the detached signature.\n * gpg_output, when set, receives the diagnostic output from GPG.\n * gpg_status, when set, receives the status output from GPG.\n+ *\n+ * Typically, the \"tool\" argument should come from a previous call to\n+ * parse_signature(). If it's NULL, then verify_signed_buffer() will\n+ * try to choose the appropriate tool based on the contents of the\n+ * \"signature\" buffer.\n */\n int verify_signed_buffer(const char *payload, size_t payload_size,\n \t\t\t const char *signature, size_t signature_size,\n-\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status);\n+\t\t\t struct strbuf *gpg_output, struct strbuf *gpg_status,\n+\t\t\t const struct signing_tool *tool);\n\n int git_gpg_config(const char *, const char *, void *);\n void set_signing_key(const char *);\ndiff --git a/log-tree.c b/log-tree.c\nindex bdf23c5f7b..f8fdff6e65 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -456,7 +456,7 @@ static void show_signature(struct rev_info *opt, struct commit *commit)\n\n \tstatus = verify_signed_buffer(payload.buf, payload.len,\n \t\t\t\t signature.buf, signature.len,\n-\t\t\t\t &gpg_output, NULL);\n+\t\t\t\t &gpg_output, NULL, NULL);\n \tif (status && !gpg_output.len)\n \t\tstrbuf_addstr(&gpg_output, \"No signature\\n\");\n\n@@ -498,6 +498,7 @@ static void show_one_mergetag(struct commit *commit,\n \tstruct strbuf verify_message;\n \tint status, nth;\n \tsize_t payload_size, gpg_message_offset;\n+\tconst struct signing_tool *tool;\n\n \thash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);\n \ttag = lookup_tag(&oid);\n@@ -520,14 +521,14 @@ static void show_one_mergetag(struct commit *commit,\n \t\t\t \"parent #%d, tagged '%s'\\n\", nth + 1, tag->tag);\n \tgpg_message_offset = verify_message.len;\n\n-\tpayload_size = parse_signature(extra->value, extra->len);\n+\tpayload_size = parse_signature(extra->value, extra->len, &tool);\n \tstatus = -1;\n \tif (extra->len > payload_size) {\n \t\t/* could have a good signature */\n \t\tif (!verify_signed_buffer(extra->value, payload_size,\n \t\t\t\t\t extra->value + payload_size,\n \t\t\t\t\t extra->len - payload_size,\n-\t\t\t\t\t &verify_message, NULL))\n+\t\t\t\t\t &verify_message, NULL, tool))\n \t\t\tstatus = 0; /* good */\n \t\telse if (verify_message.len <= gpg_message_offset)\n \t\t\tstrbuf_addstr(&verify_message, \"No signature\\n\");\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 45fc56216a..6ab66b9a7c 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1009,7 +1009,7 @@ static void find_subpos(const char *buf, unsigned long sz,\n \t\tbuf++;\n\n \t/* parse signature first; we might not even have a subject line */\n-\t*sig = buf + parse_signature(buf, strlen(buf));\n+\t*sig = buf + parse_signature(buf, strlen(buf), NULL);\n \t*siglen = strlen(*sig);\n\n \t/* subject is first non-empty line */\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex a5d3b2cbaa..b9ba47057f 100755\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -56,3 +56,33 @@ sanitize_pgp() {\n \t\t/^-----BEGIN PGP/ and $in_pgp = 1;\n \t'\n }\n+\n+create_fake_signer () {\n+\twrite_script fake-signer <<-\\EOF\n+\tif test \"$1 $2\" = \"--status-fd=2 -bsau\"\n+\tthen\n+\t\techo >&2 \"[GNUPG:] BEGIN_SIGNING\"\n+\t\techo >&2 \"[GNUPG:] SIG_CREATED D 1 SHA256 0 1513792449 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70\"\n+\t\tcat <<-\\END\n+\t\t-----BEGIN FAKE SIGNER SIGNATURE-----\n+\t\tfake-signature\n+\t\t-----END FAKE SIGNER SIGNATURE-----\n+\t\tEND\n+\t\texit 0\n+\n+\telif test \"$1 $2 $3\" = \"--status-fd=1 --keyid-format=long --verify\"\n+\tthen\n+\t\tcat <<-\\END\n+\t\t[GNUPG:] NEWSIG\n+\t\t[GNUPG:] GOODSIG 4A7FF9E2330D22B19213A4E9E9C423BE17EFEE70 /CN=Some User/EMail=some@user.email\n+\t\t[GNUPG:] TRUST_FULLY 0 shell\n+\t\tEND\n+\t\techo >&2 \"Good signature from /CN=Some User/EMail=some@user.email\"\n+\t\texit 0\n+\n+\telse\n+\t\techo \"bad arguments\" 1>&2\n+\t\texit 1\n+\tfi\n+\tEOF\n+}\ndiff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh\nindex 762135adea..fb41f98ca6 100755\n--- a/t/t7510-signed-commit.sh\n+++ b/t/t7510-signed-commit.sh\n@@ -5,6 +5,10 @@ test_description='signed commit tests'\n GNUPGHOME_NOT_USED=$GNUPGHOME\n . \"$TEST_DIRECTORY/lib-gpg.sh\"\n\n+test_expect_success 'create fake signing program' '\n+\tcreate_fake_signer\n+'\n+\n test_expect_success GPG 'create signed commits' '\n \ttest_when_finished \"test_unconfig commit.gpgsign\" &&\n\n@@ -51,13 +55,35 @@ test_expect_success GPG 'create signed commits' '\n \t# commit.gpgsign is still on but this must not be signed\n \tgit tag ninth-unsigned $(echo 9 | git commit-tree HEAD^{tree}) &&\n \t# explicit -S of course must sign.\n-\tgit tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree})\n+\tgit tag tenth-signed $(echo 9 | git commit-tree -S HEAD^{tree}) &&\n+\n+\tgit config signingtool.fake.program \"$TRASH_DIRECTORY/fake-signer\" &&\n+\tgit config signingtool.fake.pemtype \"FAKE SIGNER SIGNATURE\" &&\n+\techo 11 >file && test_tick && git commit -a -m eleventh &&\n+\tgit tag eleventh-pgp-signed &&\n+\tgit cat-file -p eleventh-pgp-signed >actual &&\n+\tgrep \"PGP SIGNATURE\" actual &&\n+\n+\tgit config gpg.program \"$TRASH_DIRECTORY/fake-signer\" &&\n+\techo 12 >file && test_tick && git commit -a -m twelfth &&\n+\ttest_unconfig gpg.program &&\n+\tgit tag twelfth-fake-signed &&\n+\tgit cat-file -p twelfth-fake-signed >actual &&\n+\tgrep \"FAKE SIGNER SIGNATURE\" actual &&\n+\n+\tgit config signingtool.default fake &&\n+\techo 13 >file && test_tick && git commit -a -m thirteenth &&\n+\ttest_unconfig signingtool.default &&\n+\tgit tag thirteenth-fake-signed &&\n+\tgit cat-file -p thirteenth-fake-signed >actual &&\n+\tgrep \"FAKE SIGNER SIGNATURE\" actual\n '\n\n test_expect_success GPG 'verify and show signatures' '\n \t(\n \t\tfor commit in initial second merge fourth-signed \\\n-\t\t\tfifth-signed sixth-signed seventh-signed tenth-signed\n+\t\t\tfifth-signed sixth-signed seventh-signed tenth-signed \\\n+\t\t\televenth-pgp-signed twelfth-fake-signed thirteenth-fake-signed\n \t\tdo\n \t\t\tgit verify-commit $commit &&\n \t\t\tgit show --pretty=short --show-signature $commit >actual &&\n@@ -98,7 +124,9 @@ test_expect_success GPG 'verify-commit exits success on untrusted signature' '\n\n test_expect_success GPG 'verify signatures with --raw' '\n \t(\n-\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed seventh-signed\n+\t\tfor commit in initial second merge fourth-signed fifth-signed sixth-signed \\\n+\t\t\tseventh-signed eleventh-pgp-signed twelfth-fake-signed \\\n+\t\t\tthirteenth-fake-signed\n \t\tdo\n \t\t\tgit verify-commit --raw $commit 2>actual &&\n \t\t\tgrep \"GOODSIG\" actual &&\ndiff --git a/tag.c b/tag.c\nindex 66210fd477..2d557e1f5e 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -15,7 +15,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n\n \tmemset(&sigc, 0, sizeof(sigc));\n\n-\tpayload_size = parse_signature(buf, size);\n+\tpayload_size = parse_signature(buf, size, NULL);\n\n \tif (size == payload_size) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n--\n2.15.1 (Apple Git-101)\n"},{"id":"344700","messageId":"20180414195954.GB14631@genre.crustytoothpaste.net","threadId":"48253","inReplyTo":"CAPig+cT3AobThgZ15iquyRQG0Qes1ZzQxycXcgHYuwQCuDEDBQ@mail.gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-14T19:59:54Z","receivedAt":"2018-04-14T20:00:07Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Apr 10, 2018 at 04:24:27AM -0400, Eric Sunshine wrote:\n> How confident are we that _all_ possible signing programs will conform\n> to the \"-----BEGIN %s-----\" pattern? If we're not confident, then\n> perhaps the user should be providing the full string here, not just\n> the '%s' part?\n\nThis is not likely to be true of other signing schemes. In fact, other\nthan OpenPGP, PEM, and CMS (S/MIME), this is probably not true at all.\nI know OpenBSD's signify has no wrappers (except a mandatory \"untrusted\ncomment:\" line at the beginning). There wouldn't be a way to match such\na signature unless we implemented prefix or regex support.\n\nIt's currently possible to hack other signatures in with wrappers if\nthey wrap the actual signature in OpenPGP-like armor; someone (I believe\nEric Wong) has gotten this to work with signify. I only mention signify\nbecause other than OpenPGP and CMS, it's the only scheme I've seen\npeople use with Git.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"344773","messageId":"xmqqbmejyc4j.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"20180414195954.GB14631@genre.crustytoothpaste.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-16T05:05:32Z","receivedAt":"2018-04-16T05:05:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On Tue, Apr 10, 2018 at 04:24:27AM -0400, Eric Sunshine wrote:\n>> How confident are we that _all_ possible signing programs will conform\n>> to the \"-----BEGIN %s-----\" pattern? If we're not confident, then\n>> perhaps the user should be providing the full string here, not just\n>> the '%s' part?\n>\n> This is not likely to be true of other signing schemes. In fact, other\n> than OpenPGP, PEM, and CMS (S/MIME), this is probably not true at all.\n\nHmph. \n\nThat argues more strongly that we would regret unless we make the\nend-user configuration to at least the whole string (which later can\nbe promoted to \"a pattern that matches the whole string\"), not just\nthe part after mandatory \"-----BEGIN \", methinks.\n\n"},{"id":"344850","messageId":"20180417001212.GC14631@genre.crustytoothpaste.net","threadId":"48253","inReplyTo":"xmqqbmejyc4j.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-04-17T00:12:12Z","receivedAt":"2018-04-17T00:12:25Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Apr 16, 2018 at 02:05:32PM +0900, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n> \n> > On Tue, Apr 10, 2018 at 04:24:27AM -0400, Eric Sunshine wrote:\n> >> How confident are we that _all_ possible signing programs will conform\n> >> to the \"-----BEGIN %s-----\" pattern? If we're not confident, then\n> >> perhaps the user should be providing the full string here, not just\n> >> the '%s' part?\n> >\n> > This is not likely to be true of other signing schemes. In fact, other\n> > than OpenPGP, PEM, and CMS (S/MIME), this is probably not true at all.\n> \n> Hmph. \n> \n> That argues more strongly that we would regret unless we make the\n> end-user configuration to at least the whole string (which later can\n> be promoted to \"a pattern that matches the whole string\"), not just\n> the part after mandatory \"-----BEGIN \", methinks.\n\nYeah, I think this patch set is \"add gpgsm support\", which I can see as\na valuable goal in and of itself, but I'm not sure the attempt to make\nit generic is in the right place. If we want to be truly generic, the\nway to do that is to invoke a helper based on signature type (e.g.\ngit-sign-gpg, git-sign-gpgsm, git-sign-signify) to do the signing and\nverification. We need not ship these helpers ourselves; interested\nthird-parties can provide them, and we can add configuration to match\nagainst regexes for non-built-in types (which is required for many other\nformats).\n\nIf we just want to add gpgsm support, that's fine, but we should be\ntransparent about that fact and try to avoid making an interface which\nis at once too generic and not generic enough.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"344855","messageId":"xmqq604qwqbj.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"20180417001212.GC14631@genre.crustytoothpaste.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-17T01:54:08Z","receivedAt":"2018-04-17T01:54:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> If we just want to add gpgsm support, that's fine, but we should be\n> transparent about that fact and try to avoid making an interface which\n> is at once too generic and not generic enough.\n\nOne thing that makes me somewhat worried is that \"add gpgsm support\"\nmay mean \"don't encourage people to use the same PGP like everybody\nelse does\" and instead promote fragmenting the world.\n\nBut that aside, assuming that it is a good idea to support gpgsm, I\nfully agree with your above assessment.\n\nThanks.\n"},{"id":"344903","messageId":"CAE=pOyE5oqtPZLQ7d9EHU3uPC2eG+2_6APFirRvHqywE=4dGpg@mail.gmail.com","threadId":"48253","inReplyTo":"xmqq604qwqbj.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-04-17T18:08:20Z","receivedAt":"2018-04-17T18:08:46Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Mon, Apr 16, 2018 at 7:54 PM, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n>\n>> If we just want to add gpgsm support, that's fine, but we should be\n>> transparent about that fact and try to avoid making an interface which\n>> is at once too generic and not generic enough.\n\nThis patch is definitely designed around PGP and CMS, but the config\noptions were intended to leave room for supporting other tools in the\nfuture. I think allowing a PEM type to be specified makes a lot of\nsense for PGP and CMS, but in the future, we can add a\n`signingtool..regex` option. Similarly, the GnuPG specific\ncommand line options and output parsing can be moved into a helper in\nthe future.\n\nMy motivation with this series is not just to \"add gpgsm support\"\nthough. I've been working on some other CMS tooling that will be open\nsource eventually. My goal was to distribute this tool with a wrapper\nthat emulates the GnuPG interface.\n\nTo me, this series feels like a good stepping stone towards more\ngeneralized support for other tooling.\n\n> One thing that makes me somewhat worried is that \"add gpgsm support\"\n> may mean \"don't encourage people to use the same PGP like everybody\n> else does\" and instead promote fragmenting the world.\n\nThere are a lot of projects for which PGP doesn't make sense. For\nexample, many large organizations and government entities don't\noperate based on a web of trust, but have established PKI based on\ncentralized trust. For organizations like this, adopting commit/tag\nsigning with CMS would be far easier than adopting PGP signing.\n\nThere's a chance that 10 different software projects will end up using\n10 different signing tools, but I don't see that as a problem if those\ntools are well suited to the projects. Developers are already\nresponsible for learning how to work with the software projects they\ncontribute to.\n"},{"id":"344919","messageId":"20180417183334.GA9802@syl.local","threadId":"48253","inReplyTo":"CAE=pOyE5oqtPZLQ7d9EHU3uPC2eG+2_6APFirRvHqywE=4dGpg@mail.gmail.com","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-17T18:33:34Z","receivedAt":"2018-04-17T18:33:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Apr 17, 2018 at 12:08:20PM -0600, Ben Toews wrote:\n> On Mon, Apr 16, 2018 at 7:54 PM, Junio C Hamano wrote:\n> > \"brian m. carlson\" writes:\n> >\n> >> If we just want to add gpgsm support, that's fine, but we should be\n> >> transparent about that fact and try to avoid making an interface which\n> >> is at once too generic and not generic enough.\n>\n> [...]\n>\n> My motivation with this series is not just to \"add gpgsm support\"\n> though. I've been working on some other CMS tooling that will be open\n> source eventually. My goal was to distribute this tool with a wrapper\n> that emulates the GnuPG interface.\n>\n> To me, this series feels like a good stepping stone towards more\n> generalized support for other tooling.\n\nI agree with Ben's assessment. A stand-in tool for gpgsm support will\nnot be useful without a way to configure it with Git. I think that\ntreating this series as ``add support for _gpgsm-like tools_'' is\nsensible, and a reasonable compromise between:\n\n - More generalized support.\n - No support at all.\n\nThanks,\nTaylor\n"},{"id":"346537","messageId":"CAE=pOyGy70kodr3DdR=cMggR8mT+yFvme672zkzztDsxuxDBRg@mail.gmail.com","threadId":"48253","inReplyTo":"20180417183334.GA9802@syl.local","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Ben Toews","fromEmail":"mastahyeti@gmail.com","sentAt":"2018-05-03T16:03:38Z","receivedAt":"2018-05-03T16:04:02Z","isPatch":true,"sender":{"key":"mastahyeti@gmail.com","avatar":"https://gravatar.com/avatar/81655150d7b5297aea788f51137084dd2e29b9aa4416e495a3717cb182ed7be9?d=mp&s=160"},"body":"On Tue, Apr 17, 2018 at 12:33 PM, Taylor Blau wrote:\n>\n> On Tue, Apr 17, 2018 at 12:08:20PM -0600, Ben Toews wrote:\n> > On Mon, Apr 16, 2018 at 7:54 PM, Junio C Hamano wrote:\n> > > \"brian m. carlson\" writes:\n> > >\n> > >> If we just want to add gpgsm support, that's fine, but we should be\n> > >> transparent about that fact and try to avoid making an interface which\n> > >> is at once too generic and not generic enough.\n> >\n> > [...]\n> >\n> > My motivation with this series is not just to \"add gpgsm support\"\n> > though. I've been working on some other CMS tooling that will be open\n> > source eventually. My goal was to distribute this tool with a wrapper\n> > that emulates the GnuPG interface.\n> >\n> > To me, this series feels like a good stepping stone towards more\n> > generalized support for other tooling.\n>\n> I agree with Ben's assessment. A stand-in tool for gpgsm support will\n> not be useful without a way to configure it with Git. I think that\n> treating this series as ``add support for _gpgsm-like tools_'' is\n> sensible, and a reasonable compromise between:\n>\n> - More generalized support.\n> - No support at all.\n>\n> Thanks,\n> Taylor\n\n\nAny more thoughts as to whether adding support for CMS tooling is\nworthwhile as a stepping stone towards supporting more general\ntooling?\n"},{"id":"346855","messageId":"20180507094459.GA3412@sigill.intra.peff.net","threadId":"48253","inReplyTo":"20180417001212.GC14631@genre.crustytoothpaste.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-07T09:45:00Z","receivedAt":"2018-05-07T09:45:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 17, 2018 at 12:12:12AM +0000, brian m. carlson wrote:\n\n> > That argues more strongly that we would regret unless we make the\n> > end-user configuration to at least the whole string (which later can\n> > be promoted to \"a pattern that matches the whole string\"), not just\n> > the part after mandatory \"-----BEGIN \", methinks.\n> \n> Yeah, I think this patch set is \"add gpgsm support\", which I can see as\n> a valuable goal in and of itself, but I'm not sure the attempt to make\n> it generic is in the right place. If we want to be truly generic, the\n> way to do that is to invoke a helper based on signature type (e.g.\n> git-sign-gpg, git-sign-gpgsm, git-sign-signify) to do the signing and\n> verification. We need not ship these helpers ourselves; interested\n> third-parties can provide them, and we can add configuration to match\n> against regexes for non-built-in types (which is required for many other\n> formats).\n\nIsn't that basically what this patch is, though? Or at least a step in\nthat direction? For generic signing support, you need:\n\n 1. A way to tell Git to recognize that a signature exists, and what\n type it is.\n\n 2. A way to tell Git how to invoke the signing tool.\n\nLet me discuss (2) first. In your git-sign-* world, then (2) requires\nus to define a set interface to those helpers, including which action to\nperform, which key to use, etc. And then the logic is inside the helper\nto translate that to the tool's interface.\n\nThe direction I anticipated for this patch was more like:\n\n - for now, we just piggy-back on gpg's interface for interacting with\n sub-programs. That makes gpgsm Just Work, and it means that you can\n plug in any other tool by writing a wrapper which converts from gpg\n options to the tool's interface. I.e., gpg's \"-bsau\" becomes the\n lingua franca, rather than us inventing a new one.\n\n - the config schema leaves room for adding new properties to each tool.\n So eventually we could support other option microformats by adding\n signingtool.foo.interface = \"signify\" or whatever.\n\n That still leaves room if we want to design our own helper interface.\n One thing we could do that this patch doesn't is require the user to\n explicitly ask for \"interface = gpg\" (and set it by default for the\n gpg tool stanza). And then leave it as an error if you have a tool\n config that doesn't specify its interface type, which leaves room for\n us later to make that default our generic interface.\n\nGetting back to (1), how do we tell Git to recognize a signature? I\nthink we have to use config here, since it would not know to invoke a\nhelper without recognizing the type (and we certainly do not want to\nspeculatively invoke each helper saying \"do you understand this?\" for\nefficiency reasons).\n\nSo let's assume we have some config to do matching. What should that\nlook like? Is it a substring match? A line match? A regex? There's a\ncontinuum of matching techniques that trade off simplicity for\nflexibility. My thought was that we'd eventually need to add multiple\nmatching methods, and users can pick the one that's simplest for the\nformat they're using.\n\nSo here we add pem-type matching, which errs very much on the side of\n\"very specific and easy, but not very flexible\". We can go further down\nthe continuum to \"match a line\" and require the user say the full:\n\n [signingtool \"gpg\"]\n matchLine = \"----- BEGIN PGP SIGNATURE -----\"\n\n(obviously they don't need to reconfigure gpg, but imagine they have a\nnew similar tool). That's not too bad. But does it extend things enough\nto match other types? It sounds like signify doesn't use a line-oriented\narmoring, and even matching a whole line wouldn't be enough. So now\nwe've erred on the other side; we made something more generic, but it's\nnot clear if it's actually generic enough to be useful.\n\nSo I'm open to the idea of line-matching, since the config above isn't\nsignificantly more complicated than the current pemType matcher. It does\nmean we can't later do pem-specific things, like matching the \"END\"\ndelimiter of the block (but so far we haven't needed to do that, since\nthe detached signature is always supposed to come at the end of the\ncontent).\n\nBut I'm not sure we're ready to go any further in making it generic,\nsince we don't know what the requirements will be (and won't until\nsomebody starts trying to plug in a new tool).\n\n-Peff\n"},{"id":"346875","messageId":"xmqq1senpk8c.fsf@gitster-ct.c.googlers.com","threadId":"48253","inReplyTo":"20180507094459.GA3412@sigill.intra.peff.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-07T15:18:43Z","receivedAt":"2018-05-07T15:18:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Apr 17, 2018 at 12:12:12AM +0000, brian m. carlson wrote:\n>\n>> > That argues more strongly that we would regret unless we make the\n>> > end-user configuration to at least the whole string (which later can\n>> > be promoted to \"a pattern that matches the whole string\"), not just\n>> > the part after mandatory \"-----BEGIN \", methinks.\n>> \n>> Yeah, I think this patch set is \"add gpgsm support\", which I can see as\n>> a valuable goal in and of itself, but I'm not sure the attempt to make\n>> it generic is in the right place. If we want to be truly generic, the\n>> way to do that is to invoke a helper based on signature type (e.g.\n>> git-sign-gpg, git-sign-gpgsm, git-sign-signify) to do the signing and\n>> verification. We need not ship these helpers ourselves; interested\n>> third-parties can provide them, and we can add configuration to match\n>> against regexes for non-built-in types (which is required for many other\n>> formats).\n>\n> Isn't that basically what this patch is, though? Or at least a step in\n> that direction?\n\nI think that is what Brian is saying, too (and if so I would also\nagree). It probably is a good step. It is just the feature may be\nsold under a wrong (or, overly wide) label, perhaps?\n"},{"id":"346927","messageId":"20180507230650.GH953644@genre.crustytoothpaste.net","threadId":"48253","inReplyTo":"20180507094459.GA3412@sigill.intra.peff.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-05-07T23:06:50Z","receivedAt":"2018-05-07T23:06:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, May 07, 2018 at 05:45:00AM -0400, Jeff King wrote:\n> Isn't that basically what this patch is, though? Or at least a step in\n> that direction? For generic signing support, you need:\n> \n> 1. A way to tell Git to recognize that a signature exists, and what\n> type it is.\n> \n> 2. A way to tell Git how to invoke the signing tool.\n> \n> Let me discuss (2) first. In your git-sign-* world, then (2) requires\n> us to define a set interface to those helpers, including which action to\n> perform, which key to use, etc. And then the logic is inside the helper\n> to translate that to the tool's interface.\n> \n> The direction I anticipated for this patch was more like:\n> \n> - for now, we just piggy-back on gpg's interface for interacting with\n> sub-programs. That makes gpgsm Just Work, and it means that you can\n> plug in any other tool by writing a wrapper which converts from gpg\n> options to the tool's interface. I.e., gpg's \"-bsau\" becomes the\n> lingua franca, rather than us inventing a new one.\n> \n> - the config schema leaves room for adding new properties to each tool.\n> So eventually we could support other option microformats by adding\n> signingtool.foo.interface = \"signify\" or whatever.\n> \n> That still leaves room if we want to design our own helper interface.\n> One thing we could do that this patch doesn't is require the user to\n> explicitly ask for \"interface = gpg\" (and set it by default for the\n> gpg tool stanza). And then leave it as an error if you have a tool\n> config that doesn't specify its interface type, which leaves room for\n> us later to make that default our generic interface.\n> \n> Getting back to (1), how do we tell Git to recognize a signature? I\n> think we have to use config here, since it would not know to invoke a\n> helper without recognizing the type (and we certainly do not want to\n> speculatively invoke each helper saying \"do you understand this?\" for\n> efficiency reasons).\n\nI think my main objection to this series is that it is generic in a way\nthat isn't necessarily useful. We know there are essentially only two\nformats of PEM-style signatures: OpenPGP and CMS[0]. Even if there are\nmore, they aren't intrinsically useful, because our codebase can only\nhandle GnuPG-style tools, and those are the only formats GnuPG-style\ntools really support (although, as you point out, other tools could\nmimic the interface).\n\nI think if we aren't going to implement some sort of interface that's\ngenerically useful for all signing tools, it would be better to simply\nsay that we support gpg and gpgsm and have signingtool.gpg.program and\nsigningtool.gpgsm.program and hard-code the logic for those two formats.\nThat way we don't have a generic interface that's really only useful for\nPEM-style tools, when we know it likely won't be useful for other tools\nas well. We can add a more generic interface when we have more varied\ntools to support and we know more about what the requirements will be.\n\nThis doesn't address Junio's concern about whether adding CMS support is\nthe right direction to go. I personally think OpenPGP is the right\ndirection for most open-source projects, but I know some companies want\nto use CMS internally and I'm not intrinsically opposed to that[1].\nThat decision is ultimately up to Junio, though.\n\n[0] I'm ignoring the original PEM, which specifies MD2 and MD5,\nalgorithms that nobody should be using these days.\n[1] I would welcome, though, if one could configure only one type of\nsignature verification by, say, setting the signing program to\n/bin/false in the config.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"346981","messageId":"20180508132814.GA24975@sigill.intra.peff.net","threadId":"48253","inReplyTo":"20180507230650.GH953644@genre.crustytoothpaste.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-08T13:28:14Z","receivedAt":"2018-05-08T13:28:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 07, 2018 at 11:06:50PM +0000, brian m. carlson wrote:\n\n> I think my main objection to this series is that it is generic in a way\n> that isn't necessarily useful. We know there are essentially only two\n> formats of PEM-style signatures: OpenPGP and CMS[0]. Even if there are\n> more, they aren't intrinsically useful, because our codebase can only\n> handle GnuPG-style tools, and those are the only formats GnuPG-style\n> tools really support (although, as you point out, other tools could\n> mimic the interface).\n> \n> I think if we aren't going to implement some sort of interface that's\n> generically useful for all signing tools, it would be better to simply\n> say that we support gpg and gpgsm and have signingtool.gpg.program and\n> signingtool.gpgsm.program and hard-code the logic for those two formats.\n> That way we don't have a generic interface that's really only useful for\n> PEM-style tools, when we know it likely won't be useful for other tools\n> as well. We can add a more generic interface when we have more varied\n> tools to support and we know more about what the requirements will be.\n\nOK, so my question then is: what does just-gpgsm support look like?\n\nDo we literally add gpgsm.program? My thought was that taking us the\nfirst step towards a more generic config scheme would prevent us having\nto backtrack later.\n\nThere are also more CMS signers than gpgsm (and I know Ben is working on\na tool). So it feels a little ugly to make it \"gpgsm.program\", since it\nreally is a more generic format.\n\nOr would you be happy if we just turned the matcher into a whole-line\nsubstring or regex match?\n\n> This doesn't address Junio's concern about whether adding CMS support is\n> the right direction to go. I personally think OpenPGP is the right\n> direction for most open-source projects, but I know some companies want\n> to use CMS internally and I'm not intrinsically opposed to that[1].\n> That decision is ultimately up to Junio, though.\n\nMy guess is that fragmentation isn't likely to be much of a problem in\npractice, because the tool choice generally falls along\nculture/community boundaries. I'd expect that open source projects are\nnever going to choose CMS, because the centralized cert management is\nawful. But it's exactly what many closed-source enterprises want, and\nthey will literally choose \"no signing\" over wrestling with PGP.\n\nI'd be much more worried about the open source world splitting into\n\"signify\" and \"gpg\" camps or similar. OTOH, I just don't see it as all\nthat big a deal. It's a project decision, and it may even allow for some\nhealthy competition between standards.\n\n-Peff\n"},{"id":"347031","messageId":"20180508230922.GB275488@genre.crustytoothpaste.net","threadId":"48253","inReplyTo":"20180508132814.GA24975@sigill.intra.peff.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2018-05-08T23:09:22Z","receivedAt":"2018-05-08T23:09:32Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, May 08, 2018 at 09:28:14AM -0400, Jeff King wrote:\n> OK, so my question then is: what does just-gpgsm support look like?\n> \n> Do we literally add gpgsm.program? My thought was that taking us the\n> first step towards a more generic config scheme would prevent us having\n> to backtrack later.\n\nI think the signingtool prefix is fine, or something similar. My \"just\ngpgsm\" proposal is literally just \"check for PGP header\" and \"check for\nCMS header\" in parse_signature and dispatch appropriately.\n\n> There are also more CMS signers than gpgsm (and I know Ben is working on\n> a tool). So it feels a little ugly to make it \"gpgsm.program\", since it\n> really is a more generic format.\n\nOkay, so signingtool.cms.program? signingtool.smime.program?\n\nI suppose Ben still intends to use the same command-line interface as\nfor gpgsm.\n\n> Or would you be happy if we just turned the matcher into a whole-line\n> substring or regex match?\n\nA first line regex would probably be fine, if you want to go that way.\nThat, I think, is generic enough that we can make use of it down the\nline, since it distinguishes all known formats, TTBOMK.\n\nIt would be nice if we could still continue to use gpg without having to\nadd specific configuration for it, at least for compatibility reasons.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"347057","messageId":"20180509080341.GB3327@sigill.intra.peff.net","threadId":"48253","inReplyTo":"20180508230922.GB275488@genre.crustytoothpaste.net","subject":"Re: [PATCH 8/8] gpg-interface: handle alternative signature types","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-05-09T08:03:41Z","receivedAt":"2018-05-09T08:03:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 08, 2018 at 11:09:22PM +0000, brian m. carlson wrote:\n\n> On Tue, May 08, 2018 at 09:28:14AM -0400, Jeff King wrote:\n> > OK, so my question then is: what does just-gpgsm support look like?\n> > \n> > Do we literally add gpgsm.program? My thought was that taking us the\n> > first step towards a more generic config scheme would prevent us having\n> > to backtrack later.\n> \n> I think the signingtool prefix is fine, or something similar. My \"just\n> gpgsm\" proposal is literally just \"check for PGP header\" and \"check for\n> CMS header\" in parse_signature and dispatch appropriately.\n\nHmm. I suppose that would work. I just didn't want to go the route of\nadding more hard-coded magic that the user couldn't override (or\nanything that is more complex than what the user could specify for their\nown tool if they wanted to). But I suppose there's probably not a big\nneed to override the GPG or CMS matching in practice.\n\n> > There are also more CMS signers than gpgsm (and I know Ben is working on\n> > a tool). So it feels a little ugly to make it \"gpgsm.program\", since it\n> > really is a more generic format.\n> \n> Okay, so signingtool.cms.program? signingtool.smime.program?\n> \n> I suppose Ben still intends to use the same command-line interface as\n> for gpgsm.\n\nAFAIK, yes.\n\n> > Or would you be happy if we just turned the matcher into a whole-line\n> > substring or regex match?\n> \n> A first line regex would probably be fine, if you want to go that way.\n> That, I think, is generic enough that we can make use of it down the\n> line, since it distinguishes all known formats, TTBOMK.\n\nThat seems like a happy medium to me. I worried at first that a regex\nmight be noticeably expensive, but probably not. During \"git log\" we\nonly feed \"gpgsig\" headers from each commit to the signature parsing\ncode (so it's few lines, and no cost when you aren't using signatures).\nFor tags we have to scan the whole tag body, but per-tag performance is\nmuch less important there, because you're not typically traversing\nhundreds of thousands of them.\n\n> It would be nice if we could still continue to use gpg without having to\n> add specific configuration for it, at least for compatibility reasons.\n\nDefinitely. Maintaining compatibility with the existing out-of-the-box\nbehavior and with the existing config options is non-negotiable (and is\nalready the case with the existing patch under discussion).\n\n-Peff\n"}]}