{"thread":{"id":"42616","subject":"[PATCH] verify-tag: allow to verify signed blob objects","startedAt":"2016-06-16T06:55:53Z","lastAt":"2016-06-16T17:27:42Z","messageCount":6,"participants":["Michael J Gruber","Junio C Hamano","Jacob Keller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"289287","messageId":"a6557333316c6f7996fa54eebc75abdf988ed9f9.1465991212.git.git@drmicha.warpmail.net","threadId":"42616","inReplyTo":null,"subject":"[PATCH] verify-tag: allow to verify signed blob objects","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-15T11:51:54Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Currently, there is no easy way to verify push certificates. They have\nthe same structure as signed tags: \"attached detached signatures\", that\nis: the concatenation of the signed material and its detached signature.\n\nIntroduce a `--blob` option to verify-tag so that it allows to verify\ntags and blobs.\n\nSigned-off-by: Michael J Gruber \n---\nThe first outcome of my long announced project to describe our signature\nformats in Documentation/technical.... (progress underway)\n\nIn fact, that whole area is in need of refactoring: gpg related bits are\nall over the place, including tag.c. The proposed patch neither improves\nnor worsens the situation in that respect. But, since we make it\nunnecessarily hard to verify signatures (git cat-file | gpg --verify fails)\nit's only fair to provide a tool for pre-receive hook writers.\n\n Documentation/git-verify-tag.txt | 4 ++++\n builtin/verify-tag.c | 1 +\n gpg-interface.h | 1 +\n t/t5534-push-signed.sh | 3 ++-\n tag.c | 2 +-\n 5 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-verify-tag.txt b/Documentation/git-verify-tag.txt\nindex d590edc..2e5cf4d 100644\n--- a/Documentation/git-verify-tag.txt\n+++ b/Documentation/git-verify-tag.txt\n@@ -20,6 +20,10 @@ OPTIONS\n \tPrint the raw gpg status output to standard error instead of the normal\n \thuman-readable output.\n \n+--blob::\n+\tAllow to verify signed blob objects (in addition to tag objects), such as the\n+\tobjects containing a push certificate.\n+\n -v::\n --verbose::\n \tPrint the contents of the tag object before validating it.\ndiff --git a/builtin/verify-tag.c b/builtin/verify-tag.c\nindex 99f8148..19d26b0 100644\n--- a/builtin/verify-tag.c\n+++ b/builtin/verify-tag.c\n@@ -33,6 +33,7 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)\n \tconst struct option verify_tag_options[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"print tag contents\")),\n \t\tOPT_BIT(0, \"raw\", &flags, N_(\"print raw gpg status output\"), GPG_VERIFY_RAW),\n+\t\tOPT_BIT(0, \"blob\", &flags, N_(\"allow to verify blob objects\"), GPG_VERIFY_BLOB),\n \t\tOPT_END()\n \t};\n \ndiff --git a/gpg-interface.h b/gpg-interface.h\nindex ea68885..a3cbfc3 100644\n--- a/gpg-interface.h\n+++ b/gpg-interface.h\n@@ -3,6 +3,7 @@\n \n #define GPG_VERIFY_VERBOSE\t1\n #define GPG_VERIFY_RAW\t\t2\n+#define GPG_VERIFY_BLOB\t\t4\n \n struct signature_check {\n \tchar *payload;\ndiff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh\nindex ecb8d44..de4d38b 100755\n--- a/t/t5534-push-signed.sh\n+++ b/t/t5534-push-signed.sh\n@@ -94,7 +94,8 @@ test_expect_success GPG 'signed push sends push certificate' '\n \t# record the push certificate\n \tif test -n \"${GIT_PUSH_CERT-}\"\n \tthen\n-\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert\n+\t\tgit cat-file blob $GIT_PUSH_CERT >../push-cert &&\n+\t\tgit verify-tag --blob $GIT_PUSH_CERT\n \tfi &&\n \n \tcat >../push-cert-status < writes:\n\n> diff --git a/tag.c b/tag.c\n> index d1dcd18..d5f090b 100644\n> --- a/tag.c\n> +++ b/tag.c\n> @@ -39,7 +39,7 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n> \tint ret;\n> \n> \ttype = sha1_object_info(sha1, NULL);\n> -\tif (type != OBJ_TAG)\n> +\tif ((type != OBJ_TAG) && ((type != OBJ_BLOB) || !(flags & GPG_VERIFY_BLOB)))\n> \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n> \t\t\t\tname_to_report ?\n> \t\t\t\tname_to_report :\n\nThe double negation is very hard to read. I wonder\n\n\tif ((type != OBJ_TAG) &&\n !((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB)))\n\nis easier to follow? \"It is not a tag object, and it's not like we\nwere asked to verify blob and the user gave us a blob, either, so\nlet's complain\" is easier to follow, at least for me.\n\nOr even\n\n\tif ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB))\n \t\"you told me to check blob but didn't give me one\";\n\t} else if (type != OBJ_TAG)\n\t\t\"you didn't give me a tag\";\n"},{"id":"289313","messageId":"9b700254-23b1-286c-70e6-68d484e9c4e8@drmicha.warpmail.net","threadId":"42616","inReplyTo":"xmqq8ty670v2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] verify-tag: allow to verify signed blob objects","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2016-06-15T19:07:15Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 15.06.2016 20:39:\n> Michael J Gruber writes:\n> \n>> diff --git a/tag.c b/tag.c\n>> index d1dcd18..d5f090b 100644\n>> --- a/tag.c\n>> +++ b/tag.c\n>> @@ -39,7 +39,7 @@ int gpg_verify_tag(const unsigned char *sha1, const char *name_to_report,\n>> \tint ret;\n>> \n>> \ttype = sha1_object_info(sha1, NULL);\n>> -\tif (type != OBJ_TAG)\n>> +\tif ((type != OBJ_TAG) && ((type != OBJ_BLOB) || !(flags & GPG_VERIFY_BLOB)))\n>> \t\treturn error(\"%s: cannot verify a non-tag object of type %s.\",\n>> \t\t\t\tname_to_report ?\n>> \t\t\t\tname_to_report :\n> \n> The double negation is very hard to read. I wonder\n> \n> \tif ((type != OBJ_TAG) &&\n> !((type == OBJ_BLOB) && (flags & GPG_VERIFY_BLOB)))\n> \n> is easier to follow? \"It is not a tag object, and it's not like we\n> were asked to verify blob and the user gave us a blob, either, so\n> let's complain\" is easier to follow, at least for me.\n\nAs a further exercise in boolean algebra, you can pull out the negation\ncompletely:\n\nif (!( (type == OBJ_TAG) || ((type == OBJ_BLOB) && (flags &\nGPG_VERIFY_BLOB)) ))\n\n> Or even\n> \n> \tif ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB))\n> \t\"you told me to check blob but didn't give me one\";\n> \t} else if (type != OBJ_TAG)\n> \t\t\"you didn't give me a tag\";\n> \n\nI just tried to stay as close to the original as possible, but I don't\ncare either way. Your latter version is more strict and would require a\nslight documentation change, but would be fine, too.\n\nMichael\n"},{"id":"289314","messageId":"xmqqziqm5k7j.fsf@gitster.mtv.corp.google.com","threadId":"42616","inReplyTo":"9b700254-23b1-286c-70e6-68d484e9c4e8@drmicha.warpmail.net","subject":"Re: [PATCH] verify-tag: allow to verify signed blob objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-15T19:24:32Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n>> Or even\n>> \n>> \tif ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB))\n>> \t\"you told me to check blob but didn't give me one\";\n>> \t} else if (type != OBJ_TAG)\n>> \t\t\"you didn't give me a tag\";\n>> \n>\n> I just tried to stay as close to the original as possible, but I don't\n> care either way. Your latter version is more strict and would require a\n> slight documentation change, but would be fine, too.\n\nActually, the message you reused is not reusable for this new mode.\nI guess starting from more strict (which makes sense, as you do not\nwant to silently say \"Yeah, the blob verifies OK\" when the user\ntells you \"I want you to verify this blob, and here it is\" and hands\nyou a tag. If that were an acceptable behaviour, you do not even\nneed VERIFY_BLOB as an option, do you?\n\nSo I do not care too strongly about this feature, if it were to be\nadded, I think you would need to separate error messages and type\nverification should not be lax, I would think.\n"},{"id":"289324","messageId":"CA+P7+xqV8t1cJeAtNQeWaFbKC5DrhpPoUJ9GWAuPrO=GZpKaiQ@mail.gmail.com","threadId":"42616","inReplyTo":"xmqqziqm5k7j.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] verify-tag: allow to verify signed blob objects","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-06-15T21:41:48Z","receivedAt":"2016-06-16T06:55:53Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, Jun 15, 2016 at 12:24 PM, Junio C Hamano wrote:\n> Michael J Gruber writes:\n>\n>>> Or even\n>>>\n>>> if ((flags & GPG_VERIFY_BLOB) && (type != OBJ_BLOB))\n>>> \"you told me to check blob but didn't give me one\";\n>>> } else if (type != OBJ_TAG)\n>>> \"you didn't give me a tag\";\n>>>\n>>\n>> I just tried to stay as close to the original as possible, but I don't\n>> care either way. Your latter version is more strict and would require a\n>> slight documentation change, but would be fine, too.\n>\n> Actually, the message you reused is not reusable for this new mode.\n> I guess starting from more strict (which makes sense, as you do not\n> want to silently say \"Yeah, the blob verifies OK\" when the user\n> tells you \"I want you to verify this blob, and here it is\" and hands\n> you a tag. If that were an acceptable behaviour, you do not even\n> need VERIFY_BLOB as an option, do you?\n>\n> So I do not care too strongly about this feature, if it were to be\n> added, I think you would need to separate error messages and type\n> verification should not be lax, I would think.\n>\n\nI agree that Junio's suggestion is (a) both easier to read and (b)\nmore clear to the end user, and thus preferable.\n\nThanks,\nJake\n"},{"id":"289373","messageId":"xmqqk2hp59iy.fsf@gitster.mtv.corp.google.com","threadId":"42616","inReplyTo":"a6557333316c6f7996fa54eebc75abdf988ed9f9.1465991212.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] verify-tag: allow to verify signed blob objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-16T17:27:33Z","receivedAt":"2016-06-16T17:27:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Currently, there is no easy way to verify push certificates. They have\n> the same structure as signed tags: \"attached detached signatures\", that\n> is: the concatenation of the signed material and its detached signature.\n>\n> Introduce a `--blob` option to verify-tag so that it allows to verify\n> tags and blobs.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> The first outcome of my long announced project to describe our signature\n> formats in Documentation/technical.... (progress underway)\n>\n> In fact, that whole area is in need of refactoring: gpg related bits are\n> all over the place, including tag.c. The proposed patch neither improves\n> nor worsens the situation in that respect. But, since we make it\n> unnecessarily hard to verify signatures (git cat-file | gpg --verify fails)\n> it's only fair to provide a tool for pre-receive hook writers.\n\n\nAnother (orthogonal) thing to think about is \"is it sensible to add\na new feature to verify blob objects that has signature?\"\n\nThat is, if we are adding one new feature, isn't it more sensible\nfor it to accept a stream of bytes from the standard input and run\nverify-signed-bufter on it? That way, if you already happen to have\na blob, you can feed it \"cat-file blob\" output to get the new\nfeature you added in this patch.\n\nBut you cannot go in the other direction without incurring downside.\n\nIf you start from \"verify signature in blob\" interface, and if all\nyou have is a log of push certificates in a flat text file [1], you\nwould need to do \"hash-objects -w\" first only to be able to use that\ninterface (which in turn means that you would need write access to\nthe object store of the repository.\n\n\n[Footnote]\n\n*1* push certificate is first written as a blob in the object store\n only so that we can safely run multiple receive-pack instances\n without them stepping on each others' toes; it is expected that\n they are collected by the server operator in chronological order\n and published in a human readable log form, so that outside\n people can verify honesty of the server operator.\n"}]}