Re: [PATCH v2 1/1] t6300: fix match with insecure memory
Kousik Sanagavarapu <five231003@gmail.com> on Tue, 2023/08/22 13:24:
Show 21 quoted lines
> Christian Hesse <list@eworm.de> wrote:
>
> > From: Christian Hesse <mail@eworm.de>
> >
> > Running the tests in a build environment makes gnupg print a warning:
> >
> > gpg: Warning: using insecure memory!
> >
> > This warning breaks the match, as `head` misses one line. Let's strip
> > the line, make `head` return what is expected and fix the match.
> >
> > Signed-off-by: Christian Hesse <mail@eworm.de>
>
> I think a bit of an explanation about why this warning is showing up in the
> commit message would be good.
>
> "man gpg" gives me <stripped>
>
> So it seems that this warning will pop up if gpg is writing memory pages to
> disk which is bad because as stated above we don't want these pages written
> to disk which is a security risk.
The Arch Linux packages are built inside a clean container, started via systemd-nspawn. Within the container the system call @memlock is not allowed by default, for security reasons. There's an upstream systemd issue on this topic:
https://github.com/systemd/systemd/issues/9414
Note this is only true at build time. If the packages are installed on the actual system the @memlock system call is available and things work as expected without issues.
Show 39 quoted lines
> > ---
> > t/t6300-for-each-ref.sh | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> > index 5b434ab451..0f9981798e 100755
> > --- a/t/t6300-for-each-ref.sh
> > +++ b/t/t6300-for-each-ref.sh
> > @@ -1764,12 +1764,13 @@ test_expect_success GPGSSH 'setup for signature
> > atom using ssh' '
> > test_expect_success GPG2 'bare signature atom' '
> > git verify-commit first-signed 2>out.raw &&
> > - grep -Ev "checking the trustdb|PGP trust model" out.raw >out &&
> > + grep -Ev "checking the trustdb|PGP trust model|using insecure
> > memory" out.raw >out && head -3 out >expect &&
> > tail -1 out >>expect &&
> > echo >>expect &&
> > git for-each-ref refs/tags/first-signed \
> > - --format="%(signature)" >actual &&
> > + --format="%(signature)" >out.raw &&
> > + grep -Ev "using insecure memory" out.raw >actual &&
> > test_cmp expect actual
> > '
> >
> > --
> > 2.41.0
>
> We skip "checking the trustdb" and "PGP trust model" lines (which are not
> warnings) here because we don't really need those from the output that GPG
> produces here but skipping a warning too seems kind of a question mark.
>
> It also seems that one could use "--no-secmem-warning" to suppress such a
> warning. So a better place to make a change would not be in t/t6300 but in
> t/lib-gpg from where the prereq GPG2 comes from. Although I'm against this,
> because we don't really want to suppress any warnings.
>
> I think it is a good thing this test is breaking because it informs us about
> the security risk. I have Cc'ed people who might have a thought on this. So
> it's better to wait for their response.
Well, after all I just want to change the tests to succeed with our build environment, let's take a detailed look at the issue. All command below are inside the build environment, so including the warning about insecure memory.
The output of `git verify-commit first-signed` is:
---- >8 ---- gpg: Warning: using insecure memory! gpg: Signature made Tue Aug 22 08:46:43 2023 UTC gpg: using DSA key 73D758744BE721698EC54E8713B6F51ECDDE430D gpg: issuer "committer@example.com" gpg: checking the trustdb gpg: marginals needed: 3 completes needed: 1 trust model: pgp gpg: depth: 0 valid: 1 signed: 0 trust: 0-, 0q, 0n, 0m, 0f, 1u gpg: Good signature from "C O Mitter <committer@example.com>" [ultimate] ---- >8 ----
Whereas `git for-each-ref refs/tags/first-signed --format="%(signature)"` gives:
---- >8 ---- gpg: Warning: using insecure memory! gpg: Signature made Tue Aug 22 08:46:43 2023 UTC gpg: using DSA key 73D758744BE721698EC54E8713B6F51ECDDE430D gpg: issuer "committer@example.com" gpg: Good signature from "C O Mitter <committer@example.com>" [ultimate]
---- >8 ----
Running `head -3` on first output causes the warning to be included, but the issuer line to be removed. That is what finally differs between `expect` and `actual`.
Just changing the number of lines brings other issues I guess... As far as I known the output on issuer was added recently with a gnupg release.
So we need a set of commands to bring the output of both command in line, with or without warning on insecure memory.
--
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}
Christian Hesse
