Re: [PATCH v2 0/6] ssh signing: verify key lifetime

On Wednesday 03 November 2021 at 08:45 pm +0100, Fabian Stelzer wrote:

On 03.11.2021 19:27, Adam Dinwoodie wrote:

quoted

On Wed, 27 Oct 2021 at 09:06, Fabian Stelzer [off-list ref] wrote:

quoted

This series adds key lifetime validity checks by parsing commit/tag
dates from the paylod and passing them to the ssh-keygen operations.

changes since v1:
 - struct signature_check is now used to input payload data into
   check_function
 - payload metadata parsing is completely internal to check_signature.
   the caller only need to set the payload type in the sigc struct
 - small nits and readability fixes
 - removed payload_signer parameter. since we now use the struct we can extend
   this later.

As part of testing v2.34-rc0 on Cygwin, I've found this patch series
is breaking t4202, t5534, and t6200.

Specifically, bisecting points to f265f2d630 (ssh signing: tests for
logs, tags & push certs, 2021-09-10) as breaking t4202 and t5534,
while responsibility for t6200 seems to be 9d12546de9 (ssh signing:
fmt-merge-msg tests & config parse, 2021-10-12).

I've not yet done any investigation into the specifics of these
failures, but I wanted to report them early so other folks could get
on with investigating as appropriate.

What exactly are you testing?

I was initially running the tests using the Cygwin packaging
paraphernalia, as I want to check the full build infrastructure is
working as expected.  When this failed, I reproduced the problems on a
clean checkout of the main Git repository.  In particular, I identified
the failing commits with the following bisect command (subject to
obvious modifications to select the correct script):

     git bisect start v2.34.0-rc0 v2.33.1 && git bisect run bash -c 'make -j 20 || exit 125; cd t || exit 125; ./t4202-*.sh'

This patch series is not patch of the rc
and will not be in v2.34. I even have a small reroll of this for after
the release.

That's odd. `git tag --contains 9d12546de9`, with a fresh pull of the
Git repo, lists v2.34.0-rc0 and v2.34.0-rc1.  It sounds like that's not
expected?

I assume the breaking tests are the new ones added in these commits.

For t4202-log.sh, the failing tests are "72 - setup sshkey signed
branch" and "75 - log ssh key fingerprint".

For t5534-push-signed.sh, the failing tests are "8 - ssh signed push
sends push certificate" and "12 - fail without key and heed
user.signingkey ssh".

For t6200-fmt-merge-msg.sh, the failing tests are "3 - created ssh
signed commit and tag", "7 - message for merging local tag signed by
good ssh key" and "8 - message for merging local tag signed by unknown
ssh key".

What openssh version are you using? (ssh -V)

    OpenSSH_8.8p1, OpenSSL 1.1.1l  24 Aug 2021

Either the feature detecion for the lifetime checks is not
working in cygwin or maybe you have the broken openssh 8.7 version.