[PATCH 2/5] Signing API: Added new signing interface API

[PATCH 2/5] Signing API: Added new signing interface API

[Ibrahim El]

From: Ibrahim El Rhezzali <ibrahim.el@pm.me>

7e3e6c9e4 Added new signing interface API

Adding files for the new signing interface and also support drivers for the two existing GPG and GPGSM X.509 tools

Signed-off-by: Ibrahim El <ibrahim.el@pm.me>
---
 Makefile               |   3 +
 signing-interface.c    | 487 +++++++++++++++++++++++++++++++++++++++++++++++++
 signing-interface.h    | 151 +++++++++++++++
 signing-tool-openpgp.c | 409 +++++++++++++++++++++++++++++++++++++++++
 signing-tool-x509.c    | 383 ++++++++++++++++++++++++++++++++++++++
 signing-tool.h         |  35 ++++
 6 files changed, 1468 insertions(+)
 create mode 100644 signing-interface.c
 create mode 100644 signing-interface.h
 create mode 100644 signing-tool-openpgp.c
 create mode 100644 signing-tool-x509.c
 create mode 100644 signing-tool.h

diff --git a/Makefile b/Makefile
index f58bf14c7..244540e8d 100644
--- a/Makefile
+++ b/Makefile
@@ -978,6 +978,9 @@ LIB_OBJS += sha1-name.o
 LIB_OBJS += shallow.o
 LIB_OBJS += sideband.o
 LIB_OBJS += sigchain.o
+LIB_OBJS += signing-interface.o
+LIB_OBJS += signing-tool-openpgp.o
+LIB_OBJS += signing-tool-x509.o
 LIB_OBJS += split-index.o
 LIB_OBJS += strbuf.o
 LIB_OBJS += streaming.o
