2021-12-10 19:48:40

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v6 00/17] ima: Namespace IMA with audit support in IMA-ns

The goal of this series of patches is to start with the namespacing of
IMA and support auditing within an IMA namespace (IMA-ns) as the first
step.

In this series the IMA namespace is piggy backing on the user namespace
and therefore an IMA namespace gets created when a user namespace is
created. The advantage of this is that the user namespace can provide
the keys infrastructure that IMA appraisal support will need later on.

We chose the goal of supporting auditing within an IMA namespace since it
requires the least changes to IMA. Following this series, auditing within
an IMA namespace can be activated by a user running the following lines
that rely on a statically linked busybox to be installed on the host for
execution within the minimal container environment:

mkdir -p rootfs/{bin,mnt,proc}
cp /sbin/busybox rootfs/bin
cp /sbin/busybox rootfs/bin/busybox2
echo >> rootfs/bin/busybox2
PATH=/bin unshare --user --map-root-user --mount-proc --pid --fork \
--root rootfs busybox sh -c \
"busybox mount -t securityfs /mnt /mnt; \
busybox echo 'audit func=BPRM_CHECK mask=MAY_EXEC' > /mnt/ima/policy; \
busybox2 cat /mnt/ima/policy"

[busybox2 is used to demonstrate 2 measurements; see below]

Following the audit log on the host the last line cat'ing the IMA policy
inside the namespace would have been audited. Unfortunately the auditing
line is not distinguishable from one stemming from actions on the host.
The hope here is that Richard Brigg's container id support for auditing
would help resolve the problem.

The following lines added to a suitable IMA policy on the host would
cause the execution of the commands inside the container (by uid 1000)
to be measured and audited as well on the host, thus leading to two
auditing messages for the 'busybox2 cat' above and log entries in IMA's
system log.

echo -e "measure func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
"audit func=BPRM_CHECK mask=MAY_EXEC uid=1000\n" \
> /sys/kernel/security/ima/policy

The goal of supporting measurement and auditing by the host, of actions
occurring within IMA namespaces, is that users, particularly root,
should not be able to evade the host's IMA policy just by spawning
new IMA namespaces, running programs there, and discarding the namespaces
again. This is achieved through 'hierarchical processing' of file
accesses that are evaluated against the policy of the namespace where
the action occurred and against all namespaces' and their policies leading
back to the root IMA namespace (init_ima_ns).

The patch series adds support for a virtualized SecurityFS with a few
new API calls that are used by IMA namespacing. Only the data relevant
to the IMA namespace are shown. The files and directories of other
security subsystems (TPM, evm, Tomoyo, safesetid) are not showing
up when secruityfs is mounted inside a user namespace.

Much of the code leading up to the virtualization of SecurityFS deals
with moving IMA's variables from various files into the IMA namespace
structure called 'ima_namespace'. When it comes to determining the
current IMA namespace I took the approach to get the current IMA
namespace (get_current_ns()) on the top level and pass the pointer all
the way down to those functions that now need access to the ima_namespace
to get to their variables. This later on comes in handy once hierarchical
processing is implemented in this series where we walk the list of
namespaces backwards and again need to pass the pointer into functions.

This patch also introduces usage of CAP_MAC_ADMIN to allow access to the
IMA policy via reduced capabilities. We would again later on use this
capability to allow users to set file extended attributes for IMA appraisal
support.

My tree with these patches is here:

git clone https://github.com/stefanberger/linux-ima-namespaces v5.15+imans.v6.posted

Regards,
Stefan

v6:
- Removed kref and pointer to user_ns in ima_namespace (patch 1)
- Moved only the policy file dentry into ima_namespace; other dentries are on
stack now and can be discarded
- Merged James's patch simplifying securityfs_remove and dropping dget()
- Added patch with Christian's suggestion to tie opened SecurityFS file to
the user/IMA namespace it belongs to
- Passing missing ima_namespace parameter in functions in ima_kexec.c (ppc64)
- Reverted v5's change to patch 4 related to protection of ima_namespace

v5:
- Followed Christian's suggestions on patch 1. Also, reverted increased reference
counter on init_user_ns since ima_ns doesn't take reference to its user_ns.
- No addtional reference is taken on securityfs dentries for user_ns != init_user_ns.
Updated documentation and removed cleanup of dentries on superblock kill.
(patches 12 & 16)
- Moved else branch to earlier patch (patch 11)
- Protect ima_namespace by taking reference on user namespace for delayed work queue.
(patch 4)

v4:
- For consistency moved 'ns = get_current_ns()' to top of functions
- Merge in James's latest SecurityFS patch

v3:
- Further modifications to virtualized SecurityFS following James's posted patch
- Dropping of early teardown for user_namespaces since not needed anymore

