From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ee0-f47.google.com (mail-ee0-f47.google.com [74.125.83.47]) by kanga.kvack.org (Postfix) with ESMTP id B93246B0031 for ; Tue, 11 Feb 2014 13:13:46 -0500 (EST) Received: by mail-ee0-f47.google.com with SMTP id d49so3797032eek.34 for ; Tue, 11 Feb 2014 10:13:46 -0800 (PST) Received: from mail-wi0-x236.google.com (mail-wi0-x236.google.com [2a00:1450:400c:c05::236]) by mx.google.com with ESMTPS id g47si33660629eev.111.2014.02.11.10.13.44 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 11 Feb 2014 10:13:45 -0800 (PST) Received: by mail-wi0-f182.google.com with SMTP id f8so4951038wiw.9 for ; Tue, 11 Feb 2014 10:13:44 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1391710528-23481-1-git-send-email-wroberts@tresys.com> <1391710528-23481-3-git-send-email-wroberts@tresys.com> <20140211163629.GM18807@madcap2.tricolour.ca> Date: Tue, 11 Feb 2014 10:13:44 -0800 Message-ID: Subject: Re: [PATCH v5 3/3] audit: Audit proc//cmdline aka proctitle From: William Roberts Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Sender: owner-linux-mm@kvack.org List-ID: To: Richard Guy Briggs Cc: "linux-audit@redhat.com" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" , "viro@zeniv.linux.org.uk" , akpm@linux-foundation.org, Stephen Smalley , William Roberts 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 wer= e inconvenienced a drink at a conference :-P Bill On Tue, Feb 11, 2014 at 9:47 AM, William Roberts 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=3Dlinux-kernel&m=3D139093195718315&w=3D2 > http://marc.info/?l=3Dlinux-kernel&m=3D139093196518317&w=3D2 > http://marc.info/?l=3Dlinux-kernel&m=3D139093197518332&w=3D2 > > Bill > > > On Tue, Feb 11, 2014 at 9:25 AM, William Roberts > wrote: >> On Tue, Feb 11, 2014 at 8:36 AM, Richard Guy Briggs wro= te: >>> On 14/02/06, William Roberts wrote: >>>> During an audit event, cache and print the value of the process's >>>> proctitle value (proc//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=3DAVC msg=3Daudit(1391217013.924:386): avc: denied { getattr } = for pid=3D1971 comm=3D"mkdir" name=3D"/" dev=3D"selinuxfs" ino=3D1 scontex= t=3Dsystem_u:system_r:consolekit_t:s0-s0:c0.c255 tcontext=3Dsystem_u:object= _r:security_t:s0 tclass=3Dfilesystem >>>> type=3DSYSCALL msg=3Daudit(1391217013.924:386): arch=3Dc000003e syscal= l=3D137 success=3Dyes exit=3D0 a0=3D7f019dfc8bd7 a1=3D7fffa6aed2c0 a2=3Dfff= ffffffff4bd25 a3=3D7fffa6aed050 items=3D0 ppid=3D1967 pid=3D1971 auid=3D429= 4967295 uid=3D0 gid=3D0 euid=3D0 suid=3D0 fsuid=3D0 egid=3D0 sgid=3D0 fsgid= =3D0 tty=3D(none) ses=3D4294967295 comm=3D"mkdir" exe=3D"/bin/mkdir" subj= =3Dsystem_u:system_r:consolekit_t:s0-s0:c0.c255 key=3D(null) >>>> type=3DUNKNOWN[1327] msg=3Daudit(1391217013.924:386): proctitle=3D6D6= B646972002D70002F7661722F72756E2F636F6E736F6C65 >>>> >>>> Signed-off-by: William Roberts >>> >>> Signed-off-by: Richard Guy Briggs >>> >>> 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 need= ed. >> >>> >>>> --- >>>> 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 even= t */ >>>> +#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 >>>> #include >>>> #include >>>> +#include >>>> >>>> #include "audit.h" >>>> >>>> @@ -79,6 +80,9 @@ >>>> /* no execve audit message should be longer than this (userspace limi= ts) */ >>>> #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_con= text(struct task_struct *tsk, >>>> return context; >>>> } >>>> >>>> +static inline void audit_proctitle_free(struct audit_context *context= ) >>>> +{ >>>> + kfree(context->proctitle.value); >>>> + context->proctitle.value =3D NULL; >>>> + context->proctitle.len =3D 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 =3D proctitle + len - 1; >>>> + while (end > proctitle && !isprint(*end)) >>>> + end--; >>>> + >>>> + /* catch the case where proctitle is only 1 non-print character = */ >>>> + len =3D end - proctitle + 1; >>>> + len -=3D isprint(proctitle[len-1]) =3D=3D 0; >>>> + return len; >>>> +} >>>> + >>>> +static void audit_log_proctitle(struct task_struct *tsk, >>>> + struct audit_context *context) >>>> +{ >>>> + int res; >>>> + char *buf; >>>> + char *msg =3D "(null)"; >>>> + int len =3D strlen(msg); >>>> + struct audit_buffer *ab; >>>> + >>>> + ab =3D audit_log_start(context, GFP_KERNEL, AUDIT_PROCTITLE); >>>> + if (!ab) >>>> + return; /* audit_panic or being filtered */ >>>> + >>>> + audit_log_format(ab, "proctitle=3D"); >>>> + >>>> + /* Not cached */ >>>> + if (!context->proctitle.value) { >>>> + buf =3D kmalloc(MAX_PROCTITLE_AUDIT_LEN, GFP_KERNEL); >>>> + if (!buf) >>>> + goto out; >>>> + /* Historically called this from procfs naming */ >>>> + res =3D get_cmdline(tsk, buf, MAX_PROCTITLE_AUDIT_LEN); >>>> + if (res =3D=3D 0) { >>>> + kfree(buf); >>>> + goto out; >>>> + } >>>> + res =3D audit_proctitle_rtrim(buf, res); >>>> + if (res =3D=3D 0) { >>>> + kfree(buf); >>>> + goto out; >>>> + } >>>> + context->proctitle.value =3D buf; >>>> + context->proctitle.len =3D res; >>>> + } >>>> + msg =3D context->proctitle.value; >>>> + len =3D 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 =3D 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 finis= hed */ >>>> ab =3D audit_log_start(context, GFP_KERNEL, AUDIT_EOE); >>>> if (ab) >>>> -- >>>> 1.7.9.5 >>>> >>> >>> - RGB >>> >>> -- >>> Richard Guy Briggs >>> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Syst= ems, 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 --=20 Respectfully, William C Roberts -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org