29 Oct, 2014
1 commit
-
…linux-integrity into for-linus
28 Oct, 2014
2 commits
-
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: [] 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:[] [] 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] [] security_inode_setxattr+0x5d/0x6a
[ 1106.400020] [] vfs_setxattr+0x6b/0x9f
[ 1106.400020] [] setxattr+0x122/0x16c
[ 1106.400020] [] ? mnt_want_write+0x21/0x45
[ 1106.400020] [] ? __sb_start_write+0x10f/0x143
[ 1106.400020] [] ? mnt_want_write+0x21/0x45
[ 1106.400020] [] ? __mnt_want_write+0x48/0x4f
[ 1106.400020] [] SyS_setxattr+0x6e/0xb0
[ 1106.400020] [] 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 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83
[ 1106.400020] RIP [] evm_inode_setxattr+0x2a/0x48
[ 1106.400020] RSP
[ 1106.400020] CR2: 0000000000000000
[ 1106.428061] ---[ end trace ae08331628ba3050 ]---Reported-by: Jan Kara
Signed-off-by: Dmitry Kasatkin
Cc: stable@vger.kernel.org
Signed-off-by: Mimi Zohar -
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: [] 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:[] [] 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] [] security_inode_setxattr+0x48/0x6a
[ 261.564109] [] vfs_setxattr+0x6b/0x9f
[ 261.564109] [] setxattr+0x122/0x16c
[ 261.564109] [] ? mnt_want_write+0x21/0x45
[ 261.564109] [] ? __sb_start_write+0x10f/0x143
[ 261.564109] [] ? mnt_want_write+0x21/0x45
[ 261.564109] [] ? __mnt_want_write+0x48/0x4f
[ 261.564109] [] SyS_setxattr+0x6e/0xb0
[ 261.564109] [] 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 80 7d 00 03 49 8b 7c 24 68 40 0f 94 c6 e8 e1 f9 ff ff 89 d8
[ 261.564109] RIP [] ima_inode_setxattr+0x3e/0x5a
[ 261.564109] RSP
[ 261.564109] CR2: 0000000000000000
[ 261.599998] ---[ end trace 39a89a3fc267e652 ]---Reported-by: Jan Kara
Signed-off-by: Dmitry Kasatkin
Cc: stable@vger.kernel.org
Signed-off-by: Mimi Zohar
14 Oct, 2014
1 commit
-
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
Reviewed-by: Mark Charlebois
Reviewed-by: Jan-Simon Möller
Acked-by: Herbert Xu
Acked-by: Dmitry Kasatkin
Cc: tglx@linutronix.de
07 Oct, 2014
1 commit
-
Latest KEYS code return error if hexadecimal string length id odd.
Fix it.Signed-off-by: Dmitry Kasatkin
Signed-off-by: David Howells
18 Sep, 2014
6 commits
-
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_READafter) 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_EXECsteps:
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 changeresult:
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 bitSigned-off-by: Roberto Sassu
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
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 changeSuggested-by: Dmitry Kasatkin
Signed-off-by: Roberto Sassu
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
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
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
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
Signed-off-by: Mimi Zohar -
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
Signed-off-by: Mimi Zohar -
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
Signed-off-by: Mimi Zohar
09 Sep, 2014
15 commits
-
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
Signed-off-by: Mimi Zohar -
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 descriptionChanges 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
Signed-off-by: Mimi Zohar -
For better visual appearance it is better to co-locate
asymmetric key options together with signature support.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
IMA uses only one template. This patch initializes only required
template to avoid unnecessary memory allocations.Signed-off-by: Dmitry Kasatkin
Reviewed-by: Roberto Sassu
Signed-off-by: Mimi Zohar -
In all cases except ima_bprm_check() the filename was not defined
and ima_d_path() was used to find the full path. Unfortunately,
the bprm filename is a relative pathname (eg. .//filename).ima_bprm_check() selects between bprm->interp and bprm->filename.
The following dump demonstrates the differences between using
filename and interp.bprm->filename
filename: ./foo.sh, pathname: /root/bin/foo.sh
filename: ./foo.sh, pathname: /bin/dashbprm->interp
filename: ./foo.sh, pathname: /root/bin/foo.sh
filename: /bin/sh, pathname: /bin/dashIn both cases the pathnames are currently the same. This patch
removes usage of filename and interp in favor of d_absolute_path.Changes v3:
- 11 extra bytes for "deleted" not needed (Mimi)
- purpose "replace relative bprm filename with full pathname" (Mimi)Changes v2:
- use d_absolute_path() instead of d_path to work in chroot environments.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
ima_get_action() sets the "action" flags based on policy.
Before collecting, measuring, appraising, or auditing the
file, the "action" flag is updated based on the cached
iint->flags.This patch removes the subsequent unnecessary appraisal
test in ima_appraise_measurement().Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Add missing keywords to the function definition to cleanup
to discard initialization code.Signed-off-by: Dmitry Kasatkin
Reviewed-by: Roberto Sassu
Signed-off-by: Mimi Zohar -
'function' variable value can be changed instead of
allocating extra '_func' variable.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Precede bit testing before string comparison makes code
faster. Also refactor statement as a single line pointer
assignment. Logic is following: we set 'xattr_ptr' to read
xattr value when we will do appraisal or in any case when
measurement template is other than 'ima'.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Commit f381c27 "integrity: move ima inode integrity data management"
(re)moved few functions but left their declarations in header files.
This patch removes them and also removes duplicated declaration of
integrity_iint_find().Commit c7de7ad "ima: remove unused cleanup functions". This patch
removes these definitions as well.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
If file has IMA signature, IMA in enforce mode, but key is missing
then file access is blocked and single error message is printed.If IMA appraisal is enabled in fix mode, then system runs as usual
but might produce tons of 'Request for unknown key' messages.This patch switches 'pr_warn' to 'pr_err_ratelimited'.
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Empty files and missing xattrs do not guarantee that a file was
just created. This patch passes FILE_CREATED flag to IMA to
reliably identify new files.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
Cc: 3.14+ -
Unless an LSM labels a file during d_instantiate(), newly created
files are not labeled with an initial security.evm xattr, until
the file closes. EVM, before allowing a protected, security xattr
to be written, verifies the existing 'security.evm' value is good.
For newly created files without a security.evm label, this
verification prevents writing any protected, security xattrs,
until the file closes.Following is the example when this happens:
fd = open("foo", O_CREAT | O_WRONLY, 0644);
setxattr("foo", "security.SMACK64", value, sizeof(value), 0);
close(fd);While INTEGRITY_NOXATTRS status is handled in other places, such
as evm_inode_setattr(), it does not handle it in all cases in
evm_protect_xattr(). By limiting the use of INTEGRITY_NOXATTRS to
newly created files, we can now allow setting "protected" xattrs.Changelog:
- limit the use of INTEGRITY_NOXATTRS to IMA identified new filesSigned-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
Cc: 3.14+ -
On ima_file_free(), newly created empty files are not labeled with
an initial security.ima value, because the iversion did not change.
Commit dff6efc "fs: fix iversion handling" introduced a change in
iversion behavior. To verify this change use the shell command:$ (exec >foo)
$ getfattr -h -e hex -d -m security fooThis patch defines the IMA_NEW_FILE flag. The flag is initially
set, when IMA detects that a new file is created, and subsequently
checked on the ima_file_free() hook to set the initial security.ima
value.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
Cc: 3.14+ -
This patch fixes a bug, where evm_verify_hmac() returns INTEGRITY_PASS
if inode->i_op->getxattr() returns an error in evm_find_protected_xattrs.Signed-off-by: Dmitry Kasatkin
03 Sep, 2014
4 commits
-
This patch fixes checkpatch 'return' warnings introduced with commit
9819cf2 "checkpatch: warn on unnecessary void function return statements".Use scripts/checkpatch.pl --file security/integrity/evm/evm_main.c
to produce the warnings.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
3.16 commit aad4f8bb42af06371aa0e85bf0cd9d52c0494985
'switch simple generic_file_aio_read() users to ->read_iter()'
replaced ->aio_read with ->read_iter in most of the file systems
and introduced new_sync_read() as a replacement for do_sync_read().Most of file systems set '->read' and ima_kernel_read is not affected.
When ->read is not set, this patch adopts fallback call changes from the
vfs_read.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
Cc: 3.16+ -
This patch fixes the case where the file's signature/hash xattr contains
an invalid hash algorithm. Although we can not verify the xattr, we still
need to measure the file. Use the default IMA hash algorithm.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
The patch 3bcced39ea7d: "ima: use ahash API for file hash
calculation" from Feb 26, 2014, leads to the following static checker
warning:security/integrity/ima/ima_crypto.c:204 ima_alloc_atfm()
error: buffer overflow 'hash_algo_name' 17
Signed-off-by: Mimi Zohar
Signed-off-by: Dmitry Kasatkin
26 Jul, 2014
1 commit
-
The "security: introduce kernel_fw_from_file hook" patch defined a
new security hook to evaluate any loaded firmware that wasn't built
into the kernel.This patch defines ima_fw_from_file(), which is called from the new
security hook, to measure and/or appraise the loaded firmware's
integrity.Signed-off-by: Mimi Zohar
Signed-off-by: Kees Cook
17 Jul, 2014
7 commits
-
Require all keys added to the IMA keyring be signed by an
existing trusted key on the system trusted keyring.Changelog v6:
- remove ifdef CONFIG_IMA_TRUSTED_KEYRING in C code - Dmitry
- update Kconfig dependency and help
- select KEYS_DEBUG_PROC_KEYS - DmitryChangelog v5:
- Move integrity_init_keyring() to init_ima() - Dmitry
- reset keyring[id] on failure - DmitryChangelog v1:
- don't link IMA trusted keyring to user keyringChangelog:
- define stub integrity_init_keyring() function (reported-by Fengguang Wu)
- differentiate between regular and trusted keyring names.
- replace printk with pr_info (D. Kasatkin)
- only make the IMA keyring a trusted keyring (reported-by D. Kastatkin)
- define stub integrity_init_keyring() definition based on
CONFIG_INTEGRITY_SIGNATURE, not CONFIG_INTEGRITY_ASYMMETRIC_KEYS.
(reported-by Jim Davis)Signed-off-by: Mimi Zohar
Signed-off-by: Dmitry Kasatkin
Acked-by: David Howells -
The asynchronous hash API allows initiating a hash calculation and
then performing other tasks, while waiting for the hash calculation
to complete.This patch introduces usage of double buffering for simultaneous
hashing and reading of the next chunk of data from storage.Changes in v3:
- better commentsSigned-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Use of multiple-page collect buffers reduces:
1) the number of block IO requests
2) the number of asynchronous hash update requestsSecond is important for HW accelerated hashing, because significant
amount of time is spent for preparation of hash update operation,
which includes configuring acceleration HW, DMA engine, etc...
Thus, HW accelerators are more efficient when working on large
chunks of data.This patch introduces usage of multi-page collect buffers. Buffer size
can be specified using 'ahash_bufsize' module parameter. Default buffer
size is 4096 bytes.Changes in v3:
- kernel parameter replaced with module parameterSigned-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Async hash API allows the use of HW acceleration for hash calculation.
It may give significant performance gain and/or reduce power consumption,
which might be very beneficial for battery powered devices.This patch introduces hash calculation using ahash API. ahash performance
depends on the data size and the particular HW. Depending on the specific
system, shash performance may be better.This patch defines 'ahash_minsize' module parameter, which is used to
define the minimal file size to use with ahash. If this minimum file size
is not set or the file is smaller than defined by the parameter, shash will
be used.Changes in v3:
- kernel parameter replaced with module parameter
- pr_crit replaced with pr_crit_ratelimited
- more comment changes - MimiChanges in v2:
- ima_ahash_size became as ima_ahash
- ahash pre-allocation moved out from __init code to be able to use
ahash crypto modules. Ahash allocated once on the first use.
- hash calculation falls back to shash if ahash allocation/calculation fails
- complex initialization separated from variable declaration
- improved commentsSigned-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Replace spaces in op keyword labels in log output since userspace audit tools
can't parse orphaned keywords.Reported-by: Steve Grubb
Signed-off-by: Richard Guy Briggs
Signed-off-by: Mimi Zohar -
process_measurement() always calls ima_template_desc_current(),
including when an IMA policy has not been defined.This patch delays template descriptor lookup until action is
determined.Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar -
Before 2.6.39 inode->i_readcount was maintained by IMA. It was not atomic
and protected using spinlock. For 2.6.39, i_readcount was converted to
atomic and maintaining was moved VFS layer. Spinlock for some unclear
reason was replaced by i_mutex.After analyzing the code, we came to conclusion that i_mutex locking is
unnecessary, especially when an IMA policy has not been defined.This patch removes i_mutex locking from ima_rdwr_violation_check().
Signed-off-by: Dmitry Kasatkin
Signed-off-by: Mimi Zohar
13 Jun, 2014
2 commits
-
Commit 8aac62706 "move exit_task_namespaces() outside of exit_notify"
introduced the kernel opps since the kernel v3.10, which happens when
Apparmor and IMA-appraisal are enabled at the same time.----------------------------------------------------------------------
[ 106.750167] BUG: unable to handle kernel NULL pointer dereference at
0000000000000018
[ 106.750221] IP: [] our_mnt+0x1a/0x30
[ 106.750241] PGD 0
[ 106.750254] Oops: 0000 [#1] SMP
[ 106.750272] Modules linked in: cuse parport_pc ppdev bnep rfcomm
bluetooth rpcsec_gss_krb5 nfsd auth_rpcgss nfs_acl nfs lockd sunrpc
fscache dm_crypt intel_rapl x86_pkg_temp_thermal intel_powerclamp
kvm_intel snd_hda_codec_hdmi kvm crct10dif_pclmul crc32_pclmul
ghash_clmulni_intel aesni_intel aes_x86_64 glue_helper lrw gf128mul
ablk_helper cryptd snd_hda_codec_realtek dcdbas snd_hda_intel
snd_hda_codec snd_hwdep snd_pcm snd_page_alloc snd_seq_midi
snd_seq_midi_event snd_rawmidi psmouse snd_seq microcode serio_raw
snd_timer snd_seq_device snd soundcore video lpc_ich coretemp mac_hid lp
parport mei_me mei nbd hid_generic e1000e usbhid ahci ptp hid libahci
pps_core
[ 106.750658] CPU: 6 PID: 1394 Comm: mysqld Not tainted 3.13.0-rc7-kds+ #15
[ 106.750673] Hardware name: Dell Inc. OptiPlex 9010/0M9KCM, BIOS A08
09/19/2012
[ 106.750689] task: ffff8800de804920 ti: ffff880400fca000 task.ti:
ffff880400fca000
[ 106.750704] RIP: 0010:[] []
our_mnt+0x1a/0x30
[ 106.750725] RSP: 0018:ffff880400fcba60 EFLAGS: 00010286
[ 106.750738] RAX: 0000000000000000 RBX: 0000000000000100 RCX:
ffff8800d51523e7
[ 106.750764] RDX: ffffffffffffffea RSI: ffff880400fcba34 RDI:
ffff880402d20020
[ 106.750791] RBP: ffff880400fcbae0 R08: 0000000000000000 R09:
0000000000000001
[ 106.750817] R10: 0000000000000000 R11: 0000000000000001 R12:
ffff8800d5152300
[ 106.750844] R13: ffff8803eb8df510 R14: ffff880400fcbb28 R15:
ffff8800d51523e7
[ 106.750871] FS: 0000000000000000(0000) GS:ffff88040d200000(0000)
knlGS:0000000000000000
[ 106.750910] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 106.750935] CR2: 0000000000000018 CR3: 0000000001c0e000 CR4:
00000000001407e0
[ 106.750962] Stack:
[ 106.750981] ffffffff813434eb ffff880400fcbb20 ffff880400fcbb18
0000000000000000
[ 106.751037] ffff8800de804920 ffffffff8101b9b9 0001800000000000
0000000000000100
[ 106.751093] 0000010000000000 0000000000000002 000000000000000e
ffff8803eb8df500
[ 106.751149] Call Trace:
[ 106.751172] [] ? aa_path_name+0x2ab/0x430
[ 106.751199] [] ? sched_clock+0x9/0x10
[ 106.751225] [] aa_path_perm+0x7d/0x170
[ 106.751250] [] ? native_sched_clock+0x15/0x80
[ 106.751276] [] aa_file_perm+0x33/0x40
[ 106.751301] [] common_file_perm+0x8e/0xb0
[ 106.751327] [] apparmor_file_permission+0x18/0x20
[ 106.751355] [] security_file_permission+0x23/0xa0
[ 106.751382] [] rw_verify_area+0x52/0xe0
[ 106.751407] [] vfs_read+0x6d/0x170
[ 106.751432] [] kernel_read+0x41/0x60
[ 106.751457] [] ima_calc_file_hash+0x225/0x280
[ 106.751483] [] ? ima_calc_file_hash+0x32/0x280
[ 106.751509] [] ima_collect_measurement+0x9d/0x160
[ 106.751536] [] ? trace_hardirqs_on+0xd/0x10
[ 106.751562] [] ? ima_file_free+0x6c/0xd0
[ 106.751587] [] ima_update_xattr+0x34/0x60
[ 106.751612] [] ima_file_free+0xc0/0xd0
[ 106.751637] [] __fput+0xd5/0x300
[ 106.751662] [] ____fput+0xe/0x10
[ 106.751687] [] task_work_run+0xc4/0xe0
[ 106.751712] [] do_exit+0x2bd/0xa90
[ 106.751738] [] ? retint_swapgs+0x13/0x1b
[ 106.751763] [] do_group_exit+0x4c/0xc0
[ 106.751788] [] SyS_exit_group+0x14/0x20
[ 106.751814] [] system_call_fastpath+0x1a/0x1f
[ 106.751839] Code: c3 0f 1f 44 00 00 55 48 89 e5 e8 22 fe ff ff 5d c3
0f 1f 44 00 00 55 65 48 8b 04 25 c0 c9 00 00 48 8b 80 28 06 00 00 48 89
e5 5d 8b 40 18 48 39 87 c0 00 00 00 0f 94 c0 c3 0f 1f 80 00 00 00
[ 106.752185] RIP [] our_mnt+0x1a/0x30
[ 106.752214] RSP
[ 106.752236] CR2: 0000000000000018
[ 106.752258] ---[ end trace 3c520748b4732721 ]---
----------------------------------------------------------------------The reason for the oops is that IMA-appraisal uses "kernel_read()" when
file is closed. kernel_read() honors LSM security hook which calls
Apparmor handler, which uses current->nsproxy->mnt_ns. The 'guilty'
commit changed the order of cleanup code so that nsproxy->mnt_ns was
not already available for Apparmor.Discussion about the issue with Al Viro and Eric W. Biederman suggested
that kernel_read() is too high-level for IMA. Another issue, except
security checking, that was identified is mandatory locking. kernel_read
honors it as well and it might prevent IMA from calculating necessary hash.
It was suggested to use simplified version of the function without security
and locking checks.This patch introduces special version ima_kernel_read(), which skips security
and mandatory locking checking. It prevents the kernel oops to happen.Signed-off-by: Dmitry Kasatkin
Suggested-by: Eric W. Biederman
Signed-off-by: Mimi Zohar
Cc: -
Calculating the 'security.evm' HMAC value requires access to the
EVM encrypted key. Only the kernel should have access to it. This
patch prevents userspace tools(eg. setfattr, cp --preserve=xattr)
from setting/modifying the 'security.evm' HMAC value directly.Signed-off-by: Mimi Zohar
Cc: