{"thread":{"id":"52570","subject":"[PATCH 0/5] refactor gpg-interface and add gpg verification for clones","startedAt":"2020-01-05T13:56:38Z","lastAt":"2020-01-07T16:54:39Z","messageCount":13,"participants":["Hans Jerry Illikainen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"389243","messageId":"20200105135616.19102-1-hji@dyntopia.com","threadId":"52570","inReplyTo":null,"subject":"[PATCH 0/5] refactor gpg-interface and add gpg verification for clones","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:11Z","receivedAt":"2020-01-05T13:56:38Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"This series starts off with refactor of print_signature_buffer() to make\nall output conditional based on the 'flags' parameter.  The print\nfunction is also extended to optionally show one-line summaries of\nsignature verifications (previously that functionality existed in\nverify_merge_signature()).\n\nThe helper functions for signature verification of commits are then\nrefactored.  The new gpg_verify_commit() function is modelled after\ngpg_verify_tag().  This allows us to remove verify_merge_signature() and\nthe file-local run_gpg_verify() (from the verify-commit builtin).  It\nalso allows us to change check_commit_signature() into a local function\nin commit.c.\n\nA new configuration option is also introduced, gpg.verifySignatures.\nThis allows users to enable signature verification for all operations\nthat support it.  Individual operations can then use\n<operation>.verifySignatures for finer-grained control.\n\nAnd finally, signature verification is added to the clone builtin.  It\nobeys --(no-)verify-signatures, clone.verifySignatures and\ngpg.verifySignatures (in decreasing order of significance).\n\nA notable quirk with signature verification for clones is\n--recurse-submodules.  As mentioned in the commit message, the current\nworkaround is to disable signature verification for submodules by\npassing --no-verify-signatures in submodule--helper.c\n\nI'm very much open to suggestions for a better approach of dealing with\nrecursive clones.  However, I don't think --verify-signatures from the\nclone builtin should propagate to submodules, because that would break a\nworkflow where a user:\n\n1. trust the hash function\n2. has audited an unsigned repository at a given point\n3. has added the repository at that point as a submodule\n4. has signed an object in the super repository where the audited\n   submodule is referenced\n\nSo, I think it'd make more sense to introduce a\nsubmodule.verifySignatures config knob to be used by both\n--recurse-submodules and when the 'submodule' command is used directly.\n\nI hope this patch series isn't too confusing/all over the place. I\nwasn't sure whether the preparatory patches would have made sense in\nisolation, so I opted to send it all in one go.\n\nHans Jerry Illikainen (5):\n  gpg-interface: conditionally show the result in\n    print_signature_buffer()\n  gpg-interface: support one-line summaries in print_signature_buffer()\n  commit: refactor signature verification helpers\n  merge: verify signatures if gpg.verifySignatures is true\n  clone: support signature verification\n\n Documentation/config.txt           |   2 +\n Documentation/config/clone.txt     |   3 +\n Documentation/config/gpg.txt       |   6 +\n Documentation/config/merge.txt     |   4 +-\n Documentation/git-clone.txt        |   4 +\n builtin/clone.c                    |  46 ++++\n builtin/merge.c                    |  22 +-\n builtin/pull.c                     |  18 +-\n builtin/submodule--helper.c        |   6 +\n builtin/tag.c                      |   4 +-\n builtin/verify-commit.c            |  26 +-\n builtin/verify-tag.c               |   4 +-\n commit.c                           |  58 ++--\n commit.h                           |  31 +--\n gpg-interface.c                    |  43 ++-\n gpg-interface.h                    |  11 +-\n pretty.c                           |   3 +-\n t/t5619-clone-verify-signatures.sh | 411 +++++++++++++++++++++++++++++\n t/t7612-merge-verify-signatures.sh |  27 ++\n tag.c                              |  19 +-\n 20 files changed, 633 insertions(+), 115 deletions(-)\n create mode 100644 Documentation/config/clone.txt\n create mode 100755 t/t5619-clone-verify-signatures.sh\n\n--\n2.25.0.rc1.302.gc71d20beed\n"},{"id":"389244","messageId":"20200105135616.19102-2-hji@dyntopia.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"[PATCH 1/5] gpg-interface: conditionally show the result in print_signature_buffer()","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:12Z","receivedAt":"2020-01-05T13:56:38Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"The print_signature_buffer() function in gpg-interface.c is used to\nprint the result of a GPG verified payload.  It takes a 'flags'\nparameter that determines what to print.\n\nPreviously, the 'flags' parameter processed 2 flags:\n\n- GPG_VERIFY_RAW: to print the raw output from GPG instead of the\n  human(ish)-readable output.  One of these outputs were always\n  shown, irregardless of any other flags.\n- GPG_VERIFY_VERBOSE: to print the payload that was verified\n\nInterestingly, there was also a third flag defined in gpg-interface.h;\nGPG_VERIFY_OMIT_STATUS.  That flag wasn't used by the print function\nitself -- instead, callers would check for the presence of\nGPG_VERIFY_OMIT_STATUS before invoking print_signature_buffer().\n\nIt seems reasonable that the GPG interface should handle all flags\nrelated to how the result should be (or shouldn't be) shown.  This patch\nimplements that behavior by removing GPG_VERIFY_OMIT_STATUS and adding\nGPG_VERIFY_FULL.  If neither GPG_VERIFY_FULL nor GPG_VERIFY_VERBOSE is\npresent, then nothing is printed.  This allows callers to invoke\nprint_signature_buffer() unconditionally.\n\nSigned-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n---\n builtin/tag.c           | 4 ++--\n builtin/verify-commit.c | 2 +-\n builtin/verify-tag.c    | 4 ++--\n gpg-interface.c         | 2 +-\n gpg-interface.h         | 6 +++---\n tag.c                   | 4 +---\n 6 files changed, 10 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e0a4c25382..8489e220e8 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -112,10 +112,10 @@ static int verify_tag(const char *name, const char *ref,\n {\n \tint flags;\n \tconst struct ref_format *format = cb_data;\n-\tflags = GPG_VERIFY_VERBOSE;\n+\tflags = GPG_VERIFY_FULL | GPG_VERIFY_VERBOSE;\n \n \tif (format->format)\n-\t\tflags = GPG_VERIFY_OMIT_STATUS;\n+\t\tflags = 0;\n \n \tif (gpg_verify_tag(oid, name, flags))\n \t\treturn -1;\ndiff --git a/builtin/verify-commit.c b/builtin/verify-commit.c\nindex 40c69a0bed..2a099ec6ba 100644\n--- a/builtin/verify-commit.c\n+++ b/builtin/verify-commit.c\n@@ -63,7 +63,7 @@ static int git_verify_commit_config(const char *var, const char *value, void *cb\n int cmd_verify_commit(int argc, const char **argv, const char *prefix)\n {\n \tint i = 1, verbose = 0, had_error = 0;\n-\tunsigned flags = 0;\n+\tunsigned flags = GPG_VERIFY_FULL;\n \tconst struct option verify_commit_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"print commit contents\")),\n \t\tOPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex f45136a06b..bd5e99925b 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -30,7 +30,7 @@ static int git_verify_tag_config(const char *var, const char *value, void *cb)\n int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n {\n \tint i = 1, verbose = 0, had_error = 0;\n-\tunsigned flags = 0;\n+\tunsigned flags = GPG_VERIFY_FULL;\n \tstruct ref_format format = REF_FORMAT_INIT;\n \tconst struct option verify_tag_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n@@ -53,7 +53,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \t\tif (verify_ref_format(&format))\n \t\t\tusage_with_options(verify_tag_usage,\n \t\t\t\t\t   verify_tag_options);\n-\t\tflags |= GPG_VERIFY_OMIT_STATUS;\n+\t\tflags = 0;\n \t}\n \n \twhile (i < argc) {\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex 2d538bcd6e..fc182d39be 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -341,7 +341,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n \tif (flags & GPG_VERIFY_VERBOSE && sigc->payload)\n \t\tfputs(sigc->payload, stdout);\n \n-\tif (output)\n+\tif (flags & GPG_VERIFY_FULL && output)\n \t\tfputs(output, stderr);\n }\n \ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex f4e9b4f371..4631a91330 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -3,9 +3,9 @@\n \n struct strbuf;\n \n-#define GPG_VERIFY_VERBOSE\t\t1\n-#define GPG_VERIFY_RAW\t\t\t2\n-#define GPG_VERIFY_OMIT_STATUS\t4\n+#define GPG_VERIFY_VERBOSE (1 << 0)\n+#define GPG_VERIFY_RAW (1 << 1)\n+#define GPG_VERIFY_FULL (1 << 2)\n \n enum signature_trust_level {\n \tTRUST_UNDEFINED,\ndiff --git a/tag.c b/tag.c\nindex 71b544467e..b8d6da81eb 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -28,9 +28,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \n \tret = check_signature(buf, payload_size, buf + payload_size,\n \t\t\t\tsize - payload_size, &sigc);\n-\n-\tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n-\t\tprint_signature_buffer(&sigc, flags);\n+\tprint_signature_buffer(&sigc, flags);\n \n \tsignature_check_clear(&sigc);\n \treturn ret;\n-- \n2.25.0.rc1.302.gc71d20beed\n\n"},{"id":"389245","messageId":"20200105135616.19102-5-hji@dyntopia.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"[PATCH 4/5] merge: verify signatures if gpg.verifySignatures is true","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:15Z","receivedAt":"2020-01-05T13:56:39Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"Merge operations has had support for a merge.verifySignatures config\nknob for quite some time.  However, there is no global option to enable\nsignature verification for all operations that support it.  This makes\nsense because only merges (and, by extent, pulls) has support for\nconfigurable signature verifications.\n\nHowever, with the upcoming introduction of signature verification for\nclones, it seems reasonable to have a global option that enables\nverification for all operations that support it.  Otherwise, users would\nhave to track down and enable every *.verifySignatures knob.\n\nThis patch adds support for a global gpg.verifySignatures in\ngit_merge_config().  The global variant is overridden by both\nmerge.verifySignatures and the --(no-)verify-signatures command-line\nparameter.\n\nSigned-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n---\n Documentation/config/gpg.txt       |  6 ++++++\n Documentation/config/merge.txt     |  4 +++-\n builtin/merge.c                    |  8 +++++---\n t/t7612-merge-verify-signatures.sh | 27 +++++++++++++++++++++++++++\n 4 files changed, 41 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/gpg.txt b/Documentation/config/gpg.txt\nindex d94025cb36..7bf64cff24 100644\n--- a/Documentation/config/gpg.txt\n+++ b/Documentation/config/gpg.txt\n@@ -33,3 +33,9 @@ gpg.minTrustLevel::\n * `marginal`\n * `fully`\n * `ultimate`\n+\n+gpg.verifySignatures::\n+\tVerify that commits are signed with a valid key for operations\n+\tthat support signature verification.  This option act as a\n+\tglobal default and can be overridden in sections specific to\n+\tindividual operations.\ndiff --git a/Documentation/config/merge.txt b/Documentation/config/merge.txt\nindex 6a313937f8..7ff72fafc2 100644\n--- a/Documentation/config/merge.txt\n+++ b/Documentation/config/merge.txt\n@@ -28,7 +28,9 @@ merge.ff::\n \n merge.verifySignatures::\n \tIf true, this is equivalent to the --verify-signatures command\n-\tline option. See linkgit:git-merge[1] for details.\n+\tline option. See linkgit:git-merge[1] for details.  Also see\n+\t`gpg.verifySignatures` for a global option to enable signature\n+\tverification.\n \n include::fmt-merge-msg.txt[]\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex e472f17738..539dd1dbfc 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -61,7 +61,7 @@ static const char * const builtin_merge_usage[] = {\n static int show_diffstat = 1, shortlog_len = -1, squash;\n static int option_commit = -1;\n static int option_edit = -1;\n-static int allow_trivial = 1, have_message, verify_signatures;\n+static int allow_trivial = 1, have_message, verify_signatures = -1;\n static int overwrite_ignore = 1;\n static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;\n static struct strbuf merge_msg = STRBUF_INIT;\n@@ -610,6 +610,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \t\tshow_diffstat = git_config_bool(k, v);\n \telse if (!strcmp(k, \"merge.verifysignatures\"))\n \t\tverify_signatures = git_config_bool(k, v);\n+\telse if (!strcmp(k, \"gpg.verifysignatures\") && verify_signatures < 0)\n+\t\tverify_signatures = git_config_bool(k, v);\n \telse if (!strcmp(k, \"pull.twohead\"))\n \t\treturn git_config_string(&pull_twohead, k, v);\n \telse if (!strcmp(k, \"pull.octopus\"))\n@@ -1399,7 +1401,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tif (remoteheads->next)\n \t\t\tdie(_(\"Can merge only exactly one commit into empty head\"));\n \n-\t\tif (verify_signatures &&\n+\t\tif (verify_signatures == 1 &&\n \t\t    gpg_verify_commit(&remoteheads->item->object.oid, NULL,\n \t\t\t\t      NULL, gpg_flags))\n \t\t\tdie(_(\"Signature verification failed\"));\n@@ -1423,7 +1425,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_merge_usage,\n \t\t\tbuiltin_merge_options);\n \n-\tif (verify_signatures) {\n+\tif (verify_signatures == 1) {\n \t\tfor (p = remoteheads; p; p = p->next) {\n \t\t\tif (gpg_verify_commit(&p->item->object.oid, NULL, NULL,\n \t\t\t\t\t      gpg_flags))\ndiff --git a/t/t7612-merge-verify-signatures.sh b/t/t7612-merge-verify-signatures.sh\nindex a426f3a89a..10ab2fa119 100755\n--- a/t/t7612-merge-verify-signatures.sh\n+++ b/t/t7612-merge-verify-signatures.sh\n@@ -125,6 +125,33 @@ test_expect_success GPG 'merge commit with bad signature with merge.verifySignat\n \tgit merge --no-verify-signatures $(cat forged.commit)\n '\n \n+test_expect_success GPG 'merge commit with bad signature with gpg.verifySignatures=true and --no-verify-signatures' '\n+\ttest_when_finished \"git reset --hard && git checkout initial\" &&\n+\ttest_config gpg.verifySignatures true &&\n+\tgit merge --no-verify-signatures $(cat forged.commit)\n+'\n+\n+test_expect_success GPG 'merge commit with bad signature with gpg.verifySignatures=true and merge.verifySignatures=false' '\n+\ttest_when_finished \"git reset --hard && git checkout initial\" &&\n+\ttest_config gpg.verifySignatures true &&\n+\ttest_config merge.verifySignatures false &&\n+\tgit merge $(cat forged.commit)\n+'\n+\n+test_expect_success GPG 'merge commit with bad signature with clone.verifySignatures=false and gpg.verifySignatures=true' '\n+\ttest_when_finished \"git reset --hard && git checkout initial\" &&\n+\ttest_config merge.verifySignatures false &&\n+\ttest_config gpg.verifySignatures true &&\n+\tgit merge $(cat forged.commit)\n+'\n+\n+test_expect_success GPG 'merge commit with bad signature with gpg.verifySignatures=true' '\n+\ttest_when_finished \"git reset --hard && git checkout initial\" &&\n+\ttest_config gpg.verifySignatures true &&\n+\ttest_must_fail git merge $(cat forged.commit) 2>mergeerror &&\n+\ttest_i18ngrep \"has a bad GPG signature allegedly by\" mergeerror\n+'\n+\n test_expect_success GPG 'merge unsigned commit into unborn branch' '\n \ttest_when_finished \"git checkout initial\" &&\n \tgit checkout --orphan unborn &&\n-- \n2.25.0.rc1.302.gc71d20beed\n\n"},{"id":"389246","messageId":"20200105135616.19102-3-hji@dyntopia.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"[PATCH 2/5] gpg-interface: support one-line summaries in print_signature_buffer()","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:13Z","receivedAt":"2020-01-05T13:56:39Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"Most consumers of the GPG interface use print_signature_buffer() to show\nthe result from signature verifications.  One notable exception was\nverify_merge_signature().\n\nPreviously, the verify_merge_signature() function processed the\nsignature_check structure in order to print a one-line summary of the\nresult.\n\nI think that the one-line summary format has potential use in other\nparts of Git; and not only for merges.  I also think that it makes sense\nto have /one/ summary format in order to avoid confusing users with\ndifferent output styles for different operations.\n\nThis patch moves the one-line summary from verify_merge_signature() to\nprint_signature_buffer().  It also introduces a GPG_VERIFY_SHORT flag\nthat determines whether the summary should be printed.\n\nSigned-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n---\n builtin/verify-commit.c |  3 ++-\n commit.c                | 23 +++++------------------\n gpg-interface.c         | 41 ++++++++++++++++++++++++++++++++++++++++-\n gpg-interface.h         |  4 +++-\n tag.c                   |  7 ++++---\n 5 files changed, 54 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/verify-commit.c b/builtin/verify-commit.c\nindex 2a099ec6ba..acc01a7be9 100644\n--- a/builtin/verify-commit.c\n+++ b/builtin/verify-commit.c\n@@ -28,7 +28,8 @@ static int run_gpg_verify(struct commit *commit, unsigned flags)\n \tmemset(&signature_check, 0, sizeof(signature_check));\n \n \tret = check_commit_signature(commit, &signature_check);\n-\tprint_signature_buffer(&signature_check, flags);\n+\tprint_signature_buffer(&commit->object.oid, &signature_check, ret,\n+\t\t\t       flags);\n \n \tsignature_check_clear(&signature_check);\n \treturn ret;\ndiff --git a/commit.c b/commit.c\nindex 3f91d3efc5..519599469b 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1139,29 +1139,16 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n void verify_merge_signature(struct commit *commit, int verbosity,\n \t\t\t    int check_trust)\n {\n-\tchar hex[GIT_MAX_HEXSZ + 1];\n \tstruct signature_check signature_check;\n \tint ret;\n \tmemset(&signature_check, 0, sizeof(signature_check));\n \n \tret = check_commit_signature(commit, &signature_check);\n-\n-\tfind_unique_abbrev_r(hex, &commit->object.oid, DEFAULT_ABBREV);\n-\tswitch (signature_check.result) {\n-\tcase 'G':\n-\t\tif (ret || (check_trust && signature_check.trust_level < TRUST_MARGINAL))\n-\t\t\tdie(_(\"Commit %s has an untrusted GPG signature, \"\n-\t\t\t      \"allegedly by %s.\"), hex, signature_check.signer);\n-\t\tbreak;\n-\tcase 'B':\n-\t\tdie(_(\"Commit %s has a bad GPG signature \"\n-\t\t      \"allegedly by %s.\"), hex, signature_check.signer);\n-\tdefault: /* 'N' */\n-\t\tdie(_(\"Commit %s does not have a GPG signature.\"), hex);\n-\t}\n-\tif (verbosity >= 0 && signature_check.result == 'G')\n-\t\tprintf(_(\"Commit %s has a good GPG signature by %s\\n\"),\n-\t\t       hex, signature_check.signer);\n+\tret |= check_trust && signature_check.trust_level < TRUST_MARGINAL;\n+\tprint_signature_buffer(&commit->object.oid, &signature_check, ret,\n+\t\t\t       GPG_VERIFY_SHORT);\n+\tif (ret)\n+\t\tdie(_(\"Signature verification failed\"));\n \n \tsignature_check_clear(&signature_check);\n }\ndiff --git a/gpg-interface.c b/gpg-interface.c\nindex fc182d39be..838ece2b37 100644\n--- a/gpg-interface.c\n+++ b/gpg-interface.c\n@@ -5,6 +5,8 @@\n #include \"gpg-interface.h\"\n #include \"sigchain.h\"\n #include \"tempfile.h\"\n+#include \"object.h\"\n+#include \"object-store.h\"\n \n static char *configured_signing_key;\n static enum signature_trust_level configured_min_trust_level = TRUST_UNDEFINED;\n@@ -333,16 +335,53 @@ int check_signature(const char *payload, size_t plen, const char *signature,\n \treturn !!status;\n }\n \n-void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n+void print_signature_buffer(const struct object_id *oid,\n+\t\t\t    const struct signature_check *sigc, int status,\n+\t\t\t    unsigned flags)\n {\n \tconst char *output = flags & GPG_VERIFY_RAW ?\n \t\tsigc->gpg_status : sigc->gpg_output;\n+\tchar hex[GIT_MAX_HEXSZ + 1];\n+\tchar *type;\n \n \tif (flags & GPG_VERIFY_VERBOSE && sigc->payload)\n \t\tfputs(sigc->payload, stdout);\n \n \tif (flags & GPG_VERIFY_FULL && output)\n \t\tfputs(output, stderr);\n+\n+\tif (flags & GPG_VERIFY_SHORT && oid) {\n+\t\ttype = xstrdup_or_null(\n+\t\t\ttype_name(oid_object_info(the_repository, oid, NULL)));\n+\t\tif (!type)\n+\t\t\ttype = xstrdup(\"object\");\n+\t\t*type = toupper(*type);\n+\n+\t\tfind_unique_abbrev_r(hex, oid, DEFAULT_ABBREV);\n+\n+\t\tswitch (sigc->result) {\n+\t\tcase 'G':\n+\t\t\tif (status)\n+\t\t\t\terror(_(\"%s %s has an untrusted GPG signature, \"\n+\t\t\t\t\t\"allegedly by %s.\"),\n+\t\t\t\t      type, hex, sigc->signer);\n+\t\t\telse\n+\t\t\t\tprintf(_(\"%s %s has a good GPG signature by %s\\n\"),\n+\t\t\t\t       type, hex, sigc->signer);\n+\t\t\tbreak;\n+\t\tcase 'N':\n+\t\t\terror(_(\"%s %s does not have a GPG signature.\"), type,\n+\t\t\t      hex);\n+\t\t\tbreak;\n+\t\tdefault:\n+\t\t\terror(_(\"%s %s has a bad GPG signature \"\n+\t\t\t\t\"allegedly by %s.\"),\n+\t\t\t      type, hex, sigc->signer);\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tfree(type);\n+\t}\n }\n \n size_t parse_signature(const char *buf, size_t size)\ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 4631a91330..2f349f8ed5 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -6,6 +6,7 @@ struct strbuf;\n #define GPG_VERIFY_VERBOSE (1 << 0)\n #define GPG_VERIFY_RAW (1 << 1)\n #define GPG_VERIFY_FULL (1 << 2)\n+#define GPG_VERIFY_SHORT (1 << 3)\n \n enum signature_trust_level {\n \tTRUST_UNDEFINED,\n@@ -60,7 +61,8 @@ const char *get_signing_key(void);\n int check_signature(const char *payload, size_t plen,\n \t\t    const char *signature, size_t slen,\n \t\t    struct signature_check *sigc);\n-void print_signature_buffer(const struct signature_check *sigc,\n+void print_signature_buffer(const struct object_id *oid,\n+\t\t\t    const struct signature_check *sigc, int status,\n \t\t\t    unsigned flags);\n \n #endif\ndiff --git a/tag.c b/tag.c\nindex b8d6da81eb..f6ad4171f9 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -10,7 +10,8 @@\n \n const char *tag_type = \"tag\";\n \n-static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n+static int run_gpg_verify(const struct object_id *oid, const char *buf,\n+\t\t\t  unsigned long size, unsigned flags)\n {\n \tstruct signature_check sigc;\n \tsize_t payload_size;\n@@ -28,7 +29,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n \n \tret = check_signature(buf, payload_size, buf + payload_size,\n \t\t\t\tsize - payload_size, &sigc);\n-\tprint_signature_buffer(&sigc, flags);\n+\tprint_signature_buffer(oid, &sigc, ret, flags);\n \n \tsignature_check_clear(&sigc);\n \treturn ret;\n@@ -57,7 +58,7 @@ int gpg_verify_tag(const struct object_id *oid, const char *name_to_report,\n \t\t\t\tname_to_report :\n \t\t\t\tfind_unique_abbrev(oid, DEFAULT_ABBREV));\n \n-\tret = run_gpg_verify(buf, size, flags);\n+\tret = run_gpg_verify(oid, buf, size, flags);\n \n \tfree(buf);\n \treturn ret;\n-- \n2.25.0.rc1.302.gc71d20beed\n\n"},{"id":"389247","messageId":"20200105135616.19102-4-hji@dyntopia.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"[PATCH 3/5] commit: refactor signature verification helpers","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:14Z","receivedAt":"2020-01-05T13:56:39Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"Previously, signature verification for commits had a number of helper\nfunctions with slightly different behaviors:\n\n- builtin/verify-commit.c had a file-local helper, so it wasn't reusable\n  outside of the verify-commit builtin.\n- commit.c had verify_merge_signature() that die()d on errors, making it\n  difficult to reuse in parts of Git that mustn't (immediately) die on\n  failure.\n- commit.c also had check_commit_signature().  It's flexible enough to\n  be used anywhere, but it isn't as nice as gpg_verify_tag().  More\n  specifically, it doesn't take care of printing the result; nor does it\n  accept and parse arbitrary object IDs.\n\nThis commit changes check_commit_signature() to file-local scope.  It\nalso introduces a gpg_verify_commit() function modelled after\ngpg_verify_tag().  It is written with the intent of removing the need to\nimplement local helpers for operations that verifies commit signatures.\nThis should hopefully make the code paths to the GPG interface easier to\nfollow.\n\nSigned-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n---\n builtin/merge.c         | 16 ++++++++------\n builtin/pull.c          | 18 +++++-----------\n builtin/verify-commit.c | 25 +---------------------\n commit.c                | 47 ++++++++++++++++++++++++++++++-----------\n commit.h                | 31 ++++++---------------------\n gpg-interface.h         |  1 +\n pretty.c                |  3 ++-\n 7 files changed, 60 insertions(+), 81 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex d127d2225f..e472f17738 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -62,8 +62,8 @@ static int show_diffstat = 1, shortlog_len = -1, squash;\n static int option_commit = -1;\n static int option_edit = -1;\n static int allow_trivial = 1, have_message, verify_signatures;\n-static int check_trust_level = 1;\n static int overwrite_ignore = 1;\n+static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;\n static struct strbuf merge_msg = STRBUF_INIT;\n static struct strategy **use_strategies;\n static size_t use_strategies_nr, use_strategies_alloc;\n@@ -633,7 +633,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n \t\treturn 0;\n \t} else if (!strcmp(k, \"gpg.mintrustlevel\")) {\n-\t\tcheck_trust_level = 0;\n+\t\tgpg_flags ^= GPG_VERIFY_COMPAT;\n \t}\n \n \tstatus = fmt_merge_msg_config(k, v, cb);\n@@ -1399,9 +1399,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\tif (remoteheads->next)\n \t\t\tdie(_(\"Can merge only exactly one commit into empty head\"));\n \n-\t\tif (verify_signatures)\n-\t\t\tverify_merge_signature(remoteheads->item, verbosity,\n-\t\t\t\t\t       check_trust_level);\n+\t\tif (verify_signatures &&\n+\t\t    gpg_verify_commit(&remoteheads->item->object.oid, NULL,\n+\t\t\t\t      NULL, gpg_flags))\n+\t\t\tdie(_(\"Signature verification failed\"));\n \n \t\tremote_head_oid = &remoteheads->item->object.oid;\n \t\tread_empty(remote_head_oid, 0);\n@@ -1424,8 +1425,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \n \tif (verify_signatures) {\n \t\tfor (p = remoteheads; p; p = p->next) {\n-\t\t\tverify_merge_signature(p->item, verbosity,\n-\t\t\t\t\t       check_trust_level);\n+\t\t\tif (gpg_verify_commit(&p->item->object.oid, NULL, NULL,\n+\t\t\t\t\t      gpg_flags))\n+\t\t\t\tdie(_(\"Signature verification failed\"));\n \t\t}\n \t}\n \ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex d4e3e77c8e..e41c4032ae 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -107,11 +107,11 @@ static char *opt_ff;\n static char *opt_verify_signatures;\n static int opt_autostash = -1;\n static int config_autostash;\n-static int check_trust_level = 1;\n static struct argv_array opt_strategies = ARGV_ARRAY_INIT;\n static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;\n static char *opt_gpg_sign;\n static int opt_allow_unrelated_histories;\n+static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;\n \n /* Options passed to git-fetch */\n static char *opt_all;\n@@ -366,7 +366,7 @@ static int git_pull_config(const char *var, const char *value, void *cb)\n \t\t\tRECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;\n \t\treturn 0;\n \t} else if (!strcmp(var, \"gpg.mintrustlevel\")) {\n-\t\tcheck_trust_level = 0;\n+\t\tgpg_flags ^= GPG_VERIFY_COMPAT;\n \t}\n \n \tstatus = git_gpg_config(var, value, cb);\n@@ -589,17 +589,9 @@ static int run_fetch(const char *repo, const char **refspecs)\n static int pull_into_void(const struct object_id *merge_head,\n \t\tconst struct object_id *curr_head)\n {\n-\tif (opt_verify_signatures) {\n-\t\tstruct commit *commit;\n-\n-\t\tcommit = lookup_commit(the_repository, merge_head);\n-\t\tif (!commit)\n-\t\t\tdie(_(\"unable to access commit %s\"),\n-\t\t\t    oid_to_hex(merge_head));\n-\n-\t\tverify_merge_signature(commit, opt_verbosity,\n-\t\t\t\t       check_trust_level);\n-\t}\n+\tif (opt_verify_signatures)\n+\t\tif (gpg_verify_commit(merge_head, NULL, NULL, gpg_flags))\n+\t\t\treturn 1;\n \n \t/*\n \t * Two-way merge: we treat the index as based on an empty tree,\ndiff --git a/builtin/verify-commit.c b/builtin/verify-commit.c\nindex acc01a7be9..3f392654dd 100644\n--- a/builtin/verify-commit.c\n+++ b/builtin/verify-commit.c\n@@ -20,37 +20,14 @@ static const char * const verify_commit_usage[] = {\n \t\tNULL\n };\n \n-static int run_gpg_verify(struct commit *commit, unsigned flags)\n-{\n-\tstruct signature_check signature_check;\n-\tint ret;\n-\n-\tmemset(&signature_check, 0, sizeof(signature_check));\n-\n-\tret = check_commit_signature(commit, &signature_check);\n-\tprint_signature_buffer(&commit->object.oid, &signature_check, ret,\n-\t\t\t       flags);\n-\n-\tsignature_check_clear(&signature_check);\n-\treturn ret;\n-}\n-\n static int verify_commit(const char *name, unsigned flags)\n {\n \tstruct object_id oid;\n-\tstruct object *obj;\n \n \tif (get_oid(name, &oid))\n \t\treturn error(\"commit '%s' not found.\", name);\n \n-\tobj = parse_object(the_repository, &oid);\n-\tif (!obj)\n-\t\treturn error(\"%s: unable to read file.\", name);\n-\tif (obj->type != OBJ_COMMIT)\n-\t\treturn error(\"%s: cannot verify a non-commit object of type %s.\",\n-\t\t\t\tname, type_name(obj->type));\n-\n-\treturn run_gpg_verify((struct commit *)obj, flags);\n+\treturn gpg_verify_commit(&oid, NULL, NULL, flags);\n }\n \n static int git_verify_commit_config(const char *var, const char *value, void *cb)\ndiff --git a/commit.c b/commit.c\nindex 519599469b..f8339916a6 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1116,7 +1116,8 @@ static void handle_signed_tag(struct commit *parent, struct commit_extra_header\n \tfree(buf);\n }\n \n-int check_commit_signature(const struct commit *commit, struct signature_check *sigc)\n+static int check_commit_signature(const struct commit *commit,\n+\t\t\t\t  struct signature_check *sigc)\n {\n \tstruct strbuf payload = STRBUF_INIT;\n \tstruct strbuf signature = STRBUF_INIT;\n@@ -1136,21 +1137,43 @@ int check_commit_signature(const struct commit *commit, struct signature_check *\n \treturn ret;\n }\n \n-void verify_merge_signature(struct commit *commit, int verbosity,\n-\t\t\t    int check_trust)\n+int gpg_verify_commit(const struct object_id *oid, const char *name_to_report,\n+\t\t      struct signature_check *sigc, unsigned flags)\n {\n-\tstruct signature_check signature_check;\n+\tstruct object *obj;\n+\tstruct signature_check tmp = { 0 };\n \tint ret;\n-\tmemset(&signature_check, 0, sizeof(signature_check));\n \n-\tret = check_commit_signature(commit, &signature_check);\n-\tret |= check_trust && signature_check.trust_level < TRUST_MARGINAL;\n-\tprint_signature_buffer(&commit->object.oid, &signature_check, ret,\n-\t\t\t       GPG_VERIFY_SHORT);\n-\tif (ret)\n-\t\tdie(_(\"Signature verification failed\"));\n+\tif (!sigc)\n+\t\tsigc = &tmp;\n \n-\tsignature_check_clear(&signature_check);\n+\tobj = parse_object(the_repository, oid);\n+\tif (!obj)\n+\t\treturn error(\"%s: unable to read file.\",\n+\t\t\t     name_to_report ?\n+\t\t\t     name_to_report :\n+\t\t\t     find_unique_abbrev(oid, DEFAULT_ABBREV));\n+\n+\tif (obj->type != OBJ_COMMIT)\n+\t\treturn error(\"%s: cannot verify a non-commit object of type %s.\",\n+\t\t\t     name_to_report ?\n+\t\t\t     name_to_report :\n+\t\t\t     find_unique_abbrev(oid, DEFAULT_ABBREV),\n+\t\t\t     type_name(obj->type));\n+\n+\tret = check_commit_signature((struct commit *)obj, sigc);\n+\t/*\n+\t * Merge operations has historically required a trust level of\n+\t * 'marginal' or higher as a default.  Backward-compatibility is\n+\t * maintained here -- however, new users of this function should\n+\t * delegate trust level verification to the GPG interface.\n+\t */\n+\tret |= flags & GPG_VERIFY_COMPAT && sigc->trust_level < TRUST_MARGINAL;\n+\n+\tprint_signature_buffer(oid, sigc, ret, flags);\n+\tsignature_check_clear(&tmp);\n+\n+\treturn ret;\n }\n \n void append_merge_tag_headers(struct commit_list *parents,\ndiff --git a/commit.h b/commit.h\nindex 008a0fa4a0..a31aaa7304 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -364,13 +364,14 @@ int parse_signed_commit(const struct commit *commit,\n int remove_signature(struct strbuf *buf);\n \n /*\n- * Check the signature of the given commit. The result of the check is stored\n- * in sig->check_result, 'G' for a good signature, 'U' for a good signature\n- * from an untrusted signer, 'B' for a bad signature and 'N' for no signature\n- * at all.  This may allocate memory for sig->gpg_output, sig->gpg_status,\n- * sig->signer and sig->key.\n+ * Check the signature of the given commit.  The result is stored in sigc if it\n+ * is non-NULL.  However, note that a return code of 0 from this function\n+ * should be used to determine if the signature verified successfully, because\n+ * multiple members in the signature_check structure are needed to sufficiently\n+ * determine the outcome.\n  */\n-int check_commit_signature(const struct commit *commit, struct signature_check *sigc);\n+int gpg_verify_commit(const struct object_id *oid, const char *name_to_report,\n+\t\t      struct signature_check *sigc, unsigned flags);\n \n /* record author-date for each commit object */\n struct author_date_slab;\n@@ -378,24 +379,6 @@ void record_author_date(struct author_date_slab *author_date,\n \t\t\tstruct commit *commit);\n \n int compare_commits_by_author_date(const void *a_, const void *b_, void *unused);\n-\n-/*\n- * Verify a single commit with check_commit_signature() and die() if it is not\n- * a good signature. This isn't really suitable for general use, but is a\n- * helper to implement consistent logic for pull/merge --verify-signatures.\n- *\n- * The check_trust parameter is meant for backward-compatibility.  The GPG\n- * interface verifies key trust with a default trust level that is below the\n- * default trust level for merge operations.  Its value should be non-zero if\n- * the user hasn't set a minimum trust level explicitly in their configuration.\n- *\n- * If the user has set a minimum trust level, then that value should be obeyed\n- * and check_trust should be zero, even if the configured trust level is below\n- * the default trust level for merges.\n- */\n-void verify_merge_signature(struct commit *commit, int verbose,\n-\t\t\t    int check_trust);\n-\n int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused);\n int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused);\n \ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex 2f349f8ed5..3f619cce0e 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -7,6 +7,7 @@ struct strbuf;\n #define GPG_VERIFY_RAW (1 << 1)\n #define GPG_VERIFY_FULL (1 << 2)\n #define GPG_VERIFY_SHORT (1 << 3)\n+#define GPG_VERIFY_COMPAT (1 << 4)\n \n enum signature_trust_level {\n \tTRUST_UNDEFINED,\ndiff --git a/pretty.c b/pretty.c\nindex f5fbbc5ffb..c19620ffe6 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1302,7 +1302,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tif (placeholder[0] == 'G') {\n \t\tif (!c->signature_check.result)\n-\t\t\tcheck_commit_signature(c->commit, &(c->signature_check));\n+\t\t\tgpg_verify_commit(&c->commit->object.oid, NULL,\n+\t\t\t\t\t  &(c->signature_check), 0);\n \t\tswitch (placeholder[1]) {\n \t\tcase 'G':\n \t\t\tif (c->signature_check.gpg_output)\n-- \n2.25.0.rc1.302.gc71d20beed\n\n"},{"id":"389248","messageId":"20200105135616.19102-6-hji@dyntopia.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"[PATCH 5/5] clone: support signature verification","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-05T13:56:16Z","receivedAt":"2020-01-05T13:56:39Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"The merge operation (and as a consequence, pull) has had support for\nsignature verification for quite some time.  However, there were no\nsupport for verifying GPG signatures for the initial clone.\n\nWithout signature verification for clone operations, users are forced to\ncheckout a repository before using verify-commit or verify-tag on the\ntree.  This is potentially problematic for a few reasons:\n\n- It's possible to forget to verify the tree when there's nothing that\n  enforce signature verification before a cloned repository is used.\n\n- Software on the users system might process the tree before it has been\n  successfully verified -- for example, file managers that automagically\n  creates thumbnails for images, videos and documents.\n\nNow, this could be worked around with a --no-checkout clone followed by\nsignature verification and checkout.  There are also various scripts\nfloating around that more-or-less re-implements 'git clone' with\nsignature verification by using a combination of init + remote add +\nfetch + verify-commit + merge [1].  But none of these options are\nparticularly user-friendly.\n\nThis patch implements signature verification for the clone builtin to\naccommodate use-cases where the tree should be verified before use.\n\nThere is one major quirk in this patch; namely, recursive clones of\nsubmodules.  It is worked around by passing --no-verify-signatures in\nclone_submodule().  The rationale for this approach is that:\n\n- The object ID for submodules are stored in the super repository.\n  Thus, if the super is trusted and signed (and the hash function is\n  secure), then it seems reasonable to also trust the submodules\n  referenced in the super repository.\n\n- Propagating signature verification to recursive clones of submodules\n  would lessen the use of signature verification for clones, because\n  then users would need the keys for each submodule author in their\n  keyring.  Also, not all submodule refs may be signed.\n\n[1] https://gist.github.com/tribut/50c0f7d0b8341fa6d1784c317d5275f0\n\nSigned-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n---\n Documentation/config.txt           |   2 +\n Documentation/config/clone.txt     |   3 +\n Documentation/git-clone.txt        |   4 +\n builtin/clone.c                    |  46 ++++\n builtin/submodule--helper.c        |   6 +\n t/t5619-clone-verify-signatures.sh | 411 +++++++++++++++++++++++++++++\n tag.c                              |  10 +-\n 7 files changed, 478 insertions(+), 4 deletions(-)\n create mode 100644 Documentation/config/clone.txt\n create mode 100755 t/t5619-clone-verify-signatures.sh\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 83e7bba872..fda69e660e 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -331,6 +331,8 @@ include::config/checkout.txt[]\n \n include::config/clean.txt[]\n \n+include::config/clone.txt[]\n+\n include::config/color.txt[]\n \n include::config/column.txt[]\ndiff --git a/Documentation/config/clone.txt b/Documentation/config/clone.txt\nnew file mode 100644\nindex 0000000000..9fd2ee3395\n--- /dev/null\n+++ b/Documentation/config/clone.txt\n@@ -0,0 +1,3 @@\n+clone.verifySignatures::\n+\tIf true, this is equivalent to the --verify-signatures command\n+\tline option. See linkgit:git-clone[1] for details.\ndiff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt\nindex bf24f1813a..47a9d1e182 100644\n--- a/Documentation/git-clone.txt\n+++ b/Documentation/git-clone.txt\n@@ -285,6 +285,10 @@ or `--mirror` is given)\n \tThe number of submodules fetched at the same time.\n \tDefaults to the `submodule.fetchJobs` option.\n \n+--verify-signatures::\n+--no-verify-signatures::\n+\tVerify that the newly created HEAD is signed with a valid key.\n+\n <repository>::\n \tThe (possibly remote) repository to clone from.  See the\n \t<<URLS,GIT URLS>> section below for more information on specifying\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 0fc89ae2b9..037953bc4b 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -32,6 +32,8 @@\n #include \"connected.h\"\n #include \"packfile.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"object.h\"\n+#include \"tag.h\"\n \n /*\n  * Overall FIXMEs:\n@@ -54,6 +56,7 @@ static int deepen;\n static char *option_template, *option_depth, *option_since;\n static char *option_origin = NULL;\n static char *option_branch = NULL;\n+static int option_verify_signatures = -1;\n static struct string_list option_not = STRING_LIST_INIT_NODUP;\n static const char *real_git_dir;\n static char *option_upload_pack = \"git-upload-pack\";\n@@ -120,6 +123,8 @@ static struct option builtin_clone_options[] = {\n \t\t   N_(\"use <name> instead of 'origin' to track upstream\")),\n \tOPT_STRING('b', \"branch\", &option_branch, N_(\"branch\"),\n \t\t   N_(\"checkout <branch> instead of the remote's HEAD\")),\n+\tOPT_BOOL(0, \"verify-signatures\", &option_verify_signatures,\n+\t\t N_(\"verify the GPG signature of the newly created HEAD\")),\n \tOPT_STRING('u', \"upload-pack\", &option_upload_pack, N_(\"path\"),\n \t\t   N_(\"path to git-upload-pack on the remote\")),\n \tOPT_STRING(0, \"depth\", &option_depth, N_(\"depth\"),\n@@ -929,6 +934,40 @@ static int path_exists(const char *path)\n \treturn !stat(path, &sb);\n }\n \n+static int verify_signature(const struct ref *our, const struct ref *remote)\n+{\n+\tconst struct object_id *oid = our ? &our->old_oid : &remote->old_oid;\n+\tenum object_type type = oid_object_info(the_repository, oid, NULL);\n+\tint flags = option_verbosity ? GPG_VERIFY_FULL : GPG_VERIFY_SHORT;\n+\n+\tif (type == OBJ_COMMIT)\n+\t\treturn gpg_verify_commit(oid, NULL, NULL, flags);\n+\tif (type == OBJ_TAG)\n+\t\treturn gpg_verify_tag(oid, NULL, flags);\n+\treturn error(_(\"%s: unknown object type: %s\"),\n+\t\t     find_unique_abbrev(oid, DEFAULT_ABBREV), type_name(type));\n+}\n+\n+static int git_clone_config(const char *var, const char *value, void *cb)\n+{\n+\tint status;\n+\n+\tif (!strcmp(var, \"clone.verifysignatures\")) {\n+\t\toption_verify_signatures = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n+\tif (!strcmp(var, \"gpg.verifysignatures\") &&\n+\t    option_verify_signatures < 0)\n+\t\toption_verify_signatures = git_config_bool(var, value);\n+\n+\tstatus = git_gpg_config(var, value, cb);\n+\tif (status)\n+\t\treturn status;\n+\n+\treturn git_default_config(var, value, cb);\n+}\n+\n int cmd_clone(int argc, const char **argv, const char *prefix)\n {\n \tint is_bundle = 0, is_local;\n@@ -952,6 +991,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstruct argv_array ref_prefixes = ARGV_ARRAY_INIT;\n \n \tpacket_trace_identity(\"clone\");\n+\n+\tgit_config(git_clone_config, NULL);\n+\n \targc = parse_options(argc, argv, prefix, builtin_clone_options,\n \t\t\t     builtin_clone_usage, 0);\n \n@@ -1267,6 +1309,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \telse if (refs && complete_refs_before_fetch)\n \t\ttransport_fetch_refs(transport, mapped_refs);\n \n+\tif (option_verify_signatures > 0)\n+\t\tif (verify_signature(our_head_points_at, remote_head))\n+\t\t\tdie(_(\"Signature verification failed\"));\n+\n \tupdate_remote_refs(refs, mapped_refs, remote_head_points_at,\n \t\t\t   branch_top.buf, reflog_msg.buf, transport,\n \t\t\t   !is_local, filter_options.choice);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c72931ecd7..9e69c767c9 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1248,6 +1248,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url\n \tif (gitdir && *gitdir)\n \t\targv_array_pushl(&cp.args, \"--separate-git-dir\", gitdir, NULL);\n \n+\t/*\n+\t * The --no-verify-signatures parameter has to be passed in order to\n+\t * make verification of super-repositories work on recursive clones.\n+\t */\n+\targv_array_push(&cp.args, \"--no-verify-signatures\");\n+\n \targv_array_push(&cp.args, \"--\");\n \targv_array_push(&cp.args, url);\n \targv_array_push(&cp.args, path);\ndiff --git a/t/t5619-clone-verify-signatures.sh b/t/t5619-clone-verify-signatures.sh\nnew file mode 100755\nindex 0000000000..7a93d57c2a\n--- /dev/null\n+++ b/t/t5619-clone-verify-signatures.sh\n@@ -0,0 +1,411 @@\n+#!/bin/sh\n+\n+test_description='Test cloning repos with signature verification'\n+\n+. ./test-lib.sh\n+. \"$TEST_DIRECTORY/lib-gpg.sh\"\n+\n+test_expect_success GPG 'create repositories with signed commits and tags' '\n+\techo 0 >a && git add a &&\n+\ttest_tick && git commit -m \"initial-unsigned\" &&\n+\tgit tag -a -m \"unsigned v0\" v0-unsigned &&\n+\n+\tgit clone . signed &&\n+\t(\n+\t\tcd signed &&\n+\t\techo 1 >b && git add b &&\n+\t\ttest_tick && git commit -S -m \"signed\" &&\n+\t\tgit branch signed-branch &&\n+\t\tgit tag -s -m \"signed v1\" v1-signed\n+\t) &&\n+\n+\tgit clone . unsigned &&\n+\t(\n+\t\tcd unsigned &&\n+\t\techo 2 >c && git add c &&\n+\t\ttest_tick && git commit -m \"unsigned\" &&\n+\t\tgit tag v2-unsigned-shallow &&\n+\t\tgit tag -a -m \"unsigned and annotated\" v2-unsigned-annotated\n+\t) &&\n+\n+\tgit clone signed unsigned-tip &&\n+\t(\n+\t\tcd unsigned-tip &&\n+\t\techo 3 >d && git add d &&\n+\t\ttest_tick && git commit -m \"unsigned tip\" &&\n+\t\tgit tag -a -m \"unsigned v3 tip\" v3-unsigned-tip &&\n+\t\tgit branch signed-branch origin/signed-branch\n+\t) &&\n+\n+\tgit clone signed unsigned-branch &&\n+\t(\n+\t\tcd unsigned-branch &&\n+\t\tgit checkout -b unsigned-branch &&\n+\t\tgit commit --amend --no-edit &&\n+\t\tgit checkout master\n+\t) &&\n+\n+\tgit clone . signed-tag-unsigned-commit &&\n+\t(\n+\t\tcd signed-tag-unsigned-commit &&\n+\t\tgit tag -s -m \"signed/unsigned v4\" v4-signed-tag-unsigned-commit\n+\t) &&\n+\n+\tgit clone . bad &&\n+\t(\n+\t\tcd bad &&\n+\t\techo 4 >d && git add d &&\n+\t\ttest_tick && git commit -S -m \"bad\" &&\n+\t\tgit cat-file commit HEAD >raw &&\n+\t\tsed -e \"s/^bad/forged bad/\" raw >forged &&\n+\t\tgit hash-object -w -t commit forged >forged.commit &&\n+\t\tgit checkout $(cat forged.commit)\n+\t) &&\n+\n+\tgit clone . untrusted &&\n+\t(\n+\t\tcd untrusted &&\n+\t\techo 5 >e && git add e &&\n+\t\ttest_tick && git commit -SB7227189 -m \"untrusted\"\n+\t) &&\n+\n+\tgit clone unsigned unsigned-detached &&\n+\t(\n+\t\tcd unsigned-detached &&\n+\t\techo 6 >f && git add f &&\n+\t\ttest_tick && git commit -S -m \"signed\" &&\n+\t\tgit checkout HEAD^\n+\t) &&\n+\n+\tgit clone signed signed-detached &&\n+\t(\n+\t\tcd signed-detached &&\n+\t\techo 7 >g && git add g &&\n+\t\ttest_tick && git commit -S -m \"signed\" &&\n+\t\tgit checkout HEAD^\n+\t) &&\n+\n+\tgit clone signed signed-with-unsigned-submodule &&\n+\t(\n+\t\tcd signed-with-unsigned-submodule &&\n+\t\tgit submodule add \"file://$PWD/../unsigned\" &&\n+\t\tgit commit -S -m \"add submodule\"\n+\t) &&\n+\n+\tgit clone signed signed-with-signed-submodule &&\n+\t(\n+\t\tcd signed-with-signed-submodule &&\n+\t\tgit submodule add \"file://$PWD/../signed\" &&\n+\t\tgit commit -S -m \"add submodule\"\n+\t) &&\n+\n+\tgit clone unsigned unsigned-with-unsigned-submodule &&\n+\t(\n+\t\tcd unsigned-with-unsigned-submodule &&\n+\t\tgit submodule add \"file://$PWD/../unsigned\" &&\n+\t\tgit commit -m \"add submodule\"\n+\t) &&\n+\n+\tgit clone unsigned unsigned-with-signed-submodule &&\n+\t(\n+\t\tcd unsigned-with-signed-submodule &&\n+\t\tgit submodule add \"file://$PWD/../signed\" &&\n+\t\tgit commit -m \"add submodule\"\n+\t)\n+'\n+\n+test_expect_success GPG 'clone signed with --verify-signatures' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit clone --verify-signatures signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed tag with --verify-signatures' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit clone -b v1-signed --verify-signatures signed dst >out &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with --depth=1 and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --depth=1 signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with --no-checkout and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --no-checkout signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with --mirror and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --mirror signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed without blobs and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --filter=blob:none signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed bare with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --bare signed dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed tag with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone -b v1-signed signed dst >out &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with defaults' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit clone unsigned dst >out 2>&1 &&\n+\t! test_i18ngrep \"GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --depth=1 and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --depth=1 unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --no-checkout and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --no-checkout unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --mirror and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --no-checkout unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned without blobs and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --filter=blob:none unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned bare with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --bare unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --verify-signatures and clone.verifySignatures=false' '\n+\ttest_config_global clone.verifySignatures false &&\n+\ttest_must_fail git clone --verify-signatures unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --no-verify-signatures and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --no-verify-signatures unsigned dst >out &&\n+\t! test_i18ngrep \"GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with --no-verify-signatures and gpg.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global gpg.verifySignatures true &&\n+\tgit clone --no-verify-signatures unsigned dst >out &&\n+\t! test_i18ngrep \"GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with clone.verifySignatures=false and gpg.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures false &&\n+\ttest_config_global gpg.verifySignatures true &&\n+\tgit clone unsigned dst >out &&\n+\t! test_i18ngrep \"GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with gpg.verifySignatures=true and clone.verifySignatures=false' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global gpg.verifySignatures true &&\n+\ttest_config_global clone.verifySignatures false &&\n+\tgit clone unsigned dst >out &&\n+\t! test_i18ngrep \"GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with gpg.verifySignatures=true' '\n+\ttest_config_global gpg.verifySignatures true &&\n+\ttest_must_fail git clone unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone bad signature with --verbose and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --verbose bad dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"gpg: BAD signature from \" out\n+'\n+\n+test_expect_success GPG 'clone bad signature with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone bad dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a bad GPG signature \" out\n+'\n+\n+test_expect_success GPG 'clone untrusted with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone untrusted dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone untrusted with clone.verifySignatures=true and gpg.minTrustLevel=fully' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_config_global gpg.minTrustLevel fully &&\n+\ttest_must_fail git clone untrusted dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has an untrusted GPG signature\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned tip with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone unsigned-tip dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned tip tag with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone -b v3-unsigned-tip unsigned-tip dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone signed tag from unsigned tip tag with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone -b v1-signed unsigned-tip dst >out &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed branch from unsigned tip tag with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone -b signed-branch unsigned-tip dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone unsigned branch with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone -b unsigned-branch unsigned-branch dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned shallow tag with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone -b v2-unsigned-shallow unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned annotated tag with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone -b v2-unsigned-annotated unsigned dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone signed tag for unsigned commit with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone -b v4-signed-tag-unsigned-commit signed-tag-unsigned-commit dst >out &&\n+\ttest_i18ngrep \"Tag [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone unsigned detached HEAD with clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone unsigned-detached dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone signed detached HEAD with clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone signed-detached dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with unsigned submodules and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --recurse-submodules signed-with-unsigned-submodule dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with unsigned submodules and --verify-signatures' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\tgit clone --recurse-submodules --verify-signatures \\\n+\t\tsigned-with-unsigned-submodule dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone signed with signed submodules and clone.verifySignatures=true' '\n+\ttest_when_finished \"rm -rf dst\" &&\n+\ttest_config_global clone.verifySignatures true &&\n+\tgit clone --recurse-submodules signed-with-signed-submodule dst >out &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ has a good GPG signature by \" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with signed submodules and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --recurse-submodules unsigned-with-signed-submodule dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with signed submodules and --verify-signatures' '\n+\ttest_must_fail git clone --recurse-submodules --verify-signatures \\\n+\t\tunsigned-with-signed-submodule dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_expect_success GPG 'clone unsigned with unsigned submodules and clone.verifySignatures=true' '\n+\ttest_config_global clone.verifySignatures true &&\n+\ttest_must_fail git clone --recurse-submodules unsigned-with-unsigned-submodule dst 2>out &&\n+\ttest_path_is_missing dst &&\n+\ttest_i18ngrep \"Commit [0-9a-f]\\+ does not have a GPG signature.\" out\n+'\n+\n+test_done\ndiff --git a/tag.c b/tag.c\nindex f6ad4171f9..deff55198a 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -13,17 +13,19 @@ const char *tag_type = \"tag\";\n static int run_gpg_verify(const struct object_id *oid, const char *buf,\n \t\t\t  unsigned long size, unsigned flags)\n {\n-\tstruct signature_check sigc;\n+\tstruct signature_check sigc = { .result = 'N' };\n \tsize_t payload_size;\n-\tint ret;\n-\n-\tmemset(&sigc, 0, sizeof(sigc));\n+\tint ret = 1;\n \n \tpayload_size = parse_signature(buf, size);\n \n \tif (size == payload_size) {\n \t\tif (flags & GPG_VERIFY_VERBOSE)\n \t\t\twrite_in_full(1, buf, payload_size);\n+\n+\t\t/* maybe print a detailed error message */\n+\t\tprint_signature_buffer(oid, &sigc, ret, flags);\n+\n \t\treturn error(\"no signature found\");\n \t}\n \n-- \n2.25.0.rc1.302.gc71d20beed\n\n"},{"id":"389256","messageId":"xmqq36ctbis8.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"20200105135616.19102-1-hji@dyntopia.com","subject":"Re: [PATCH 0/5] refactor gpg-interface and add gpg verification for clones","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-05T23:11:35Z","receivedAt":"2020-01-05T23:11:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n> And finally, signature verification is added to the clone builtin.  It\n> obeys --(no-)verify-signatures, clone.verifySignatures and\n> gpg.verifySignatures (in decreasing order of significance).\n\nIt is clear for a merge or a pull what it means to verify\nsignature---you trust your local history, and you are willing to\nmerge in a commit only when it has a trusted signature (which\nautomatically means that you trust the hash function and also the\nsigner did some reasonable vetting of the history behind the tip\ncommit, or you never check out your intermediate state, depending\non your threat model).\n\nI am not sure what it should mean to verify signature on clone.  I'd\nassume that our threat model and verification policy are consistent\nwith what we use for a merge/pull, in that we trust all history\nbehind a commit that has a trusted signature, so it is clear that\nyou would want the tip commit of the default branch (or if you are\nnaming a single branch to clone, then the tip of that branch) to\ncarry a trusted signature.  But what about the commits that are\nreachable from other branches and tags that are *not* contained\nin the branch that is initially checked out?\n\nLater in the proposed log message of 5/5 you allude to risk of\nmerely checking out a potentially untrustworthy contents to the\nworking tree.  This is far stricter than the usual threat model we\ntend to think about as the developers of source code management\nsystem, where checking out is perfectly OK but running \"make\" or its\nequivalent is the first contact between the victim's system with\nmalicious contents.\n\nVerifying the tip of the default/sole branch upon cloning before the\ntree of the commit is checked out certainly would cover that single\ncase (i.e. the initial checkout after cloning), but I am not sure if\nit is the best way, and I am reasonably certain it is insufficient\nagainst your threat model.  After such a clone is made, when the\nuser checks out another branch obtained from the \"origin\" remote,\nthere is no mechanism that protects the user from the same risk you\njust covered with the new signature verification mechanism upon\ncloning, without validating the tip of that other branch, somewhere\nbetween the time the clone is made and that other branch gets\nchecked out.\n\nIt almost makes me suspect that what you are trying to do with the\n\"verification upon cloning\" may be much better done as a \"verify the\ntree for trustworthyness before checking it out to the working tree\"\nmechanism, where the trustworthyness of a tree-ish object may be\ndefined (and possibly made customizable by the policy of the project\nthe user is working on) like so:\n\n - A tree is trustworthy if it is the tree of a trustworthy commit.\n\n - A commit is trustworthy if\n\n   . it carries a trusted signature, or\n   . it is pointed by a tag that carries a trusted signature, or\n   . it is reachable from a trustworthy commit.\n\nNow, it is prohibitively expensive to compute the trusttworthiness\nof a tree, defined like the above, when checking it out, UNLESS you\nforce each and every commit to carry a trusted signature, which is\nnot necessarily practical in the real world.\n\nAnother approach to ensure that any and all checkout would work only\non a trustworthy tree might be to make sure that tips of *all* the\nrefs are trustworthy commits immediately after cloning (instead of\nverifying only the branch you are going to checkout, which is what\nyour patch does IIUC).  That way, any subsequent checkout in the\nrepository would either be checking out a commit that was\n\n - originally cloned from the remote, and is reachable from some ref\n   that was validated back when the repository was cloned, or\n\n - created locally in this repository, which we trust, or\n\n - fetched from somewhere else, and is reachable from the commit\n   that was validated back when \"git pull\" validated what was about\n   to be integrated into the history of *this* repository.\n\nHmm?\n"},{"id":"389276","messageId":"xmqq36cs9zet.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"20200105135616.19102-2-hji@dyntopia.com","subject":"Re: [PATCH 1/5] gpg-interface: conditionally show the result in print_signature_buffer()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T19:07:38Z","receivedAt":"2020-01-06T19:07:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n> The print_signature_buffer() function in gpg-interface.c is used to\n> print the result of a GPG verified payload.  It takes a 'flags'\n> parameter that determines what to print.\n>\n> Previously, the 'flags' parameter processed 2 flags:\n>\n> - GPG_VERIFY_RAW: to print the raw output from GPG instead of the\n>   human(ish)-readable output.  One of these outputs were always\n>   shown, irregardless of any other flags.\n> - GPG_VERIFY_VERBOSE: to print the payload that was verified\n>\n> Interestingly, there was also a third flag defined in gpg-interface.h;\n> GPG_VERIFY_OMIT_STATUS.  That flag wasn't used by the print function\n> itself -- instead, callers would check for the presence of\n> GPG_VERIFY_OMIT_STATUS before invoking print_signature_buffer().\n>\n> It seems reasonable that the GPG interface should handle all flags\n> related to how the result should be (or shouldn't be) shown.  This patch\n> implements that behavior by removing GPG_VERIFY_OMIT_STATUS and adding\n> GPG_VERIFY_FULL.  If neither GPG_VERIFY_FULL nor GPG_VERIFY_VERBOSE is\n> present, then nothing is printed.  This allows callers to invoke\n> print_signature_buffer() unconditionally.\n\nSo in short, VERIFY_FULL is equivalent to !OMIT_STATUS?\n\nAs the direct callers of \"print\" are not the ones that set up bits\nin flags, I think the proposed change makes the API easier to use.\n\nWill queue.  Thanks.\n\n> Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com>\n> ---\n>  builtin/tag.c           | 4 ++--\n>  builtin/verify-commit.c | 2 +-\n>  builtin/verify-tag.c    | 4 ++--\n>  gpg-interface.c         | 2 +-\n>  gpg-interface.h         | 6 +++---\n>  tag.c                   | 4 +---\n>  6 files changed, 10 insertions(+), 12 deletions(-)\n>\n> diff --git a/builtin/tag.c b/builtin/tag.c\n> index e0a4c25382..8489e220e8 100644\n> --- a/builtin/tag.c\n> +++ b/builtin/tag.c\n> @@ -112,10 +112,10 @@ static int verify_tag(const char *name, const char *ref,\n>  {\n>  \tint flags;\n>  \tconst struct ref_format *format = cb_data;\n> -\tflags = GPG_VERIFY_VERBOSE;\n> +\tflags = GPG_VERIFY_FULL | GPG_VERIFY_VERBOSE;\n>  \n>  \tif (format->format)\n> -\t\tflags = GPG_VERIFY_OMIT_STATUS;\n> +\t\tflags = 0;\n>  \n>  \tif (gpg_verify_tag(oid, name, flags))\n>  \t\treturn -1;\n> diff --git a/builtin/verify-commit.c b/builtin/verify-commit.c\n> index 40c69a0bed..2a099ec6ba 100644\n> --- a/builtin/verify-commit.c\n> +++ b/builtin/verify-commit.c\n> @@ -63,7 +63,7 @@ static int git_verify_commit_config(const char *var, const char *value, void *cb\n>  int cmd_verify_commit(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint i = 1, verbose = 0, had_error = 0;\n> -\tunsigned flags = 0;\n> +\tunsigned flags = GPG_VERIFY_FULL;\n>  \tconst struct option verify_commit_options[] = {\n>  \t\tOPT__VERBOSE(&verbose, N_(\"print commit contents\")),\n>  \t\tOPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\n> diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\n> index f45136a06b..bd5e99925b 100644\n> --- a/builtin/verify-tag.c\n> +++ b/builtin/verify-tag.c\n> @@ -30,7 +30,7 @@ static int git_verify_tag_config(const char *var, const char *value, void *cb)\n>  int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint i = 1, verbose = 0, had_error = 0;\n> -\tunsigned flags = 0;\n> +\tunsigned flags = GPG_VERIFY_FULL;\n>  \tstruct ref_format format = REF_FORMAT_INIT;\n>  \tconst struct option verify_tag_options[] = {\n>  \t\tOPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n> @@ -53,7 +53,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n>  \t\tif (verify_ref_format(&format))\n>  \t\t\tusage_with_options(verify_tag_usage,\n>  \t\t\t\t\t   verify_tag_options);\n> -\t\tflags |= GPG_VERIFY_OMIT_STATUS;\n> +\t\tflags = 0;\n>  \t}\n>  \n>  \twhile (i < argc) {\n> diff --git a/gpg-interface.c b/gpg-interface.c\n> index 2d538bcd6e..fc182d39be 100644\n> --- a/gpg-interface.c\n> +++ b/gpg-interface.c\n> @@ -341,7 +341,7 @@ void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n>  \tif (flags & GPG_VERIFY_VERBOSE && sigc->payload)\n>  \t\tfputs(sigc->payload, stdout);\n>  \n> -\tif (output)\n> +\tif (flags & GPG_VERIFY_FULL && output)\n>  \t\tfputs(output, stderr);\n>  }\n>  \n> diff --git a/gpg-interface.h b/gpg-interface.h\n> index f4e9b4f371..4631a91330 100644\n> --- a/gpg-interface.h\n> +++ b/gpg-interface.h\n> @@ -3,9 +3,9 @@\n>  \n>  struct strbuf;\n>  \n> -#define GPG_VERIFY_VERBOSE\t\t1\n> -#define GPG_VERIFY_RAW\t\t\t2\n> -#define GPG_VERIFY_OMIT_STATUS\t4\n> +#define GPG_VERIFY_VERBOSE (1 << 0)\n> +#define GPG_VERIFY_RAW (1 << 1)\n> +#define GPG_VERIFY_FULL (1 << 2)\n>  \n>  enum signature_trust_level {\n>  \tTRUST_UNDEFINED,\n> diff --git a/tag.c b/tag.c\n> index 71b544467e..b8d6da81eb 100644\n> --- a/tag.c\n> +++ b/tag.c\n> @@ -28,9 +28,7 @@ static int run_gpg_verify(const char *buf, unsigned long size, unsigned flags)\n>  \n>  \tret = check_signature(buf, payload_size, buf + payload_size,\n>  \t\t\t\tsize - payload_size, &sigc);\n> -\n> -\tif (!(flags & GPG_VERIFY_OMIT_STATUS))\n> -\t\tprint_signature_buffer(&sigc, flags);\n> +\tprint_signature_buffer(&sigc, flags);\n>  \n>  \tsignature_check_clear(&sigc);\n>  \treturn ret;\n"},{"id":"389279","messageId":"xmqqy2uk8jmr.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"20200105135616.19102-3-hji@dyntopia.com","subject":"Re: [PATCH 2/5] gpg-interface: support one-line summaries in print_signature_buffer()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T19:33:48Z","receivedAt":"2020-01-06T19:33:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n> Most consumers of the GPG interface use print_signature_buffer() to show\n> This patch moves the one-line summary from verify_merge_signature() to\n> print_signature_buffer().  It also introduces a GPG_VERIFY_SHORT flag\n> that determines whether the summary should be printed.\n\nI think the motivation makes sense.\n\n> -void print_signature_buffer(const struct signature_check *sigc, unsigned flags)\n> +void print_signature_buffer(const struct object_id *oid,\n> +\t\t\t    const struct signature_check *sigc, int status,\n> +\t\t\t    unsigned flags)\n>  {\n>  \tconst char *output = flags & GPG_VERIFY_RAW ?\n>  \t\tsigc->gpg_status : sigc->gpg_output;\n> +\tchar hex[GIT_MAX_HEXSZ + 1];\n> +\tchar *type;\n>  \n>  \tif (flags & GPG_VERIFY_VERBOSE && sigc->payload)\n>  \t\tfputs(sigc->payload, stdout);\n>  \n>  \tif (flags & GPG_VERIFY_FULL && output)\n>  \t\tfputs(output, stderr);\n> +\n> +\tif (flags & GPG_VERIFY_SHORT && oid) {\n\nI am not sure about the wisdom of \"&& oid\" here.  Wouldn't it be a\nbug for a caller who asks for _SHORT format to feed a NULL oid to\nus?  I would understand\n\n\tif (flags & GPG_VERIFY_SHORT) {\n\t\tif (!oid)\n\t\t\tBUG(\"Caller asking for SHORT without oid???\");\n\nmuch better, but if there is a caller that has a legitimate need\ntobe able to pass NULL and still request SHORT, let's see it and\ndiscuss if it is a good idea.\n\nFor that matter, the two print routines we have immediately above\nshare the same issue.\n\n> +\t\ttype = xstrdup_or_null(\n> +\t\t\ttype_name(oid_object_info(the_repository, oid, NULL)));\n> +\t\tif (!type)\n> +\t\t\ttype = xstrdup(\"object\");\n\nThis is minor, but I wonder if this is easier to follow.\n\n\t\ttype = type_name(oid_object_info(the_repository, oid, NULL));\n\t\tif (!type)\n\t\t\ttype = \"object\";\n\t\ttype = xstrdup(type);\n\n> +\t\t*type = toupper(*type);\n\nThis is not helpful at all for i18n/l10n, I am afraid.\n\n> +\t\tfind_unique_abbrev_r(hex, oid, DEFAULT_ABBREV);\n> +\n> +\t\tswitch (sigc->result) {\n> +\t\tcase 'G':\n> +\t\t\tif (status)\n> +\t\t\t\terror(_(\"%s %s has an untrusted GPG signature, \"\n> +\t\t\t\t\t\"allegedly by %s.\"),\n> +\t\t\t\t      type, hex, sigc->signer);\n\nThe original was of course\n\n        die(_(\"Commit %s has an untrusted GPG signature, \"\n              \"allegedly by %s.\"), hex, signature_check.signer);\n\nso the message can properly localized including the typename.  That\nis no longer true with this conversion.\n\nYou probably would need to prepare a 3-element array (one element\nfor each of <object, commit, tag>), each element of the array being\na 3-element array that holds the message for these three cases\n(i.e. G, N and default) instead.\n\n\n"},{"id":"389280","messageId":"xmqqtv588ji7.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"20200105135616.19102-4-hji@dyntopia.com","subject":"Re: [PATCH 3/5] commit: refactor signature verification helpers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T19:36:32Z","receivedAt":"2020-01-06T19:36:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n> +static unsigned gpg_flags = GPG_VERIFY_SHORT | GPG_VERIFY_COMPAT;\n>  static struct strbuf merge_msg = STRBUF_INIT;\n>  static struct strategy **use_strategies;\n>  static size_t use_strategies_nr, use_strategies_alloc;\n> @@ -633,7 +633,7 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n>  \t\tsign_commit = git_config_bool(k, v) ? \"\" : NULL;\n>  \t\treturn 0;\n>  \t} else if (!strcmp(k, \"gpg.mintrustlevel\")) {\n> -\t\tcheck_trust_level = 0;\n> +\t\tgpg_flags ^= GPG_VERIFY_COMPAT;\n\nDid you really mean to \"toggle the bit each time the variable\nappears\"?  Or is this better written as\n\n\t\tgpg_flags &= ~GPG_VERIFY_COMPAT;\n\ninstead?  There may be another instance of the same in this patch.\n\n"},{"id":"389286","messageId":"xmqqa7708flc.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"20200105135616.19102-5-hji@dyntopia.com","subject":"Re: [PATCH 4/5] merge: verify signatures if gpg.verifySignatures is true","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-06T21:01:03Z","receivedAt":"2020-01-06T21:01:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n> @@ -610,6 +610,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)\n>  \t\tshow_diffstat = git_config_bool(k, v);\n>  \telse if (!strcmp(k, \"merge.verifysignatures\"))\n>  \t\tverify_signatures = git_config_bool(k, v);\n> +\telse if (!strcmp(k, \"gpg.verifysignatures\") && verify_signatures < 0)\n> +\t\tverify_signatures = git_config_bool(k, v);\n\nHmm, the next person who attempts to generalize the mechanism\nfurther would have a hard time introducing a fallback configuration\nthat is even common than \"gpg\" when s/he has to start with this\ncode, no?  That is, this patch introduced \"gpg.verifysignatures is\nused when merge.verifysignatures is not defined\" and with the two\nlevel override the code works OK, but to implement \"if neither gpg.*\nor merge.* is defined, common.verifysignatures is used instead\", the\nabove part needs to be dismantled and redone.\n\nKeeping the \"initialize verify_signatures to -1 (unspecified)\" from\nthis patch, setting a separate gpg_verify_signatures variable upon\nseeing gpg.verifysignatures, and consolidating the final finding\nafter git_config(git_merge_config, NULL) returns into verify_signatures\nlike so:\n\n\tinit_diff_ui_defaults();\n\tgit_config(git_merge_config, NULL);\n\n+\tif (verify_signatures < 0)\n+\t\tverify_signatures = (0 <= gpg_verify_signatures) \n+\t\t\t\t  ? gpg_verify_signatures \n+\t\t\t\t  : 0;\n\nwould be more in line with the way we arrange multiple configuration\nvariables to serve as fallback defaults.  And that is more easily\nextensible.\n\nAlso with such an arrangement, \"if (verify_signatures == 1)\" we see\nbelow will become unnecessary, which is another plus.\n\nThanks.\n\n>  \telse if (!strcmp(k, \"pull.twohead\"))\n>  \t\treturn git_config_string(&pull_twohead, k, v);\n>  \telse if (!strcmp(k, \"pull.octopus\"))\n> @@ -1399,7 +1401,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \t\tif (remoteheads->next)\n>  \t\t\tdie(_(\"Can merge only exactly one commit into empty head\"));\n>  \n> -\t\tif (verify_signatures &&\n> +\t\tif (verify_signatures == 1 &&\n>  \t\t    gpg_verify_commit(&remoteheads->item->object.oid, NULL,\n>  \t\t\t\t      NULL, gpg_flags))\n>  \t\t\tdie(_(\"Signature verification failed\"));\n> @@ -1423,7 +1425,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n>  \t\tusage_with_options(builtin_merge_usage,\n>  \t\t\tbuiltin_merge_options);\n>  \n> -\tif (verify_signatures) {\n> +\tif (verify_signatures == 1) {\n>  \t\tfor (p = remoteheads; p; p = p->next) {\n>  \t\t\tif (gpg_verify_commit(&p->item->object.oid, NULL, NULL,\n>  \t\t\t\t\t      gpg_flags))\n\n"},{"id":"389318","messageId":"87lfqjnc5o.hji@dyntopia.com","threadId":"52570","inReplyTo":"xmqq36ctbis8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/5] refactor gpg-interface and add gpg verification for clones","fromName":"Hans Jerry Illikainen","fromEmail":"hji@dyntopia.com","sentAt":"2020-01-07T04:06:11Z","receivedAt":"2020-01-07T04:09:24Z","isPatch":true,"sender":{"key":"hji@dyntopia.com","avatar":"https://avatars.githubusercontent.com/u/8175517?v=4"},"body":"On Sun, Jan 05 2020, Junio C Hamano wrote:\n> Hans Jerry Illikainen <hji@dyntopia.com> writes:\n>\n>> And finally, signature verification is added to the clone builtin.  It\n>> obeys --(no-)verify-signatures, clone.verifySignatures and\n>> gpg.verifySignatures (in decreasing order of significance).\n>\n> I am not sure what it should mean to verify signature on clone.  I'd\n> assume that our threat model and verification policy are consistent\n> with what we use for a merge/pull, in that we trust all history\n> behind a commit that has a trusted signature, so it is clear that\n> you would want the tip commit of the default branch (or if you are\n> naming a single branch to clone, then the tip of that branch) to\n> carry a trusted signature.\n\nYes, that's how it's implemented in v0 -- the tip of the branch/tag that\nis to be checked out is verified.\n\n\n> But what about the commits that are reachable from other branches and\n> tags that are *not* contained in the branch that is initially checked\n> out?\n\nI thought about that and figured that adding signature verification for\nthe checkout builtin would be the next step after this series.  That\nthought should probably have been mentioned in the cover letter for\ncriticism -- because now I'm not sure if that's a reasonable approach\nanymore.\n\n\n> Later in the proposed log message of 5/5 you allude to risk of\n> merely checking out a potentially untrustworthy contents to the\n> working tree.  This is far stricter than the usual threat model we\n> tend to think about as the developers of source code management\n> system, where checking out is perfectly OK but running \"make\" or its\n> equivalent is the first contact between the victim's system with\n> malicious contents.\n\nModern desktop environments perform enough magic that I think it makes\nsense to assume that simply having malicious content on disk is enough\nfor a compromise -- even though no explicit user interaction takes place\nwith that content.  The NSF bug in GStreamer that made headlines a few\nyears back is a good example of that [1].  And the numerous AV bugs\npublished by taviso [2].\n\n\n> Verifying the tip of the default/sole branch upon cloning before the\n> tree of the commit is checked out certainly would cover that single\n> case (i.e. the initial checkout after cloning), but I am not sure if\n> it is the best way, and I am reasonably certain it is insufficient\n> against your threat model.  After such a clone is made, when the\n> user checks out another branch obtained from the \"origin\" remote,\n> there is no mechanism that protects the user from the same risk you\n> just covered with the new signature verification mechanism upon\n> cloning, without validating the tip of that other branch, somewhere\n> between the time the clone is made and that other branch gets\n> checked out.\n\nI agree.  Again, I should have mentioned my thoughts on adding signature\nverification to the checkout builtin in the cover letter.\n\n\n> It almost makes me suspect that what you are trying to do with the\n> \"verification upon cloning\" may be much better done as a \"verify the\n> tree for trustworthyness before checking it out to the working tree\"\n> mechanism, where the trustworthyness of a tree-ish object may be\n> defined (and possibly made customizable by the policy of the project\n> the user is working on) like so:\n>\n>  - A tree is trustworthy if it is the tree of a trustworthy commit.\n>\n>  - A commit is trustworthy if\n>\n>    . it carries a trusted signature, or\n>    . it is pointed by a tag that carries a trusted signature, or\n>    . it is reachable from a trustworthy commit.\n>\n> Now, it is prohibitively expensive to compute the trusttworthiness\n> of a tree, defined like the above, when checking it out, UNLESS you\n> force each and every commit to carry a trusted signature, which is\n> not necessarily practical in the real world.\n\nEven though you mention that it's computationally expensive, I kind of\nlike this approach.  It seems generalized enough that it doesn't need to\nbe tied to a single operation like 'clone'.\n\n\n> Another approach to ensure that any and all checkout would work only\n> on a trustworthy tree might be to make sure that tips of *all* the\n> refs are trustworthy commits immediately after cloning (instead of\n> verifying only the branch you are going to checkout, which is what\n> your patch does IIUC).  That way, any subsequent checkout in the\n> repository would either be checking out a commit that was\n>\n>  - originally cloned from the remote, and is reachable from some ref\n>    that was validated back when the repository was cloned, or\n>\n>  - created locally in this repository, which we trust, or\n>\n>  - fetched from somewhere else, and is reachable from the commit\n>    that was validated back when \"git pull\" validated what was about\n>    to be integrated into the history of *this* repository.\n\nWouldn't that still forgo signature verification when doing something\nlike:\n\n    $ git fetch\n    $ git checkout -b foo origin/branch-with-previously-unseen-commits\n\nunless either fetch or checkout is equipped with signature verification?\n\nAlso, in case of a revoked key, this approach would require that tags\nsigned with that key be deleted on origin.  Maybe that's to be\nconsidered best practice anyway, but distro maintainers might not\nappreciate disappearing release tags.\n\nAlso, an interesting observation (but probably a \"not our problem\" as\nfar as Git is concerned) -- I have noticed that certain git forges might\ncreate branches unexpectedly.  A few of my repos has dependabot/...\nbranches created by a GitHub bot that is enabled by default:\n\n\"\"\"\nAutomated security updates are opened by Dependabot on behalf of\nGitHub. The Dependabot GitHub App is automatically installed on every\nrepository where automated security updates are enabled.\n\"\"\"\n\nI can foresee a scenario where validating the tip of every ref on a\nfresh clone would result in a DoS for lot of automated CI/CD-type\nworkflows when bots on GitHub (and other hosts) creates unexpected\nbranches in the users repos.\n\n    $ git branch -r\n      origin/HEAD -> origin/master\n      origin/dependabot/pip/requirements/typed-ast-1.3.2\n      origin/master\n\n\n> Hmm?\n\nI appreciate you taking the time to write your thoughts!  Unfortunately\nthere doesn't seem to be an obvious approach that is significantly\npreferable to the alternatives.  I will experiment with the ideas you\nmentioned above and see if I can come up with something that makes\nsense.\n\nWould you prefer that I break off the series before 5/5 in a new series\nto fix the comments you made on the other patches that seem\nalmost-mergeable?\n\nThanks!\n\n\n[1] https://scarybeastsecurity.blogspot.com/2016/11/0day-exploit-compromising-linux-desktop.html\n[2] https://bugs.chromium.org/p/project-zero/issues/detail?id=769\n\n-- \nhji\n"},{"id":"389395","messageId":"xmqq4kx76wc6.fsf@gitster-ct.c.googlers.com","threadId":"52570","inReplyTo":"87lfqjnc5o.hji@dyntopia.com","subject":"Re: [PATCH 0/5] refactor gpg-interface and add gpg verification for clones","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-07T16:54:33Z","receivedAt":"2020-01-07T16:54:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hans Jerry Illikainen <hji@dyntopia.com> writes:\n\n>> It almost makes me suspect that what you are trying to do with the\n>> \"verification upon cloning\" may be much better done as a \"verify the\n>> tree for trustworthyness before checking it out to the working tree\"\n>> mechanism, where the trustworthyness of a tree-ish object may be\n>> defined (and possibly made customizable by the policy of the project\n>> the user is working on) like so:\n>>\n>>  - A tree is trustworthy if it is the tree of a trustworthy commit.\n>>\n>>  - A commit is trustworthy if\n>>\n>>    . it carries a trusted signature, or\n>>    . it is pointed by a tag that carries a trusted signature, or\n>>    . it is reachable from a trustworthy commit.\n>>\n>> Now, it is prohibitively expensive to compute the trusttworthiness\n>> of a tree, defined like the above, when checking it out, UNLESS you\n>> force each and every commit to carry a trusted signature, which is\n>> not necessarily practical in the real world.\n>\n> Even though you mention that it's computationally expensive, I kind of\n> like this approach.  It seems generalized enough that it doesn't need to\n> be tied to a single operation like 'clone'.\n\nWell, I don't ;-)  But even if you are not signing each and every\ncommit, it may be possible that the reachability information kept in\nthe commit-graph may help reduce the cost of the computation.  We'd\nprobably need a way to memoise which tags and commits have already\nbeen verified earlier for trustworthiness for the scheme to work.\n\n> Wouldn't that still forgo signature verification when doing something\n> like:\n>\n>     $ git fetch\n>     $ git checkout -b foo origin/branch-with-previously-unseen-commits\n>\n> unless either fetch or checkout is equipped with signature verification?\n\nYes, but I was assuming that \"fetch\", as an underlying mechanism for\n\"pull\" would be what you'd teach the verification.\n\n> Also, in case of a revoked key, this approach would require that tags\n> signed with that key be deleted on origin.  \n\nI am not sure if I follow.  An object that was signed back when a\nkey was OK and then the key gets revoked later---what should happen\nto the trustworthiness of that object?  Another object that was\nobtained from elsewhere was verified to be trustworthy, but later we\nfound out that the key we thought was trusted was already revoked---\nwhat should happen to the trustworthiness of that object?\n\nIn either case, I think it is an unrelated matter if the tag object\nshould be removed either here or at the origin.  When spread *now*,\nthe key that was used to sign that tag object is known to have been\nrevoked, so while it is of no use to convey trustworthiness, if the\nupstream project chooses to keep the tag (perhaps while re-issuing\na new version of the tag that points at the same object but signed\nwith a different key), that is perfectly fine, I would say.\n\n> Maybe that's to be\n> considered best practice anyway, but distro maintainers might not\n> appreciate disappearing release tags.\n\nI think that is best left as a policy decision for each project, and\nwe are not in the business of removing objects---we just should give\nthem tools to determine what key was used to sign an object, GPG or\nwhatever signing and key management infrastructure the projects\nchooses to use gives them tools to decide if the key is what the\nproject trusts, and we give them tools to remove tags the project\nfinds unwanted.  Combining the tools according to the policy decision\nthe project makes is outside the scope of what we do here.\n"}]}