Skip to content

Commit 2e324a8

Browse files
gitstergitster
committed
Merge branch 'cc/fast-import-strip-if-invalid' into seen
"git fast-import" learns "--strip-if-invalid" option to drop invalid cryptographic signature from objects. Comments? * cc/fast-import-strip-if-invalid: fast-import: add 'strip-if-invalid' mode to --signed-commits=<mode> commit: refactor verify_commit_buffer() fast-import: refactor finalize_commit_buffer()
2 parents c1fc5d3 + 6c723ad commit 2e324a8

File tree

8 files changed

+206
-29
lines changed

8 files changed

+206
-29
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: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -797,10 +797,7 @@ 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+
/* Exporting modes */
804801
case SIGN_WARN_VERBATIM:
805802
warning(_("exporting %"PRIuMAX" signature(s) for commit %s"),
806803
(uintmax_t)signatures.nr, oid_to_hex(&commit->object.oid));
@@ -811,12 +808,25 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,
811808
print_signature(item->string, item->util);
812809
}
813810
break;
811+
812+
/* Stripping modes */
814813
case SIGN_WARN_STRIP:
815814
warning(_("stripping signature(s) from commit %s"),
816815
oid_to_hex(&commit->object.oid));
817816
/* fallthru */
818817
case SIGN_STRIP:
819818
break;
819+
820+
/* Aborting modes */
821+
case SIGN_ABORT:
822+
die(_("encountered signed commit %s; use "
823+
"--signed-commits=<mode> to handle it"),
824+
oid_to_hex(&commit->object.oid));
825+
case SIGN_STRIP_IF_INVALID:
826+
die(_("'strip-if-invalid' is not a valid mode for "
827+
"git fast-export with --signed-commits=<mode>"));
828+
default:
829+
BUG("invalid signed_commit_mode value %d", signed_commit_mode);
820830
}
821831
string_list_clear(&signatures, 0);
822832
}
@@ -935,23 +945,33 @@ static void handle_tag(const char *name, struct tag *tag)
935945
size_t sig_offset = parse_signed_buffer(message, message_size);
936946
if (sig_offset < message_size)
937947
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));
948+
/* Exporting modes */
942949
case SIGN_WARN_VERBATIM:
943950
warning(_("exporting signed tag %s"),
944951
oid_to_hex(&tag->object.oid));
945952
/* fallthru */
946953
case SIGN_VERBATIM:
947954
break;
955+
956+
/* Stripping modes */
948957
case SIGN_WARN_STRIP:
949958
warning(_("stripping signature from tag %s"),
950959
oid_to_hex(&tag->object.oid));
951960
/* fallthru */
952961
case SIGN_STRIP:
953962
message_size = sig_offset;
954963
break;
964+
965+
/* Aborting modes */
966+
case SIGN_ABORT:
967+
die(_("encountered signed tag %s; use "
968+
"--signed-tags=<mode> to handle it"),
969+
oid_to_hex(&tag->object.oid));
970+
case SIGN_STRIP_IF_INVALID:
971+
die(_("'strip-if-invalid' is not a valid mode for "
972+
"git fast-export with --signed-tags=<mode>"));
973+
default:
974+
BUG("invalid signed_commit_mode value %d", signed_commit_mode);
955975
}
956976
}
957977

‎builtin/fast-import.c‎

Lines changed: 65 additions & 9 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);
@@ -2815,6 +2815,57 @@ static void import_one_signature(struct signature_data *sig_sha1,
28152815
die(_("parse_one_signature() returned unknown hash algo"));
28162816
}
28172817

2818+
static void finalize_commit_buffer(struct strbuf *new_data,
2819+
struct signature_data *sig_sha1,
2820+
struct signature_data *sig_sha256,
2821+
struct strbuf *msg)
2822+
{
2823+
add_gpgsig_to_commit(new_data, "gpgsig ", sig_sha1);
2824+
add_gpgsig_to_commit(new_data, "gpgsig-sha256 ", sig_sha256);
2825+
2826+
strbuf_addch(new_data, '\n');
2827+
strbuf_addbuf(new_data, msg);
2828+
}
2829+
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+
28182869
static void parse_new_commit(const char *arg)
28192870
{
28202871
static struct strbuf msg = STRBUF_INIT;
@@ -2866,6 +2917,7 @@ static void parse_new_commit(const char *arg)
28662917
warning(_("importing a commit signature verbatim"));
28672918
/* fallthru */
28682919
case SIGN_VERBATIM:
2920+
case SIGN_STRIP_IF_INVALID:
28692921
import_one_signature(&sig_sha1, &sig_sha256, v);
28702922
break;
28712923

@@ -2950,11 +3002,12 @@ static void parse_new_commit(const char *arg)
29503002
"encoding %s\n",
29513003
encoding);
29523004

