X509 (gpgsm) commit signing support

PATCH In review Henning Schild 3 participants 24 messages Jul 17, 2018 – Jul 20, 2018 Documentation

lore

Share & tools

lore

Henning Schild

author

2018-07-17 12:50 UTC #0 lore raw

v4 cover X509 (gpgsm) commit signing support

Changes in v4:
- make gpg.format matching case sensitive
- (final?) coding style and wording changes

Changes in v3:
- drop patches 1 and 2 known from < v3, see pu hs/push-cert-check-cleanup
- dropped some testcases from p6, added two t7004 bad key/sig
- ship a binary x509 certificate for tests, no generation anymore
- silence gpgsm in tests
- switch all tests to use test_config instead of "git config"
- p4 deal with invalid input
- p3 several refactorings, see "PATCH v2 5/9" discussion

Changes in v2:
- removed trailing commas in array initializers and add leading space
- replaced assert(0) with BUG in p5
- consolidated 2 format lookups reusing get_format_data p5
- changed from format "PGP" to "openpgp", later X509 to "x509"
- use strcasecmp instead of strcmp for format matching
- introduce gpg.<format>.program in p8, no gpg.programX509 anymore
- moved t/7510 patch inbetween the two patches changing validation code
- changed t/7510 patch to use test_config and test_must_fail

This series adds support for signing commits with gpgsm.

The first 5 patches prepare for the introduction of the actual feature in patch 6.
Finally patch 7 extends the testsuite to cover the new feature.

This series can be seen as a follow up of a series that appeared under the name "gpg-interface: Multiple signing tools" in april 2018 [1]. After that series was not merged i decided to get my patches ready. The original series aimed at being generic for any sort of signing tool, while this series just introduced the X509 variant of gpg. (gpgsm)
I collected authors and reviewers of that first series and already put them on cc.

[1] https://public-inbox.org/git/[email protected]/

Henning Schild (7):
gpg-interface: add new config to select how to sign a commit t/t7510: check the validation of the new config gpg.format
gpg-interface: introduce an abstraction for multiple gpg formats
gpg-interface: do not hardcode the key string len anymore
gpg-interface: introduce new config to select per gpg format program
gpg-interface: introduce new signature format "x509" using gpgsm gpg-interface t: extend the existing GPG tests with GPGSM

 Documentation/config.txt   |  10 +++++
 gpg-interface.c            | 108 +++++++++++++++++++++++++++++++++++++--------
 t/lib-gpg.sh               |  28 +++++++++++-
 t/lib-gpg/gpgsm-gen-key.in |   8 ++++
 t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes
 t/t4202-log.sh             |  39 ++++++++++++++++
 t/t5534-push-signed.sh     |  52 ++++++++++++++++++++++
 t/t7004-tag.sh             |  13 ++++++
 t/t7030-verify-tag.sh      |  33 ++++++++++++++
 t/t7510-signed-commit.sh   |   9 ++++
 10 files changed, 281 insertions(+), 19 deletions(-)
 create mode 100644 t/lib-gpg/gpgsm-gen-key.in
 create mode 100644 t/lib-gpg/gpgsm_cert.p12

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-17 12:50 UTC #1 lore raw

PATCH v4 1/7 gpg-interface: add new config to select how to sign a commit

Add "gpg.format" where the user can specify which type of signature to use for commits. At the moment only "openpgp" is supported and the value is not even used. This commit prepares for a new types of signatures.

Signed-off-by Henning Schild <[email protected]>

---
 Documentation/config.txt |  4 ++++
 gpg-interface.c          | 10 ++++++++++
 2 files changed, 14 insertions(+)

Documentation/config.txt +4 -0

index 1cc18a828..ac373e3f4 100644
@@ -1828,6 +1828,10 @@ gpg.program::
 	signed, and the program is expected to send the result to its
 	standard output.
 
+gpg.format::
+	Specifies which key format to use when signing with `--gpg-sign`.
+	Default is "openpgp", that is also the only supported value.
+
 gui.commitMsgWidth::
 	Defines how wide the commit message window is in the
 	linkgit:git-gui[1]. "75" is the default.

gpg-interface.c +10 -0

index 09ddfbc26..b39a27980 100644
@@ -7,6 +7,7 @@
 #include "tempfile.h"
 
 static char *configured_signing_key;
+static const char *gpg_format = "openpgp";
 static const char *gpg_program = "gpg";
 
 #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"

@@ -138,6 +139,15 @@ int git_gpg_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "gpg.format")) {
+		if (!value)
+			return config_error_nonbool(var);
+		if (strcmp(value, "openpgp"))
+			return error("unsupported value for %s: %s",
+				     var, value);
+		return git_config_string(&gpg_format, var, value);
+	}
+
 	if (!strcmp(var, "gpg.program")) {
 		if (!value)
 			return config_error_nonbool(var);

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-17 12:50 UTC #2 lore raw

PATCH v4 2/7 t/t7510: check the validation of the new config gpg.format

In reply to Henning Schild

Test setting gpg.format to both invalid and valid values.

Signed-off-by Henning Schild <[email protected]>

---
 t/t7510-signed-commit.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

t/t7510-signed-commit.sh +9 -0

index 6e2015ed9..7bdad570c 100755
@@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' '
 	grep "gpg: Good signature" actual
 '
 
+test_expect_success GPG 'check config gpg.format values' '
+	test_config gpg.format openpgp &&
+	git commit -S --amend -m "success" &&
+	test_config gpg.format OpEnPgP &&
+	test_must_fail git commit -S --amend -m "fail" &&
+	test_config gpg.format malformed &&
+	test_must_fail git commit -S --amend -m "fail"
+'
+
 test_done

Signature · 1 line

2.16.4

Junio C Hamano

2018-07-17 21:32 UTC #3 lore raw

Re: [PATCH v4 2/7] t/t7510: check the validation of the new config gpg.format

Quoted 12 lines · Henning Schild

Test setting gpg.format to both invalid and valid values.
Signed-off-by Henning Schild <[email protected]>
---
 t/t7510-signed-commit.sh | 9 +++++++++
 1 file changed, 9 insertions(+)
t/t7510-signed-commit.sh +0 -0
index 6e2015ed9..7bdad570c 100755

t/t7510-signed-commit.sh test_expect_success GPG 'log.showsignature behaves like --show-signature' ' · line 234

Quoted 8 lines

t/t7510-signed-commit.sh +5 -0

@@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' '
 	grep "gpg: Good signature" actual
 '
 
+test_expect_success GPG 'check config gpg.format values' '
+	test_config gpg.format openpgp &&
+	git commit -S --amend -m "success" &&
+	test_config gpg.format OpEnPgP &&
+	test_must_fail git commit -S --amend -m "fail" &&

This second one is a good demonstration that the value for this variable is case sensitive.

t/t7510-signed-commit.sh test_expect_success GPG 'log.showsignature behaves like --show-signature' ' · line 236

Quoted 2 lines

t/t7510-signed-commit.sh +2 -0

+	test_config gpg.format malformed &&
+	test_must_fail git commit -S --amend -m "fail"

And there is no longer a good reason to try another one. Let's drop this last/third case.

Quoted 3 lines

t/t7510-signed-commit.sh +2 -0
+'
+
 test_done

Henning Schild

author

2018-07-18 10:36 UTC #4 lore raw

Re: [PATCH v4 2/7] t/t7510: check the validation of the new config gpg.format

Am Tue, 17 Jul 2018 14:31:56 -0700
schrieb Junio C Hamano <[email protected]>:

Quoted 26 lines

Quoted 15 lines · Henning Schild
Test setting gpg.format to both invalid and valid values.
Signed-off-by Henning Schild <[email protected]>
---
 t/t7510-signed-commit.sh | 9 +++++++++
 1 file changed, 9 insertions(+)
t/t7510-signed-commit.sh +0 -0
index 6e2015ed9..7bdad570c 100755
@@ -227,4 +227,13 @@ test_expect_success GPG 'log.showsignature
behaves like --show-signature' ' grep "gpg: Good signature" actual
'
t/t7510-signed-commit.sh test_expect_success GPG 'log.showsignature behaves like --show-signature' ' · line 234
Quoted 5 lines
t/t7510-signed-commit.sh +5 -0
+test_expect_success GPG 'check config gpg.format values' '
+	test_config gpg.format openpgp &&
+	git commit -S --amend -m "success" &&
+	test_config gpg.format OpEnPgP &&
+	test_must_fail git commit -S --amend -m "fail" &&  
This second one is a good demonstration that the value for this variable is case sensitive.

Quoted 5 lines

t/t7510-signed-commit.sh test_expect_success GPG 'log.showsignature behaves like --show-signature' ' · line 236
Quoted 2 lines
t/t7510-signed-commit.sh +2 -0
+	test_config gpg.format malformed &&
+	test_must_fail git commit -S --amend -m "fail"  
And there is no longer a good reason to try another one. Let's drop this last/third case.

Done.

Henning

Quoted 3 lines

Quoted 3 lines
+'
+
test_done

Henning Schild

author

2018-07-18 09:30 UTC #5 lore raw

PATCH v5 2/7 t/t7510: check the validation of the new config gpg.format

In reply to Henning Schild

Test setting gpg.format to both invalid and valid values.

Signed-off-by Henning Schild <[email protected]>

---
 t/t7510-signed-commit.sh | 7 +++++++
 1 file changed, 7 insertions(+)

t/t7510-signed-commit.sh +7 -0

index 6e2015ed9..4e37ff8f1 100755
@@ -227,4 +227,11 @@ test_expect_success GPG 'log.showsignature behaves like --show-signature' '
 	grep "gpg: Good signature" actual
 '
 
+test_expect_success GPG 'check config gpg.format values' '
+	test_config gpg.format openpgp &&
+	git commit -S --amend -m "success" &&
+	test_config gpg.format OpEnPgP &&
+	test_must_fail git commit -S --amend -m "fail"
+'
+
 test_done

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-17 12:50 UTC #6 lore raw

PATCH v4 3/7 gpg-interface: introduce an abstraction for multiple gpg formats

In reply to Henning Schild

Create a struct that holds the format details for the supported formats. At the moment that is still just "openpgp". This commit prepares for the introduction of more formats, that might use other programs and match other signatures.

Signed-off-by Henning Schild <[email protected]>

---
 gpg-interface.c | 88 +++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 67 insertions(+), 21 deletions(-)

gpg-interface.c +67 ~~-21~~

index b39a27980..a02db7658 100644
@@ -7,11 +7,52 @@
 #include "tempfile.h"
 
 static char *configured_signing_key;
-static const char *gpg_format = "openpgp";
-static const char *gpg_program = "gpg";
+struct gpg_format {
+	const char *name;
+	const char *program;
+	const char **verify_args;
+	const char **sigs;
+};
+
+static const char *openpgp_verify_args[] = {
+	"--keyid-format=long",
+	NULL
+};
+static const char *openpgp_sigs[] = {
+	"-----BEGIN PGP SIGNATURE-----",
+	"-----BEGIN PGP MESSAGE-----",
+	NULL
+};
+
+static struct gpg_format gpg_format[] = {
+	{ .name = "openpgp", .program = "gpg",
+	  .verify_args = openpgp_verify_args,
+	  .sigs = openpgp_sigs
+	},
+};
+
+static struct gpg_format *use_format = &gpg_format[0];
 
-#define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
-#define PGP_MESSAGE "-----BEGIN PGP MESSAGE-----"
+static struct gpg_format *get_format_by_name(const char *str)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(gpg_format); i++)
+		if (!strcmp(gpg_format[i].name, str))
+			return gpg_format + i;
+	return NULL;
+}
+
+static struct gpg_format *get_format_by_sig(const char *sig)
+{
+	int i, j;
+
+	for (i = 0; i < ARRAY_SIZE(gpg_format); i++)
+		for (j = 0; gpg_format[i].sigs[j]; j++)
+			if (starts_with(sig, gpg_format[i].sigs[j]))
+				return gpg_format + i;
+	return NULL;
+}
 
 void signature_check_clear(struct signature_check *sigc)
 {

@@ -102,12 +143,6 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)
 		fputs(output, stderr);
 }
 
