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

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

[Robin H. Johnson]

[-- Attachment #1: Type: text/plain, Size: 967 bytes --]

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 Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead, Foundation Trustee
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 445 bytes --]

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

[Junio C Hamano]

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

> 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.

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 the case of the signed push with the unknown key, they should remain
> set.

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.

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

[Robin H. Johnson]

On Mon, Feb 01, 2016 at 02:49:09PM -0800,  Junio C Hamano wrote:
> Are you talking about something other than prepare_push_cert_sha1()?
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-----
====


> > In the case of the signed push with the unknown key, they should remain
> > set.
> 
> 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.
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).

> 
> 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 it fetches the new key, and finds it be in some WOT or external trustdb, it
could accept it.

> 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.
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 Hugh Johnson
Gentoo Linux: Developer, Infrastructure Lead, Foundation Trustee
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85

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

[Junio C Hamano]

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

> On Mon, Feb 01, 2016 at 02:49:09PM -0800,  Junio C Hamano wrote:
>> Are you talking about something other than prepare_push_cert_sha1()?
> 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'

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.