From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.6 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FSL_HELO_FAKE,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1,USER_IN_DEF_DKIM_WL autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8CEB5C5DF60 for ; Thu, 7 Nov 2019 23:33:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2AC492178F for ; Thu, 7 Nov 2019 23:33:48 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="VCSF0q3Y" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726281AbfKGXdo (ORCPT ); Thu, 7 Nov 2019 18:33:44 -0500 Received: from mail-pf1-f193.google.com ([209.85.210.193]:35257 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725906AbfKGXdo (ORCPT ); Thu, 7 Nov 2019 18:33:44 -0500 Received: by mail-pf1-f193.google.com with SMTP id d13so3514731pfq.2 for ; Thu, 07 Nov 2019 15:33:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=U9LCQzLECFkJez50E4J6R+X6khDWEPIZnHrW73R9i5E=; b=VCSF0q3YAX7JfT36ea1mW70HVF+YkyVrrVNTW2SZ4JLxyAeawqVF5Rdh/9rH2i6SAG FoQiW4BL2Au/RpCOeuFpBTfOXv4RQtMPCBLk6e6QZxC/urZTkrbpLiC+YEq9NmzAG5cv F+3JZZa34J+jp5deo2+K4zBb9HUa7wRMlJQsvk8KX9STNPJBMoHbGn/Q46TMgSZ3qeLo ngTvyzyK3mDIAm6PbpO9h1Beh1EI6Xb9/jJ3eovQN+C9cXnxaZQ1izArK9tovZRpWLOD tLEtBET4eAM8hFBHeMHotaYpbI6I69R5joMhIlXZrcEQGsztakOvy2J+1ogM7Gq4/lfG d6oA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=U9LCQzLECFkJez50E4J6R+X6khDWEPIZnHrW73R9i5E=; b=R9OX1CP8qMU7KmLx6FMFD7Pror/Hu9GeMhH/Yc5X2Dl5D+1lUgp8xisn3dK8i8J77W k9iwLzsnI9WfJ/zhzpcChxKFlBkZqopVIG21vzKiDGJvzb6LsGp0BmyHbl/w4z+QscmX 2vkcbPciD2pMQ382PK/IW3DojuIPe7dTuv9TE1gRCykAgTVOdOX4koRrwR56rH/Aq41b B9y93f+CHl85k3zomtgRUekrB5DP66wYm2loB+yh3riX9oWmwhbqhqIthcX23qunCn0/ UFIlo7Vql3JrVmRE/6cRPUbRDhAEP/gEBbHUC6c2WU8N/BmEOO79zpS1GPmphZfT1rsR rP2Q== X-Gm-Message-State: APjAAAWF9pcfULd+B4Qd37HSiixZBVuYfE3jOg/VJ7K77BpITy4lgzHC qwsYPrzBbKNYBHL5vH6Q4BPLQw== X-Google-Smtp-Source: APXvYqxZsw+8wS5vW5TZg88AZWSvd3uOCaJzI81XZmIohq0NEHnExqON51fuwGRb5S8Qgc5fO9qnzg== X-Received: by 2002:aa7:9aa9:: with SMTP id x9mr7834759pfi.207.1573169622713; Thu, 07 Nov 2019 15:33:42 -0800 (PST) Received: from google.com ([2620:15c:2cb:1:e90c:8e54:c2b4:29e7]) by smtp.gmail.com with ESMTPSA id u3sm3438966pgp.51.2019.11.07.15.33.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Nov 2019 15:33:41 -0800 (PST) Date: Thu, 7 Nov 2019 15:33:37 -0800 From: Brendan Higgins To: Kees Cook Cc: shuah@kernel.org, john.johansen@canonical.com, jmorris@namei.org, serge@hallyn.com, alan.maguire@oracle.com, yzaikin@google.com, davidgow@google.com, mcgrof@kernel.org, tytso@mit.edu, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, Mike Salvatore Subject: Re: [PATCH linux-kselftest/test v2] apparmor: add AppArmor KUnit tests for policy unpack Message-ID: <20191107233337.GA191231@google.com> References: <20191106004329.16991-1-brendanhiggins@google.com> <201911060916.AC9E14B@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201911060916.AC9E14B@keescook> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: On Wed, Nov 06, 2019 at 09:18:27AM -0800, Kees Cook wrote: > On Tue, Nov 05, 2019 at 04:43:29PM -0800, Brendan Higgins wrote: > > From: Mike Salvatore > > > > 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 > > Signed-off-by: Mike Salvatore > > --- > > 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. Less intrusive, yes, but I think I actually like the ifdef here; it makes it clear from the source that the test is only a part of the build when configured to do so. Nevertheless, I will change it if anyone feels strongly about it. > 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? Yep, just verified, first I ran the tests and everything passed. Then I applied the following diff: diff --git a/security/apparmor/policy_unpack_test.c b/security/apparmor/policy_unpack_test.c index 533137f45361c..e1b0670dbdc27 100644 --- a/security/apparmor/policy_unpack_test.c +++ b/security/apparmor/policy_unpack_test.c @@ -161,7 +161,7 @@ static void policy_unpack_test_unpack_array_with_name(struct kunit *test) array_size = unpack_array(puf->e, name); - KUNIT_EXPECT_EQ(test, array_size, (u16)TEST_ARRAY_SIZE); + KUNIT_EXPECT_EQ(test, array_size + 1, (u16)TEST_ARRAY_SIZE); KUNIT_EXPECT_PTR_EQ(test, puf->e->pos, puf->e->start + TEST_ARRAY_BUF_OFFSET + sizeof(u16) + 1); } and reran the tests (to trigger an incremental build) and the test failed as expected indicating that the dependency is properly tracked. Cheers!