> On Feb 23, 2023, at 11:52 AM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Chuck, > > On Thu, Feb 23, 2023 at 5:19 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: >>> On Feb 23, 2023, at 10:16 AM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: >>> On Thu, Feb 23, 2023 at 3:00 PM Chuck Lever III <chuck.lever@xxxxxxxxxx> wrote: >>>>> On Feb 23, 2023, at 8:05 AM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> 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