2021-02-03 00:52:26

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: [PATCH 2/2] audit: show (grand)parents information of an audit context

From: Phil Zhang <[email protected]>

To ease the root cause analysis of SELinux AVCs, this new feature
traverses task structs to iteratively find all parent processes
starting with the denied process and ending at the kernel. Meanwhile,
it prints out the command lines and subject contexts of those parents.

This provides developers a clear view of how processes were spawned
and where transitions happened, without the need to reproduce the
issue and manually audit interesting events.

Example on bash over ssh:
$ runcon -u system_u -r system_r -t polaris_hm_t ls
...
type=PARENT msg=audit(1610548241.033:255): subj=root:unconfined_r:unconfined_t:s0-s0:c0.c1023 cmdline="-bash"
type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="sshd: root@pts/0"
type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="/tmp/sw/rp/0/0/rp_security/mount/usr/sbin/sshd
type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:init_t:s0 cmdline="/init"
type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:kernel_t:s0
...

Cc: [email protected]
Signed-off-by: Phil Zhang <[email protected]>
Signed-off-by: Daniel Walker <[email protected]>
---
include/uapi/linux/audit.h | 5 ++-
init/Kconfig | 7 +++++
kernel/audit.c | 3 +-
kernel/auditsc.c | 64 ++++++++++++++++++++++++++++++++++++++
4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 7bea44b1c028..8f1a2880b198 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -119,6 +119,7 @@
#define AUDIT_BPF 1334 /* BPF subsystem */
#define AUDIT_EVENT_LISTENER 1335 /* Task joined multicast read socket */
#define AUDIT_UBACKTRACE 1336 /* User land backtrace */
+#define AUDIT_PARENT 1340 /* Process Parent emit event */

