Skip to content

Commit e325533

Browse files
chriscoolchriscool
committed
fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode>
Tools like `git filter-repo`[1] use `git fast-export` and `git fast-import` to rewrite repository history. When rewriting history using one such tool though, commit signatures might become invalid because the commits they sign changed due to the changes in the repository history made by the tool between the fast-export and the fast-import steps. Note that as far as signature handling goes: * Since fast-export doesn't know what changes filter-repo may make to the stream, it can't know whether the signatures will still be valid. * Since filter-repo doesn't know what history canonicalizations fast-export performed (and it performs a few), it can't know whether the signatures will still be valid. * Therefore, fast-import is the only process in the pipeline that can know whether a specified signature remains valid. Having invalid signatures in a rewritten repository could be confusing, so users rewritting history might prefer to simply discard signatures that are invalid at the fast-import step. For example a common use case is to rewrite only "recent" history. While specifying commit ranges corresponding to "recent" commits could work, users worry about getting it wrong and want to just automatically rewrite everything, expecting older commit signatures to be untouched. To let them do that, let's add a new 'strip-if-invalid' mode to the `--signed-commits=<mode>` option of `git fast-import`. It would be interesting for the `--signed-tags=<mode>` option to have this mode too, but we leave that for a future improvement. It might also be possible for `git fast-export` to have such a mode in its `--signed-commits=<mode>` and `--signed-tags=<mode>` options, but the use cases for it are much less clear, so we also leave that for possible future improvements. For now let's just die() if 'strip-if-invalid' is passed to these options where it hasn't been implemented yet. [1]: https://github.com/newren/git-filter-repo Helped-by: Elijah Newren <[email protected]> Signed-off-by: Christian Couder <[email protected]>
1 parent d22b753 commit e325533

File tree

6 files changed

+174
-24
lines changed

6 files changed

+174
-24
lines changed

‎Documentation/git-fast-import.adoc‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,26 @@ fast-import stream! This option is enabled automatically for
6666
remote-helpers that use the `import` capability, as they are
6767
already trusted to run their own code.
6868

69-
--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::
70-
Specify how to handle signed tags. Behaves in the same way
71-
as the same option in linkgit:git-fast-export[1], except that
72-
default is 'verbatim' (instead of 'abort').
73-
74-
--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::
75-
Specify how to handle signed commits. Behaves in the same way
76-
as the same option in linkgit:git-fast-export[1], except that
77-
default is 'verbatim' (instead of 'abort').
69+
`--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)`::
70+
Specify how to handle signed tags. Behaves in the same way as
71+
the `--signed-commits=<mode>` below, except that the
72+
`strip-if-invalid` mode is not yet supported. Like for signed
73+
commits, the default mode is `verbatim`.
74+
75+
`--signed-commits=<mode>`::
76+
Specify how to handle signed commits. The following <mode>s
77+
are supported:
78+
+
79+
* `verbatim`, which is the default, will silently import commit
80+
signatures.
81+
* `warn-verbatim` will import them, but will display a warning.
82+
* `abort` will make this program die when encountering a signed
83+
commit.
84+
* `strip` will silently make the commits unsigned.
85+
* `warn-strip` will make them unsigned, but will display a warning.
86+
* `strip-if-invalid` will check signatures and, if they are invalid,
87+
will strip them and display a warning. The validation is performed
88+
in the same way as linkgit:git-verify-commit[1] does it.
7889

7990
Options for Frontends
8091
~~~~~~~~~~~~~~~~~~~~~

‎builtin/fast-export.c‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -797,10 +797,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
797797
(int)(committer_end - committer), committer);
798798
if (signatures.nr) {
799799
switch (signed_commit_mode) {
800-
case SIGN_ABORT:
801-
die(_("encountered signed commit %s; use "
802-
"--signed-commits=<mode> to handle it"),
803-
oid_to_hex(&commit->object.oid));
800+
801+
/* Exporting modes */
804802
case SIGN_WARN_VERBATIM:
805803
warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
806804
(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
@@ -811,12 +809,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
811809
print_signature(item->string, item->util);
812810
}
813811
break;
812+
813+
/* Stripping modes */
814814
case SIGN_WARN_STRIP:
815815
warning(_("stripping signature(s) from commit %s"),
816816
oid_to_hex(&commit->object.oid));
817817
/* fallthru */
818818
case SIGN_STRIP:
819819
break;
820+
821+
/* Aborting modes */
822+
case SIGN_ABORT:
823+
die(_("encountered signed commit %s; use "
824+
"--signed-commits=<mode> to handle it"),
825+
oid_to_hex(&commit->object.oid));
826+
case SIGN_STRIP_IF_INVALID:
827+
die(_("'strip-if-invalid' is not a valid mode for "
828+
"git fast-export with --signed-commits=<mode>"));
829+
default:
830+
BUG("invalid signed_commit_mode value %d", signed_commit_mode);
820831
}
821832
string_list_clear(&signatures, 0);
822833
}
@@ -935,23 +946,34 @@ static void handle_tag(const char *name, struct tag *tag)
935946
size_t sig_offset = parse_signed_buffer(message, message_size);
936947
if (sig_offset < message_size)
937948
switch (signed_tag_mode) {
938-
case SIGN_ABORT:
939-
die(_("encountered signed tag %s; use "
940-
"--signed-tags=<mode> to handle it"),
941-
oid_to_hex(&tag->object.oid));
949+
950+
/* Exporting modes */
942951
case SIGN_WARN_VERBATIM:
943952
warning(_("exporting signed tag %s"),
944953
oid_to_hex(&tag->object.oid));
945954
/* fallthru */
946955
case SIGN_VERBATIM:
947956
break;
957+
958+
/* Stripping modes */
948959
case SIGN_WARN_STRIP:
949960
warning(_("stripping signature from tag %s"),
950961
oid_to_hex(&tag->object.oid));
951962
/* fallthru */
952963
case SIGN_STRIP:
953964
message_size = sig_offset;
954965
break;
966+
967+
/* Aborting modes */
968+
case SIGN_ABORT:
969+
die(_("encountered signed tag %s; use "
970+
"--signed-tags=<mode> to handle it"),
971+
oid_to_hex(&tag->object.oid));
972+
case SIGN_STRIP_IF_INVALID:
973+
die(_("'strip-if-invalid' is not a valid mode for "
974+
"git fast-export with --signed-tags=<mode>"));
975+
default:
976+
BUG("invalid signed_commit_mode value %d", signed_commit_mode);
955977
}
956978
}
957979

‎builtin/fast-import.c‎

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,7 +2772,7 @@ static void add_gpgsig_to_commit(struct strbuf *commit_data,
27722772
{
27732773
struct string_list siglines = STRING_LIST_INIT_NODUP;
27742774

2775-
if (!sig->hash_algo)
2775+
if (!sig || !sig->hash_algo)
27762776
return;
27772777

27782778
strbuf_addstr(commit_data, header);
@@ -2827,6 +2827,45 @@ static void finalize_commit_buffer(struct strbuf *new_data,
28272827
strbuf_addbuf(new_data, msg);
28282828
}
28292829

2830+
static void handle_strip_if_invalid(struct strbuf *new_data,
2831+
struct signature_data *sig_sha1,
2832+
struct signature_data *sig_sha256,
2833+
struct strbuf *msg)
2834+
{
2835+
struct strbuf tmp_buf = STRBUF_INIT;
2836+
struct signature_check signature_check = { 0 };
2837+
int ret;
2838+
2839+
/* Check signature in a temporary commit buffer */
2840+
strbuf_addbuf(&tmp_buf, new_data);
2841+
finalize_commit_buffer(&tmp_buf, sig_sha1, sig_sha256, msg);
2842+
ret = verify_commit_buffer(tmp_buf.buf, tmp_buf.len, &signature_check);
2843+
2844+
if (ret) {
2845+
const char *signer = signature_check.signer ?
2846+
signature_check.signer : _("unknown");
2847+
const char *subject;
2848+
int subject_len = find_commit_subject(msg->buf, &subject);
2849+
2850+
if (subject_len > 100)
2851+
warning(_("stripping invalid signature for commit '%.100s...'\n"
2852+
" allegedly by %s"), subject, signer);
2853+
else if (subject_len > 0)
2854+
warning(_("stripping invalid signature for commit '%.*s'\n"
2855+
" allegedly by %s"), subject_len, subject, signer);
2856+
else
2857+
warning(_("stripping invalid signature for commit\n"
2858+
" allegedly by %s"), signer);
2859+
2860+
finalize_commit_buffer(new_data, NULL, NULL, msg);
2861+
} else {
2862+
strbuf_swap(new_data, &tmp_buf);
2863+
}
2864+
2865+
signature_check_clear(&signature_check);
2866+
strbuf_release(&tmp_buf);
2867+
}
2868+
28302869
static void parse_new_commit(const char *arg)
28312870
{
28322871
static struct strbuf msg = STRBUF_INIT;
@@ -2878,6 +2917,7 @@ static void parse_new_commit(const char *arg)
28782917
warning(_("importing a commit signature verbatim"));
28792918
/* fallthru */
28802919
case SIGN_VERBATIM:
2920+
case SIGN_STRIP_IF_INVALID:
28812921
import_one_signature(&sig_sha1, &sig_sha256, v);
28822922
break;
28832923

@@ -2962,7 +3002,11 @@ static void parse_new_commit(const char *arg)
29623002
"encoding %s\n",
29633003
encoding);
29643004

2965-
finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
3005+
if (signed_commit_mode == SIGN_STRIP_IF_INVALID &&
3006+
(sig_sha1.hash_algo || sig_sha256.hash_algo))
3007+
handle_strip_if_invalid(&new_data, &sig_sha1, &sig_sha256, &msg);
3008+
else
3009+
finalize_commit_buffer(&new_data, &sig_sha1, &sig_sha256, &msg);
29663010

29673011
free(author);
29683012
free(committer);
@@ -2984,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
29843028
switch (signed_tag_mode) {
29853029

29863030
/* First, modes that don't change anything */
2987-
case SIGN_ABORT:
2988-
die(_("encountered signed tag; use "
2989-
"--signed-tags=<mode> to handle it"));
29903031
case SIGN_WARN_VERBATIM:
29913032
warning(_("importing a tag signature verbatim for tag '%s'"), name);
29923033
/* fallthru */
@@ -3003,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
30033044
strbuf_setlen(msg, sig_offset);
30043045
break;
30053046

3006-
/* Third, BUG */
3047+
/* Third, aborting modes */
3048+
case SIGN_ABORT:
3049+
die(_("encountered signed tag; use "
3050+
"--signed-tags=<mode> to handle it"));
3051+
case SIGN_STRIP_IF_INVALID:
3052+
die(_("'strip-if-invalid' is not a valid mode for "
3053+
"git fast-import with --signed-tags=<mode>"));
30073054
default:
30083055
BUG("invalid signed_tag_mode value %d from tag '%s'",
30093056
signed_tag_mode, name);

‎gpg-interface.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,6 +1146,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
11461146
*mode = SIGN_WARN_STRIP;
11471147
else if (!strcmp(arg, "strip"))
11481148
*mode = SIGN_STRIP;
1149+
else if (!strcmp(arg, "strip-if-invalid"))
1150+
*mode = SIGN_STRIP_IF_INVALID;
11491151
else
11501152
return -1;
11511153
return 0;

‎gpg-interface.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ enum sign_mode {
111111
SIGN_VERBATIM,
112112
SIGN_WARN_STRIP,
113113
SIGN_STRIP,
114+
SIGN_STRIP_IF_INVALID,
114115
};
115116

116117
/*

‎t/t9305-fast-import-signatures.sh‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ test_expect_success GPG 'setup a commit with dual OpenPGP signatures on its SHA-
7979
echo B >explicit-sha256/B &&
8080
git -C explicit-sha256 add B &&
8181
test_tick &&
82-
git -C explicit-sha256 commit -S -m "signed" B &&
82+
git -C explicit-sha256 commit -S -m "signed commit" B &&
8383
SHA256_B=$(git -C explicit-sha256 rev-parse dual-signed) &&
8484
8585
# Create the corresponding SHA-1 commit
@@ -103,4 +103,71 @@ test_expect_success GPG 'strip both OpenPGP signatures with --signed-commits=war
103103
test_line_count = 2 out
104104
'
105105

106+
test_expect_success GPG 'import commit with no signature with --signed-commits=strip-if-invalid' '
107+
git fast-export main >output &&
108+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
109+
test_must_be_empty log
110+
'
111+
112+
test_expect_success GPG 'keep valid OpenPGP signature with --signed-commits=strip-if-invalid' '
113+
rm -rf new &&
114+
git init new &&
115+
116+
git fast-export --signed-commits=verbatim openpgp-signing >output &&
117+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
118+
IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
119+
test $OPENPGP_SIGNING = $IMPORTED &&
120+
git -C new cat-file commit "$IMPORTED" >actual &&
121+
test_grep -E "^gpgsig(-sha256)? " actual &&
122+
test_must_be_empty log
123+
'
124+
125+
test_expect_success GPG 'strip signature invalidated by message change with --signed-commits=strip-if-invalid' '
126+
rm -rf new &&
127+
git init new &&
128+
129+
git fast-export --signed-commits=verbatim openpgp-signing >output &&
130+
131+
# Change the commit message, which invalidates the signature.
132+
# The commit message length should not change though, otherwise the
133+
# corresponding `data <length>` command would have to be changed too.
134+
sed "s/OpenPGP signed commit/OpenPGP forged commit/" output >modified &&
135+
136+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
137+
138+
IMPORTED=$(git -C new rev-parse --verify refs/heads/openpgp-signing) &&
139+
test $OPENPGP_SIGNING != $IMPORTED &&
140+
git -C new cat-file commit "$IMPORTED" >actual &&
141+
test_grep ! -E "^gpgsig" actual &&
142+
test_grep "stripping invalid signature" log
143+
'
144+
145+
test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
146+
rm -rf new &&
147+
git init new &&
148+
149+
git fast-export --signed-commits=verbatim x509-signing >output &&
150+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
151+
IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
152+
test $X509_SIGNING = $IMPORTED &&
153+
git -C new cat-file commit "$IMPORTED" >actual &&
154+
test_grep -E "^gpgsig(-sha256)? " actual &&
155+
test_must_be_empty log
156+
'
157+
158+
test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
159+
rm -rf new &&
160+
git init new &&
161+
162+
test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
163+
164+
git fast-export --signed-commits=verbatim ssh-signing >output &&
165+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
166+
IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
167+
test $SSH_SIGNING = $IMPORTED &&
168+
git -C new cat-file commit "$IMPORTED" >actual &&
169+
test_grep -E "^gpgsig(-sha256)? " actual &&
170+
test_must_be_empty log
171+
'
172+
106173
test_done

0 commit comments

Comments
 (0)