Inside unpack_profile() data->data is allocated using kvmemdup() so it
should be freed with the corresponding kvfree_sensitive().
Also add missing data->data release for rhashtable insertion failure path
in unpack_profile().
Found by Linux Verification Center (linuxtesting.org).
Fixes: e025be0f26 ("apparmor: support querying extended trusted helper extra data")
Cc: stable@vger.kernel.org
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fix some kernel-doc comments to silence the warnings:
security/apparmor/policy.c:117: warning: Function parameter or member 'kref' not described in 'aa_pdb_free_kref'
security/apparmor/policy.c:117: warning: Excess function parameter 'kr' description in 'aa_pdb_free_kref'
security/apparmor/policy.c:882: warning: Function parameter or member 'subj_cred' not described in 'aa_may_manage_policy'
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=7037
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
unprivileged unconfined can use change_profile to alter the confinement
set by the mac admin.
Allow restricting unprivileged unconfined by still allowing change_profile
but stacking the change against unconfined. This allows unconfined to
still apply system policy but allows the task to enter the new confinement.
If unprivileged unconfined is required a sysctl is provided to switch
to the previous behavior.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
With the move to permission tables the dfa is no longer a stand
alone entity when used, needing a minimum of a permission table.
However it still could be shared among different pdbs each using
a different permission table.
Instead of duping the permission table when sharing a pdb, add a
refcount to the pdb so it can be easily shared.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The cred is needed to properly audit some messages, and will be needed
in the future for uid conditional mediation. So pass it through to
where the apparmor_audit_data struct gets defined.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
rename audit_data's label field to subj_label to better reflect its
use. Also at the same time drop unneeded assignments to ->subj_label
as the later call to aa_check_perms will do the assignment if needed.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Everywhere where common_audit_data is used apparmor audit_data is also
used. We can simplify the code and drop the use of the aad macro
everywhere by combining the two structures.
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
profile->disconnected was storing an invalid reference to the
disconnected path. Fix it by duplicating the string using
aa_unpack_strdup and freeing accordingly.
Fixes: 72c8a76864 ("apparmor: allow profiles to provide info to disconnected paths")
Signed-off-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fix kernel-doc warnings:
security/apparmor/policy.c:294: warning: Function parameter or
member 'proxy' not described in 'aa_alloc_profile'
security/apparmor/policy.c:785: warning: Function parameter or
member 'label' not described in 'aa_policy_view_capable'
security/apparmor/policy.c:785: warning: Function parameter or
member 'ns' not described in 'aa_policy_view_capable'
security/apparmor/policy.c:847: warning: Function parameter or
member 'ns' not described in 'aa_may_manage_policy'
security/apparmor/policy.c:964: warning: Function parameter or
member 'hname' not described in '__lookup_replace'
security/apparmor/policy.c:964: warning: Function parameter or
member 'info' not described in '__lookup_replace'
security/apparmor/policy.c:964: warning: Function parameter or
member 'noreplace' not described in '__lookup_replace'
security/apparmor/policy.c:964: warning: Function parameter or
member 'ns' not described in '__lookup_replace'
security/apparmor/policy.c:964: warning: Function parameter or
member 'p' not described in '__lookup_replace'
Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
These allocations should use the gfp flags from the caller instead of
GFP_KERNEL. But from what I can see, all the callers pass in GFP_KERNEL
so this does not affect runtime.
Fixes: e31dd6e412f7 ("apparmor: fix: kzalloc perms tables for shared dfas")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Currently the permstables of the shared dfas are not shared, and need
to be allocated and copied. In the future this should be addressed
with a larger rework on dfa and pdb ref counts and structure sharing.
BugLink: http://bugs.launchpad.net/bugs/2017903
Fixes: 217af7e2f4 ("apparmor: refactor profile rules and attachments")
Cc: stable@vger.kernel.org
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Jon Tourville <jontourville@me.com>
Unfortunately it is possible for some userspace's to load children
profiles before the parent profile. This can even happen when the
child and the parent are in different load sets.
Fix this by creating a null place holder profile that grants no permissions
and can be replaced by the parent once it is loaded.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Bother unconfined and learning profiles use the null profile as their
base. Refactor so they are share a common base routine. This doesn't
save much atm but will be important when the feature set of the
parent is inherited.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Convert profile->rules to a list as the next step towards supporting
multiple rulesets in a profile. For this step only support a single
list entry item. The logic for iterating the list will come as a
separate step.
Signed-off-by: John Johansen <john.johansen@canonical.com>
In preparation for moving from a single set of rules and a single
attachment to multiple rulesets and attachments separate from the
profile refactor attachment information and ruleset info into their
own structures.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Allow the profile to contain a user mode prompt flag. This works similar
to complain mode but will try to send messages to a userspace daemon.
If the daemon is not present or timesout regular informent will occur.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Audit messages currently don't contain the mediation class which can
make them less clear than they should be in some circumstances. With
newer mediation classes coming this potential confusion will become
worse.
Fix this by adding the mediatin class to the messages.
Signed-off-by: John Johansen <john.johansen@canonical.com>
continue permission unification by converting xmatch to use the
policydb struct that is used by the other profile dfas.
Signed-off-by: John Johansen <john.johansen@canonical.com>
file_rules and policydb are almost the same and will need the same
features in the future so combine them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Rather than computing policydb permissions for each access
permissions can be computed once on profile load and stored for lookup.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Rather than computing xmatch permissions each time access is requested,
these permissions can be computed once on profile load and stored for
lookup.
Signed-off-by: Mike Salvatore <mike.salvatore@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fix the following lockdep warning
[ 1119.158984] ============================================
[ 1119.158988] WARNING: possible recursive locking detected
[ 1119.158996] 6.0.0-rc1+ #257 Tainted: G E N
[ 1119.158999] --------------------------------------------
[ 1119.159001] bash/80100 is trying to acquire lock:
[ 1119.159007] ffff88803e79b4a0 (&ns->lock/1){+.+.}-{4:4}, at: destroy_ns.part.0+0x43/0x140
[ 1119.159028]
but task is already holding lock:
[ 1119.159030] ffff8881009764a0 (&ns->lock/1){+.+.}-{4:4}, at: aa_remove_profiles+0x3f0/0x640
[ 1119.159040]
other info that might help us debug this:
[ 1119.159042] Possible unsafe locking scenario:
[ 1119.159043] CPU0
[ 1119.159045] ----
[ 1119.159047] lock(&ns->lock/1);
[ 1119.159051] lock(&ns->lock/1);
[ 1119.159055]
*** DEADLOCK ***
Which is caused by an incorrect lockdep nesting notation
Fixes: feb3c766a3 ("apparmor: fix possible recursive lock warning in __aa_create_ns")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Embedded systems have limited space and don't need the introspection
or checkpoint restore capability provided by exporting the raw
profile binary data so make it so make it a config option.
This will reduce run time memory use and also speed up policy loads.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Fix function name in security/apparmor/label.c, policy.c, procattr.c
kernel-doc comment to remove some warnings found by clang(make W=1 LLVM=1).
security/apparmor/label.c:499: warning: expecting prototype for
aa_label_next_not_in_set(). Prototype was for
__aa_label_next_not_in_set() instead
security/apparmor/label.c:2147: warning: expecting prototype for
__aa_labelset_udate_subtree(). Prototype was for
__aa_labelset_update_subtree() instead
security/apparmor/policy.c:434: warning: expecting prototype for
aa_lookup_profile(). Prototype was for aa_lookupn_profile() instead
security/apparmor/procattr.c:101: warning: expecting prototype for
aa_setprocattr_chagnehat(). Prototype was for aa_setprocattr_changehat()
instead
Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Yang Li <yang.lee@linux.alibaba.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Make use of the struct_size() helper instead of an open-coded version,
in order to avoid any potential type mistakes or integer overflows that,
in the worse scenario, could lead to heap overflows.
Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Signed-off-by: John Johansen <john.johansen@canonical.com>
The check was incorrectly treating a returned error as a boolean.
Fixes: 31ec99e133 ("apparmor: switch to apparmor to internal capable check for policy management")
Signed-off-by: John Johansen <john.johansen@canonical.com>
With LSM stacking calling back into capable to check for MAC_ADMIN
for apparmor policy results in asking the other stacked LSMs for
MAC_ADMIN resulting in the other LSMs answering based on their
policy management.
For apparmor policy management we just need to call apparmor's
capability fn directly.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Previously the policy capable checks assumed they were using the
current task. Make them take the task label so the query can be
made against an arbitrary task.
Signed-off-by: John Johansen <john.johansen@canonical.com>
As said by Linus:
A symmetric naming is only helpful if it implies symmetries in use.
Otherwise it's actively misleading.
In "kzalloc()", the z is meaningful and an important part of what the
caller wants.
In "kzfree()", the z is actively detrimental, because maybe in the
future we really _might_ want to use that "memfill(0xdeadbeef)" or
something. The "zero" part of the interface isn't even _relevant_.
The main reason that kzfree() exists is to clear sensitive information
that should not be leaked to other future users of the same memory
objects.
Rename kzfree() to kfree_sensitive() to follow the example of the recently
added kvfree_sensitive() and make the intention of the API more explicit.
In addition, memzero_explicit() is used to clear the memory to make sure
that it won't get optimized away by the compiler.
The renaming is done by using the command sequence:
git grep -w --name-only kzfree |\
xargs sed -i 's/kzfree/kfree_sensitive/'
followed by some editing of the kfree_sensitive() kerneldoc and adding
a kzfree backward compatibility macro in slab.h.
[akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h]
[akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more]
Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Acked-by: David Howells <dhowells@redhat.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Joe Perches <joe@perches.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: "Jason A . Donenfeld" <Jason@zx2c4.com>
Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
When the proxy isn't replaced and the profile is removed, the proxy
is being leaked resulting in a kmemleak check message of
unreferenced object 0xffff888077a3a490 (size 16):
comm "apparmor_parser", pid 128041, jiffies 4322684109 (age 1097.028s)
hex dump (first 16 bytes):
03 00 00 00 00 00 00 00 b0 92 fd 4b 81 88 ff ff ...........K....
backtrace:
[<0000000084d5daf2>] aa_alloc_proxy+0x58/0xe0
[<00000000ecc0e21a>] aa_alloc_profile+0x159/0x1a0
[<000000004cc9ce15>] unpack_profile+0x275/0x1c40
[<000000007332b3ca>] aa_unpack+0x1e7/0x7e0
[<00000000e25e31bd>] aa_replace_profiles+0x18a/0x1d10
[<00000000350d9415>] policy_update+0x237/0x650
[<000000003fbf934e>] profile_load+0x122/0x160
[<0000000047f7b781>] vfs_write+0x139/0x290
[<000000008ad12358>] ksys_write+0xcd/0x170
[<000000001a9daa7b>] do_syscall_64+0x70/0x310
[<00000000b9efb0cf>] entry_SYSCALL_64_after_hwframe+0x49/0xb3
Make sure to cleanup the profile's embedded label which will result
on the proxy being properly freed.
Fixes: 637f688dc3 ("apparmor: switch from profiles to using labels on contexts")
Signed-off-by: John Johansen <john.johansen@canonical.com>
aa_xattrs_match() is unfortunately calling vfs_getxattr_alloc() from a
context protected by an rcu_read_lock. This can not be done as
vfs_getxattr_alloc() may sleep regardles of the gfp_t value being
passed to it.
Fix this by breaking the rcu_read_lock on the policy search when the
xattr match feature is requested and restarting the search if a policy
changes occur.
Fixes: 8e51f9087f ("apparmor: Add support for attaching profiles via xattr, presence and value")
Reported-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Reported-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: John Johansen <john.johansen@canonical.com>
- increase left match history buffer size to provide inproved conflict
resolution in overlapping execution rules.
- switch buffer allocation to use a memory pool and GFP_KERNEL
where possible.
- add compression of policy blobs to reduce memory usage.
+ Cleanups
- fix spelling mistake "immutible" -> "immutable"
+ Bug fixes
- fix unsigned len comparison in update_for_len macro
- fix sparse warning for type-casting of current->real_cred
-----BEGIN PGP SIGNATURE-----
iQIzBAABCgAdFiEE7cSDD705q2rFEEf7BS82cBjVw9gFAl3mvPUACgkQBS82cBjV
w9gM8hAArhbBiGHlYlsGCOws4+ObCSIAxPkKw9ZC+FjTOKE6uN+GDUM+s4TWjbkL
65NKGBqHfHIzRYHD6BNi5I3Yf0xKCXuMenZVptiDHYQ+65mCL6QlZOA5K2Mp67fY
uMKoOIMSAkDkLJHEsH8o1YURAlvY5DjK2XfSrc2GeaExnBZTisfhDwbYjv9OYI6U
JPDP361zzJMSpkcDf5WX5vVuvfjTnAXjfH3av61hiSNAzivd4P1Mp34ellOkz7Ya
Ch6K+32agVcE8LIbalRKhWVw7Fhfbys2+/nBZ0Tb5HPG0tRWbm+ueggOsp8/liWQ
Ik9NigK61lHjd5ttDrswD0UfslTxac2pPFhlYRYoSUSMITOjJke50Q12ZosK4wUY
pdsBiWVDo2W3/E9sretmFpWlzish8q3tNJU55aKD+FTo0yqMC3X7H/l9xGLuLUt/
vHwUcGZNSrAWqc8yMamzEvqj9e1DECMJZQIlE3YJgGLCkcO6LFY+5pSWSvMQIG7v
451oob3QalzqIDyh3OOxlA8pfUVyk9HL48Kw7+0ZJrbJK6pAjHZhE8gFVMPECB7b
n22XrABdPdjAFvlqCzkm4qZ5sjqdk8T9Iexc5bnrFvBW4teHnAX0xrk+gxVpnEYf
dV6ERcxmRjnZhT6FtOQkLOia3gIiAQVi6Rd9K6HHhPH83wNyjjI=
=lPsA
-----END PGP SIGNATURE-----
Merge tag 'apparmor-pr-2019-12-03' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor
Pull apparmor updates from John Johansen:
"Features:
- increase left match history buffer size to provide improved
conflict resolution in overlapping execution rules.
- switch buffer allocation to use a memory pool and GFP_KERNEL where
possible.
- add compression of policy blobs to reduce memory usage.
Cleanups:
- fix spelling mistake "immutible" -> "immutable"
Bug fixes:
- fix unsigned len comparison in update_for_len macro
- fix sparse warning for type-casting of current->real_cred"
* tag 'apparmor-pr-2019-12-03' of git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor:
apparmor: make it so work buffers can be allocated from atomic context
apparmor: reduce rcu_read_lock scope for aa_file_perm mediation
apparmor: fix wrong buffer allocation in aa_new_mount
apparmor: fix unsigned len comparison with less than zero
apparmor: increase left match history buffer size
apparmor: Switch to GFP_KERNEL where possible
apparmor: Use a memory pool instead per-CPU caches
apparmor: Force type-casting of current->real_cred
apparmor: fix spelling mistake "immutible" -> "immutable"
apparmor: fix blob compression when ns is forced on a policy load
apparmor: fix missing ZLIB defines
apparmor: fix blob compression build failure on ppc
apparmor: Initial implementation of raw policy blob compression
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation version 2 of the license
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-only
has been chosen to replace the boilerplate/reference in 315 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Armijn Hemel <armijn@tjaldur.nl>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190531190115.503150771@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There is a spelling mistake in an information message string, fix it.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
When blob compression is turned on, if the policy namespace is forced
onto a policy load, the policy load will fail as the namespace name
being referenced is inside the compressed policy blob, resulting in
invalid or names that are too long. So duplicate the name before the
blob is compressed.
Fixes: 876dd866c084 ("apparmor: Initial implementation of raw policy blob compression")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Add support for parsing secmark policy provided by userspace, and
store that in the overall policy.
Signed-off-by: Matthew Garrett <mjg59@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
AppArmor is leaking the newly loaded profile and its proxy when
the profile is an exact match to the currently loaded version.
In this case the dedup check results in the profile being skipped and
put without dealing with the proxy ref thus not breaking a circular
refcount and causing a leak.
BugLink: http://bugs.launchpad.net/bugs/1750594
Fixes: 5d5182cae4 ("apparmor: move to per loaddata files, instead of replicating in profiles")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Acked-by: Christian Boltz <apparmor@cboltz.de>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Use a radix tree to provide a map between the secid and the label,
and along with it a basic ability to provide secctx conversion.
Shared/cached secctx will be added later.
Signed-off-by: John Johansen <john.johansen@canonical.com>
This converts profile attachment based on xattrs to a fixed extended
conditional using dfa matching.
This has a couple of advantages
- pattern matching can be used for the xattr match
- xattrs can be optional for an attachment or marked as required
- the xattr attachment conditional will be able to be combined with
other extended conditionals when the flexible extended conditional
work lands.
The xattr fixed extended conditional is appended to the xmatch
conditional. If an xattr attachment is specified the profile xmatch
will be generated regardless of whether there is a pattern match on
the executable name.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Make it possible to tie Apparmor profiles to the presence of one or more
extended attributes, and optionally their values. An example usecase for
this is to automatically transition to a more privileged Apparmor profile
if an executable has a valid IMA signature, which can then be appraised
by the IMA subsystem.
Signed-off-by: Matthew Garrett <mjg59@google.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Now that file contexts have been moved into file, and task context
fns() and data have been split from the context, only the cred context
remains in context.h so rename to cred.h to better reflect what it
deals with.
Signed-off-by: John Johansen <john.johansen@canonical.com>
