[SRU][J][PATCH v2 0/3] Fix failing net selftests

[Magali Lemes]

On Thu, Aug 24, 2023 at 6:07 AM Stefan Bader <stefan.bader at canonical.com> wrote:
>
> On 23.08.23 22:20, Magali Lemes wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2019868
> > BugLink: https://bugs.launchpad.net/bugs/2019880
> >
> > [Impact]
> > Due to the introduction of net tests that rely on cryptographic
> > functions to work, some test cases from net/tls and net/vrf-xfrm-tests
> > that use non-compliant FIPS algorithms fail when fips=1.
> >
> > [Fix]
> > To fix these failures in FIPS mode, we can, on a case-by-case basis,
> >    1) skip the tests that require non-compliant FIPS algorithms or
> >    2) change the algorithms to FIPS-compliant ones.
> > For net/tls, we skip the test variants that use the ChaCha20-Poly1305
> > algorithm.
> > For net/net:vrf-xfrm-tests, we can simply replace the algorithms that
> > are not FIPS-compliant with compliant ones.
> >
> > [Test Plan]
> > With a fips kernel installed, pass fips=1 as a kernel parameter, run the
> > net/tls and net/vrf-xfrm-tests tests with these patches applied, and
> > check that they are all passing.
> >
> > [Where problems could occur]
> > Regression risk is very low and would hardly affect any user, since the
> > changes only touch the selftests.
> >
> > [Other Info]
> > I'm sending this to be applied on the generic kernel, as Jammy FIPS
> > derivative kernels will easily inherit these changes.
>
> I do not feel quite happy there. The first patch, sure no problem. That
> might even allow tweaking things we know are failing in general.
> For patch #2: has that ever been run on a non-fips kernel? What happens
> if the file in the new constructor does not exist? Which would be any
> non-fips kernel, right?

Yes, I locally ran this test on 5.15.0-79-generic and checked that the
output remains unchanged.
Right, `/proc/sys/crypto/fips_enabled` only exists in FIPS kernels.
For all the other kernels, `fopen("/proc/sys/crypto/fips_enabled",
"r");` should return NULL and `fips_enabled` is kept as 0. So I don't
think this would be a cause of issue to the other kernels.

> With patch #3, that would change the algorithms used for any kernel.
> Apart from the worry that this might cause problems on some hw you do
> not see with fips, this makes results at least not comparable across series.

The new algos should be common enough so as not to cause such
problems. Plus, the tests are no longer comparable in series with
kernel version v6.4 and greater, since these patches landed in
upstream v6.4.

>
> Also having two bug reports mingled in one submission is at least a
> source of confusion (or likely missed in some process).
>
> So I don't want to refuse straight away but there is a bad feeling about
> this in all kernels...

I understand the concern, yet I feel like the regression risk is very
low and would make it a lot easier to keep things synced across FIPS
(and other) kernels.

Kind regards,
Magali.

>
> -Stefan
>
> >
> > Changes in v2:
> > - Target generic kernel.
> > - fcnal-test.sh: dropped as it will be picked from upstream stable.
> > - tls.c: skip tests right at setup if in FIPS mode, this requires commit
> >    372b304c ("selftests/harness: allow tests to be skipped during setup").
> >
> > Magali Lemes (3):
> >    selftests/harness: allow tests to be skipped during setup
> >    selftests: net: tls: check if FIPS mode is enabled
> >    selftests: net: vrf-xfrm-tests: change authentication and encryption
> >      algos
> >
> >   tools/testing/selftests/kselftest_harness.h   |  6 ++--
> >   tools/testing/selftests/net/tls.c             | 21 ++++++++++++
> >   tools/testing/selftests/net/vrf-xfrm-tests.sh | 32 +++++++++----------
> >   3 files changed, 40 insertions(+), 19 deletions(-)
> >
>