#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
@@ -485,7 +486,9 @@ struct audit_features {
#define AUDIT_FEATURE_ONLY_UNSET_LOGINUID 0
#define AUDIT_FEATURE_LOGINUID_IMMUTABLE 1
#define AUDIT_FEATURE_UBACKTRACE_CONTEXT 2
-#define AUDIT_LAST_FEATURE AUDIT_FEATURE_UBACKTRACE_CONTEXT
+#define AUDIT_FEATURE_LIST_PARENTS 3
+#define AUDIT_LAST_FEATURE AUDIT_FEATURE_LIST_PARENTS
+

#define audit_feature_valid(x) ((x) >= 0 && (x) <= AUDIT_LAST_FEATURE)
#define AUDIT_FEATURE_TO_MASK(x) (1 << ((x) & 31)) /* mask for __u32 */
diff --git a/init/Kconfig b/init/Kconfig
index 4327a8afb1f9..2dbc1c2aa833 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -446,6 +446,13 @@ config AUDIT_USER_BACKTRACE_SIZE
depends on AUDIT_USER_BACKTRACE
default 40

+config AUDIT_LIST_PARENTS
+ bool "Displaying parent processes in audit context messages"
+ def_bool n
+ depends on AUDITSYSCALL
+ help
+ Capture contexts and cmdlines of parent processes when auditing syscalls
+
source "kernel/irq/Kconfig"
source "kernel/time/Kconfig"
source "kernel/Kconfig.preempt"
diff --git a/kernel/audit.c b/kernel/audit.c
index 4608cddb4bb9..834adc462f47 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -165,10 +165,11 @@ static struct audit_features af = {.vers = AUDIT_FEATURE_VERSION,
.features = 0,
.lock = 0,};

-static char *audit_feature_names[3] = {
+static char *audit_feature_names[4] = {
"only_unset_loginuid",
"loginuid_immutable",
"ubacktrace_context",
+ "list_parents",
};

/**
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d048b01345b8..c27e9f928bf1 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -96,6 +96,10 @@
/* number of audit rules */
int audit_n_rules;

+/* max length of per audit entry and max number of entries */
+#define MAX_PARENT_AUDIT_LEN 256
+#define MAX_PARENT_ENTRY_CNT 16
+
/* determines whether we collect data for signals sent */
int audit_signals;

@@ -1472,6 +1476,61 @@ static void audit_log_proctitle(void)
audit_log_end(ab);
}

+static void audit_log_parents(void)
+{
+ int res, len, item_cnt;
+ u32 sid;
+ char *buf;
+ char *ctx = NULL;
+ struct audit_context *context = audit_context();
+ struct audit_buffer *ab;
+ struct task_struct *task_iter;
+
+ if (!context || context->dummy)
+ return;
+
+ buf = kmalloc(MAX_PARENT_AUDIT_LEN, GFP_KERNEL);
+ if (!buf)
+ return;
+
+ rcu_read_lock();
+ task_iter = rcu_dereference(current->real_parent);
+ for (item_cnt = 0; item_cnt < MAX_PARENT_ENTRY_CNT; ++item_cnt) {
+ ab = audit_log_start(context, GFP_ATOMIC, AUDIT_PARENT);
+ if (!ab)
+ break;
+
+ // get subject context
+ security_task_getsecid(task_iter, &sid);
+ if (sid) {
+ res = security_secid_to_secctx(sid, &ctx, &len);
+ if (!res) {
+ audit_log_format(ab, "subj=%-60s", ctx);
+ security_release_secctx(ctx, len);
+ }
+ }
+
+ // get cmdline
+ res = get_cmdline(task_iter, buf, MAX_PARENT_AUDIT_LEN);
+ if (res) {
+ res = audit_proctitle_rtrim(buf, res);
+ if (res) {
+ audit_log_format(ab, " proctitle=");
+ audit_log_n_untrustedstring(ab, buf, res);
+ }
+ }
+
+ audit_log_end(ab);
+
+ if (task_iter == task_iter->real_parent)
+ break;
+ task_iter = rcu_dereference(task_iter->real_parent);
+ }
+ rcu_read_unlock();
+
+ kfree(buf);
+}
+
#ifdef CONFIG_AUDIT_USER_BACKTRACE
static void audit_log_print_backtrace(struct audit_buffer *ab,
struct task_struct *tsk,
@@ -1682,6 +1741,11 @@ static void audit_log_exit(void)

audit_log_proctitle();

+#ifdef CONFIG_AUDIT_LIST_PARENTS
+ if (is_audit_feature_set(AUDIT_FEATURE_LIST_PARENTS))
+ audit_log_parents();
+#endif /* CONFIG_AUDIT_LIST_PARENTS */
+
#ifdef CONFIG_AUDIT_USER_BACKTRACE
if (is_audit_feature_set(AUDIT_FEATURE_UBACKTRACE_CONTEXT))
audit_log_ubacktrace(current, context);
--
2.17.1


2021-02-03 00:53:13

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH 2/2] audit: show (grand)parents information of an audit context

On Tue, Feb 2, 2021 at 4:29 PM Daniel Walker <[email protected]> wrote:
> From: Phil Zhang <[email protected]>
>
> To ease the root cause analysis of SELinux AVCs, this new feature
> traverses task structs to iteratively find all parent processes
> starting with the denied process and ending at the kernel. Meanwhile,
> it prints out the command lines and subject contexts of those parents.
>
> This provides developers a clear view of how processes were spawned
> and where transitions happened, without the need to reproduce the
> issue and manually audit interesting events.
>
> Example on bash over ssh:
> $ runcon -u system_u -r system_r -t polaris_hm_t ls
> ...
> type=PARENT msg=audit(1610548241.033:255): subj=root:unconfined_r:unconfined_t:s0-s0:c0.c1023 cmdline="-bash"
> type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="sshd: root@pts/0"
> type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="/tmp/sw/rp/0/0/rp_security/mount/usr/sbin/sshd
> type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:init_t:s0 cmdline="/init"
> type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:kernel_t:s0
> ...
>
> Cc: [email protected]
> Signed-off-by: Phil Zhang <[email protected]>
> Signed-off-by: Daniel Walker <[email protected]>
> ---
> include/uapi/linux/audit.h | 5 ++-
> init/Kconfig | 7 +++++
> kernel/audit.c | 3 +-
> kernel/auditsc.c | 64 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 77 insertions(+), 2 deletions(-)

This is just for development/testing of SELinux policy, right? It
seems like this is better done in userspace to me through a
combination of policy analysis and just understanding of how your
system is put together.

If you really need this information in the audit log for some
production use, it seems like you could audit the various
fork()/exec() syscalls to get an understanding of the various process
(sub)trees on the system. It would require a bit of work to sift
through the audit log and reconstruct the events that led to a process
being started, and generating the AVC you are interested in debugging,
but folks who live The Audit Life supposedly do this sort of thing a
lot (this sort of thing being tracing a process/session).

--
paul moore
http://www.paul-moore.com

2021-02-03 06:29:07

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] audit: show (grand)parents information of an audit context

Hi Daniel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on pcmoore-audit/next]
[also build test WARNING on linus/master v5.11-rc6 next-20210125]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Daniel-Walker/audit-show-user-land-backtrace-as-part-of-audit-context-messages/20210203-112432
base: https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git next
config: i386-randconfig-s002-20210203 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-215-g0fb77bb6-dirty
# https://github.com/0day-ci/linux/commit/75f9146bb5f0b81f6e398a1e7d509df9b2dd990a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Daniel-Walker/audit-show-user-land-backtrace-as-part-of-audit-context-messages/20210203-112432
git checkout 75f9146bb5f0b81f6e398a1e7d509df9b2dd990a
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>


