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

William Roberts
2013-10-30T00:43:36Z

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

On Tue, Oct 29, 2013 at 4:24 PM, William Roberts
<bill.c.roberts@gmail.com>wrote:

>
>
>
> On Tue, Oct 29, 2013 at 1:25 PM, William Roberts <bill.c.roberts@gmail.com
> > wrote:
>
>>
>>
>>
>> On Tue, Oct 29, 2013 at 12:55 PM, Steve Grubb <sgrubb@redhat.com> wrote:
>>
>>> On Tuesday, October 29, 2013 12:12:29 PM William Roberts wrote:
>>> > > > to small for most package names, and
>>> > > > already contains the VM command. I really have no information of
>>> what
>>> > > > Android App has created the issue.
>>> > >
>>> > > This is true for all arches. Usually you can have it pretty narrowly
>>> > > defined to
>>> > > where you have a pretty good guess between 2 or 3 apps with the same
>>> root
>>> > > name. But in your case its totally named wrong.
>>> >
>>> > I could set the title via prctl and PR_SET_NAME, but again I would be
>>> > limited at 16 bytes, at least with cmdline I am limited at a page.
>>>
>>> A page would be a problem for audit records. What I see is a NULL
>>> terminated
>>> list of arguments which the program name is argv[0]. So, you'd want to
>>> grab
>>> that one. Butyou could have something in there with PATH_MAX and
>>> whitespaces
>>> which would be excessively long.
>>>
>>> > As a simple example, a basic example from samsung gets truncated.
>>> >
>>> > com.samsung.myapp
>>> >
>>> > > > Solution:
>>> > > > Get the proc cmdline info (not trust worthy, but can help debugging
>>> > >
>>> > > Android)
>>> > >
>>> > > > type=1300 msg=audit(1383068585.326:205): arch=40000028 syscall=5
>>> > >
>>> > > per=840000
>>> > >
>>> > > > success=yes exit=38 a0=74d86d34 a1=20241 a2=180 a3=74d86d0c items=1
>>> > > > ppid=296 pid=1378 auid=4294967295 uid=1027 gid=1027 euid=1027
>>> suid=1027
>>> > > > fsuid=1027 egid=1027 sgid=1027 fsgid=1027 tty=(none) ses=4294967295
>>> > > > comm=4173796E635461736B202331 exe="/system/bin/app_process"
>>> > > > cmdline="com.android.nfc" subj=u:r:nfc:s0 key=(null)
>>> > > >
>>> > > > Now I know it was the NFC app
>>> > >
>>> > > What do you get on x86_64 auditing a shell or python script with
>>> your same
>>> > > patch? Also, does cmdline potentially include arguments?
>>> >
>>> > I would have to get back to you on this, but whatever is set in
>>> > /proc/<pid>/cmdline shows up here, which means
>>> > it could have arguments etc.
>>>
>>> The reason I'm asking is that it might be better for all arches to
>>> switch. All
>>> have the 16 character limit. But we would only want argv[0] and not the
>>> arguments.
>>>
>>> -Steve
>>>
>>
>> I guess i'm thinking about how can I access the smallest set of data that
>> I need to get the information I want.... however, wouldn't argv[0]
>> typically be the vm name...
>> <vm> <program>
>> And on Android, to make it even more of a pain.... A VM is already
>> running, that then forks itself and then invokes the classloader, so their
>> is no
>> explicit exec.
>>
>> 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. I think
> exe= in the audit logs is essentially arg[0]... so thats not going to work
> here, 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

the proc cmdline info is truncated at PAGE_SIZE, which most of the time in
4096.. so its even smaller then that.

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?

--
Respectfully,

William C Roberts

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