summaryrefslogtreecommitdiffstats
path: root/security (follow)
Commit message (Collapse)AuthorAgeFilesLines
* KEYS: request_key() should reget expired keys rather than give EKEYEXPIREDDavid Howells2014-12-013-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since the keyring facility can be viewed as a cache (at least in some applications), the local expiration time on the key should probably be viewed as a 'needs updating after this time' property rather than an absolute 'anyone now wanting to use this object is out of luck' property. Since request_key() is the main interface for the usage of keys, this should update or replace an expired key rather than issuing EKEYEXPIRED if the local expiration has been reached (ie. it should refresh the cache). For absolute conditions where refreshing the cache probably doesn't help, the key can be negatively instantiated using KEYCTL_REJECT_KEY with EKEYEXPIRED given as the error to issue. This will still cause request_key() to return EKEYEXPIRED as that was explicitly set. In the future, if the key type has an update op available, we might want to upcall with the expired key and allow the upcall to update it. We would pass a different operation name (the first column in /etc/request-key.conf) to the request-key program. request_key() returning EKEYEXPIRED is causing an NFS problem which Chuck Lever describes thusly: After about 10 minutes, my NFSv4 functional tests fail because the ownership of the test files goes to "-2". Looking at /proc/keys shows that the id_resolv keys that map to my test user ID have expired. The ownership problem persists until the expired keys are purged from the keyring, and fresh keys are obtained. I bisected the problem to 3.13 commit b2a4df200d57 ("KEYS: Expand the capacity of a keyring"). This commit inadvertantly changes the API contract of the internal function keyring_search_aux(). The root cause appears to be that b2a4df200d57 made "no state check" the default behavior. "No state check" means the keyring search iterator function skips checking the key's expiry timeout, and returns expired keys. request_key_and_link() depends on getting an -EAGAIN result code to know when to perform an upcall to refresh an expired key. This patch can be tested directly by: keyctl request2 user debug:fred a @s keyctl timeout %user:debug:fred 3 sleep 4 keyctl request2 user debug:fred a @s Without the patch, the last command gives error EKEYEXPIRED, but with the command it gives a new key. Reported-by: Carl Hetherington <cth@carlh.net> Reported-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Chuck Lever <chuck.lever@oracle.com>
* KEYS: Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flagsDavid Howells2014-12-013-3/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Simplify KEYRING_SEARCH_{NO,DO}_STATE_CHECK flags to be two variations of the same flag. They are effectively mutually exclusive and one or the other should be provided, but not both. Keyring cycle detection and key possession determination are the only things that set NO_STATE_CHECK, except that neither flag really does anything there because neither purpose makes use of the keyring_search_iterator() function, but rather provides their own. For cycle detection we definitely want to check inside of expired keyrings, just so that we don't create a cycle we can't get rid of. Revoked keyrings are cleared at revocation time and can't then be reused, so shouldn't be a problem either way. For possession determination, we *might* want to validate each keyring before searching it: do you possess a key that's hidden behind an expired or just plain inaccessible keyring? Currently, the answer is yes. Note that you cannot, however, possess a key behind a revoked keyring because they are cleared on revocation. keyring_search() sets DO_STATE_CHECK, which is correct. request_key_and_link() currently doesn't specify whether to check the key state or not - but it should set DO_STATE_CHECK. key_get_instantiation_authkey() also currently doesn't specify whether to check the key state or not - but it probably should also set DO_STATE_CHECK. Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Chuck Lever <chuck.lever@oracle.com>
* KEYS: Fix the size of the key description passed to/from userspaceDavid Howells2014-12-011-30/+26
| | | | | | | | | | | | | | | | | | | | When a key description argument is imported into the kernel from userspace, as happens in add_key(), request_key(), KEYCTL_JOIN_SESSION_KEYRING, KEYCTL_SEARCH, the description is copied into a buffer up to PAGE_SIZE in size. PAGE_SIZE, however, is a variable quantity, depending on the arch. Fix this at 4096 instead (ie. 4095 plus a NUL termination) and define a constant (KEY_MAX_DESC_SIZE) to this end. When reading the description back with KEYCTL_DESCRIBE, a PAGE_SIZE internal buffer is allocated into which the information and description will be rendered. This means that the description will get truncated if an extremely long description it has to be crammed into the buffer with the stringified information. There is no particular need to copy the description into the buffer, so just copy it directly to userspace in a separate operation. Reported-by: Christian Kastner <debian@kvr.at> Signed-off-by: David Howells <dhowells@redhat.com> Tested-by: Christian Kastner <debian@kvr.at>
* Merge branch 'stable-3.18' of git://git.infradead.org/users/pcmoore/selinux ↵James Morris2014-11-131-3/+4
|\ | | | | | | into for-linus
| * selinux: convert WARN_ONCE() to printk() in selinux_nlmsg_perm()Richard Guy Briggs2014-11-121-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | Convert WARN_ONCE() to printk() in selinux_nlmsg_perm(). After conversion from audit_log() in commit e173fb26, WARN_ONCE() was deemed too alarmist, so switch it to printk(). Signed-off-by: Richard Guy Briggs <rgb@redhat.com> [PM: Changed to printk(WARNING) so we catch all of the different invalid netlink messages. In Richard's defense, he brought this point up earlier, but I didn't understand his point at the time.] Signed-off-by: Paul Moore <pmoore@redhat.com>
* | Merge branch 'for-linus' of ↵James Morris2014-10-293-3/+9
|\ \ | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity into for-linus
| * | evm: check xattr value length and type in evm_inode_setxattr()Dmitry Kasatkin2014-10-281-3/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | evm_inode_setxattr() can be called with no value. The function does not check the length so that following command can be used to produce the kernel oops: setfattr -n security.evm FOO. This patch fixes it. Changes in v3: * there is no reason to return different error codes for EVM_XATTR_HMAC and non EVM_XATTR_HMAC. Remove unnecessary test then. Changes in v2: * testing for validity of xattr type [ 1106.396921] BUG: unable to handle kernel NULL pointer dereference at (null) [ 1106.398192] IP: [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48 [ 1106.399244] PGD 29048067 PUD 290d7067 PMD 0 [ 1106.399953] Oops: 0000 [#1] SMP [ 1106.400020] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse [ 1106.400020] CPU: 0 PID: 3635 Comm: setxattr Not tainted 3.16.0-kds+ #2936 [ 1106.400020] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 1106.400020] task: ffff8800291a0000 ti: ffff88002917c000 task.ti: ffff88002917c000 [ 1106.400020] RIP: 0010:[<ffffffff812af7b8>] [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48 [ 1106.400020] RSP: 0018:ffff88002917fd50 EFLAGS: 00010246 [ 1106.400020] RAX: 0000000000000000 RBX: ffff88002917fdf8 RCX: 0000000000000000 [ 1106.400020] RDX: 0000000000000000 RSI: ffffffff818136d3 RDI: ffff88002917fdf8 [ 1106.400020] RBP: ffff88002917fd68 R08: 0000000000000000 R09: 00000000003ec1df [ 1106.400020] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800438a0a00 [ 1106.400020] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 1106.400020] FS: 00007f7dfa7d7740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000 [ 1106.400020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1106.400020] CR2: 0000000000000000 CR3: 000000003763e000 CR4: 00000000000006f0 [ 1106.400020] Stack: [ 1106.400020] ffff8800438a0a00 ffff88002917fdf8 0000000000000000 ffff88002917fd98 [ 1106.400020] ffffffff812a1030 ffff8800438a0a00 ffff88002917fdf8 0000000000000000 [ 1106.400020] 0000000000000000 ffff88002917fde0 ffffffff8116d08a ffff88002917fdc8 [ 1106.400020] Call Trace: [ 1106.400020] [<ffffffff812a1030>] security_inode_setxattr+0x5d/0x6a [ 1106.400020] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f [ 1106.400020] [<ffffffff8116d1e0>] setxattr+0x122/0x16c [ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45 [ 1106.400020] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143 [ 1106.400020] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45 [ 1106.400020] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f [ 1106.400020] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0 [ 1106.400020] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b [ 1106.400020] Code: c3 0f 1f 44 00 00 55 48 89 e5 41 55 49 89 d5 41 54 49 89 fc 53 48 89 f3 48 c7 c6 d3 36 81 81 48 89 df e8 18 22 04 00 85 c0 75 07 <41> 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83 [ 1106.400020] RIP [<ffffffff812af7b8>] evm_inode_setxattr+0x2a/0x48 [ 1106.400020] RSP <ffff88002917fd50> [ 1106.400020] CR2: 0000000000000000 [ 1106.428061] ---[ end trace ae08331628ba3050 ]--- Reported-by: Jan Kara <jack@suse.cz> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | ima: check xattr value length and type in the ima_inode_setxattr()Dmitry Kasatkin2014-10-282-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ima_inode_setxattr() can be called with no value. Function does not check the length so that following command can be used to produce kernel oops: setfattr -n security.ima FOO. This patch fixes it. Changes in v3: * for stable reverted "allow setting hash only in fix or log mode" It will be a separate patch. Changes in v2: * testing validity of xattr type * allow setting hash only in fix or log mode (Mimi) [ 261.562522] BUG: unable to handle kernel NULL pointer dereference at (null) [ 261.564109] IP: [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a [ 261.564109] PGD 3112f067 PUD 42965067 PMD 0 [ 261.564109] Oops: 0000 [#1] SMP [ 261.564109] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse [ 261.564109] CPU: 0 PID: 3299 Comm: setxattr Not tainted 3.16.0-kds+ #2924 [ 261.564109] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 261.564109] task: ffff8800428c2430 ti: ffff880042be0000 task.ti: ffff880042be0000 [ 261.564109] RIP: 0010:[<ffffffff812af272>] [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a [ 261.564109] RSP: 0018:ffff880042be3d50 EFLAGS: 00010246 [ 261.564109] RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000015 [ 261.564109] RDX: 0000001500000000 RSI: 0000000000000000 RDI: ffff8800375cc600 [ 261.564109] RBP: ffff880042be3d68 R08: 0000000000000000 R09: 00000000004d6256 [ 261.564109] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88002149ba00 [ 261.564109] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 261.564109] FS: 00007f6c1e219740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000 [ 261.564109] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 261.564109] CR2: 0000000000000000 CR3: 000000003b35a000 CR4: 00000000000006f0 [ 261.564109] Stack: [ 261.564109] ffff88002149ba00 ffff880042be3df8 0000000000000000 ffff880042be3d98 [ 261.564109] ffffffff812a101b ffff88002149ba00 ffff880042be3df8 0000000000000000 [ 261.564109] 0000000000000000 ffff880042be3de0 ffffffff8116d08a ffff880042be3dc8 [ 261.564109] Call Trace: [ 261.564109] [<ffffffff812a101b>] security_inode_setxattr+0x48/0x6a [ 261.564109] [<ffffffff8116d08a>] vfs_setxattr+0x6b/0x9f [ 261.564109] [<ffffffff8116d1e0>] setxattr+0x122/0x16c [ 261.564109] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45 [ 261.564109] [<ffffffff8114d011>] ? __sb_start_write+0x10f/0x143 [ 261.564109] [<ffffffff811687e8>] ? mnt_want_write+0x21/0x45 [ 261.564109] [<ffffffff811687c0>] ? __mnt_want_write+0x48/0x4f [ 261.564109] [<ffffffff8116d3e6>] SyS_setxattr+0x6e/0xb0 [ 261.564109] [<ffffffff81529da9>] system_call_fastpath+0x16/0x1b [ 261.564109] Code: 48 89 f7 48 c7 c6 58 36 81 81 53 31 db e8 73 27 04 00 85 c0 75 28 bf 15 00 00 00 e8 8a a5 d9 ff 84 c0 75 05 83 cb ff eb 15 31 f6 <41> 80 7d 00 03 49 8b 7c 24 68 40 0f 94 c6 e8 e1 f9 ff ff 89 d8 [ 261.564109] RIP [<ffffffff812af272>] ima_inode_setxattr+0x3e/0x5a [ 261.564109] RSP <ffff880042be3d50> [ 261.564109] CR2: 0000000000000000 [ 261.599998] ---[ end trace 39a89a3fc267e652 ]--- Reported-by: Jan Kara <jack@suse.cz> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
* | | Merge branch 'stable-3.18' of git://git.infradead.org/users/pcmoore/selinux ↵James Morris2014-10-161-1/+1
|\ \ \ | | |/ | |/| | | | into for-linus2
| * | selinux: fix inode security list corruptionStephen Smalley2014-10-151-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | sb_finish_set_opts() can race with inode_free_security() when initializing inode security structures for inodes created prior to initial policy load or by the filesystem during ->mount(). This appears to have always been a possible race, but commit 3dc91d4 ("SELinux: Fix possible NULL pointer dereference in selinux_inode_permission()") made it more evident by immediately reusing the unioned list/rcu element of the inode security structure for call_rcu() upon an inode_free_security(). But the underlying issue was already present before that commit as a possible use-after-free of isec. Shivnandan Kumar reported the list corruption and proposed a patch to split the list and rcu elements out of the union as separate fields of the inode_security_struct so that setting the rcu element would not affect the list element. However, this would merely hide the issue and not truly fix the code. This patch instead moves up the deletion of the list entry prior to dropping the sbsec->isec_lock initially. Then, if the inode is dropped subsequently, there will be no further references to the isec. Reported-by: Shivnandan Kumar <shivnandan.k@samsung.com> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Cc: stable@vger.kernel.org Signed-off-by: Paul Moore <pmoore@redhat.com>
* | | security, crypto: LLVMLinux: Remove VLAIS from ima_crypto.cBehan Webster2014-10-141-28/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replaced the use of a Variable Length Array In Struct (VLAIS) with a C99 compliant equivalent. This patch allocates the appropriate amount of memory using a char array using the SHASH_DESC_ON_STACK macro. The new code can be compiled with both gcc and clang. Signed-off-by: Behan Webster <behanw@converseincode.com> Reviewed-by: Mark Charlebois <charlebm@gmail.com> Reviewed-by: Jan-Simon Möller <dl9pf@gmx.de> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Acked-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Cc: tglx@linutronix.de
* | | Merge branch 'next' of ↵Linus Torvalds2014-10-1237-593/+912
|\ \ \ | | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security Pull security subsystem updates from James Morris. Mostly ima, selinux, smack and key handling updates. * 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: (65 commits) integrity: do zero padding of the key id KEYS: output last portion of fingerprint in /proc/keys KEYS: strip 'id:' from ca_keyid KEYS: use swapped SKID for performing partial matching KEYS: Restore partial ID matching functionality for asymmetric keys X.509: If available, use the raw subjKeyId to form the key description KEYS: handle error code encoded in pointer selinux: normalize audit log formatting selinux: cleanup error reporting in selinux_nlmsg_perm() KEYS: Check hex2bin()'s return when generating an asymmetric key ID ima: detect violations for mmaped files ima: fix race condition on ima_rdwr_violation_check and process_measurement ima: added ima_policy_flag variable ima: return an error code from ima_add_boot_aggregate() ima: provide 'ima_appraise=log' kernel option ima: move keyring initialization to ima_init() PKCS#7: Handle PKCS#7 messages that contain no X.509 certs PKCS#7: Better handling of unsupported crypto KEYS: Overhaul key identification when searching for asymmetric keys KEYS: Implement binary asymmetric key ID handling ...
| * | integrity: do zero padding of the key idDmitry Kasatkin2014-10-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | Latest KEYS code return error if hexadecimal string length id odd. Fix it. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: David Howells <dhowells@redhat.com>
| * | Merge branch 'next' of ↵James Morris2014-10-026-47/+94
| |\ \ | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity into next
| | * | ima: detect violations for mmaped filesRoberto Sassu2014-09-182-9/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes the detection of the 'open_writers' violation for mmaped files. before) an 'open_writers' violation is detected if the policy contains a rule with the criteria: func=FILE_CHECK mask=MAY_READ after) an 'open_writers' violation is detected if the current event matches one of the policy rules. With the old behaviour, the 'open_writers' violation is not detected in the following case: policy: measure func=FILE_MMAP mask=MAY_EXEC steps: 1) open a shared library for writing 2) execute a binary that links that shared library 3) during the binary execution, modify the shared library and save the change result: the 'open_writers' violation measurement is not present in the IMA list. Only binaries executed are protected from writes. For libraries mapped in memory there is the flag MAP_DENYWRITE for this purpose, but according to the output of 'man mmap', the mmap flag is ignored. Since ima_rdwr_violation_check() is now called by process_measurement() the information about if the inode must be measured is already provided by ima_get_action(). Thus the unnecessary function ima_must_measure() has been removed. Changes in v3 (Dmitry Kasatkin): - Violation for MMAP_CHECK function are verified since this patch - Changed patch description a bit Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| | * | ima: fix race condition on ima_rdwr_violation_check and process_measurementRoberto Sassu2014-09-181-21/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch fixes a race condition between two functions that try to access the same inode. Since the i_mutex lock is held and released separately in the two functions, there may be the possibility that a violation is not correctly detected. Suppose there are two processes, A (reader) and B (writer), if the following sequence happens: A: ima_rdwr_violation_check() B: ima_rdwr_violation_check() B: process_measurement() B: starts writing the inode A: process_measurement() the ToMToU violation (a reader may be accessing a content different from that measured, due to a concurrent modification by a writer) will not be detected. To avoid this issue, the violation check and the measurement must be done atomically. This patch fixes the problem by moving the violation check inside process_measurement() when the i_mutex lock is held. Differently from the old code, the violation check is executed also for the MMAP_CHECK hook (other than for FILE_CHECK). This allows to detect ToMToU violations that are possible because shared libraries can be opened for writing while they are in use (according to the output of 'man mmap', the mmap() flag MAP_DENYWRITE is ignored). Changes in v5 (Roberto Sassu): * get iint if action is not zero * exit process_measurement() after the violation check if action is zero * reverse order process_measurement() exit cleanup (Mimi) Changes in v4 (Dmitry Kasatkin): * iint allocation is done before calling ima_rdrw_violation_check() (Suggested-by Mimi) * do not check for violations if the policy does not contain 'measure' rules (done by Roberto Sassu) Changes in v3 (Dmitry Kasatkin): * no violation checking for MMAP_CHECK function in this patch * remove use of filename from violation * removes checking if ima is enabled from ima_rdrw_violation_check * slight style change Suggested-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| | * | ima: added ima_policy_flag variableRoberto Sassu2014-09-174-5/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch introduces the new variable 'ima_policy_flag', whose bits are set depending on the action of the current policy rules. Only the flags IMA_MEASURE, IMA_APPRAISE and IMA_AUDIT are set. The new variable will be used to improve performance by skipping the unnecessary execution of IMA code if the policy does not contain rules with the above actions. Changes in v6 (Roberto Sassu) * do not check 'ima_initialized' before calling ima_update_policy_flag() in ima_update_policy() (suggested by Dmitry) * calling ima_update_policy_flag() moved to init_ima to co-locate with ima_initialized (Dmitry) * add/revise comments (Mimi) Changes in v5 (Roberto Sassu) * reset IMA_APPRAISE flag in 'ima_policy_flag' if 'ima_appraise' is set to zero (reported by Dmitry) * update 'ima_policy_flag' only if IMA initialization is successful (suggested by Mimi and Dmitry) * check 'ima_policy_flag' instead of 'ima_initialized' (suggested by Mimi and Dmitry) Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| | * | ima: return an error code from ima_add_boot_aggregate()Roberto Sassu2014-09-171-6/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch modifies ima_add_boot_aggregate() to return an error code. This way we can determine if all the initialization procedures have been executed successfully. Signed-off-by: Roberto Sassu <roberto.sassu@polito.it> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| | * | ima: provide 'ima_appraise=log' kernel optionDmitry Kasatkin2014-09-172-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The kernel boot parameter "ima_appraise" currently defines 'off', 'enforce' and 'fix' modes. When designing a policy and labeling the system, access to files are either blocked in the default 'enforce' mode or automatically fixed in the 'fix' mode. It is beneficial to be able to run the system in a logging only mode, without fixing it, in order to properly analyze the system. This patch adds a 'log' mode to run the system in a permissive mode and log the appraisal results. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| | * | ima: move keyring initialization to ima_init()Dmitry Kasatkin2014-09-172-8/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ima_init() is used as a single place for all initializations. Experimental keyring patches used the 'late_initcall' which was co-located with the late_initcall(init_ima). When the late_initcall for the keyring initialization was abandoned, initialization moved to init_ima, though it would be more logical to move it to ima_init, where the rest of the initialization is done. This patch moves the keyring initialization to ima_init() as a preparatory step for loading the keys which will be added to ima_init() in following patches. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | | Merge branch 'next' of git://git.infradead.org/users/pcmoore/selinux into nextJames Morris2014-09-305-83/+115
| |\ \ \ | | | |/ | | |/|
| | * | selinux: normalize audit log formattingRichard Guy Briggs2014-09-221-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Restructure to keyword=value pairs without spaces. Drop superfluous words in text. Make invalid_context a keyword. Change result= keyword to seresult=. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> [Minor rewrite to the patch subject line] Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | selinux: cleanup error reporting in selinux_nlmsg_perm()Richard Guy Briggs2014-09-221-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert audit_log() call to WARN_ONCE(). Rename "type=" to nlmsg_type=" to avoid confusion with the audit record type. Added "protocol=" to help track down which protocol (NETLINK_AUDIT?) was used within the netlink protocol family. Signed-off-by: Richard Guy Briggs <rgb@redhat.com> [Rewrote the patch subject line] Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | selinux: make the netif cache namespace awarePaul Moore2014-09-104-36/+46
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While SELinux largely ignores namespaces, for good reason, there are some places where it needs to at least be aware of namespaces in order to function correctly. Network namespaces are one example. Basic awareness of network namespaces are necessary in order to match a network interface's index number to an actual network device. This patch corrects a problem with network interfaces added to a non-init namespace, and can be reproduced with the following commands: [NOTE: the NetLabel configuration is here only to active the dynamic networking controls ] # netlabelctl unlbl add default address:0.0.0.0/0 \ label:system_u:object_r:unlabeled_t:s0 # netlabelctl unlbl add default address:::/0 \ label:system_u:object_r:unlabeled_t:s0 # netlabelctl cipsov4 add pass doi:100 tags:1 # netlabelctl map add domain:lspp_test_netlabel_t \ protocol:cipsov4,100 # ip link add type veth # ip netns add myns # ip link set veth1 netns myns # ip a add dev veth0 10.250.13.100/24 # ip netns exec myns ip a add dev veth1 10.250.13.101/24 # ip l set veth0 up # ip netns exec myns ip l set veth1 up # ping -c 1 10.250.13.101 # ip netns exec myns ping -c 1 10.250.13.100 Reported-by: Jiri Jaburek <jjaburek@redhat.com> Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | selinux: register nf hooks with single nf_register_hooks callJiri Pirko2014-09-091-25/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Push ipv4 and ipv6 nf hooks into single array and register/unregister them via single call. Signed-off-by: Jiri Pirko <jiri@resnulli.us> Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | selinux: fix a problem with IPv6 traffic denials in selinux_ip_postroute()Paul Moore2014-09-031-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A previous commit c0828e50485932b7e019df377a6b0a8d1ebd3080 ("selinux: process labeled IPsec TCP SYN-ACK packets properly in selinux_ip_postroute()") mistakenly left out a 'break' from a switch statement which caused problems with IPv6 traffic. Thanks to Florian Westphal for reporting and debugging the issue. Reported-by: Florian Westphal <fwestpha@redhat.com> Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | selinux: Permit bounded transitions under NO_NEW_PRIVS or NOSUID.Stephen Smalley2014-08-281-12/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the callee SID is bounded by the caller SID, then allowing the transition to occur poses no risk of privilege escalation and we can therefore safely allow the transition to occur. Add this exemption for both the case where a transition was explicitly requested by the application and the case where an automatic transition is defined in policy. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Reviewed-by: Andy Lutomirski <luto@amacapital.net> Signed-off-by: Paul Moore <pmoore@redhat.com>
| | * | Merge tag 'v3.16' into nextPaul Moore2014-08-0527-175/+490
| | |\ \ | | | | | | | | | | | | | | | Linux 3.16
| * | \ \ Merge tag 'keys-next-20140922' of ↵James Morris2014-09-2212-71/+82
| |\ \ \ \ | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs into next
| | * | | | KEYS: Make the key matching functions return boolDavid Howells2014-09-163-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make the key matching functions pointed to by key_match_data::cmp return bool rather than int. Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com>
| | * | | | KEYS: Remove key_type::match in favour of overriding default by match_preparseDavid Howells2014-09-169-23/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A previous patch added a ->match_preparse() method to the key type. This is allowed to override the function called by the iteration algorithm. Therefore, we can just set a default that simply checks for an exact match of the key description with the original criterion data and allow match_preparse to override it as needed. The key_type::match op is then redundant and can be removed, as can the user_match() function. Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com>
| | * | | | KEYS: Remove key_type::def_lookup_typeDavid Howells2014-09-163-9/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove key_type::def_lookup_type as it's no longer used. The information now defaults to KEYRING_SEARCH_LOOKUP_DIRECT but may be overridden by type->match_preparse(). Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com>
| | * | | | KEYS: Preparse match dataDavid Howells2014-09-167-44/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Preparse the match data. This provides several advantages: (1) The preparser can reject invalid criteria up front. (2) The preparser can convert the criteria to binary data if necessary (the asymmetric key type really wants to do binary comparison of the key IDs). (3) The preparser can set the type of search to be performed. This means that it's not then a one-off setting in the key type. (4) The preparser can set an appropriate comparator function. Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com>
| | * | | | Merge tag 'keys-next-fixes-20140916' into keys-nextDavid Howells2014-09-161-2/+2
| | |\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Merge in keyrings fixes for next: (1) Insert some missing 'static' annotations. Signed-off-by: David Howells <dhowells@redhat.com>
| | | * | | | KEYS: Fix missing staticsDavid Howells2014-09-161-2/+2
| | | | |_|/ | | | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix missing statics (found by checker). Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com>
| | * | | | Merge tag 'keys-fixes-20140916' into keys-nextDavid Howells2014-09-161-0/+2
| | |\ \ \ \ | | | |/ / / | | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Merge in keyrings fixes, at least some of which later patches depend on: (1) Reinstate the production of EPERM for key types beginning with '.' in requests from userspace. (2) Tidy up the cleanup of PKCS#7 message signed information blocks and fix a bug this made more obvious. Signed-off-by: David Howells <dhowells@redhat.coM>
| | | * | | KEYS: Reinstate EPERM for a key type name beginning with a '.'David Howells2014-09-161-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Reinstate the generation of EPERM for a key type name beginning with a '.' in a userspace call. Types whose name begins with a '.' are internal only. The test was removed by: commit a4e3b8d79a5c6d40f4a9703abf7fe3abcc6c3b8d Author: Mimi Zohar <zohar@linux.vnet.ibm.com> Date: Thu May 22 14:02:23 2014 -0400 Subject: KEYS: special dot prefixed keyring name bug fix I think we want to keep the restriction on type name so that userspace can't add keys of a special internal type. Note that removal of the test causes several of the tests in the keyutils testsuite to fail. Signed-off-by: David Howells <dhowells@redhat.com> Acked-by: Vivek Goyal <vgoyal@redhat.com> cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | | | | Merge branch 'smack-for-3.18' of git://git.gitorious.org/smack-next/kernel ↵James Morris2014-09-185-274/+520
| |\ \ \ \ \ | | |/ / / / | |/| | | | | | | | | | into next
| | * | | | Make Smack operate on smack_known struct where it still used char*Lukasz Pawelczyk2014-08-294-255/+233
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Smack used to use a mix of smack_known struct and char* throughout its APIs and implementation. This patch unifies the behaviour and makes it store and operate exclusively on smack_known struct pointers when managing labels. Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com> Conflicts: security/smack/smack_access.c security/smack/smack_lsm.c
| | * | | | Fix a bidirectional UDS connect check typoLukasz Pawelczyk2014-08-291-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 54e70ec5eb090193b03e69d551fa6771a5a217c4 commit introduced a bidirectional check that should have checked for mutual WRITE access between two labels. Due to a typo the second check was incorrect. Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
| | * | | | Small fixes in comments describing function parametersLukasz Pawelczyk2014-08-291-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com>
| | * | | | Smack: Bring-up access modeCasey Schaufler2014-08-285-27/+294
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | People keep asking me for permissive mode, and I keep saying "no". Permissive mode is wrong for more reasons than I can enumerate, but the compelling one is that it's once on, never off. Nonetheless, there is an argument to be made for running a process with lots of permissions, logging which are required, and then locking the process down. There wasn't a way to do that with Smack, but this provides it. The notion is that you start out by giving the process an appropriate Smack label, such as "ATBirds". You create rules with a wide range of access and the "b" mode. On Tizen it might be: ATBirds System rwxalb ATBirds User rwxalb ATBirds _ rwxalb User ATBirds wb System ATBirds wb Accesses that fail will generate audit records. Accesses that succeed because of rules marked with a "b" generate log messages identifying the rule, the program and as much object information as is convenient. When the system is properly configured and the programs brought in line with the labeling scheme the "b" mode can be removed from the rules. When the system is ready for production the facility can be configured out. This provides the developer the convenience of permissive mode without creating a system that looks like it is enforcing a policy while it is not. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
| | * | | | Smack: Fix setting label on successful file openMarcin Niesluchowski2014-08-251-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While opening with CAP_MAC_OVERRIDE file label is not set. Other calls may access it after CAP_MAC_OVERRIDE is dropped from process. Signed-off-by: Marcin Niesluchowski <m.niesluchow@samsung.com>
| | * | | | Smack: remove unneeded NULL-termination from securtity labelKonstantin Khlebnikov2014-08-081-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Values of extended attributes are stored as binary blobs. NULL-termination of them isn't required. It just wastes disk space and confuses command-line tools like getfattr because they have to print that zero byte at the end. This patch removes terminating zero byte from initial security label in smack_inode_init_security and cuts it out in function smack_inode_getsecurity which is used by syscall getxattr. This change seems completely safe, because function smk_parse_smack ignores everything after first zero byte. Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
| | * | | | Smack: handle zero-length security labels without panicKonstantin Khlebnikov2014-08-082-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Zero-length security labels are invalid but kernel should handle them. This patch fixes kernel panic after setting zero-length security labels: # attr -S -s "SMACK64" -V "" file And after writing zero-length string into smackfs files syslog and onlycp: # python -c 'import os; os.write(1, "")' > /smack/syslog The problem is caused by brain-damaged logic in function smk_parse_smack() which takes pointer to buffer and its length but if length below or equal zero it thinks that the buffer is zero-terminated. Unfortunately callers of this function are widely used and proper fix requires serious refactoring. Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
| | * | | | Smack: fix behavior of smack_inode_listsecurityKonstantin Khlebnikov2014-08-081-5/+4
| | |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Security operation ->inode_listsecurity is used for generating list of available extended attributes for syscall listxattr. Currently it's used only in nfs4 or if filesystem doesn't provide i_op->listxattr. The list is the set of NULL-terminated names, one after the other. This method must include zero byte at the and into result. Also this function must return length even if string does not fit into output buffer or it is NULL, see similar method in selinux and man listxattr. Signed-off-by: Konstantin Khlebnikov <k.khlebnikov@samsung.com>
| * | | | integrity: make integrity files as 'integrity' moduleDmitry Kasatkin2014-09-091-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The kernel print macros use the KBUILD_MODNAME, which is initialized to the module name. The current integrity/Makefile makes every file as its own module, so pr_xxx messages are prefixed with the file name instead of the module. Similar to the evm/Makefile and ima/Makefile, this patch fixes the integrity/Makefile to use the single name 'integrity'. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | | | integrity: base integrity subsystem kconfig options on integrityDmitry Kasatkin2014-09-093-14/+18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The integrity subsystem has lots of options and takes more than half of the security menu. This patch consolidates the options under "integrity", which are hidden if not enabled. This change does not affect existing configurations. Re-configuration is not needed. Changes v4: - no need to change "integrity subsystem" to menuconfig as options are hidden, when not enabled. (Mimi) - add INTEGRITY Kconfig help description Changes v3: - dependency to INTEGRITY removed when behind 'if INTEGRITY' Changes v2: - previous patch moved integrity out of the 'security' menu. This version keeps integrity as a security option (Mimi). Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | | | integrity: move asymmetric keys config optionDmitry Kasatkin2014-09-091-12/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For better visual appearance it is better to co-locate asymmetric key options together with signature support. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * | | | ima: initialize only required templateDmitry Kasatkin2014-09-091-24/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | IMA uses only one template. This patch initializes only required template to avoid unnecessary memory allocations. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Reviewed-by: Roberto Sassu <roberto.sassu@polito.it> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>