When __d_path and d_absolute_path fail due to the name being outside of
the current namespace no name is reported. Use dentry_path to provide
some hint as to which file was being accessed.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Post unpacking of policy a verification pass is made on x transition
indexes. When this fails a call to audit_iface is made resulting in an
oops, because audit_iface is expecting a valid buffer position but
since the failure comes from post unpack verification there is none.
Make the position argument optional so that audit_iface can be called
from post unpack verification.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The returning of -ESATLE when a path lookup fails as disconnected is wrong.
Since AppArmor is rejecting the access return -EACCES instead.
This also fixes a bug in complain (learning) mode where disconnected paths
are denied because -ESTALE errors are not ignored causing failures that
can change application behavior.
Signed-off-by: John Johansen <john.johansen@canonical.com>
When a chroot relative pathname lookup fails it is falling through to
do a d_absolute_path lookup. This is incorrect as d_absolute_path should
only be used to lookup names for namespace absolute paths.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The mapping of AA_MAY_META_READ for the allow mask was also being mapped
to the audit and quiet masks. This would result in some operations being
audited when the should not.
This flaw was hidden by the previous audit bug which would drop some
messages that where supposed to be audited.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
If the xindex value stored in the accept tables is 0, the extraction of
that value will result in an underflow (0 - 4).
In properly compiled policy this should not happen for file rules but
it may be possible for other rule types in the future.
To exploit this underflow a user would have to be able to load a corrupt
policy, which requires CAP_MAC_ADMIN, overwrite system policy in kernel
memory or know of a compiler error resulting in the flaw being present
for loaded policy (no such flaw is known at this time).
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The audit permission flag, that specifies an audit message should be
provided when an operation is allowed, was being ignored in some cases.
This is because the auto audit mode (which determines the audit mode from
system flags) was incorrectly assigned the same value as audit mode. The
shared value would result in messages that should be audited going through
a second evaluation as to whether they should be audited based on the
auto audit, resulting in some messages being dropped.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
The unpacking of struct capsx is missing a check for the end of the
caps structure. This can lead to unpack failures depending on what else
is packed into the policy file being unpacked.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Kees Cook <kees@ubuntu.com>
Since the parser needs to know which rlimits are known to the kernel,
export the list via a mask file in the "rlimit" subdirectory in the
securityfs "features" directory.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Create the "file" directory in the securityfs for tracking features
related to files.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
This adds the "features" subdirectory to the AppArmor securityfs
to display boolean features flags and the known capability mask.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Use a file tree structure to represent the AppArmor securityfs.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
* 'for-linus' of git://selinuxproject.org/~jmorris/linux-security:
capabilities: remove __cap_full_set definition
security: remove the security_netlink_recv hook as it is equivalent to capable()
ptrace: do not audit capability check when outputing /proc/pid/stat
capabilities: remove task_ns_* functions
capabitlies: ns_capable can use the cap helpers rather than lsm call
capabilities: style only - move capable below ns_capable
capabilites: introduce new has_ns_capabilities_noaudit
capabilities: call has_ns_capability from has_capability
capabilities: remove all _real_ interfaces
capabilities: introduce security_capable_noaudit
capabilities: reverse arguments to security_capable
capabilities: remove the task from capable LSM hook entirely
selinux: sparse fix: fix several warnings in the security server cod
selinux: sparse fix: fix warnings in netlink code
selinux: sparse fix: eliminate warnings for selinuxfs
selinux: sparse fix: declare selinux_disable() in security.h
selinux: sparse fix: move selinux_complete_init
selinux: sparse fix: make selinux_secmark_refcount static
SELinux: Fix RCU deref check warning in sel_netport_insert()
Manually fix up a semantic mis-merge wrt security_netlink_recv():
- the interface was removed in commit fd77846152 ("security: remove
the security_netlink_recv hook as it is equivalent to capable()")
- a new user of it appeared in commit a38f7907b9 ("crypto: Add
userspace configuration API")
causing no automatic merge conflict, but Eric Paris pointed out the
issue.
module_param(bool) used to counter-intuitively take an int. In
fddd5201 (mid-2009) we allowed bool or int/unsigned int using a messy
trick.
It's time to remove the int/unsigned int option. For this version
it'll simply give a warning, but it'll break next kernel version.
Acked-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* 'for-linus' of git://selinuxproject.org/~jmorris/linux-security: (32 commits)
ima: fix invalid memory reference
ima: free duplicate measurement memory
security: update security_file_mmap() docs
selinux: Casting (void *) value returned by kmalloc is useless
apparmor: fix module parameter handling
Security: tomoyo: add .gitignore file
tomoyo: add missing rcu_dereference()
apparmor: add missing rcu_dereference()
evm: prevent racing during tfm allocation
evm: key must be set once during initialization
mpi/mpi-mpow: NULL dereference on allocation failure
digsig: build dependency fix
KEYS: Give key types their own lockdep class for key->sem
TPM: fix transmit_cmd error logic
TPM: NSC and TIS drivers X86 dependency fix
TPM: Export wait_for_stat for other vendor specific drivers
TPM: Use vendor specific function for status probe
tpm_tis: add delay after aborting command
tpm_tis: Check return code from getting timeouts/durations
tpm: Introduce function to poll for result of self test
...
Fix up trivial conflict in lib/Makefile due to addition of CONFIG_MPI
and SIGSIG next to CONFIG_DQL addition.
The capabilities framework is based around credentials, not necessarily the
current task. Yet we still passed the current task down into LSMs from the
security_capable() LSM hook as if it was a meaningful portion of the security
decision. This patch removes the 'generic' passing of current and instead
forces individual LSMs to use current explicitly if they think it is
appropriate. In our case those LSMs are SELinux and AppArmor.
I believe the AppArmor use of current is incorrect, but that is wholely
unrelated to this patch. This patch does not change what AppArmor does, it
just makes it clear in the AppArmor code that it is doing it.
The SELinux code still uses current in it's audit message, which may also be
wrong and needs further investigation. Again this is NOT a change, it may
have always been wrong, this patch just makes it clear what is happening.
Signed-off-by: Eric Paris <eparis@redhat.com>
it's not needed anymore; we used to, back when we had to do
mount_subtree() by hand, complete with put_mnt_ns() in it.
No more... Apparmor didn't need it since the __d_path() fix.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
The 'aabool' wrappers actually pass off to the 'bool' parse functions,
so you should use the same check function. Similarly for aauint and
uint.
(Note that 'bool' module parameters also allow 'int', which is why you
got away with this, but that's changing very soon.)
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <jmorris@namei.org>
Adds a missed rcu_dereference() around real_parent.
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: James Morris <jmorris@namei.org>
__d_path() API is asking for trouble and in case of apparmor d_namespace_path()
getting just that. The root cause is that when __d_path() misses the root
it had been told to look for, it stores the location of the most remote ancestor
in *root. Without grabbing references. Sure, at the moment of call it had
been pinned down by what we have in *path. And if we raced with umount -l, we
could have very well stopped at vfsmount/dentry that got freed as soon as
prepend_path() dropped vfsmount_lock.
It is safe to compare these pointers with pre-existing (and known to be still
alive) vfsmount and dentry, as long as all we are asking is "is it the same
address?". Dereferencing is not safe and apparmor ended up stepping into
that. d_namespace_path() really wants to examine the place where we stopped,
even if it's not connected to our namespace. As the result, it looked
at ->d_sb->s_magic of a dentry that might've been already freed by that point.
All other callers had been careful enough to avoid that, but it's really
a bad interface - it invites that kind of trouble.
The fix is fairly straightforward, even though it's bigger than I'd like:
* prepend_path() root argument becomes const.
* __d_path() is never called with NULL/NULL root. It was a kludge
to start with. Instead, we have an explicit function - d_absolute_root().
Same as __d_path(), except that it doesn't get root passed and stops where
it stops. apparmor and tomoyo are using it.
* __d_path() returns NULL on path outside of root. The main
caller is show_mountinfo() and that's precisely what we pass root for - to
skip those outside chroot jail. Those who don't want that can (and do)
use d_path().
* __d_path() root argument becomes const. Everyone agrees, I hope.
* apparmor does *NOT* try to use __d_path() or any of its variants
when it sees that path->mnt is an internal vfsmount. In that case it's
definitely not mounted anywhere and dentry_path() is exactly what we want
there. Handling of sysctl()-triggered weirdness is moved to that place.
* if apparmor is asked to do pathname relative to chroot jail
and __d_path() tells it we it's not in that jail, the sucker just calls
d_absolute_path() instead. That's the other remaining caller of __d_path(),
BTW.
* seq_path_root() does _NOT_ return -ENAMETOOLONG (it's stupid anyway -
the normal seq_file logics will take care of growing the buffer and redoing
the call of ->show() just fine). However, if it gets path not reachable
from root, it returns SEQ_SKIP. The only caller adjusted (i.e. stopped
ignoring the return value as it used to do).
Reviewed-by: John Johansen <john.johansen@canonical.com>
ACKed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Cc: stable@vger.kernel.org
Fix sparse warnings:
security/apparmor/procattr.c:35:5: warning: symbol 'aa_getprocattr' was not declared. Should it be static?
security/apparmor/procattr.c:113:5: warning: symbol 'aa_setprocattr_changehat' was not declared. Should it be static?
security/apparmor/procattr.c:158:5: warning: symbol 'aa_setprocattr_changeprofile' was not declared. Should it be static?
security/apparmor/procattr.c:166:5: warning: symbol 'aa_setprocattr_permipc' was not declared. Should it be static?
Signed-off-by: James Morris <jmorris@namei.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Fix the following warnings:
security/apparmor/policy_unpack.c:384:35: warning: symbol 'size' shadows an earlier one
security/apparmor/policy_unpack.c:370:24: originally declared here
security/apparmor/policy_unpack.c:443:29: warning: symbol 'tmp' shadows an earlier one
security/apparmor/policy_unpack.c:434:21: originally declared here
Signed-off-by: James Morris <jmorris@namei.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Fix the following sparse warnings:
security/apparmor/lib.c:37:6: warning: symbol 'aa_split_fqname' was not declared. Should it be static?
security/apparmor/lib.c:63:6: warning: symbol 'aa_info_message' was not declared. Should it be static?
security/apparmor/lib.c:83:6: warning: symbol 'kvmalloc' was not declared. Should it be static?
security/apparmor/lib.c:123:6: warning: symbol 'kvfree' was not declared. Should it be static?
Signed-off-by: James Morris <jmorris@namei.org>
Include ipc.h to eliminate sparse warnings.
security/apparmor/ipc.c:61:5: warning: symbol 'aa_may_ptrace' was not declared. Should it be static?
security/apparmor/ipc.c:83:5: warning: symbol 'aa_ptrace' was not declared. Should it be static
Signed-off-by: James Morris <jmorris@namei.org>
Acked-by: John Johansen <john.johansen@canonical.com>
* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6: (54 commits)
tpm_nsc: Fix bug when loading multiple TPM drivers
tpm: Move tpm_tis_reenable_interrupts out of CONFIG_PNP block
tpm: Fix compilation warning when CONFIG_PNP is not defined
TOMOYO: Update kernel-doc.
tpm: Fix a typo
tpm_tis: Probing function for Intel iTPM bug
tpm_tis: Fix the probing for interrupts
tpm_tis: Delay ACPI S3 suspend while the TPM is busy
tpm_tis: Re-enable interrupts upon (S3) resume
tpm: Fix display of data in pubek sysfs entry
tpm_tis: Add timeouts sysfs entry
tpm: Adjust interface timeouts if they are too small
tpm: Use interface timeouts returned from the TPM
tpm_tis: Introduce durations sysfs entry
tpm: Adjust the durations if they are too small
tpm: Use durations returned from TPM
TOMOYO: Enable conditional ACL.
TOMOYO: Allow using argv[]/envp[] of execve() as conditions.
TOMOYO: Allow using executable's realpath and symlink's target as conditions.
TOMOYO: Allow using owner/group etc. of file objects as conditions.
...
Fix up trivial conflict in security/tomoyo/realpath.c
* 'ptrace' of git://git.kernel.org/pub/scm/linux/kernel/git/oleg/misc: (39 commits)
ptrace: do_wait(traced_leader_killed_by_mt_exec) can block forever
ptrace: fix ptrace_signal() && STOP_DEQUEUED interaction
connector: add an event for monitoring process tracers
ptrace: dont send SIGSTOP on auto-attach if PT_SEIZED
ptrace: mv send-SIGSTOP from do_fork() to ptrace_init_task()
ptrace_init_task: initialize child->jobctl explicitly
has_stopped_jobs: s/task_is_stopped/SIGNAL_STOP_STOPPED/
ptrace: make former thread ID available via PTRACE_GETEVENTMSG after PTRACE_EVENT_EXEC stop
ptrace: wait_consider_task: s/same_thread_group/ptrace_reparented/
ptrace: kill real_parent_is_ptracer() in in favor of ptrace_reparented()
ptrace: ptrace_reparented() should check same_thread_group()
redefine thread_group_leader() as exit_signal >= 0
do not change dead_task->exit_signal
kill task_detached()
reparent_leader: check EXIT_DEAD instead of task_detached()
make do_notify_parent() __must_check, update the callers
__ptrace_detach: avoid task_detached(), check do_notify_parent()
kill tracehook_notify_death()
make do_notify_parent() return bool
ptrace: s/tracehook_tracer_task()/ptrace_parent()/
...
AppArmor is masking the capabilities returned by capget against the
capabilities mask in the profile. This is wrong, in complain mode the
profile has effectively all capabilities, as the profile restrictions are
not being enforced, merely tested against to determine if an access is
known by the profile.
This can result in the wrong behavior of security conscience applications
like sshd which examine their capability set, and change their behavior
accordingly. In this case because of the masked capability set being
returned sshd fails due to DAC checks, even when the profile is in complain
mode.
Kernels affected: 2.6.36 - 3.0.
Signed-off-by: John Johansen <john.johansen@canonical.com>
The pointer returned from tracehook_tracer_task() is only valid inside
the rcu_read_lock. However the tracer pointer obtained is being passed
to aa_may_ptrace outside of the rcu_read_lock critical section.
Mover the aa_may_ptrace test into the rcu_read_lock critical section, to
fix this.
Kernels affected: 2.6.36 - 3.0
Reported-by: Oleg Nesterov <oleg@redhat.com>
Cc: stable@kernel.org
Signed-off-by: John Johansen <john.johansen@canonical.com>
tracehook.h is on the way out. Rename tracehook_tracer_task() to
ptrace_parent() and move it from tracehook.h to ptrace.h.
Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Remove linux/mm.h inclusion from netdevice.h -- it's unused (I've checked manually).
To prevent mm.h inclusion via other channels also extract "enum dma_data_direction"
definition into separate header. This tiny piece is what gluing netdevice.h with mm.h
via "netdevice.h => dmaengine.h => dma-mapping.h => scatterlist.h => mm.h".
Removal of mm.h from scatterlist.h was tried and was found not feasible
on most archs, so the link was cutoff earlier.
Hope people are OK with tiny include file.
Note, that mm_types.h is still dragged in, but it is a separate story.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Affected kernels 2.6.36 - 3.0
AppArmor may do a GFP_KERNEL memory allocation with task_lock(tsk->group_leader);
held when called from security_task_setrlimit. This will only occur when the
task's current policy has been replaced, and the task's creds have not been
updated before entering the LSM security_task_setrlimit() hook.
BUG: sleeping function called from invalid context at mm/slub.c:847
in_atomic(): 1, irqs_disabled(): 0, pid: 1583, name: cupsd
2 locks held by cupsd/1583:
#0: (tasklist_lock){.+.+.+}, at: [<ffffffff8104dafa>] do_prlimit+0x61/0x189
#1: (&(&p->alloc_lock)->rlock){+.+.+.}, at: [<ffffffff8104db2d>]
do_prlimit+0x94/0x189
Pid: 1583, comm: cupsd Not tainted 3.0.0-rc2-git1 #7
Call Trace:
[<ffffffff8102ebf2>] __might_sleep+0x10d/0x112
[<ffffffff810e6f46>] slab_pre_alloc_hook.isra.49+0x2d/0x33
[<ffffffff810e7bc4>] kmem_cache_alloc+0x22/0x132
[<ffffffff8105b6e6>] prepare_creds+0x35/0xe4
[<ffffffff811c0675>] aa_replace_current_profile+0x35/0xb2
[<ffffffff811c4d2d>] aa_current_profile+0x45/0x4c
[<ffffffff811c4d4d>] apparmor_task_setrlimit+0x19/0x3a
[<ffffffff811beaa5>] security_task_setrlimit+0x11/0x13
[<ffffffff8104db6b>] do_prlimit+0xd2/0x189
[<ffffffff8104dea9>] sys_setrlimit+0x3b/0x48
[<ffffffff814062bb>] system_call_fastpath+0x16/0x1b
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reported-by: Miles Lane <miles.lane@gmail.com>
Cc: stable@kernel.org
Signed-off-by: James Morris <jmorris@namei.org>
When invalid parameters are passed to apparmor_setprocattr a NULL deref
oops occurs when it tries to record an audit message. This is because
it is passing NULL for the profile parameter for aa_audit. But aa_audit
now requires that the profile passed is not NULL.
Fix this by passing the current profile on the task that is trying to
setprocattr.
Signed-off-by: Kees Cook <kees@ubuntu.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Cc: stable@kernel.org
Signed-off-by: James Morris <jmorris@namei.org>
move LSM-, credentials-, and keys-related files from Documentation/
to Documentation/security/,
add Documentation/security/00-INDEX, and
update all occurrences of Documentation/<moved_file>
to Documentation/security/<moved_file>.
- Introduce ns_capable to test for a capability in a non-default
user namespace.
- Teach cap_capable to handle capabilities in a non-default
user namespace.
The motivation is to get to the unprivileged creation of new
namespaces. It looks like this gets us 90% of the way there, with
only potential uid confusion issues left.
I still need to handle getting all caps after creation but otherwise I
think I have a good starter patch that achieves all of your goals.
Changelog:
11/05/2010: [serge] add apparmor
12/14/2010: [serge] fix capabilities to created user namespaces
Without this, if user serge creates a user_ns, he won't have
capabilities to the user_ns he created. THis is because we
were first checking whether his effective caps had the caps
he needed and returning -EPERM if not, and THEN checking whether
he was the creator. Reverse those checks.
12/16/2010: [serge] security_real_capable needs ns argument in !security case
01/11/2011: [serge] add task_ns_capable helper
01/11/2011: [serge] add nsown_capable() helper per Bastian Blank suggestion
02/16/2011: [serge] fix a logic bug: the root user is always creator of
init_user_ns, but should not always have capabilities to
it! Fix the check in cap_capable().
02/21/2011: Add the required user_ns parameter to security_capable,
fixing a compile failure.
02/23/2011: Convert some macros to functions as per akpm comments. Some
couldn't be converted because we can't easily forward-declare
them (they are inline if !SECURITY, extern if SECURITY). Add
a current_user_ns function so we can use it in capability.h
without #including cred.h. Move all forward declarations
together to the top of the #ifdef __KERNEL__ section, and use
kernel-doc format.
02/23/2011: Per dhowells, clean up comment in cap_capable().
02/23/2011: Per akpm, remove unreachable 'return -EPERM' in cap_capable.
(Original written and signed off by Eric; latest, modified version
acked by him)
[akpm@linux-foundation.org: fix build]
[akpm@linux-foundation.org: export current_user_ns() for ecryptfs]
[serge.hallyn@canonical.com: remove unneeded extra argument in selinux's task_has_capability]
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Acked-by: Daniel Lezcano <daniel.lezcano@free.fr>
Acked-by: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Signed-off-by: Serge E. Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
clean-files should be defined as a variable not a target.
Signed-off-by: Michal Hocko <mhocko@suse.cz>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Cleanups based on comments from Sam Ravnborg,
* remove references to the currently unused af_names.h
* add rlim_names.h to clean-files:
* rework cmd_make-XXX to make them more readable by adding comments,
reworking the expressions to put logical components on individual lines,
and keep lines < 80 characters.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
Remove kobject.h from files which don't need it, notably,
sched.h and fs.h.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Remove path.h from sched.h and other files.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
John Johansen