Re: [PATCH v3 7/7] ssh signing: verify ssh-keygen in test prereq

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 19, 2021, 06:15 UTC
Message-ID: <xmqqfsrssmar.fsf@gitster.g>
In-Reply-To: <20211117093529.13953-8-fs@gigacodes.de>

Fabian Stelzer <fs@gigacodes.de> writes:

Show 9 quoted lines

> @@ -123,12 +119,19 @@ test_lazy_prereq GPGSSH '
>  	echo "\"principal with number 2\" $(cat "${GPGSSH_KEY_SECONDARY}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
>  	ssh-keygen -t ed25519 -N "${GPGSSH_KEY_PASSPHRASE}" -C "git ed25519 encrypted key" -f "${GPGSSH_KEY_WITH_PASSPHRASE}" >/dev/null &&
>  	echo "\"principal with number 3\" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
> -	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
> +	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null &&
> +
> +	# Verify if at least one key and ssh-keygen works as expected
> +	echo "testpayload" | ssh-keygen -Y sign -n "git" -f "${GPGSSH_KEY_PRIMARY}" > gpgssh_prereq.sig &&

Style:
 . Avoid overlong line by breaking the line after pipe.
 . No SP between redirection operator '>' and redirection target.

	echo "testpayload" |
	ssh-keygen -Y sign -n "git" -f "${GPGSSH_KEY_PRIMARY}" >gpgssh_prereq.sig &&

Also I wonder if preparation of GPGSSH_ALLOWED_SIGNERS file can be simplified and made easier to read. Instead of adding one line at a time like this:

>  	echo "\"principal with not yet valid key\" valid-after=\"29990101000000\" $(cat "${GPGSSH_KEY_NOTYETVALID}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}"

if you define variables with meaningful name and used here document, e.g.

	ts2005a=200504070000 ts2005b=200504100000 &&
	key_timeboxedvalid=$(cat "${GPGSSH_KEY_TIMEBOXEDVALID}.pub") &&

	ts2999=299901010000 &&
	key_notyetvalid=$(cat "${GPGSSH_KEY_NOTYETVALID}.pub") &&

	cat >"${GPGSSH_ALLOWED_SIGNERS}" <<-EOF &&
	"principal with not yet valid key" valid-after="$ts2999" $key_notyetvalid
	"timeboxed valid key" valid-after="$ts2005a",valid-before="$ts2005b" $key_timeboxedvalid
	...
	EOF