diff --git a/signing-interface.c b/signing-interface.c
new file mode 100644
index 000000000..c744ef499
--- /dev/null
+++ b/signing-interface.c
@@ -0,0 +1,487 @@
+#include <sys/types.h>
+#include <unistd.h>
+#include "cache.h"
+#include "config.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "signing-interface.h"
+#include "signing-tool.h"
+#include "sigchain.h"
+#include "tempfile.h"
+
+extern const struct signing_tool openpgp_tool;
+extern const struct signing_tool x509_tool;
+
+static const struct signing_tool *signing_tools[SIGNATURE_TYPE_COUNT] = {
+	&openpgp_tool,
+	&x509_tool,
+};
+
+enum signature_type default_type = SIGNATURE_TYPE_DEFAULT;
+static const char* unknown_signature_type = "unknown signature type";
+static char* default_signing_key = NULL;
+
+static void add_signature(struct signatures *sigs, struct signature *sig) {
+	if (!sigs || !sig)
+		return;
+	ALLOC_GROW(sigs->sigs, sigs->nsigs + 1, sigs->alloc);
+	sigs->sigs[sigs->nsigs++] = sig;
+}
+
+void signatures_clear(struct signatures *sigs)
+{
+	size_t i;
+	struct signature *psig;
+
+	if (!sigs) return;
+	
+	for (i = 0; i < sigs->nsigs; i++) {
+		psig = sigs->sigs[i];
+		strbuf_release(&(psig->sig));
+		strbuf_release(&(psig->output));
+		strbuf_release(&(psig->status));
+		FREE_AND_NULL(psig->signer);
+		FREE_AND_NULL(psig->key);
+		FREE_AND_NULL(psig->fingerprint);
+		FREE_AND_NULL(psig->key);
+		FREE_AND_NULL(psig);
+	}
+	FREE_AND_NULL(sigs->sigs);
+	sigs->nsigs = 0;
+	sigs->alloc = 0;
+}
+
+void signature_clear(struct signature *sigc)
+{
+	FREE_AND_NULL(sigc->sig.buf);
+	FREE_AND_NULL(sigc->output.buf);
+	FREE_AND_NULL(sigc->status.buf);
+	FREE_AND_NULL(sigc->signer);
+	FREE_AND_NULL(sigc->key);
+	FREE_AND_NULL(sigc->fingerprint);
+	FREE_AND_NULL(sigc->primary_key_fingerprint);
+}
+
+int sign_payload(const char *payload, size_t size, struct signatures *sigs,
+		enum signature_type st, const char *signing_key)
+{
+	const struct signing_tool *tool;
+	struct signature *psig = xmalloc(sizeof(struct signature));
+	int ret;
+
+	fflush(stdout);
+
+	if (!sigs)
+		return error("invalid signatures passed to sign function");
+
+	if (!VALID_SIGNATURE_TYPE(st))
+		return error("unsupported signature type: %d", st);
+
+	tool = signing_tools[st];
+
+	if (!tool || !tool->sign)
+		BUG("signing tool %s undefined", signature_type_name(st));
+
+	ret = tool->sign(payload, size, &psig, signing_key);
+	if (!ret)
+		add_signature(sigs, psig);
+	else
+
+		return error("signing operation failed");
+
+	return 0;
+}
+
+int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key)
+{
+	struct signatures sigs = SIGNATURES_INIT;
+	enum signature_type st = default_type;
+
+	int ret = sign_payload(buffer->buf, buffer->len, &sigs, st, signing_key);
+
+	if (!ret)
+	{
+		strbuf_addstr(signature, sigs.sigs[0]->sig.buf);
+	}
+
+	return ret;
+}
+
+size_t parse_signatures(const char *payload, size_t size, 
+		struct signatures *sigs)
+{
+	enum signature_type st;
+	size_t first;
+	size_t begin = 0;
+	const struct signing_tool *tool;
+	struct signature *psig = NULL;
+
+	first = size;
+	for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++) {
+		tool = signing_tools[st];
+
+		if (!tool || !tool->parse)
+			BUG("signing tool %s undefined", signature_type_name(st));
+
+		begin = tool->parse(payload, size, &psig);
+		if (begin < size) {
+			if (sigs)
+				add_signature(sigs, psig);
+			else
+				FREE_AND_NULL(psig);
+
+			first = begin;
+			continue;
+		}
+	}
+
+	return first;
+}
+
+size_t parse_signature(const char *buf, size_t size)
+{
+	size_t match;
+	struct signatures sigs = SIGNATURES_INIT;
+
+	if ( !buf || !size )
+		return size;
+
+	match = parse_signatures(buf, size, &sigs);
+
+	return match;
+}
+
+int verify_buffer_signatures(const char *payload, size_t size,
+		struct signatures *sigs)
+{
+	int ret = 0;
+	size_t i;
+	const struct signing_tool *tool;
+	struct signature *psig;
+
+	if (!sigs)
+		error("invalid signatures passed to verify function");
+
+	for (i = 0; i < sigs->nsigs; i++) {
+		psig = sigs->sigs[i];
+		tool = signing_tools[psig->st];
+
+		if (!tool || !tool->verify)
+			BUG("signing tool %s undefined", signature_type_name(psig->st));
+
+		ret |= tool->verify(payload, size, psig);
+	}
+
+	return ret;
+}
+
+int verify_signed_buffer(const char *payload, size_t payload_size,
+			 const char *signature, size_t signature_size,
+			 struct strbuf *output, struct strbuf *status)
+{
+	int ret;
+	enum signature_type st;
+	struct signature sig = SIGNATURE_INIT;
+	struct signatures sigs = SIGNATURES_INIT;
+
+	if ( !payload || !signature )
+		return error("invalid payload or signature sent !");
+
+	strbuf_addstr(&(sig.sig), signature);
+	add_signature(&sigs, &sig);
+
+	ret = verify_buffer_signatures(payload, payload_size, &sigs);
+
+	/*  Some how gpg.format is not sometimes applied, temporary fix to loop and STs */
+	if (ret)
+	{
+		for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++)
+		{
+			sig.st = st;
+			ret = verify_buffer_signatures(payload, payload_size, &sigs);
+			if (!ret || sig.result != '0')
+				break;
+		}
+	}
+
+	if (output)
+		strbuf_addstr(output, sig.output.buf);
+	if (status)
+		strbuf_addstr(status, sig.status.buf);
+
+	return ret;
+}
+
+int check_signature(const char *payload, size_t plen, const char *signature,
+	size_t slen, struct signature *sigc)
+{
+	int status;
+	enum signature_type st;
+	struct signatures sigs = SIGNATURES_INIT;
+	struct signature sig = SIGNATURE_INIT;
+	
+	if (!payload || !signature || !sigc)
+		BUG("invalid payload or signature sent !");
+
+	strbuf_addstr(&(sig.sig), signature);
+	sig.result = 'N';
+	sig.st = default_type;
+
+	add_signature(&sigs, &sig);
+
+	status = verify_buffer_signatures(payload, plen, &sigs);
+
+	/*  Some how gpg.format is not sometimes applied, temporary fix to loop and STs */
+	if (status)
+	{
+		for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++)
+		{
+			sig.st = st;
+			status = verify_buffer_signatures(payload, plen, &sigs);
+			if (!status || sig.result != 'N')
+				break;
+		}
+	}
+	status |= sig.result != 'G' && sig.result != 'U';
+
+	if (sig.signer && !sigc->signer)
+		sigc->signer = xstrdup(sig.signer);
+	if (sig.key && !sigc->key)
+		sigc->key = xstrdup(sig.key);
+	if (sig.fingerprint && !sigc->fingerprint)
+		sigc->fingerprint = xstrdup(sig.fingerprint);
+	if (sig.primary_key_fingerprint && !sigc->primary_key_fingerprint)
+		sigc->primary_key_fingerprint = xstrdup(sig.primary_key_fingerprint);	
+
+	sigc->st = sig.st;
+	sigc->result = sig.result;
+	
+	strbuf_addstr(&(sigc->sig), payload);
+	strbuf_addstr(&(sigc->output), sig.output.buf);
+	strbuf_addstr(&(sigc->status), sig.status.buf);
+
+	return !!status;
+}
+
+size_t strbuf_append_signatures(struct strbuf *buf, const struct signatures *sigs)
+{
+	size_t i;
+	struct signature *psig;
+
+	if (!buf)
+		BUG("invalid buffer passed to signature append function");
+
+	if (!sigs)
+		return 0;
+
+	for (i = 0; i < sigs->nsigs; i++) {
+		psig = sigs->sigs[i];
+		strbuf_addbuf(buf, &(psig->sig));
+	}
+
+	return sigs->nsigs;
+}
+
+void print_signatures(const struct signatures *sigs, unsigned flags)
+{
+	size_t i;
+	const struct signing_tool *tool;
+	const struct signature *psig;
+
+	if (!sigs)
+		error("invalid signatures passed to verify function");
+
+	for (i = 0; i < sigs->nsigs; i++) {
+		psig = sigs->sigs[i];
+		tool = signing_tools[psig->st];
+
+		if (!tool || !tool->print)
+			BUG("signing tool %s undefined", signature_type_name(psig->st));
+
+		tool->print(psig, flags);
+	}
+}
+
+void print_signature_buffer(const struct signature *sigc, unsigned flags)
+{
+	const struct signing_tool *tool;
+
+	if (!sigc)
+		error("invalid signatures passed to verify function");
+
+	tool = signing_tools[default_type];
+
+	if (!tool || !tool->print)
+		BUG("signing tool %s undefined", signature_type_name(sigc->st));
+
+	tool->print(sigc, flags);
+}
+
+enum signature_type signature_type_by_name(const char *name)
+{
+	enum signature_type st;
+
+	if (!name)
+		return default_type;
+
+	for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++)
+		if (!strcmp(signing_tools[st]->name, name))
+			return st;
+
+	return error("unknown signature type: %s", name);
+}
+
+const char *signature_type_name(enum signature_type st)
+{
+	if (!VALID_SIGNATURE_TYPE(st))
+		return unknown_signature_type;
+
+	return signing_tools[st]->name;
+}
+
+int git_signing_config(const char *var, const char *value, void *cb)
+{
+	int ret = 0;
+	char *t1, *t2, *t3, *buf;
+	enum signature_type st;
+	const struct signing_tool *tool;
+
+	/* user.signingkey is a deprecated alias for signing.<signing.default>.key */
+	if (!strcmp(var, "user.signingkey")) {
+		if (!value)
+			return config_error_nonbool(var);
+		
+		set_signing_key(value, default_type);
+
+		return 0;
+	}
+
+	/* gpg.format is a deprecated alias for signing.default */
+	if (!strcmp(var, "gpg.format") || !strcmp(var, "signing.default")) {
+		if (!value)
+			return config_error_nonbool(var);
+
+		if (!VALID_SIGNATURE_TYPE((st = signature_type_by_name(value))))
+			return config_error_nonbool(var);
+
+		set_signature_type(st);
+
+		return 0;
+	}
+
+	/* gpg.program is a deprecated alias for signing.openpgp.program */
+	if (!strcmp(var, "gpg.program") || !strcmp(var, "signing.openpgp.program")) {
+		ret = signing_tools[OPENPGP_SIGNATURE]->config(
+				"program", value, cb);
+
+		return ret;
+	}
+
+	/* gpg.x509.program is a deprecated alias for signing.x509.program */
+	if (!strcmp(var, "gpg.x509.program") || !strcmp(var, "signing.x509.program")) {
+		ret = signing_tools[X509_SIGNATURE]->config(
+				"program", value, cb);
+
+		return ret;
+	}
+
+	buf = xstrdup(var);
+	t1 = strtok(buf, ".");
+	t2 = strtok(NULL, ".");
+	t3 = strtok(NULL, ".");
+
+	/* gpg.<format>.* is a deprecated alias for signing.<format>.* */
+	if (!strcmp(t1, "gpg") || !strcmp(t1, "signing")) {
+		if (!VALID_SIGNATURE_TYPE((st = signature_type_by_name(t2)))) {
+			free(buf);
+			return error("unsupported variable: %s", var);
+		}
+
+		tool = signing_tools[st];
+		if (!tool || !tool->config) {
+			free(buf);
+			BUG("signing tool %s undefined", signature_type_name(tool->st));
+		}
+
+		ret = tool->config(t3, value, cb);
+	}
+
+	free(buf);
+	return ret;
+}
+
+void set_signing_key(const char *key, enum signature_type st)
+{
+	/*
+	 * Make sure we track the latest default signing key so that if the
+	 * default signing format changes after this, we can make sure the
+	 * default signing tool knows the key to use.
+	 */
+	free(default_signing_key);
+	default_signing_key = xstrdup(key);
+
+	if (!VALID_SIGNATURE_TYPE(st))
+		signing_tools[default_type]->set_key(key);
+	else
+		signing_tools[st]->set_key(key);
+}
+
+const char *get_signing_key(enum signature_type st)
+{
+	if (!VALID_SIGNATURE_TYPE(st))
+		return signing_tools[default_type]->get_key();
+
+	return signing_tools[default_type]->get_key();
+}
+
+void set_signing_program(const char *signing_program, enum signature_type st)
+{
+	/*
+	 * Make sure we track the latest default signing program so that if the
+	 * default signing format changes after this, we can make sure the
+	 * default signing tool knows the program to use.
+	 */
+
+	if (!VALID_SIGNATURE_TYPE(st))
+		signing_tools[default_type]->set_program(signing_program);
+	else
+		signing_tools[st]->set_program(signing_program);
+}
+
+const char *get_signing_program(enum signature_type st)
+{
+	const char *signing_program = NULL;
+
+	if (!VALID_SIGNATURE_TYPE(st)) {
+		signing_program = signing_tools[default_type]->get_program();
+
+		return signing_program;
+	}
+
+	signing_program = signing_tools[st]->get_program();
+
+	return signing_program;
+}
+
+void set_signature_type(enum signature_type st)
+{
+	if (!VALID_SIGNATURE_TYPE(st))
+		return;
+
+	default_type = st;
+
+	/* 
+	 * If the signing key has been set, then make sure the new default
+	 * signing tool knows about it. this fixes the order of operations
+	 * error of parsing the default signing key and default signing
+	 * format in arbitrary order.
+	 */
+	if (default_signing_key) {
+		set_signing_key(default_signing_key, default_type);
+	}
+}
+
+enum signature_type get_signature_type(void)
+{
+	return default_type;
+}
\ No newline at end of file
diff --git a/signing-interface.h b/signing-interface.h
new file mode 100644
index 000000000..b55edbdb8
--- /dev/null
+++ b/signing-interface.h
@@ -0,0 +1,151 @@
+#ifndef SIGNING_INTERFACE_H
+#define SIGNING_INTERFACE_H
+
+struct strbuf;
+
+#define OUTPUT_VERBOSE		1
+#define OUTPUT_RAW			2
+#define OUTPUT_OMIT_STATUS	4
+
+enum signature_type {
+	OPENPGP_SIGNATURE,
+	X509_SIGNATURE,
+
+	SIGNATURE_TYPE_LAST,
+	SIGNATURE_TYPE_FIRST = OPENPGP_SIGNATURE,
+	SIGNATURE_TYPE_COUNT = SIGNATURE_TYPE_LAST - SIGNATURE_TYPE_FIRST,
+	SIGNATURE_TYPE_DEFAULT = OPENPGP_SIGNATURE,
+	SIGNATURE_TYPE_UNKNOWN = -1
+};
+enum signature_type default_type;
+
+#define VALID_SIGNATURE_TYPE(x) \
+	((x >= SIGNATURE_TYPE_FIRST) && (x < SIGNATURE_TYPE_LAST))
+
+struct signature {
+	struct strbuf sig;
+	struct strbuf output;
+	struct strbuf status;
+	enum signature_type st;
+
+	/*
+	 * possible "result":
+	 * 0 (not checked)
+	 * N (checked but no further result)
+	 * U (untrusted good)
+	 * G (good)
+	 * B (bad)
+	 */
+	char result;
+	char *signer;
+	char *key;
+	char *fingerprint;
+	char *primary_key_fingerprint;
+};
+
+struct signatures {
+	size_t nsigs;
+	size_t alloc;
+	struct signature **sigs;
+};
+
+#define SIGNATURES_INIT  { .nsigs = 0, .alloc = 0, .sigs = NULL }
+#define SIGNATURE_INIT  { .sig = STRBUF_INIT, .output = STRBUF_INIT, .status = STRBUF_INIT, .st = OPENPGP_SIGNATURE, .result = '0', .signer = NULL, .key = NULL }
+
+void signatures_clear(struct signatures *sigs);
+void signature_clear(struct signature *sig);
+
+/*
+ * Create a detached signature for the contents of "payload" and append
+ * it to the list of signatures in "sigs". The signature type determines which
+ * type of signature to create and the optional "signing_key" specifies
+ * the key. If no signing key is specified the default key from the
+ * config will be used. If no default is found, then an error is
+ * returned. If the signing operation fails an error is returned.
+ */
+int sign_payload(const char *payload, size_t size, struct signatures *sigs,
+		enum signature_type st, const char *signing_key);
+
+/*
+ * Bridge function to be called by the git code for buffer signature
+ */
+int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key);
+
+/* 
+ * Look at the signed content (e.g. a signed tag object), whose payload
+ * is followed by one or more detached signatures. Return the offset of
+ * the first signature, or the size of the buf when there are no 
+ * signatures. If a valid signatures struct is passed in, the signatures 
+ * will be parsed and copied into its array of sigs.
+ */
+size_t parse_signatures(const char *payload, size_t size,
+		struct signatures *sigs);
+
+/*
+ * Bridge function to be called by the git code for parsing signatures in a buffer
+ */
+size_t parse_signature(const char *buf, size_t size);
+
+/*
+ * Run the signature verification tools to see if the payload matches
+ * the detached signatures. The output and status of the of the checks
+ * is recorded in the signatures struct. The caller must use
+ * parse_signatures or sign_buffer to initialize the signatures struct
+ * before calling this function.
+ */
+int verify_signed_buffer(const char *payload, size_t payload_size,
+			 const char *signature, size_t signature_size,
+			 struct strbuf *output, struct strbuf *status);
+
+/*
+ * Verify multiple signatures in a single buffer
+ */
+int verify_buffer_signatures(const char *payload, size_t size,
+		struct signatures *sigs);
+
+/*
+ * Bridge function to be called by the git code to verify a signed payload
+ */
+int check_signature(const char *payload, size_t plen, const char *signature,
+	size_t slen, struct signature *sigc);
+
+/*
+ * Prints the results of either signing or verifying the payload in the
+ * signatures struct. If the OUTPUT_VERBOSE flag is specified, then the
+ * payload is printed to stdout. If the OUTPUT_RAW flag is specified, 
+ * the raw status output from the signing tool is printed to stderr, 
+ * otherwise, the nice results from the tool is printed to stderr.
+ */
+void print_signatures(const struct signatures *sigs, unsigned flags);
+
+/*
+ * Bridge function to be called by the git code to print a signature
+ */
+void print_signature_buffer(const struct signature *sigc, unsigned flags);
+
+/*
+ * Appends each of the detached signatures to the end of the strbuf
+ * passed in. Returns the number of signatures appended to the buffer.
+ */
+size_t strbuf_append_signatures(struct strbuf *buf, const struct signatures *sigs);
+
+/*
+ * Translate the name of the signature tool into the enumerated value
+ * for the signature type.
+ */
+enum signature_type signature_type_by_name(const char *name);
+const char *signature_type_name(enum signature_type st);
+
+/*
+ * Config related functions
+ */
+int git_signing_config(const char *var, const char *value, void *cb);
+void set_signing_key(const char *key, enum signature_type st);
+const char *get_signing_key(enum signature_type st);
+void set_signing_program(const char *program, enum signature_type st);
+const char *get_signing_program(enum signature_type st);
+void set_signature_type(enum signature_type st);
+enum signature_type get_signature_type(void);
+
+#endif
+
diff --git a/signing-tool-openpgp.c b/signing-tool-openpgp.c
new file mode 100644
index 000000000..93b63b36d
--- /dev/null
+++ b/signing-tool-openpgp.c
@@ -0,0 +1,409 @@
+#include "cache.h"
+#include "config.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "signing-interface.h"
+#include "signing-tool.h"
+#include "sigchain.h"
+#include "tempfile.h"
+
+static int openpgp_sign(const char *payload, size_t size,
+		struct signature **sig, const char *key);
+static size_t openpgp_parse(const char *payload, size_t size,
+		struct signature **sig);
+static int openpgp_verify(const char *payload, size_t size,
+		struct signature *sig);
+static void openpgp_print(const struct signature *sig, unsigned flags);
+static int openpgp_config(const char *, const char *, void *);
+static void openpgp_set_key(const char *);
+static const char *openpgp_get_key(void);
+static void openpgp_set_program(const char *);
+static const char *openpgp_get_program(void);
+
+const struct signing_tool openpgp_tool = {
+	.st = OPENPGP_SIGNATURE,
+	.name = "openpgp",
+	.sign = &openpgp_sign,
+	.parse = &openpgp_parse,
+	.verify = &openpgp_verify,
+	.print = &openpgp_print,
+	.config = &openpgp_config,
+	.set_key = &openpgp_set_key,
+	.get_key = &openpgp_get_key,
+	.set_program = &openpgp_set_program,
+	.get_program = &openpgp_get_program
+};
+
+static const char *program = "gpg";
+static const char *signing_key = NULL;
+static const char *keyring = NULL;
+static int no_default_keyring = 0;
+struct regex_pattern {
+	const char * begin;
+	const char * end;
+};
+static struct regex_pattern patterns[2] = {
+	{ "^-----BEGIN PGP SIGNATURE-----\n", "-----END PGP SIGNATURE-----\n" },
+	{ "^-----BEGIN PGP MESSAGE-----\n", "-----END PGP MESSAGE-----\n" }
+};
+
+static int openpgp_sign(const char *payload, size_t size,
+		struct signature **sig, const char *key)
+{
+	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct signature *psig;
+	struct strbuf *psignature, *pstatus;
+	int ret;
+	size_t i, j;
+	const char *skey = (!key || !*key) ? signing_key : key;
+
+	/*
+	 * Create the signature.
+	 */
+	if (sig) {
+		psig = *sig;
+		strbuf_init(&(psig->sig), 0);
+		strbuf_init(&(psig->output), 0);
+		strbuf_init(&(psig->status), 0);
+		psig->st = OPENPGP_SIGNATURE;
+		psig->result = 0;
+		psig->signer = NULL;
+		psig->key = NULL;
+		psignature = &(psig->sig);
+		pstatus = &(psig->status);
+	} else {
+		psignature = NULL;
+		pstatus = NULL;
+	}
+
+	argv_array_pushl(&gpg.args,
+			program,
+			"--status-fd=2",
+			"-bsau", skey,
+			NULL);
+
+	/*
+	 * When the username signingkey is bad, program could be terminated
+	 * because gpg exits without reading and then write gets SIGPIPE.
+	 */
+	sigchain_push(SIGPIPE, SIG_IGN);
+	ret = pipe_command(&gpg, payload, size,
+			psignature, 1024, pstatus, 0);
+	sigchain_pop(SIGPIPE);
+
+	if (!sig)
+		return !!ret;
+
+	/* Check for success status from gpg */
+	ret |= !strstr(pstatus->buf, "\n[GNUPG:] SIG_CREATED ");
+
+	if (ret)
+		return error(_("gpg failed to sign the data"));
+
+	/* Mark the signature as good */
+	psig->result = 'G';
+
+	/* Strip CR from the line endings, in case we are on Windows. */
+	for (i = j = 0; i < psig->sig.len; i++)
+		if (psig->sig.buf[i] != '\r') {
+			if (i != j)
+				psig->sig.buf[j] = psig->sig.buf[i];
+			j++;
+		}
+	strbuf_setlen(&(psig->sig), j);
+
+	/* Store the key we used */
+	psig->key = xstrdup(skey);
+
+	return 0;
+}
+
+/*
+ * To get all OpenPGP signatures in a payload, repeatedly call this function
+ * giving it the remainder of the payload as the payload pointer. The return
+ * value is the index of the first char of the signature in the payload. If
+ * no signature is found, size is returned.
+ */
+static size_t openpgp_parse(const char *payload, size_t size,
+		struct signature **sig)
+{
+	int i, ret;
+	regex_t rbegin;
+	regex_t rend;
+	regmatch_t bmatch;
+	regmatch_t ematch;
+	size_t begin, end;
+	struct signature *psig;
+	static char errbuf[1024];
+
+	if (size == 0)
+		return size;
+
+	/*
+	 * Figure out if any OpenPGP signatures are in the payload and which
+	 * begin pattern matches the first signature in the payload.
+	 */
+	for (i = 0; i < ARRAY_SIZE(patterns); i++) {
+		if ((ret = regcomp(&rbegin, patterns[i].begin, REG_EXTENDED|REG_NEWLINE))) {
+			regerror(ret, &rbegin, errbuf, 1024);
+			BUG("Failed to compile regex: %s\n", errbuf);
+
+			return size;
+		}
+		if ((ret = regcomp(&rend, patterns[i].end, REG_EXTENDED|REG_NEWLINE))) {
+			regerror(ret, &rend, errbuf, 1024);
+			BUG("Failed to compile regex: %s\n", errbuf);
+
+			return size;
+		}
+
+		begin = end = 0;
+		if (regexec(&rbegin, payload, 1, &bmatch, 0) ||
+			regexec(&rend, payload, 1, &ematch, 0)) {
+			begin = size;
+			continue;
+		}
+		begin = bmatch.rm_so;
+		end = ematch.rm_eo;
+
+		break;
+	}
+	if (begin == size)
+		goto next;
+
+	/*
+	 * Create the signature.
+	 */
+	if (sig) {
+		psig = *sig;
+		psig = xmalloc(sizeof(struct signature));
+		strbuf_init(&(psig->sig), end - begin);
+		strbuf_add(&(psig->sig), payload + begin, end - begin);
+		strbuf_init(&(psig->output), 0);
+		strbuf_init(&(psig->status), 0);
+		psig->st = OPENPGP_SIGNATURE;
+		psig->result = 0;
+		psig->signer = NULL;
+		psig->key = NULL;
+	}
+	next:
+		regfree(&rbegin);
+		regfree(&rend);
+
+	return begin;
+}
+
+/* An exclusive status -- only one of them can appear in output */
+#define GPG_STATUS_EXCLUSIVE	(1<<0)
+/* The status includes key identifier */
+#define GPG_STATUS_KEYID	(1<<1)
+/* The status includes user identifier */
+#define GPG_STATUS_UID		(1<<2)
+/* The status includes key fingerprints */
+#define GPG_STATUS_FINGERPRINT	(1<<3)
+
+/* Short-hand for standard exclusive *SIG status with keyid & UID */
+#define GPG_STATUS_STDSIG	(GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID|GPG_STATUS_UID)
+
+static struct {
+	char result;
+	const char *check;
+	unsigned int flags;
+} sigcheck_gpg_status[] = {
+	{ 'G', "GOODSIG ", GPG_STATUS_STDSIG },
+	{ 'B', "BADSIG ", GPG_STATUS_STDSIG },
+	{ 'U', "TRUST_NEVER", 0 },
+	{ 'U', "TRUST_UNDEFINED", 0 },
+	{ 'E', "ERRSIG ", GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID },
+	{ 'X', "EXPSIG ", GPG_STATUS_STDSIG },
+	{ 'Y', "EXPKEYSIG ", GPG_STATUS_STDSIG },
+	{ 'R', "REVKEYSIG ", GPG_STATUS_STDSIG },
+	{ 0, "VALIDSIG ", GPG_STATUS_FINGERPRINT },
+};
+
+static void parse_output(struct signature *sigc)
+{
+	const char *buf = sigc->status.buf;
+	const char *line, *next;
+	int i, j;
+	int seen_exclusive_status = 0;
+
+	/* Iterate over all lines */
+	for (line = buf; *line; line = strchrnul(line+1, '\n')) {
+		while (*line == '\n')
+			line++;
+		/* Skip lines that don't start with GNUPG status */
+		if (!skip_prefix(line, "[GNUPG:] ", &line))
+			continue;
+
+		/* Iterate over all search strings */
+		for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
+			if (skip_prefix(line, sigcheck_gpg_status[i].check, &line)) {
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_EXCLUSIVE) {
+					if (seen_exclusive_status++)
+						goto found_duplicate_status;
+				}
+
+				if (sigcheck_gpg_status[i].result)
+					sigc->result = sigcheck_gpg_status[i].result;
+				/* Do we have key information? */
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_KEYID) {
+					next = strchrnul(line, ' ');
+					free(sigc->key);
+					sigc->key = xmemdupz(line, next - line);
+					/* Do we have signer information? */
+					if (*next && (sigcheck_gpg_status[i].flags & GPG_STATUS_UID)) {
+						line = next + 1;
+						next = strchrnul(line, '\n');
+						free(sigc->signer);
+						sigc->signer = xmemdupz(line, next - line);
+					}
+				}
+				/* Do we have fingerprint? */
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_FINGERPRINT) {
+					next = strchrnul(line, ' ');
+					free(sigc->fingerprint);
+					sigc->fingerprint = xmemdupz(line, next - line);
+
+					/* Skip interim fields */
+					for (j = 9; j > 0; j--) {
+						if (!*next)
+							break;
+						line = next + 1;
+						next = strchrnul(line, ' ');
+					}
+
+					next = strchrnul(line, '\n');
+					free(sigc->primary_key_fingerprint);
+					sigc->primary_key_fingerprint = xmemdupz(line, next - line);
+				}
+
+				break;
+			}
+		}
+	}
+	return;
+
+found_duplicate_status:
+	/*
+	 * GOODSIG, BADSIG etc. can occur only once for each signature.
+	 * Therefore, if we had more than one then we're dealing with multiple
+	 * signatures.  We don't support them currently, and they're rather
+	 * hard to create, so something is likely fishy and we should reject
+	 * them altogether.
+	 */
+	sigc->result = 'E';
+	/* Clear partial data to avoid confusion */
+	FREE_AND_NULL(sigc->primary_key_fingerprint);
+	FREE_AND_NULL(sigc->fingerprint);
+	FREE_AND_NULL(sigc->signer);
+	FREE_AND_NULL(sigc->key);
+}
+
+static int openpgp_verify(const char *payload, size_t size,
+		struct signature *sig)
+{
+	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct tempfile *temp;
+	int ret;
+
+	temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
+	if (!temp)
+		return error_errno(_("could not create temporary file"));
+	if (write_in_full(temp->fd, sig->sig.buf, sig->sig.len) < 0 ||
+	    close_tempfile_gently(temp) < 0) {
+		error_errno(_("failed writing detached signature to '%s'"),
+				temp->filename.buf);
+		delete_tempfile(&temp);
+		return -1;
+	}
+
+	argv_array_push(&gpg.args, program);
+	if (keyring)
+		argv_array_pushl(&gpg.args, "--keyring", keyring, NULL);
+	if (no_default_keyring)
+		argv_array_push(&gpg.args, "--no-default-keyring");
+	argv_array_pushl(&gpg.args,
+			"--keyid-format=long",
+			"--status-fd=1",
+			"--verify", temp->filename.buf, "-",
+			NULL);
+
+	strbuf_reset(&(sig->status));
+	strbuf_reset(&(sig->output));
+
+	sigchain_push(SIGPIPE, SIG_IGN);
+	ret = pipe_command(&gpg, payload, size,
+			&(sig->status), 0, &(sig->output), 0);
+	sigchain_pop(SIGPIPE);
+
+	delete_tempfile(&temp);
+
+	ret |= !strstr(sig->status.buf, "\n[GNUPG:] GOODSIG ");
+
+	if (ret && !sig->output.len)
+		return !!ret;
+
+	parse_output(sig);
+
+	ret |= sig->result != 'G' && sig->result != 'U';
+
+	return !!ret;
+}
+
+static void openpgp_print(const struct signature *sig, unsigned flags)
+{
+	const char *output = flags & OUTPUT_RAW ?
+		sig->status.buf : sig->output.buf;
+
+	if (flags & OUTPUT_VERBOSE && sig->sig.buf)
+		fputs(sig->sig.buf, stdout);
+
+	if (output)
+		fputs(output, stderr);
+}
+
+static int openpgp_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "program"))
+		return git_config_string(&program, var, value);
+
+	if (!strcmp(var, "key"))
+		return git_config_string(&signing_key, var, value);
+
+	if (!strcmp(var, "keyring"))
+		return git_config_string(&keyring, var, value);
+
+	if (!strcmp(var, "nodefaultkeyring")) {
+		no_default_keyring = git_config_bool(var, value);
+		return 0;
+	}
+	return 0;
+}
+
+static void openpgp_set_key(const char *key)
+{
+	free((void*)signing_key);
+	signing_key = xstrdup(key);
+}
+
+static const char *openpgp_get_key(void)
+{
+	if (signing_key)
+		return signing_key;
+	return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
+}
+
+static void openpgp_set_program(const char *signing_program)
+{
+	free((void*)program);
+	program = xstrdup(signing_program);
+}
+
+
+static const char *openpgp_get_program(void)
+{
+	if (program)
+		return program;
+	return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
+}
\ No newline at end of file
diff --git a/signing-tool-x509.c b/signing-tool-x509.c
new file mode 100644
index 000000000..b7de56924
--- /dev/null
+++ b/signing-tool-x509.c
@@ -0,0 +1,383 @@
+#include "cache.h"
+#include "config.h"
+#include "run-command.h"
+#include "strbuf.h"
+#include "signing-interface.h"
+#include "signing-tool.h"
+#include "sigchain.h"
+#include "tempfile.h"
+
+static int x509_sign(const char *payload, size_t size,
+		struct signature **sig, const char *key);
+static size_t x509_parse(const char *payload, size_t size,
+		struct signature **sig);
+static int x509_verify(const char *payload, size_t size,
+		struct signature *sig);
+static void x509_print(const struct signature *sig, unsigned flags);
+static int x509_config(const char *, const char *, void *);
+static void x509_set_key(const char *);
+static const char *x509_get_key(void);
+static void x509_set_program(const char *);
+static const char *x509_get_program(void);
+
+const struct signing_tool x509_tool = {
+	.st = X509_SIGNATURE,
+	.name = "x509",
+	.sign = &x509_sign,
+	.parse = &x509_parse,
+	.verify = &x509_verify,
+	.print = &x509_print,
+	.config = &x509_config,
+	.set_key = &x509_set_key,
+	.get_key = &x509_get_key,
+	.set_program = &x509_set_program,
+	.get_program = &x509_get_program
+};
+
+static const char *program = "gpgsm";
+static const char *signing_key = NULL;
+struct regex_pattern {
+	const char * begin;
+	const char * end;
+};
+static struct regex_pattern pattern = {
+	"^-----BEGIN SIGNED MESSAGE-----\n",
+	"^-----END SIGNED MESSAGE-----\n"
+};
+
+static int x509_sign(const char *payload, size_t size,
+		struct signature **sig, const char *key)
+{
+	struct child_process gpgsm = CHILD_PROCESS_INIT;
+	struct signature *psig;
+	struct strbuf *psignature, *pstatus;
+	int ret;
+	size_t i, j;
+	const char *skey = (!key || !*key) ? signing_key : key;
+
+	/*
+	 * Create the signature.
+	 */
+	if (sig) {
+		psig = *sig;
+		strbuf_init(&(psig->sig), 0);
+		strbuf_init(&(psig->output), 0);
+		strbuf_init(&(psig->status), 0);
+		psig->st = X509_SIGNATURE;
+		psig->result = 0;
+		psig->signer = NULL;
+		psig->key = NULL;
+		psignature = &(psig->sig);
+		pstatus = &(psig->status);
+	} else {
+		psignature = NULL;
+		pstatus = NULL;
+	}
+
+	argv_array_pushl(&gpgsm.args,
+			program,
+			"--status-fd=2",
+			"-bsau", skey,
+			NULL);
+
+	/*
+	 * When the username signingkey is bad, program could be terminated
+	 * because gpgsm exits without reading and then write gets SIGPIPE.
+	 */
+	sigchain_push(SIGPIPE, SIG_IGN);
+	ret = pipe_command(&gpgsm, payload, size,
+			psignature, 1024, pstatus, 0);
+	sigchain_pop(SIGPIPE);
+
+	if (!sig)
+		return !!ret;
+
+	ret |= !strstr(pstatus->buf, "\n[GNUPG:] SIG_CREATED ");
+	if (ret)
+		return error(_("gpgsm failed to sign the data"));
+
+	/* Mark the signature as good. */
+	psig->result = 'G';
+
+	/* Strip CR from the line endings, in case we are on Windows. */
+	for (i = j = 0; i < psig->sig.len; i++)
+		if (psig->sig.buf[i] != '\r') {
+			if (i != j)
+				psig->sig.buf[j] = psig->sig.buf[i];
+			j++;
+		}
+	strbuf_setlen(&(psig->sig), j);
+
+	/* Store the key we used */
+	psig->key = xstrdup(skey);
+
+	return 0;
+}
+
+static size_t x509_parse(const char *payload, size_t size,
+		struct signature **sig)
+{
+	int ret;
+	regex_t rbegin;
+	regex_t rend;
+	regmatch_t bmatch;
+	regmatch_t ematch;
+	size_t begin, end;
+	struct signature *psig;
+	static char errbuf[1024];
+
+	if (size == 0)
+		return size;
+
+	/*
+	 * Find the first x509 signature in the payload and copy it into the
+	 * signature struct.
+	 */
+	if ((ret = regcomp(&rbegin, pattern.begin, REG_EXTENDED|REG_NEWLINE))) {
+		regerror(ret, &rbegin, errbuf, 1024);
+		BUG("Failed to compile regex: %s\n", errbuf);
+
+		return size;
+	}
+	if ((ret = regcomp(&rend, pattern.end, REG_EXTENDED|REG_NEWLINE))) {
+		regerror(ret, &rend, errbuf, 1024);
+		BUG("Failed to compile regex: %s\n", errbuf);
+
+		return size;
+	}
+
+	begin = end = 0;
+	if (regexec(&rbegin, payload, 1, &bmatch, 0) ||
+		regexec(&rend, payload, 1, &ematch, 0)) {
+		begin = size;
+	}
+	if (begin == size)
+		goto next;
+
+	begin = bmatch.rm_so;
+	end = ematch.rm_eo;
+
+	/*
+	 * Create the signature.
+	 */
+	if (sig) {
+		psig = *sig;
+		psig = xmalloc(sizeof(struct signature));
+		strbuf_init(&(psig->sig), end - begin);
+		strbuf_add(&(psig->sig), payload + begin, end - begin);
+		strbuf_init(&(psig->output), 0);
+		strbuf_init(&(psig->status), 0);
+		psig->st = X509_SIGNATURE;
+		psig->result = 0;
+		psig->signer = NULL;
+		psig->key = NULL;
+	}
+
+	next:
+		regfree(&rbegin);
+		regfree(&rend);
+
+	return begin;
+}
+
+/* An exclusive status -- only one of them can appear in output */
+#define GPG_STATUS_EXCLUSIVE	(1<<0)
+/* The status includes key identifier */
+#define GPG_STATUS_KEYID	(1<<1)
+/* The status includes user identifier */
+#define GPG_STATUS_UID		(1<<2)
+/* The status includes key fingerprints */
+#define GPG_STATUS_FINGERPRINT	(1<<3)
+
+/* Short-hand for standard exclusive *SIG status with keyid & UID */
+#define GPG_STATUS_STDSIG	(GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID|GPG_STATUS_UID)
+
+static struct {
+	char result;
+	const char *check;
+	unsigned int flags;
+} sigcheck_gpg_status[] = {
+	{ 'G', "GOODSIG ", GPG_STATUS_STDSIG },
+	{ 'B', "BADSIG ", GPG_STATUS_STDSIG },
+	{ 'U', "TRUST_NEVER", 0 },
+	{ 'U', "TRUST_UNDEFINED", 0 },
+	{ 'E', "ERRSIG ", GPG_STATUS_EXCLUSIVE|GPG_STATUS_KEYID },
+	{ 'X', "EXPSIG ", GPG_STATUS_STDSIG },
+	{ 'Y', "EXPKEYSIG ", GPG_STATUS_STDSIG },
+	{ 'R', "REVKEYSIG ", GPG_STATUS_STDSIG },
+	{ 0, "VALIDSIG ", GPG_STATUS_FINGERPRINT },
+};
+
+static void parse_output(struct signature *sigc)
+{
+	const char *buf = sigc->status.buf;
+	const char *line, *next;
+	int i, j;
+	int seen_exclusive_status = 0;
+
+	/* Iterate over all lines */
+	for (line = buf; *line; line = strchrnul(line+1, '\n')) {
+		while (*line == '\n')
+			line++;
+		/* Skip lines that don't start with GNUPG status */
+		if (!skip_prefix(line, "[GNUPG:] ", &line))
+			continue;
+
+		/* Iterate over all search strings */
+		for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
+			if (skip_prefix(line, sigcheck_gpg_status[i].check, &line)) {
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_EXCLUSIVE) {
+					if (seen_exclusive_status++)
+						goto found_duplicate_status;
+				}
+
+				if (sigcheck_gpg_status[i].result)
+					sigc->result = sigcheck_gpg_status[i].result;
+				/* Do we have key information? */
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_KEYID) {
+					next = strchrnul(line, ' ');
+					free(sigc->key);
+					sigc->key = xmemdupz(line, next - line);
+					/* Do we have signer information? */
+					if (*next && (sigcheck_gpg_status[i].flags & GPG_STATUS_UID)) {
+						line = next + 1;
+						next = strchrnul(line, '\n');
+						free(sigc->signer);
+						sigc->signer = xmemdupz(line, next - line);
+					}
+				}
+				/* Do we have fingerprint? */
+				if (sigcheck_gpg_status[i].flags & GPG_STATUS_FINGERPRINT) {
+					next = strchrnul(line, ' ');
+					free(sigc->fingerprint);
+					sigc->fingerprint = xmemdupz(line, next - line);
+
+					/* Skip interim fields */
+					for (j = 9; j > 0; j--) {
+						if (!*next)
+							break;
+						line = next + 1;
+						next = strchrnul(line, ' ');
+					}
+
+					next = strchrnul(line, '\n');
+					free(sigc->primary_key_fingerprint);
+					sigc->primary_key_fingerprint = xmemdupz(line, next - line);
+				}
+
+				break;
+			}
+		}
+	}
+	return;
+
+found_duplicate_status:
+	/*
+	 * GOODSIG, BADSIG etc. can occur only once for each signature.
+	 * Therefore, if we had more than one then we're dealing with multiple
+	 * signatures.  We don't support them currently, and they're rather
+	 * hard to create, so something is likely fishy and we should reject
+	 * them altogether.
+	 */
+	sigc->result = 'E';
+	/* Clear partial data to avoid confusion */
+	FREE_AND_NULL(sigc->primary_key_fingerprint);
+	FREE_AND_NULL(sigc->fingerprint);
+	FREE_AND_NULL(sigc->signer);
+	FREE_AND_NULL(sigc->key);
+}
+
+static int x509_verify(const char *payload, size_t size,
+		struct signature *sig)
+{
+	struct child_process gpgsm = CHILD_PROCESS_INIT;
+	struct tempfile *temp;
+	int ret;
+
+	temp = mks_tempfile_t(".git_vtag_tmpXXXXXX");
+	if (!temp)
+		return error_errno(_("could not create temporary file"));
+	if (write_in_full(temp->fd, sig->sig.buf, sig->sig.len) < 0 ||
+	    close_tempfile_gently(temp) < 0) {
+		error_errno(_("failed writing detached signature to '%s'"),
+				temp->filename.buf);
+		delete_tempfile(&temp);
+		return -1;
+	}
+
+	argv_array_push(&gpgsm.args, program);
+	argv_array_pushl(&gpgsm.args,
+			"--status-fd=1",
+			"--verify", temp->filename.buf, "-",
+			NULL);
+
+	strbuf_reset(&(sig->status));
+	strbuf_reset(&(sig->output));
+
+	sigchain_push(SIGPIPE, SIG_IGN);
+	ret = pipe_command(&gpgsm, payload, size,
+			&(sig->status), 0, &(sig->output), 0);
+	sigchain_pop(SIGPIPE);
+
+	delete_tempfile(&temp);
+
+	ret |= !strstr(sig->status.buf, "\n[GNUPG:] GOODSIG ");
+
+	if (ret && !sig->output.len)
+		return !!ret;
+
+	parse_output(sig);
+
+	ret |= sig->result != 'G' && sig->result != 'U';
+
+	return !!ret;
+}
+
+static void x509_print(const struct signature *sig, unsigned flags)
+{
+	const char *output = flags & OUTPUT_RAW ?
+		sig->status.buf : sig->output.buf;
+
+	if (flags & OUTPUT_VERBOSE && sig->sig.buf)
+		fputs(sig->sig.buf, stdout);
+
+	if (output)
+		fputs(output, stderr);
+}
+
+static int x509_config(const char *var, const char *value, void *cb)
+{
+	if (!strcmp(var, "program"))
+		return git_config_string(&program, var, value);
+
+	if (!strcmp(var, "key"))
+		return git_config_string(&signing_key, var, value);
+
+	return 0;
+}
+
+static void x509_set_key(const char *key)
+{
+	free((void*)signing_key);
+	signing_key = xstrdup(key);
+}
+
+static const char *x509_get_key(void)
+{
+	if (signing_key)
+		return signing_key;
+	return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
+}
+
+static void x509_set_program(const char *signing_program)
+{
+	free((void*)program);
+	program = xstrdup(signing_program);
+}
+
+static const char *x509_get_program(void)
+{
+	if (program)
+		return program;
+	return git_committer_info(IDENT_STRICT|IDENT_NO_DATE);
+}
\ No newline at end of file
diff --git a/signing-tool.h b/signing-tool.h
new file mode 100644
index 000000000..ee7ccc7a5
--- /dev/null
+++ b/signing-tool.h
@@ -0,0 +1,35 @@
+#ifndef SIGNING_TOOL_H
+#define SIGNING_TOOL_H
+
+struct strbuf;
+struct signature;
+
+typedef int (*sign_fn)(const char *payload, size_t size,
+	struct signature **sig, const char *key);
+typedef size_t (*parse_fn)(const char *payload, size_t size,
+	struct signature **sig);
+typedef int (*verify_fn)(const char *payload, size_t size,
+	struct signature *sig);
+typedef void (*print_fn)(const struct signature *sig, unsigned flags);
+typedef int (*config_fn)(const char *var, const char *value, void *cb);
+typedef void (*set_key_fn)(const char *key);
+typedef const char *(*get_key_fn)(void);
+typedef void (*set_program_fn)(const char *signing_program);
+typedef const char *(*get_program_fn)(void);
+
+struct signing_tool {
+	const enum signature_type st;
+	const char* name;
+	sign_fn sign;
+	parse_fn parse;
+	verify_fn verify;
+	print_fn print;
+	config_fn config;
+	set_key_fn set_key;
+	get_key_fn get_key;
+	set_program_fn set_program;
+	get_program_fn get_program;
+};
+
+#endif
+
-- 
2.11.0

