[RFC] GPG-Signed pushes & commits: differentiating between no signature and an unknown key

Format string %G? includes state 'N', which is described as "no signature".

If you try to verify a commit or push for which you have no key (and you don't automatically fetch from the keyservers [1]), then the format string ALSO contains 'N', which is incorrect.

It should be possible to differentiate between a commit/push with NO signature, and a commit/push signed with an unknown key.

In the case of verifying signed pushes before accepting them, this is critical to providing a useful error message to the user. Presently, if %G? evaluates to 'N', then none of the GIT_PUSH_CERT* env vars are set.

In the case of the signed push with the unknown key, they should remain set.

[1] Eg, if you have an externally curated keyring and use trust-model always.

"Robin H. Johnson" <robbat2@gentoo.org> writes:

Hrm, I think GIT_PUSH_CERT* environment variables are exported whenever push_cert_sha1[] is not "0"{40}, and push_cert_sha1[] is cleared only when write_sha1_file() fails. The failure from calling parse_signature(), verify_signed_buffer() and parse_gpg_output() does not clear push_cert_sha1[], so I am not sure if we are reading the same code.

Are you talking about something other than prepare_push_cert_sha1()?

In any case, I think "should" is relative to the balance between convenience and safety. If you set up your receiving end to use a keyring that holds trusted keys and nothing else, it makes it harder to make mistakes in the script and accept something you shouldn't if the validation script is fed "No, this is not good" if a push is signed by unknown key, so showing "known to be bad" and "unsure if it is good" the same way with 'N' is what "should" happen from that point of view.

Of course, a set-up that fetches unknown keys lazily when they are first encountered would need more work to do the verification themselves, but as long as GIT_PUSH_CERT itself is exported that should be possible (iow, we are trying to make simple way less error prone, while allowing more advanced use possible, if harder).

If the blob object name is not exported depending on the validation status, that certainly sounds like a bug, as that would make "more advanced use" above impossible. But I am not sure how that happens.

On Mon, Feb 01, 2016 at 02:49:09PM -0800,  Junio C Hamano wrote:

I went and verified it, and what was reported to me was slightly wrong. Only some of the field are empty, notably CERT_KEY and SIGNER.

Signed push with an unknown key:
===
remote: No signature found
remote: Your push was not signed with a known key.
remote: You MUST use git push --signed with a known key.
remote: If you just updated your key, please wait 15 minutes for sync.
remote: git-receive-pack variables:
remote: GIT_PUSH_CERT='1c471177906014e65e2825ee71572bf749970c16'
remote: GIT_PUSH_CERT_KEY=''
remote: GIT_PUSH_CERT_NONCE='1454372558-35db7be4533958f14731'
remote: GIT_PUSH_CERT_NONCE_SLOP=''
remote: GIT_PUSH_CERT_NONCE_STATUS='OK'
remote: GIT_PUSH_CERT_SIGNER=''
remote: GIT_PUSH_CERT_STATUS='N'
To git+ssh://git@git.gentoo.org/repo/gentoo
 ! [remote rejected] master -> master (pre-receive hook declined)
===

Unsigned push:
===
remote: Unknown GIT_PUSH_CERT_STATUS
remote: Your push was not signed with a known key.
remote: You MUST use git push --signed with a known key.
remote: If you just updated your key, please wait 15 minutes for sync.
remote: git-receive-pack variables:
remote: GIT_PUSH_CERT=''
remote: GIT_PUSH_CERT_KEY=''
remote: GIT_PUSH_CERT_NONCE=''
remote: GIT_PUSH_CERT_NONCE_SLOP=''
remote: GIT_PUSH_CERT_NONCE_STATUS=''
remote: GIT_PUSH_CERT_SIGNER=''
remote: GIT_PUSH_CERT_STATUS=''
To git+ssh://git@git.gentoo.org/repo/gentoo
 ! [remote rejected] master -> master (pre-receive hook declined)
===

Here's the raw blob, while it still exists. ==== certificate version 0.1 pusher 0x55272E9740B89B54 1454372558 -0800 pushee git+ssh://git.gentoo.org/repo/gentoo nonce 1454372558-35db7be4533958f14731

99a4d87ed7b79ea050adb99f941accf33e4ba963 71535a9475cdd4949c4031676238dc9f60e1828a refs/heads/master -----BEGIN PGP SIGNATURE----- ... -----END PGP SIGNATURE----- ====

Ok, at the very least, can we consider populating GIT_PUSH_CERT_KEY and GIT_PUSH_CERT_SIGNER even with GIT_PUSH_CERT_STATUS set to N?

Then change the documentation to say 'No valid signature' rather than 'No signature'? (introducing another letter would be better I think).

If it fetches the new key, and finds it be in some WOT or external trustdb, it could accept it.

I think the earlier blobs got GC'd, hence why I didn't find them.

Advanced usage: Maybe record them to a ref of failed pushes (would be in the hook to check the signature).

I think after this is cleaned up, I'll send the Gentoo require-signed-push hook for inclusion in contrib/.

"Robin H. Johnson" <robbat2@gentoo.org> writes:

OK, this matches my expectation, and my earlier response to you is consistent with the above output, so there isn't much to add to the discussion from me. I was primarily worried about GIT_PUSH_CERT not being passed, but that does not seem to be the case, which is good. We still give GIT_PUSH_CERT, which makes it possible to write a validation hook that lazily fetches unknown keys as needed to implement its own more advanced checks, while allowing validation hooks that rely on a set of a-priori known keys to be written in a less error-prone way by saying "N" for "unknown" case.

Thanks.