git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Fabian Stelzer via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Han-Wen Nienhuys" <hanwen@google.com>,
	"Fabian Stelzer" <fs@gigacodes.de>,
	"brian m. carlson" <sandals@crustytoothpaste.net>,
	"Randall S. Becker" <rsbecker@nexbridge.com>,
	"Bagas Sanjaya" <bagasdotme@gmail.com>,
	"Hans Jerry Illikainen" <hji@dyntopia.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>
Subject: Re: [PATCH v3 3/9] ssh signing: retrieve a default key from ssh-agent
Date: Wed, 14 Jul 2021 13:20:47 -0700	[thread overview]
Message-ID: <xmqqr1g0ve8w.fsf@gitster.g> (raw)
In-Reply-To: <b84b2812470ea45a85d624ec339f35bb0107493d.1626264613.git.gitgitgadget@gmail.com> (Fabian Stelzer via GitGitGadget's message of "Wed, 14 Jul 2021 12:10:07 +0000")

"Fabian Stelzer via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Fabian Stelzer <fs@gigacodes.de>
>
> calls ssh-add -L and uses the first key

Documentation/SubmittingPatches::[[describe-changes]].

> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
> ---
>  gpg-interface.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/gpg-interface.c b/gpg-interface.c
> index 3c9a48c8e7e..c956ed87475 100644
> --- a/gpg-interface.c
> +++ b/gpg-interface.c
> @@ -467,6 +467,23 @@ int git_gpg_config(const char *var, const char *value, void *cb)
>  	return 0;
>  }
>  
> +/* Returns the first public key from an ssh-agent to use for signing */
> +static char *get_default_ssh_signing_key(void) {

Style.  Open and close braces around a function sit on their own
lines by themselves.

> +	struct child_process ssh_add = CHILD_PROCESS_INIT;
> +	int ret = -1;
> +	struct strbuf key_stdout = STRBUF_INIT;
> +	struct strbuf **keys;

Whose releasing the resource held by "keys" when we return?

> +	strvec_pushl(&ssh_add.args, "ssh-add", "-L", NULL);
> +	ret = pipe_command(&ssh_add, NULL, 0, &key_stdout, 0, NULL, 0);

I often load about half a dozen keys to my ssh-agent so "ssh-add -L"
will give me multi-line output.  I know you wrote "the first public
key" above, but that does not mean users who needs to have multiple
keys can be limited to use only the first key for signing.  There
should be a way to say "I may have many keys for other reasons, but
for signing I want to use this key, not the other ones".

> +	if (!ret) {
> +		keys = strbuf_split_max(&key_stdout, '\n', 2);

Let's not use strbuf_split_*() that is a horribly wrong interface.
You do not want a set of elastic buffer after splitting.  You only
are peeking the first line, no?  You are leaking keys[] array and
probably keys[1], too.

	eol = strchrnul(key_stdout.buf, '\n');
	strbuf_setlen(&key_stdout, eol - key_stdout.buf);

or something along that line, perhaps?

> +		if (keys[0])
> +			return strbuf_detach(keys[0], NULL);
> +	}
> +
> +	return "";
> +}
>  const char *get_signing_key(void)

Missing blank line after the function body.

