Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support
@ 2020-06-26 22:38 Tyler Hicks
  2020-06-26 22:38 ` [PATCH v2 01/11] ima: Have the LSM free its audit rule Tyler Hicks
                   ` (11 more replies)
  0 siblings, 12 replies; 25+ messages in thread
From: Tyler Hicks @ 2020-06-26 22:38 UTC (permalink / raw)
  To: Mimi Zohar, Dmitry Kasatkin
  Cc: James Morris, Serge E . Hallyn, Lakshmi Ramasubramanian,
	Prakhar Srivastava, linux-kernel, linux-integrity,
	linux-security-module, Janne Karhunen, Eric Biederman, kexec,
	Casey Schaufler

This series ultimately extends the supported IMA rule conditionals for
the KEXEC_CMDLINE hook function. As of today, there's an imbalance in
IMA language conditional support for KEXEC_CMDLINE rules in comparison
to KEXEC_KERNEL_CHECK and KEXEC_INITRAMFS_CHECK rules. The KEXEC_CMDLINE
rules do not support *any* conditionals so you cannot have a sequence of
rules like this:

 dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t
 dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t
 dont_measure func=KEXEC_CMDLINE obj_type=foo_t
 measure func=KEXEC_KERNEL_CHECK
 measure func=KEXEC_INITRAMFS_CHECK
 measure func=KEXEC_CMDLINE

Instead, KEXEC_CMDLINE rules can only be measured or not measured and
there's no additional flexibility in today's implementation of the
KEXEC_CMDLINE hook function.

With this series, the above sequence of rules becomes valid and any
calls to kexec_file_load() with a kernel and initramfs inode type of
foo_t will not be measured (that includes the kernel cmdline buffer)
while all other objects given to a kexec_file_load() syscall will be
measured. There's obviously not an inode directly associated with the
kernel cmdline buffer but this patch series ties the inode based
decision making for KEXEC_CMDLINE to the kernel's inode. I think this
will be intuitive to policy authors.

While reading IMA code and preparing to make this change, I realized
that the buffer based hook functions (KEXEC_CMDLINE and KEY_CHECK) are
quite special in comparison to longer standing hook functions. These
buffer based hook functions can only support measure actions and there
are some restrictions on the conditionals that they support. However,
the rule parser isn't enforcing any of those restrictions and IMA policy
authors wouldn't have any immediate way of knowing that the policy that
they wrote is invalid. For example, the sequence of rules above parses
successfully in today's kernel but the
"dont_measure func=KEXEC_CMDLINE ..." rule is incorrectly handled in
ima_match_rules(). The dont_measure rule is *always* considered to be a
match so, surprisingly, no KEXEC_CMDLINE measurements are made.

While making the rule parser more strict, I realized that the parser
does not correctly free all of the allocated memory associated with an
ima_rule_entry when going down some error paths. Invalid policy loaded
by the policy administrator could result in small memory leaks.

I envision patches 1-6 going to stable. The series is ordered in a way
that has all the fixes up front, followed by cleanups, followed by the
feature patch. The breakdown of patches looks like so:

 Memory leak fixes: 1-3
 Parser strictness fixes: 4-6
 Code cleanups made possible by the fixes: 7-10
 Extend KEXEC_CMDLINE rule support: 11

Perhaps the most logical ordering for code review is:

 1, 2, 3, 7, 8, 4, 5, 6, 9, 10, 11

If you'd like me to re-order or split up the series, just let me know.
Thanks for considering these patches!

* Series-wide v2 changes
  - Rebased onto next-integrity-testing
  - Squashed patches 2 and 3 from v1
    + Updated this cover letter to account for changes to patch index
      changes
    + See patch 2 for specific code changes

Tyler

Tyler Hicks (11):
  ima: Have the LSM free its audit rule
  ima: Free the entire rule when deleting a list of rules
  ima: Free the entire rule if it fails to parse
  ima: Fail rule parsing when buffer hook functions have an invalid
    action
  ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an
    invalid cond
  ima: Fail rule parsing when the KEY_CHECK hook is combined with an
    invalid cond
  ima: Shallow copy the args_p member of ima_rule_entry.lsm elements
  ima: Use correct type for the args_p member of ima_rule_entry.lsm
    elements
  ima: Move validation of the keyrings conditional into
    ima_validate_rule()
  ima: Use the common function to detect LSM conditionals in a rule
  ima: Support additional conditionals in the KEXEC_CMDLINE hook
    function

 include/linux/ima.h                          |   4 +-
 kernel/kexec_file.c                          |   2 +-
 security/integrity/ima/ima.h                 |   7 +-
 security/integrity/ima/ima_api.c             |   2 +-
 security/integrity/ima/ima_appraise.c        |   2 +-
 security/integrity/ima/ima_asymmetric_keys.c |   2 +-
 security/integrity/ima/ima_main.c            |  23 ++-
 security/integrity/ima/ima_policy.c          | 161 ++++++++++++++-----
 security/integrity/ima/ima_queue_keys.c      |   2 +-
 9 files changed, 148 insertions(+), 57 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, back to index

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26 22:38 [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 01/11] ima: Have the LSM free its audit rule Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 02/11] ima: Free the entire rule when deleting a list of rules Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 03/11] ima: Free the entire rule if it fails to parse Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 04/11] ima: Fail rule parsing when buffer hook functions have an invalid action Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 05/11] ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an invalid cond Tyler Hicks
2020-06-27 23:40   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 06/11] ima: Fail rule parsing when the KEY_CHECK " Tyler Hicks
2020-06-27 23:39   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 07/11] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 08/11] ima: Use correct type for " Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule() Tyler Hicks
2020-06-27 23:49   ` Lakshmi Ramasubramanian
2020-06-29 14:16     ` Tyler Hicks
2020-06-30 23:07   ` Mimi Zohar
2020-07-02 22:16     ` Tyler Hicks
2020-07-03 14:15       ` Mimi Zohar
2020-07-06 13:18         ` Tyler Hicks
2020-07-07  3:18           ` Mimi Zohar
2020-06-26 22:38 ` [PATCH v2 10/11] ima: Use the common function to detect LSM conditionals in a rule Tyler Hicks
2020-06-26 22:39 ` [PATCH v2 11/11] ima: Support additional conditionals in the KEXEC_CMDLINE hook function Tyler Hicks
2020-06-28  0:03   ` Lakshmi Ramasubramanian
2020-07-01  8:04   ` Dave Young
2020-07-01 14:38     ` Tyler Hicks
2020-07-01  0:29 ` [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Mimi Zohar

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git