{"thread":{"id":"41303","subject":"[RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key","startedAt":"2016-02-01T22:22:31Z","lastAt":"2016-02-02T03:17:16Z","messageCount":4,"participants":["Robin H. Johnson","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"277190","messageId":"robbat2-20160201T220940-187080200Z@orbis-terrarum.net","threadId":"41303","inReplyTo":null,"subject":"[RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2016-02-01T22:22:31Z","receivedAt":"2016-02-01T22:22:31Z","isPatch":false,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"Format string %G? includes state 'N', which is described as \"no\nsignature\".\n\nIf you try to verify a commit or push for which you have no key (and you\ndon't automatically fetch from the keyservers [1]), then the format\nstring ALSO contains 'N', which is incorrect.\n\nIt should be possible to differentiate between a commit/push with NO\nsignature, and a commit/push signed with an unknown key.\n\nIn the case of verifying signed pushes before accepting them, this is\ncritical to providing a useful error message to the user. Presently, if\n%G? evaluates to 'N', then none of the GIT_PUSH_CERT* env vars are set.\n\nIn the case of the signed push with the unknown key, they should remain\nset.\n\n[1] Eg, if you have an externally curated keyring and use trust-model\nalways.\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Developer, Infrastructure Lead, Foundation Trustee\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85\n"},{"id":"277193","messageId":"xmqqmvrkt5ay.fsf@gitster.mtv.corp.google.com","threadId":"41303","inReplyTo":"robbat2-20160201T220940-187080200Z@orbis-terrarum.net","subject":"Re: [RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-01T22:49:09Z","receivedAt":"2016-02-01T22:49:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n\n> Format string %G? includes state 'N', which is described as \"no\n> signature\".\n>\n> If you try to verify a commit or push for which you have no key (and you\n> don't automatically fetch from the keyservers [1]), then the format\n> string ALSO contains 'N', which is incorrect.\n>\n> It should be possible to differentiate between a commit/push with NO\n> signature, and a commit/push signed with an unknown key.\n>\n> In the case of verifying signed pushes before accepting them, this is\n> critical to providing a useful error message to the user. Presently, if\n> %G? evaluates to 'N', then none of the GIT_PUSH_CERT* env vars are set.\n\nHrm, I think GIT_PUSH_CERT* environment variables are exported\nwhenever push_cert_sha1[] is not \"0\"{40}, and push_cert_sha1[] is\ncleared only when write_sha1_file() fails.  The failure from calling\nparse_signature(), verify_signed_buffer() and parse_gpg_output()\ndoes not clear push_cert_sha1[], so I am not sure if we are reading\nthe same code.\n\nAre you talking about something other than prepare_push_cert_sha1()?\n\n> In the case of the signed push with the unknown key, they should remain\n> set.\n\nIn any case, I think \"should\" is relative to the balance between\nconvenience and safety.  If you set up your receiving end to use a\nkeyring that holds trusted keys and nothing else, it makes it harder\nto make mistakes in the script and accept something you shouldn't if\nthe validation script is fed \"No, this is not good\" if a push is\nsigned by unknown key, so showing \"known to be bad\" and \"unsure if\nit is good\" the same way with 'N' is what \"should\" happen from that\npoint of view.\n\nOf course, a set-up that fetches unknown keys lazily when they are\nfirst encountered would need more work to do the verification\nthemselves, but as long as GIT_PUSH_CERT itself is exported that\nshould be possible (iow, we are trying to make simple way less error\nprone, while allowing more advanced use possible, if harder).\n\nIf the blob object name is not exported depending on the validation\nstatus, that certainly sounds like a bug, as that would make \"more\nadvanced use\" above impossible.  But I am not sure how that happens.\n"},{"id":"277199","messageId":"robbat2-20160202T002257-169038640Z@orbis-terrarum.net","threadId":"41303","inReplyTo":"xmqqmvrkt5ay.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2016-02-02T00:43:30Z","receivedAt":"2016-02-02T00:43:30Z","isPatch":false,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Mon, Feb 01, 2016 at 02:49:09PM -0800,  Junio C Hamano wrote:\n> Are you talking about something other than prepare_push_cert_sha1()?\nI went and verified it, and what was reported to me was slightly wrong. Only\nsome of the field are empty, notably CERT_KEY and SIGNER.\n\nSigned push with an unknown key:\n===\nremote: No signature found\nremote: Your push was not signed with a known key.\nremote: You MUST use git push --signed with a known key.\nremote: If you just updated your key, please wait 15 minutes for sync.\nremote: git-receive-pack variables:\nremote: GIT_PUSH_CERT='1c471177906014e65e2825ee71572bf749970c16'\nremote: GIT_PUSH_CERT_KEY=''\nremote: GIT_PUSH_CERT_NONCE='1454372558-35db7be4533958f14731'\nremote: GIT_PUSH_CERT_NONCE_SLOP=''\nremote: GIT_PUSH_CERT_NONCE_STATUS='OK'\nremote: GIT_PUSH_CERT_SIGNER=''\nremote: GIT_PUSH_CERT_STATUS='N'\nTo git+ssh://git@git.gentoo.org/repo/gentoo\n ! [remote rejected] master -> master (pre-receive hook declined)\n===\n\nUnsigned push:\n===\nremote: Unknown GIT_PUSH_CERT_STATUS\nremote: Your push was not signed with a known key.\nremote: You MUST use git push --signed with a known key.\nremote: If you just updated your key, please wait 15 minutes for sync.\nremote: git-receive-pack variables:\nremote: GIT_PUSH_CERT=''\nremote: GIT_PUSH_CERT_KEY=''\nremote: GIT_PUSH_CERT_NONCE=''\nremote: GIT_PUSH_CERT_NONCE_SLOP=''\nremote: GIT_PUSH_CERT_NONCE_STATUS=''\nremote: GIT_PUSH_CERT_SIGNER=''\nremote: GIT_PUSH_CERT_STATUS=''\nTo git+ssh://git@git.gentoo.org/repo/gentoo\n ! [remote rejected] master -> master (pre-receive hook declined)\n===\n\nHere's the raw blob, while it still exists.\n====\ncertificate version 0.1\npusher 0x55272E9740B89B54 1454372558 -0800\npushee git+ssh://git.gentoo.org/repo/gentoo\nnonce 1454372558-35db7be4533958f14731\n\n99a4d87ed7b79ea050adb99f941accf33e4ba963 71535a9475cdd4949c4031676238dc9f60e1828a refs/heads/master\n-----BEGIN PGP SIGNATURE-----\n...\n-----END PGP SIGNATURE-----\n====\n\n\n> > In the case of the signed push with the unknown key, they should remain\n> > set.\n> \n> In any case, I think \"should\" is relative to the balance between\n> convenience and safety.  If you set up your receiving end to use a\n> keyring that holds trusted keys and nothing else, it makes it harder\n> to make mistakes in the script and accept something you shouldn't if\n> the validation script is fed \"No, this is not good\" if a push is\n> signed by unknown key, so showing \"known to be bad\" and \"unsure if\n> it is good\" the same way with 'N' is what \"should\" happen from that\n> point of view.\nOk, at the very least, can we consider populating GIT_PUSH_CERT_KEY and\nGIT_PUSH_CERT_SIGNER even with GIT_PUSH_CERT_STATUS set to N?\n\nThen change the documentation to say 'No valid signature' rather than 'No\nsignature'? (introducing another letter would be better I think).\n\n> \n> Of course, a set-up that fetches unknown keys lazily when they are\n> first encountered would need more work to do the verification\n> themselves, but as long as GIT_PUSH_CERT itself is exported that\n> should be possible (iow, we are trying to make simple way less error\n> prone, while allowing more advanced use possible, if harder).\nIf it fetches the new key, and finds it be in some WOT or external trustdb, it\ncould accept it.\n\n> If the blob object name is not exported depending on the validation\n> status, that certainly sounds like a bug, as that would make \"more\n> advanced use\" above impossible.  But I am not sure how that happens.\nI think the earlier blobs got GC'd, hence why I didn't find them.\n\nAdvanced usage: Maybe record them to a ref of failed pushes (would be in the\nhook to check the signature).\n\nI think after this is cleaned up, I'll send the Gentoo require-signed-push hook\nfor inclusion in contrib/.\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Developer, Infrastructure Lead, Foundation Trustee\nE-Mail     : robbat2@gentoo.org\nGnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85\n"},{"id":"277205","messageId":"xmqq1t8vu7gj.fsf@gitster.mtv.corp.google.com","threadId":"41303","inReplyTo":"robbat2-20160202T002257-169038640Z@orbis-terrarum.net","subject":"Re: [RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-02T03:17:16Z","receivedAt":"2016-02-02T03:17:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Robin H. Johnson\" <robbat2@gentoo.org> writes:\n\n> On Mon, Feb 01, 2016 at 02:49:09PM -0800,  Junio C Hamano wrote:\n>> Are you talking about something other than prepare_push_cert_sha1()?\n> I went and verified it, and what was reported to me was slightly wrong. Only\n> some of the field are empty, notably CERT_KEY and SIGNER.\n>\n> Signed push with an unknown key:\n> ===\n> remote: No signature found\n> remote: Your push was not signed with a known key.\n> remote: You MUST use git push --signed with a known key.\n> remote: If you just updated your key, please wait 15 minutes for sync.\n> remote: git-receive-pack variables:\n> remote: GIT_PUSH_CERT='1c471177906014e65e2825ee71572bf749970c16'\n> remote: GIT_PUSH_CERT_KEY=''\n> remote: GIT_PUSH_CERT_NONCE='1454372558-35db7be4533958f14731'\n> remote: GIT_PUSH_CERT_NONCE_SLOP=''\n> remote: GIT_PUSH_CERT_NONCE_STATUS='OK'\n> remote: GIT_PUSH_CERT_SIGNER=''\n> remote: GIT_PUSH_CERT_STATUS='N'\n\nOK, this matches my expectation, and my earlier response to you is\nconsistent with the above output, so there isn't much to add to the\ndiscussion from me.  I was primarily worried about GIT_PUSH_CERT not\nbeing passed, but that does not seem to be the case, which is good.\nWe still give GIT_PUSH_CERT, which makes it possible to write a\nvalidation hook that lazily fetches unknown keys as needed to\nimplement its own more advanced checks, while allowing validation\nhooks that rely on a set of a-priori known keys to be written in a\nless error-prone way by saying \"N\" for \"unknown\" case.\n\nThanks.\n"}]}