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]
Message-ID: <CAFftDdoMBg-7bcgQ_VPh_1Cy4jkfcamz4NZJexm5BGqi6iRvvg@mail.gmail.com>
Date:	Tue, 11 Feb 2014 10:13:44 -0800
From:	William Roberts <bill.c.roberts@...il.com>
To:	Richard Guy Briggs <rgb@...hat.com>
Cc:	"linux-audit@...hat.com" <linux-audit@...hat.com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
	akpm@...ux-foundation.org, Stephen Smalley <sds@...ho.nsa.gov>,
	William Roberts <wroberts@...sys.com>
Subject: Re: [PATCH v5 3/3] audit: Audit proc/<pid>/cmdline aka proctitle

I'm a liar v6 does not include rtrim and appears to be the same thing as v5.

I'm posting v7 now, that has the acks I have received on the patches
as well as rtrim on
patch 3, the patch that adds the support to audit.

Sorry for the confusion, ill buy the first 3 people who come to me that were
inconvenienced a drink at a conference :-P

Bill

On Tue, Feb 11, 2014 at 9:47 AM, William Roberts
<bill.c.roberts@...il.com> wrote:
> The most up to date patches were v6. The difference between v5 and v6
> is rtrim(). Did you not want the rtrim?
> Most things end with null bytes, this helps prevent hex-escaping when
> not needed.
>
> v6 - adds rtrim
> http://marc.info/?l=linux-kernel&m=139093195718315&w=2
> http://marc.info/?l=linux-kernel&m=139093196518317&w=2
> http://marc.info/?l=linux-kernel&m=139093197518332&w=2
>
> Bill
>
>
> On Tue, Feb 11, 2014 at 9:25 AM, William Roberts
> <bill.c.roberts@...il.com> wrote:
>> On Tue, Feb 11, 2014 at 8:36 AM, Richard Guy Briggs <rgb@...hat.com> wrote:
>>> On 14/02/06, William Roberts wrote:
>>>> During an audit event, cache and print the value of the process's
>>>> proctitle value (proc/<pid>/cmdline). This is useful in situations
>>>> where processes are started via fork'd virtual machines where the
>>>> comm field is incorrect. Often times, setting the comm field still
>>>> is insufficient as the comm width is not very wide and most
>>>> virtual machine "package names" do not fit. Also, during execution,
>>>> many threads have their comm field set as well. By tying it back to
>>>> the global cmdline value for the process, audit records will be more
>>>> complete in systems with these properties. An example of where this
>>>> is useful and applicable is in the realm of Android. With Android,
>>>> their is no fork/exec for VM instances. The bare, preloaded Dalvik
>>>> VM listens for a fork and specialize request. When this request comes
>>>> in, the VM forks, and the loads the specific application (specializing).
>>>> This was done to take advantage of COW and to not require a load of
>>>> basic packages by the VM on very app spawn. When this spawn occurs,
>>>> the package name is set via setproctitle() and shows up in procfs.
>>>> Many of these package names are longer then 16 bytes, the historical
>>>> width of task->comm. Having the cmdline in the audit records will
>>>> couple the application back to the record directly. Also, on my
>>>> Debian development box, some audit records were more useful then
>>>> what was printed under comm.
>>>>
>>>> The cached proctitle is tied to the life-cycle of the audit_context
>>>> structure and is built on demand.
>>>>
>>>> Proctitle is controllable by userspace, and thus should not be trusted.
>>>> It is meant as an aid to assist in debugging. The proctitle event is
>>>> emitted during syscall audits, and can be filtered with auditctl.
>>>>
>>>> Example:
>>>> type=AVC msg=audit(1391217013.924:386): avc:  denied  { getattr } for  pid=1971 comm="mkdir" name="/" dev="selinuxfs" ino=1 scontext=system_u:system_r:consolekit_t:s0-s0:c0.c255 tcontext=system_u:object_r:security_t:s0 tclass=filesystem
>>>> type=SYSCALL msg=audit(1391217013.924:386): arch=c000003e syscall=137 success=yes exit=0 a0=7f019dfc8bd7 a1=7fffa6aed2c0 a2=fffffffffff4bd25 a3=7fffa6aed050 items=0 ppid=1967 pid=1971 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="mkdir" exe="/bin/mkdir" subj=system_u:system_r:consolekit_t:s0-s0:c0.c255 key=(null)
>>>> type=UNKNOWN[1327] msg=audit(1391217013.924:386):  proctitle=6D6B646972002D70002F7661722F72756E2F636F6E736F6C65
>>>>
>>>> Signed-off-by: William Roberts <wroberts@...sys.com>
>>>
>>> Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
>>>
>>> Though, I would prefer to see the size of the proctitle copy buffer
>>> dynamically allocated based on the size of the original rather than
>>> pinned at 128.
>>
>> Not as good as it originally seems as this could be a whole page,
>> which would result in 2*PAGE_SIZE if hex escaped back to
>> userspace. A tuneable interface could be added in the future if its needed.
>>
>>>
>>>> ---
>>>>  include/uapi/linux/audit.h |    1 +
>>>>  kernel/audit.h             |    6 ++++
>>>>  kernel/auditsc.c           |   67 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 74 insertions(+)
>>>>
>>>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>>>> index 2d48fe1..4315ee9 100644
>>>> --- a/include/uapi/linux/audit.h
>>>> +++ b/include/uapi/linux/audit.h
>>>> @@ -109,6 +109,7 @@
>>>>  #define AUDIT_NETFILTER_PKT  1324    /* Packets traversing netfilter chains */
>>>>  #define AUDIT_NETFILTER_CFG  1325    /* Netfilter chain modifications */
>>>>  #define AUDIT_SECCOMP                1326    /* Secure Computing event */
>>>> +#define AUDIT_PROCTITLE              1327    /* Proctitle emit event */
>>>>
>>>>  #define AUDIT_AVC            1400    /* SE Linux avc denial or grant */
>>>>  #define AUDIT_SELINUX_ERR    1401    /* Internal SE Linux Errors */
>>>> diff --git a/kernel/audit.h b/kernel/audit.h
>>>> index 57cc64d..38c967d 100644
>>>> --- a/kernel/audit.h
>>>> +++ b/kernel/audit.h
>>>> @@ -106,6 +106,11 @@ struct audit_names {
>>>>       bool                    should_free;
>>>>  };
>>>>
>>>> +struct audit_proctitle {
>>>> +     int     len;    /* length of the cmdline field. */
>>>> +     char    *value; /* the cmdline field */
>>>> +};
>>>> +
>>>>  /* The per-task audit context. */
>>>>  struct audit_context {
>>>>       int                 dummy;      /* must be the first element */
>>>> @@ -202,6 +207,7 @@ struct audit_context {
>>>>               } execve;
>>>>       };
>>>>       int fds[2];
>>>> +     struct audit_proctitle proctitle;
>>>>
>>>>  #if AUDIT_DEBUG
>>>>       int                 put_count;
>>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>>> index 10176cd..e342eb0 100644
>>>> --- a/kernel/auditsc.c
>>>> +++ b/kernel/auditsc.c
>>>> @@ -68,6 +68,7 @@
>>>>  #include <linux/capability.h>
>>>>  #include <linux/fs_struct.h>
>>>>  #include <linux/compat.h>
>>>> +#include <linux/ctype.h>
>>>>
>>>>  #include "audit.h"
>>>>
>>>> @@ -79,6 +80,9 @@
>>>>  /* no execve audit message should be longer than this (userspace limits) */
>>>>  #define MAX_EXECVE_AUDIT_LEN 7500
>>>>
>>>> +/* max length to print of cmdline/proctitle value during audit */
>>>> +#define MAX_PROCTITLE_AUDIT_LEN 128
>>>> +
>>>>  /* number of audit rules */
>>>>  int audit_n_rules;
>>>>
>>>> @@ -842,6 +846,13 @@ static inline struct audit_context *audit_get_context(struct task_struct *tsk,
>>>>       return context;
>>>>  }
>>>>
>>>> +static inline void audit_proctitle_free(struct audit_context *context)
>>>> +{
>>>> +     kfree(context->proctitle.value);
>>>> +     context->proctitle.value = NULL;
>>>> +     context->proctitle.len = 0;
>>>> +}
>>>> +
>>>>  static inline void audit_free_names(struct audit_context *context)
>>>>  {
>>>>       struct audit_names *n, *next;
>>>> @@ -955,6 +966,7 @@ static inline void audit_free_context(struct audit_context *context)
>>>>       audit_free_aux(context);
>>>>       kfree(context->filterkey);
>>>>       kfree(context->sockaddr);
>>>> +     audit_proctitle_free(context);
>>>>       kfree(context);
>>>>  }
>>>>
>>>> @@ -1271,6 +1283,59 @@ static void show_special(struct audit_context *context, int *call_panic)
>>>>       audit_log_end(ab);
>>>>  }
>>>>
>>>> +static inline int audit_proctitle_rtrim(char *proctitle, int len)
>>>> +{
>>>> +     char *end = proctitle + len - 1;
>>>> +     while (end > proctitle && !isprint(*end))
>>>> +             end--;
>>>> +
>>>> +     /* catch the case where proctitle is only 1 non-print character */
>>>> +     len = end - proctitle + 1;
>>>> +     len -= isprint(proctitle[len-1]) == 0;
>>>> +     return len;
>>>> +}
>>>> +
>>>> +static void audit_log_proctitle(struct task_struct *tsk,
>>>> +                      struct audit_context *context)
>>>> +{
>>>> +     int res;
>>>> +     char *buf;
>>>> +     char *msg = "(null)";
>>>> +     int len = strlen(msg);
>>>> +     struct audit_buffer *ab;
>>>> +
>>>> +     ab = audit_log_start(context, GFP_KERNEL, AUDIT_PROCTITLE);
>>>> +     if (!ab)
>>>> +             return; /* audit_panic or being filtered */
>>>> +
>>>> +     audit_log_format(ab, "proctitle=");
>>>> +
>>>> +     /* Not  cached */
>>>> +     if (!context->proctitle.value) {
>>>> +             buf = kmalloc(MAX_PROCTITLE_AUDIT_LEN, GFP_KERNEL);
>>>> +             if (!buf)
>>>> +                     goto out;
>>>> +             /* Historically called this from procfs naming */
>>>> +             res = get_cmdline(tsk, buf, MAX_PROCTITLE_AUDIT_LEN);
>>>> +             if (res == 0) {
>>>> +                     kfree(buf);
>>>> +                     goto out;
>>>> +             }
>>>> +             res = audit_proctitle_rtrim(buf, res);
>>>> +             if (res == 0) {
>>>> +                     kfree(buf);
>>>> +                     goto out;
>>>> +             }
>>>> +             context->proctitle.value = buf;
>>>> +             context->proctitle.len = res;
>>>> +     }
>>>> +     msg = context->proctitle.value;
>>>> +     len = context->proctitle.len;
>>>> +out:
>>>> +     audit_log_n_untrustedstring(ab, msg, len);
>>>> +     audit_log_end(ab);
>>>> +}
>>>> +
>>>>  static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
>>>>  {
>>>>       int i, call_panic = 0;
>>>> @@ -1388,6 +1453,8 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
>>>>               audit_log_name(context, n, NULL, i++, &call_panic);
>>>>       }
>>>>
>>>> +     audit_log_proctitle(tsk, context);
>>>> +
>>>>       /* Send end of event record to help user space know we are finished */
>>>>       ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
>>>>       if (ab)
>>>> --
>>>> 1.7.9.5
>>>>
>>>
>>> - RGB
>>>
>>> --
>>> Richard Guy Briggs <rbriggs@...hat.com>
>>> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
>>> Remote, Ottawa, Canada
>>> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
>>
>>
>>
>> --
>> Respectfully,
>>
>> William C Roberts
>
>
>
> --
> Respectfully,
>
> William C Roberts



-- 
Respectfully,

William C Roberts
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