v2:
- Folllwed Christian's suggestion to virtualize securitytfs; no more securityfs_ns
- Followed James's advice for late 'population' of securityfs for IMA namespaces
- Squashed 2 patches dealing with capabilities
- Added missing 'depends on USER_NS' to Kconfig
- Added missing 'static' to several functions



Mehmet Kayaalp (2):
ima: Define ns_status for storing namespaced iint data
ima: Namespace audit status flags

Stefan Berger (15):
ima: Add IMA namespace support
ima: Move delayed work queue and variables into ima_namespace
ima: Move IMA's keys queue related variables into ima_namespace
ima: Move policy related variables into ima_namespace
ima: Move ima_htable into ima_namespace
ima: Move measurement list related variables into ima_namespace
ima: Only accept AUDIT rules for IMA non-init_ima_ns namespaces for
now
ima: Implement hierarchical processing of file accesses
securityfs: Only use simple_pin_fs/simple_release_fs for init_user_ns
securityfs: Extend securityfs with namespacing support
ima: Move some IMA policy and filesystem related variables into
ima_namespace
ima: Tie opened SecurityFS files to the IMA namespace it belongs to
ima: Use mac_admin_ns_capable() to check corresponding capability
ima: Move dentry into ima_namespace and others onto stack
ima: Setup securityfs for IMA namespace

include/linux/capability.h | 6 +
include/linux/ima.h | 109 ++++++++++++
include/linux/user_namespace.h | 4 +
init/Kconfig | 13 ++
kernel/user.c | 7 +
kernel/user_namespace.c | 8 +
security/inode.c | 55 +++---
security/integrity/ima/Makefile | 4 +-
security/integrity/ima/ima.h | 145 ++++++++++-----
security/integrity/ima/ima_api.c | 33 ++--
security/integrity/ima/ima_appraise.c | 26 +--
security/integrity/ima/ima_asymmetric_keys.c | 8 +-
security/integrity/ima/ima_fs.c | 178 +++++++++++++------
security/integrity/ima/ima_init.c | 20 ++-
security/integrity/ima/ima_init_ima_ns.c | 71 ++++++++
security/integrity/ima/ima_kexec.c | 15 +-
security/integrity/ima/ima_main.c | 144 ++++++++++-----
security/integrity/ima/ima_ns.c | 98 ++++++++++
security/integrity/ima/ima_ns_status.c | 132 ++++++++++++++
security/integrity/ima/ima_policy.c | 146 ++++++++-------
security/integrity/ima/ima_queue.c | 75 ++++----
security/integrity/ima/ima_queue_keys.c | 73 +++-----
security/integrity/ima/ima_template.c | 4 +-
23 files changed, 1027 insertions(+), 347 deletions(-)
create mode 100644 security/integrity/ima/ima_init_ima_ns.c
create mode 100644 security/integrity/ima/ima_ns.c
create mode 100644 security/integrity/ima/ima_ns_status.c

--
2.31.1



2021-12-10 19:48:43

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v6 03/17] ima: Namespace audit status flags

From: Mehmet Kayaalp <[email protected]>

The iint cache stores whether the file is measured, appraised, audited
etc. This patch moves the IMA_AUDITED flag into the per-namespace
ns_status, enabling IMA audit mechanism to audit the same file each time
it is accessed in a new namespace.

The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
any of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) is not
enabled.

Read and write operations on the iint flags is replaced with function
calls. For reading, iint_flags() returns the bitwise AND of iint->flags
and ns_status->flags. The ns_status flags are masked with
IMA_NS_STATUS_FLAGS (currently only IMA_AUDITED). Similarly
set_iint_flags() only writes the masked portion to the ns_status flags,
while the iint flags is set as before. The ns_status parameter added to
ima_audit_measurement() is used with the above functions to query and
set the ns_status flags.

Signed-off-by: Mehmet Kayaalp <[email protected]>

Changelog:
v2:
* fixed flag calculation in iint_flags()
---
init/Kconfig | 3 +++
security/integrity/ima/ima.h | 23 ++++++++++++++++++++++-
security/integrity/ima/ima_api.c | 8 +++++---
security/integrity/ima/ima_main.c | 25 ++++++++++++++++++-------
security/integrity/ima/ima_ns.c | 20 ++++++++++++++++++++
5 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 27890607e8cb..1e1c49f1d129 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1251,6 +1251,9 @@ config IMA_NS
Allow the creation of IMA namespaces for each user namespace.
Namespaced IMA enables having IMA features work separately
in each IMA namespace.
+ Currently, only the audit status flags are stored in the namespace,
+ which allows the same file to be audited each time it is accessed
+ in a new namespace.

