Re: [PATCH v6 4/9] ssh signing: provide a textual representation of the signing key

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 28.07.21 23:34, Junio C Hamano wrote:
"Fabian Stelzer via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

From: Fabian Stelzer <fs@xxxxxxxxxxxx>

for ssh the user.signingkey can be a filename/path or even a literal ssh pubkey.
in push certs and textual output we prefer the ssh fingerprint instead.

These sentences that lack the initial capital letters would look
unusual and distracting in our "git log --no-merges" stream.


Fixed

+/* Returns a textual but unique representation ot the signing key */

"ot" -> "of".


Fixed

+const char *get_signing_key_id(void)
+{
+	if (!strcmp(use_format->name, "ssh")) {
+		return get_ssh_key_fingerprint(get_signing_key());
+	} else {
+		/* GPG/GPGSM only store a key id on this variable */
+		return get_signing_key();

Hmph, we could ask gpg key fingerprint if we wanted to, and we
cannot tell why "ssh" side needs a separate "key" and "key_id"
while "gpg" side does not.  Hopefully it will become clear as we
read on?

Again, dispatching on use_format->name looked rather unexpected.


i will put the two strcmp(ssh) ifs on my todo list to also replace with a callback function.

-	char *signing_key = xstrdup(get_signing_key());
+	char *signing_key_id = xstrdup(get_signing_key_id());
  	const char *cp, *np;
  	struct strbuf cert = STRBUF_INIT;
  	int update_seen = 0;
strbuf_addstr(&cert, "certificate version 0.1\n");
-	strbuf_addf(&cert, "pusher %s ", signing_key);
+	strbuf_addf(&cert, "pusher %s ", signing_key_id);

Ahh...  We do not send GPG fingerprint in push certificate but you
want to use the fingerprint when signing with SSH keys, and that is
where the need for signing_key_id comes from?

OK.


Previously the push certs contained the configured user.signingkey as "pusher". For gpg this is usually the key id. (e.g.: ABCDEF01) For ssh signing this can now be a file path which would not make much sense to put into the push cert. I did not use the public ssh key since the file can also contain an encrypted private key, so i would have to ask ssh-keygen for the public key anyway.
Since the ssh fingerprint more resembles the gpg key id i used it instead.

As far as i understand the actual contents of the "pusher" header is not really relevant for the push-cert. The unique nonce is important but besides that its just a signed text blob. If we don't care about having a local users file path in this header we could drop this commit.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux