Menu
▾
▴
Re: [Audit-test-developer] [design] How to write / run / audit syscall wrappers?
|
From: Jiri J. <jja...@re...> - 2014-07-29 11:22:27
|
On 07/28/2014 06:39 PM, Linda Knippers wrote:
> Hi Jiri,
>
> On 07/28/2014 09:33 AM, Jiri Jaburek wrote:
>> Hello Linda & others,
>>
>> I've been doing some syscall work recently and while doing it,
>> I decided to "clean up" the do_ipc and do_socketcall wrappers, which
>> seemed like a duplicated functionality, since they call exactly the same
>> library functions as normal do_* wrappers. Also, I really wanted to get
>> rid of the ipc headerhack. :)
>
> That's a good goal.:-)
>
>> This was somewhat amplified by the fact that the code in ipc_common.c
>> was over-shared, meaning that ie. semget was using flags for semctl
>> or semop. So I made a series of ~6 commits, carefully moving all the
>> functionality from ipc_common.c into separate wrappers and removing the
>> do_ipc wrapper. I did the same for do_socketcall, which just calls bind,
>> using a library function, like do_bind. All this with removing
>> respective sections from syscalls/*.conf, of course.
>>
>> I was quite happy with the series, since - functionality wise - it was
>> transparent. However when I tested it on MODE=32, the syscalls bucket
>> started throwing ERRORs.
>
> Right. The 32-bit x86 syscalls add a lot of complexity.
> Did you see any problems on non-x86 architectures?
I didn't test other architectures, but the code suggests that all 32bit
variants are affected, for ipc(2) at least.
>
>> Some investigation uncovered that the syscalls
>> bucket was actually using these "duplicated" wrappers for proper
>> auditing - because auditctl works with real syscalls, not libc
>> functions. The extra wrappers were therefore nothing more than a name,
>> simplifying logic in the syscalls bucket.
>>
>> This goes against some other approaches used in the suite - in the
>> network bucket, for example, which - based on the architecture - selects
>> proper syscall name for auditctl, while still calling the original
>> syscall wrapper (which uses library functions).
>>
>> -----------------------------------------------------------------------
>>
>> This led me into a certain design question I'd like to ask here; how to
>> design syscall wrappers and the execution and auditing infrastructure
>> around them? What would be the best approach?
>>
>> I've identified 3 most obvious ways to write a syscall wrapper:
>>
>> A) use syscall(__NR_syscallname, ...) directly, bypassing libc
>> B) use libc functions
>> C) use (A), but simulate libc using #ifdefs manually
>
> Today we use both A) and B), depending on the syscall. B) is easiest
> from a coding perspective. A) is sometimes necessary because libc
> might not actually be using the syscall we want in the mode we want
> or may be doing error checking of it's own that prevent some of the case
> we want to test. I'm not sure I understand C).
(C) for do_chmod could look like
#ifdef ARM
exitval = syscall(__NR_fchmodat, ...);
#else
exitval = syscall(__NR_chmod, ...);
#endif
essentially simulating glibc in a controlled manner.
>
>> These approaches solve the "which syscalls to run where" problem
>> somewhat differently and therefore have different benefits and
>> drawbacks in various situations:
>>
>> 1) compile time
>>
>> (A) is a bit problematic, since we would need to come up with a full
>> logic of what syscalls to compile on what architectures. We already
>> do that on some level, but this option calls for a separate mapping
>> file (or so) instead of simple Makefile-based conditions,
>> integrating it somehow into the make system.
>
> Right - we seem to be reinventing libc.
Please note that (A) isn't about reinventing libc - it isn't about
providing abstractions for syscalls, since it's these abstractions that
later cause "hacky" auditing code in tests.
>
>> (B) and (C) are really easy - the libc (or custom #ifdefs) take care
>> of which syscall should be called on which architecture. (C) has
>> the disadvantage of actually doing the mapping from (A), just in
>> a less visible way.
>>
>> 2) run (execution) time
>>
>> (A) again needs to re-use the mapping file (or logic) in most cases,
>> since - if we want to call ie. open-like syscall, we need to call
>> explicitly either do_open (non-arm) or do_openat (arm)
>
> And in some cases, we need to do both when the arch supports both.
>
>> (B) and (C) simplify this case a lot, but may fall short if we
>> *really* want to call openat instead of open on non-arm
>
> We do that today. We test both open and openat using the do_* programs.
>
>> 3) auditing time
>>
>> (A) shines here, IMO, since the auditctl mapping is 1:1 to the do_*
>> wrappers. It still needs to re-use the mapping file (and therefore
>> needs to manually specify which syscalls to audit on which archs),
>> but it's very straightforward and clear.
>>
>> (B) and (C) have a significant problem here - we don't know which
>> syscalls are being called "under the hood" of the do_* wrappers,
>
> Well, we do for all the existing calls.
*We* do, but the tests don't, which is why we have to create ad-hoc
conditions to tell them.
>
>> so
>> we need to try them out and then create per-arch hacks in the code
>> similar to what we've seen in the arm patchset recently, ie.
>> "set up auditing for fchmodat and call do_chmod", which can be
>> somewhat confusing. The other option is to duplicate wrappers like
>> do_socketcall, but the per-arch hacks still persist. We could create
>> a mapping file for them, but then we might as well use (A).
>>
>>
>> So what would be the best approach for new (and existing, over time)
>> syscall wrappers?
>>
>> I personally really like (A) due to its clean design - there are no
>> "Note: There is no glibc wrapper for this system call" exceptions
>> and it's clear what syscalls run on which architectures and 32/64bit
>> variations. The mapping file, with its helper bash functions doing ie.
>> "is this syscall relevant for current arch/mode?" or "list all relevant
>> syscalls for current arch/mode", along with some documentation, should
>> be mostly easy to implement.
>> A practical example in the syscalls bucket would be checking for arch
>> relevancy in the `+' function with no per-arch or per-mode conditions
>> in the .conf files. The rollup log (or run.bash --list) would then show
>> which syscalls were actually run.
>
> I don't think there is one "best approach". Some of what we have today
> is because the suite has evolved over time but some if it is because for
> the most part, using the libc functions is simple and appropriate.
> Where's it's not simple or appropriate, we drop back to the the syscall(_NR...)
> function. Sometimes due to legacy syscalls, like the multiplexed 32-bit
> syscalls, it gets a bit complicated but hey, we've written that code already. :-)
I agree that using libc is often better and it would be perhaps cleaner
if auditctl supported syscall translation (ie. chmod->fchmodat on arm),
but it doesn't, it needs a direct syscall name for a given arch / mode.
My points for (A) go mostly towards the auditing code, see the current
state of augrok_default and auwatch_default in network/run.conf.
Or the per-arch conditions in syscalls/*-run.conf.
With a unified way of telling which syscalls are relevant for which
architectures, the conditions in syscalls/ could go away. The entire
case/esac structure in network/ could as well, since $syscall would
always be the tested syscall and no other.
The "unified way" doesn't have to be a static file, it can be generated
using gcc from unistd.h dynamically.
Also, "we've written that code already" doesn't mean there won't be more
of it in the future (well, near future).
>
>> This also means that I would have to throw away most of my series on
>> do_ipc and do_socketcall, possibly re-implementing them in the future,
>> but I'm fine with that.
>>
>> What's your opinion?
>
> If it ain't broke....?
Right, I was wondering whether a "clean" solution wouldn't be an
overkill, since there aren't that much differences aside from do_ipc
and do_socketcall. For me, it's just another cleanup, like the netfilter
or networking-related code.
>
>> Thanks,
>> Jiri
>>
>>
>> PS: I'm asking because we currently have ~70 new syscall wrappers staged
>> for review and there's not much consistency in terms of (A) vs (B)
>> vs (C).
>
> I'm interesting in knowing more about your new syscalls and why you have
> a mix of A), B) and C) (which I still don't understand but I'm sure a simple
> example would clear things up). Is it because 3 different people wrote them
> or because there really is no "best approach"?
Various reasons, really. Written mostly by a single person, (A) was used
where using (B) would pull in another suite dependency (NUMA),
(B) was used where (A) would result in two independent wrappers (fstatat
vs newfstatat), and various mix of both. Yes, because there's no defined
policy of "best approach".
>
> I'm not trying to say that we have to keep doing things the way we've always done it.
> This suite has clearly evolved and improved as you and others have worked on it so I'd
> like to continue this discussion.
Sure, I'd like to get as much feedback as possible before implementing
anything (or deciding to leave it be in the current state).
>
> -- ljk
>
>>
>> ------------------------------------------------------------------------------
>> Infragistics Professional
>> Build stunning WinForms apps today!
>> Reboot your WinForms applications with our WinForms controls.
>> Build a bridge from your legacy apps to the future.
>> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
>> _______________________________________________
>> Audit-test-developer mailing list
>> Aud...@li...
>> https://lists.sourceforge.net/lists/listinfo/audit-test-developer
>
>
> ------------------------------------------------------------------------------
> Infragistics Professional
> Build stunning WinForms apps today!
> Reboot your WinForms applications with our WinForms controls.
> Build a bridge from your legacy apps to the future.
> http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
> _______________________________________________
> Audit-test-developer mailing list
> Aud...@li...
> https://lists.sourceforge.net/lists/listinfo/audit-test-developer
>
Jiri
|