Re: [PATCH v2 00/41] RPCSEC GSS krb5 enhancements

Chuck Lever III
2023-02-23T19:33:10Z

> On Feb 23, 2023, at 11:52 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Chuck,
>
> On Thu, Feb 23, 2023 at 5:19 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>> On Feb 23, 2023, at 10:16 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>> On Thu, Feb 23, 2023 at 3:00 PM Chuck Lever III <chuck.lever@oracle.com> wrote:
>>>>> On Feb 23, 2023, at 8:05 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>> On Sun, 15 Jan 2023, Chuck Lever wrote:
>>>>>> The purpose of this series is to improve/harden the security
>>>>>> provided by the Linux kernel's RPCSEC GSS Kerberos 5 mechanism.
>>>>>> There are lots of clean-ups in this series, but the pertinent
>>>>>> feature is the addition of a clean deprecation path for the DES-
>>>>>> and SHA1-based encryption types in accordance with Internet BCPs.
>>>>>>
>>>>>> This series disables DES-based enctypes by default, provides a
>>>>>> mechanism for disabling SHA1-based enctypes, and introduces two
>>>>>> modern enctypes that do not use deprecated crypto algorithms.
>>>>>>
>>>>>> Not only does that improve security for Kerberos 5 users, but it
>>>>>> also prepares SunRPC for eventually switching to a shared common
>>>>>> kernel Kerberos 5 implementation, which surely will not implement
>>>>>> any deprecated encryption types (in particular, DES-based ones).
>>>>>>
>>>>>> Today, MIT supports both of the newly-introduced enctypes, but
>>>>>> Heimdal does not appear to. Thus distributions can enable and
>>>>>> disable kernel enctype support to match the set of enctypes
>>>>>> supported in their user space Kerberos libraries.
>>>>>>
>>>>>> Scott has been kicking the tires -- we've found no regressions with
>>>>>> the current SHA1-based enctypes, while the new ones are disabled by
>>>>>> default until we have an opportunity for interop testing. The KUnit
>>>>>> tests for the new enctypes pass and this implementation successfully
>>>>>> interoperates with itself using these enctypes. Therefore I believe
>>>>>> it to be safe to merge.
>>>>>>
>>>>>> When this series gets merged, the Linux NFS community should select
>>>>>> and announce a date-certain for removal of SunRPC's DES-based
>>>>>> enctype code.
>>>>>
>>>>> As this is now upstream, I gave it a try on m68k (on the ARAnyM
>>>>> emulator), using a config based on atari_defconfig:
>>>>>
>>>>> KTAP version 1
>>>>> # Subtest: RFC 3961 tests
>>>>> 1..3
>>>>> KTAP version 1
>>>>> # Subtest: RFC 3961 n-fold
>>>>> ok 1 64-fold("012345")
>>>>> ok 2 56-fold("password")
>>>>> ok 3 64-fold("Rough Consensus, and Running Code")
>>>>> ok 4 168-fold("password")
>>>>> ok 5 192-fold("MASSACHVSETTS INSTITVTE OF TECHNOLOGY")
>>>>> ok 6 168-fold("Q")
>>>>> ok 7 168-fold("ba")
>>>>> ok 8 64-fold("kerberos")
>>>>> ok 9 128-fold("kerberos")
>>>>> ok 10 168-fold("kerberos")
>>>>> ok 11 256-fold("kerberos")
>>>>> # RFC 3961 n-fold: pass:11 fail:0 skip:0 total:11
>>>>> ok 1 RFC 3961 n-fold
>>>>> KTAP version 1
>>>>> # Subtest: RFC 3961 key derivation
>>>>> # RFC 3961 key derivation: ASSERTION FAILED at net/sunrpc/auth_gss/gss_krb5_test.c:52
>>>>> Expected gk5e != ((void *)0), but
>>>>> gk5e == 00000000
>>>>> ((void *)0) == 00000000
>>>>> not ok 1 des3-hmac-sha1 key derivation case 1
>>>>
>>>> Geert, thanks for testing GSS on m68k.
>>>>
>>>> This assertion failure means that support for the encryption types
>>>> specified in RFC 3961 is not built into your kernel.
>>>>
>>>> The new Kunit tests don't work unless everything is built in --
>>>>
>>>> there's a net/sunrpc/.kunitconfig that provides the supported
>>>> build configuration for running them. I typically use a command
>>>> line similar to this:
>>>>
>>>> ./tools/testing/kunit/kunit.py run --raw_output=all --kunitconfig ./net/sunrpc/.kunitconfig
>>>
>>> Aren't modular crypto algorithms auto-loaded when needed?
>>
>> The ciphers and digests are handled via the kernel's crypto
>> manager. They will indeed get auto-loaded by SunRPC's GSS on
>> demand, but of course, the set of algorithms used by GSS
>> has to be enabled by Kconfig options first.
>>
>> SunRPC GSS has a set of Kerberos encryption types that make
>> use of individual ciphers and digests. Those have never been
>> modularized, and they are each enabled by Kconfig options,
>> as explained below.
>>
>>
>>> In general, it's a good idea to make the tests test only functionality
>>> that is available, either through "depends on" in Kconfig, or "#if
>>> IS_ENABLED(...)".
>>
>> An earlier version of this patch set did just that. It became
>> quite a mess. That's why I chose the .kunitconfig approach.
>>
>>
>>> Of course that does not preclude providing a
>>> .kunitconfig to enable and test everything.
>>
>> The suite should test every Kerberos encryption type that
>> SunRPC GSS has support for. There's no reason to disable a
>> particular encryption type when running the unit tests...
>> unless I'm missing something?
>
> That depends: do you want to test everything, or do you want to test
> (only) the functionality you enabled for your product?