endif # NAMESPACES

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 28896d256e36..dd06e16c4e1c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -282,7 +282,8 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
int pcr, const char *func_data,
bool buf_hash, u8 *digest, size_t digest_len);
void ima_audit_measurement(struct integrity_iint_cache *iint,
- const unsigned char *filename);
+ const unsigned char *filename,
+ struct ns_status *status);
int ima_alloc_init_template(struct ima_event_data *event_data,
struct ima_template_entry **entry,
struct ima_template_desc *template_desc);
@@ -426,6 +427,9 @@ static inline void ima_free_modsig(struct modsig *modsig)
}
#endif /* CONFIG_IMA_APPRAISE_MODSIG */

+#define IMA_NS_STATUS_ACTIONS IMA_AUDIT
+#define IMA_NS_STATUS_FLAGS IMA_AUDITED
+
int ima_ns_init(void);
struct ima_namespace;
int ima_init_namespace(struct ima_namespace *ns);
@@ -436,6 +440,10 @@ struct ns_status *ima_get_ns_status(struct ima_namespace *ns,

void free_ns_status_cache(struct ima_namespace *ns);

+unsigned long iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status);
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status, unsigned long flags);
#else

static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
@@ -444,6 +452,19 @@ static inline struct ns_status *ima_get_ns_status(struct ima_namespace *ns,
return NULL;
}

+static inline unsigned long iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status)
+{
+ return iint->flags;
+}
+
+static inline unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status,
+ unsigned long flags)
+{
+ iint->flags = flags;
+ return flags;
+}
#endif /* CONFIG_IMA_NS */

/* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index a64fb0130b01..8f7bab17b784 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -342,14 +342,16 @@ void ima_store_measurement(struct integrity_iint_cache *iint,
}

void ima_audit_measurement(struct integrity_iint_cache *iint,
- const unsigned char *filename)
+ const unsigned char *filename,
+ struct ns_status *status)
{
struct audit_buffer *ab;
char *hash;
const char *algo_name = hash_algo_name[iint->ima_hash->algo];
int i;
+ unsigned long flags = iint_flags(iint, status);

- if (iint->flags & IMA_AUDITED)
+ if (flags & IMA_AUDITED)
return;

hash = kzalloc((iint->ima_hash->length * 2) + 1, GFP_KERNEL);
@@ -372,7 +374,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
audit_log_task_info(ab);
audit_log_end(ab);

- iint->flags |= IMA_AUDITED;
+ set_iint_flags(iint, status, flags | IMA_AUDITED);
out:
kfree(hash);
return;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 465865412100..4386010a480e 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -202,8 +202,10 @@ static int process_measurement(struct file *file, const struct cred *cred,
u32 secid, char *buf, loff_t size, int mask,
enum ima_hooks func)
{
+ struct ima_namespace *ns = get_current_ns();
struct inode *inode = file_inode(file);
struct integrity_iint_cache *iint = NULL;
+ struct ns_status *status = NULL;
struct ima_template_desc *template_desc = NULL;
char *pathbuf = NULL;
char filename[NAME_MAX];
@@ -216,6 +218,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
bool violation_check;
enum hash_algo hash_algo;
unsigned int allowed_algos = 0;
+ unsigned long flags;

if (!ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;
@@ -244,6 +247,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
iint = integrity_inode_get(inode);
if (!iint)
rc = -ENOMEM;
+
+ if (!rc && (action & IMA_NS_STATUS_ACTIONS)) {
+ status = ima_get_ns_status(ns, inode);
+ if (IS_ERR(status))
+ rc = PTR_ERR(status);
+ }
}

if (!rc && violation_check)
@@ -259,11 +268,13 @@ static int process_measurement(struct file *file, const struct cred *cred,

mutex_lock(&iint->mutex);

+ flags = iint_flags(iint, status);
+
if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags))
/* reset appraisal flags if ima_inode_post_setattr was called */
- iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
- IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
- IMA_ACTION_FLAGS);
+ flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
+ IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
+ IMA_ACTION_FLAGS);

/*
* Re-evaulate the file if either the xattr has changed or the
@@ -274,7 +285,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
!(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) &&
!(action & IMA_FAIL_UNVERIFIABLE_SIGS))) {
- iint->flags &= ~IMA_DONE_MASK;
+ flags &= ~IMA_DONE_MASK;
iint->measured_pcrs = 0;
}

@@ -282,9 +293,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
* (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
* IMA_AUDIT, IMA_AUDITED)
*/
- iint->flags |= action;
+ flags = set_iint_flags(iint, status, flags | action);
action &= IMA_DO_MASK;
- action &= ~((iint->flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);
+ action &= ~((flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1);

/* If target pcr is already measured, unset IMA_MEASURE action */
if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr)))
@@ -359,7 +370,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
&pathname, filename);
}
if (action & IMA_AUDIT)
- ima_audit_measurement(iint, pathname);
+ ima_audit_measurement(iint, pathname, status);