>  {
>  	if (configured_signing_key)

  reply	other threads:[~2021-07-14 20:20 UTC|newest]

Thread overview: 153+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-06  8:19 [PATCH] Add commit & tag signing/verification via SSH keys using ssh-keygen Fabian Stelzer via GitGitGadget
2021-07-06 10:07 ` Han-Wen Nienhuys
2021-07-06 11:23   ` Fabian Stelzer
2021-07-06 14:44 ` brian m. carlson
2021-07-06 15:33   ` Fabian Stelzer
2021-07-06 15:04 ` Junio C Hamano
2021-07-06 15:45   ` Fabian Stelzer
2021-07-06 17:55     ` Junio C Hamano
2021-07-06 19:39     ` Randall S. Becker
2021-07-07  6:26 ` Bagas Sanjaya
2021-07-07  8:48   ` Fabian Stelzer
2021-07-12 12:19 ` [PATCH v2] Add commit, tag & push " Fabian Stelzer via GitGitGadget
2021-07-12 16:55   ` Ævar Arnfjörð Bjarmason
2021-07-12 20:35     ` Fabian Stelzer
2021-07-12 21:16       ` Felipe Contreras
2021-07-14 12:10   ` [PATCH v3 0/9] RFC: Add commit & tag " Fabian Stelzer via GitGitGadget
2021-07-14 12:10     ` [PATCH v3 1/9] Add commit, tag & push signing via SSH keys Fabian Stelzer via GitGitGadget
2021-07-14 18:19       ` Junio C Hamano
2021-07-14 23:57         ` Eric Sunshine
2021-07-15  8:20         ` Fabian Stelzer
2021-07-14 12:10     ` [PATCH v3 2/9] ssh signing: add documentation Fabian Stelzer via GitGitGadget
2021-07-14 20:07       ` Junio C Hamano
2021-07-15  8:48         ` Fabian Stelzer
2021-07-15 10:43           ` Bagas Sanjaya
2021-07-15 16:29           ` Junio C Hamano
2021-07-14 12:10     ` [PATCH v3 3/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-07-14 20:20       ` Junio C Hamano [this message]
2021-07-15  7:49         ` Han-Wen Nienhuys
2021-07-15  8:06           ` Fabian Stelzer
2021-07-15  8:13         ` Fabian Stelzer
2021-07-14 12:10     ` [PATCH v3 4/9] ssh signing: sign using either gpg or ssh keys Fabian Stelzer via GitGitGadget
2021-07-14 20:32       ` Junio C Hamano
2021-07-15  8:28         ` Fabian Stelzer
2021-07-14 12:10     ` [PATCH v3 5/9] ssh signing: provide a textual representation of the signing key Fabian Stelzer via GitGitGadget
2021-07-14 12:10     ` [PATCH v3 6/9] ssh signing: parse ssh-keygen output and verify signatures Fabian Stelzer via GitGitGadget
2021-07-16  0:07       ` Gwyneth Morgan
2021-07-16  7:00         ` Fabian Stelzer
2021-07-14 12:10     ` [PATCH v3 7/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-07-14 12:10     ` [PATCH v3 8/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-07-14 12:10     ` [PATCH v3 9/9] ssh signing: add more tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-07-19 13:33     ` [PATCH v4 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 1/9] ssh signing: preliminary refactoring and clean-up Fabian Stelzer via GitGitGadget
2021-07-19 23:07         ` Junio C Hamano
2021-07-19 13:33       ` [PATCH v4 2/9] ssh signing: add ssh signature format and signing using ssh keys Fabian Stelzer via GitGitGadget
2021-07-19 23:53         ` Junio C Hamano
2021-07-20 12:26           ` Fabian Stelzer
2021-07-19 13:33       ` [PATCH v4 3/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 4/9] ssh signing: provide a textual representation of the signing key Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 5/9] ssh signing: parse ssh-keygen output and verify signatures Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 6/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 7/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 8/9] ssh signing: add more tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-07-19 13:33       ` [PATCH v4 9/9] ssh signing: add documentation Fabian Stelzer via GitGitGadget
2021-07-20  0:38       ` [PATCH v4 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Junio C Hamano
2021-07-27 13:15       ` [PATCH v5 " Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 1/9] ssh signing: preliminary refactoring and clean-up Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 2/9] ssh signing: add ssh signature format and signing using ssh keys Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 3/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 4/9] ssh signing: provide a textual representation of the signing key Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 5/9] ssh signing: parse ssh-keygen output and verify signatures Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 6/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 7/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 8/9] ssh signing: add more tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-07-27 13:15         ` [PATCH v5 9/9] ssh signing: add documentation Fabian Stelzer via GitGitGadget
2021-07-28 19:36         ` [PATCH v6 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Fabian Stelzer via GitGitGadget
2021-07-28 19:36           ` [PATCH v6 1/9] ssh signing: preliminary refactoring and clean-up Fabian Stelzer via GitGitGadget
2021-07-28 22:32             ` Jonathan Tan
2021-07-29  0:58               ` Junio C Hamano
2021-07-29  7:44                 ` Fabian Stelzer
2021-07-29  8:43               ` Fabian Stelzer
2021-07-28 19:36           ` [PATCH v6 2/9] ssh signing: add ssh signature format and signing using ssh keys Fabian Stelzer via GitGitGadget
2021-07-28 22:45             ` Jonathan Tan
2021-07-29  1:01               ` Junio C Hamano
2021-07-29 11:01               ` Fabian Stelzer
2021-07-29 19:09             ` Josh Steadmon
2021-07-29 21:25               ` Fabian Stelzer
2021-07-28 19:36           ` [PATCH v6 3/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-07-28 21:29             ` Junio C Hamano
2021-07-28 22:48             ` Jonathan Tan
2021-07-29  8:59               ` Fabian Stelzer
2021-07-29 19:09                 ` Josh Steadmon
2021-07-29 19:56                   ` Junio C Hamano
2021-07-29 21:21                   ` Fabian Stelzer
2021-07-28 19:36           ` [PATCH v6 4/9] ssh signing: provide a textual representation of the signing key Fabian Stelzer via GitGitGadget
2021-07-28 21:34             ` Junio C Hamano
2021-07-29  8:21               ` Fabian Stelzer
2021-07-28 19:36           ` [PATCH v6 5/9] ssh signing: parse ssh-keygen output and verify signatures Fabian Stelzer via GitGitGadget
2021-07-28 21:55             ` Junio C Hamano
2021-07-29  9:12               ` Fabian Stelzer
2021-07-29 20:43                 ` Junio C Hamano
2021-07-28 23:04             ` Jonathan Tan
2021-07-29  9:48               ` Fabian Stelzer
2021-07-29 13:52                 ` Fabian Stelzer
2021-08-03  7:43                   ` Fabian Stelzer
2021-08-03  9:33                     ` Fabian Stelzer
2021-07-29 20:46                 ` Junio C Hamano
2021-07-29 21:01                   ` Randall S. Becker
2021-07-29 21:12                     ` Fabian Stelzer
2021-07-29 21:25                       ` Randall S. Becker
2021-07-29 21:28                         ` Fabian Stelzer
2021-07-29 22:28                           ` Randall S. Becker
2021-07-30  8:17                             ` Fabian Stelzer
2021-07-30 14:26                               ` Randall S. Becker
2021-07-30 14:32                                 ` Fabian Stelzer
2021-07-30 15:05                                   ` Randall S. Becker
2021-07-28 19:36           ` [PATCH v6 6/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-07-29 19:09             ` Josh Steadmon
2021-07-29 19:57               ` Junio C Hamano
2021-07-30  7:32               ` Fabian Stelzer
2021-07-28 19:36           ` [PATCH v6 7/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-07-28 19:36           ` [PATCH v6 8/9] ssh signing: add more tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-07-28 19:36           ` [PATCH v6 9/9] ssh signing: add documentation Fabian Stelzer via GitGitGadget
2021-07-29  8:19           ` [PATCH v6 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Bagas Sanjaya
2021-07-29 11:03             ` Fabian Stelzer
2021-08-03 13:45           ` [PATCH v7 " Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 1/9] ssh signing: preliminary refactoring and clean-up Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 2/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 3/9] ssh signing: add ssh key format and signing code Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 4/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 5/9] ssh signing: provide a textual signing_key_id Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 6/9] ssh signing: verify signatures using ssh-keygen Fabian Stelzer via GitGitGadget
2021-08-03 23:47               ` Junio C Hamano
2021-08-04  9:01                 ` Fabian Stelzer
2021-08-04 17:32                   ` Junio C Hamano
2021-08-03 13:45             ` [PATCH v7 7/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 8/9] ssh signing: tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-08-03 13:45             ` [PATCH v7 9/9] ssh signing: test that gpg fails for unkown keys Fabian Stelzer via GitGitGadget
2021-08-29 22:15             ` [PATCH v7 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Junio C Hamano
2021-08-29 23:56               ` Gwyneth Morgan
2021-08-30 10:35               ` Fabian Stelzer
2021-09-07 17:35                 ` Junio C Hamano
2021-09-10  8:03                   ` Fabian Stelzer
2021-09-10 18:44                     ` Junio C Hamano
2021-09-10 19:49                       ` Fabian Stelzer
2021-09-10 20:20                         ` Carlo Arenas
2021-09-10 20:07             ` [PATCH v8 " Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 1/9] ssh signing: preliminary refactoring and clean-up Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 2/9] ssh signing: add test prereqs Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 3/9] ssh signing: add ssh key format and signing code Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 4/9] ssh signing: retrieve a default key from ssh-agent Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 5/9] ssh signing: provide a textual signing_key_id Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 6/9] ssh signing: verify signatures using ssh-keygen Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 7/9] ssh signing: duplicate t7510 tests for commits Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 8/9] ssh signing: tests for logs, tags & push certs Fabian Stelzer via GitGitGadget
2021-09-10 20:07               ` [PATCH v8 9/9] ssh signing: test that gpg fails for unknown keys Fabian Stelzer via GitGitGadget
2021-12-22  3:18                 ` t7510-signed-commit.sh hangs on old gpg, regression in 1bfb57f642d (was: [PATCH v8 9/9] ssh signing: test that gpg fails for unknown keys) Ævar Arnfjörð Bjarmason
2021-12-22 10:13                   ` Fabian Stelzer
2021-12-22 15:58                     ` brian m. carlson
2021-12-26 22:53                     ` Ævar Arnfjörð Bjarmason
2021-12-30 11:10                       ` Fabian Stelzer
2021-09-10 20:23               ` [PATCH v8 0/9] ssh signing: Add commit & tag signing/verification via SSH keys using ssh-keygen Junio C Hamano
2021-09-10 20:48                 ` Fabian Stelzer
2021-09-10 21:01                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqr1g0ve8w.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=fs@gigacodes.de \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hji@dyntopia.com \
    --cc=rsbecker@nexbridge.com \
    --cc=sandals@crustytoothpaste.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).