Re: [PATCH 2/5] Signing API: Added new signing interface API

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

On 2019-08-26 at 19:58:00, Ibrahim El wrote:
> From: Ibrahim El Rhezzali <ibrahim.el@pm.me>
> 
> 7e3e6c9e4 Added new signing interface API
> 
> Adding files for the new signing interface and also support drivers for the two existing GPG and GPGSM X.509 tools

I'd like to see an explanation here why a new signing interface is
necessary and we need to make a wholesale replacement of the existing
one instead of making incremental changes.

> diff --git a/signing-interface.c b/signing-interface.c
> new file mode 100644
> index 000000000..c744ef499
> --- /dev/null
> +++ b/signing-interface.c
> @@ -0,0 +1,487 @@
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include "cache.h"
> +#include "config.h"
> +#include "run-command.h"
> +#include "strbuf.h"
> +#include "signing-interface.h"
> +#include "signing-tool.h"
> +#include "sigchain.h"
> +#include "tempfile.h"
> +
> +extern const struct signing_tool openpgp_tool;
> +extern const struct signing_tool x509_tool;
> +
> +static const struct signing_tool *signing_tools[SIGNATURE_TYPE_COUNT] = {
> +	&openpgp_tool,
> +	&x509_tool,
> +};

It looks like we've hard-coded only two tools here.  I was under the
impression this series was supposed to make signing pluggable with any
tool, but that doesn't seem to be the case.