Each Kunit test is supposed to test one thing in particular.
These tests each test a feature of the GSS Kerberos
encryption type implementation.

So... they are really not for the purpose of configuration or
integration testing. I didn't have distributors in mind for
running these tests. They are instead targeted to kernel
developers and platform maintainers.

> I tend to enable all modular tests, so I can use insmod to run any
> relevant test when needed. If a test suddenly needs something that
> is not enabled, you can not run the test without enabling extra
> functionality (which you may not want to enable).

I wouldn't expect these tests to be run as part of a product
CI suite. They are most valuable when making changes to the
SunRPC GSS code, or crypto code that GSS depends on.

"Did I just break this code?"

>>> Note that net/sunrpc/.kunitconfig has
>>>
>>> CONFIG_RPCSEC_GSS_KRB5_KUNIT_TEST=y
>>>
>>> which needs KUNIT_ALL_TESTS=y, else it will still be modular.
>>>
>>> First, I tried getting my modular setup working.
>>> After enabling:
>>> CONFIG_RPCSEC_GSS_KRB5_ENCTYPES_DES=y
>>
>> And CONFIG_RPCSEC_GSS_KRB5_ENCTYPES_AES_SHA1=y ??
>
> Sure, I had that enabled, thanks to "default y".
>
>>> Third, with net/sunrpc/.kunitconfig, and
>>> CONFIG_RPCSEC_GSS_KRB5_KUNIT_TEST=y:
>
> [...]
>
>>> Unable to handle kernel access at virtual address af06da84
>
>>> I.e. a slightly different crash.
>>> As the difference between the two crashes is modular vs. builtin,
>>> this looks like an out-of-bound access in the test.
>>
>> Why not run the test suite just as I suggested?
>
> I don't think I can use tools/testing/kunit/kunit.py to run the tests
> when cross-compiling my kernel?

You should be able to... that tool runs under UML and
compiles what it needs to run the tests.

> My third case (adding options from net/sunrpc/.kunitconfig, and
> setting CONFIG_RPCSEC_GSS_KRB5_KUNIT_TEST=y) should be equivalent to
> that, right?
>
>> Since I cannot reproduce this crash and do not have an m68k
>> platform available to me, I will need you to continue to
>> pursue the issue. I'll help as much as I can.
>>
>> I would very much like to see successful test results on
>> non-x86 platforms.
>
> Thanks, I'll give it a try on some other platforms, later...

--
Chuck Lever