if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO))
rc = 0;
diff --git a/security/integrity/ima/ima_ns.c b/security/integrity/ima/ima_ns.c
index 20472959ec26..472535cd7126 100644
--- a/security/integrity/ima/ima_ns.c
+++ b/security/integrity/ima/ima_ns.c
@@ -62,6 +62,26 @@ void free_ima_ns(struct user_namespace *user_ns)
destroy_ima_ns(ns);
}

+unsigned long iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status)
+{
+ if (!status)
+ return iint->flags;
+
+ return (iint->flags & ~IMA_NS_STATUS_FLAGS) |
+ (status->flags & IMA_NS_STATUS_FLAGS);
+}
+
+unsigned long set_iint_flags(struct integrity_iint_cache *iint,
+ struct ns_status *status, unsigned long flags)
+{
+ iint->flags = flags;
+ if (status)
+ status->flags = flags & IMA_NS_STATUS_FLAGS;
+
+ return flags;
+}
+
static int __init imans_cache_init(void)
{
imans_cachep = KMEM_CACHE(ima_namespace, SLAB_PANIC);
--
2.31.1


2021-12-15 21:16:21

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] ima: Namespace audit status flags

On Fri, 2021-12-10 at 14:47 -0500, Stefan Berger wrote:
> From: Mehmet Kayaalp <[email protected]>
>
> The iint cache stores whether the file is measured, appraised, audited
> etc. This patch moves the IMA_AUDITED flag into the per-namespace
> ns_status, enabling IMA audit mechanism to audit the same file each time
> it is accessed in a new namespace.
>
> The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
> any of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) is not
> enabled.

^none of the ... are enabled.

thanks,

Mimi

>
> Read and write operations on the iint flags is replaced with function
> calls. For reading, iint_flags() returns the bitwise AND of iint->flags
> and ns_status->flags. The ns_status flags are masked with
> IMA_NS_STATUS_FLAGS (currently only IMA_AUDITED). Similarly
> set_iint_flags() only writes the masked portion to the ns_status flags,
> while the iint flags is set as before. The ns_status parameter added to
> ima_audit_measurement() is used with the above functions to query and
> set the ns_status flags.
>


2021-12-16 02:39:07

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] ima: Namespace audit status flags


On 12/15/21 16:15, Mimi Zohar wrote:
> On Fri, 2021-12-10 at 14:47 -0500, Stefan Berger wrote:
>> From: Mehmet Kayaalp <[email protected]>
>>
>> The iint cache stores whether the file is measured, appraised, audited
>> etc. This patch moves the IMA_AUDITED flag into the per-namespace
>> ns_status, enabling IMA audit mechanism to audit the same file each time
>> it is accessed in a new namespace.
>>
>> The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
>> any of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) is not
>> enabled.
> ^none of the ... are enabled.

You want me to rephrase it?


>
> thanks,
>
> Mimi
>
>> Read and write operations on the iint flags is replaced with function
>> calls. For reading, iint_flags() returns the bitwise AND of iint->flags
>> and ns_status->flags. The ns_status flags are masked with
>> IMA_NS_STATUS_FLAGS (currently only IMA_AUDITED). Similarly
>> set_iint_flags() only writes the masked portion to the ns_status flags,
>> while the iint flags is set as before. The ns_status parameter added to
>> ima_audit_measurement() is used with the above functions to query and
>> set the ns_status flags.
>>

2021-12-16 03:55:07

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v6 03/17] ima: Namespace audit status flags

On Wed, 2021-12-15 at 21:38 -0500, Stefan Berger wrote:
> On 12/15/21 16:15, Mimi Zohar wrote:
> > On Fri, 2021-12-10 at 14:47 -0500, Stefan Berger wrote:
> >> From: Mehmet Kayaalp <[email protected]>
> >>
> >> The iint cache stores whether the file is measured, appraised, audited
> >> etc. This patch moves the IMA_AUDITED flag into the per-namespace
> >> ns_status, enabling IMA audit mechanism to audit the same file each time
> >> it is accessed in a new namespace.
> >>
> >> The ns_status is not looked up if the CONFIG_IMA_NS is disabled or if
> >> any of the IMA_NS_STATUS_ACTIONS (currently only IMA_AUDIT) is not
> >> enabled.
> > ^none of the ... are enabled.
>
> You want me to rephrase it?

Yes, please.

thanks,

Mimi