From 38347b2882fb57e83b69dc119df6ff28c409003f Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 10 Mar 2026 15:11:14 -0500 Subject: [PATCH 1/3] commit: remove unused forward declaration In 6206089cbd (commit: write commits for both hashes, 2023-10-01), `sign_with_header()` was removed, but its forward declaration in "commit.h" was left. Remove the unused declaration. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- commit.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/commit.h b/commit.h index f2f39e1a89..e9dee8d26b 100644 --- a/commit.h +++ b/commit.h @@ -400,8 +400,6 @@ LAST_ARG_MUST_BE_NULL int run_commit_hook(int editor_is_used, const char *index_file, int *invoked_hook, const char *name, ...); -/* Sign a commit or tag buffer, storing the result in a header. */ -int sign_with_header(struct strbuf *buf, const char *keyid); /* Parse the signature out of a header. */ int parse_buffer_signed_by_header(const char *buffer, unsigned long size, From 5e004be19974fe2432c82204ee056c7b9b11658f Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 10 Mar 2026 15:11:15 -0500 Subject: [PATCH 2/3] gpg-interface: introduce sign_buffer_with_key() The `sign_commit_to_strbuf()` helper in "commit.c" provides fallback logic to get the default configured signing key when a key is not provided and handles generating the commit signature accordingly. This signing operation is not really specific to commits as any arbitrary buffer can be signed. Also, in a subsequent commit, this same logic is reused by git-fast-import(1) when re-signing invalid commit signatures. Move the `sign_commit_to_strbuf()` helper from "commit.c" to "gpg-interface.c" and rename it to `sign_buffer_with_key()`. Also export this function so it can be used by "commit.c" and "builtin/fast-import.c" in the subsequent commit. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- commit.c | 16 ++-------------- gpg-interface.c | 13 +++++++++++++ gpg-interface.h | 7 +++++++ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/commit.c b/commit.c index 0ffdd6679e..be502a29d9 100644 --- a/commit.c +++ b/commit.c @@ -1170,18 +1170,6 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi return 0; } -static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid) -{ - char *keyid_to_free = NULL; - int ret = 0; - if (!keyid || !*keyid) - keyid = keyid_to_free = get_signing_key(); - if (sign_buffer(buf, sig, keyid)) - ret = -1; - free(keyid_to_free); - return ret; -} - int parse_signed_commit(const struct commit *commit, struct strbuf *payload, struct strbuf *signature, const struct git_hash_algo *algop) @@ -1759,7 +1747,7 @@ int commit_tree_extended(const char *msg, size_t msg_len, oidcpy(&parent_buf[i++], &p->item->object.oid); write_commit_tree(&buffer, msg, msg_len, tree, parent_buf, nparents, author, committer, extra); - if (sign_commit && sign_commit_to_strbuf(&sig, &buffer, sign_commit)) { + if (sign_commit && sign_buffer_with_key(&buffer, &sig, sign_commit)) { result = -1; goto out; } @@ -1791,7 +1779,7 @@ int commit_tree_extended(const char *msg, size_t msg_len, free_commit_extra_headers(compat_extra); free(mapped_parents); - if (sign_commit && sign_commit_to_strbuf(&compat_sig, &compat_buffer, sign_commit)) { + if (sign_commit && sign_buffer_with_key(&compat_buffer, &compat_sig, sign_commit)) { result = -1; goto out; } diff --git a/gpg-interface.c b/gpg-interface.c index 7e6a1520bd..48c30a499c 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -981,6 +981,19 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *sig return use_format->sign_buffer(buffer, signature, signing_key); } +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature, + const char *signing_key) +{ + char *keyid_to_free = NULL; + int ret = 0; + if (!signing_key || !*signing_key) + signing_key = keyid_to_free = get_signing_key(); + if (sign_buffer(buffer, signature, signing_key)) + ret = -1; + free(keyid_to_free); + return ret; +} + /* * Strip CR from the line endings, in case we are on Windows. * NEEDSWORK: make it trim only CRs before LFs and rename diff --git a/gpg-interface.h b/gpg-interface.h index 789d1ffac4..a32741aeda 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -83,6 +83,13 @@ size_t parse_signed_buffer(const char *buf, size_t size); int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const char *signing_key); +/* + * Similar to `sign_buffer()`, but uses the default configured signing key as + * returned by `get_signing_key()` when the provided "signing_key" is NULL or + * empty. Returns 0 on success, non-zero on failure. + */ +int sign_buffer_with_key(struct strbuf *buffer, struct strbuf *signature, + const char *signing_key); /* * Returns corresponding string in lowercase for a given member of From 8849f9497a4ac11791b365a2368ebb96afd039f2 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Tue, 10 Mar 2026 15:11:16 -0500 Subject: [PATCH 3/3] fast-import: add mode to re-sign invalid commit signatures With git-fast-import(1), handling of signed commits is controlled via the `--signed-commits=` option. When an invalid signature is encountered, a user may want the option to re-sign the commit as opposed to just stripping the signature. To facilitate this, introduce a "re-sign-if-invalid" mode for the `--signed-commits` option. Optionally, a key ID may be explicitly provided in the form `re-sign-if-invalid[=]` to specify which signing key should be used when re-signing invalid commit signatures. Note that to properly support interoperability mode when re-signing commit signatures, the commit buffer must be created in both the repository and compatability object formats to generate the appropriate signatures accordingly. As currently implemented, the commit buffer for the compatability object format is not reconstructed and thus re-signing commits in interoperability mode is not yet supported. Support may be added in the future. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- Documentation/git-fast-import.adoc | 4 + builtin/fast-export.c | 8 +- builtin/fast-import.c | 99 ++++++++++++++++----- gpg-interface.c | 23 +++-- gpg-interface.h | 7 +- t/t9305-fast-import-signatures.sh | 134 +++++++++++++++++++---------- 6 files changed, 196 insertions(+), 79 deletions(-) diff --git a/Documentation/git-fast-import.adoc b/Documentation/git-fast-import.adoc index 479c4081da..08f7d5d89a 100644 --- a/Documentation/git-fast-import.adoc +++ b/Documentation/git-fast-import.adoc @@ -86,6 +86,10 @@ already trusted to run their own code. * `strip-if-invalid` will check signatures and, if they are invalid, will strip them and display a warning. The validation is performed in the same way as linkgit:git-verify-commit[1] does it. +* `re-sign-if-invalid[=]`, similar to `strip-if-invalid`, verifies + commit signatures and replaces invalid signatures with newly created ones. + Valid signatures are left unchanged. If `` is provided, that key is + used for re-signing; otherwise the configured default signing key is used. Options for Frontends ~~~~~~~~~~~~~~~~~~~~~ diff --git a/builtin/fast-export.c b/builtin/fast-export.c index 0c5d2386d8..2067613267 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -64,7 +64,7 @@ static int parse_opt_sign_mode(const struct option *opt, if (unset) return 0; - if (parse_sign_mode(arg, val)) + if (parse_sign_mode(arg, val, NULL)) return error(_("unknown %s mode: %s"), opt->long_name, arg); return 0; @@ -825,6 +825,9 @@ static void handle_commit(struct commit *commit, struct rev_info *rev, case SIGN_STRIP_IF_INVALID: die(_("'strip-if-invalid' is not a valid mode for " "git fast-export with --signed-commits=")); + case SIGN_RE_SIGN_IF_INVALID: + die(_("'re-sign-if-invalid' is not a valid mode for " + "git fast-export with --signed-commits=")); default: BUG("invalid signed_commit_mode value %d", signed_commit_mode); } @@ -970,6 +973,9 @@ static void handle_tag(const char *name, struct tag *tag) case SIGN_STRIP_IF_INVALID: die(_("'strip-if-invalid' is not a valid mode for " "git fast-export with --signed-tags=")); + case SIGN_RE_SIGN_IF_INVALID: + die(_("'re-sign-if-invalid' is not a valid mode for " + "git fast-export with --signed-tags=")); default: BUG("invalid signed_commit_mode value %d", signed_commit_mode); } diff --git a/builtin/fast-import.c b/builtin/fast-import.c index b8a7757cfd..472d9ab712 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -190,6 +190,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; /* Memory pools */ static struct mem_pool fi_mem_pool = { @@ -2836,10 +2837,46 @@ static void finalize_commit_buffer(struct strbuf *new_data, strbuf_addbuf(new_data, msg); } -static void handle_strip_if_invalid(struct strbuf *new_data, - struct signature_data *sig_sha1, - struct signature_data *sig_sha256, - struct strbuf *msg) +static void warn_invalid_signature(struct signature_check *check, + const char *msg, enum sign_mode mode) +{ + const char *signer = check->signer ? check->signer : _("unknown"); + const char *subject; + int subject_len = find_commit_subject(msg, &subject); + + switch (mode) { + case SIGN_STRIP_IF_INVALID: + if (subject_len > 100) + warning(_("stripping invalid signature for commit '%.100s...'\n" + " allegedly by %s"), subject, signer); + else if (subject_len > 0) + warning(_("stripping invalid signature for commit '%.*s'\n" + " allegedly by %s"), subject_len, subject, signer); + else + warning(_("stripping invalid signature for commit\n" + " allegedly by %s"), signer); + break; + case SIGN_RE_SIGN_IF_INVALID: + if (subject_len > 100) + warning(_("re-signing invalid signature for commit '%.100s...'\n" + " allegedly by %s"), subject, signer); + else if (subject_len > 0) + warning(_("re-signing invalid signature for commit '%.*s'\n" + " allegedly by %s"), subject_len, subject, signer); + else + warning(_("re-signing invalid signature for commit\n" + " allegedly by %s"), signer); + break; + default: + BUG("unsupported signing mode"); + } +} + +static void handle_signature_if_invalid(struct strbuf *new_data, + struct signature_data *sig_sha1, + struct signature_data *sig_sha256, + struct strbuf *msg, + enum sign_mode mode) { struct strbuf tmp_buf = STRBUF_INIT; struct signature_check signature_check = { 0 }; @@ -2851,20 +2888,34 @@ static void handle_strip_if_invalid(struct strbuf *new_data, ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check); if (ret) { - const char *signer = signature_check.signer ? - signature_check.signer : _("unknown"); - const char *subject; - int subject_len = find_commit_subject(msg->buf, &subject); + warn_invalid_signature(&signature_check, msg->buf, mode); - if (subject_len > 100) - warning(_("stripping invalid signature for commit '%.100s...'\n" - " allegedly by %s"), subject, signer); - else if (subject_len > 0) - warning(_("stripping invalid signature for commit '%.*s'\n" - " allegedly by %s"), subject_len, subject, signer); - else - warning(_("stripping invalid signature for commit\n" - " allegedly by %s"), signer); + if (mode == SIGN_RE_SIGN_IF_INVALID) { + struct strbuf signature = STRBUF_INIT; + struct strbuf payload = STRBUF_INIT; + + /* + * NEEDSWORK: To properly support interoperability mode + * when re-signing commit signatures, the commit buffer + * must be provided in both the repository and + * compatibility object formats. As currently + * implemented, only the repository object format is + * considered meaning compatibility signatures cannot be + * generated. Thus, attempting to re-sign commit + * signatures in interoperability mode is currently + * unsupported. + */ + if (the_repository->compat_hash_algo) + die(_("re-signing signatures in interoperability mode is unsupported")); + + strbuf_addstr(&payload, signature_check.payload); + if (sign_buffer_with_key(&payload, &signature, signed_commit_keyid)) + die(_("failed to sign commit object")); + add_header_signature(new_data, &signature, the_hash_algo); + + strbuf_release(&signature); + strbuf_release(&payload); + } finalize_commit_buffer(new_data, NULL, NULL, msg); } else { @@ -2927,6 +2978,7 @@ static void parse_new_commit(const char *arg) /* fallthru */ case SIGN_VERBATIM: case SIGN_STRIP_IF_INVALID: + case SIGN_RE_SIGN_IF_INVALID: import_one_signature(&sig_sha1, &sig_sha256, v); break; @@ -3011,9 +3063,11 @@ static void parse_new_commit(const char *arg) "encoding %s\n", encoding); - if (signed_commit_mode == SIGN_STRIP_IF_INVALID && + if ((signed_commit_mode == SIGN_STRIP_IF_INVALID || + signed_commit_mode == SIGN_RE_SIGN_IF_INVALID) && (sig_sha1.hash_algo || sig_sha256.hash_algo)) - handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg); + handle_signature_if_invalid(&new_data, &sig_sha1, &sig_sha256, + &msg, signed_commit_mode); else finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg); @@ -3060,6 +3114,9 @@ static void handle_tag_signature(struct strbuf *msg, const char *name) case SIGN_STRIP_IF_INVALID: die(_("'strip-if-invalid' is not a valid mode for " "git fast-import with --signed-tags=")); + case SIGN_RE_SIGN_IF_INVALID: + die(_("'re-sign-if-invalid' is not a valid mode for " + "git fast-import with --signed-tags=")); default: BUG("invalid signed_tag_mode value %d from tag '%s'", signed_tag_mode, name); @@ -3649,10 +3706,10 @@ static int parse_one_option(const char *option) } else if (skip_prefix(option, "export-pack-edges=", &option)) { option_export_pack_edges(option); } else if (skip_prefix(option, "signed-commits=", &option)) { - if (parse_sign_mode(option, &signed_commit_mode)) + 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)) + if (parse_sign_mode(option, &signed_tag_mode, NULL)) usagef(_("unknown --signed-tags mode '%s'"), option); } else if (!strcmp(option, "quiet")) { show_stats = 0; diff --git a/gpg-interface.c b/gpg-interface.c index 48c30a499c..3eea522dda 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -1156,21 +1156,28 @@ out: return ret; } -int parse_sign_mode(const char *arg, enum sign_mode *mode) +int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid) { - if (!strcmp(arg, "abort")) + if (!strcmp(arg, "abort")) { *mode = SIGN_ABORT; - else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) + } else if (!strcmp(arg, "verbatim") || !strcmp(arg, "ignore")) { *mode = SIGN_VERBATIM; - else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) + } else if (!strcmp(arg, "warn-verbatim") || !strcmp(arg, "warn")) { *mode = SIGN_WARN_VERBATIM; - else if (!strcmp(arg, "warn-strip")) + } else if (!strcmp(arg, "warn-strip")) { *mode = SIGN_WARN_STRIP; - else if (!strcmp(arg, "strip")) + } else if (!strcmp(arg, "strip")) { *mode = SIGN_STRIP; - else if (!strcmp(arg, "strip-if-invalid")) + } else if (!strcmp(arg, "strip-if-invalid")) { *mode = SIGN_STRIP_IF_INVALID; - else + } else if (!strcmp(arg, "re-sign-if-invalid")) { + *mode = SIGN_RE_SIGN_IF_INVALID; + } else if (skip_prefix(arg, "re-sign-if-invalid=", &arg)) { + *mode = SIGN_RE_SIGN_IF_INVALID; + if (keyid) + *keyid = arg; + } else { return -1; + } return 0; } diff --git a/gpg-interface.h b/gpg-interface.h index a32741aeda..e9f451f366 100644 --- a/gpg-interface.h +++ b/gpg-interface.h @@ -119,12 +119,15 @@ enum sign_mode { SIGN_WARN_STRIP, SIGN_STRIP, SIGN_STRIP_IF_INVALID, + SIGN_RE_SIGN_IF_INVALID, }; /* * Return 0 if `arg` can be parsed into an `enum sign_mode`. Return -1 - * otherwise. + * otherwise. If the parsed mode is SIGN_RE_SIGN_IF_INVALID and GPG key provided + * in the arguments in the form `re-sign-if-invalid=`, the key-ID is + * parsed into `char **keyid`. */ -int parse_sign_mode(const char *arg, enum sign_mode *mode); +int parse_sign_mode(const char *arg, enum sign_mode *mode, const char **keyid); #endif diff --git a/t/t9305-fast-import-signatures.sh b/t/t9305-fast-import-signatures.sh index 022dae02e4..2a3f04b42d 100755 --- a/t/t9305-fast-import-signatures.sh +++ b/t/t9305-fast-import-signatures.sh @@ -103,26 +103,85 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war test_line_count = 2 out ' -test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' ' - git fast-export main >output && - git -C new fast-import --quiet --signed-commits=strip-if-invalid log 2>&1 && - test_must_be_empty log -' +for mode in strip-if-invalid re-sign-if-invalid +do + test_expect_success GPG "import commit with no signature with --signed-commits=$mode" ' + git fast-export main >output && + git -C new fast-import --quiet --signed-commits=$mode log 2>&1 && + test_must_be_empty log + ' -test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' ' - rm -rf new && - git init new && + test_expect_success GPG "keep valid OpenPGP signature with --signed-commits=$mode" ' + rm -rf new && + git init new && - git fast-export --signed-commits=verbatim openpgp-signing >output && - git -C new fast-import --quiet --signed-commits=strip-if-invalid log 2>&1 && - IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) && - test $OPENPGP_SIGNING = $IMPORTED && - git -C new cat-file commit "$IMPORTED" >actual && - test_grep -E "^gpgsig(-sha256)? " actual && - test_must_be_empty log -' + git fast-export --signed-commits=verbatim openpgp-signing >output && + git -C new fast-import --quiet --signed-commits=$mode log 2>&1 && + IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) && + test $OPENPGP_SIGNING = $IMPORTED && + git -C new cat-file commit "$IMPORTED" >actual && + test_grep -E "^gpgsig(-sha256)? " actual && + test_must_be_empty log + ' -test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' ' + test_expect_success GPG "handle signature invalidated by message change with --signed-commits=$mode" ' + rm -rf new && + git init new && + + git fast-export --signed-commits=verbatim openpgp-signing >output && + + # Change the commit message, which invalidates the signature. + # The commit message length should not change though, otherwise the + # corresponding `data ` command would have to be changed too. + sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified && + + git -C new fast-import --quiet --signed-commits=$mode log 2>&1 && + + IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) && + test $OPENPGP_SIGNING != $IMPORTED && + git -C new cat-file commit "$IMPORTED" >actual && + + if test "$mode" = strip-if-invalid + then + test_grep "stripping invalid signature" log && + test_grep ! -E "^gpgsig" actual + else + test_grep "re-signing invalid signature" log && + test_grep -E "^gpgsig(-sha256)? " actual && + git -C new verify-commit "$IMPORTED" + fi + ' + + test_expect_success GPGSM "keep valid X.509 signature with --signed-commits=$mode" ' + rm -rf new && + git init new && + + git fast-export --signed-commits=verbatim x509-signing >output && + git -C new fast-import --quiet --signed-commits=$mode log 2>&1 && + IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) && + test $X509_SIGNING = $IMPORTED && + git -C new cat-file commit "$IMPORTED" >actual && + test_grep -E "^gpgsig(-sha256)? " actual && + test_must_be_empty log + ' + + test_expect_success GPGSSH "keep valid SSH signature with --signed-commits=$mode" ' + rm -rf new && + git init new && + + test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && + + git fast-export --signed-commits=verbatim ssh-signing >output && + git -C new fast-import --quiet --signed-commits=$mode log 2>&1 && + IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) && + test $SSH_SIGNING = $IMPORTED && + git -C new cat-file commit "$IMPORTED" >actual && + test_grep -E "^gpgsig(-sha256)? " actual && + test_must_be_empty log + ' +done + +test_expect_success GPGSSH "re-sign invalid commit with explicit keyid" ' rm -rf new && git init new && @@ -133,41 +192,22 @@ test_expect_success GPG 'strip signature invalidated by message change with --si # corresponding `data ` command would have to be changed too. sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified && - git -C new fast-import --quiet --signed-commits=strip-if-invalid log 2>&1 && + # Configure the target repository with an invalid default signing key. + test_config -C new user.signingkey "not-a-real-key-id" && + test_config -C new gpg.format ssh && + test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && + test_must_fail git -C new fast-import --quiet \ + --signed-commits=re-sign-if-invalid /dev/null 2>&1 && + + # Import using explicitly provided signing key. + git -C new fast-import --quiet \ + --signed-commits=re-sign-if-invalid="${GPGSSH_KEY_PRIMARY}" actual && - test_grep ! -E "^gpgsig" actual && - test_grep "stripping invalid signature" log -' - -test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' ' - rm -rf new && - git init new && - - git fast-export --signed-commits=verbatim x509-signing >output && - git -C new fast-import --quiet --signed-commits=strip-if-invalid log 2>&1 && - IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) && - test $X509_SIGNING = $IMPORTED && - git -C new cat-file commit "$IMPORTED" >actual && test_grep -E "^gpgsig(-sha256)? " actual && - test_must_be_empty log -' - -test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' ' - rm -rf new && - git init new && - - test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && - - git fast-export --signed-commits=verbatim ssh-signing >output && - git -C new fast-import --quiet --signed-commits=strip-if-invalid log 2>&1 && - IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) && - test $SSH_SIGNING = $IMPORTED && - git -C new cat-file commit "$IMPORTED" >actual && - test_grep -E "^gpgsig(-sha256)? " actual && - test_must_be_empty log + git -C new verify-commit "$IMPORTED" ' test_done