On 1/13/22 15:26, Mimi Zohar wrote:
Hi Stefan,
On Tue, 2022-01-04 at 12:04 -0500, Stefan Berger wrote:
From: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Move variables related to the IMA policy into the ima_namespace. This way
the IMA policy of an IMA namespace can be set and displayed using a
front-end like SecurityFS.
Implement ima_ns_from_file() to get the IMA namespace via the user
namespace of the SecurityFS superblock that a file belongs to.
To get the current ima_namespace use get_current_ns() when a function
that is related to a policy rule is called. In other cases where functions
are called due file attribute modifications, use init_ima_ns, since these
functions are related to IMA appraisal and changes to file attributes are
only relevant to the init_ima_ns until IMA namespaces also support IMA
appraisal. In ima_file_free() use init_ima_ns since in this case flags
related to file measurements may be affected, which is not supported in
IMA namespaces, yet.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
Please split this patch into "ima: pass through ima namespace", or some
other name, and "ima: Move policy related variables into
What is supposed to happen in the 'ima: pass through ima namespace'
patch? What part of this patch do you expect to see there and what do
you want to see deferred to a 2nd patch?
ima_namespace". The other option is to combine the "pass through ima
namespace" with the 2nd patch, like Christian's example.
You mean I should merge this patch with the 2nd?
I am a bit confused as to what you are proposing. The first option would
create 2 patches out of this one, the 2nd option would merge this one
with the 2nd patch.
The equivalent of Christian's 2nd patch would be to merge this patch
into the 2nd. So then let's do that?
---
security/integrity/ima/ima.h | 49 ++++---
security/integrity/ima/ima_api.c | 8 +-
security/integrity/ima/ima_appraise.c | 28 ++--
security/integrity/ima/ima_asymmetric_keys.c | 4 +-
security/integrity/ima/ima_fs.c | 16 ++-
security/integrity/ima/ima_init.c | 8 +-
security/integrity/ima/ima_init_ima_ns.c | 6 +
security/integrity/ima/ima_main.c | 83 +++++++----
security/integrity/ima/ima_policy.c | 142 ++++++++++---------
security/integrity/ima/ima_queue_keys.c | 11 +-
10 files changed, 213 insertions(+), 142 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c4af3275f015..0b3dc9425076 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -20,6 +20,7 @@
#include <linux/hash.h>
#include <linux/tpm.h>
#include <linux/audit.h>
+#include <linux/user_namespace.h>
#include <crypto/hash_info.h>
#include "../integrity.h"
@@ -43,9 +44,6 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
#define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
-/* current content of the policy */
-extern int ima_policy_flag;
-
/* bitset of digests algorithms allowed in the setxattr hook */
extern atomic_t ima_setxattr_allowed_hash_algorithms;
@@ -120,6 +118,14 @@ struct ima_kexec_hdr {
};
struct ima_namespace {
+ struct list_head ima_default_rules;
+ /* ns's policy rules */
Thank you for adding comments. Why is the ima_default_rules not
considered "ns's policy rules"? Will this come later or is it limited
to init_ima_ns?
Let me move the comment up.
+ struct list_head ima_policy_rules;
+ struct list_head ima_temp_rules;
+ /* Pointer to ns's current policy */
+ struct list_head __rcu *ima_rules;
Since "Pointer to ns's current policy" only refers to ima_rules, append
it to the variable definition.
Ok, I will move it onto the same line then.
+ /* current content of the policy */
+ int ima_policy_flag;
Similarly here append the comment to the variable definition.
Will do. Thanks.
} __randomize_layout;
extern struct ima_namespace init_ima_ns;
thanks,
Mimi