> +size_t parse_signatures(const char *payload, size_t size, 
> +		struct signatures *sigs)
> +{
> +	enum signature_type st;
> +	size_t first;
> +	size_t begin = 0;
> +	const struct signing_tool *tool;
> +	struct signature *psig = NULL;
> +
> +	first = size;
> +	for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++) {
> +		tool = signing_tools[st];
> +
> +		if (!tool || !tool->parse)
> +			BUG("signing tool %s undefined", signature_type_name(st));

If this is supposed to make parsing generic, won't we have to add
support for each individual tool in the codebase so tool->parse is
defined?  Having to do that would defeat the point of having a pluggable
interface set up in the configuration.

> +	buf = xstrdup(var);
> +	t1 = strtok(buf, ".");
> +	t2 = strtok(NULL, ".");
> +	t3 = strtok(NULL, ".");

I don't think we make a lot of use of strtok.  Perhaps you'd like to use
parse_config_key or another function in config.c?
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 2/5] Signing API: Added new signing interface API

[Ibrahim El]

Thx for your feedback. I will incorporate the detailed explanation and re-submit the patches.


Ibrahim El Rhezzali

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, August 26, 2019 11:04 PM, brian m. carlson <sandals@crustytoothpaste.net> wrote:

> On 2019-08-26 at 19:58:00, Ibrahim El wrote:
>
> > From: Ibrahim El Rhezzali ibrahim.el@pm.me
> > 7e3e6c9e4 Added new signing interface API
> > Adding files for the new signing interface and also support drivers for the two existing GPG and GPGSM X.509 tools
>
> I'd like to see an explanation here why a new signing interface is
> necessary and we need to make a wholesale replacement of the existing
> one instead of making incremental changes.
>
> > diff --git a/signing-interface.c b/signing-interface.c
> > new file mode 100644
> > index 000000000..c744ef499
> > --- /dev/null
> > +++ b/signing-interface.c
> > @@ -0,0 +1,487 @@
> > +#include <sys/types.h>
> > +#include <unistd.h>
> > +#include "cache.h"
> > +#include "config.h"
> > +#include "run-command.h"
> > +#include "strbuf.h"
> > +#include "signing-interface.h"
> > +#include "signing-tool.h"
> > +#include "sigchain.h"
> > +#include "tempfile.h"
> > +
> > +extern const struct signing_tool openpgp_tool;
> > +extern const struct signing_tool x509_tool;
> > +
> > +static const struct signing_tool *signing_tools[SIGNATURE_TYPE_COUNT] = {
> >
> > -   &openpgp_tool,
> > -   &x509_tool,
> >     +};
> >
>
> It looks like we've hard-coded only two tools here. I was under the
> impression this series was supposed to make signing pluggable with any
> tool, but that doesn't seem to be the case.
>
> > +size_t parse_signatures(const char *payload, size_t size,
> >
> > -       struct signatures *sigs)
> >
> >
> >
> > +{
> >
> > -   enum signature_type st;
> > -   size_t first;
> > -   size_t begin = 0;
> > -   const struct signing_tool *tool;
> > -   struct signature *psig = NULL;
> > -
> > -   first = size;
> > -   for (st = SIGNATURE_TYPE_FIRST; st < SIGNATURE_TYPE_LAST; st++) {
> > -       tool = signing_tools[st];
> >
> >
> > -
> > -       if (!tool || !tool->parse)
> >
> >
> > -       	BUG("signing tool %s undefined", signature_type_name(st));
> >
> >
>
> If this is supposed to make parsing generic, won't we have to add
> support for each individual tool in the codebase so tool->parse is
> defined? Having to do that would defeat the point of having a pluggable
> interface set up in the configuration.
>
> > -   buf = xstrdup(var);
> > -   t1 = strtok(buf, ".");
> > -   t2 = strtok(NULL, ".");
> > -   t3 = strtok(NULL, ".");
>
> I don't think we make a lot of use of strtok. Perhaps you'd like to use
> parse_config_key or another function in config.c?
>
> ----------------------------------------------------------------------------------------------------------------------------
>
> brian m. carlson: Houston, Texas, US
> OpenPGP: https://keybase.io/bk2204