{"thread":{"id":"59790","subject":"[PATCH] t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0","startedAt":"2023-05-25T03:10:42Z","lastAt":"2023-06-06T21:47:31Z","messageCount":5,"participants":["Todd Zullinger","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"477709","messageId":"20230525031026.3554406-1-tmz@pobox.com","threadId":"59790","inReplyTo":null,"subject":"[PATCH] t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-25T03:10:24Z","receivedAt":"2023-05-25T03:10:42Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"OpenSSH-9.0 requires a namespace option with `-Y check-novalidate`.\nThis was added in openssh-portable commit a0b5816f8 (upstream:\nssh-keygen -Y check-novalidate requires namespace or SEGV, 2022-03-18).\n\nThe -n option was documented as a required option since check-novalidate\nwas added in openssh-portable 8aa2aa3cd (upstream: Allow testing\nsignature syntax and validity without verifying, 2019-09-16).\n\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nHi,\n\nI only recently noticed the GPGSSH_VERIFYTIME prereq had\nbeen failing in the Fedora builds.  This began when openssh\nwas updated to 9.0 in the distribution, which means I've\nbeen slack on checking missing prereqs since last August. :/\n\nInitially, I thought it was another issue, where the final\nssh-keygen call in the prereq lacked a message on stdin,\nwhich caused it to hang.\n\nThat only occurs when running the contents of the prereq\nmanually, so it may not be worth touching.  But if that\nseems worthwhile -- if only to avoid anyone else getting\nspending time debugging the wrong problem -- I can send the\npatch I had prepared which does:\n\n  diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\n  index 114785586a..2815df8503 100644\n  --- a/t/lib-gpg.sh\n  +++ b/t/lib-gpg.sh\n  @@ -165,7 +165,7 @@ test_lazy_prereq GPGSSH_VERIFYTIME '\n   \t# and verify ssh-keygen verifies the key lifetime\n   \techo \"testpayload\" |\n   \tssh-keygen -Y sign -n \"git\" -f \"${GPGSSH_KEY_EXPIRED}\" >gpgssh_verifytime_prereq.sig &&\n  -\t! (ssh-keygen -Y verify -n \"git\" -f \"${GPGSSH_ALLOWED_SIGNERS}\" -I \"principal with expired key\" -s gpgssh_verifytime_prereq.sig)\n  +\t! (echo \"testpayload\" | ssh-keygen -Y verify -n \"git\" -f \"${GPGSSH_ALLOWED_SIGNERS}\" -I \"principal with expired key\" -s gpgssh_verifytime_prereq.sig)\n   '\n   \n   sanitize_pgp() {\n\n t/lib-gpg.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 114785586a..28652ed91f 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -136,7 +136,7 @@ test_lazy_prereq GPGSSH '\n \n test_lazy_prereq GPGSSH_VERIFYTIME '\n \t# Check if ssh-keygen has a verify-time option by passing an invalid date to it\n-\tssh-keygen -Overify-time=INVALID -Y check-novalidate -s doesnotmatter 2>&1 | grep -q -F \"Invalid \\\"verify-time\\\"\" &&\n+\tssh-keygen -Overify-time=INVALID -Y check-novalidate -n \"git\" -s doesnotmatter 2>&1 | grep -q -F \"Invalid \\\"verify-time\\\"\" &&\n \n \t# Set up keys with key lifetimes\n \tssh-keygen -t ed25519 -N \"\" -C \"timeboxed valid key\" -f \"${GPGSSH_KEY_TIMEBOXEDVALID}\" >/dev/null &&\n-- \n2.41.0.rc2\n"},{"id":"477728","messageId":"xmqqsfbjeltg.fsf@gitster.g","threadId":"59790","inReplyTo":"20230525031026.3554406-1-tmz@pobox.com","subject":"Re: [PATCH] t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-26T04:52:27Z","receivedAt":"2023-05-26T04:52:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> OpenSSH-9.0 requires a namespace option with `-Y check-novalidate`.\n> This was added in openssh-portable commit a0b5816f8 (upstream:\n> ssh-keygen -Y check-novalidate requires namespace or SEGV, 2022-03-18).\n>\n> The -n option was documented as a required option since check-novalidate\n> was added in openssh-portable 8aa2aa3cd (upstream: Allow testing\n> signature syntax and validity without verifying, 2019-09-16).\n>\n> Signed-off-by: Todd Zullinger <tmz@pobox.com>\n> ---\n> Hi,\n>\n> I only recently noticed the GPGSSH_VERIFYTIME prereq had\n> been failing in the Fedora builds.  This began when openssh\n> was updated to 9.0 in the distribution, which means I've\n> been slack on checking missing prereqs since last August. :/\n\nBetter late than never.  Thanks.\n\nWhile I was trying to see if the symptom reproduces in my\nenvironment roughly based on Debian testing, I had this trivial test\nscript\n\n    #!/bin/sh\n\n    test_description='heh???'\n\n    . ./test-lib.sh\n    . \"$TEST_DIRECTORY/lib-gpg.sh\"\n\n    test_expect_success setup '\n            : test_have_prereq GPG &&\n            test_have_prereq GPGSSH_VERIFYTIME\n    '\n\n    test_done\n\nand noticed that GPGSSH_VERIFYTIME prerequisite does not pass\nregardless of the version of ssh-keygen installed, without first\ntriggering GPG prereq to cause \"$GNUPGHOME\" to get created.\nOtherwise, this part\n\n\t# Set up keys with key lifetimes\n\tssh-keygen -t ed25519 -N \"\" -C \"timeboxed valid key\" -f \"${GPGSSH_KEY_TIMEBOXEDVALID}\" >/dev/null &&\n\nbecause GPGSSH_KEY_TIMEBOXEDVALID is defined to be created under\nGNUPGHOME, would not work.\n\nI notice that GPGSM lazy prereq forces GPG prereq to be triggered\nby starting it like so:\n\n    test_lazy_prereq GPGSM '\n            test_have_prereq GPG &&\n\nand I think we should do the same for GPGSSH_VERIFYTIME for\ncompleteness in the longer term.  The current users of the\nprerequisite all seem to trigger GPG prerequisite check so\nthis is not all that urgent, though.\n"},{"id":"477730","messageId":"ZHBDbGjid-33cJb4@pobox.com","threadId":"59790","inReplyTo":"xmqqsfbjeltg.fsf@gitster.g","subject":"Re: [PATCH] t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-05-26T05:28:12Z","receivedAt":"2023-05-26T05:28:23Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Junio C Hamano wrote:\n> While I was trying to see if the symptom reproduces in my\n> environment roughly based on Debian testing, I had this trivial test\n> script\n> \n>     #!/bin/sh\n> \n>     test_description='heh???'\n> \n>     . ./test-lib.sh\n>     . \"$TEST_DIRECTORY/lib-gpg.sh\"\n> \n>     test_expect_success setup '\n>             : test_have_prereq GPG &&\n>             test_have_prereq GPGSSH_VERIFYTIME\n>     '\n> \n>     test_done\n> \n> and noticed that GPGSSH_VERIFYTIME prerequisite does not pass\n> regardless of the version of ssh-keygen installed, without first\n> triggering GPG prereq to cause \"$GNUPGHOME\" to get created.\n> Otherwise, this part\n> \n> \t# Set up keys with key lifetimes\n> \tssh-keygen -t ed25519 -N \"\" -C \"timeboxed valid key\" -f \"${GPGSSH_KEY_TIMEBOXEDVALID}\" >/dev/null &&\n> \n> because GPGSSH_KEY_TIMEBOXEDVALID is defined to be created under\n> GNUPGHOME, would not work.\n> \n> I notice that GPGSM lazy prereq forces GPG prereq to be triggered\n> by starting it like so:\n> \n>     test_lazy_prereq GPGSM '\n>             test_have_prereq GPG &&\n> \n> and I think we should do the same for GPGSSH_VERIFYTIME for\n> completeness in the longer term.  The current users of the\n> prerequisite all seem to trigger GPG prerequisite check so\n> this is not all that urgent, though.\n\nGood idea. Perhaps:\n\n\ttest_lazy_prereq GPGSSH_VERIFYTIME '\n\t\ttest_have_prereq GPGSSH &&\n\nis best there?  The GPGSSH prereq creates ${GNUPGHOME}.  It\nmay not be common, but there may be folks who want to run\nthe SSH tests and don't care about GPG.\n\nSomething like this?  (Sorry to distract you further in the\nRC period. :)\n\n-- 8< --\nSubject: [PATCH] t/lib-gpg: require GPGSSH for GPGSSH_VERIFYTIME prereq\n\nThe GPGSSH_VERIFYTIME prequeq makes use of \"${GNUPGHOME}\" but does not\ncreate it.  Require GPGSSH which creates the \"${GNUPGHOME}\" directory.\n\nAdditionally, it makes sense to require GPGSSH in GPGSSH_VERIFYTIME\nbecause the latter builds on the former.  If we can't use GPGSSH,\nthere's little point in checking whether GPGSSH_VERIFYTIME is usable.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 114785586a..db63aeb6ed 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -135,6 +135,7 @@ test_lazy_prereq GPGSSH '\n '\n \n test_lazy_prereq GPGSSH_VERIFYTIME '\n+\ttest_have_prereq GPGSSH &&\n \t# Check if ssh-keygen has a verify-time option by passing an invalid date to it\n \tssh-keygen -Overify-time=INVALID -Y check-novalidate -s doesnotmatter 2>&1 | grep -q -F \"Invalid \\\"verify-time\\\"\" &&\n \n-- 8< --\n\n-- \nTodd\n"},{"id":"477849","messageId":"xmqqa5xjeqmm.fsf@gitster.g","threadId":"59790","inReplyTo":"ZHBDbGjid-33cJb4@pobox.com","subject":"Re: [PATCH] t/lib-gpg: fix ssh-keygen -Y check-novalidate with openssh-9.0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-06-01T04:46:41Z","receivedAt":"2023-06-01T04:46:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger <tmz@pobox.com> writes:\n\n> Good idea. Perhaps:\n>\n> \ttest_lazy_prereq GPGSSH_VERIFYTIME '\n> \t\ttest_have_prereq GPGSSH &&\n>\n> is best there?  The GPGSSH prereq creates ${GNUPGHOME}.  It\n> may not be common, but there may be folks who want to run\n> the SSH tests and don't care about GPG.\n\nOK.  I'll certainly forget, so hold on to the patch and resend after\nthe dust settles from the release.\n\nThanks.\n"},{"id":"478117","messageId":"20230606214707.55739-1-tmz@pobox.com","threadId":"59790","inReplyTo":"xmqqa5xjeqmm.fsf@gitster.g","subject":"[PATCH] t/lib-gpg: require GPGSSH for GPGSSH_VERIFYTIME prereq","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-06-06T21:47:07Z","receivedAt":"2023-06-06T21:47:31Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"The GPGSSH_VERIFYTIME prequeq makes use of \"${GNUPGHOME}\" but does not\ncreate it.  Require GPGSSH which creates the \"${GNUPGHOME}\" directory.\n\nAdditionally, it makes sense to require GPGSSH in GPGSSH_VERIFYTIME\nbecause the latter builds on the former.  If we can't use GPGSSH,\nthere's little point in checking whether GPGSSH_VERIFYTIME is usable.\n\nSuggested-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Todd Zullinger <tmz@pobox.com>\n---\nHi,\n\nJunio C Hamano wrote:\n>> Good idea. Perhaps:\n>>\n>>      test_lazy_prereq GPGSSH_VERIFYTIME '\n>>              test_have_prereq GPGSSH &&\n>>\n>> is best there?  The GPGSSH prereq creates ${GNUPGHOME}.  It\n>> may not be common, but there may be folks who want to run\n>> the SSH tests and don't care about GPG.\n> \n> OK.  I'll certainly forget, so hold on to the patch and resend after\n> the dust settles from the release.\n\nAlright.  Here's that patch.  Hopefully it's not too dusty\nwhere you are.  If so, I can re-send later. :)\n\nCheers,\n\nTodd\n\n t/lib-gpg.sh | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/t/lib-gpg.sh b/t/lib-gpg.sh\nindex 114785586a..db63aeb6ed 100644\n--- a/t/lib-gpg.sh\n+++ b/t/lib-gpg.sh\n@@ -135,6 +135,7 @@ test_lazy_prereq GPGSSH '\n '\n \n test_lazy_prereq GPGSSH_VERIFYTIME '\n+\ttest_have_prereq GPGSSH &&\n \t# Check if ssh-keygen has a verify-time option by passing an invalid date to it\n \tssh-keygen -Overify-time=INVALID -Y check-novalidate -s doesnotmatter 2>&1 | grep -q -F \"Invalid \\\"verify-time\\\"\" &&\n \n-- \n2.41.0\n"}]}