linux-kernel - Re: [PATCH linux-kselftest/test v2] apparmor: add AppArmor KUnit tests for policy unpack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 6 Nov 2019 09:18:27 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Brendan Higgins <brendanhiggins@...gle.com>
Cc:     shuah@...nel.org, john.johansen@...onical.com, jmorris@...ei.org,
        serge@...lyn.com, alan.maguire@...cle.com, yzaikin@...gle.com,
        davidgow@...gle.com, mcgrof@...nel.org, tytso@....edu,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org, kunit-dev@...glegroups.com,
        linux-kselftest@...r.kernel.org,
        Mike Salvatore <mike.salvatore@...onical.com>
Subject: Re: [PATCH linux-kselftest/test v2] apparmor: add AppArmor KUnit
 tests for policy unpack

On Tue, Nov 05, 2019 at 04:43:29PM -0800, Brendan Higgins wrote:
> From: Mike Salvatore <mike.salvatore@...onical.com>
> 
> Add KUnit tests to test AppArmor unpacking of userspace policies.
> AppArmor uses a serialized binary format for loading policies. To find
> policy format documentation see
> Documentation/admin-guide/LSM/apparmor.rst.
> 
> In order to write the tests against the policy unpacking code, some
> static functions needed to be exposed for testing purposes. One of the
> goals of this patch is to establish a pattern for which testing these
> kinds of functions should be done in the future.
> 
> Signed-off-by: Brendan Higgins <brendanhiggins@...gle.com>
> Signed-off-by: Mike Salvatore <mike.salvatore@...onical.com>
> ---
>  security/apparmor/Kconfig              |  16 +
>  security/apparmor/policy_unpack.c      |   4 +
>  security/apparmor/policy_unpack_test.c | 607 +++++++++++++++++++++++++
>  3 files changed, 627 insertions(+)
>  create mode 100644 security/apparmor/policy_unpack_test.c
> 
> diff --git a/security/apparmor/Kconfig b/security/apparmor/Kconfig
> index d8b1a360a6368..78a33ccac2574 100644
> --- a/security/apparmor/Kconfig
> +++ b/security/apparmor/Kconfig
> @@ -66,3 +66,19 @@ config SECURITY_APPARMOR_DEBUG_MESSAGES
>  	  Set the default value of the apparmor.debug kernel parameter.
>  	  When enabled, various debug messages will be logged to
>  	  the kernel message buffer.
> +
> +config SECURITY_APPARMOR_KUNIT_TEST
> +	bool "Build KUnit tests for policy_unpack.c"
> +	depends on KUNIT && SECURITY_APPARMOR
> +	help
> +	  This builds the AppArmor KUnit tests.
> +
> +	  KUnit tests run during boot and output the results to the debug log
> +	  in TAP format (http://testanything.org/). Only useful for kernel devs
> +	  running KUnit test harness and are not for inclusion into a
> +	  production build.
> +
> +	  For more information on KUnit and unit tests in general please refer
> +	  to the KUnit documentation in Documentation/dev-tools/kunit/.
> +
> +	  If unsure, say N.
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index 8cfc9493eefc7..37c1dd3178fc0 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -1120,3 +1120,7 @@ int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
>  
>  	return error;
>  }
> +
> +#ifdef CONFIG_SECURITY_APPARMOR_KUNIT_TEST
> +#include "policy_unpack_test.c"
> +#endif /* CONFIG_SECURITY_APPARMOR_KUNIT_TEST */

To make this even LESS intrusive, the ifdefs could live in ..._test.c.

Also, while I *think* the kernel build system will correctly track this
dependency, can you double-check that changes to ..._test.c correctly
trigger a recompile of policy_unpack.c?

-- 
Kees Cook