Re: [PATCH] audit: Add cmdline to taskinfo output
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.
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,
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.
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.
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