Merge branch 'jt/fast-import-signed-modes'

Handling of signed commits and tags in fast-import has been made more
configurable.

* jt/fast-import-signed-modes:
  fast-import: add 'abort-if-invalid' mode to '--signed-tags=<mode>'
  fast-import: add 'sign-if-invalid' mode to '--signed-tags=<mode>'
  fast-import: add 'strip-if-invalid' mode to '--signed-tags=<mode>'
  fast-import: add 'abort-if-invalid' mode to '--signed-commits=<mode>'
  fast-export: check for unsupported signing modes earlier

Documentation/git-fast-import.adoc

@@ -66,11 +66,10 @@ fast-import stream! This option is enabled automatically for
 remote-helpers that use the `import` capability, as they are
 already trusted to run their own code.
 
-`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::
+`--signed-tags=<mode>`::
 	Specify how to handle signed tags. Behaves in the same way as
-	the `--signed-commits=<mode>` below, except that the
-	`strip-if-invalid` mode is not yet supported. Like for signed
-	commits, the default mode is `verbatim`.
+	the `--signed-commits=<mode>` below. Like for signed commits,
+	the default mode is `verbatim`.
 
 `--signed-commits=<mode>`::
 	Specify how to handle signed commits. The following <mode>s
@@ -90,6 +89,8 @@ already trusted to run their own code.
   commit signatures and replaces invalid signatures with newly created ones.
   Valid signatures are left unchanged. If `<keyid>` is provided, that key is
   used for signing; otherwise the configured default signing key is used.
+* `abort-if-invalid` will make this program die when encountering a signed
+  commit that is unable to be verified.
 
 Options for Frontends
 ~~~~~~~~~~~~~~~~~~~~~

builtin/fast-export.c

@@ -64,7 +64,8 @@ static int parse_opt_sign_mode(const struct option *opt,
 	if (unset)
 		return 0;
 
-	if (parse_sign_mode(arg, val, NULL))
+	if (parse_sign_mode(arg, val, NULL) || (*val == SIGN_STRIP_IF_INVALID) ||
+	    (*val == SIGN_SIGN_IF_INVALID) || (*val == SIGN_ABORT_IF_INVALID))
 		return error(_("unknown %s mode: %s"), opt->long_name, arg);
 
 	return 0;
@@ -822,12 +823,6 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
 			die(_("encountered signed commit %s; use "
 			      "--signed-commits=<mode> to handle it"),
 			    oid_to_hex(&commit->object.oid));
-		case SIGN_STRIP_IF_INVALID:
-			die(_("'strip-if-invalid' is not a valid mode for "
-			      "git fast-export with --signed-commits=<mode>"));
-		case SIGN_SIGN_IF_INVALID:
-			die(_("'sign-if-invalid' is not a valid mode for "
-			      "git fast-export with --signed-commits=<mode>"));
 		default:
 			BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 		}
@@ -970,12 +965,6 @@ static void handle_tag(const char *name, struct tag *tag)
 				die(_("encountered signed tag %s; use "
 				      "--signed-tags=<mode> to handle it"),
 				    oid_to_hex(&tag->object.oid));
-			case SIGN_STRIP_IF_INVALID:
-				die(_("'strip-if-invalid' is not a valid mode for "
-				      "git fast-export with --signed-tags=<mode>"));
-			case SIGN_SIGN_IF_INVALID:
-				die(_("'sign-if-invalid' is not a valid mode for "
-				      "git fast-export with --signed-tags=<mode>"));
 			default:
 				BUG("invalid signed_commit_mode value %d", signed_commit_mode);
 			}

builtin/fast-import.c

@@ -191,6 +191,7 @@ static const char *global_prefix;
 static enum sign_mode signed_tag_mode = SIGN_VERBATIM;
 static enum sign_mode signed_commit_mode = SIGN_VERBATIM;
 static const char *signed_commit_keyid;
+static const char *signed_tag_keyid;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -2892,6 +2893,9 @@ static void handle_signature_if_invalid(struct strbuf *new_data,
 	ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
 
 	if (ret) {
+		if (mode == SIGN_ABORT_IF_INVALID)
+			die(_("aborting due to invalid signature"));
+
 		warn_invalid_signature(&signature_check, msg->buf, mode);
 
 		if (mode == SIGN_SIGN_IF_INVALID) {
@@ -2983,6 +2987,7 @@ static void parse_new_commit(const char *arg)
 		case SIGN_VERBATIM:
 		case SIGN_STRIP_IF_INVALID:
 		case SIGN_SIGN_IF_INVALID:
+		case SIGN_ABORT_IF_INVALID:
 			import_one_signature(&sig_sha1, &sig_sha256, v);
 			break;
 
@@ -3068,7 +3073,8 @@ static void parse_new_commit(const char *arg)
 			encoding);
 
 	if ((signed_commit_mode == SIGN_STRIP_IF_INVALID ||
-	     signed_commit_mode == SIGN_SIGN_IF_INVALID) &&
+	     signed_commit_mode == SIGN_SIGN_IF_INVALID ||
+	     signed_commit_mode == SIGN_ABORT_IF_INVALID) &&
 	    (sig_sha1.hash_algo || sig_sha256.hash_algo))
 		handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256,
 					    &msg, signed_commit_mode);
@@ -3084,7 +3090,50 @@ static void parse_new_commit(const char *arg)
 	b->last_commit = object_count_by_type[OBJ_COMMIT];
 }
 
-static void handle_tag_signature(struct strbuf *msg, const char *name)
+static void handle_tag_signature_if_invalid(struct strbuf *buf,
+					    struct strbuf *msg,
+					    size_t sig_offset)
+{
+	struct strbuf signature = STRBUF_INIT;
+	struct strbuf payload = STRBUF_INIT;
+	struct signature_check sigc = { 0 };
+
+	strbuf_addbuf(&payload, buf);
+	strbuf_addch(&payload, '\n');
+	strbuf_add(&payload, msg->buf, sig_offset);
+	strbuf_add(&signature, msg->buf + sig_offset, msg->len - sig_offset);
+
+	sigc.payload_type = SIGNATURE_PAYLOAD_TAG;
+	sigc.payload = strbuf_detach(&payload, &sigc.payload_len);
+
+	if (!check_signature(&sigc, signature.buf, signature.len))
+		goto out;
+
+	if (signed_tag_mode == SIGN_ABORT_IF_INVALID)
+		die(_("aborting due to invalid signature"));
+
+	strbuf_setlen(msg, sig_offset);
+
+	if (signed_tag_mode == SIGN_SIGN_IF_INVALID) {
+		strbuf_attach(&payload, sigc.payload, sigc.payload_len,
+			      sigc.payload_len + 1);
+		sigc.payload = NULL;
+		strbuf_reset(&signature);
+
+		if (sign_buffer(&payload, &signature, signed_tag_keyid,
+				SIGN_BUFFER_USE_DEFAULT_KEY))
+			die(_("failed to sign tag object"));
+
+		strbuf_addbuf(msg, &signature);
+	}
+
+out:
+	signature_check_clear(&sigc);
+	strbuf_release(&signature);
+	strbuf_release(&payload);
+}
+
+static void handle_tag_signature(struct strbuf *buf, struct strbuf *msg, const char *name)
 {
 	size_t sig_offset = parse_signed_buffer(msg->buf, msg->len);
 
@@ -3110,17 +3159,16 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
 		/* Truncate the buffer to remove the signature */
 		strbuf_setlen(msg, sig_offset);
 		break;
+	case SIGN_ABORT_IF_INVALID:
+	case SIGN_SIGN_IF_INVALID:
+	case SIGN_STRIP_IF_INVALID:
+		handle_tag_signature_if_invalid(buf, msg, sig_offset);
+		break;
 
 	/* Third, aborting modes */
 	case SIGN_ABORT:
 		die(_("encountered signed tag; use "
 		      "--signed-tags=<mode> to handle it"));
-	case SIGN_STRIP_IF_INVALID:
-		die(_("'strip-if-invalid' is not a valid mode for "
-		      "git fast-import with --signed-tags=<mode>"));
-	case SIGN_SIGN_IF_INVALID:
-		die(_("'sign-if-invalid' is not a valid mode for "
-		      "git fast-import with --signed-tags=<mode>"));
 	default:
 		BUG("invalid signed_tag_mode value %d from tag '%s'",
 		    signed_tag_mode, name);
@@ -3190,8 +3238,6 @@ static void parse_new_tag(const char *arg)
 	/* tag payload/message */
 	parse_data(&msg, 0, NULL);
 
-	handle_tag_signature(&msg, t->name);
-
 	/* build the tag object */
 	strbuf_reset(&new_data);
 
@@ -3203,6 +3249,9 @@ static void parse_new_tag(const char *arg)
 	if (tagger)
 		strbuf_addf(&new_data,
 			    "tagger %s\n", tagger);
+
+	handle_tag_signature(&new_data, &msg, t->name);
+
 	strbuf_addch(&new_data, '\n');
 	strbuf_addbuf(&new_data, &msg);
 	free(tagger);
@@ -3713,7 +3762,7 @@ static int parse_one_option(const char *option)
 		if (parse_sign_mode(option, &signed_commit_mode, &signed_commit_keyid))
 			usagef(_("unknown --signed-commits mode '%s'"), option);
 	} else if (skip_prefix(option, "signed-tags=", &option)) {
-		if (parse_sign_mode(option, &signed_tag_mode, NULL))
+		if (parse_sign_mode(option, &signed_tag_mode, &signed_tag_keyid))
 			usagef(_("unknown --signed-tags mode '%s'"), option);
 	} else if (!strcmp(option, "quiet")) {
 		show_stats = 0;

gpg-interface.c

@@ -1164,6 +1164,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid)
 		*mode = SIGN_WARN_STRIP;
 	} else if (!strcmp(arg, "strip")) {
 		*mode = SIGN_STRIP;
+	} else if (!strcmp(arg, "abort-if-invalid")) {
+		*mode = SIGN_ABORT_IF_INVALID;
 	} else if (!strcmp(arg, "strip-if-invalid")) {
 		*mode = SIGN_STRIP_IF_INVALID;
 	} else if (!strcmp(arg, "sign-if-invalid")) {

gpg-interface.h

@@ -115,6 +115,7 @@ void print_signature_buffer(const struct signature_check *sigc,
 /* Modes for --signed-tags=<mode> and --signed-commits=<mode> options. */
 enum sign_mode {
 	SIGN_ABORT,
+	SIGN_ABORT_IF_INVALID,
 	SIGN_WARN_VERBATIM,
 	SIGN_VERBATIM,
 	SIGN_WARN_STRIP,

t/t9305-fast-import-signatures.sh

@@ -103,7 +103,7 @@ test_expect_success RUST,GPG 'strip both OpenPGP signatures with --signed-commit
 	test_line_count = 2 out
 '
 
-for mode in strip-if-invalid sign-if-invalid
+for mode in strip-if-invalid sign-if-invalid abort-if-invalid
 do
 	test_expect_success GPG "import commit with no signature with --signed-commits=$mode" '
 		git fast-export main >output &&
@@ -135,6 +135,14 @@ do
 		# corresponding `data <length>` command would have to be changed too.
 		sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
 
+		if test "$mode" = abort-if-invalid
+		then
+			test_must_fail git -C new fast-import --quiet \
+				--signed-commits=$mode <modified >log 2>&1 &&
+			test_grep "aborting due to invalid signature" log &&
+			return 0
+		fi &&
+
 		git -C new fast-import --quiet --signed-commits=$mode <modified >log 2>&1 &&
 
 		IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&

t/t9306-fast-import-signed-tags.sh

@@ -77,4 +77,122 @@ test_expect_success GPGSSH 'import SSH signed tag with --signed-tags=strip' '
 	test_grep ! "SSH SIGNATURE" out
 '
 
+for mode in strip-if-invalid sign-if-invalid abort-if-invalid
+do
+	test_expect_success GPG "import tag with no signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "keep valid OpenPGP signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim openpgp-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
+		test $OPENPGP_SIGNED = $IMPORTED &&
+		git -C import cat-file tag "$IMPORTED" >actual &&
+		test_grep -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPG "handle signature invalidated by message change with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim openpgp-signed >output &&
+
+		# Change the tag message, which invalidates the signature. The tag
+		# message length should not change though, otherwise the corresponding
+		# `data <length>` command would have to be changed too.
+		sed "s/OpenPGP signed tag/OpenPGP forged tag/" output >modified &&
+
+		if test "$mode" = abort-if-invalid
+		then
+			test_must_fail git -C import fast-import --quiet \
+				--signed-tags=$mode <modified >log 2>&1 &&
+			test_grep "aborting due to invalid signature" log &&
+			return 0
+		fi &&
+
+		git -C import fast-import --quiet --signed-tags=$mode <modified >log 2>&1 &&
+
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/openpgp-signed) &&
+		test $OPENPGP_SIGNED != $IMPORTED &&
+		git -C import cat-file tag "$IMPORTED" >actual &&
+
+		if test "$mode" = strip-if-invalid
+		then
+			test_grep ! -E "^-----BEGIN PGP SIGNATURE-----" actual
+		else
+			test_grep -E "^-----BEGIN PGP SIGNATURE-----" actual &&
+			git -C import verify-tag "$IMPORTED"
+		fi &&
+
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSM "keep valid X.509 signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		git fast-export --signed-tags=verbatim x509-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/x509-signed) &&
+		test $X509_SIGNED = $IMPORTED &&
+		git -C import cat-file tag x509-signed >actual &&
+		test_grep -E "^-----BEGIN SIGNED MESSAGE-----" actual &&
+		test_must_be_empty log
+	'
+
+	test_expect_success GPGSSH "keep valid SSH signature with --signed-tags=$mode" '
+		test_when_finished rm -rf import &&
+		git init import &&
+
+		test_config -C import gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+
+		git fast-export --signed-tags=verbatim ssh-signed >output &&
+		git -C import fast-import --quiet --signed-tags=$mode <output >log 2>&1 &&
+		IMPORTED=$(git -C import rev-parse --verify refs/tags/ssh-signed) &&
+		test $SSH_SIGNED = $IMPORTED &&
+		git -C import cat-file tag ssh-signed >actual &&
+		test_grep -E "^-----BEGIN SSH SIGNATURE-----" actual &&
+		test_must_be_empty log
+	'
+done
+
+test_expect_success GPGSSH 'sign invalid tag with explicit keyid' '
+	test_when_finished rm -rf import &&
+	git init import &&
+
+	git fast-export --signed-tags=verbatim ssh-signed >output &&
+
+	# Change the tag message, which invalidates the signature. The tag
+	# message length should not change though, otherwise the corresponding
+	# `data <length>` command would have to be changed too.
+	sed "s/SSH signed tag/SSH forged tag/" output >modified &&
+
+	# Configure the target repository with an invalid default signing key.
+	test_config -C import user.signingkey "not-a-real-key-id" &&
+	test_config -C import gpg.format ssh &&
+	test_config -C import gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
+	test_must_fail git -C import fast-import --quiet \
+		--signed-tags=sign-if-invalid <modified >/dev/null 2>&1 &&
+
+	# Import using explicitly provided signing key.
+	git -C import fast-import --quiet \
+		--signed-tags=sign-if-invalid="${GPGSSH_KEY_PRIMARY}" <modified &&
+
+	IMPORTED=$(git -C import rev-parse --verify refs/tags/ssh-signed) &&
+	test $SSH_SIGNED != $IMPORTED &&
+	git -C import cat-file tag "$IMPORTED" >actual &&
+	test_grep -E "^-----BEGIN SSH SIGNATURE-----" actual &&
+	git -C import verify-tag "$IMPORTED"
+'
+
 test_done