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

On 28.07.21 23:34, Junio C Hamano wrote:

"Fabian Stelzer via GitGitGadget" [off-list ref] writes:

quoted

From: Fabian Stelzer <redacted>

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

quoted

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

"ot" -> "of".

Fixed

quoted

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

quoted

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