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

From: Fabian Stelzer <fs@gigacodes.de>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: Re: [PATCH v4 7/7] ssh signing: verify ssh-keygen in test prereq
Date: Thu, 2 Dec 2021 10:31:26 +0100	[thread overview]
Message-ID: <20211202093126.nuuvxjnhbkdu5pwn@fs> (raw)
In-Reply-To: <xmqqczmfyi44.fsf@gitster.g>

On 01.12.2021 16:18, Junio C Hamano wrote:
>Fabian Stelzer <fs@gigacodes.de> writes:
>
>> Do a full ssh signing, find-principals and verify operation in the test
>> prereq's to make sure ssh-keygen works as expected. Only generating the
>> keys and verifying its presence is not sufficient in some situations.
>> One example was ssh-keygen creating unusable ssh keys in cygwin because
>> of unsafe default permissions for the key files. The other a broken
>> openssh 8.7 that segfaulted on any find-principals operation. This
>> extended prereq check avoids future test breakages in case ssh-keygen or
>> any environment behaviour changes.
>>
>> Signed-off-by: Fabian Stelzer <fs@gigacodes.de>
>> ---
>
>The way keys are set-up has become much easier to follow.
>
>This unfortunately interacts with the old way of adding a test key
>done in <20211119150707.3924636-2-fs@gigacodes.de> 350a2518 (ssh
>signing: support non ssh-* keytypes, 2021-11-19)
>
>Here is my attempt (which is in 'seen') to resolve the inevitable
>merge conflicts between the topics.

Yes, that looks good. In this case the conflict is rather trivial, but how 
could i prevent this / make it easier for you to merge these?
Especially since in this case the conflict only arose after a reroll when 
both topics were already in seen. For a new topic i can of course make them 
as "on top of XXX". Should I in the future rebase the "support non ssh-* 
keytypes" topic on top of this series and mark it as such? Or whats a good 
way to deal with things like this? (besides avoiding merge conflicts 
altogether :D)

Thanks

>
>Thanks.
>
>commit fa6c2973744b419c95b5eaf6a697c795ab7823fa
>Merge: 2a8505f6a0 3b4b5a793a
>Author: Junio C Hamano <gitster@pobox.com>
>Date:   Wed Dec 1 16:01:54 2021 -0800
>
>    Merge branch 'fs/ssh-signing-other-keytypes' into jch
>
>    * fs/ssh-signing-other-keytypes:
>      ssh signing: make sign/amend test more resilient
>      ssh signing: support non ssh-* keytypes
>
>diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
>index ff944f0548..3e7ee1386a 100644
>--- a/t/lib-gpg.sh
>+++ b/t/lib-gpg.sh
>@@ -117,13 +117,14 @@ test_lazy_prereq GPGSSH '
> 	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_PRIMARY}" >/dev/null &&
> 	ssh-keygen -t rsa -b 2048 -N "" -C "git rsa2048 key" -f "${GPGSSH_KEY_SECONDARY}" >/dev/null &&
> 	ssh-keygen -t ed25519 -N "${GPGSSH_KEY_PASSPHRASE}" -C "git ed25519 encrypted key" -f "${GPGSSH_KEY_WITH_PASSPHRASE}" >/dev/null &&
>-<<<<<<< 2a8505f6a0 (Merge branch 'fs/ssh-signing-key-lifetime' into jch)
>+	ssh-keygen -t ecdsa -N "" -f "${GPGSSH_KEY_ECDSA}" >/dev/null &&
> 	ssh-keygen -t ed25519 -N "" -C "git ed25519 key" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null &&
>
> 	cat >"${GPGSSH_ALLOWED_SIGNERS}" <<-EOF &&
> 	"principal with number 1" $(cat "${GPGSSH_KEY_PRIMARY}.pub")"
> 	"principal with number 2" $(cat "${GPGSSH_KEY_SECONDARY}.pub")"
> 	"principal with number 3" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")"
>+	"principal with number 4" $(cat "${GPGSSH_KEY_ECDSA}.pub")"
> 	EOF
>
> 	# Verify if at least one key and ssh-keygen works as expected
>@@ -166,15 +167,6 @@ test_lazy_prereq GPGSSH_VERIFYTIME '
> 	echo "testpayload" |
> 	ssh-keygen -Y sign -n "git" -f "${GPGSSH_KEY_EXPIRED}" >gpgssh_verifytime_prereq.sig &&
> 	! (ssh-keygen -Y verify -n "git" -f "${GPGSSH_ALLOWED_SIGNERS}" -I "principal with expired key" -s gpgssh_verifytime_prereq.sig)
>-||||||| cd3e606211
>-	echo "\"principal with number 3\" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
>-	ssh-keygen -t ed25519 -N "" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
>-=======
>-	echo "\"principal with number 3\" $(cat "${GPGSSH_KEY_WITH_PASSPHRASE}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
>-	ssh-keygen -t ecdsa -N "" -f "${GPGSSH_KEY_ECDSA}" >/dev/null
>-	echo "\"principal with number 4\" $(cat "${GPGSSH_KEY_ECDSA}.pub")" >> "${GPGSSH_ALLOWED_SIGNERS}" &&
>-	ssh-keygen -t ed25519 -N "" -f "${GPGSSH_KEY_UNTRUSTED}" >/dev/null
>->>>>>>> 3b4b5a793a (ssh signing: make sign/amend test more resilient)
> '
>
> sanitize_pgp() {