"sparse warnings: (new ones prefixed by >>)"
>> kernel/auditsc.c:1525:31: sparse: sparse: incompatible types in comparison expression (different address spaces):
>> kernel/auditsc.c:1525:31: sparse: struct task_struct *
>> kernel/auditsc.c:1525:31: sparse: struct task_struct [noderef] __rcu *

vim +1525 kernel/auditsc.c

1478
1479 static void audit_log_parents(void)
1480 {
1481 int res, len, item_cnt;
1482 u32 sid;
1483 char *buf;
1484 char *ctx = NULL;
1485 struct audit_context *context = audit_context();
1486 struct audit_buffer *ab;
1487 struct task_struct *task_iter;
1488
1489 if (!context || context->dummy)
1490 return;
1491
1492 buf = kmalloc(MAX_PARENT_AUDIT_LEN, GFP_KERNEL);
1493 if (!buf)
1494 return;
1495
1496 rcu_read_lock();
1497 task_iter = rcu_dereference(current->real_parent);
1498 for (item_cnt = 0; item_cnt < MAX_PARENT_ENTRY_CNT; ++item_cnt) {
1499 ab = audit_log_start(context, GFP_ATOMIC, AUDIT_PARENT);
1500 if (!ab)
1501 break;
1502
1503 // get subject context
1504 security_task_getsecid(task_iter, &sid);
1505 if (sid) {
1506 res = security_secid_to_secctx(sid, &ctx, &len);
1507 if (!res) {
1508 audit_log_format(ab, "subj=%-60s", ctx);
1509 security_release_secctx(ctx, len);
1510 }
1511 }
1512
1513 // get cmdline
1514 res = get_cmdline(task_iter, buf, MAX_PARENT_AUDIT_LEN);
1515 if (res) {
1516 res = audit_proctitle_rtrim(buf, res);
1517 if (res) {
1518 audit_log_format(ab, " proctitle=");
1519 audit_log_n_untrustedstring(ab, buf, res);
1520 }
1521 }
1522
1523 audit_log_end(ab);
1524
> 1525 if (task_iter == task_iter->real_parent)
1526 break;
1527 task_iter = rcu_dereference(task_iter->real_parent);
1528 }
1529 rcu_read_unlock();
1530
1531 kfree(buf);
1532 }
1533

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (3.44 kB)
.config.gz (31.71 kB)
Download all attachments

2021-02-03 18:59:22

by Daniel Walker (danielwa)

[permalink] [raw]
Subject: Re: [PATCH 2/2] audit: show (grand)parents information of an audit context

On Tue, Feb 02, 2021 at 04:44:47PM -0500, Paul Moore wrote:
> On Tue, Feb 2, 2021 at 4:29 PM Daniel Walker <[email protected]> wrote:
> > From: Phil Zhang <[email protected]>
> >
> > To ease the root cause analysis of SELinux AVCs, this new feature
> > traverses task structs to iteratively find all parent processes
> > starting with the denied process and ending at the kernel. Meanwhile,
> > it prints out the command lines and subject contexts of those parents.
> >
> > This provides developers a clear view of how processes were spawned
> > and where transitions happened, without the need to reproduce the
> > issue and manually audit interesting events.
> >
> > Example on bash over ssh:
> > $ runcon -u system_u -r system_r -t polaris_hm_t ls
> > ...
> > type=PARENT msg=audit(1610548241.033:255): subj=root:unconfined_r:unconfined_t:s0-s0:c0.c1023 cmdline="-bash"
> > type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="sshd: root@pts/0"
> > type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:sshd_t:s0-s0:c0.c1023 cmdline="/tmp/sw/rp/0/0/rp_security/mount/usr/sbin/sshd
> > type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:init_t:s0 cmdline="/init"
> > type=PARENT msg=audit(1610548241.033:255): subj=system_u:system_r:kernel_t:s0
> > ...
> >
> > Cc: [email protected]
> > Signed-off-by: Phil Zhang <[email protected]>
> > Signed-off-by: Daniel Walker <[email protected]>
> > ---
> > include/uapi/linux/audit.h | 5 ++-
> > init/Kconfig | 7 +++++
> > kernel/audit.c | 3 +-
> > kernel/auditsc.c | 64 ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 77 insertions(+), 2 deletions(-)
>
> This is just for development/testing of SELinux policy, right? It
> seems like this is better done in userspace to me through a
> combination of policy analysis and just understanding of how your
> system is put together.

That's why the patch was create to better understand the system.

