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

From: Adam Dinwoodie <adam@dinwoodie.org>
Date: Nov 4, 2021, 16:31 UTC
Message-ID: <20211104163142.GA1419@dinwoodie.org>
In-Reply-To: <20211103194558.xqcsaljvcoontnal@fs>

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

Show 27 quoted lines

> On 03.11.2021 19:27, Adam Dinwoodie wrote:
> > On Wed, 27 Oct 2021 at 09:06, Fabian Stelzer <fs@gigacodes.de> wrote:
> > > 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.