From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Fri, 22 Oct 2021 18:12:11 GMT
Subject: Re: [PATCH 6/6] ssh signing: fmt-merge-msg/check_signature with tag date
Message-ID: <211022.86a6j0kjoi.gmgdl@evledraar.gmail.com>
In-Reply-To: <20211022150949.1754477-7-fs@gigacodes.de>


On Fri, Oct 22 2021, Fabian Stelzer wrote:

>  			buf = payload.buf;
>  			len = payload.len;
> -			if (check_signature(payload.buf, payload.len, 0, NULL,
> +
> +			if (parse_signed_buffer_metadata(payload.buf, "tagger",
> +							 &payload_timestamp,
> +							 &payload_signer))
> +				strbuf_addstr(&sig,
> +					_("failed to parse timestamp and signer info from payload"));
> +
> +			if (check_signature(payload.buf, payload.len,
> +					    payload_timestamp, &payload_signer,
>  					    sig.buf, sig.len, &sigc) &&
>  			    !sigc.output)
>  				strbuf_addstr(&sig, "gpg verification failed.\n");

I haven't tested this, but your addition to &sig here lacks a \n,
compared to the &sig seen right above here in the diff context.

Isn't one or the other either missing a \n, or shouldn't have one?
*Looks ath the surrounding code*, yeah if I'm not wrong it's the \n in
the new code here that's missing.

The whole business of seemingly mixing error messages and a signature
payload in the same variable seems a bit odd, but maybe I'm misreading
it. In any case it seems to pre-date this series...