> If you really need this information in the audit log for some
> production use, it seems like you could audit the various
> fork()/exec() syscalls to get an understanding of the various process
> (sub)trees on the system. It would require a bit of work to sift
> through the audit log and reconstruct the events that led to a process
> being started, and generating the AVC you are interested in debugging,
> but folks who live The Audit Life supposedly do this sort of thing a
> lot (this sort of thing being tracing a process/session).

We have a very complex and constantly changing system, and we use shell scripts
some of the time. If a shell script triggers an AVC it will typically show the
tool called instead of the shell script which triggered calling the tool.

We do have audit enabled in production systems, and I think we collect these
logs in case of issues on the production system. Phil is better to address this.

We're willing to try alternatives like what you suggested above.

Daniel

2021-02-03 19:13:42

by Phil Zhang (xuanyzha)

[permalink] [raw]
Subject: Re: [PATCH 2/2] audit: show (grand)parents information of an audit context

On top of what Daniel just said:

As there are many components being tested in regression runs, it's
expensive to look back and reproduce the AVCs. And there's no prior
knowledge of what processes may generate AVCs. The alternative would be
to audit all fork/exec, but this could easily blow up the audit log.

But we'd like to hear alternatives.

On Wed, 2021-02-03 at 18:57 +0000, Daniel Walker (danielwa) wrote:
> On Tue, Feb 02, 2021 at 04:44:47PM -0500, Paul Moore wrote:
> > On Tue, Feb 2, 2021 at 4:29 PM Daniel Walker <
> > [email protected]
> > > wrote:
> > > From: Phil Zhang <
> > > [email protected]
> > > >
> > >
> > > To ease the root cause analysis of SELinux AVCs, this new feature
> > > traverses task structs to iteratively find all parent processes
> > > starting with the denied process and ending at the kernel.
> > > Meanwhile,
> > > it prints out the command lines and subject contexts of those
> > > parents.
> > >
> > > This provides developers a clear view of how processes were
> > > spawned
> > > and where transitions happened, without the need to reproduce the
> > > issue and manually audit interesting events.
> > >
> > > Example on bash over ssh:
> > > $ runcon -u system_u -r system_r -t polaris_hm_t ls
> > > ...
> > > type=PARENT msg=audit(1610548241.033:255):
> > > subj=root:unconfined_r:unconfined_t:s0-s0:c0.c1023 cmdline="-
> > > bash"
> > > type=PARENT msg=audit(1610548241.033:255):
> > > subj=system_u:system_r:sshd_t:s0-
> > > s0:c0.c1023 cmdline="sshd: root@pts/0"
> > > type=PARENT msg=audit(1610548241.033:255):
> > > subj=system_u:system_r:sshd_t:s0-
> > > s0:c0.c1023 cmdline="/tmp/sw/rp/0/0/rp_security/mount/usr/
> > > sbin/sshd
> > > type=PARENT msg=audit(1610548241.033:255):
> > > subj=system_u:system_r:init_t:s0 cmdline="/ini
> > > t"
> > > type=PARENT msg=audit(1610548241.033:255):
> > > subj=system_u:system_r:kernel_t:s0
> > > ...
> > >
> > > Cc:
> > > [email protected]
> > >
> > > Signed-off-by: Phil Zhang <
> > > [email protected]
> > > >
> > > Signed-off-by: Daniel Walker <
> > > [email protected]
> > > >
> > > ---
> > > include/uapi/linux/audit.h | 5 ++-
> > > init/Kconfig | 7 +++++
> > > kernel/audit.c | 3 +-
> > > kernel/auditsc.c | 64
> > > ++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 77 insertions(+), 2 deletions(-)
> >
> > This is just for development/testing of SELinux policy, right? It
> > seems like this is better done in userspace to me through a
> > combination of policy analysis and just understanding of how your
> > system is put together.
>
>
> That's why the patch was create to better understand the system.
>
> > If you really need this information in the audit log for some
> > production use, it seems like you could audit the various
> > fork()/exec() syscalls to get an understanding of the various
> > process
> > (sub)trees on the system. It would require a bit of work to sift
> > through the audit log and reconstruct the events that led to a
> > process
> > being started, and generating the AVC you are interested in
> > debugging,
> > but folks who live The Audit Life supposedly do this sort of thing
> > a
> > lot (this sort of thing being tracing a process/session).
>
> We have a very complex and constantly changing system, and we use
> shell scripts
> some of the time. If a shell script triggers an AVC it will typically
> show the
> tool called instead of the shell script which triggered calling the
> tool.
>
> We do have audit enabled in production systems, and I think we
> collect these
> logs in case of issues on the production system. Phil is better to
> address this.
>
> We're willing to try alternatives like what you suggested above.
>
> Daniel
>