-static int is_gpg_start(const char *line)
-{
-	return starts_with(line, PGP_SIGNATURE) ||
-		starts_with(line, PGP_MESSAGE);
-}
-
 size_t parse_signature(const char *buf, size_t size)
 {
 	size_t len = 0;

@@ -115,7 +150,7 @@ size_t parse_signature(const char *buf, size_t size)
 	while (len < size) {
 		const char *eol;
 
-		if (is_gpg_start(buf + len))
+		if (get_format_by_sig(buf + len))
 			match = len;
 
 		eol = memchr(buf + len, '\n', size - len);

@@ -132,6 +167,9 @@ void set_signing_key(const char *key)
 
 int git_gpg_config(const char *var, const char *value, void *cb)
 {
+	struct gpg_format *fmt = NULL;
+	char *fmtname = NULL;
+
 	if (!strcmp(var, "user.signingkey")) {
 		if (!value)
 			return config_error_nonbool(var);

@@ -142,17 +180,20 @@ int git_gpg_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "gpg.format")) {
 		if (!value)
 			return config_error_nonbool(var);
-		if (strcmp(value, "openpgp"))
+		fmt = get_format_by_name(value);
+		if (!fmt)
 			return error("unsupported value for %s: %s",
 				     var, value);
-		return git_config_string(&gpg_format, var, value);
+		use_format = fmt;
+		return 0;
 	}
 
-	if (!strcmp(var, "gpg.program")) {
-		if (!value)
-			return config_error_nonbool(var);
-		gpg_program = xstrdup(value);
-		return 0;
+	if (!strcmp(var, "gpg.program"))
+		fmtname = "openpgp";
+
+	if (fmtname) {
+		fmt = get_format_by_name(fmtname);
+		return git_config_string(&fmt->program, var, value);
 	}
 
 	return 0;

@@ -173,7 +214,7 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig
 	struct strbuf gpg_status = STRBUF_INIT;
 
 	argv_array_pushl(&gpg.args,
-			 gpg_program,
+			 use_format->program,
 			 "--status-fd=2",
 			 "-bsau", signing_key,
 			 NULL);

@@ -211,6 +252,7 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 			 struct strbuf *gpg_output, struct strbuf *gpg_status)
 {
 	struct child_process gpg = CHILD_PROCESS_INIT;
+	struct gpg_format *fmt;
 	struct tempfile *temp;
 	int ret;
 	struct strbuf buf = STRBUF_INIT;

@@ -226,10 +268,14 @@ int verify_signed_buffer(const char *payload, size_t payload_size,
 		return -1;
 	}
 
+	fmt = get_format_by_sig(signature);
+	if (!fmt)
+		BUG("bad signature '%s'", signature);
+
+	argv_array_push(&gpg.args, fmt->program);
+	argv_array_pushv(&gpg.args, fmt->verify_args);
 	argv_array_pushl(&gpg.args,
-			 gpg_program,
 			 "--status-fd=1",
-			 "--keyid-format=long",
 			 "--verify", temp->filename.buf, "-",
 			 NULL);

Signature · 1 line

2.16.4

Junio C Hamano

2018-07-17 20:56 UTC #7 lore raw

Re: [PATCH v4 3/7] gpg-interface: introduce an abstraction for multiple gpg formats

Quoted 7 lines · Henning Schild

Create a struct that holds the format details for the supported formats. At the moment that is still just "openpgp". This commit prepares for the introduction of more formats, that might use other programs and match other signatures.
Signed-off-by Henning Schild <[email protected]>
---

I didn't spot anything questionable in this version. Looking good.

Henning Schild

author

2018-07-17 12:50 UTC #8 lore raw

PATCH v4 4/7 gpg-interface: do not hardcode the key string len anymore

In reply to Henning Schild

gnupg does print the keyid followed by a space and the signer comes next. The same pattern is also used in gpgsm, but there the key length would be 40 instead of 16. Instead of hardcoding the expected length, find the first space and calculate it.
Input that does not match the expected format will be ignored now, before we jumped to found+17 which might have been behind the end of an unexpected string.

Signed-off-by Henning Schild <[email protected]>

---
 gpg-interface.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

gpg-interface.c +4 -3

index a02db7658..51cad9081 100644
@@ -95,10 +95,11 @@ static void parse_gpg_output(struct signature_check *sigc)
 		sigc->result = sigcheck_gpg_status[i].result;
 		/* The trust messages are not followed by key/signer information */
 		if (sigc->result != 'U') {
-			sigc->key = xmemdupz(found, 16);
+			next = strchrnul(found, ' ');
+			sigc->key = xmemdupz(found, next - found);
 			/* The ERRSIG message is not followed by signer information */
-			if (sigc-> result != 'E') {
-				found += 17;
+			if (*next && sigc-> result != 'E') {
+				found = next + 1;
 				next = strchrnul(found, '\n');
 				sigc->signer = xmemdupz(found, next - found);
 			}

Signature · 1 line

2.16.4

Junio C Hamano

2018-07-17 20:56 UTC #9 lore raw

Re: [PATCH v4 4/7] gpg-interface: do not hardcode the key string len anymore

Quoted 8 lines · Henning Schild

gnupg does print the keyid followed by a space and the signer comes next. The same pattern is also used in gpgsm, but there the key length would be 40 instead of 16. Instead of hardcoding the expected length, find the first space and calculate it.
Input that does not match the expected format will be ignored now, before we jumped to found+17 which might have been behind the end of an unexpected string.

Quoted 4 lines

Signed-off-by Henning Schild <[email protected]>
---
 gpg-interface.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Again, really nice.

Quoted 20 lines

gpg-interface.c +4 -3

index a02db7658..51cad9081 100644
@@ -95,10 +95,11 @@ static void parse_gpg_output(struct signature_check *sigc)
 		sigc->result = sigcheck_gpg_status[i].result;
 		/* The trust messages are not followed by key/signer information */
 		if (sigc->result != 'U') {
-			sigc->key = xmemdupz(found, 16);
+			next = strchrnul(found, ' ');
+			sigc->key = xmemdupz(found, next - found);
 			/* The ERRSIG message is not followed by signer information */
-			if (sigc-> result != 'E') {
-				found += 17;
+			if (*next && sigc-> result != 'E') {
+				found = next + 1;
 				next = strchrnul(found, '\n');
 				sigc->signer = xmemdupz(found, next - found);
 			}

Henning Schild

author

2018-07-17 12:50 UTC #10 lore raw

PATCH v4 5/7 gpg-interface: introduce new config to select per gpg format program

In reply to Henning Schild

Supporting multiple signing formats we will have the need to configure a custom program each. Add a new config value to cater for that.

Signed-off-by Henning Schild <[email protected]>

---
 Documentation/config.txt | 5 +++++
 gpg-interface.c          | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

Documentation/config.txt +5 -0

index ac373e3f4..0e871346a 100644
@@ -1832,6 +1832,11 @@ gpg.format::
 	Specifies which key format to use when signing with `--gpg-sign`.
 	Default is "openpgp", that is also the only supported value.
 
+gpg.<format>.program::
+	Use this to customize the program used for the signing format you
+	chose. (see `gpg.program` and `gpg.format`) `gpg.program` can still
+	be used as a legacy synonym for	`gpg.openpgp.program`.
+
 gui.commitMsgWidth::
 	Defines how wide the commit message window is in the
 	linkgit:git-gui[1]. "75" is the default.

gpg-interface.c +1 -1

index 51cad9081..a158f08c1 100644
@@ -189,7 +189,7 @@ int git_gpg_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(var, "gpg.program"))
+	if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program"))
 		fmtname = "openpgp";
 
 	if (fmtname) {

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-17 12:50 UTC #11 lore raw

PATCH v4 6/7 gpg-interface: introduce new signature format "x509" using gpgsm

In reply to Henning Schild

This commit allows git to create and check x509 type signatures using gpgsm.

Signed-off-by Henning Schild <[email protected]>

---
 Documentation/config.txt |  5 +++--
 gpg-interface.c          | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 2 deletions(-)

Documentation/config.txt +3 -2

index 0e871346a..ff1d4a76c 100644
@@ -1830,12 +1830,13 @@ gpg.program::
 
 gpg.format::
 	Specifies which key format to use when signing with `--gpg-sign`.
-	Default is "openpgp", that is also the only supported value.
+	Default is "openpgp" and another possible value is "x509".
 
 gpg.<format>.program::
 	Use this to customize the program used for the signing format you
 	chose. (see `gpg.program` and `gpg.format`) `gpg.program` can still
-	be used as a legacy synonym for	`gpg.openpgp.program`.
+	be used as a legacy synonym for	`gpg.openpgp.program`. The default
+	value for `gpg.x509.program` is "gpgsm".
 
 gui.commitMsgWidth::
 	Defines how wide the commit message window is in the

gpg-interface.c +15 -0

index a158f08c1..bb8ea668b 100644
@@ -24,11 +24,23 @@ static const char *openpgp_sigs[] = {
 	NULL
 };
 
+static const char *x509_verify_args[] = {
+	NULL
+};
+static const char *x509_sigs[] = {
+	"-----BEGIN SIGNED MESSAGE-----",
+	NULL
+};
+
 static struct gpg_format gpg_format[] = {
 	{ .name = "openpgp", .program = "gpg",
 	  .verify_args = openpgp_verify_args,
 	  .sigs = openpgp_sigs
 	},
+	{ .name = "x509", .program = "gpgsm",
+	  .verify_args = x509_verify_args,
+	  .sigs = x509_sigs
+	},
 };
 
 static struct gpg_format *use_format = &gpg_format[0];

@@ -192,6 +204,9 @@ int git_gpg_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "gpg.program") || !strcmp(var, "gpg.openpgp.program"))
 		fmtname = "openpgp";
 
+	if (!strcmp(var, "gpg.x509.program"))
+		fmtname = "x509";
+
 	if (fmtname) {
 		fmt = get_format_by_name(fmtname);
 		return git_config_string(&fmt->program, var, value);

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-17 12:50 UTC #12 lore raw

PATCH v4 7/7 gpg-interface t: extend the existing GPG tests with GPGSM

In reply to Henning Schild

Add test cases to cover the new X509/gpgsm support. Most of them resemble existing ones. They just switch the format to x509 and set the signingkey when creating signatures. Validation of signatures does not need any configuration of git, it does need gpgsm to be configured to trust the key(-chain).
Several of the testcases build on top of existing gpg testcases. The commit ships a self-signed key for [email protected] and configures gpgsm to trust it.

Signed-off-by Henning Schild <[email protected]>

---
 t/lib-gpg.sh               |  28 +++++++++++++++++++++++-
 t/lib-gpg/gpgsm-gen-key.in |   8 +++++++
 t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes
 t/t4202-log.sh             |  39 ++++++++++++++++++++++++++++++++++
 t/t5534-push-signed.sh     |  52 +++++++++++++++++++++++++++++++++++++++++++++
 t/t7004-tag.sh             |  13 ++++++++++++
 t/t7030-verify-tag.sh      |  33 ++++++++++++++++++++++++++++
 7 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 t/lib-gpg/gpgsm-gen-key.in
 create mode 100644 t/lib-gpg/gpgsm_cert.p12

t/lib-gpg.sh +27 -1

index a5d3b2cba..3fe02876c 100755
@@ -38,7 +38,33 @@ then
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
 		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
 			--sign -u [email protected] &&
-		test_set_prereq GPG
+		test_set_prereq GPG &&
+		# Available key info:
+		# * see t/lib-gpg/gpgsm-gen-key.in
+		# To generate new certificate:
+		#  * no passphrase
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o /tmp/gpgsm.crt.user \
+		#		--generate-key \
+		#		--batch t/lib-gpg/gpgsm-gen-key.in
+		# To import certificate:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		--import /tmp/gpgsm.crt.user
+		# To export into a .p12 we can later import:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o t/lib-gpg/gpgsm_cert.p12 \
+		#		--export-secret-key-p12 "[email protected]"
+		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
+			--passphrase-fd 0 --pinentry-mode loopback \
+			--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+			| grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+			${GNUPGHOME}/trustlist.txt &&
+		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+		echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+			-u [email protected] -o /dev/null --sign - 2>&1 &&
+		test_set_prereq GPGSM
 		;;
 	esac
 fi

t/lib-gpg/gpgsm-gen-key.in new file +8 -0

new file mode 100644
index 000000000..a7fd87c06
@@ -0,0 +1,8 @@
+Key-Type: RSA
+Key-Length: 2048
+Key-Usage: sign
+Serial: random
+Name-DN: CN=C O Mitter, O=Example, SN=C O, GN=Mitter
+Name-Email: [email protected]
+Not-Before: 1970-01-01 00:00:00
+Not-After: 3000-01-01 00:00:00

t/lib-gpg/gpgsm_cert.p12 new file binary +0 -0

new file mode 100644
index 0000000000000000000000000000000000000000..94ffad0d31a3b6c4e849b29d960762e485ba7ea8
GIT binary patch

t/t4202-log.sh +39 -0

index 25b1f8cc7..f57781e39 100755
@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed branch' '
 	git commit -S -m signed_commit
 '
 
+test_expect_success GPGSM 'setup signed branch x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	git checkout -b signed-x509 master &&
+	echo foo >foo &&
+	git add foo &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git commit -S -m signed_commit
+'
+
 test_expect_success GPG 'log --graph --show-signature' '
 	git log --graph --show-signature -n1 signed >actual &&
 	grep "^| gpg: Signature made" actual &&
 	grep "^| gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature x509' '
+	git log --graph --show-signature -n1 signed-x509 >actual &&
+	grep "^| gpgsm: Signature made" actual &&
+	grep "^| gpgsm: Good signature" actual
+'
+
 test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	test_when_finished "git reset --hard && git checkout master" &&
 	git checkout -b plain master &&

@@ -1581,6 +1597,29 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git checkout -b plain-x509 master &&
+	echo aaa >bar &&
+	git add bar &&
+	git commit -m bar_commit &&
+	git checkout -b tagged-x509 master &&
+	echo bbb >baz &&
+	git add baz &&
+	git commit -m baz_commit &&
+	git tag -s -m signed_tag_msg signed_tag_x509 &&
+	git checkout plain-x509 &&
+	git merge --no-ff -m msg signed_tag_x509 &&
+	git log --graph --show-signature -n1 plain-x509 >actual &&
+	grep "^|\\\  merged tag" actual &&
+	grep "^| | gpgsm: Signature made" actual &&
+	grep "^| | gpgsm: Good signature" actual &&
+	git config --unset gpg.format &&
+	git config --unset user.signingkey
+'
+
 test_expect_success GPG '--no-show-signature overrides --show-signature' '
 	git log -1 --show-signature --no-show-signature signed >actual &&
 	! grep "^gpg:" actual

t/t5534-push-signed.sh +52 -0

56 lines not on this page · raw message

t/t7004-tag.sh +13 -0

index d7b319e91..2147938aa 100755
@@ -1354,6 +1354,19 @@ test_expect_success GPG \
 	'test_config gpg.program echo &&
 	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
+# try to sign with bad user.signingkey
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad key)' \
+	'test_config user.signingkey BobTheMouse &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
+
+# try to produce invalid signature
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad signature format)' \
+	'test_config gpg.x509.program echo &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
 # try to verify without gpg:

t/t7030-verify-tag.sh +33 -0

index 291a1e2b0..db6295d65 100755
@@ -41,6 +41,13 @@ test_expect_success GPG 'create signed tags' '
 	git tag -uB7227189 -m eighth eighth-signed-alt
 '
 
+test_expect_success GPGSM 'create signed tags x509 ' '
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo 9 >file && test_tick && git commit -a -m "nineth gpgsm-signed" &&
+	git tag -s -m nineth nineth-signed-x509
+'
+
 test_expect_success GPG 'verify and show signatures' '
 	(
 		for tag in initial second merge fourth-signed sixth-signed seventh-signed

@@ -72,6 +79,13 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPGSM 'verify and show signatures x509' '
+	git verify-tag nineth-signed-x509 2>actual &&
+	grep "Good signature from" actual &&
+	! grep "BAD signature from" actual &&
+	echo nineth-signed-x509 OK
+'
+
 test_expect_success GPG 'detect fudged signature' '
 	git cat-file tag seventh-signed >raw &&
 	sed -e "/^tag / s/seventh/7th forged/" raw >forged1 &&

@@ -112,6 +126,12 @@ test_expect_success GPG 'verify signatures with --raw' '
 	)
 '
 
+test_expect_success GPGSM 'verify signatures with --raw x509' '
+	git verify-tag --raw nineth-signed-x509 2>actual &&
+	grep "GOODSIG" actual &&
+	! grep "BADSIG" actual &&
+	echo nineth-signed-x509 OK
+'
 test_expect_success GPG 'verify multiple tags' '
 	tags="fourth-signed sixth-signed seventh-signed" &&
 	for i in $tags

@@ -125,6 +145,19 @@ test_expect_success GPG 'verify multiple tags' '
 	test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success GPGSM 'verify multiple tags x509' '
+	tags="seventh-signed nineth-signed-x509" &&
+	for i in $tags
+	do
+		git verify-tag -v --raw $i || return 1
+	done >expect.stdout 2>expect.stderr.1 &&
+	grep "^.GNUPG:." <expect.stderr.1 >expect.stderr &&
+	git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+	grep "^.GNUPG:." <actual.stderr.1 >actual.stderr &&
+	test_cmp expect.stdout actual.stdout &&
+	test_cmp expect.stderr actual.stderr
+'
+
 test_expect_success GPG 'verifying tag with --format' '
 	cat >expect <<-\EOF &&
 	tagname : fourth-signed

Signature · 1 line

2.16.4

Junio C Hamano

2018-07-17 21:31 UTC #13 lore raw

Re: [PATCH v4 7/7] gpg-interface t: extend the existing GPG tests with GPGSM

t/lib-gpg.sh then · line 41

Quoted 10 lines · Henning Schild

t/lib-gpg.sh +1 -1

index a5d3b2cba..3fe02876c 100755
@@ -38,7 +38,33 @@ then
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
 		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
 			--sign -u [email protected] &&
-		test_set_prereq GPG
+		test_set_prereq GPG &&

We do not mind making GPGSM dependent on GPG, hence this && is justified.

Quoted 16 lines

t/lib-gpg.sh +16 -0

+		# Available key info:
+		# * see t/lib-gpg/gpgsm-gen-key.in
+		# To generate new certificate:
+		#  * no passphrase
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o /tmp/gpgsm.crt.user \
+		#		--generate-key \
+		#		--batch t/lib-gpg/gpgsm-gen-key.in
+		# To import certificate:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		--import /tmp/gpgsm.crt.user
+		# To export into a .p12 we can later import:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o t/lib-gpg/gpgsm_cert.p12 \
+		#		--export-secret-key-p12 "[email protected]"
+		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \

t/lib-gpg.sh then · line 67

Quoted 10 lines

t/lib-gpg.sh +10 -0

+			--passphrase-fd 0 --pinentry-mode loopback \
+			--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+			| grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+			${GNUPGHOME}/trustlist.txt &&
+		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+		echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+			-u [email protected] -o /dev/null --sign - 2>&1 &&
+		test_set_prereq GPGSM

And when any of the above fails, we refrain from setting GPGSM prereq. Otherwise we are prepared to perform tests with gpgsm and get the prereq.

Quoted 7 lines

t/t4202-log.sh +0 -0

index 25b1f8cc7..f57781e39 100755
@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed branch' '
 	git commit -S -m signed_commit
 '

t/t4202-log.sh test_expect_success GPG 'setup signed branch' ' · line 1567

Quoted 10 lines

t/t4202-log.sh +9 -0

 
+test_expect_success GPGSM 'setup signed branch x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	git checkout -b signed-x509 master &&
+	echo foo >foo &&
+	git add foo &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git commit -S -m signed_commit
+'

OK.

t/t4202-log.sh test_expect_success GPG 'setup signed branch' ' · line 1579

Quoted 5 lines

t/t4202-log.sh +5 -0

+test_expect_success GPGSM 'log --graph --show-signature x509' '
+	git log --graph --show-signature -n1 signed-x509 >actual &&
+	grep "^| gpgsm: Signature made" actual &&
+	grep "^| gpgsm: Good signature" actual
+'

OK.

Quoted 15 lines

t/t4202-log.sh +11 -0

@@ -1581,6 +1597,29 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git checkout -b plain-x509 master &&
+	echo aaa >bar &&
+	git add bar &&
+	git commit -m bar_commit &&
+	git checkout -b tagged-x509 master &&
+	echo bbb >baz &&
+	git add baz &&

t/t4202-log.sh test_expect_success GPG 'log --graph --show-signature for merged tag' ' · line 1620

Quoted 10 lines

t/t4202-log.sh +10 -0

+	git commit -m baz_commit &&
+	git tag -s -m signed_tag_msg signed_tag_x509 &&
+	git checkout plain-x509 &&
+	git merge --no-ff -m msg signed_tag_x509 &&
+	git log --graph --show-signature -n1 plain-x509 >actual &&
+	grep "^|\\\  merged tag" actual &&
+	grep "^| | gpgsm: Signature made" actual &&
+	grep "^| | gpgsm: Good signature" actual &&
+	git config --unset gpg.format &&
+	git config --unset user.signingkey

You are using test_config early enough in this test; doesn't that take care of the last two steps for you, even when an earlier step failed? If that is the case, then remove the last two line (and && at the end of the line before).

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 223

Quoted 11 lines

t/t5534-push-signed.sh +3 -0

index 1cea758f7..a3a12bd05 100755
@@ -218,4 +218,56 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '
 	test_cmp expect dst/push-cert-status
 '
 
+test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
+	test_config gpg.format x509 &&
+	env | grep GIT > envfile &&

The "envfile" is unused, no? Remove this line.

Quoted 11 lines

t/t5534-push-signed.sh +11 -0

+	prepare_dst &&
+	mkdir -p dst/.git/hooks &&
+	git -C dst config receive.certnonceseed sekrit &&
+	write_script dst/.git/hooks/post-receive <<-\EOF &&
+	# discard the update list
+	cat >/dev/null
+	# record the push certificate
+	if test -n "${GIT_PUSH_CERT-}"
+	then
+		git cat-file blob $GIT_PUSH_CERT >../push-cert
+	fi &&

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 244

Quoted 10 lines

t/t5534-push-signed.sh +10 -0

+
+	cat >../push-cert-status <<E_O_F
+	SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
+	KEY=${GIT_PUSH_CERT_KEY-nokey}
+	STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
+	NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
+	NONCE=${GIT_PUSH_CERT_NONCE-nononce}
+	E_O_F
+
+	EOF

OK, so up to this are what is done by post-receive, including the overwriting of ../push-cert (which is one level above the receiving repository's .git/, i.e. dst/push-cert) and ../push-cert-status.

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 248

Quoted 4 lines

t/t5534-push-signed.sh +4 -0

+	unset GIT_COMMITTER_EMAIL &&
+	git config user.email [email protected] &&
+	git config user.signingkey "" &&
+	test_must_fail git push --signed dst noop ff +noff &&

This is OK for a test that is known to be always at the end, but also forbids others to further update this script to add more tests at the end, as the standard setting of environment is blown away (the config is probably OK, but test_config to arrange them to be cleaned up would have been nicer), which is not very nice. I think it should be easy to fix it when it becomes necessary, but at the same time if it is easy to fix, then probably we shouldn't introduce a breakage in the first place, so I am on the fence.

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 250

Quoted 2 lines

t/t5534-push-signed.sh +2 -0

+	git config user.signingkey [email protected] &&
+	git push --signed dst noop ff +noff &&

So,... this is run without resetting user.email and demonstrates that signingkey is the only thing that matters, which makes sense.

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 262

Quoted 11 lines

t/t5534-push-signed.sh +11 -0

+	(
+		cat <<-\EOF &&
+		SIGNER=/CN=C O Mitter/O=Example/SN=C O/GN=Mitter
+		KEY=
+		STATUS=G
+		NONCE_STATUS=OK
+		EOF
+		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q" dst/push-cert
+	) >expect.in &&
+	key=$(cat "${GNUPGHOME}/trustlist.txt" | cut -d" " -f1 | tr -d ":") &&
+	sed -e "s/^KEY=/KEY=${key}/" expect.in > expect &&

s/> expect/>expect/;

Quoted 21 lines

t/t5534-push-signed.sh +9 -0

+	noop=$(git rev-parse noop) &&
+	ff=$(git rev-parse ff) &&
+	noff=$(git rev-parse noff) &&
+	grep "$noop $ff refs/heads/ff" dst/push-cert &&
+	grep "$noop $noff refs/heads/noff" dst/push-cert &&
+	test_cmp expect dst/push-cert-status
+'
+
+
 test_done

t/t7004-tag.sh +3 -0

index d7b319e91..2147938aa 100755
@@ -1354,6 +1354,19 @@ test_expect_success GPG \
 	'test_config gpg.program echo &&
 	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
+# try to sign with bad user.signingkey
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad key)' \

t/t7004-tag.sh test_expect_success GPG \ · line 1369

Quoted 10 lines

t/t7004-tag.sh +10 -0

+	'test_config user.signingkey BobTheMouse &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
+
+# try to produce invalid signature
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad signature format)' \
+	'test_config gpg.x509.program echo &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'

I can see that it is a gpgsm parallel of the earlier test we can see in the precontext of this hunk done for gpg, but how does the last one (and the original this one was modeled after) fail?

We say "echo" is the program that signs for the chosen format, "tag -s" tries to run "echo" instead of "gpgsm" with "--status-fd=2 -bsau" or whatever args we usually give, and...?

I would guess you would either get "I don't know what you wanted me to do with --status-fd=2 option, I am erroring out" from "echo", or the "echo" command exiting without consuming any input, causing the feeder in "tag -s" to get SIGPIPE (or write(2) error), but the latter happens only when the payload to be signed is large enough. On a platform whose "echo" pays no attention to unknown option, "echo" itself may not even error out. And then we try to read from "echo" and we do not get anything (which is expected).

And then who in "git tag -s" notice the breakage?

... goes and looks at gpg-interface.c::sign_buffer() ...

Ah, we check the status-fd output for "[GnuPG:] SIG_CREATED", which would never happen if we are talking to "echo". OK, that is how this thing is expected to fail.

What I have been getting at is if this is really trying to trigger the "(bad signature format)" breakage. The test uses a wrong program to simulate the case where a configured gpg/gpgsm failed to report "SIG_CREATED". "bad signature format" does not sound exactly like that, but you inherited the badness from the original, so let's leave it as is.

Thanks. Modulo a few nits I pointed out above, buried in all the other good bits, this looks reasonable to me.

Henning Schild

author

2018-07-18 10:36 UTC #14 lore raw

Re: [PATCH v4 7/7] gpg-interface t: extend the existing GPG tests with GPGSM

Am Tue, 17 Jul 2018 14:31:36 -0700
schrieb Junio C Hamano <[email protected]>:

Quoted 106 lines

t/lib-gpg.sh then · line 41

Quoted 10 lines · Henning Schild

t/lib-gpg.sh +0 -0

index a5d3b2cba..3fe02876c 100755
@@ -38,7 +38,33 @@ then
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
 		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null

2>&1 \ --sign -u [email protected] &&

t/lib-gpg.sh +1 -1

-		test_set_prereq GPG
+		test_set_prereq GPG &&

We do not mind making GPGSM dependent on GPG, hence this && is justified.

Quoted 29 lines

t/lib-gpg.sh +15 -0

+		# Available key info:
+		# * see t/lib-gpg/gpgsm-gen-key.in
+		# To generate new certificate:
+		#  * no passphrase
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o /tmp/gpgsm.crt.user \
+		#		--generate-key \
+		#		--batch t/lib-gpg/gpgsm-gen-key.in
+		# To import certificate:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		--import /tmp/gpgsm.crt.user
+		# To export into a .p12 we can later import:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o t/lib-gpg/gpgsm_cert.p12 \
+		#		--export-secret-key-p12

"[email protected]"

t/lib-gpg.sh +2 -0

+		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
+			--passphrase-fd 0 --pinentry-mode loopback

t/lib-gpg.sh +1 -0

+			--import

"$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&

t/lib-gpg.sh +6 -1

+		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+			| grep fingerprint: | cut -d" " -f4 | tr
-d '\n' > \
+			${GNUPGHOME}/trustlist.txt &&
+		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+		echo hello | gpgsm --homedir "${GNUPGHOME}"

Quoted 1 line

/dev/null \

t/lib-gpg.sh

Quoted 3 lines

t/lib-gpg.sh +2 -1

+			-u [email protected] -o /dev/null
--sign - 2>&1 &&
+		test_set_prereq GPGSM

And when any of the above fails, we refrain from setting GPGSM prereq. Otherwise we are prepared to perform tests with gpgsm and get the prereq.

Quoted 11 lines

t/t4202-log.sh +0 -0

index 25b1f8cc7..f57781e39 100755
@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed

branch' ' git commit -S -m signed_commit
'

t/t4202-log.sh +2 -0

+test_expect_success GPGSM 'setup signed branch x509' '
+	test_when_finished "git reset --hard && git checkout

master" &&

t/t4202-log.sh test_expect_success GPG 'setup signed branch' ' · line 1567

Quoted 7 lines

t/t4202-log.sh +7 -0

+	git checkout -b signed-x509 master &&
+	echo foo >foo &&
+	git add foo &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git commit -S -m signed_commit
+'

OK.

t/t4202-log.sh test_expect_success GPG 'setup signed branch' ' · line 1579

Quoted 5 lines

t/t4202-log.sh +5 -0

+test_expect_success GPGSM 'log --graph --show-signature x509' '
+	git log --graph --show-signature -n1 signed-x509 >actual &&
+	grep "^| gpgsm: Signature made" actual &&
+	grep "^| gpgsm: Good signature" actual
+'

OK.

Quoted 24 lines

t/t4202-log.sh +0 -1

@@ -1581,6 +1597,29 @@ test_expect_success GPG 'log --graph
--show-signature for merged tag' ' grep "^| | gpg: Good signature"

actual '

+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '

t/t4202-log.sh +1 -0

+	test_when_finished "git reset --hard && git checkout

master" &&

t/t4202-log.sh +16 -0

+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git checkout -b plain-x509 master &&
+	echo aaa >bar &&
+	git add bar &&
+	git commit -m bar_commit &&
+	git checkout -b tagged-x509 master &&
+	echo bbb >baz &&
+	git add baz &&
+	git commit -m baz_commit &&
+	git tag -s -m signed_tag_msg signed_tag_x509 &&
+	git checkout plain-x509 &&
+	git merge --no-ff -m msg signed_tag_x509 &&
+	git log --graph --show-signature -n1 plain-x509 >actual &&
+	grep "^|\\\  merged tag" actual &&
+	grep "^| | gpgsm: Signature made" actual &&

Quoted 8 lines

t/t4202-log.sh test_expect_success GPG 'log --graph --show-signature for merged tag' ' · line 1620
Quoted 3 lines
t/t4202-log.sh +3 -0
+	grep "^| | gpgsm: Good signature" actual &&
+	git config --unset gpg.format &&
+	git config --unset user.signingkey  
You are using test_config early enough in this test; doesn't that take care of the last two steps for you, even when an earlier step failed? If that is the case, then remove the last two line (and && at the end of the line before).

Right, dropped those two lines and &&

Quoted 8 lines

Quoted 8 lines
t/t5534-push-signed.sh +0 -0
index 1cea758f7..a3a12bd05 100755
@@ -218,4 +218,56 @@ test_expect_success GPG 'fail without key and
heed user.signingkey' ' test_cmp expect dst/push-cert-status
'

Quoted 6 lines

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 223
Quoted 4 lines
+test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
t/t5534-push-signed.sh +2 -0
+	test_config gpg.format x509 &&
+	env | grep GIT > envfile &&  
The "envfile" is unused, no? Remove this line.

Thanks, debugging leftovers.

Quoted 26 lines

Quoted 11 lines
t/t5534-push-signed.sh +11 -0
+	prepare_dst &&
+	mkdir -p dst/.git/hooks &&
+	git -C dst config receive.certnonceseed sekrit &&
+	write_script dst/.git/hooks/post-receive <<-\EOF &&
+	# discard the update list
+	cat >/dev/null
+	# record the push certificate
+	if test -n "${GIT_PUSH_CERT-}"
+	then
+		git cat-file blob $GIT_PUSH_CERT >../push-cert
+	fi &&
t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 244
Quoted 10 lines
t/t5534-push-signed.sh +10 -0
+
+	cat >../push-cert-status <<E_O_F
+	SIGNER=${GIT_PUSH_CERT_SIGNER-nobody}
+	KEY=${GIT_PUSH_CERT_KEY-nokey}
+	STATUS=${GIT_PUSH_CERT_STATUS-nostatus}
+	NONCE_STATUS=${GIT_PUSH_CERT_NONCE_STATUS-nononcestatus}
+	NONCE=${GIT_PUSH_CERT_NONCE-nononce}
+	E_O_F
+
+	EOF  
OK, so up to this are what is done by post-receive, including the overwriting of ../push-cert (which is one level above the receiving repository's .git/, i.e. dst/push-cert) and ../push-cert-status.

Quoted 13 lines

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 248
Quoted 4 lines
t/t5534-push-signed.sh +4 -0
+	unset GIT_COMMITTER_EMAIL &&
+	git config user.email [email protected] &&
+	git config user.signingkey "" &&
+	test_must_fail git push --signed dst noop ff +noff &&  
This is OK for a test that is known to be always at the end, but also forbids others to further update this script to add more tests at the end, as the standard setting of environment is blown away (the config is probably OK, but test_config to arrange them to be cleaned up would have been nicer), which is not very nice. I think it should be easy to fix it when it becomes necessary, but at the same time if it is easy to fix, then probably we shouldn't introduce a breakage in the first place, so I am on the fence.

Switched to test_config, this is all coming from copying the previous tests, which i left as is.

Quoted 6 lines

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 250
Quoted 2 lines
t/t5534-push-signed.sh +2 -0
+	git config user.signingkey [email protected] &&
+	git push --signed dst noop ff +noff &&  
So,... this is run without resetting user.email and demonstrates that signingkey is the only thing that matters, which makes sense.

Quoted 15 lines

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 262
Quoted 13 lines
t/t5534-push-signed.sh +8 -0
+	(
+		cat <<-\EOF &&
+		SIGNER=/CN=C O Mitter/O=Example/SN=C O/GN=Mitter
+		KEY=
+		STATUS=G
+		NONCE_STATUS=OK
+		EOF
+		sed -n -e "s/^nonce /NONCE=/p" -e "/^$/q"
dst/push-cert
t/t5534-push-signed.sh +2 -0
+	) >expect.in &&
+	key=$(cat "${GNUPGHOME}/trustlist.txt" | cut -d" " -f1 |
tr -d ":") &&
t/t5534-push-signed.sh +1 -0
+	sed -e "s/^KEY=/KEY=${key}/" expect.in > expect &&  
s/> expect/>expect/;

Done.

Quoted 58 lines

Quoted 22 lines
t/t5534-push-signed.sh +9 -0
+	noop=$(git rev-parse noop) &&
+	ff=$(git rev-parse ff) &&
+	noff=$(git rev-parse noff) &&
+	grep "$noop $ff refs/heads/ff" dst/push-cert &&
+	grep "$noop $noff refs/heads/noff" dst/push-cert &&
+	test_cmp expect dst/push-cert-status
+'
+
+
 test_done
t/t7004-tag.sh +4 -0
index d7b319e91..2147938aa 100755
@@ -1354,6 +1354,19 @@ test_expect_success GPG \
 	'test_config gpg.program echo &&
 	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
+# try to sign with bad user.signingkey
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad key)' \
+	'test_config user.signingkey BobTheMouse &&
t/t7004-tag.sh test_expect_success GPG \ · line 1370
Quoted 10 lines
t/t7004-tag.sh +9 -0
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
+
+# try to produce invalid signature
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad signature
 format)' \
+	'test_config gpg.x509.program echo &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'  
I can see that it is a gpgsm parallel of the earlier test we can see in the precontext of this hunk done for gpg, but how does the last one (and the original this one was modeled after) fail?
We say "echo" is the program that signs for the chosen format, "tag
(quoted diff) +0 -2
-s" tries to run "echo" instead of "gpgsm" with "--status-fd=2
-bsau" or whatever args we usually give, and...?
I would guess you would either get "I don't know what you wanted me to do with --status-fd=2 option, I am erroring out" from "echo", or the "echo" command exiting without consuming any input, causing the feeder in "tag -s" to get SIGPIPE (or write(2) error), but the latter happens only when the payload to be signed is large enough. On a platform whose "echo" pays no attention to unknown option, "echo" itself may not even error out. And then we try to read from "echo" and we do not get anything (which is expected).
And then who in "git tag -s" notice the breakage?
... goes and looks at gpg-interface.c::sign_buffer() ...
Ah, we check the status-fd output for "[GnuPG:] SIG_CREATED", which would never happen if we are talking to "echo". OK, that is how this thing is expected to fail.

Quoted 6 lines

What I have been getting at is if this is really trying to trigger the "(bad signature format)" breakage. The test uses a wrong program to simulate the case where a configured gpg/gpgsm failed to report "SIG_CREATED". "bad signature format" does not sound exactly like that, but you inherited the badness from the original, so let's leave it as is.

All valid points and yes this is coming from copying the other test. Leaving as is.

Quoted 2 lines

Thanks. Modulo a few nits I pointed out above, buried in all the other good bits, this looks reasonable to me.

Cool, Thanks.

Henning

Junio C Hamano

2018-07-18 17:06 UTC #15 lore raw

Re: [PATCH v4 7/7] gpg-interface t: extend the existing GPG tests with GPGSM

Quoted 14 lines · Henning Schild

Quoted 5 lines
t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 248
Quoted 4 lines
t/t5534-push-signed.sh +4 -0
+	unset GIT_COMMITTER_EMAIL &&
+	git config user.email [email protected] &&
+	git config user.signingkey "" &&
+	test_must_fail git push --signed dst noop ff +noff &&  
Quoted 8 lines
This is OK for a test that is known to be always at the end, but also forbids others to further update this script to add more tests at the end, as the standard setting of environment is blown away (the config is probably OK, but test_config to arrange them to be cleaned up would have been nicer), which is not very nice. I think it should be easy to fix it when it becomes necessary, but at the same time if it is easy to fix, then probably we shouldn't introduce a breakage in the first place, so I am on the fence.

Quoted 2 lines

Switched to test_config, this is all coming from copying the previous tests, which i left as is.

I was specifically talking about the unsetting of GIT_COMMITTER_EMAIL, not the configuration variables.

An easy way to try one step without the environment without affecting later tests would be to run the above in a subshell, perhaps like

	test_config user.email ... &&
	test_config user.signingkey "" &&
	(
		safe_unset GIT_COMMITTER_EMAIL &&
		test_must_fail git push --signed ...
	) &&

Henning Schild

author

2018-07-19 12:14 UTC #16 lore raw

PATCH 1/1 t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests

Unsetting the varibale for good can have unwanted effects for new tests added in the future It also meant we needed to hardcode the value for "user.signingkey".
Move the unset into a subshell, get rid of the hardcoded "[email protected]", and switch the GPG variant to using test_config just like GPGSM.

Signed-off-by Henning Schild <[email protected]>

---
 t/t5534-push-signed.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

t/t5534-push-signed.sh +12 -7

index 0cb88aa6f..f6d674156 100755
@@ -194,10 +194,12 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '
 
 	EOF
 
-	unset GIT_COMMITTER_EMAIL &&
-	git config user.email [email protected] &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	git config user.signingkey [email protected] &&
+	test_config user.email [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&
 
 	(

@@ -241,11 +243,14 @@ test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
 	E_O_F
 
 	EOF
-	unset GIT_COMMITTER_EMAIL &&
+
 	test_config user.email [email protected] &&
 	test_config user.signingkey "" &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	test_config user.signingkey [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&
 
 	(

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-19 12:16 UTC #17 lore raw

Re: [PATCH] t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests

Looking at "what is cooking" i assume i should not add/fold this to/in the serien anymore. So it comes as a separate patch on top.

Henning

Am Thu, 19 Jul 2018 14:14:09 +0200
schrieb Henning Schild <[email protected]>:

Quoted 51 lines

Signed-off-by Henning Schild <[email protected]>

---
 t/t5534-push-signed.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

t/t5534-push-signed.sh +0 -0

index 0cb88aa6f..f6d674156 100755
@@ -194,10 +194,12 @@ test_expect_success GPG 'fail without key and

heed user.signingkey' '

t/t5534-push-signed.sh +6 -4

 	EOF
 
-	unset GIT_COMMITTER_EMAIL &&
-	git config user.email [email protected] &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	git config user.signingkey [email protected] &&
+	test_config user.email [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&
 
 	(

@@ -241,11 +243,14 @@ test_expect_success GPGSM 'fail without key and

heed user.signingkey x509' ' E_O_F

t/t5534-push-signed.sh +6 -3

 	EOF
-	unset GIT_COMMITTER_EMAIL &&
+
 	test_config user.email [email protected] &&
 	test_config user.signingkey "" &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	test_config user.signingkey [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&
 
 	(

Junio C Hamano

2018-07-19 22:28 UTC #18 lore raw

Re: [PATCH] t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests

Quoted 2 lines · Henning Schild

Looking at "what is cooking" i assume i should not add/fold this to/in the serien anymore. So it comes as a separate patch on top.

Thanks. I only said:

I think this round is mostly ready, except for a minor nit in the last step. I do not mind merging this to 'next' and leave fixing of the test to a later clean-up.

meaing that I _do not mind_ merging the slightly broken one to 'next' and fixing it with a follow-up (i.e. this patch), but fixing with a replacement patch so that we pretend we never made a mistake that needs fixing from the beginning is of course preferrable ;-)

Let's squash this patch into the last one in the series.

Henning Schild

author

2018-07-20 08:28 UTC #19 lore raw

Re: [PATCH] t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests

Am Thu, 19 Jul 2018 15:27:56 -0700
schrieb Junio C Hamano <[email protected]>:

Quoted 11 lines

Quoted 2 lines · Henning Schild
Looking at "what is cooking" i assume i should not add/fold this to/in the serien anymore. So it comes as a separate patch on top.
Thanks. I only said:
I think this round is mostly ready, except for a minor nit in the last step. I do not mind merging this to 'next' and leave fixing of the test to a later clean-up.

Quoted 6 lines

meaing that I _do not mind_ merging the slightly broken one to 'next' and fixing it with a follow-up (i.e. this patch), but fixing with a replacement patch so that we pretend we never made a mistake that needs fixing from the beginning is of course preferrable ;-)
Let's squash this patch into the last one in the series.

Ok, just sent a v6 of 7/7.

Henning

Taylor Blau

2018-07-19 15:10 UTC #20 lore raw

Re: [PATCH] t/t5534: do not unset GIT_COMMITTER_EMAIL for other tests

In reply to Henning Schild

Quoted 1 line · Henning Schild

Unsetting the varibale for good can have unwanted effects for new

s/varibale/variable

Quoted 1 line

tests added in the future It also meant we needed to hardcode the

s/future/&.

Quoted 4 lines

value for "user.signingkey".
Move the unset into a subshell, get rid of the hardcoded "[email protected]", and switch the GPG variant to using test_config just like GPGSM.

OK, sounds good.

Quoted 14 lines

Signed-off-by Henning Schild <[email protected]>

---
 t/t5534-push-signed.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

t/t5534-push-signed.sh +0 -1

index 0cb88aa6f..f6d674156 100755
@@ -194,10 +194,12 @@ test_expect_success GPG 'fail without key and heed user.signingkey' '
 
 	EOF
 
-	unset GIT_COMMITTER_EMAIL &&

t/t5534-push-signed.sh test_expect_success GPG 'fail without key and heed user.signingkey' ' · line 203

Quoted 10 lines

t/t5534-push-signed.sh +6 -3

-	git config user.email [email protected] &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	git config user.signingkey [email protected] &&
+	test_config user.email [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&

I think replacing the 'git config' invocations with their 'test_config' equivalents in its own patch would be easier to grok, but I don't think that splitting the change out is worth a re-roll on its own.

t/t5534-push-signed.sh test_expect_success GPGSM 'fail without key and heed user.signingkey x509' ' · line 246

Quoted 7 lines

t/t5534-push-signed.sh +1 -1

@@ -241,11 +243,14 @@ test_expect_success GPGSM 'fail without key and heed user.signingkey x509' '
 	E_O_F
 
 	EOF
-	unset GIT_COMMITTER_EMAIL &&
+

Nit; remove this newline.

t/t5534-push-signed.sh test_expect_success GPGSM 'fail without key and heed user.signingkey x509' ' · line 254

Quoted 10 lines

t/t5534-push-signed.sh +5 -2

 	test_config user.email [email protected] &&
 	test_config user.signingkey "" &&
-	test_must_fail git push --signed dst noop ff +noff &&
-	test_config user.signingkey [email protected] &&
+	(
+		unset GIT_COMMITTER_EMAIL &&
+		test_must_fail git push --signed dst noop ff +noff
+	) &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
 	git push --signed dst noop ff +noff &&

Looks good.

Quoted 3 lines

t/t5534-push-signed.sh +0 -1
 	(
--
2.16.4

Thanks,
Taylor

Henning Schild

author

2018-07-18 09:31 UTC #21 lore raw

PATCH v5 7/7 gpg-interface t: extend the existing GPG tests with GPGSM

In reply to Henning Schild

Signed-off-by Henning Schild <[email protected]>

---
 t/lib-gpg.sh               |  28 ++++++++++++++++++++++++-
 t/lib-gpg/gpgsm-gen-key.in |   8 +++++++
 t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes
 t/t4202-log.sh             |  37 ++++++++++++++++++++++++++++++++
 t/t5534-push-signed.sh     |  51 +++++++++++++++++++++++++++++++++++++++++++++
 t/t7004-tag.sh             |  13 ++++++++++++
 t/t7030-verify-tag.sh      |  33 +++++++++++++++++++++++++++++
 7 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 t/lib-gpg/gpgsm-gen-key.in
 create mode 100644 t/lib-gpg/gpgsm_cert.p12

t/lib-gpg.sh +27 -1

index a5d3b2cba..3fe02876c 100755
@@ -38,7 +38,33 @@ then
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
 		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
 			--sign -u [email protected] &&
-		test_set_prereq GPG
+		test_set_prereq GPG &&
+		# Available key info:
+		# * see t/lib-gpg/gpgsm-gen-key.in
+		# To generate new certificate:
+		#  * no passphrase
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o /tmp/gpgsm.crt.user \
+		#		--generate-key \
+		#		--batch t/lib-gpg/gpgsm-gen-key.in
+		# To import certificate:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		--import /tmp/gpgsm.crt.user
+		# To export into a .p12 we can later import:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o t/lib-gpg/gpgsm_cert.p12 \
+		#		--export-secret-key-p12 "[email protected]"
+		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
+			--passphrase-fd 0 --pinentry-mode loopback \
+			--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+			| grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+			${GNUPGHOME}/trustlist.txt &&
+		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+		echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+			-u [email protected] -o /dev/null --sign - 2>&1 &&
+		test_set_prereq GPGSM
 		;;
 	esac
 fi

t/lib-gpg/gpgsm-gen-key.in new file +8 -0

new file mode 100644
index 000000000..a7fd87c06
@@ -0,0 +1,8 @@
+Key-Type: RSA
+Key-Length: 2048
+Key-Usage: sign
+Serial: random
+Name-DN: CN=C O Mitter, O=Example, SN=C O, GN=Mitter
+Name-Email: [email protected]
+Not-Before: 1970-01-01 00:00:00
+Not-After: 3000-01-01 00:00:00

t/lib-gpg/gpgsm_cert.p12 new file binary +0 -0

new file mode 100644
index 0000000000000000000000000000000000000000..94ffad0d31a3b6c4e849b29d960762e485ba7ea8
GIT binary patch

t/t4202-log.sh +37 -0

index 25b1f8cc7..05d3707e3 100755
@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed branch' '
 	git commit -S -m signed_commit
 '
 
+test_expect_success GPGSM 'setup signed branch x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	git checkout -b signed-x509 master &&
+	echo foo >foo &&
+	git add foo &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git commit -S -m signed_commit
+'
+
 test_expect_success GPG 'log --graph --show-signature' '
 	git log --graph --show-signature -n1 signed >actual &&
 	grep "^| gpg: Signature made" actual &&
 	grep "^| gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature x509' '
+	git log --graph --show-signature -n1 signed-x509 >actual &&
+	grep "^| gpgsm: Signature made" actual &&
+	grep "^| gpgsm: Good signature" actual
+'
+
 test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	test_when_finished "git reset --hard && git checkout master" &&
 	git checkout -b plain master &&

@@ -1581,6 +1597,27 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git checkout -b plain-x509 master &&
+	echo aaa >bar &&
+	git add bar &&
+	git commit -m bar_commit &&
+	git checkout -b tagged-x509 master &&
+	echo bbb >baz &&
+	git add baz &&
+	git commit -m baz_commit &&
+	git tag -s -m signed_tag_msg signed_tag_x509 &&
+	git checkout plain-x509 &&
+	git merge --no-ff -m msg signed_tag_x509 &&
+	git log --graph --show-signature -n1 plain-x509 >actual &&
+	grep "^|\\\  merged tag" actual &&
+	grep "^| | gpgsm: Signature made" actual &&
+	grep "^| | gpgsm: Good signature" actual
+'
+
 test_expect_success GPG '--no-show-signature overrides --show-signature' '
 	git log -1 --show-signature --no-show-signature signed >actual &&
 	! grep "^gpg:" actual

t/t5534-push-signed.sh +51 -0

55 lines not on this page · raw message

t/t7004-tag.sh +13 -0

index d7b319e91..2147938aa 100755
@@ -1354,6 +1354,19 @@ test_expect_success GPG \
 	'test_config gpg.program echo &&
 	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
+# try to sign with bad user.signingkey
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad key)' \
+	'test_config user.signingkey BobTheMouse &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
+
+# try to produce invalid signature
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad signature format)' \
+	'test_config gpg.x509.program echo &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
 # try to verify without gpg:

t/t7030-verify-tag.sh +33 -0

index 291a1e2b0..db6295d65 100755
@@ -41,6 +41,13 @@ test_expect_success GPG 'create signed tags' '
 	git tag -uB7227189 -m eighth eighth-signed-alt
 '
 
+test_expect_success GPGSM 'create signed tags x509 ' '
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo 9 >file && test_tick && git commit -a -m "nineth gpgsm-signed" &&
+	git tag -s -m nineth nineth-signed-x509
+'
+
 test_expect_success GPG 'verify and show signatures' '
 	(
 		for tag in initial second merge fourth-signed sixth-signed seventh-signed

@@ -72,6 +79,13 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPGSM 'verify and show signatures x509' '
+	git verify-tag nineth-signed-x509 2>actual &&
+	grep "Good signature from" actual &&
+	! grep "BAD signature from" actual &&
+	echo nineth-signed-x509 OK
+'
+
 test_expect_success GPG 'detect fudged signature' '
 	git cat-file tag seventh-signed >raw &&
 	sed -e "/^tag / s/seventh/7th forged/" raw >forged1 &&

@@ -112,6 +126,12 @@ test_expect_success GPG 'verify signatures with --raw' '
 	)
 '
 
+test_expect_success GPGSM 'verify signatures with --raw x509' '
+	git verify-tag --raw nineth-signed-x509 2>actual &&
+	grep "GOODSIG" actual &&
+	! grep "BADSIG" actual &&
+	echo nineth-signed-x509 OK
+'
 test_expect_success GPG 'verify multiple tags' '
 	tags="fourth-signed sixth-signed seventh-signed" &&
 	for i in $tags

@@ -125,6 +145,19 @@ test_expect_success GPG 'verify multiple tags' '
 	test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success GPGSM 'verify multiple tags x509' '
+	tags="seventh-signed nineth-signed-x509" &&
+	for i in $tags
+	do
+		git verify-tag -v --raw $i || return 1
+	done >expect.stdout 2>expect.stderr.1 &&
+	grep "^.GNUPG:." <expect.stderr.1 >expect.stderr &&
+	git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+	grep "^.GNUPG:." <actual.stderr.1 >actual.stderr &&
+	test_cmp expect.stdout actual.stdout &&
+	test_cmp expect.stderr actual.stderr
+'
+
 test_expect_success GPG 'verifying tag with --format' '
 	cat >expect <<-\EOF &&
 	tagname : fourth-signed

Signature · 1 line

2.16.4

Henning Schild

author

2018-07-20 08:28 UTC #22 lore raw

PATCH v6 7/7 gpg-interface t: extend the existing GPG tests with GPGSM

Signed-off-by Henning Schild <[email protected]>

---
 t/lib-gpg.sh               |  28 +++++++++++++++++++-
 t/lib-gpg/gpgsm-gen-key.in |   8 ++++++
 t/lib-gpg/gpgsm_cert.p12   | Bin 0 -> 2652 bytes
 t/t4202-log.sh             |  37 ++++++++++++++++++++++++++
 t/t5534-push-signed.sh     |  63 ++++++++++++++++++++++++++++++++++++++++++---
 t/t7004-tag.sh             |  13 ++++++++++
 t/t7030-verify-tag.sh      |  34 ++++++++++++++++++++++++
 7 files changed, 178 insertions(+), 5 deletions(-)
 create mode 100644 t/lib-gpg/gpgsm-gen-key.in
 create mode 100644 t/lib-gpg/gpgsm_cert.p12

t/lib-gpg.sh +27 -1

index a5d3b2cba..3fe02876c 100755
@@ -38,7 +38,33 @@ then
 			"$TEST_DIRECTORY"/lib-gpg/ownertrust &&
 		gpg --homedir "${GNUPGHOME}" </dev/null >/dev/null 2>&1 \
 			--sign -u [email protected] &&
-		test_set_prereq GPG
+		test_set_prereq GPG &&
+		# Available key info:
+		# * see t/lib-gpg/gpgsm-gen-key.in
+		# To generate new certificate:
+		#  * no passphrase
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o /tmp/gpgsm.crt.user \
+		#		--generate-key \
+		#		--batch t/lib-gpg/gpgsm-gen-key.in
+		# To import certificate:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		--import /tmp/gpgsm.crt.user
+		# To export into a .p12 we can later import:
+		#	gpgsm --homedir /tmp/gpghome/ \
+		#		-o t/lib-gpg/gpgsm_cert.p12 \
+		#		--export-secret-key-p12 "[email protected]"
+		echo | gpgsm --homedir "${GNUPGHOME}" 2>/dev/null \
+			--passphrase-fd 0 --pinentry-mode loopback \
+			--import "$TEST_DIRECTORY"/lib-gpg/gpgsm_cert.p12 &&
+		gpgsm --homedir "${GNUPGHOME}" 2>/dev/null -K \
+			| grep fingerprint: | cut -d" " -f4 | tr -d '\n' > \
+			${GNUPGHOME}/trustlist.txt &&
+		echo " S relax" >> ${GNUPGHOME}/trustlist.txt &&
+		(gpgconf --kill gpg-agent >/dev/null 2>&1 || : ) &&
+		echo hello | gpgsm --homedir "${GNUPGHOME}" >/dev/null \
+			-u [email protected] -o /dev/null --sign - 2>&1 &&
+		test_set_prereq GPGSM
 		;;
 	esac
 fi

t/lib-gpg/gpgsm-gen-key.in new file +8 -0

new file mode 100644
index 000000000..a7fd87c06
@@ -0,0 +1,8 @@
+Key-Type: RSA
+Key-Length: 2048
+Key-Usage: sign
+Serial: random
+Name-DN: CN=C O Mitter, O=Example, SN=C O, GN=Mitter
+Name-Email: [email protected]
+Not-Before: 1970-01-01 00:00:00
+Not-After: 3000-01-01 00:00:00

t/lib-gpg/gpgsm_cert.p12 new file binary +0 -0

new file mode 100644
index 0000000000000000000000000000000000000000..94ffad0d31a3b6c4e849b29d960762e485ba7ea8
GIT binary patch

t/t4202-log.sh +37 -0

index 25b1f8cc7..05d3707e3 100755
@@ -1556,12 +1556,28 @@ test_expect_success GPG 'setup signed branch' '
 	git commit -S -m signed_commit
 '
 
+test_expect_success GPGSM 'setup signed branch x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	git checkout -b signed-x509 master &&
+	echo foo >foo &&
+	git add foo &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git commit -S -m signed_commit
+'
+
 test_expect_success GPG 'log --graph --show-signature' '
 	git log --graph --show-signature -n1 signed >actual &&
 	grep "^| gpg: Signature made" actual &&
 	grep "^| gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature x509' '
+	git log --graph --show-signature -n1 signed-x509 >actual &&
+	grep "^| gpgsm: Signature made" actual &&
+	grep "^| gpgsm: Good signature" actual
+'
+
 test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	test_when_finished "git reset --hard && git checkout master" &&
 	git checkout -b plain master &&

@@ -1581,6 +1597,27 @@ test_expect_success GPG 'log --graph --show-signature for merged tag' '
 	grep "^| | gpg: Good signature" actual
 '
 
+test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' '
+	test_when_finished "git reset --hard && git checkout master" &&
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	git checkout -b plain-x509 master &&
+	echo aaa >bar &&
+	git add bar &&
+	git commit -m bar_commit &&
+	git checkout -b tagged-x509 master &&
+	echo bbb >baz &&
+	git add baz &&
+	git commit -m baz_commit &&
+	git tag -s -m signed_tag_msg signed_tag_x509 &&
+	git checkout plain-x509 &&
+	git merge --no-ff -m msg signed_tag_x509 &&
+	git log --graph --show-signature -n1 plain-x509 >actual &&
+	grep "^|\\\  merged tag" actual &&
+	grep "^| | gpgsm: Signature made" actual &&
+	grep "^| | gpgsm: Good signature" actual
+'
+
 test_expect_success GPG '--no-show-signature overrides --show-signature' '
 	git log -1 --show-signature --no-show-signature signed >actual &&
 	! grep "^gpg:" actual

t/t5534-push-signed.sh +59 -4

73 lines not on this page · raw message

t/t7004-tag.sh +13 -0

index d7b319e91..2147938aa 100755
@@ -1354,6 +1354,19 @@ test_expect_success GPG \
 	'test_config gpg.program echo &&
 	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
+# try to sign with bad user.signingkey
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad key)' \
+	'test_config user.signingkey BobTheMouse &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
+
+# try to produce invalid signature
+test_expect_success GPGSM \
+	'git tag -s fails if gpgsm is misconfigured (bad signature format)' \
+	'test_config gpg.x509.program echo &&
+	 test_config gpg.format x509 &&
+	 test_must_fail git tag -s -m tail tag-gpg-failure'
 
 # try to verify without gpg:

t/t7030-verify-tag.sh +34 -0

index 291a1e2b0..99f35a5bb 100755
@@ -41,6 +41,13 @@ test_expect_success GPG 'create signed tags' '
 	git tag -uB7227189 -m eighth eighth-signed-alt
 '
 
+test_expect_success GPGSM 'create signed tags x509 ' '
+	test_config gpg.format x509 &&
+	test_config user.signingkey $GIT_COMMITTER_EMAIL &&
+	echo 9 >file && test_tick && git commit -a -m "nineth gpgsm-signed" &&
+	git tag -s -m nineth nineth-signed-x509
+'
+
 test_expect_success GPG 'verify and show signatures' '
 	(
 		for tag in initial second merge fourth-signed sixth-signed seventh-signed

@@ -72,6 +79,13 @@ test_expect_success GPG 'verify and show signatures' '
 	)
 '
 
+test_expect_success GPGSM 'verify and show signatures x509' '
+	git verify-tag nineth-signed-x509 2>actual &&
+	grep "Good signature from" actual &&
+	! grep "BAD signature from" actual &&
+	echo nineth-signed-x509 OK
+'
+
 test_expect_success GPG 'detect fudged signature' '
 	git cat-file tag seventh-signed >raw &&
 	sed -e "/^tag / s/seventh/7th forged/" raw >forged1 &&

@@ -112,6 +126,13 @@ test_expect_success GPG 'verify signatures with --raw' '
 	)
 '
 
+test_expect_success GPGSM 'verify signatures with --raw x509' '
+	git verify-tag --raw nineth-signed-x509 2>actual &&
+	grep "GOODSIG" actual &&
+	! grep "BADSIG" actual &&
+	echo nineth-signed-x509 OK
+'
+
 test_expect_success GPG 'verify multiple tags' '
 	tags="fourth-signed sixth-signed seventh-signed" &&
 	for i in $tags

@@ -125,6 +146,19 @@ test_expect_success GPG 'verify multiple tags' '
 	test_cmp expect.stderr actual.stderr
 '
 
+test_expect_success GPGSM 'verify multiple tags x509' '
+	tags="seventh-signed nineth-signed-x509" &&
+	for i in $tags
+	do
+		git verify-tag -v --raw $i || return 1
+	done >expect.stdout 2>expect.stderr.1 &&
+	grep "^.GNUPG:." <expect.stderr.1 >expect.stderr &&
+	git verify-tag -v --raw $tags >actual.stdout 2>actual.stderr.1 &&
+	grep "^.GNUPG:." <actual.stderr.1 >actual.stderr &&
+	test_cmp expect.stdout actual.stdout &&
+	test_cmp expect.stderr actual.stderr
+'
+
 test_expect_success GPG 'verifying tag with --format' '
 	cat >expect <<-\EOF &&
 	tagname : fourth-signed

Signature · 1 line

2.16.4

Junio C Hamano

2018-07-20 15:42 UTC #23 lore raw

Re: [PATCH v6 7/7] gpg-interface t: extend the existing GPG tests with GPGSM

Thanks.