2953-
add_gpgsig_to_commit(&new_data, "gpgsig ", &sig_sha1);
2954-
add_gpgsig_to_commit(&new_data, "gpgsig-sha256 ", &sig_sha256);
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);
29553010

2956-
strbuf_addch(&new_data, '\n');
2957-
strbuf_addbuf(&new_data, &msg);
29583011
free(author);
29593012
free(committer);
29603013
free(encoding);
@@ -2975,9 +3028,6 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
29753028
switch (signed_tag_mode) {
29763029

29773030
/* First, modes that don't change anything */
2978-
case SIGN_ABORT:
2979-
die(_("encountered signed tag; use "
2980-
"--signed-tags=<mode> to handle it"));
29813031
case SIGN_WARN_VERBATIM:
29823032
warning(_("importing a tag signature verbatim for tag '%s'"), name);
29833033
/* fallthru */
@@ -2994,7 +3044,13 @@ static void handle_tag_signature(struct strbuf *msg, const char *name)
29943044
strbuf_setlen(msg, sig_offset);
29953045
break;
29963046

2997-
/* 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>"));
29983054
default:
29993055
BUG("invalid signed_tag_mode value %d from tag '%s'",
30003056
signed_tag_mode, name);

‎commit.c‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,15 +1315,17 @@ static void handle_signed_tag(const struct commit *parent, struct commit_extra_h
13151315
free(buf);
13161316
}
13171317

1318-
int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
1318+
int verify_commit_buffer(const char *buffer, size_t size,
1319+
struct signature_check *sigc)
13191320
{
13201321
struct strbuf payload = STRBUF_INIT;
13211322
struct strbuf signature = STRBUF_INIT;
13221323
int ret = 1;
13231324

13241325
sigc->result = 'N';
13251326

1326-
if (parse_signed_commit(commit, &payload, &signature, the_hash_algo) <= 0)
1327+
if (parse_buffer_signed_by_header(buffer, size, &payload,
1328+
&signature, the_hash_algo) <= 0)
13271329
goto out;
13281330

13291331
sigc->payload_type = SIGNATURE_PAYLOAD_COMMIT;
@@ -1337,6 +1339,17 @@ int check_commit_signature(const struct commit *commit, struct signature_check *
13371339
return ret;
13381340
}
13391341

1342+
int check_commit_signature(const struct commit *commit, struct signature_check *sigc)
1343+
{
1344+
unsigned long size;
1345+
const char *buffer = repo_get_commit_buffer(the_repository, commit, &size);
1346+
int ret = verify_commit_buffer(buffer, size, sigc);
1347+
1348+
repo_unuse_commit_buffer(the_repository, commit, buffer);
1349+
1350+
return ret;
1351+
}
1352+
13401353
void verify_merge_signature(struct commit *commit, int verbosity,
13411354
int check_trust)
13421355
{

‎commit.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,13 @@ int remove_signature(struct strbuf *buf);
333333
*/
334334
int check_commit_signature(const struct commit *commit, struct signature_check *sigc);
335335

336+
/*
337+
* Same as check_commit_signature() but accepts a commit buffer and
338+
* its size, instead of a `struct commit *`.
339+
*/
340+
int verify_commit_buffer(const char *buffer, size_t size,
341+
struct signature_check *sigc);
342+
336343
/* record author-date for each commit object */
337344
struct author_date_slab;
338345
void record_author_date(struct author_date_slab *author_date,

‎gpg-interface.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,6 +1154,8 @@ int parse_sign_mode(const char *arg, enum sign_mode *mode)
11541154
*mode = SIGN_WARN_STRIP;
11551155
else if (!strcmp(arg, "strip"))
11561156
*mode = SIGN_STRIP;
1157+
else if (!strcmp(arg, "strip-if-invalid"))
1158+
*mode = SIGN_STRIP_IF_INVALID;
11571159
else
11581160
return -1;
11591161
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)