Re: [PATCH v6 4/9] ssh signing: provide a textual representation of the signing key
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 28, 2021, 21:34 UTC
- Message-ID
- <xmqqeebi86m7.fsf@gitster.g>
- In-Reply-To
- <7d1d131ff5b43559c8a750ebdfd6faaba93c1ad1.1627501009.git.gitgitgadget@gmail.com>
"Fabian Stelzer via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Fabian Stelzer <fs@gigacodes.de> > > 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.
Show 43 quoted lines
> +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".
Show 7 quoted lines
> +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.
Show 40 quoted lines
> + }
> +}
> +
> 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.