[ANNOUNCE] Linux Security Summit Europe 2018 - CFP

2018-05-17 Thread Reshetova, Elena
==
   ANNOUNCEMENT AND CALL FOR PARTICIPATION

   LINUX SECURITY SUMMIT EUROPE 2018
 
 25-26 October
   EDINBURGH, UK
==


DESCRIPTION

 The Linux Security Summit (LSS) is a technical forum for collaboration
 between Linux developers, researchers, and end users. Its primary aim is to
 foster community efforts in analyzing and solving Linux security challenges.

 This year, for the first time, the Linux Security Summit is going to
 be also held in Europe (LSS-EU) in order to facilitate broader participation 
in Linux
 Security development. Similar to LSS-North America, LSS-EU provides a unique
 opportunity for to have discussions and networking opportunities with key 
people 
 in  the Linux kernel security community , present your work and ideas and 
 affect the future direction of Linux security.  

 In addition to the refereed presentations, panels and BoF sessions, this year's
 LSS-EU program will have an introduction into various Linux kernel security
 subsystems in order to get participants acquainted with their main concepts
 and goals, as well as outline the areas of future development, where 
contribution
 by the community is welcomed. 


  The program committee currently seeks proposals for:

* Refereed Presentations:
  45 minutes in length.

* Panel Discussion Topics:
  45 minutes in length.

* Short Topics:
  30 minutes in total, including at least 10 minutes discussion.

* BoF Sessions.

  Topic areas include, but are not limited to:

* Kernel self-protection
* Access control
* Cryptography and key management
* Integrity control
* Hardware Security
* Iot and embedded security
* Virtualization and containers
* System-specific system hardening
* Case studies
* Security tools
* Security UX
* Emerging technologies, threats & techniques 

  Proposals should be submitted via:

https://events.linuxfoundation.org/events/linux-security-summit-europe-2018/program/cfp/


DATES

  * CFP Close: July 16, 2018
  * CFP Notifications: July 23, 2018
  * Schedule Announced: Aug 25, 2018
  * Event: October 25-26, 2018


WHO SHOULD ATTEND

  We're seeking a diverse range of attendees, and welcome participation by
  people involved in Linux security development, operations, and research.

  The LSS is a unique global event which provides the opportunity to present
  and discuss your work or research with key Linux security community members
  and maintainers. It?s also useful for those who wish to keep up with the
  latest in Linux security development, and to provide input to the
  development process.


WEB SITE

  https://events.linuxfoundation.org/events/linux-security-summit-europe-2018/


TWITTER

  For event updates and announcements, follow:

https://twitter.com/LinuxSecSummit
  

PROGRAM COMMITTEE

  The program committee for LSS-EU 2018 is:

* Elena Reshetova, Intel
* James Morris, Microsoft
* Serge Hallyn, Cisco
* Paul Moore, Red Hat
* Stephen Smalley, NSA
* John Johansen, Canonical
* Kees Cook, Google
* Casey Schaufler, Intel
* Mimi Zohar, IBM
* David A. Wheeler, Institute for Defense Analyses

  The program committee may be contacted as a group via email:
lss-pc () lists.linuxfoundation.org

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH ghak81 V3 3/3] audit: collect audit task parameters

