Skip to content

Commit ca2ba7c

Browse files
chriscoolgitster
chriscool
authored and
gitster
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. 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. 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. While at it, let's also mark for translation some error messages linked to the `--signed-commits=<mode>` and `--signed-tags=<mode>` in `git fast-export`. [1]: https://github.com/newren/git-filter-repo Signed-off-by: Christian Couder <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 500bafb commit ca2ba7c

File tree

6 files changed

+226
-28
lines changed

6 files changed

+226
-28
lines changed

‎Documentation/git-fast-import.adoc‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,25 @@ 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.
7888

7989
Options for Frontends
8090
~~~~~~~~~~~~~~~~~~~~~

‎builtin/fast-export.c‎

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -797,12 +797,10 @@ 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:
805-
warning("exporting %"PRIuMAX" signature(s) for commit %s",
803+
warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
806804
(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
807805
/* fallthru */
808806
case SIGN_VERBATIM:
@@ -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:
815-
warning("stripping signature(s) from commit %s",
815+
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
}
@@ -934,23 +945,34 @@ static void handle_tag(const char *name, struct tag *tag)
934945
size_t sig_offset = parse_signed_buffer(message, message_size);
935946
if (sig_offset < message_size)
936947
switch (signed_tag_mode) {
937-
case SIGN_ABORT:
938-
die("encountered signed tag %s; use "
939-
"--signed-tags=<mode> to handle it",
940-
oid_to_hex(&tag->object.oid));
948+
949+
/* Exporting modes */
941950
case SIGN_WARN_VERBATIM:
942-
warning("exporting signed tag %s",
951+
warning(_("exporting signed tag %s"),
943952
oid_to_hex(&tag->object.oid));
944953
/* fallthru */
945954
case SIGN_VERBATIM:
946955
break;
956+
957+
/* Stripping modes */
947958
case SIGN_WARN_STRIP:
948-
warning("stripping signature from tag %s",
959+
warning(_("stripping signature from tag %s"),
949960
oid_to_hex(&tag->object.oid));
950961
/* fallthru */
951962
case SIGN_STRIP:
952963
message_size = sig_offset;
953964
break;
965+
966+
/* Aborting modes */
967+
case SIGN_ABORT:
968+
die(_("encountered signed tag %s; use "
969+
"--signed-tags=<mode> to handle it"),
970+
oid_to_hex(&tag->object.oid));
971+
case SIGN_STRIP_IF_INVALID:
972+
die(_("'strip-if-invalid' is not a valid mode for "
973+
"git fast-export with --signed-tags=<mode>"));
974+
default:
975+
BUG("invalid signed_commit_mode value %d", signed_commit_mode);
954976
}
955977
}
956978

‎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: 117 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,120 @@ 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 GPG 'keep valid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
146+
rm -rf new &&
147+
git init new &&
148+
149+
git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
150+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
151+
152+
git -C new cat-file commit refs/heads/dual-signed >actual &&
153+
test_grep -E "^gpgsig " actual &&
154+
test_grep -E "^gpgsig-sha256 " actual &&
155+
test_must_be_empty log &&
156+
157+
IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
158+
if test "$GIT_DEFAULT_HASH" = "sha1"
159+
then
160+
test $SHA1_B = $IMPORTED
161+
else
162+
test $SHA256_B = $IMPORTED
163+
fi
164+
'
165+
166+
test_expect_success GPG 'strip both invalid dual OpenPGP signatures with --signed-commits=strip-if-invalid' '
167+
rm -rf new &&
168+
git init new &&
169+
170+
git -C explicit-sha256 fast-export --signed-commits=verbatim dual-signed >output &&
171+
172+
# Change the commit message, which invalidates the signature.
173+
# The commit message length should not change though, otherwise the
174+
# corresponding `data <length>` command would have to be changed too.
175+
sed "s/signed commit/forged commit/" output >modified &&
176+
177+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <modified >log 2>&1 &&
178+
179+
git -C new cat-file commit refs/heads/dual-signed >actual &&
180+
test_grep ! -E "^gpgsig " actual &&
181+
test_grep ! -E "^gpgsig-sha256 " actual &&
182+
183+
IMPORTED=$(git -C new rev-parse refs/heads/dual-signed) &&
184+
if test "$GIT_DEFAULT_HASH" = "sha1"
185+
then
186+
test $SHA1_B != $IMPORTED
187+
else
188+
test $SHA256_B != $IMPORTED
189+
fi &&
190+
191+
test_grep "stripping invalid signature" log
192+
'
193+
194+
test_expect_success GPGSM 'keep valid X.509 signature with --signed-commits=strip-if-invalid' '
195+
rm -rf new &&
196+
git init new &&
197+
198+
git fast-export --signed-commits=verbatim x509-signing >output &&
199+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
200+
IMPORTED=$(git -C new rev-parse --verify refs/heads/x509-signing) &&
201+
test $X509_SIGNING = $IMPORTED &&
202+
git -C new cat-file commit "$IMPORTED" >actual &&
203+
test_grep -E "^gpgsig(-sha256)? " actual &&
204+
test_must_be_empty log
205+
'
206+
207+
test_expect_success GPGSSH 'keep valid SSH signature with --signed-commits=strip-if-invalid' '
208+
rm -rf new &&
209+
git init new &&
210+
211+
test_config -C new gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" &&
212+
213+
git fast-export --signed-commits=verbatim ssh-signing >output &&
214+
git -C new fast-import --quiet --signed-commits=strip-if-invalid <output >log 2>&1 &&
215+
IMPORTED=$(git -C new rev-parse --verify refs/heads/ssh-signing) &&
216+
test $SSH_SIGNING = $IMPORTED &&
217+
git -C new cat-file commit "$IMPORTED" >actual &&
218+
test_grep -E "^gpgsig(-sha256)? " actual &&
219+
test_must_be_empty log
220+
'
221+
106222
test_done

0 commit comments

Comments
 (0)