RFE: add support for SECCOMP_RET_AUDIT #64
Conversation
Extend libseccomp to support SECCOMP_RET_AUDIT, which is intended to audit before allowing an action. Signed-off-by: Tyler Hicks <[email protected]>
The basics needed to handle tests that use the new SCMP_ACT_AUDIT action. Signed-off-by: Tyler Hicks <[email protected]>
Extend the 06-sim-actions set of tests to include tests for SCMP_ACT_AUDIT. Signed-off-by: Tyler Hicks <[email protected]>
Test SCMP_ACT_AUDIT as the default action which all syscalls trigger. Signed-off-by: Tyler Hicks <[email protected]>
|
Thanks for the PR. I'm going to defer looking at this until we see if/when the kernel functionality lands upstream. However, one thing to think about, and my apologies if you've already taken this into consideration in this PR, how do we detect if SECCOMP_RET_AUDIT is supported at runtime? |
|
On 01/02/2017 03:59 PM, Paul Moore wrote:
Thanks for the PR. I'm going to defer looking at this until we see
if/when the kernel functionality lands upstream. However, one thing to
think about, and my apologies if you've already taken this into
consideration in this PR, how do we detect if SECCOMP_RET_AUDIT is
supported at runtime?
I considered the need for runtime detection of SECCOMP_RET_AUDIT and
decided that it likely wouldn't be a requirement for two reasons:
1) The seccomp kernel code fails close to SECCOMP_RET_KILL when the
return action is unrecognized.
2) Runtime detection wasn't added for the kill, trap, or errno actions
(commit 308ad5f).
Do you consider runtime detection for SECCOMP_RET_AUDIT to be a
requirement? If so, it'll likely drive changes in the kernel patch set.
|
While this may be the "safe" option, it seems unlikely that application developers like converting an AUDIT action into a KILL action. I can already see the bug reports and hate mail flowing into my inbox ...
Look again at when that commit landed in libseccomp. Those actions have been supported by libseccomp since our first tagged release (v0.1.0) and were supported by the current kernel at that time. Since there was no libseccomp "legacy" at that point it was reasonable to require certain things, now that libseccomp is a bit more mature and is in use by a number of projects, we have to be more careful about older kernel versions.
We need some way to safely handle SECCOMP_RET_AUDIT on older kernels that are not aware of this action; that's the requirement, and runtime detection is one way to solve that (we can safely avoid using AUDIT on kernels where it is not supported), but there may be others. Don't be afraid to get clever if needed, just remember that the code can't be too ugly, I've got to maintain this thing for a while ;) |
|
You've probably already seen this, but if you haven't, we do have some detection hacks in src/system.c ... look at sys_chk_seccomp_syscall(). |
|
1. The seccomp kernel code fails close to SECCOMP_RET_KILL when the
return action is unrecognized.
While this may be the "safe" option, it seems unlikely that application
developers like converting an AUDIT action into a KILL action. I can
already see the bug reports and hate mail flowing into my inbox ...
Hehe, good point. :)
2. Runtime detection wasn't added for the kill, trap, or errno
actions (commit 308ad5f
<308ad5f>).
Look again at when that commit landed in libseccomp. Those actions have
been supported by libseccomp since our first tagged release (v0.1.0) and
were supported by the current kernel at that time. Since there was no
libseccomp "legacy" at that point it was reasonable to require certain
things, now that libseccomp is a bit more mature and is in use by a
number of projects, we have to be more careful about older kernel versions.
That makes sense.
Do you consider runtime detection for SECCOMP_RET_AUDIT to be a
requirement? If so, it'll likely drive changes in the kernel patch set.
We need some way to safely handle SECCOMP_RET_AUDIT on older kernels
that are not aware of this action; that's the requirement, and runtime
detection is one way to solve that (we can safely avoid using AUDIT on
kernels where it is not supported), but there may be others. Don't be
afraid to get clever if needed, just remember that the code can't be too
ugly, I've got to maintain this thing for a while ;)
Understood. I'll see what I can come up with.
|
|
@tyhicks what is the status of the kernel patches? I thought you were planning on submitting a revision taking into account the feedback you received, but I haven't seen anything; is this still on your todo list? |
|
@pcmoore I'm still working on the second revision. A couple weeks travel and another week of catchup starved those patches for time but look for the patch set to go out later this week or early next. |
|
@tyhicks Great, no rush, I just wanted to make sure someone was still working on this; I'm happy to hear you are following up with a second revision. |
|
I think we can close this PR since it isn't relevant anymore, right @tyhicks? |
|
Yeah, I should just create an all new PR. Please close this one. Thanks!On Aug 17, 2017 12:44 PM, Paul Moore <[email protected]> wrote:I think we can close this PR since it isn't relevant anymore, right @tyhicks?
—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or mute the thread.
|
This pull request adds support for SECCOMP_RET_AUDIT which is a newly proposed seccomp return action that audits the syscall before allowing it to happen. It is similar to SECCOMP_RET_ALLOW in that the syscall is allowed but differs in that the syscall will be audited.