2018-05-17 Thread kbuild test robot
Hi Richard,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20180516]
[cannot apply to linus/master tip/sched/core v4.17-rc5 v4.17-rc4 v4.17-rc3 
v4.17-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Richard-Guy-Briggs/audit-group-task-params/20180517-090703
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   kernel/fork.c: In function 'copy_process':
>> kernel/fork.c:1739:3: error: 'struct task_struct' has no member named 'audit'
 p->audit = NULL;
  ^~

vim +1739 kernel/fork.c

  1728  
  1729  p->default_timer_slack_ns = current->timer_slack_ns;
  1730  
  1731  task_io_accounting_init(&p->ioac);
  1732  acct_clear_integrals(p);
  1733  
  1734  posix_cpu_timers_init(p);
  1735  
  1736  p->start_time = ktime_get_ns();
  1737  p->real_start_time = ktime_get_boot_ns();
  1738  p->io_context = NULL;
> 1739  p->audit = NULL;
  1740  cgroup_fork(p);
  1741  #ifdef CONFIG_NUMA
  1742  p->mempolicy = mpol_dup(p->mempolicy);
  1743  if (IS_ERR(p->mempolicy)) {
  1744  retval = PTR_ERR(p->mempolicy);
  1745  p->mempolicy = NULL;
  1746  goto bad_fork_cleanup_threadgroup_lock;
  1747  }
  1748  #endif
  1749  #ifdef CONFIG_CPUSETS
  1750  p->cpuset_mem_spread_rotor = NUMA_NO_NODE;
  1751  p->cpuset_slab_spread_rotor = NUMA_NO_NODE;
  1752  seqcount_init(&p->mems_allowed_seq);
  1753  #endif
  1754  #ifdef CONFIG_TRACE_IRQFLAGS
  1755  p->irq_events = 0;
  1756  p->hardirqs_enabled = 0;
  1757  p->hardirq_enable_ip = 0;
  1758  p->hardirq_enable_event = 0;
  1759  p->hardirq_disable_ip = _THIS_IP_;
  1760  p->hardirq_disable_event = 0;
  1761  p->softirqs_enabled = 1;
  1762  p->softirq_enable_ip = _THIS_IP_;
  1763  p->softirq_enable_event = 0;
  1764  p->softirq_disable_ip = 0;
  1765  p->softirq_disable_event = 0;
  1766  p->hardirq_context = 0;
  1767  p->softirq_context = 0;
  1768  #endif
  1769  
  1770  p->pagefault_disabled = 0;
  1771  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit

[PATCH ghak82] audit: Fix wrong task in comparison of session ID

2018-05-17 Thread Ondrej Mosnacek
The audit_filter_rules() function in auditsc.c compared the session ID
with the credentials of the current task, while it should use the
credentials of the task given to audit_filter_rules() as a parameter
(tsk).

GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82

Fixes: 8fae47705685 ("audit: add support for session ID user filter")
Cc: sta...@vger.kernel.org
Signed-off-by: Ondrej Mosnacek 
---
 kernel/auditsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index ec38e4d97c23..6d577a34b16b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk,
result = audit_gid_comparator(cred->fsgid, f->op, 
f->gid);
break;
case AUDIT_SESSIONID:
-   sessionid = audit_get_sessionid(current);
+   sessionid = audit_get_sessionid(tsk);
result = audit_comparator(sessionid, f->op, f->val);
break;
case AUDIT_PERS:
-- 
2.17.0

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[PATCH ghak82] audit: Fix extended comparison of GID/EGID

2018-05-17 Thread Ondrej Mosnacek
The audit_filter_rules() function in auditsc.c used the in_[e]group_p()
functions to check GID/EGID match, but these functions use the current
task's credentials, while the comparison should use the credentials of
the task given to audit_filter_rules() as a parameter (tsk).

Note that we can use group_search(cred->group_info, ...) as a
replacement for both in_group_p and in_egroup_p as these functions only
compare the parameter to cred->fsgid/egid and then call group_search.

In fact, the usage of in_group_p was incorrect also because it compared
to cred->fsgid and not cred->gid.

GitHub issue:
https://github.com/linux-audit/audit-kernel/issues/82

Fixes: 37eebe39c973 ("audit: improve GID/EGID comparation logic")
Cc: sta...@vger.kernel.org
Signed-off-by: Ondrej Mosnacek 
---
 kernel/auditsc.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index cbab0da86d15..ec38e4d97c23 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -490,20 +490,20 @@ static int audit_filter_rules(struct task_struct *tsk,
result = audit_gid_comparator(cred->gid, f->op, f->gid);
if (f->op == Audit_equal) {
if (!result)
-   result = in_group_p(f->gid);
+   result = 
groups_search(cred->group_info, f->gid);
} else if (f->op == Audit_not_equal) {
if (result)
-   result = !in_group_p(f->gid);
+   result = 
!groups_search(cred->group_info, f->gid);
}
break;
case AUDIT_EGID:
result = audit_gid_comparator(cred->egid, f->op, 
f->gid);
if (f->op == Audit_equal) {
if (!result)
-   result = in_egroup_p(f->gid);
+   result = 
groups_search(cred->group_info, f->gid);
} else if (f->op == Audit_not_equal) {
if (result)
-   result = !in_egroup_p(f->gid);
+   result = 
!groups_search(cred->group_info, f->gid);
}
break;
case AUDIT_SGID:
-- 
2.17.0

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH] audit: add containerid support for IMA-audit

2018-05-17 Thread Stefan Berger

On 03/08/2018 06:21 AM, Richard Guy Briggs wrote:

On 2018-03-05 09:24, Mimi Zohar wrote:

On Mon, 2018-03-05 at 08:50 -0500, Richard Guy Briggs wrote:

On 2018-03-05 08:43, Mimi Zohar wrote:

Hi Richard,

This patch has been compiled, but not runtime tested.

Ok, great, thank you.  I assume you are offering this patch to be
included in this patchset?

Yes, thank you.


I'll have a look to see where it fits in the
IMA record.  It might be better if it were an AUDIT_CONTAINER_INFO
auxiliary record, but I'll have a look at the circumstances of the
event.

I had a look at the context of this record to see if adding the contid
field to it made sense.  I think the only records for which the contid
field makes sense are the two newly proposed records, AUDIT_CONTAINER
which introduces the container ID and the and AUDIT_CONTAINER_INFO which
documents the presence of the container ID in a process event (or
process-less network event).  All others should use the auxiliary record
AUDIT_CONTAINER_INFO rather than include the contid field directly
itself.  There are several reasons for this including record length, the
ability to filter unwanted records, the difficulty of changing the order
of or removing fields in the future.

Syscalls get this information automatically if the container ID is set
for a task via the AUDIT_CONTAINER_INFO auxiliary record.  Generally a
syscall event is one that uses the task's audit_context while a
standalone event uses NULL or builds a local audit_context that is
discarded immediately after the local use.

Looking at the two cases of AUDIT_INTEGRITY_RULE record generation, it
appears that they should be split into two distinct audit record types.

The record created in ima_audit_measurement() is a syscall record that
could possibly stand on its own since the subject attributes are
present.  If it remains a syscall auxiliary record it will automatically
have the AUDIT_CONTAINER_INFO record accompany it anyways.  If it is
decided to detach it (which would save cpu/netlink/disk bandwidth but is
not recommended due to not wanting to throw away any other syscall
information or other involved records (PATH, CWD, etc...) then a local
audit_context would be created for the AUDIT_INTEGRITY_RULE and
AUDIT_CONTAINERID_INFO records only and immediately discarded.


What does 'detach it' mean? Does it mean we're not using 
current->audit_context?




The record created in ima_parse_rule() is not currently a syscall record
since it is passed an audit_context of NULL and it has a very different
format that does not include any subject attributes (except subj_*=).
At first glance it appears this one should be a syscall accompanied
auxiliary record.  Either way it should have an AUDIT_CONTAINER_INFO


Do you have an example (pointer) to the format for a 'syscall 
accompanied auxiliary record'?



auxiliary record either by being converted to a syscall auxiliary record
by using current->audit_context rather than NULL when calling
audit_log_start(), or creating a local audit_context and calling


ima_parse_rule() is invoked when setting a policy by writing it into 
/sys/kernel/security/ima/policy. We unfortunately don't have the 
current->audit_context in this case.



audit_log_container_info() then releasing the local context.  This
version of the record has additional concerns covered here:
https://github.com/linux-audit/audit-kernel/issues/52


Following the discussion there and the concern with breaking user space, 
how can we split up the AUDIT_INTEGRITY_RULE that is used in 
ima_audit_measurement() and ima_parse_rule(), without 'breaking user space'?


A message produced by ima_parse_rule() looks like this here:

type=INTEGRITY_RULE msg=audit(1526566213.870:305): action="dont_measure" 
fsmagic="0x9fa0" res=1


in contrast to that an INTEGRITY_PCR record type:

type=INTEGRITY_PCR msg=audit(1526566235.193:334): pid=1615 uid=0 auid=0 
ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 
op="invalid_pcr" cause="open_writers" comm="scp" 
name="/var/log/audit/audit.log" dev="dm-0" ino=1962625 res=1


Should some of the fields from INTEGRITY_PCR also appear in 
INTEGRITY_RULE? If so, which ones? We could probably refactor the 
current  integrity_audit_message() and have ima_parse_rule() call into 
it to get those fields as well. I suppose adding new fields to it 
wouldn't be considered breaking user space?


    Stefan




Can you briefly describe the circumstances under which these two
different identically-numbered records are produced as a first step
towards splitting them into two distict records?

The four AUDIT_INTEGRITY _METADATA, _PCR, _DATA and _STATUS records
appear to be already properly covered for AUDIT_CONTAINER_INFO records
by being syscall auxiliary records.  The AUDIT_INTEGRITY_HASH record
appears to be unused.


Can you suggest a procedure to test it?

Like IMA-measurement and IMA-appraisal, IMA-audit is enabled based on
policy. The example IMA policy, below, includes IMA-a

Re: [PATCH ghak82] audit: Fix extended comparison of GID/EGID

2018-05-17 Thread Richard Guy Briggs
On 2018-05-17 17:31, Ondrej Mosnacek wrote:
> The audit_filter_rules() function in auditsc.c used the in_[e]group_p()
> functions to check GID/EGID match, but these functions use the current
> task's credentials, while the comparison should use the credentials of
> the task given to audit_filter_rules() as a parameter (tsk).
> 
> Note that we can use group_search(cred->group_info, ...) as a
> replacement for both in_group_p and in_egroup_p as these functions only
> compare the parameter to cred->fsgid/egid and then call group_search.
> 
> In fact, the usage of in_group_p was incorrect also because it compared
> to cred->fsgid and not cred->gid.
> 
> GitHub issue:
> https://github.com/linux-audit/audit-kernel/issues/82
> 
> Fixes: 37eebe39c973 ("audit: improve GID/EGID comparation logic")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ondrej Mosnacek 

Nice.  You found a function that let you not have to roll a new global
one.  Is the comparision with cred->fsgid important?

If you run ./scripts/checkpatch.pl on the patch file it will complain on
those lines longer than 80 chars.  I don't have a problem with it.


> ---
>  kernel/auditsc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index cbab0da86d15..ec38e4d97c23 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -490,20 +490,20 @@ static int audit_filter_rules(struct task_struct *tsk,
>   result = audit_gid_comparator(cred->gid, f->op, f->gid);
>   if (f->op == Audit_equal) {
>   if (!result)
> - result = in_group_p(f->gid);
> + result = 
> groups_search(cred->group_info, f->gid);
>   } else if (f->op == Audit_not_equal) {
>   if (result)
> - result = !in_group_p(f->gid);
> + result = 
> !groups_search(cred->group_info, f->gid);
>   }
>   break;
>   case AUDIT_EGID:
>   result = audit_gid_comparator(cred->egid, f->op, 
> f->gid);
>   if (f->op == Audit_equal) {
>   if (!result)
> - result = in_egroup_p(f->gid);
> + result = 
> groups_search(cred->group_info, f->gid);
>   } else if (f->op == Audit_not_equal) {
>   if (result)
> - result = !in_egroup_p(f->gid);
> + result = 
> !groups_search(cred->group_info, f->gid);
>   }
>   break;
>   case AUDIT_SGID:
> -- 
> 2.17.0
> 

- RGB

--
Richard Guy Briggs 
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH ghak82] audit: Fix wrong task in comparison of session ID

2018-05-17 Thread Richard Guy Briggs
On 2018-05-17 17:31, Ondrej Mosnacek wrote:
> The audit_filter_rules() function in auditsc.c compared the session ID
> with the credentials of the current task, while it should use the
> credentials of the task given to audit_filter_rules() as a parameter
> (tsk).
> 
> GitHub issue:
> https://github.com/linux-audit/audit-kernel/issues/82
> 
> Fixes: 8fae47705685 ("audit: add support for session ID user filter")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ondrej Mosnacek 

Reviewed-by: Richard Guy Briggs 

> ---
>  kernel/auditsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index ec38e4d97c23..6d577a34b16b 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -513,7 +513,7 @@ static int audit_filter_rules(struct task_struct *tsk,
>   result = audit_gid_comparator(cred->fsgid, f->op, 
> f->gid);
>   break;
>   case AUDIT_SESSIONID:
> - sessionid = audit_get_sessionid(current);
> + sessionid = audit_get_sessionid(tsk);
>   result = audit_comparator(sessionid, f->op, f->val);
>   break;
>   case AUDIT_PERS:
> -- 
> 2.17.0
> 

- RGB

--
Richard Guy Briggs 
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[PATCH ghak81 V3a] fixup! audit: collect audit task parameters

2018-05-17 Thread Richard Guy Briggs
Enable fork.c compilation with audit disabled.

Signed-off-by: Richard Guy Briggs 
---
Hi Paul, this one got caught by the 0-day kbuildbot.  Can you squash it
down if you haven't merged it yet?
---
 kernel/fork.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/fork.c b/kernel/fork.c
index 92ab849..ff82928 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1713,7 +1713,9 @@ static __latent_entropy struct task_struct *copy_process(
p->start_time = ktime_get_ns();
p->real_start_time = ktime_get_boot_ns();
p->io_context = NULL;
+#ifdef CONFIG_AUDITSYSCALL
p->audit = NULL;
+#endif /* CONFIG_AUDITSYSCALL */
cgroup_fork(p);
 #ifdef CONFIG_NUMA
p->mempolicy = mpol_dup(p->mempolicy);
-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [RFC PATCH ghak32 V2 01/13] audit: add container id

2018-05-17 Thread Steve Grubb
On Fri, 16 Mar 2018 05:00:28 -0400
Richard Guy Briggs  wrote:

> Implement the proc fs write to set the audit container ID of a
> process, emitting an AUDIT_CONTAINER record to document the event.
> 
> This is a write from the container orchestrator task to a proc entry
> of the form /proc/PID/containerid where PID is the process ID of the
> newly created task that is to become the first task in a container,
> or an additional task added to a container.
> 
> The write expects up to a u64 value (unset: 18446744073709551615).
> 
> This will produce a record such as this:
> type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0
> tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455
> res=0

The was one thing I was wondering about. Currently when we set the
loginuid, the record is AUDIT_LOGINUID. The corollary is that when we
set the container id, the event should be AUDIT_CONTAINERID or
AUDIT_CONTAINER_ID.

During syscall events, the path info is returned in a a record simply
called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So, rather than
calling the record that gets attached to everything
AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.


> The "op" field indicates an initial set.  The "pid" to "ses" fields
> are the orchestrator while the "opid" field is the object's PID, the
> process being "contained".  Old and new container ID values are given
> in the "contid" fields, while res indicates its success.
> 
> It is not permitted to self-set, unset or re-set the container ID.  A
> child inherits its parent's container ID, but then can be set only
> once after.
> 
> See: https://github.com/linux-audit/audit-kernel/issues/32
> 
> Signed-off-by: Richard Guy Briggs 
> ---
>  fs/proc/base.c | 37 
>  include/linux/audit.h  | 16 +
>  include/linux/init_task.h  |  4 ++-
>  include/linux/sched.h  |  1 +
>  include/uapi/linux/audit.h |  2 ++
>  kernel/auditsc.c   | 84
> ++ 6 files changed, 143
> insertions(+), 1 deletion(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 60316b5..6ce4fbe 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file
> * file, char __user * buf, .read  = proc_sessionid_read,
>   .llseek = generic_file_llseek,
>  };
> +
> +static ssize_t proc_containerid_write(struct file *file, const char
> __user *buf,
> +size_t count, loff_t *ppos)
> +{
> + struct inode *inode = file_inode(file);
> + u64 containerid;
> + int rv;
> + struct task_struct *task = get_proc_task(inode);
> +
> + if (!task)
> + return -ESRCH;
> + if (*ppos != 0) {
> + /* No partial writes. */
> + put_task_struct(task);
> + return -EINVAL;
> + }
> +
> + rv = kstrtou64_from_user(buf, count, 10, &containerid);
> + if (rv < 0) {
> + put_task_struct(task);
> + return rv;
> + }
> +
> + rv = audit_set_containerid(task, containerid);
> + put_task_struct(task);
> + if (rv < 0)
> + return rv;
> + return count;
> +}
> +
> +static const struct file_operations proc_containerid_operations = {
> + .write  = proc_containerid_write,
> + .llseek = generic_file_llseek,
> +};
> +
>  #endif
>  
>  #ifdef CONFIG_FAULT_INJECTION
> @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file
> *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
>   REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
>   REG("sessionid",  S_IRUGO, proc_sessionid_operations),
> + REG("containerid", S_IWUSR, proc_containerid_operations),
>  #endif
>  #ifdef CONFIG_FAULT_INJECTION
>   REG("make-it-fail", S_IRUGO|S_IWUSR,
> proc_fault_inject_operations), @@ -3355,6 +3391,7 @@ static int
> proc_tid_comm_permission(struct inode *inode, int mask) #ifdef
> CONFIG_AUDITSYSCALL REG("loginuid",  S_IWUSR|S_IRUGO,
> proc_loginuid_operations), REG("sessionid",  S_IRUGO,
> proc_sessionid_operations),
> + REG("containerid", S_IWUSR, proc_containerid_operations),
>  #endif
>  #ifdef CONFIG_FAULT_INJECTION
>   REG("make-it-fail", S_IRUGO|S_IWUSR,
> proc_fault_inject_operations), diff --git a/include/linux/audit.h
> b/include/linux/audit.h index af410d9..fe4ba3f 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -29,6 +29,7 @@
>  
>  #define AUDIT_INO_UNSET ((unsigned long)-1)
>  #define AUDIT_DEV_UNSET ((dev_t)-1)
> +#define INVALID_CID AUDIT_CID_UNSET
>  
>  struct audit_sig_info {
>   uid_t   uid;
> @@ -321,6 +322,7 @@ static inline void audit_ptrace(struct
> task_struct *t) extern int auditsc_get_stamp(struct audit_context
> *ctx, struct timespec64 *t, unsigned int *serial);
>  extern int audit_set_loginuid(k

Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls

2018-05-17 Thread Steve Grubb
On Fri, 16 Mar 2018 05:00:30 -0400
Richard Guy Briggs  wrote:

> Create a new audit record AUDIT_CONTAINER_INFO to document the
> container ID of a process if it is present.

As mentioned in a previous email, I think AUDIT_CONTAINER is more
suitable for the container record. One more comment below...

> Called from audit_log_exit(), syscalls are covered.
> 
> A sample raw event:
> type=SYSCALL msg=audit(1519924845.499:257): arch=c03e syscall=257
> success=yes exit=3 a0=ff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> cap_fp= cap_fi= cap_fe=0 cap_fver=0
> type=PATH msg=audit(1519924845.499:257): item=1
> name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> nametype=CREATE cap_fp= cap_fi=
> cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> contid=123458
> 
> See: https://github.com/linux-audit/audit-kernel/issues/32
> Signed-off-by: Richard Guy Briggs 
> ---
>  include/linux/audit.h  |  5 +
>  include/uapi/linux/audit.h |  1 +
>  kernel/audit.c | 20 
>  kernel/auditsc.c   |  2 ++
>  4 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index fe4ba3f..3acbe9d 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -154,6 +154,8 @@ extern void
> audit_log_link_denied(const char *operation, extern int
> audit_log_task_context(struct audit_buffer *ab); extern void
> audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> +extern int audit_log_container_info(struct task_struct *tsk,
> +  struct audit_context *context);
>  
>  extern int   audit_update_lsm_rules(void);
>  
> @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> audit_buffer *ab) static inline void audit_log_task_info(struct
> audit_buffer *ab, struct task_struct *tsk)
>  { }
> +static inline int audit_log_container_info(struct task_struct *tsk,
> + struct audit_context
> *context); +{ }
>  #define audit_enabled 0
>  #endif /* CONFIG_AUDIT */
>  
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 921a71f..e83ccbd 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -115,6 +115,7 @@
>  #define AUDIT_REPLACE1329/* Replace auditd
> if this packet unanswerd */ #define AUDIT_KERN_MODULE
> 1330  /* Kernel Module events */ #define
> AUDIT_FANOTIFY1331/* Fanotify access decision
> */ +#define AUDIT_CONTAINER_INFO  1332/* Container ID
> information */ #define AUDIT_AVC  1400/* SE
> Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> 1401  /* Internal SE Linux Errors */ diff --git
> a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> sessionid); }
>  
> +/*
> + * audit_log_container_info - report container info
> + * @tsk: task to be recorded
> + * @context: task or local context for record
> + */
> +int audit_log_container_info(struct task_struct *tsk, struct
> audit_context *context) +{
> + struct audit_buffer *ab;
> +
> + if (!audit_containerid_set(tsk))
> + return 0;
> + /* Generate AUDIT_CONTAINER_INFO with container ID */
> + ab = audit_log_start(context, GFP_KERNEL,
> AUDIT_CONTAINER_INFO);
> + if (!ab)
> + return -ENOMEM;
> + audit_log_format(ab, "contid=%llu",
> audit_get_containerid(tsk));
> + audit_log_end(ab);
> + return 0;
> +}
> +
>  void audit_log_key(struct audit_buffer *ab, char *key)
>  {
>   audit_log_format(ab, " key=");
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index a6b0a52..65be110 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> *context, struct task_struct *ts 
>   audit_log_proctitle(tsk, context);
>  
> + audit_log_container_info(tsk, context);

Would there be any problem moving audit_log_container_info before
audit_log_proctitle? There are some assumptions that proctitle is the
last r

Re: [PATCH] audit: add containerid support for IMA-audit

2018-05-17 Thread Richard Guy Briggs
On 2018-05-17 10:18, Stefan Berger wrote:
> On 03/08/2018 06:21 AM, Richard Guy Briggs wrote:
> > On 2018-03-05 09:24, Mimi Zohar wrote:
> > > On Mon, 2018-03-05 at 08:50 -0500, Richard Guy Briggs wrote:
> > > > On 2018-03-05 08:43, Mimi Zohar wrote:
> > > > > Hi Richard,
> > > > > 
> > > > > This patch has been compiled, but not runtime tested.
> > > > Ok, great, thank you.  I assume you are offering this patch to be
> > > > included in this patchset?
> > > Yes, thank you.
> > > 
> > > > I'll have a look to see where it fits in the
> > > > IMA record.  It might be better if it were an AUDIT_CONTAINER_INFO
> > > > auxiliary record, but I'll have a look at the circumstances of the
> > > > event.
> > I had a look at the context of this record to see if adding the contid
> > field to it made sense.  I think the only records for which the contid
> > field makes sense are the two newly proposed records, AUDIT_CONTAINER
> > which introduces the container ID and the and AUDIT_CONTAINER_INFO which
> > documents the presence of the container ID in a process event (or
> > process-less network event).  All others should use the auxiliary record
> > AUDIT_CONTAINER_INFO rather than include the contid field directly
> > itself.  There are several reasons for this including record length, the
> > ability to filter unwanted records, the difficulty of changing the order
> > of or removing fields in the future.
> > 
> > Syscalls get this information automatically if the container ID is set
> > for a task via the AUDIT_CONTAINER_INFO auxiliary record.  Generally a
> > syscall event is one that uses the task's audit_context while a
> > standalone event uses NULL or builds a local audit_context that is
> > discarded immediately after the local use.
> > 
> > Looking at the two cases of AUDIT_INTEGRITY_RULE record generation, it
> > appears that they should be split into two distinct audit record types.
> > 
> > The record created in ima_audit_measurement() is a syscall record that
> > could possibly stand on its own since the subject attributes are
> > present.  If it remains a syscall auxiliary record it will automatically
> > have the AUDIT_CONTAINER_INFO record accompany it anyways.  If it is
> > decided to detach it (which would save cpu/netlink/disk bandwidth but is
> > not recommended due to not wanting to throw away any other syscall
> > information or other involved records (PATH, CWD, etc...) then a local
> > audit_context would be created for the AUDIT_INTEGRITY_RULE and
> > AUDIT_CONTAINERID_INFO records only and immediately discarded.
> 
> What does 'detach it' mean? Does it mean we're not using
> current->audit_context?

Exactly.

> > The record created in ima_parse_rule() is not currently a syscall record
> > since it is passed an audit_context of NULL and it has a very different
> > format that does not include any subject attributes (except subj_*=).
> > At first glance it appears this one should be a syscall accompanied
> > auxiliary record.  Either way it should have an AUDIT_CONTAINER_INFO
> 
> Do you have an example (pointer) to the format for a 'syscall accompanied
> auxiliary record'?

Any that uses current->audit_context (or recently proposed
audit_context() function) will be a syscall auxiliary record.  Well
formed record formats are = and named as listed:


https://github.com/linux-audit/audit-documentation/wiki/SPEC-Writing-Good-Events

https://github.com/linux-audit/audit-documentation/blob/master/specs/fields/field-dictionary.csv

> > auxiliary record either by being converted to a syscall auxiliary record
> > by using current->audit_context rather than NULL when calling
> > audit_log_start(), or creating a local audit_context and calling
> 
> ima_parse_rule() is invoked when setting a policy by writing it into
> /sys/kernel/security/ima/policy. We unfortunately don't have the
> current->audit_context in this case.

Sure you do.  What process writes to that file?  That's the one we care
about, unless it is somehow handed off to a queue and processed later in
a different context.

> > audit_log_container_info() then releasing the local context.  This
> > version of the record has additional concerns covered here:
> > https://github.com/linux-audit/audit-kernel/issues/52
> 
> Following the discussion there and the concern with breaking user space, how
> can we split up the AUDIT_INTEGRITY_RULE that is used in
> ima_audit_measurement() and ima_parse_rule(), without 'breaking user space'?

Arguably userspace is already broken by this wildly diverging pair of
formats for the same record.

> A message produced by ima_parse_rule() looks like this here:
> 
> type=INTEGRITY_RULE msg=audit(1526566213.870:305): action="dont_measure"
> fsmagic="0x9fa0" res=1
> 
> in contrast to that an INTEGRITY_PCR record type:
> 
> type=INTEGRITY_PCR msg=audit(1526566235.193:334): pid=1615 uid=0 auid=0
> ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> op="invalid_pcr" cause="open_wri

Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls

2018-05-17 Thread Richard Guy Briggs
On 2018-05-17 17:09, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:30 -0400
> Richard Guy Briggs  wrote:
> 
> > Create a new audit record AUDIT_CONTAINER_INFO to document the
> > container ID of a process if it is present.
> 
> As mentioned in a previous email, I think AUDIT_CONTAINER is more
> suitable for the container record. One more comment below...
> 
> > Called from audit_log_exit(), syscalls are covered.
> > 
> > A sample raw event:
> > type=SYSCALL msg=audit(1519924845.499:257): arch=c03e syscall=257
> > success=yes exit=3 a0=ff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> > ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> > sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> > cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> > name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> > rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> > cap_fp= cap_fi= cap_fe=0 cap_fver=0
> > type=PATH msg=audit(1519924845.499:257): item=1
> > name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> > ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> > nametype=CREATE cap_fp= cap_fi=
> > cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> > proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> > contid=123458
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> > Signed-off-by: Richard Guy Briggs 
> > ---
> >  include/linux/audit.h  |  5 +
> >  include/uapi/linux/audit.h |  1 +
> >  kernel/audit.c | 20 
> >  kernel/auditsc.c   |  2 ++
> >  4 files changed, 28 insertions(+)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index fe4ba3f..3acbe9d 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -154,6 +154,8 @@ extern void
> > audit_log_link_denied(const char *operation, extern int
> > audit_log_task_context(struct audit_buffer *ab); extern void
> > audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> > +extern int audit_log_container_info(struct task_struct *tsk,
> > +struct audit_context *context);
> >  
> >  extern int audit_update_lsm_rules(void);
> >  
> > @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> > audit_buffer *ab) static inline void audit_log_task_info(struct
> > audit_buffer *ab, struct task_struct *tsk)
> >  { }
> > +static inline int audit_log_container_info(struct task_struct *tsk,
> > +   struct audit_context
> > *context); +{ }
> >  #define audit_enabled 0
> >  #endif /* CONFIG_AUDIT */
> >  
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 921a71f..e83ccbd 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -115,6 +115,7 @@
> >  #define AUDIT_REPLACE  1329/* Replace auditd
> > if this packet unanswerd */ #define AUDIT_KERN_MODULE
> > 1330/* Kernel Module events */ #define
> > AUDIT_FANOTIFY  1331/* Fanotify access decision
> > */ +#define AUDIT_CONTAINER_INFO1332/* Container ID
> > information */ #define AUDIT_AVC1400/* SE
> > Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> > 1401/* Internal SE Linux Errors */ diff --git
> > a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> > audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> > sessionid); }
> >  
> > +/*
> > + * audit_log_container_info - report container info
> > + * @tsk: task to be recorded
> > + * @context: task or local context for record
> > + */
> > +int audit_log_container_info(struct task_struct *tsk, struct
> > audit_context *context) +{
> > +   struct audit_buffer *ab;
> > +
> > +   if (!audit_containerid_set(tsk))
> > +   return 0;
> > +   /* Generate AUDIT_CONTAINER_INFO with container ID */
> > +   ab = audit_log_start(context, GFP_KERNEL,
> > AUDIT_CONTAINER_INFO);
> > +   if (!ab)
> > +   return -ENOMEM;
> > +   audit_log_format(ab, "contid=%llu",
> > audit_get_containerid(tsk));
> > +   audit_log_end(ab);
> > +   return 0;
> > +}
> > +
> >  void audit_log_key(struct audit_buffer *ab, char *key)
> >  {
> > audit_log_format(ab, " key=");
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index a6b0a52..65be110 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> > *context, struct task_struct

Re: [RFC PATCH ghak32 V2 01/13] audit: add container id

2018-05-17 Thread Richard Guy Briggs
On 2018-05-17 17:00, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:28 -0400
> Richard Guy Briggs  wrote:
> 
> > Implement the proc fs write to set the audit container ID of a
> > process, emitting an AUDIT_CONTAINER record to document the event.
> > 
> > This is a write from the container orchestrator task to a proc entry
> > of the form /proc/PID/containerid where PID is the process ID of the
> > newly created task that is to become the first task in a container,
> > or an additional task added to a container.
> > 
> > The write expects up to a u64 value (unset: 18446744073709551615).
> > 
> > This will produce a record such as this:
> > type=CONTAINER msg=audit(1519903238.968:261): op=set pid=596 uid=0
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 auid=0
> > tty=pts0 ses=1 opid=596 old-contid=18446744073709551615 contid=123455
> > res=0
> 
> The was one thing I was wondering about. Currently when we set the
> loginuid, the record is AUDIT_LOGINUID. The corollary is that when we
> set the container id, the event should be AUDIT_CONTAINERID or
> AUDIT_CONTAINER_ID.

The record type is actually AUDIT_LOGIN.  The field type is
AUDIT_LOGINUID.  Given that correction, I think we're fine and could
potentially violently agree.  The existing naming is consistent.

> During syscall events, the path info is returned in a a record simply
> called AUDIT_PATH, cwd info is returned in AUDIT_CWD. So, rather than
> calling the record that gets attached to everything
> AUDIT_CONTAINER_INFO, how about simply AUDIT_CONTAINER.

Considering the container initiation record is different than the record
to document the container involved in an otherwise normal syscall, we
need two names.  I don't have a strong opinion what they are.

I'd prefer AUDIT_CONTAINER and AUDIT_CONTAINER_INFO so that the two are
different enough to be visually distinct while leaving
AUDIT_CONTAINERID for the field type in patch 4 ("audit: add containerid
filtering")

> > The "op" field indicates an initial set.  The "pid" to "ses" fields
> > are the orchestrator while the "opid" field is the object's PID, the
> > process being "contained".  Old and new container ID values are given
> > in the "contid" fields, while res indicates its success.
> > 
> > It is not permitted to self-set, unset or re-set the container ID.  A
> > child inherits its parent's container ID, but then can be set only
> > once after.
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> > 
> > Signed-off-by: Richard Guy Briggs 
> > ---
> >  fs/proc/base.c | 37 
> >  include/linux/audit.h  | 16 +
> >  include/linux/init_task.h  |  4 ++-
> >  include/linux/sched.h  |  1 +
> >  include/uapi/linux/audit.h |  2 ++
> >  kernel/auditsc.c   | 84
> > ++ 6 files changed, 143
> > insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index 60316b5..6ce4fbe 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -1299,6 +1299,41 @@ static ssize_t proc_sessionid_read(struct file
> > * file, char __user * buf, .read= proc_sessionid_read,
> > .llseek = generic_file_llseek,
> >  };
> > +
> > +static ssize_t proc_containerid_write(struct file *file, const char
> > __user *buf,
> > +  size_t count, loff_t *ppos)
> > +{
> > +   struct inode *inode = file_inode(file);
> > +   u64 containerid;
> > +   int rv;
> > +   struct task_struct *task = get_proc_task(inode);
> > +
> > +   if (!task)
> > +   return -ESRCH;
> > +   if (*ppos != 0) {
> > +   /* No partial writes. */
> > +   put_task_struct(task);
> > +   return -EINVAL;
> > +   }
> > +
> > +   rv = kstrtou64_from_user(buf, count, 10, &containerid);
> > +   if (rv < 0) {
> > +   put_task_struct(task);
> > +   return rv;
> > +   }
> > +
> > +   rv = audit_set_containerid(task, containerid);
> > +   put_task_struct(task);
> > +   if (rv < 0)
> > +   return rv;
> > +   return count;
> > +}
> > +
> > +static const struct file_operations proc_containerid_operations = {
> > +   .write  = proc_containerid_write,
> > +   .llseek = generic_file_llseek,
> > +};
> > +
> >  #endif
> >  
> >  #ifdef CONFIG_FAULT_INJECTION
> > @@ -2961,6 +2996,7 @@ static int proc_pid_patch_state(struct seq_file
> > *m, struct pid_namespace *ns, #ifdef CONFIG_AUDITSYSCALL
> > REG("loginuid",   S_IWUSR|S_IRUGO, proc_loginuid_operations),
> > REG("sessionid",  S_IRUGO, proc_sessionid_operations),
> > +   REG("containerid", S_IWUSR, proc_containerid_operations),
> >  #endif
> >  #ifdef CONFIG_FAULT_INJECTION
> > REG("make-it-fail", S_IRUGO|S_IWUSR,
> > proc_fault_inject_operations), @@ -3355,6 +3391,7 @@ static int
> > proc_tid_comm_permission(struct inode *inode, int mask) #ifdef
> > CONFIG_AUDITSYSCALL REG("loginuid",  S_IWUSR|S_IRUGO,
> > proc_loginuid_operations), REG("sessio

Re: [PATCH ghak81 V3 1/3] audit: use new audit_context access funciton for seccomp_actions_logged

2018-05-17 Thread Paul Moore
On Wed, May 16, 2018 at 7:55 AM, Richard Guy Briggs  wrote:
> On the rebase of the following commit on the new seccomp actions_logged
> function, one audit_context access was missed.
>
> commit cdfb6b341f0f2409aba24b84f3b4b2bba50be5c5
> ("audit: use inline function to get audit context")
>
> Signed-off-by: Richard Guy Briggs 
> ---
>  kernel/auditsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Merged into audit/next, thanks for the follow-up.

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index cbab0da..f3d3dc6 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2497,7 +2497,7 @@ void audit_seccomp_actions_logged(const char *names, 
> const char *old_names,
> if (!audit_enabled)
> return;
>
> -   ab = audit_log_start(current->audit_context, GFP_KERNEL,
> +   ab = audit_log_start(audit_context(), GFP_KERNEL,
>  AUDIT_CONFIG_CHANGE);
> if (unlikely(!ab))
> return;
> --
> 1.8.3.1

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH ghak81 V3 2/3] audit: normalize loginuid read access

2018-05-17 Thread Paul Moore
On Wed, May 16, 2018 at 7:55 AM, Richard Guy Briggs  wrote:
> Recognizing that the loginuid is an internal audit value, use an access
> function to retrieve the audit loginuid value for the task rather than
> reaching directly into the task struct to get it.
>
> Signed-off-by: Richard Guy Briggs 
> ---
>  kernel/auditsc.c | 24 +++-
>  1 file changed, 15 insertions(+), 9 deletions(-)

Also merged into audit/next.

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index f3d3dc6..ef3e189 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
> case AUDIT_COMPARE_EGID_TO_OBJ_GID:
> return audit_compare_gid(cred->egid, name, f, ctx);
> case AUDIT_COMPARE_AUID_TO_OBJ_UID:
> -   return audit_compare_uid(tsk->loginuid, name, f, ctx);
> +   return audit_compare_uid(audit_get_loginuid(tsk), name, f, 
> ctx);
> case AUDIT_COMPARE_SUID_TO_OBJ_UID:
> return audit_compare_uid(cred->suid, name, f, ctx);
> case AUDIT_COMPARE_SGID_TO_OBJ_GID:
> @@ -385,7 +385,8 @@ static int audit_field_compare(struct task_struct *tsk,
> return audit_compare_gid(cred->fsgid, name, f, ctx);
> /* uid comparisons */
> case AUDIT_COMPARE_UID_TO_AUID:
> -   return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
> +   return audit_uid_comparator(cred->uid, f->op,
> +   audit_get_loginuid(tsk));
> case AUDIT_COMPARE_UID_TO_EUID:
> return audit_uid_comparator(cred->uid, f->op, cred->euid);
> case AUDIT_COMPARE_UID_TO_SUID:
> @@ -394,11 +395,14 @@ static int audit_field_compare(struct task_struct *tsk,
> return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
> /* auid comparisons */
> case AUDIT_COMPARE_AUID_TO_EUID:
> -   return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
> +   return audit_uid_comparator(audit_get_loginuid(tsk), f->op,
> +   cred->euid);
> case AUDIT_COMPARE_AUID_TO_SUID:
> -   return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
> +   return audit_uid_comparator(audit_get_loginuid(tsk), f->op,
> +   cred->suid);
> case AUDIT_COMPARE_AUID_TO_FSUID:
> -   return audit_uid_comparator(tsk->loginuid, f->op, 
> cred->fsuid);
> +   return audit_uid_comparator(audit_get_loginuid(tsk), f->op,
> +   cred->fsuid);
> /* euid comparisons */
> case AUDIT_COMPARE_EUID_TO_SUID:
> return audit_uid_comparator(cred->euid, f->op, cred->suid);
> @@ -611,7 +615,8 @@ static int audit_filter_rules(struct task_struct *tsk,
> result = match_tree_refs(ctx, rule->tree);
> break;
> case AUDIT_LOGINUID:
> -   result = audit_uid_comparator(tsk->loginuid, f->op, 
> f->uid);
> +   result = audit_uid_comparator(audit_get_loginuid(tsk),
> + f->op, f->uid);
> break;
> case AUDIT_LOGINUID_SET:
> result = audit_comparator(audit_loginuid_set(tsk), 
> f->op, f->val);
> @@ -2278,14 +2283,15 @@ int audit_signal_info(int sig, struct task_struct *t)
>  {
> struct audit_aux_data_pids *axp;
> struct audit_context *ctx = audit_context();
> -   kuid_t uid = current_uid(), t_uid = task_uid(t);
> +   kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
>
> if (auditd_test_task(t) &&
> (sig == SIGTERM || sig == SIGHUP ||
>  sig == SIGUSR1 || sig == SIGUSR2)) {
> audit_sig_pid = task_tgid_nr(current);
> -   if (uid_valid(current->loginuid))
> -   audit_sig_uid = current->loginuid;
> +   auid = audit_get_loginuid(current);
> +   if (uid_valid(auid))
> +   audit_sig_uid = auid;
> else
> audit_sig_uid = uid;
> security_task_getsecid(current, &audit_sig_sid);
> --
> 1.8.3.1
>



-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


Re: [PATCH ghak81 V3 3/3] audit: collect audit task parameters

2018-05-17 Thread Paul Moore
On Wed, May 16, 2018 at 7:55 AM, Richard Guy Briggs  wrote:
> The audit-related parameters in struct task_struct should ideally be
> collected together and accessed through a standard audit API.
>
> Collect the existing loginuid, sessionid and audit_context together in a
> new struct audit_task_info called "audit" in struct task_struct.
>
> Use kmem_cache to manage this pool of memory.
> Un-inline audit_free() to be able to always recover that memory.
>
> See: https://github.com/linux-audit/audit-kernel/issues/81
>
> Signed-off-by: Richard Guy Briggs 
> ---
>  include/linux/audit.h | 34 --
>  include/linux/sched.h |  5 +
>  init/init_task.c  |  3 +--
>  init/main.c   |  2 ++
>  kernel/auditsc.c  | 51 
> ++-
>  kernel/fork.c |  2 +-
>  6 files changed, 71 insertions(+), 26 deletions(-)

As discussed on-list and offline, I'm going to hold off on this change
until the audit container ID work is father along.  That is the main
driver for this change, and until that is closer to ready I just can't
justify the extra overhead.

> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 69c7847..4f824c4 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -216,8 +216,15 @@ static inline void audit_log_task_info(struct 
> audit_buffer *ab,
>
>  /* These are defined in auditsc.c */
> /* Public API */
> +struct audit_task_info {
> +   kuid_t  loginuid;
> +   unsigned intsessionid;
> +   struct audit_context*ctx;
> +};
> +extern struct audit_task_info init_struct_audit;
> +extern void __init audit_task_init(void);
>  extern int  audit_alloc(struct task_struct *task);
> -extern void __audit_free(struct task_struct *task);
> +extern void audit_free(struct task_struct *task);
>  extern void __audit_syscall_entry(int major, unsigned long a0, unsigned long 
> a1,
>   unsigned long a2, unsigned long a3);
>  extern void __audit_syscall_exit(int ret_success, long ret_value);
> @@ -239,12 +246,15 @@ extern void audit_seccomp_actions_logged(const char 
> *names,
>
>  static inline void audit_set_context(struct task_struct *task, struct 
> audit_context *ctx)
>  {
> -   task->audit_context = ctx;
> +   task->audit->ctx = ctx;
>  }
>
>  static inline struct audit_context *audit_context(void)
>  {
> -   return current->audit_context;
> +   if (current->audit)
> +   return current->audit->ctx;
> +   else
> +   return NULL;
>  }
>
>  static inline bool audit_dummy_context(void)
> @@ -252,11 +262,7 @@ static inline bool audit_dummy_context(void)
> void *p = audit_context();
> return !p || *(int *)p;
>  }
> -static inline void audit_free(struct task_struct *task)
> -{
> -   if (unlikely(task->audit_context))
> -   __audit_free(task);
> -}
> +
>  static inline void audit_syscall_entry(int major, unsigned long a0,
>unsigned long a1, unsigned long a2,
>unsigned long a3)
> @@ -328,12 +334,18 @@ extern int auditsc_get_stamp(struct audit_context *ctx,
>
>  static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
>  {
> -   return tsk->loginuid;
> +   if (tsk->audit)
> +   return tsk->audit->loginuid;
> +   else
> +   return INVALID_UID;
>  }
>
>  static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
>  {
> -   return tsk->sessionid;
> +   if (tsk->audit)
> +   return tsk->audit->sessionid;
> +   else
> +   return AUDIT_SID_UNSET;
>  }
>
>  extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);
> @@ -458,6 +470,8 @@ static inline void audit_fanotify(unsigned int response)
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> +static inline void __init audit_task_init(void)
> +{ }
>  static inline int audit_alloc(struct task_struct *task)
>  {
> return 0;
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index b3d697f..6a5db0e 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -29,7 +29,6 @@
>  #include 
>
>  /* task_struct member predeclarations (sorted alphabetically): */
> -struct audit_context;
>  struct backing_dev_info;
>  struct bio_list;
>  struct blk_plug;
> @@ -832,10 +831,8 @@ struct task_struct {
>
> struct callback_head*task_works;
>
> -   struct audit_context*audit_context;
>  #ifdef CONFIG_AUDITSYSCALL
> -   kuid_t  loginuid;
> -   unsigned intsessionid;
> +   struct audit_task_info  *audit;
>  #endif
> struct seccomp  seccomp;
>
> diff --git a/init/init_task.c b/init/init_task.c
> index 74f60ba..4058840 100644
> --- a/init/init_task

Re: [PATCH ghak81 V3a] fixup! audit: collect audit task parameters

2018-05-17 Thread Paul Moore
On Thu, May 17, 2018 at 1:20 PM, Richard Guy Briggs  wrote:
> Enable fork.c compilation with audit disabled.
>
> Signed-off-by: Richard Guy Briggs 
> ---
> Hi Paul, this one got caught by the 0-day kbuildbot.  Can you squash it
> down if you haven't merged it yet?

See my comment in the original patch.  I would just recommend
squashing this into the original patch the next time you post it.

> ---
>  kernel/fork.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 92ab849..ff82928 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1713,7 +1713,9 @@ static __latent_entropy struct task_struct 
> *copy_process(
> p->start_time = ktime_get_ns();
> p->real_start_time = ktime_get_boot_ns();
> p->io_context = NULL;
> +#ifdef CONFIG_AUDITSYSCALL
> p->audit = NULL;
> +#endif /* CONFIG_AUDITSYSCALL */
> cgroup_fork(p);
>  #ifdef CONFIG_NUMA
> p->mempolicy = mpol_dup(p->mempolicy);
> --
> 1.8.3.1

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit


[PATCH] audit: use existing session info function

2018-05-17 Thread Richard Guy Briggs
Use the existing audit_log_session_info() function rather than
hardcoding its functionality.

Signed-off-by: Richard Guy Briggs 
---
 kernel/auditfilter.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index d7a807e..9e87377 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1089,8 +1089,6 @@ static void audit_list_rules(int seq, struct sk_buff_head 
*q)
 static void audit_log_rule_change(char *action, struct audit_krule *rule, int 
res)
 {
struct audit_buffer *ab;
-   uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
-   unsigned int sessionid = audit_get_sessionid(current);
 
if (!audit_enabled)
return;
@@ -1098,7 +1096,7 @@ static void audit_log_rule_change(char *action, struct 
audit_krule *rule, int re
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
if (!ab)
return;
-   audit_log_format(ab, "auid=%u ses=%u" ,loginuid, sessionid);
+   audit_log_session_info(ab);
audit_log_task_context(ab);
audit_log_format(ab, " op=%s", action);
audit_log_key(ab, rule->filterkey);
-- 
1.8.3.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit