Re: [PATCH] audit: Add cmdline to taskinfo output

William Roberts
2013-10-30T20:18:13Z

[-- Attachment #1.1: Type: text/plain, Size: 5332 bytes --]

On Wed, Oct 30, 2013 at 12:42 PM, Steve Grubb <sgrubb@redhat.com> wrote:

> On Tuesday, October 29, 2013 05:43:36 PM William Roberts wrote:
> > >> I guess I could just set the comm field explicitly via the packagename
> > >> when the classloader loads the value, but I was hoping for something
> more
> > >> generic that would
> > >> let me get larger package names then 16.
> > >
> > > I made the change of setting the comm field from within the VM, but its
> > > less then ideal... that 16char limitation is a pain. In Android Java
> Land,
> > > some of the packages that get run can be quite large. Also, current
> APIs
> > > in Javaland already change this...
> > >
> > > Also, a more generic solution would be desired.
> > >
> > > Lets look at what happens:
> > > type=SYSCALL msg=audit(10/29/2013 15:16:08.185:177) : arch=unknown elf
> > > type(40000028) syscall=fstat per=840000 success=yes exit=38 a0=7432ed34
> > > a1=20241 a2=180 a3=7432ed0c items=1 ppid=322 pid=1432 auid=unset
> > > uid=unknown(1027) gid=unknown(1027) euid=unknown(1027)
> suid=unknown(1027)
> > > fsuid=unknown(1027) egid=unknown(1027) sgid=unknown(1027)
> > > fsgid=unknown(1027) tty=(none) ses=4294967295 comm=AsyncTask #1
> > > exe=/system/bin/app_process cmdline="com.android.nfc" subj=u:r:nfc:s0
> > > key=(null)
> > >
> > > Here the nfc task has an async task, that async task api sets the cmd
> > > field when it attaches a thread to the VM....
> > >
> > > type=1300 msg=audit(1383088554.170:322): arch=40000028 syscall=54
> > > per=840000 success=yes exit=0 a0=a a1=c0186201 a2=be985430 a3=be98542c
> > > items=0 ppid=321 pid=1181 auid=4294967295 uid=10036 gid=10036
> euid=10036
> > > suid=10036 fsuid=10036 egid=10036 sgid=10036 fsgid=10036 tty=(none)
> > > ses=4294967295 comm="putmethod.latin" exe="/system/bin/app_process"
> > > cmdline="com.android.inputmethod.latin" subj=u:r:shared_app:s0
> key=(null)
> > >
> > > Again... the comm field got cut off and now I have no idea again.
>
> Which is the same as all arches. What I'm trying to say is that all arches
> would benefit from fixing this problem. I don't like the idea of it
> getting fixed
> for one platform and leaving it for all others to figure out another day.
>

By arches your don't mean arm right? This code runs the same on other
architectures. If
you mean platforms, like Android, vs some other Linux distro, then yes I
want a generic
approach, which I think cmdline gets us... no mater how many layers of VM
exec/forking
indirection hell you may find yourself in, you at least get a chance at
what started the chain.
On Android, that happens to be the packagename.

>
> Is there some reason that the length of that field must be set to 16? I've
> seen
> user id numbers increased by a config option. It might be that the naming
> convention of android apps is enough to get a change.
>
> > I think exe= in the audit logs is essentially arg[0]... so thats not
> going
> > to work here,
>

We can't change the naming convention of andorid apps, too large of an
ecosystem to
change and no real easy way to be backwards compatible. That one is off the
table.

I have compiled kernels in the past with custom COMM widths, but the memory
footprint
goes up, at least here were not keeping a bunch of possibly unused data
around in the kernel
plus we're not allocating anything on the common case of it being turned
off.

> The original problem was shell scripts. We got /bin/bash and had no idea
> what
> was being executed. So, cmd was offered as a quick fix.
>
> > and I don't think I can change that value from userspace as its not
> > > logged with untrusted string, which is a good indication its mutable
> from
> > > userspace.
> > >
> > > Why dont I just limit the size of what is displayed on cmdline to
> > > something like 128 or 256?
> > >
> > > Eventually some limit has to be set, whether its PAGE_SIZE or
> not..their
> > > will always be an argument of "too much memory". But its also
> important to
> > > note its off by default, you have to turn it on, so most desktop
> instances
> > > will leave it off, whilst I will dynamically enable it as needed.
> > >
> > > Thanks again for your review and help, I appreciate it.
> >
> > Looking further into your size concerns, EXECVE is truncated at 7500
> >
> > kernel/auditsc.c:
> > #define MAX_EXECVE_AUDIT_LEN 7500
>
> I think that is for the purposes of splitting records. At the time,
> compiling
> a kernel or maybe linking it allowed passing thousands of file names on the
> command line and Eric wrote a patch to split the execve record into
> multiple
> ones. Userspace had to be adapted to glue them back together.
>

Ahh ok, thanks.

>
> > the proc cmdline info is truncated at PAGE_SIZE, which most of the time
> in
> > 4096.. so its even smaller then that.
>
> And then if there is a space in the path, it gets encoded so double that
> size.
>

Ahh yeah.. forgot about those.

>
> > So based on our discussion, whats the next step at moving forward on
> this?
> >
> > Do you want a separate limit other then PAGE_SIZE on this?
>
> The limit is PATH_MAX. You could have an absolute path that uses all
> available
> characters.
>
> -Steve
>

So looking at PATH_MAX...
include/linux/limits.h:12:#define PATH_MAX 4096 /* # chars in a path
name including nul */

I can set it to that...NP

- Bill

[-- Attachment #1.2: Type: text/html, Size: 7657 bytes --]