Re: [PATCH v6 4/9] ssh signing: provide a textual representation of the signing key
From: Junio C Hamano <hidden>
Date: 2021-07-28 21:35:04
"Fabian Stelzer via GitGitGadget" [off-list ref] writes:
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.
quoted hunk
+static char *get_ssh_key_fingerprint(const char *signing_key) +{ + struct child_process ssh_keygen = CHILD_PROCESS_INIT; + int ret = -1; + struct strbuf fingerprint_stdout = STRBUF_INIT; + struct strbuf **fingerprint; + + /* + * With SSH Signing this can contain a filename or a public key + * For textual representation we usually want a fingerprint + */ + if (istarts_with(signing_key, "ssh-")) { + strvec_pushl(&ssh_keygen.args, "ssh-keygen", "-lf", "-", NULL); + ret = pipe_command(&ssh_keygen, signing_key, + strlen(signing_key), &fingerprint_stdout, 0, + NULL, 0); + } else { + strvec_pushl(&ssh_keygen.args, "ssh-keygen", "-lf", + configured_signing_key, NULL); + ret = pipe_command(&ssh_keygen, NULL, 0, &fingerprint_stdout, 0, + NULL, 0); + } + + if (!!ret) + die_errno(_("failed to get the ssh fingerprint for key '%s'"), + signing_key); + + fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3); + if (!fingerprint[1]) + die_errno(_("failed to get the ssh fingerprint for key '%s'"), + signing_key); + + return strbuf_detach(fingerprint[1], NULL); +} + /* Returns the first public key from an ssh-agent to use for signing */ static char *get_default_ssh_signing_key(void) {@@ -490,6 +525,17 @@ static char *get_default_ssh_signing_key(void) return ""; } +/* Returns a textual but unique representation ot the signing key */
"ot" -> "of".
+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.
quoted hunk
+ } +} + const char *get_signing_key(void) { if (configured_signing_key)diff --git a/gpg-interface.h b/gpg-interface.h index feac4decf8b..beefacbb1e9 100644 --- a/gpg-interface.h +++ b/gpg-interface.h@@ -64,6 +64,12 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature, int git_gpg_config(const char *, const char *, void *); void set_signing_key(const char *); const char *get_signing_key(void); + +/* + * Returns a textual unique representation of the signing key in use + * Either a GPG KeyID or a SSH Key Fingerprint + */ +const char *get_signing_key_id(void); int check_signature(const char *payload, size_t plen, const char *signature, size_t slen, struct signature_check *sigc);diff --git a/send-pack.c b/send-pack.c index 5a79e0e7110..50cca7e439b 100644 --- a/send-pack.c +++ b/send-pack.c@@ -341,13 +341,13 @@ static int generate_push_cert(struct strbuf *req_buf, { const struct ref *ref; struct string_list_item *item; - 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.