| Age | Commit message (Collapse) | Author |
|---|
|
Begin the actual switch to using domain labels by storing them on
the context and converting the label to a singular profile where
possible.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Begin moving apparmor to using broader domain labels, that will allow
run time computation of domain type splitting via "stacking" of
profiles into a domain label vec.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Instead of running file revalidation lazily when read/write are called
copy selinux and revalidate the file table on exec. This avoids
extra mediation overhead in read/write and also prevents file handles
being passed through to a grand child unchecked.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Instead of passing multiple booleans consolidate on a single flags
field.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Remove the partially implemented code, until this can be properly
implemented.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The profile names are the same, leverage this.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
There are still a few places where profile replacement fails to update
and a stale profile is used for mediation. Fix this by moving to
accessing the current label through a critical section that will
always ensure mediation is using the current label regardless of
whether the tasks cred has been updated or not.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
There is no reason to have the small stubs that don't use domain
private functions in domain.c, instead move them to lsm.c and make
them static.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The ns name being displayed should go through an ns view lookup.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The data being queried isn't always the current profile and a lookup
relative to the current profile should be done.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The namespace being passed into the replace/remove profiles fns() is
not the view, but the namespace specified by the inode from the
file hook (if present) or the loading tasks ns, if accessing the
top level virtualized load/replace file interface.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Currently lookups are restricted to a single ns component in the
path. However when namespaces are allowed to have separate views, and
scopes this will not be sufficient, as it will be possible to have
a multiple component ns path in scope.
Add some ns lookup fns() to allow this and use them.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Allow userspace to query a profile about permissions, through the
transaction interface that is already used to allow userspace to
query about key,value data.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
The simple_transaction interface is slow. It requires 4 syscalls
(open, write, read, close) per query and shares a single lock for each
queries.
So replace its use with a compatible in multi_transaction interface.
It allows for a faster 2 syscall pattern per query. After an initial
open, an arbitrary number of writes and reads can be issued. Each
write will reset the query with new data that can be read. Reads do
not clear the data, and can be issued multiple times, and used with
seek, until a new write is performed which will reset the data
available and the seek position.
Note: this keeps the single lock design, if needed moving to a per
file lock will have to come later.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
gsettings mediation needs to be able to determine if apparmor supports
label data queries. A label data query can be done to test for support
but its failure is indistinguishable from other failures, making it an
unreliable indicator.
Fix by making support of label data queries available as a flag in the
apparmorfs features dir tree.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
When setting up namespaces for containers its easier for them to use
an fs interface to create the namespace for the containers
policy. Allow mkdir/rmdir under the policy/namespaces/ dir to be used
to create and remove namespaces.
BugLink: http://bugs.launchpad.net/bugs/1611078
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Add a policy revision file to find the current revision of a ns's policy.
There is a revision file per ns, as well as a virtualized global revision
file in the base apparmor fs directory. The global revision file when
opened will provide the revision of the opening task namespace.
The revision file can be waited on via select/poll to detect apparmor
policy changes from the last read revision of the opened file. This
means that the revision file must be read after the select/poll other
wise update data will remain ready for reading.
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
labeling behavior
When an NFSv4 client performs a mount operation, it first mounts the
NFSv4 root and then does path walk to the exported path and performs a
submount on that, cloning the security mount options from the root's
superblock to the submount's superblock in the process.
Unless the NFS server has an explicit fsid=0 export with the
"security_label" option, the NFSv4 root superblock will not have
SBLABEL_MNT set, and neither will the submount superblock after cloning
the security mount options. As a result, setxattr's of security labels
over NFSv4.2 will fail. In a similar fashion, NFSv4.2 mounts mounted
with the context= mount option will not show the correct labels because
the nfs_server->caps flags of the cloned superblock will still have
NFS_CAP_SECURITY_LABEL set.
Allowing the NFSv4 client to enable or disable SECURITY_LSM_NATIVE_LABELS
behavior will ensure that the SBLABEL_MNT flag has the correct value
when the client traverses from an exported path without the
"security_label" option to one with the "security_label" option and
vice versa. Similarly, checking to see if SECURITY_LSM_NATIVE_LABELS is
set upon return from security_sb_clone_mnt_opts() and clearing
NFS_CAP_SECURITY_LABEL if necessary will allow the correct labels to
be displayed for NFSv4.2 mounts mounted with the context= mount option.
Resolves: https://github.com/SELinuxProject/selinux-kernel/issues/35
Signed-off-by: Scott Mayhew <smayhew@redhat.com>
Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
Tested-by: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Paul Moore <paul@paul-moore.com>
|
|
The allocated size for each ebitmap_node is 192byte by kzalloc().
Then, ebitmap_node size is fixed, so it's possible to use only 144byte
for each object by kmem_cache_zalloc().
It can reduce some dynamic allocation size.
Signed-off-by: Junil Lee <junil0814.lee@lge.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
If a key's refcount is dropped to zero between key_lookup() peeking at
the refcount and subsequently attempting to increment it, refcount_inc()
will see a zero refcount. Here, refcount_inc() will WARN_ONCE(), and
will *not* increment the refcount, which will remain zero.
Once key_lookup() drops key_serial_lock, it is possible for the key to
be freed behind our back.
This patch uses refcount_inc_not_zero() to perform the peek and increment
atomically.
Fixes: fff292914d3a2f1e ("security, keys: convert key.usage from atomic_t to refcount_t")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: David Windsor <dwindsor@gmail.com>
Cc: Elena Reshetova <elena.reshetova@intel.com>
Cc: Hans Liljestrand <ishkamiel@gmail.com>
Cc: James Morris <james.l.morris@oracle.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
The initial Diffie-Hellman computation made direct use of the MPI
library because the crypto module did not support DH at the time. Now
that KPP is implemented, KEYCTL_DH_COMPUTE should use it to get rid of
duplicate code and leverage possible hardware acceleration.
This fixes an issue whereby the input to the KDF computation would
include additional uninitialized memory when the result of the
Diffie-Hellman computation was shorter than the input prime number.
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Accessing a 'u8[4]' through a '__be32 *' violates alignment rules. Just
make the counter a __be32 instead.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
If userspace called KEYCTL_DH_COMPUTE with kdf_params containing NULL
otherinfo but nonzero otherinfolen, the kernel would allocate a buffer
for the otherinfo, then feed it into the KDF without initializing it.
Fix this by always doing the copy from userspace (which will fail with
EFAULT in this scenario).
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Requesting "digest_null" in the keyctl_kdf_params caused an infinite
loop in kdf_ctr() because the "null" hash has a digest size of 0. Fix
it by rejecting hash algorithms with a digest size of 0.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
While a 'struct key' itself normally does not contain sensitive
information, Documentation/security/keys.txt actually encourages this:
"Having a payload is not required; and the payload can, in fact,
just be a value stored in the struct key itself."
In case someone has taken this advice, or will take this advice in the
future, zero the key structure before freeing it. We might as well, and
as a bonus this could make it a bit more difficult for an adversary to
determine which keys have recently been in use.
This is safe because the key_jar cache does not use a constructor.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
As the previous patch did for encrypted-keys, zero sensitive any
potentially sensitive data related to the "trusted" key type before it
is freed. Notably, we were not zeroing the tpm_buf structures in which
the actual key is stored for TPM seal and unseal, nor were we zeroing
the trusted_key_payload in certain error paths.
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Safford <safford@us.ibm.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
For keys of type "encrypted", consistently zero sensitive key material
before freeing it. This was already being done for the decrypted
payloads of encrypted keys, but not for the master key and the keys
derived from the master key.
Out of an abundance of caution and because it is trivial to do so, also
zero buffers containing the key payload in encrypted form, although
depending on how the encrypted-keys feature is used such information
does not necessarily need to be kept secret.
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Safford <safford@us.ibm.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Zero the payloads of user and logon keys before freeing them. This
prevents sensitive key material from being kept around in the slab
caches after a key is released.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Before returning from add_key() or one of the keyctl() commands that
takes in a key payload, zero the temporary buffer that was allocated to
hold the key payload copied from userspace. This may contain sensitive
key material that should not be kept around in the slab caches.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
key_update() freed the key_preparsed_payload even if it was not
initialized first. This would cause a crash if userspace called
keyctl_update() on a key with type like "asymmetric" that has a
->preparse() method but not an ->update() method. Possibly it could
even be triggered for other key types by racing with keyctl_setperm() to
make the KEY_NEED_WRITE check fail (the permission was already checked,
so normally it wouldn't fail there).
Reproducer with key type "asymmetric", given a valid cert.der:
keyctl new_session
keyid=$(keyctl padd asymmetric desc @s < cert.der)
keyctl setperm $keyid 0x3f000000
keyctl update $keyid data
[ 150.686666] BUG: unable to handle kernel NULL pointer dereference at 0000000000000001
[ 150.687601] IP: asymmetric_key_free_kids+0x12/0x30
[ 150.688139] PGD 38a3d067
[ 150.688141] PUD 3b3de067
[ 150.688447] PMD 0
[ 150.688745]
[ 150.689160] Oops: 0000 [#1] SMP
[ 150.689455] Modules linked in:
[ 150.689769] CPU: 1 PID: 2478 Comm: keyctl Not tainted 4.11.0-rc4-xfstests-00187-ga9f6b6b8cd2f #742
[ 150.690916] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
[ 150.692199] task: ffff88003b30c480 task.stack: ffffc90000350000
[ 150.692952] RIP: 0010:asymmetric_key_free_kids+0x12/0x30
[ 150.693556] RSP: 0018:ffffc90000353e58 EFLAGS: 00010202
[ 150.694142] RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000000004
[ 150.694845] RDX: ffffffff81ee3920 RSI: ffff88003d4b0700 RDI: 0000000000000001
[ 150.697569] RBP: ffffc90000353e60 R08: ffff88003d5d2140 R09: 0000000000000000
[ 150.702483] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000001
[ 150.707393] R13: 0000000000000004 R14: ffff880038a4d2d8 R15: 000000000040411f
[ 150.709720] FS: 00007fcbcee35700(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
[ 150.711504] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 150.712733] CR2: 0000000000000001 CR3: 0000000039eab000 CR4: 00000000003406e0
[ 150.714487] Call Trace:
[ 150.714975] asymmetric_key_free_preparse+0x2f/0x40
[ 150.715907] key_update+0xf7/0x140
[ 150.716560] ? key_default_cmp+0x20/0x20
[ 150.717319] keyctl_update_key+0xb0/0xe0
[ 150.718066] SyS_keyctl+0x109/0x130
[ 150.718663] entry_SYSCALL_64_fastpath+0x1f/0xc2
[ 150.719440] RIP: 0033:0x7fcbce75ff19
[ 150.719926] RSP: 002b:00007ffd5d167088 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa
[ 150.720918] RAX: ffffffffffffffda RBX: 0000000000404d80 RCX: 00007fcbce75ff19
[ 150.721874] RDX: 00007ffd5d16785e RSI: 000000002866cd36 RDI: 0000000000000002
[ 150.722827] RBP: 0000000000000006 R08: 000000002866cd36 R09: 00007ffd5d16785e
[ 150.723781] R10: 0000000000000004 R11: 0000000000000206 R12: 0000000000404d80
[ 150.724650] R13: 00007ffd5d16784d R14: 00007ffd5d167238 R15: 000000000040411f
[ 150.725447] Code: 83 c4 08 31 c0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 85 ff 74 23 55 48 89 e5 53 48 89 fb <48> 8b 3f e8 06 21 c5 ff 48 8b 7b 08 e8 fd 20 c5 ff 48 89 df e8
[ 150.727489] RIP: asymmetric_key_free_kids+0x12/0x30 RSP: ffffc90000353e58
[ 150.728117] CR2: 0000000000000001
[ 150.728430] ---[ end trace f7f8fe1da2d5ae8d ]---
Fixes: 4d8c0250b841 ("KEYS: Call ->free_preparse() even after ->preparse() returns an error")
Cc: stable@vger.kernel.org # 3.17+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
sys_add_key() and the KEYCTL_UPDATE operation of sys_keyctl() allowed a
NULL payload with nonzero length to be passed to the key type's
->preparse(), ->instantiate(), and/or ->update() methods. Various key
types including asymmetric, cifs.idmap, cifs.spnego, and pkcs7_test did
not handle this case, allowing an unprivileged user to trivially cause a
NULL pointer dereference (kernel oops) if one of these key types was
present. Fix it by doing the copy_from_user() when 'plen' is nonzero
rather than when '_payload' is non-NULL, causing the syscall to fail
with EFAULT as expected when an invalid buffer is specified.
Cc: stable@vger.kernel.org # 2.6.10+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
MACs should, in general, be compared using crypto_memneq() to prevent
timing attacks.
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
The encrypted-keys module was using a single global HMAC transform,
which could be rekeyed by multiple threads concurrently operating on
different keys, causing incorrect HMAC values to be calculated. Fix
this by allocating a new HMAC transform whenever we need to calculate a
HMAC. Also simplify things a bit by allocating the shash_desc's using
SHASH_DESC_ON_STACK() for both the HMAC and unkeyed hashes.
The following script reproduces the bug:
keyctl new_session
keyctl add user master "abcdefghijklmnop" @s
for i in $(seq 2); do
(
set -e
for j in $(seq 1000); do
keyid=$(keyctl add encrypted desc$i "new user:master 25" @s)
datablob="$(keyctl pipe $keyid)"
keyctl unlink $keyid > /dev/null
keyid=$(keyctl add encrypted desc$i "load $datablob" @s)
keyctl unlink $keyid > /dev/null
done
) &
done
Output with bug:
[ 439.691094] encrypted_key: bad hmac (-22)
add_key: Invalid argument
add_key: Invalid argument
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
With the 'encrypted' key type it was possible for userspace to provide a
data blob ending with a master key description shorter than expected,
e.g. 'keyctl add encrypted desc "new x" @s'. When validating such a
master key description, validate_master_desc() could read beyond the end
of the buffer. Fix this by using strncmp() instead of memcmp(). [Also
clean up the code to deduplicate some logic.]
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Since v4.9, the crypto API cannot (normally) be used to encrypt/decrypt
stack buffers because the stack may be virtually mapped. Fix this for
the padding buffers in encrypted-keys by using ZERO_PAGE for the
encryption padding and by allocating a temporary heap buffer for the
decryption padding.
Tested with CONFIG_DEBUG_SG=y:
keyctl new_session
keyctl add user master "abcdefghijklmnop" @s
keyid=$(keyctl add encrypted desc "new user:master 25" @s)
datablob="$(keyctl pipe $keyid)"
keyctl unlink $keyid
keyid=$(keyctl add encrypted desc "load $datablob" @s)
datablob2="$(keyctl pipe $keyid)"
[ "$datablob" = "$datablob2" ] && echo "Success!"
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org # 4.9+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
In join_session_keyring(), if install_session_keyring_to_cred() were to
fail, we would leak the keyring reference, just like in the bug fixed by
commit 23567fd052a9 ("KEYS: Fix keyring ref leak in
join_session_keyring()"). Fortunately this cannot happen currently, but
we really should be more careful. Do this by adding and using a new
error label at which the keyring reference is dropped.
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
get_derived_key()
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
With the new standardized functions, we can replace all ACCESS_ONCE()
calls across relevant security/keyrings/.
ACCESS_ONCE() does not work reliably on non-scalar types. For example
gcc 4.6 and 4.7 might remove the volatile tag for such accesses during
the SRA (scalar replacement of aggregates) step:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58145
Update the new calls regardless of if it is a scalar type, this is
cleaner than having three alternatives.
Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
CONFIG_KEYS_COMPAT is defined in arch-specific Kconfigs and is missing for
several 64-bit architectures : mips, parisc, tile.
At the moment and for those architectures, calling in 32-bit userspace the
keyctl syscall would return an ENOSYS error.
This patch moves the CONFIG_KEYS_COMPAT option to security/keys/Kconfig, to
make sure the compatibility wrapper is registered by default for any 64-bit
architecture as long as it is configured with CONFIG_COMPAT.
[DH: Modified to remove arm64 compat enablement also as requested by Eric
Biggers]
Signed-off-by: Bilal Amarni <bilal.amarni@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
cc: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
|
|
Signed-off-by: John Johansen <john.johansen@canonical.com>
|
|
Virtualize the apparmor policy/ directory so that the current
namespace affects what part of policy is seen. To do this convert to
using apparmorfs for policy namespace files and setup a magic symlink
in the securityfs apparmor dir to access those files.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Reviewed-by: Seth Arnold <seth.arnold@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
|
