{"thread":{"id":"55522","subject":"[PATCH 1/3] git-fast-import.txt: add missing LF in the BNF","startedAt":"2021-04-19T23:04:39Z","lastAt":"2021-04-21T22:07:22Z","messageCount":15,"participants":["Luke Shumaker","Taylor Blau","brian m. carlson","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"422348","messageId":"20210419225441.3139048-2-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"20210419225441.3139048-1-lukeshu@lukeshu.com","subject":"[PATCH 1/3] git-fast-import.txt: add missing LF in the BNF","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-19T22:54:39Z","receivedAt":"2021-04-19T23:04:39Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"From: Luke Shumaker <lukeshu@datawire.io>\n\nSigned-off-by: Luke Shumaker <lukeshu@datawire.io>\n---\n Documentation/git-fast-import.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 39cfa05b28..458af0a2d6 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -437,7 +437,7 @@ change to the project.\n \toriginal-oid?\n \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n-\t('encoding' SP <encoding>)?\n+\t('encoding' SP <encoding> LF)?\n \tdata\n \t('from' SP <commit-ish> LF)?\n \t('merge' SP <commit-ish> LF)*\n-- \n2.31.1\n\n"},{"id":"422349","messageId":"20210419225441.3139048-3-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"20210419225441.3139048-1-lukeshu@lukeshu.com","subject":"[PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim'","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-19T22:54:40Z","receivedAt":"2021-04-19T23:04:40Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"From: Luke Shumaker <lukeshu@datawire.io>\n\nBut still keep --signed-tags=warn as an undocumented alias.  This name\nis clearer as it has symmetry with warn-strip:\n\n                   action              ->                   action\n       +----------------------------+  ->       +----------------------------+\n  msg? |      verbatim |      strip |  ->  msg? |      verbatim |      strip |\n       |          warn | warn-strip |  ->       | warn-verbatim | warn-strip |\n       +----------------------------+  ->       +----------------------------+\n\nSigned-off-by: Luke Shumaker <lukeshu@datawire.io>\n---\n Documentation/git-fast-export.txt |  6 +++---\n builtin/fast-export.c             |  2 +-\n t/t9350-fast-export.sh            | 12 ++++++++++++\n 3 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex 1978dbdc6a..d4a2bfe037 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -27,7 +27,7 @@ OPTIONS\n \tInsert 'progress' statements every <n> objects, to be shown by\n \t'git fast-import' during import.\n \n---signed-tags=(verbatim|warn|warn-strip|strip|abort)::\n+--signed-tags=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n \tSpecify how to handle signed tags.  Since any transformation\n \tafter the export can change the tag names (which can also happen\n \twhen excluding revisions) the signatures will not match.\n@@ -36,8 +36,8 @@ When asking to 'abort' (which is the default), this program will die\n when encountering a signed tag.  With 'strip', the tags will silently\n be made unsigned, with 'warn-strip' they will be made unsigned but a\n warning will be displayed, with 'verbatim', they will be silently\n-exported and with 'warn', they will be exported, but you will see a\n-warning.\n+exported and with 'warn-verbatim', they will be exported, but you will\n+see a warning.\n \n --tag-of-filtered-object=(abort|drop|rewrite)::\n \tSpecify how to handle tags whose tagged object is filtered out.\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 85a76e0ef8..d121dd2ee6 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -55,7 +55,7 @@ static int parse_opt_signed_tag_mode(const struct option *opt,\n \t\tsigned_tag_mode = SIGNED_TAG_ABORT;\n \telse if (!strcmp(arg, \"verbatim\") || !strcmp(arg, \"ignore\"))\n \t\tsigned_tag_mode = VERBATIM;\n-\telse if (!strcmp(arg, \"warn\"))\n+\telse if (!strcmp(arg, \"warn-verbatim\") || !strcmp(arg, \"warn\"))\n \t\tsigned_tag_mode = WARN;\n \telse if (!strcmp(arg, \"warn-strip\"))\n \t\tsigned_tag_mode = WARN_STRIP;\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex 409b48e244..db0e58b1e8 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -253,6 +253,18 @@ test_expect_success 'signed-tags=verbatim' '\n \n '\n \n+test_expect_success 'signed-tags=warn' '\n+\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n+\tgrep PGP output &&\n+\ttest -s err\n+'\n+\n+test_expect_success 'signed-tags=warn-verbatim' '\n+\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n+\tgrep PGP output &&\n+\ttest -s err\n+'\n+\n test_expect_success 'signed-tags=strip' '\n \n \tgit fast-export --signed-tags=strip sign-your-name > output &&\n-- \n2.31.1\n\n"},{"id":"422350","messageId":"20210419225441.3139048-4-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"20210419225441.3139048-1-lukeshu@lukeshu.com","subject":"[PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-19T22:54:41Z","receivedAt":"2021-04-19T23:04:41Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"From: Luke Shumaker <lukeshu@datawire.io>\n\nfast-export has an existing --signed-tags= flag that controls how to\nhandle tag signatures.  However, there is no equivalent for commit\nsignatures; it just silently strips the signature out of the commit\n(analogously to --signed-tags=strip).\n\nWhile signatures are generally problematic for fast-export/fast-import\n(because hashes are likely to change), if they're going to support tag\nsignatures, there's no reason to not also support commit signatures.\n\nSo, implement signed-commits.\n\nOn the fast-export side, try to be as much like signed-tags as possible,\nin both implementation and in user-interface; with the exception that\nthe default should be `--signed-commits=strip` (compared to the default\n`--signed-tags=abort`), in order to continue defaulting to the\nhistorical behavior.  Only bother implementing \"gpgsig\", not\n\"gpgsig-sha256\"; the existing signed-tag support doesn't implement\n\"gpgsig-sha256\" either.\n\nOn the fast-import side, I'm not entirely sure that I got the ordering\ncorrect between \"gpgsig\" and \"encoding\" when generating the commit\nobject.\n\nSigned-off-by: Luke Shumaker <lukeshu@datawire.io>\n---\n Documentation/git-fast-export.txt | 12 +++++\n Documentation/git-fast-import.txt |  7 +++\n builtin/fast-export.c             | 86 +++++++++++++++++++++++++------\n builtin/fast-import.c             | 15 ++++++\n t/t9350-fast-export.sh            | 70 +++++++++++++++++++++++++\n 5 files changed, 174 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\nindex d4a2bfe037..6fdb678b54 100644\n--- a/Documentation/git-fast-export.txt\n+++ b/Documentation/git-fast-export.txt\n@@ -39,6 +39,18 @@ warning will be displayed, with 'verbatim', they will be silently\n exported and with 'warn-verbatim', they will be exported, but you will\n see a warning.\n \n+--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n+\tSpecify how to handle signed commits.  Since any transformation\n+\tafter the export can change the commit (which can also happen\n+\twhen excluding revisions) the signatures will not match.\n++\n+When asking to 'abort', this program will die when encountering a\n+signed commit.  With 'strip' (which is the default), the commits will\n+silently be made unsigned, with 'warn-strip' they will be made\n+unsigned but a warning will be displayed, with 'verbatim', they will\n+be silently exported and with 'warn-verbatim', they will be exported,\n+but you will see a warning.\n+\n --tag-of-filtered-object=(abort|drop|rewrite)::\n \tSpecify how to handle tags whose tagged object is filtered out.\n \tSince revisions and files to export can be limited by path,\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 458af0a2d6..3d0c5dbf7d 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -437,6 +437,7 @@ change to the project.\n \toriginal-oid?\n \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n+\t('gpgsig' LF data)?\n \t('encoding' SP <encoding> LF)?\n \tdata\n \t('from' SP <commit-ish> LF)?\n@@ -505,6 +506,12 @@ that was selected by the --date-format=<fmt> command-line option.\n See ``Date Formats'' above for the set of supported formats, and\n their syntax.\n \n+`gpgsig`\n+^^^^^^^^\n+\n+The optional `gpgsig` command is used to include a PGP/GPG signature\n+that signs the commit data.\n+\n `encoding`\n ^^^^^^^^^^\n The optional `encoding` command indicates the encoding of the commit\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex d121dd2ee6..d48adbc9b9 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -30,8 +30,11 @@ static const char *fast_export_usage[] = {\n \tNULL\n };\n \n+enum sign_mode { SIGN_ABORT, SIGN_VERBATIM, SIGN_STRIP, SIGN_VERBATIM_WARN, SIGN_STRIP_WARN };\n+\n static int progress;\n-static enum { SIGNED_TAG_ABORT, VERBATIM, WARN, WARN_STRIP, STRIP } signed_tag_mode = SIGNED_TAG_ABORT;\n+static enum sign_mode signed_tag_mode = SIGN_ABORT;\n+static enum sign_mode signed_commit_mode = SIGN_STRIP;\n static enum { TAG_FILTERING_ABORT, DROP, REWRITE } tag_of_filtered_mode = TAG_FILTERING_ABORT;\n static enum { REENCODE_ABORT, REENCODE_YES, REENCODE_NO } reencode_mode = REENCODE_ABORT;\n static int fake_missing_tagger;\n@@ -48,21 +51,24 @@ static int anonymize;\n static struct hashmap anonymized_seeds;\n static struct revision_sources revision_sources;\n \n-static int parse_opt_signed_tag_mode(const struct option *opt,\n+static int parse_opt_sign_mode(const struct option *opt,\n \t\t\t\t     const char *arg, int unset)\n {\n-\tif (unset || !strcmp(arg, \"abort\"))\n-\t\tsigned_tag_mode = SIGNED_TAG_ABORT;\n+\tenum sign_mode *valptr = opt->value;\n+\tif (unset)\n+\t\treturn 0;\n+\telse if (!strcmp(arg, \"abort\"))\n+\t\t*valptr = SIGN_ABORT;\n \telse if (!strcmp(arg, \"verbatim\") || !strcmp(arg, \"ignore\"))\n-\t\tsigned_tag_mode = VERBATIM;\n+\t\t*valptr = SIGN_VERBATIM;\n \telse if (!strcmp(arg, \"warn-verbatim\") || !strcmp(arg, \"warn\"))\n-\t\tsigned_tag_mode = WARN;\n+\t\t*valptr = SIGN_VERBATIM_WARN;\n \telse if (!strcmp(arg, \"warn-strip\"))\n-\t\tsigned_tag_mode = WARN_STRIP;\n+\t\t*valptr = SIGN_STRIP_WARN;\n \telse if (!strcmp(arg, \"strip\"))\n-\t\tsigned_tag_mode = STRIP;\n+\t\t*valptr = SIGN_STRIP;\n \telse\n-\t\treturn error(\"Unknown signed-tags mode: %s\", arg);\n+\t\treturn error(\"Unknown %s mode: %s\", opt->long_name, arg);\n \treturn 0;\n }\n \n@@ -499,6 +505,28 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t}\n }\n \n+static const char *find_signature(const char *begin, const char *end)\n+{\n+\tconst char *needle = \"\\ngpgsig \";\n+\tchar *bod, *eod, *eol;\n+\n+\tbod = memmem(begin, end ? end - begin : strlen(begin),\n+\t\t     needle, strlen(needle));\n+\tif (!bod)\n+\t\treturn NULL;\n+\tbod += strlen(needle);\n+\teod = strchrnul(bod, '\\n');\n+\twhile (eod[0] == '\\n' && eod[1] == ' ') {\n+\t\teod = strchrnul(eod+1, '\\n');\n+\t}\n+\t*eod = '\\0';\n+\n+\twhile ((eol = strstr(bod, \"\\n \")))\n+\t\tmemmove(eol+1, eol+2, strlen(eol+1));\n+\n+\treturn bod;\n+}\n+\n static const char *find_encoding(const char *begin, const char *end)\n {\n \tconst char *needle = \"\\nencoding \";\n@@ -621,7 +649,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tint saved_output_format = rev->diffopt.output_format;\n \tconst char *commit_buffer;\n \tconst char *author, *author_end, *committer, *committer_end;\n-\tconst char *encoding, *message;\n+\tconst char *encoding, *signature, *message;\n \tchar *reencoded = NULL;\n \tstruct commit_list *p;\n \tconst char *refname;\n@@ -644,6 +672,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tcommitter++;\n \tcommitter_end = strchrnul(committer, '\\n');\n \tmessage = strstr(committer_end, \"\\n\\n\");\n+\tsignature = find_signature(committer_end, message);\n \tencoding = find_encoding(committer_end, message);\n \tif (message)\n \t\tmessage += 2;\n@@ -703,6 +732,28 @@ static void handle_commit(struct commit *commit, struct rev_info *rev,\n \tprintf(\"%.*s\\n%.*s\\n\",\n \t       (int)(author_end - author), author,\n \t       (int)(committer_end - committer), committer);\n+\tif (signature)\n+\t\tswitch(signed_commit_mode) {\n+\t\tcase SIGN_ABORT:\n+\t\t\tdie(\"encountered signed commit %s; use \"\n+\t\t\t    \"--signed-commits=<mode> to handle it\",\n+\t\t\t    oid_to_hex(&commit->object.oid));\n+\t\tcase SIGN_VERBATIM_WARN:\n+\t\t\twarning(\"exporting signed commit %s\",\n+\t\t\t\toid_to_hex(&commit->object.oid));\n+\t\t\t/* fallthru */\n+\t\tcase SIGN_VERBATIM:\n+\t\t\tprintf(\"gpgsig\\ndata %u\\n%s\",\n+\t\t\t       (unsigned)strlen(signature),\n+\t\t\t       signature);\n+\t\t\tbreak;\n+\t\tcase SIGN_STRIP_WARN:\n+\t\t\twarning(\"stripping signature from commit %s\",\n+\t\t\t       oid_to_hex(&commit->object.oid));\n+\t\t\t/* fallthru */\n+\t\tcase SIGN_STRIP:\n+\t\t\tbreak;\n+\t\t}\n \tif (!reencoded && encoding)\n \t\tprintf(\"encoding %s\\n\", encoding);\n \tprintf(\"data %u\\n%s\",\n@@ -830,21 +881,21 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t\t\t\t       \"\\n-----BEGIN PGP SIGNATURE-----\\n\");\n \t\tif (signature)\n \t\t\tswitch(signed_tag_mode) {\n-\t\t\tcase SIGNED_TAG_ABORT:\n+\t\t\tcase SIGN_ABORT:\n \t\t\t\tdie(\"encountered signed tag %s; use \"\n \t\t\t\t    \"--signed-tags=<mode> to handle it\",\n \t\t\t\t    oid_to_hex(&tag->object.oid));\n-\t\t\tcase WARN:\n+\t\t\tcase SIGN_VERBATIM_WARN:\n \t\t\t\twarning(\"exporting signed tag %s\",\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n \t\t\t\t/* fallthru */\n-\t\t\tcase VERBATIM:\n+\t\t\tcase SIGN_VERBATIM:\n \t\t\t\tbreak;\n-\t\t\tcase WARN_STRIP:\n+\t\t\tcase SIGN_STRIP_WARN:\n \t\t\t\twarning(\"stripping signature from tag %s\",\n \t\t\t\t\toid_to_hex(&tag->object.oid));\n \t\t\t\t/* fallthru */\n-\t\t\tcase STRIP:\n+\t\t\tcase SIGN_STRIP:\n \t\t\t\tmessage_size = signature + 1 - message;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -1197,7 +1248,10 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\t\t    N_(\"show progress after <n> objects\")),\n \t\tOPT_CALLBACK(0, \"signed-tags\", &signed_tag_mode, N_(\"mode\"),\n \t\t\t     N_(\"select handling of signed tags\"),\n-\t\t\t     parse_opt_signed_tag_mode),\n+\t\t\t     parse_opt_sign_mode),\n+\t\tOPT_CALLBACK(0, \"signed-commits\", &signed_commit_mode, N_(\"mode\"),\n+\t\t\t     N_(\"select handling of signed commits\"),\n+\t\t\t     parse_opt_sign_mode),\n \t\tOPT_CALLBACK(0, \"tag-of-filtered-object\", &tag_of_filtered_mode, N_(\"mode\"),\n \t\t\t     N_(\"select handling of tags that tag filtered objects\"),\n \t\t\t     parse_opt_tag_of_filtered_mode),\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 3afa81cf9a..74d08e09fd 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -2669,7 +2669,9 @@ static struct hash_list *parse_merge(unsigned int *count)\n \n static void parse_new_commit(const char *arg)\n {\n+\tstatic struct strbuf sig = STRBUF_INIT;\n \tstatic struct strbuf msg = STRBUF_INIT;\n+\tstruct string_list siglines = STRING_LIST_INIT_NODUP;\n \tstruct branch *b;\n \tchar *author = NULL;\n \tchar *committer = NULL;\n@@ -2696,6 +2698,12 @@ static void parse_new_commit(const char *arg)\n \t}\n \tif (!committer)\n \t\tdie(\"Expected committer but didn't get one\");\n+\tif (!strcmp(command_buf.buf, \"gpgsig\")) {\n+\t\tread_next_command();\n+\t\tparse_data(&sig, 0, NULL);\n+\t\tread_next_command();\n+\t} else\n+\t\tstrbuf_setlen(&sig, 0);\n \tif (skip_prefix(command_buf.buf, \"encoding \", &v)) {\n \t\tencoding = xstrdup(v);\n \t\tread_next_command();\n@@ -2769,8 +2777,15 @@ static void parse_new_commit(const char *arg)\n \t\tstrbuf_addf(&new_data,\n \t\t\t\"encoding %s\\n\",\n \t\t\tencoding);\n+\tif (sig.len) {\n+\t\tstrbuf_addstr(&new_data, \"gpgsig \");\n+\t\tstring_list_split_in_place(&siglines, sig.buf, '\\n', -1);\n+\t\tstrbuf_add_separated_string_list(&new_data, \"\\n \", &siglines);\n+\t\tstrbuf_addch(&new_data, '\\n');\n+\t}\n \tstrbuf_addch(&new_data, '\\n');\n \tstrbuf_addbuf(&new_data, &msg);\n+\tstring_list_clear(&siglines, 1);\n \tfree(author);\n \tfree(committer);\n \tfree(encoding);\ndiff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh\nindex db0e58b1e8..49a2827be2 100755\n--- a/t/t9350-fast-export.sh\n+++ b/t/t9350-fast-export.sh\n@@ -8,6 +8,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-gpg.sh\"\n \n test_expect_success 'setup' '\n \n@@ -278,9 +279,78 @@ test_expect_success 'signed-tags=warn-strip' '\n \ttest -s err\n '\n \n+test_expect_success GPG 'set up signed commit' '\n+\n+\t# Generate a commit with both \"gpgsig\" and \"encoding\" set, so\n+\t# that we can test that fast-import gets the ordering correct\n+\t# between the two.\n+\ttest_config i18n.commitEncoding ISO-8859-1 &&\n+\tgit checkout -f -b commit-signing main &&\n+\techo Sign your name > file-sign &&\n+\tgit add file-sign &&\n+\tgit commit -S -m \"signed commit\" &&\n+\tCOMMIT_SIGNING=$(git rev-parse --verify commit-signing)\n+\n+'\n+\n+test_expect_success GPG 'signed-commits=abort' '\n+\n+\ttest_must_fail git fast-export --signed-commits=abort commit-signing\n+\n+'\n+\n+test_expect_success GPG 'signed-commits=verbatim' '\n+\n+\tgit fast-export --signed-commits=verbatim --reencode=no commit-signing >output &&\n+\tgrep ^gpgsig output &&\n+\tgrep \"encoding ISO-8859-1\" output &&\n+\t(cd new &&\n+\t git fast-import &&\n+\t test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output\n+\n+'\n+\n+test_expect_success GPG 'signed-commits=warn-verbatim' '\n+\n+\tgit fast-export --signed-commits=warn-verbatim --reencode=no commit-signing >output 2>err &&\n+\tgrep ^gpgsig output &&\n+\tgrep \"encoding ISO-8859-1\" output &&\n+\ttest -s err &&\n+\t(cd new &&\n+\t git fast-import &&\n+\t test $COMMIT_SIGNING = $(git rev-parse --verify refs/heads/commit-signing)) <output\n+\n+'\n+\n+test_expect_success GPG 'signed-commits=strip' '\n+\n+\tgit fast-export --signed-commits=strip --reencode=no commit-signing >output &&\n+\t! grep ^gpgsig output &&\n+\tgrep \"^encoding ISO-8859-1\" output &&\n+\tsed \"s/commit-signing/commit-strip-signing/\" output |\n+\t\t(cd new &&\n+\t\t git fast-import &&\n+\t\t test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))\n+\n+'\n+\n+test_expect_success GPG 'signed-commits=warn-strip' '\n+\n+\tgit fast-export --signed-commits=warn-strip --reencode=no commit-signing >output 2>err &&\n+\t! grep ^gpgsig output &&\n+\tgrep \"^encoding ISO-8859-1\" output &&\n+\ttest -s err &&\n+\tsed \"s/commit-signing/commit-strip-signing/\" output |\n+\t\t(cd new &&\n+\t\t git fast-import &&\n+\t\t test $COMMIT_SIGNING != $(git rev-parse --verify refs/heads/commit-strip-signing))\n+\n+'\n+\n test_expect_success 'setup submodule' '\n \n \tgit checkout -f main &&\n+\t{ git update-ref -d refs/heads/commit-signing || true; } &&\n \tmkdir sub &&\n \t(\n \t\tcd sub &&\n-- \n2.31.1\n\n"},{"id":"422351","messageId":"20210419225441.3139048-1-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":null,"subject":"[PATCH 0/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-19T22:54:38Z","receivedAt":"2021-04-19T23:04:41Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"From: Luke Shumaker <lukeshu@datawire.io>\n\nfast-export has an existing --signed-tags= flag that controls how to\nhandle tag signatures.  However, there is no equivalent for commit\nsignatures; it just silently strips the signature out of the commit\n(analogously to --signed-tags=strip).\n\nSo implement a --signed-commits= flag in fast-export, and implement\nthe receiving side of it in fast-import.\n\nLuke Shumaker (3):\n  git-fast-import.txt: add missing LF in the BNF\n  fast-export: rename --signed-tags='warn' to 'warn-verbatim'\n  fast-export, fast-import: implement signed-commits\n\n Documentation/git-fast-export.txt | 18 +++++--\n Documentation/git-fast-import.txt |  9 +++-\n builtin/fast-export.c             | 88 +++++++++++++++++++++++++------\n builtin/fast-import.c             | 15 ++++++\n t/t9350-fast-export.sh            | 82 ++++++++++++++++++++++++++++\n 5 files changed, 191 insertions(+), 21 deletions(-)\n\n-- \n2.31.1\n\nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422357","messageId":"YH4f97oreblENi3V@nand.local","threadId":"55522","inReplyTo":"20210419225441.3139048-3-lukeshu@lukeshu.com","subject":"Re: [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-04-20T00:27:35Z","receivedAt":"2021-04-20T00:27:39Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Apr 19, 2021 at 04:54:40PM -0600, Luke Shumaker wrote:\n> From: Luke Shumaker <lukeshu@datawire.io>\n>\n> But still keep --signed-tags=warn as an undocumented alias.  This name\n> is clearer as it has symmetry with warn-strip:\n>\n>                    action              ->                   action\n>        +----------------------------+  ->       +----------------------------+\n>   msg? |      verbatim |      strip |  ->  msg? |      verbatim |      strip |\n>        |          warn | warn-strip |  ->       | warn-verbatim | warn-strip |\n>        +----------------------------+  ->       +----------------------------+\n\nThis table is rather confusing to me. What's unclear to me is what\n\"msg?\" and \"action\" are referring to. After reading your patch, I think\nit may be clearer to say:\n\n    The --signed-tags option takes one of five arguments specifying how\n    to handle singed tags during export. Among these arguments, strip is\n    to warn-strip as verbatim is to warn. (The unmentioned argument is\n    'abort', which stops the fast-export process entirely). That is,\n    signatures are either stripped or copied verbatim while exporting,\n    with or without a warning.\n\n    Make clear that the \"warn\" option instructs fast-export to copy\n    signatures verbatim by matching the pattern (and calling the option\n    \"warn-verbatim\").\n\n    To maintain backwards compatibility, \"warn\" is still recognized as\n    an undocumented alias.\n\n> +test_expect_success 'signed-tags=warn' '\n> +\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n> +\tgrep PGP output &&\n> +\ttest -s err\n> +'\n> +\n> +test_expect_success 'signed-tags=warn-verbatim' '\n> +\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n\ns/warn/warn-verbatim ?\n\nThanks,\nTaylor\n"},{"id":"422361","messageId":"YH4xY/oSwYIUmJyL@camp.crustytoothpaste.net","threadId":"55522","inReplyTo":"20210419225441.3139048-4-lukeshu@lukeshu.com","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-04-20T01:41:55Z","receivedAt":"2021-04-20T01:42:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-04-19 at 22:54:41, Luke Shumaker wrote:\n> From: Luke Shumaker <lukeshu@datawire.io>\n> \n> fast-export has an existing --signed-tags= flag that controls how to\n> handle tag signatures.  However, there is no equivalent for commit\n> signatures; it just silently strips the signature out of the commit\n> (analogously to --signed-tags=strip).\n> \n> While signatures are generally problematic for fast-export/fast-import\n> (because hashes are likely to change), if they're going to support tag\n> signatures, there's no reason to not also support commit signatures.\n> \n> So, implement signed-commits.\n> \n> On the fast-export side, try to be as much like signed-tags as possible,\n> in both implementation and in user-interface; with the exception that\n> the default should be `--signed-commits=strip` (compared to the default\n> `--signed-tags=abort`), in order to continue defaulting to the\n> historical behavior.  Only bother implementing \"gpgsig\", not\n> \"gpgsig-sha256\"; the existing signed-tag support doesn't implement\n> \"gpgsig-sha256\" either.\n\nI would appreciate it if we did in fact implement it.  I would like to\nuse this functionality to round-trip objects between SHA-1 and SHA-256,\nand it would be nice if both worked.\n\nThe situation with tags is different: the signature using the current\nalgorithm is always trailing, and the signature for the other algorithm\nis in the header.  That wasn't how we intended it to be, but that's how\nit ended up being.\n\nAs a result, tag output can support SHA-256 data, but with your\nproposal, SHA-256 commits wouldn't work at all.  Considering SHA-1 is\nwildly insecure and therefore signing SHA-1 commits adds very little\nsecurity, whereas SHA-256 is presently considered strong, I'd argue that\nonly supporting SHA-1 isn't the right move here.\n\nProvided we do that and the test suite passes under both algorithms, I'm\nstrongly in favor of this feature.  In fact, I had been thinking about\nimplementing this feature myself just the other day, so I'm delighted\nyou decided to do it.\n\n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index 458af0a2d6..3d0c5dbf7d 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -437,6 +437,7 @@ change to the project.\n>  \toriginal-oid?\n>  \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n>  \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n> +\t('gpgsig' LF data)?\n\nCould we emit this as \"gpgsig sha1 data\" and \"gpgsig sha256 data\"?  That\nwould allow us to consider the future case where the hash algorithm\nchanges again without requiring a change of format.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"422362","messageId":"YH4yN2OAghWB9/97@nand.local","threadId":"55522","inReplyTo":"20210419225441.3139048-4-lukeshu@lukeshu.com","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Taylor Blau","fromEmail":"ttaylorr@github.com","sentAt":"2021-04-20T01:45:46Z","receivedAt":"2021-04-20T01:45:50Z","isPatch":true,"sender":{"key":"ttaylorr@github.com","avatar":"https://gravatar.com/avatar/d5f3476f26b6f99cbb6b467e7ed7482f5762c8157bc73f569196e428bdcbea25?d=mp&s=160"},"body":"On Mon, Apr 19, 2021 at 04:54:41PM -0600, Luke Shumaker wrote:\n> From: Luke Shumaker <lukeshu@datawire.io>\n>\n> fast-export has an existing --signed-tags= flag that controls how to\n> handle tag signatures.  However, there is no equivalent for commit\n> signatures; it just silently strips the signature out of the commit\n> (analogously to --signed-tags=strip).\n>\n> While signatures are generally problematic for fast-export/fast-import\n> (because hashes are likely to change), if they're going to support tag\n> signatures, there's no reason to not also support commit signatures.\n>\n> So, implement signed-commits.\n>\n> On the fast-export side, try to be as much like signed-tags as possible,\n> in both implementation and in user-interface; with the exception that\n> the default should be `--signed-commits=strip` (compared to the default\n> `--signed-tags=abort`), in order to continue defaulting to the\n> historical behavior.  Only bother implementing \"gpgsig\", not\n> \"gpgsig-sha256\"; the existing signed-tag support doesn't implement\n> \"gpgsig-sha256\" either.\n>\n> On the fast-import side, I'm not entirely sure that I got the ordering\n> correct between \"gpgsig\" and \"encoding\" when generating the commit\n> object.\n>\n> Signed-off-by: Luke Shumaker <lukeshu@datawire.io>\n> ---\n>  Documentation/git-fast-export.txt | 12 +++++\n>  Documentation/git-fast-import.txt |  7 +++\n>  builtin/fast-export.c             | 86 +++++++++++++++++++++++++------\n>  builtin/fast-import.c             | 15 ++++++\n>  t/t9350-fast-export.sh            | 70 +++++++++++++++++++++++++\n>  5 files changed, 174 insertions(+), 16 deletions(-)\n>\n> diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> index d4a2bfe037..6fdb678b54 100644\n> --- a/Documentation/git-fast-export.txt\n> +++ b/Documentation/git-fast-export.txt\n> @@ -39,6 +39,18 @@ warning will be displayed, with 'verbatim', they will be silently\n>  exported and with 'warn-verbatim', they will be exported, but you will\n>  see a warning.\n>\n> +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n> +\tSpecify how to handle signed commits.  Since any transformation\n> +\tafter the export can change the commit (which can also happen\n> +\twhen excluding revisions) the signatures will not match.\n> ++\n> +When asking to 'abort', this program will die when encountering a\n> +signed commit.  With 'strip' (which is the default), the commits will\n> +silently be made unsigned, with 'warn-strip' they will be made\n> +unsigned but a warning will be displayed, with 'verbatim', they will\n> +be silently exported and with 'warn-verbatim', they will be exported,\n> +but you will see a warning.\n> +\n\nOK, this all seems normal to me. But it may be worth shortening it to\nsay \"behaves exactly as --signed-tags, but for commits\", or something.\n\n>  --tag-of-filtered-object=(abort|drop|rewrite)::\n>  \tSpecify how to handle tags whose tagged object is filtered out.\n>  \tSince revisions and files to export can be limited by path,\n> diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> index 458af0a2d6..3d0c5dbf7d 100644\n> --- a/Documentation/git-fast-import.txt\n> +++ b/Documentation/git-fast-import.txt\n> @@ -437,6 +437,7 @@ change to the project.\n>  \toriginal-oid?\n>  \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n>  \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n> +\t('gpgsig' LF data)?\n\nIs this missing a LF after data?\n\n> +static const char *find_signature(const char *begin, const char *end)\n> +{\n> +\tconst char *needle = \"\\ngpgsig \";\n> +\tchar *bod, *eod, *eol;\n> +\n> +\tbod = memmem(begin, end ? end - begin : strlen(begin),\n> +\t\t     needle, strlen(needle));\n> +\tif (!bod)\n> +\t\treturn NULL;\n> +\tbod += strlen(needle);\n> +\teod = strchrnul(bod, '\\n');\n> +\twhile (eod[0] == '\\n' && eod[1] == ' ') {\n> +\t\teod = strchrnul(eod+1, '\\n');\n> +\t}\n> +\t*eod = '\\0';\n> +\n> +\twhile ((eol = strstr(bod, \"\\n \")))\n> +\t\tmemmove(eol+1, eol+2, strlen(eol+1));\n\nHmm. I'm not quite sure I follow these last two lines. Perhaps a comment\nwould help? The rest of this patch looks reasonable to me.\n\nThanks,\nTaylor\n"},{"id":"422443","messageId":"87y2ddot7p.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"YH4f97oreblENi3V@nand.local","subject":"Re: [PATCH 2/3] fast-export: rename --signed-tags='warn' to 'warn-verbatim'","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-20T15:45:30Z","receivedAt":"2021-04-20T15:45:39Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Mon, 19 Apr 2021 18:27:35 -0600,\nTaylor Blau wrote:\n> \n> On Mon, Apr 19, 2021 at 04:54:40PM -0600, Luke Shumaker wrote:\n> > From: Luke Shumaker <lukeshu@datawire.io>\n> >\n> > But still keep --signed-tags=warn as an undocumented alias.  This name\n> > is clearer as it has symmetry with warn-strip:\n> >\n> >                    action              ->                   action\n> >        +----------------------------+  ->       +----------------------------+\n> >   msg? |      verbatim |      strip |  ->  msg? |      verbatim |      strip |\n> >        |          warn | warn-strip |  ->       | warn-verbatim | warn-strip |\n> >        +----------------------------+  ->       +----------------------------+\n> \n> This table is rather confusing to me. What's unclear to me is what\n> \"msg?\" and \"action\" are referring to. After reading your patch, I think\n> it may be clearer to say:\n> \n>     The --signed-tags option takes one of five arguments specifying how\n>     to handle singed tags during export. Among these arguments, strip is\n>     to warn-strip as verbatim is to warn. (The unmentioned argument is\n>     'abort', which stops the fast-export process entirely). That is,\n>     signatures are either stripped or copied verbatim while exporting,\n>     with or without a warning.\n> \n>     Make clear that the \"warn\" option instructs fast-export to copy\n>     signatures verbatim by matching the pattern (and calling the option\n>     \"warn-verbatim\").\n> \n>     To maintain backwards compatibility, \"warn\" is still recognized as\n>     an undocumented alias.\n\nThank you, I'll take much of this wording.\n\n> > +test_expect_success 'signed-tags=warn' '\n> > +\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n> > +\tgrep PGP output &&\n> > +\ttest -s err\n> > +'\n> > +\n> > +test_expect_success 'signed-tags=warn-verbatim' '\n> > +\tgit fast-export --signed-tags=warn sign-your-name >output 2>err &&\n> \n> s/warn/warn-verbatim ?\n\nIndeed, oops!\n\nI'll also add a comment clarifying that the signed-tags=warn test is\ntesting for backward compatibility.\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422445","messageId":"87wnsxosxc.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"20210419225441.3139048-4-lukeshu@lukeshu.com","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-20T15:51:43Z","receivedAt":"2021-04-20T15:51:49Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Mon, 19 Apr 2021 16:54:41 -0600,\nLuke Shumaker wrote:\n> On the fast-import side, I'm not entirely sure that I got the ordering\n> correct between \"gpgsig\" and \"encoding\" when generating the commit\n> object.\n\nWhoops, I forgot to take that out of the commit message; I did check\nthe ordering before submitting the patch.\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422449","messageId":"87v98gq60k.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"YH4yN2OAghWB9/97@nand.local","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-20T16:23:39Z","receivedAt":"2021-04-20T16:23:46Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Mon, 19 Apr 2021 19:45:46 -0600,\nTaylor Blau wrote:\n> > diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt\n> > index d4a2bfe037..6fdb678b54 100644\n> > --- a/Documentation/git-fast-export.txt\n> > +++ b/Documentation/git-fast-export.txt\n> > @@ -39,6 +39,18 @@ warning will be displayed, with 'verbatim', they will be silently\n> >  exported and with 'warn-verbatim', they will be exported, but you will\n> >  see a warning.\n> >\n> > +--signed-commits=(verbatim|warn-verbatim|warn-strip|strip|abort)::\n> > +\tSpecify how to handle signed commits.  Since any transformation\n> > +\tafter the export can change the commit (which can also happen\n> > +\twhen excluding revisions) the signatures will not match.\n> > ++\n> > +When asking to 'abort', this program will die when encountering a\n> > +signed commit.  With 'strip' (which is the default), the commits will\n> > +silently be made unsigned, with 'warn-strip' they will be made\n> > +unsigned but a warning will be displayed, with 'verbatim', they will\n> > +be silently exported and with 'warn-verbatim', they will be exported,\n> > +but you will see a warning.\n> > +\n> \n> OK, this all seems normal to me. But it may be worth shortening it to\n> say \"behaves exactly as --signed-tags, but for commits\", or something.\n\nGood suggestion, it would also make it more obvious that the default\nis different, since I'd have to call it out explictly.\n\n> >  --tag-of-filtered-object=(abort|drop|rewrite)::\n> >  \tSpecify how to handle tags whose tagged object is filtered out.\n> >  \tSince revisions and files to export can be limited by path,\n> > diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> > index 458af0a2d6..3d0c5dbf7d 100644\n> > --- a/Documentation/git-fast-import.txt\n> > +++ b/Documentation/git-fast-import.txt\n> > @@ -437,6 +437,7 @@ change to the project.\n> >  \toriginal-oid?\n> >  \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n> >  \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n> > +\t('gpgsig' LF data)?\n> \n> Is this missing a LF after data?\n\nNo, the definition of `data` has a byte-count prefix, so it doesn't\nneed an `LF` to act as a terminator (and it also already includes an\noptional trailing `LF?` just in case you want to include one).\n\nIn fact, my implementation in fast-export does not include the LF,\nwhich is why the test greps for \"encoding ISO-8859-1\" instead of\n\"^encoding ISO-8859-1\".\n\nI'll add a comment saying that it's intentional.\n\n> > +static const char *find_signature(const char *begin, const char *end)\n> > +{\n> > +\tconst char *needle = \"\\ngpgsig \";\n> > +\tchar *bod, *eod, *eol;\n> > +\n> > +\tbod = memmem(begin, end ? end - begin : strlen(begin),\n> > +\t\t     needle, strlen(needle));\n> > +\tif (!bod)\n> > +\t\treturn NULL;\n> > +\tbod += strlen(needle);\n> > +\teod = strchrnul(bod, '\\n');\n> > +\twhile (eod[0] == '\\n' && eod[1] == ' ') {\n> > +\t\teod = strchrnul(eod+1, '\\n');\n> > +\t}\n> > +\t*eod = '\\0';\n> > +\n> > +\twhile ((eol = strstr(bod, \"\\n \")))\n> > +\t\tmemmove(eol+1, eol+2, strlen(eol+1));\n> \n> Hmm. I'm not quite sure I follow these last two lines. Perhaps a comment\n> would help? The rest of this patch looks reasonable to me.\n\nIn the commit object, multi-line header values are stored by prefixing\ncontinuation lines begin with a space.  So within the commit object,\nit looks like\n\n    \"gpgsig -----BEGIN PGP SIGNATURE-----\\n\"\n    \" Version: GnuPG v1.4.5 (GNU/Linux)\\n\"\n    \" \\n\"\n    \" base64_pem_here\\n\"\n    \" -----END PGP SIGNATURE-----\\n\"\n\nHowever, we want the raw value; we want to return\n\n    \"-----BEGIN PGP SIGNATURE-----\\n\"\n    \"Version: GnuPG v1.4.5 (GNU/Linux)\\n\"\n    \"\\n\"\n    \"base64_pem_here\\n\"\n    \"-----END PGP SIGNATURE-----\\n\"\n\nwithout all the extra spaces.  That's what those two lines are doing,\nstripping out the extra spaces.\n\nI'll add some comments.\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422456","messageId":"87tuo0q3ma.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"YH4xY/oSwYIUmJyL@camp.crustytoothpaste.net","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-20T17:15:25Z","receivedAt":"2021-04-20T17:15:29Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Mon, 19 Apr 2021 19:41:55 -0600,\nbrian m. carlson wrote:\n> \n> [1  <text/plain; utf-8 (quoted-printable)>]\n> On 2021-04-19 at 22:54:41, Luke Shumaker wrote:\n> > From: Luke Shumaker <lukeshu@datawire.io>\n> > \n> > fast-export has an existing --signed-tags= flag that controls how to\n> > handle tag signatures.  However, there is no equivalent for commit\n> > signatures; it just silently strips the signature out of the commit\n> > (analogously to --signed-tags=strip).\n> > \n> > While signatures are generally problematic for fast-export/fast-import\n> > (because hashes are likely to change), if they're going to support tag\n> > signatures, there's no reason to not also support commit signatures.\n> > \n> > So, implement signed-commits.\n> > \n> > On the fast-export side, try to be as much like signed-tags as possible,\n> > in both implementation and in user-interface; with the exception that\n> > the default should be `--signed-commits=strip` (compared to the default\n> > `--signed-tags=abort`), in order to continue defaulting to the\n> > historical behavior.  Only bother implementing \"gpgsig\", not\n> > \"gpgsig-sha256\"; the existing signed-tag support doesn't implement\n> > \"gpgsig-sha256\" either.\n> \n> I would appreciate it if we did in fact implement it.  I would like to\n> use this functionality to round-trip objects between SHA-1 and SHA-256,\n> and it would be nice if both worked.\n> \n> The situation with tags is different: the signature using the current\n> algorithm is always trailing, and the signature for the other algorithm\n> is in the header.  That wasn't how we intended it to be, but that's how\n> it ended up being.\n> \n> As a result, tag output can support SHA-256 data,\n\nI don't believe that's true?  With SHA-1-signed tags, the signature\ngets included in the fast-import stream as part of the tag message\n(the `data` line in the BNF).  Since SHA-256-signed tags have their\nsignature as a header (rather than just appending it to the message),\nwe'd have to add a 'gpgsig' sub-command to the 'tag' top-level-command\n(like I've done to the 'commit' top-level-command).\n\n>                                                   but with your\n> proposal, SHA-256 commits wouldn't work at all.  Considering SHA-1 is\n> wildly insecure and therefore signing SHA-1 commits adds very little\n> security, whereas SHA-256 is presently considered strong, I'd argue that\n> only supporting SHA-1 isn't the right move here.\n\nThe main reason I didn't implement SHA-256 support (well, besides that\nthe repo I'm working on turned out to not have any SHA-256-signed\ncommits in it) is that I had questions about SHA-256 that I didn't\nknow/couldn't find the answers to.\n\nHowever, looking again, I see a few of the answers in\nt7510-signed-commit.sh, so I'll have a go at it.  If I get stuck, I'll\ngo ahead and implement the below \"gpgsig sha1\" suggestion, and leave\nthe sha256 implementation to someone else.\n\n> Provided we do that and the test suite passes under both algorithms, I'm\n> strongly in favor of this feature.  In fact, I had been thinking about\n> implementing this feature myself just the other day, so I'm delighted\n> you decided to do it.\n\nThat's one of the big reasons I didn't implement both--I wasn't sure\nhow to test sha256 (within the test harness, `git commit -S` gives a\nsha1 signature).\n\nI see that t7510-signed-commit.sh 'verify-commit verifies multiply\nsigned commits' tests sha256 by hard-coding a raw commit object in the\ntest itself, and feeding that to `git hash-object`.  I'd prefer to\nfigure out how to get `git commit` itself to generate a sha256\nsignature rather than a sha1 signature, so that I can _know_ that I'm\ngetting the ordering of headers the same as `git commit`.  But I don't\nthink that needs to be a blocker; if the test doesn't do the same\nordering as `git commit`, I guess that can just be a bugfix later?\n\n> > diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\n> > index 458af0a2d6..3d0c5dbf7d 100644\n> > --- a/Documentation/git-fast-import.txt\n> > +++ b/Documentation/git-fast-import.txt\n> > @@ -437,6 +437,7 @@ change to the project.\n> >  \toriginal-oid?\n> >  \t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n> >  \t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n> > +\t('gpgsig' LF data)?\n> \n> Could we emit this as \"gpgsig sha1 data\" and \"gpgsig sha256 data\"?  That\n> would allow us to consider the future case where the hash algorithm\n> changes again without requiring a change of format.\n\nI like that idea.  I'll implement it.\n\nFWIW, I thought about instead adding a fast-import command to insert\narbitrary headers in to the commit object, rather than having to add a\nnew command for every new header we want to be able to round-trip.\nBut it's like, if we're exposing that much of the low-levels of a\ncommit object, why are we keeping up the facade fast-import stream at\nall, instead of streaming raw Git objects around?\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422494","messageId":"YH9enUedtHjE87ET@camp.crustytoothpaste.net","threadId":"55522","inReplyTo":"87tuo0q3ma.wl-lukeshu@lukeshu.com","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2021-04-20T23:07:09Z","receivedAt":"2021-04-20T23:07:48Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2021-04-20 at 17:15:25, Luke Shumaker wrote:\n> On Mon, 19 Apr 2021 19:41:55 -0600,\n> brian m. carlson wrote:\n> > I would appreciate it if we did in fact implement it.  I would like to\n> > use this functionality to round-trip objects between SHA-1 and SHA-256,\n> > and it would be nice if both worked.\n> > \n> > The situation with tags is different: the signature using the current\n> > algorithm is always trailing, and the signature for the other algorithm\n> > is in the header.  That wasn't how we intended it to be, but that's how\n> > it ended up being.\n> > \n> > As a result, tag output can support SHA-256 data,\n> \n> I don't believe that's true?  With SHA-1-signed tags, the signature\n> gets included in the fast-import stream as part of the tag message\n> (the `data` line in the BNF).  Since SHA-256-signed tags have their\n> signature as a header (rather than just appending it to the message),\n> we'd have to add a 'gpgsig' sub-command to the 'tag' top-level-command\n> (like I've done to the 'commit' top-level-command).\n\nIf you're using a repository that's SHA-1, then the tag signature that's\npart of the message is a signature over the SHA-1 contents of the\nobject, and the gpgsig-sha256 header is a signature over the SHA-256\ncontents of the object.  If you're using a repository that's SHA-256,\nit's reversed: the signature at the end of the message covers the\nSHA-256 contents of the object and the gpgsig header covers the SHA-1\ncontents.\n\nIt isn't currently possible to create objects with both signatures in\nplace, but that will be possible in the future.\n\n> >                                                   but with your\n> > proposal, SHA-256 commits wouldn't work at all.  Considering SHA-1 is\n> > wildly insecure and therefore signing SHA-1 commits adds very little\n> > security, whereas SHA-256 is presently considered strong, I'd argue that\n> > only supporting SHA-1 isn't the right move here.\n> \n> The main reason I didn't implement SHA-256 support (well, besides that\n> the repo I'm working on turned out to not have any SHA-256-signed\n> commits in it) is that I had questions about SHA-256 that I didn't\n> know/couldn't find the answers to.\n\nCurrently, repositories using SHA-256 currently don't interoperate with\nSHA-1 repositories.  However, if you want to create a test repo, you can\ndo so with \"git init --object-format=sha256\" in an empty directory.\n\nIf you want to run the testsuite in SHA-256 mode, set\nGIT_TEST_DEFAULT_HASH=sha256, and all the repositories created will use\nSHA-256.\n\nThat should be sufficient to get this series such that it will work with\nsimple SHA-256 repos.  If you have more questions about this work or how\nto get things working, I'm happy to answer them.\n\n> However, looking again, I see a few of the answers in\n> t7510-signed-commit.sh, so I'll have a go at it.  If I get stuck, I'll\n> go ahead and implement the below \"gpgsig sha1\" suggestion, and leave\n> the sha256 implementation to someone else.\n\nNot implementing this means the CI will fail when the testsuite is run\nin SHA-256 mode, so your patch probably won't be accepted.\n\n> > Provided we do that and the test suite passes under both algorithms, I'm\n> > strongly in favor of this feature.  In fact, I had been thinking about\n> > implementing this feature myself just the other day, so I'm delighted\n> > you decided to do it.\n> \n> That's one of the big reasons I didn't implement both--I wasn't sure\n> how to test sha256 (within the test harness, `git commit -S` gives a\n> sha1 signature).\n> \n> I see that t7510-signed-commit.sh 'verify-commit verifies multiply\n> signed commits' tests sha256 by hard-coding a raw commit object in the\n> test itself, and feeding that to `git hash-object`.  I'd prefer to\n> figure out how to get `git commit` itself to generate a sha256\n> signature rather than a sha1 signature, so that I can _know_ that I'm\n> getting the ordering of headers the same as `git commit`.  But I don't\n> think that needs to be a blocker; if the test doesn't do the same\n> ordering as `git commit`, I guess that can just be a bugfix later?\n\nYes, dual-signed objects have to manually created right now; there's no\ntooling to create them because that code hasn't landed yet.  It's in my\ntree and very broken.  But you can create SHA-256 repositories as I\nmentioned above and test those, and the testsuite does run in that mode,\nso it should be easy enough to check at least single-signed commits for\nnow, even if you don't implement dual-signed ones.  I think it's fine if\nthat comes later, and I can pick that up as part of a future series.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"422612","messageId":"CABPp-BEz-QykELpvofsrZzbQtQUE0fvijUcaJXhRFbCZpKJuYQ@mail.gmail.com","threadId":"55522","inReplyTo":"20210419225441.3139048-1-lukeshu@lukeshu.com","subject":"Re: [PATCH 0/3] fast-export, fast-import: implement signed-commits","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2021-04-21T18:12:40Z","receivedAt":"2021-04-21T18:12:54Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Mon, Apr 19, 2021 at 3:54 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:\n>\n> From: Luke Shumaker <lukeshu@datawire.io>\n>\n> fast-export has an existing --signed-tags= flag that controls how to\n> handle tag signatures.  However, there is no equivalent for commit\n> signatures; it just silently strips the signature out of the commit\n> (analogously to --signed-tags=strip).\n>\n> So implement a --signed-commits= flag in fast-export, and implement\n> the receiving side of it in fast-import.\n\nI understand adding an option to fast-export, but shouldn't there also\nbe one for fast-import?  In particular, I can see users wanting any of\nthe following:\n\n* I want these signatures exported and imported because I know I won't\ntweak them and they'll still be valid.\n* I want these signatures even though they'll be invalid.  Whatever,\nI'll just deal with it.\n* I want the signatures exported and imported *when they will remain\nvalid*.  Always exporting them makes sense, because fast-export\ndoesn't know about tweaks I'll be making to its output before feeding\nit to fast-import.  But fast-import should have options to\nstrip-if-invalid/warn-if-invalid/error-if-invalid/import-without-warning\nfor these tags (though they don't have to use these exact names).\n\nI know fast-import doesn't do anything of the sort for signed tags,\nbut fast-import also doesn't support signed tags as per this comment\nin the documentation:\n\n\"\"\"\nSigning annotated tags during import from within fast-import is not\nsupported.  Trying to include your own PGP/GPG signature is not\nrecommended, as the frontend does not (easily) have access to the\ncomplete set of bytes which normally goes into such a signature.\nIf signing is required, create lightweight tags from within fast-import with\n`reset`, then create the annotated versions of those tags offline\nwith the standard 'git tag' process.\n\"\"\"\n\nit just happens to \"work\" since the signature is part of the\nannotation and fast-import doesn't attempt to read or validate the\nannotation in any way, treating it as free-from text.  I'd say users\nrelying on this are on somewhat shaky ground.\n\nBut here you're adding explicit fast-import directives to the language\nfor signatures of commits, so you clearly do need to care.  And I\nsuspect fast-import's default should be error-if-invalid rather than\nimport-without-warning.\n\n> Luke Shumaker (3):\n>   git-fast-import.txt: add missing LF in the BNF\n>   fast-export: rename --signed-tags='warn' to 'warn-verbatim'\n>   fast-export, fast-import: implement signed-commits\n>\n>  Documentation/git-fast-export.txt | 18 +++++--\n>  Documentation/git-fast-import.txt |  9 +++-\n>  builtin/fast-export.c             | 88 +++++++++++++++++++++++++------\n>  builtin/fast-import.c             | 15 ++++++\n>  t/t9350-fast-export.sh            | 82 ++++++++++++++++++++++++++++\n>  5 files changed, 191 insertions(+), 21 deletions(-)\n>\n> --\n> 2.31.1\n>\n> Happy hacking,\n> ~ Luke Shumaker\n"},{"id":"422627","messageId":"87fszj309c.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"CABPp-BEz-QykELpvofsrZzbQtQUE0fvijUcaJXhRFbCZpKJuYQ@mail.gmail.com","subject":"Re: [PATCH 0/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-21T19:28:47Z","receivedAt":"2021-04-21T19:31:03Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Wed, 21 Apr 2021 12:12:40 -0600,\nElijah Newren wrote:\n> \n> On Mon, Apr 19, 2021 at 3:54 PM Luke Shumaker <lukeshu@lukeshu.com> wrote:\n> >\n> > From: Luke Shumaker <lukeshu@datawire.io>\n> >\n> > fast-export has an existing --signed-tags= flag that controls how to\n> > handle tag signatures.  However, there is no equivalent for commit\n> > signatures; it just silently strips the signature out of the commit\n> > (analogously to --signed-tags=strip).\n> >\n> > So implement a --signed-commits= flag in fast-export, and implement\n> > the receiving side of it in fast-import.\n> \n> I understand adding an option to fast-export, but shouldn't there also\n> be one for fast-import?  In particular, I can see users wanting any of\n> the following:\n> \n> * I want these signatures exported and imported because I know I won't\n> tweak them and they'll still be valid.\n> * I want these signatures even though they'll be invalid.  Whatever,\n> I'll just deal with it.\n> * I want the signatures exported and imported *when they will remain\n> valid*.  Always exporting them makes sense, because fast-export\n> doesn't know about tweaks I'll be making to its output before feeding\n> it to fast-import.  But fast-import should have options to\n> strip-if-invalid/warn-if-invalid/error-if-invalid/import-without-warning\n> for these tags (though they don't have to use these exact names).\n> \n> I know fast-import doesn't do anything of the sort for signed tags,\n> but fast-import also doesn't support signed tags as per this comment\n> in the documentation:\n> \n> \"\"\"\n> Signing annotated tags during import from within fast-import is not\n> supported.  Trying to include your own PGP/GPG signature is not\n> recommended, as the frontend does not (easily) have access to the\n> complete set of bytes which normally goes into such a signature.\n> If signing is required, create lightweight tags from within fast-import with\n> `reset`, then create the annotated versions of those tags offline\n> with the standard 'git tag' process.\n> \"\"\"\n> \n> it just happens to \"work\" since the signature is part of the\n> annotation and fast-import doesn't attempt to read or validate the\n> annotation in any way, treating it as free-from text.  I'd say users\n> relying on this are on somewhat shaky ground.\n> \n> But here you're adding explicit fast-import directives to the language\n> for signatures of commits, so you clearly do need to care.  And I\n> suspect fast-import's default should be error-if-invalid rather than\n> import-without-warning.\n\nI agree that this would be a good and useful flag to add to\nfast-import.  However, I don't think that it's a necessary thing to\nadd for this work, and I think that it's beyond the scope of what I am\nwilling to implement.\n\nI see where you're coming from when you say that tags and commits are\ndifferent in this regard, but I don't agree.  The signed tags\nsituation does look like shakey ground, but IMO once fast-export\ngained the --signed-tags option, that was saying \"this is the correct\nand stable way of encoding a tag signature in a fast-import stream\".\nThe fact that it is wonky in that it is shoved in to the message is an\naccident of implementation history, and to me doesn't say anything\nabout the nature of the thing.\n\nI think that such a flag in fast-import is just as worthy of existing\nfor tags as it is worthy of existing for commits.\n\nActually, I'm not sure I think such flags belong in fast-import\nitself, I think it may make good sense to have such a filter\nimplemented as an external tool.  Fast-export and fast-import seem to\navoid using the many parts of standard git library functions,\napparently (to me) to avoid allocations because \"fast\".  Given that\ntrouble gone to just to avoid allocations, calling out to GPG seems\nprohibitively slow by comparison.  But I agree such functionality\nwould be good and useful, regardless of whether I think it belongs in\nfast-import itself.\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"},{"id":"422635","messageId":"87eef32t3q.wl-lukeshu@lukeshu.com","threadId":"55522","inReplyTo":"YH9enUedtHjE87ET@camp.crustytoothpaste.net","subject":"Re: [PATCH 3/3] fast-export, fast-import: implement signed-commits","fromName":"Luke Shumaker","fromEmail":"lukeshu@lukeshu.com","sentAt":"2021-04-21T22:03:21Z","receivedAt":"2021-04-21T22:07:22Z","isPatch":true,"sender":{"key":"lukeshu@lukeshu.com","avatar":"https://gravatar.com/avatar/b040e950069ff2e81f026755b364d668aab9d278527aaa2415d0a8845f640bc5?d=mp&s=160"},"body":"On Tue, 20 Apr 2021 17:07:09 -0600,\nbrian m. carlson wrote:\n> On 2021-04-20 at 17:15:25, Luke Shumaker wrote:\n> > I don't believe that's true?  With SHA-1-signed tags, the signature\n> > gets included in the fast-import stream as part of the tag message\n> > (the `data` line in the BNF).  Since SHA-256-signed tags have their\n> > signature as a header (rather than just appending it to the message),\n> > we'd have to add a 'gpgsig' sub-command to the 'tag' top-level-command\n> > (like I've done to the 'commit' top-level-command).\n> \n> If you're using a repository that's SHA-1, then the tag signature that's\n> part of the message is a signature over the SHA-1 contents of the\n> object, and the gpgsig-sha256 header is a signature over the SHA-256\n> contents of the object.  If you're using a repository that's SHA-256,\n> it's reversed: the signature at the end of the message covers the\n> SHA-256 contents of the object and the gpgsig header covers the SHA-1\n> contents.\n\nGood to know!  It seems I've been mislead by\nDocumentation/technical/hash-function-transition.txt\n\n> Not implementing this means the CI will fail when the testsuite is run\n> in SHA-256 mode, so your patch probably won't be accepted.\n\nGotcha.  I guess I will be implementing it then.  I'll let you know if\nI have any further questions, the information you've given already has\nbeen very helpful!\n\n-- \nHappy hacking,\n~ Luke Shumaker\n"}]}