All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V4 1/3] security: Add a cred_getsecid hook
@ 2018-01-03  1:20 ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-integrity
  Cc: Matthew Garrett, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, Casey Schaufler, linux-security-module, Mimi Zohar,
	Dmitry Kasatkin

For IMA purposes, we want to be able to obtain the prepared secid in the
bprm structure before the credentials are committed. Add a cred_getsecid
hook that makes this possible.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module@vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity@vger.kernel.org
---
 include/linux/lsm_hooks.h  |  6 ++++++
 include/linux/security.h   |  1 +
 security/security.c        |  7 +++++++
 security/selinux/hooks.c   |  6 ++++++
 security/smack/smack_lsm.c | 18 ++++++++++++++++++
 5 files changed, 38 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7161d8e7ee79..72932dabbaed 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -554,6 +554,10 @@
  *	@new points to the new credentials.
  *	@old points to the original credentials.
  *	Transfer data from original creds to new creds
+ * @cred_getsecid:
+ *	Retrieve the security identifier of the cred structure @c
+ *	@c contains the credentials, secid will be placed into @secid.
+ *	In case of failure, @secid will be set to zero.
  * @kernel_act_as:
  *	Set the credentials for a kernel service to act as (subjective context).
  *	@new points to the credentials to be modified.
@@ -1541,6 +1545,7 @@ union security_list_options {
 	int (*cred_prepare)(struct cred *new, const struct cred *old,
 				gfp_t gfp);
 	void (*cred_transfer)(struct cred *new, const struct cred *old);
+	void (*cred_getsecid)(const struct cred *c, u32 *secid);
 	int (*kernel_act_as)(struct cred *new, u32 secid);
 	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
 	int (*kernel_module_request)(char *kmod_name);
@@ -1824,6 +1829,7 @@ struct security_hook_heads {
 	struct list_head cred_free;
 	struct list_head cred_prepare;
 	struct list_head cred_transfer;
+	struct list_head cred_getsecid;
 	struct list_head kernel_act_as;
 	struct list_head kernel_create_files_as;
 	struct list_head kernel_read_file;
diff --git a/include/linux/security.h b/include/linux/security.h
index 73f1ef625d40..5cfff15ac378 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -324,6 +324,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
 void security_cred_free(struct cred *cred);
 int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
 void security_transfer_creds(struct cred *new, const struct cred *old);
+void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
diff --git a/security/security.c b/security/security.c
index 1cd8526cb0b7..35cbd75844c2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1005,6 +1005,13 @@ void security_transfer_creds(struct cred *new, const struct cred *old)
 	call_void_hook(cred_transfer, new, old);
 }
 
+void security_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	*secid = 0;
+	call_void_hook(cred_getsecid, c, secid);
+}
+EXPORT_SYMBOL(security_cred_getsecid);
+
 int security_kernel_act_as(struct cred *new, u32 secid)
 {
 	return call_int_hook(kernel_act_as, 0, new, secid);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8644d864e3c1..d3009c027de8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3844,6 +3844,11 @@ static void selinux_cred_transfer(struct cred *new, const struct cred *old)
 	*tsec = *old_tsec;
 }
 
+static void selinux_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	*secid = cred_sid(c);
+}
+
 /*
  * set the security data for a kernel service
  * - all the creation contexts are set to unlabelled
@@ -6479,6 +6484,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(cred_free, selinux_cred_free),
 	LSM_HOOK_INIT(cred_prepare, selinux_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, selinux_cred_transfer),
+	LSM_HOOK_INIT(cred_getsecid, selinux_cred_getsecid),
 	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
 	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 14cc7940b36d..b27327ebb031 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2049,6 +2049,23 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old)
 	/* cbs copy rule list */
 }
 
+/**
+ * smack_cred_getsecid - get the secid corresponding to a creds structure
+ * @c: the object creds
+ * @secid: where to put the result
+ *
+ * Sets the secid to contain a u32 version of the smack label.
+ */
+static void smack_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	struct smack_known *skp;
+
+	rcu_read_lock();
+	skp = smk_of_task(c->security);
+	*secid = skp->smk_secid;
+	rcu_read_unlock();
+}
+
 /**
  * smack_kernel_act_as - Set the subjective context in a set of credentials
  * @new: points to the set of credentials to be modified.
@@ -4727,6 +4744,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(cred_free, smack_cred_free),
 	LSM_HOOK_INIT(cred_prepare, smack_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, smack_cred_transfer),
+	LSM_HOOK_INIT(cred_getsecid, smack_cred_getsecid),
 	LSM_HOOK_INIT(kernel_act_as, smack_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, smack_kernel_create_files_as),
 	LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
-- 
2.15.1.620.gb9897f4670-goog

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

* [PATCH V4 1/3] security: Add a cred_getsecid hook
@ 2018-01-03  1:20 ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-security-module

For IMA purposes, we want to be able to obtain the prepared secid in the
bprm structure before the credentials are committed. Add a cred_getsecid
hook that makes this possible.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Paul Moore <paul@paul-moore.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux at tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module at vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity at vger.kernel.org
---
 include/linux/lsm_hooks.h  |  6 ++++++
 include/linux/security.h   |  1 +
 security/security.c        |  7 +++++++
 security/selinux/hooks.c   |  6 ++++++
 security/smack/smack_lsm.c | 18 ++++++++++++++++++
 5 files changed, 38 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 7161d8e7ee79..72932dabbaed 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -554,6 +554,10 @@
  *	@new points to the new credentials.
  *	@old points to the original credentials.
  *	Transfer data from original creds to new creds
+ * @cred_getsecid:
+ *	Retrieve the security identifier of the cred structure @c
+ *	@c contains the credentials, secid will be placed into @secid.
+ *	In case of failure, @secid will be set to zero.
  * @kernel_act_as:
  *	Set the credentials for a kernel service to act as (subjective context).
  *	@new points to the credentials to be modified.
@@ -1541,6 +1545,7 @@ union security_list_options {
 	int (*cred_prepare)(struct cred *new, const struct cred *old,
 				gfp_t gfp);
 	void (*cred_transfer)(struct cred *new, const struct cred *old);
+	void (*cred_getsecid)(const struct cred *c, u32 *secid);
 	int (*kernel_act_as)(struct cred *new, u32 secid);
 	int (*kernel_create_files_as)(struct cred *new, struct inode *inode);
 	int (*kernel_module_request)(char *kmod_name);
@@ -1824,6 +1829,7 @@ struct security_hook_heads {
 	struct list_head cred_free;
 	struct list_head cred_prepare;
 	struct list_head cred_transfer;
+	struct list_head cred_getsecid;
 	struct list_head kernel_act_as;
 	struct list_head kernel_create_files_as;
 	struct list_head kernel_read_file;
diff --git a/include/linux/security.h b/include/linux/security.h
index 73f1ef625d40..5cfff15ac378 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -324,6 +324,7 @@ int security_cred_alloc_blank(struct cred *cred, gfp_t gfp);
 void security_cred_free(struct cred *cred);
 int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
 void security_transfer_creds(struct cred *new, const struct cred *old);
+void security_cred_getsecid(const struct cred *c, u32 *secid);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
diff --git a/security/security.c b/security/security.c
index 1cd8526cb0b7..35cbd75844c2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1005,6 +1005,13 @@ void security_transfer_creds(struct cred *new, const struct cred *old)
 	call_void_hook(cred_transfer, new, old);
 }
 
+void security_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	*secid = 0;
+	call_void_hook(cred_getsecid, c, secid);
+}
+EXPORT_SYMBOL(security_cred_getsecid);
+
 int security_kernel_act_as(struct cred *new, u32 secid)
 {
 	return call_int_hook(kernel_act_as, 0, new, secid);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 8644d864e3c1..d3009c027de8 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3844,6 +3844,11 @@ static void selinux_cred_transfer(struct cred *new, const struct cred *old)
 	*tsec = *old_tsec;
 }
 
+static void selinux_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	*secid = cred_sid(c);
+}
+
 /*
  * set the security data for a kernel service
  * - all the creation contexts are set to unlabelled
@@ -6479,6 +6484,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(cred_free, selinux_cred_free),
 	LSM_HOOK_INIT(cred_prepare, selinux_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, selinux_cred_transfer),
+	LSM_HOOK_INIT(cred_getsecid, selinux_cred_getsecid),
 	LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as),
 	LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request),
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 14cc7940b36d..b27327ebb031 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2049,6 +2049,23 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old)
 	/* cbs copy rule list */
 }
 
+/**
+ * smack_cred_getsecid - get the secid corresponding to a creds structure
+ * @c: the object creds
+ * @secid: where to put the result
+ *
+ * Sets the secid to contain a u32 version of the smack label.
+ */
+static void smack_cred_getsecid(const struct cred *c, u32 *secid)
+{
+	struct smack_known *skp;
+
+	rcu_read_lock();
+	skp = smk_of_task(c->security);
+	*secid = skp->smk_secid;
+	rcu_read_unlock();
+}
+
 /**
  * smack_kernel_act_as - Set the subjective context in a set of credentials
  * @new: points to the set of credentials to be modified.
@@ -4727,6 +4744,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(cred_free, smack_cred_free),
 	LSM_HOOK_INIT(cred_prepare, smack_cred_prepare),
 	LSM_HOOK_INIT(cred_transfer, smack_cred_transfer),
+	LSM_HOOK_INIT(cred_getsecid, smack_cred_getsecid),
 	LSM_HOOK_INIT(kernel_act_as, smack_kernel_act_as),
 	LSM_HOOK_INIT(kernel_create_files_as, smack_kernel_create_files_as),
 	LSM_HOOK_INIT(task_setpgid, smack_task_setpgid),
-- 
2.15.1.620.gb9897f4670-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03  1:20 ` Matthew Garrett
@ 2018-01-03  1:20   ` Matthew Garrett
  -1 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-integrity
  Cc: Matthew Garrett, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, Casey Schaufler, linux-security-module, Mimi Zohar,
	Dmitry Kasatkin

Right now most of the IMA code is using current->creds, but the LSM
checks are using security_task_getsecid() which ends up looking at
real_creds. Switch to using security_cred_getsecid() in order to make
this consistent.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module@vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity@vger.kernel.org
---
 security/integrity/ima/ima_policy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ee4613fa5840..52951ac445ea 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -249,7 +249,6 @@ static void ima_lsm_update_rules(void)
 static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 			    enum ima_hooks func, int mask)
 {
-	struct task_struct *tsk = current;
 	const struct cred *cred = current_cred();
 	int i;
 
@@ -305,7 +304,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
-			security_task_getsecid(tsk, &sid);
+			security_cred_getsecid(cred, &sid);
 			rc = security_filter_rule_match(sid,
 							rule->lsm[i].type,
 							Audit_equal,
-- 
2.15.1.620.gb9897f4670-goog

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03  1:20   ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-security-module

Right now most of the IMA code is using current->creds, but the LSM
checks are using security_task_getsecid() which ends up looking at
real_creds. Switch to using security_cred_getsecid() in order to make
this consistent.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux at tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module at vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity at vger.kernel.org
---
 security/integrity/ima/ima_policy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index ee4613fa5840..52951ac445ea 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -249,7 +249,6 @@ static void ima_lsm_update_rules(void)
 static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 			    enum ima_hooks func, int mask)
 {
-	struct task_struct *tsk = current;
 	const struct cred *cred = current_cred();
 	int i;
 
@@ -305,7 +304,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
 		case LSM_SUBJ_USER:
 		case LSM_SUBJ_ROLE:
 		case LSM_SUBJ_TYPE:
-			security_task_getsecid(tsk, &sid);
+			security_cred_getsecid(cred, &sid);
 			rc = security_filter_rule_match(sid,
 							rule->lsm[i].type,
 							Audit_equal,
-- 
2.15.1.620.gb9897f4670-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH V4 3/3] IMA: Support using new creds in appraisal policy
  2018-01-03  1:20 ` Matthew Garrett
@ 2018-01-03  1:20   ` Matthew Garrett
  -1 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-integrity
  Cc: Matthew Garrett, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, Casey Schaufler, linux-security-module, Mimi Zohar,
	Dmitry Kasatkin

The existing BPRM_CHECK functionality in IMA validates against the
credentials of the existing process, not any new credentials that the
child process may transition to. Add an additional CREDS_CHECK target
and refactor IMA to pass the appropriate creds structure. In
ima_bprm_check(), check with both the existing process credentials and
the credentials that will be committed when the new process is started.
This will not change behaviour unless the system policy is extended to
include CREDS_CHECK targets - BPRM_CHECK will continue to check the same
credentials that it did previously.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module@vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity@vger.kernel.org
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 security/integrity/iint.c             |  1 +
 security/integrity/ima/ima.h          |  7 ++++---
 security/integrity/ima/ima_api.c      |  8 +++++---
 security/integrity/ima/ima_appraise.c | 10 +++++++++-
 security/integrity/ima/ima_main.c     | 26 +++++++++++++++++---------
 security/integrity/ima/ima_policy.c   | 16 +++++++++++-----
 security/integrity/integrity.h        |  9 +++++++--
 8 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..5dc9eed035fb 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,7 +25,7 @@ Description:
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [permit_directio]
 
-		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
+		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c84e05866052..62846e331a8b 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint)
 	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
 	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
 	iint->ima_read_status = INTEGRITY_UNKNOWN;
+	iint->ima_creds_status = INTEGRITY_UNKNOWN;
 	iint->evm_status = INTEGRITY_UNKNOWN;
 	iint->measured_pcrs = 0;
 	kmem_cache_free(iint_cache, iint);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..0703a96072b5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(FILE_CHECK)		\
 	hook(MMAP_CHECK)		\
 	hook(BPRM_CHECK)		\
+	hook(CREDS_CHECK)		\
 	hook(POST_SETATTR)		\
 	hook(MODULE_CHECK)		\
 	hook(FIRMWARE_CHECK)		\
@@ -191,7 +192,7 @@ enum ima_hooks {
 };
 
 /* LIM API function definitions */
-int ima_get_action(struct inode *inode, int mask,
+int ima_get_action(struct inode *inode, const struct cred *cred, int mask,
 		   enum ima_hooks func, int *pcr);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
@@ -212,8 +213,8 @@ void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 
 /* IMA policy related functions */
-int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr);
+int ima_match_policy(struct inode *inode, const struct cred *cred,
+		     enum ima_hooks func, int mask, int flags, int *pcr);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7e8db0ea4c0..42ccc2dd3c34 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -157,6 +157,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 /**
  * ima_get_action - appraise & measure decision based on policy.
  * @inode: pointer to inode to measure
+ * @cred: pointer to credentials structure to validate
  * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC,
  *        MAY_APPEND)
  * @func: caller identifier
@@ -165,20 +166,21 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
- *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
+ *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
  * Returns IMA_MEASURE, IMA_APPRAISE mask.
  *
  */
-int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr)
+int ima_get_action(struct inode *inode, const struct cred *cred, int mask,
+		   enum ima_hooks func, int *pcr)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE;
 
 	flags &= ima_policy_flag;
 
-	return ima_match_policy(inode, func, mask, flags, pcr);
+	return ima_match_policy(inode, cred, func, mask, flags, pcr);
 }
 
 /*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 65fbcf3c32c7..1b56ee949315 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -53,7 +53,8 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
 	if (!ima_appraise)
 		return 0;
 
-	return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL);
+	return ima_match_policy(inode, current_cred(), func, mask,
+				IMA_APPRAISE, NULL);
 }
 
 static int ima_fix_xattr(struct dentry *dentry,
@@ -86,6 +87,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_mmap_status;
 	case BPRM_CHECK:
 		return iint->ima_bprm_status;
+	case CREDS_CHECK:
+		return iint->ima_creds_status;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return iint->ima_file_status;
@@ -106,6 +109,8 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->ima_bprm_status = status;
 		break;
+	case CREDS_CHECK:
+		iint->ima_creds_status = status;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->ima_file_status = status;
@@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
 		break;
+	case CREDS_CHECK:
+		iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED);
+		break;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 770654694efc..c7db078d6a19 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -159,8 +159,9 @@ void ima_file_free(struct file *file)
 	ima_check_last_writer(iint, inode, file);
 }
 
-static int process_measurement(struct file *file, char *buf, loff_t size,
-			       int mask, enum ima_hooks func, int opened)
+static int process_measurement(struct file *file, const struct cred *cred,
+			       char *buf, loff_t size, int mask,
+			       enum ima_hooks func, int opened)
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;
@@ -182,7 +183,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	 * bitmask based on the appraise/audit/measurement policy.
 	 * Included is the appraise submask.
 	 */
-	action = ima_get_action(inode, mask, func, &pcr);
+	action = ima_get_action(inode, cred, mask, func, &pcr);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
 	if (!action && !violation_check)
@@ -285,8 +286,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	if (file && (prot & PROT_EXEC))
-		return process_measurement(file, NULL, 0, MAY_EXEC,
-					   MMAP_CHECK, 0);
+		return process_measurement(file, current_cred(), NULL, 0,
+					   MAY_EXEC, MMAP_CHECK, 0);
 	return 0;
 }
 
@@ -305,8 +306,14 @@ int ima_file_mmap(struct file *file, unsigned long prot)
  */
 int ima_bprm_check(struct linux_binprm *bprm)
 {
-	return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
-				   BPRM_CHECK, 0);
+	int ret;
+
+	ret = process_measurement(bprm->file, current_cred(), NULL, 0,
+				  MAY_EXEC, BPRM_CHECK, 0);
+	if (ret)
+		return ret;
+	return process_measurement(bprm->file, bprm->cred, NULL, 0,
+				   MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -321,7 +328,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  */
 int ima_file_check(struct file *file, int mask, int opened)
 {
-	return process_measurement(file, NULL, 0,
+	return process_measurement(file, current_cred(), NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
 					   MAY_APPEND), FILE_CHECK, opened);
 }
@@ -420,7 +427,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	}
 
 	func = read_idmap[read_id] ?: FILE_CHECK;
-	return process_measurement(file, buf, size, MAY_READ, func, 0);
+	return process_measurement(file, current_cred(), buf, size, MAY_READ,
+				   func, 0);
 }
 
 static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 52951ac445ea..cd2289daacee 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -247,9 +247,9 @@ static void ima_lsm_update_rules(void)
  * Returns true on rule match, false on failure.
  */
 static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
-			    enum ima_hooks func, int mask)
+			    const struct cred *cred, enum ima_hooks func,
+			    int mask)
 {
-	const struct cred *cred = current_cred();
 	int i;
 
 	if ((rule->flags & IMA_FUNC) &&
@@ -338,6 +338,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 		return IMA_MMAP_APPRAISE;
 	case BPRM_CHECK:
 		return IMA_BPRM_APPRAISE;
+	case CREDS_CHECK:
+		return IMA_CREDS_APPRAISE;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return IMA_FILE_APPRAISE;
@@ -350,6 +352,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 /**
  * ima_match_policy - decision based on LSM and other conditions
  * @inode: pointer to an inode for which the policy decision is being made
+ * @cred: pointer to a credentials structure for which the policy decision is
+ *        being made
  * @func: IMA hook identifier
  * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
  * @pcr: set the pcr to extend
@@ -361,8 +365,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * list when walking it.  Reads are many orders of magnitude more numerous
  * than writes so ima_match_policy() is classical RCU candidate.
  */
-int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr)
+int ima_match_policy(struct inode *inode, const struct cred *cred,
+		     enum ima_hooks func, int mask, int flags, int *pcr)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -373,7 +377,7 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
 		if (!(entry->action & actmask))
 			continue;
 
-		if (!ima_match_rules(entry, inode, func, mask))
+		if (!ima_match_rules(entry, inode, cred, func, mask))
 			continue;
 
 		action |= entry->flags & IMA_ACTION_FLAGS;
@@ -690,6 +694,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
 				entry->func = BPRM_CHECK;
+			else if (strcmp(args[0].from, "CREDS_CHECK") == 0)
+				entry->func = CREDS_CHECK;
 			else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
 				 0)
 				entry->func = KEXEC_KERNEL_CHECK;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e1bf040fb110..d70fd875d62f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -48,10 +48,14 @@
 #define IMA_BPRM_APPRAISED	0x00002000
 #define IMA_READ_APPRAISE	0x00004000
 #define IMA_READ_APPRAISED	0x00008000
+#define IMA_CREDS_APPRAISE	0x00010000
+#define IMA_CREDS_APPRAISED	0x00020000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
-				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
+				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
+				 IMA_CREDS_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
-				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
+				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
+				 IMA_CREDS_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,
@@ -108,6 +112,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_mmap_status:4;
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_read_status:4;
+	enum integrity_status ima_creds_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;
 };
-- 
2.15.1.620.gb9897f4670-goog

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

* [PATCH V4 3/3] IMA: Support using new creds in appraisal policy
@ 2018-01-03  1:20   ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03  1:20 UTC (permalink / raw)
  To: linux-security-module

The existing BPRM_CHECK functionality in IMA validates against the
credentials of the existing process, not any new credentials that the
child process may transition to. Add an additional CREDS_CHECK target
and refactor IMA to pass the appropriate creds structure. In
ima_bprm_check(), check with both the existing process credentials and
the credentials that will be committed when the new process is started.
This will not change behaviour unless the system policy is extended to
include CREDS_CHECK targets - BPRM_CHECK will continue to check the same
credentials that it did previously.

Signed-off-by: Matthew Garrett <mjg59@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Eric Paris <eparis@parisplace.org>
Cc: selinux at tycho.nsa.gov
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: linux-security-module at vger.kernel.org
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: linux-integrity at vger.kernel.org
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 security/integrity/iint.c             |  1 +
 security/integrity/ima/ima.h          |  7 ++++---
 security/integrity/ima/ima_api.c      |  8 +++++---
 security/integrity/ima/ima_appraise.c | 10 +++++++++-
 security/integrity/ima/ima_main.c     | 26 +++++++++++++++++---------
 security/integrity/ima/ima_policy.c   | 16 +++++++++++-----
 security/integrity/integrity.h        |  9 +++++++--
 8 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..5dc9eed035fb 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,7 +25,7 @@ Description:
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[[appraise_type=]] [permit_directio]
 
-		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
+		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c84e05866052..62846e331a8b 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint)
 	iint->ima_mmap_status = INTEGRITY_UNKNOWN;
 	iint->ima_bprm_status = INTEGRITY_UNKNOWN;
 	iint->ima_read_status = INTEGRITY_UNKNOWN;
+	iint->ima_creds_status = INTEGRITY_UNKNOWN;
 	iint->evm_status = INTEGRITY_UNKNOWN;
 	iint->measured_pcrs = 0;
 	kmem_cache_free(iint_cache, iint);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..0703a96072b5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	hook(FILE_CHECK)		\
 	hook(MMAP_CHECK)		\
 	hook(BPRM_CHECK)		\
+	hook(CREDS_CHECK)		\
 	hook(POST_SETATTR)		\
 	hook(MODULE_CHECK)		\
 	hook(FIRMWARE_CHECK)		\
@@ -191,7 +192,7 @@ enum ima_hooks {
 };
 
 /* LIM API function definitions */
-int ima_get_action(struct inode *inode, int mask,
+int ima_get_action(struct inode *inode, const struct cred *cred, int mask,
 		   enum ima_hooks func, int *pcr);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct integrity_iint_cache *iint,
@@ -212,8 +213,8 @@ void ima_free_template_entry(struct ima_template_entry *entry);
 const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
 
 /* IMA policy related functions */
-int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr);
+int ima_match_policy(struct inode *inode, const struct cred *cred,
+		     enum ima_hooks func, int mask, int flags, int *pcr);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c7e8db0ea4c0..42ccc2dd3c34 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -157,6 +157,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
 /**
  * ima_get_action - appraise & measure decision based on policy.
  * @inode: pointer to inode to measure
+ * @cred: pointer to credentials structure to validate
  * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC,
  *        MAY_APPEND)
  * @func: caller identifier
@@ -165,20 +166,21 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
  *	subj,obj, and type: are LSM specific.
- *	func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
+ *	func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
  *	mask: contains the permission mask
  *	fsmagic: hex value
  *
  * Returns IMA_MEASURE, IMA_APPRAISE mask.
  *
  */
-int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr)
+int ima_get_action(struct inode *inode, const struct cred *cred, int mask,
+		   enum ima_hooks func, int *pcr)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE;
 
 	flags &= ima_policy_flag;
 
-	return ima_match_policy(inode, func, mask, flags, pcr);
+	return ima_match_policy(inode, cred, func, mask, flags, pcr);
 }
 
 /*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 65fbcf3c32c7..1b56ee949315 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -53,7 +53,8 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
 	if (!ima_appraise)
 		return 0;
 
-	return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL);
+	return ima_match_policy(inode, current_cred(), func, mask,
+				IMA_APPRAISE, NULL);
 }
 
 static int ima_fix_xattr(struct dentry *dentry,
@@ -86,6 +87,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
 		return iint->ima_mmap_status;
 	case BPRM_CHECK:
 		return iint->ima_bprm_status;
+	case CREDS_CHECK:
+		return iint->ima_creds_status;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return iint->ima_file_status;
@@ -106,6 +109,8 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->ima_bprm_status = status;
 		break;
+	case CREDS_CHECK:
+		iint->ima_creds_status = status;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->ima_file_status = status;
@@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
 	case BPRM_CHECK:
 		iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
 		break;
+	case CREDS_CHECK:
+		iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED);
+		break;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 770654694efc..c7db078d6a19 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -159,8 +159,9 @@ void ima_file_free(struct file *file)
 	ima_check_last_writer(iint, inode, file);
 }
 
-static int process_measurement(struct file *file, char *buf, loff_t size,
-			       int mask, enum ima_hooks func, int opened)
+static int process_measurement(struct file *file, const struct cred *cred,
+			       char *buf, loff_t size, int mask,
+			       enum ima_hooks func, int opened)
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;
@@ -182,7 +183,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	 * bitmask based on the appraise/audit/measurement policy.
 	 * Included is the appraise submask.
 	 */
-	action = ima_get_action(inode, mask, func, &pcr);
+	action = ima_get_action(inode, cred, mask, func, &pcr);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
 			   (ima_policy_flag & IMA_MEASURE));
 	if (!action && !violation_check)
@@ -285,8 +286,8 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	if (file && (prot & PROT_EXEC))
-		return process_measurement(file, NULL, 0, MAY_EXEC,
-					   MMAP_CHECK, 0);
+		return process_measurement(file, current_cred(), NULL, 0,
+					   MAY_EXEC, MMAP_CHECK, 0);
 	return 0;
 }
 
@@ -305,8 +306,14 @@ int ima_file_mmap(struct file *file, unsigned long prot)
  */
 int ima_bprm_check(struct linux_binprm *bprm)
 {
-	return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
-				   BPRM_CHECK, 0);
+	int ret;
+
+	ret = process_measurement(bprm->file, current_cred(), NULL, 0,
+				  MAY_EXEC, BPRM_CHECK, 0);
+	if (ret)
+		return ret;
+	return process_measurement(bprm->file, bprm->cred, NULL, 0,
+				   MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -321,7 +328,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
  */
 int ima_file_check(struct file *file, int mask, int opened)
 {
-	return process_measurement(file, NULL, 0,
+	return process_measurement(file, current_cred(), NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
 					   MAY_APPEND), FILE_CHECK, opened);
 }
@@ -420,7 +427,8 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	}
 
 	func = read_idmap[read_id] ?: FILE_CHECK;
-	return process_measurement(file, buf, size, MAY_READ, func, 0);
+	return process_measurement(file, current_cred(), buf, size, MAY_READ,
+				   func, 0);
 }
 
 static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 52951ac445ea..cd2289daacee 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -247,9 +247,9 @@ static void ima_lsm_update_rules(void)
  * Returns true on rule match, false on failure.
  */
 static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
-			    enum ima_hooks func, int mask)
+			    const struct cred *cred, enum ima_hooks func,
+			    int mask)
 {
-	const struct cred *cred = current_cred();
 	int i;
 
 	if ((rule->flags & IMA_FUNC) &&
@@ -338,6 +338,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 		return IMA_MMAP_APPRAISE;
 	case BPRM_CHECK:
 		return IMA_BPRM_APPRAISE;
+	case CREDS_CHECK:
+		return IMA_CREDS_APPRAISE;
 	case FILE_CHECK:
 	case POST_SETATTR:
 		return IMA_FILE_APPRAISE;
@@ -350,6 +352,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 /**
  * ima_match_policy - decision based on LSM and other conditions
  * @inode: pointer to an inode for which the policy decision is being made
+ * @cred: pointer to a credentials structure for which the policy decision is
+ *        being made
  * @func: IMA hook identifier
  * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
  * @pcr: set the pcr to extend
@@ -361,8 +365,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * list when walking it.  Reads are many orders of magnitude more numerous
  * than writes so ima_match_policy() is classical RCU candidate.
  */
-int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
-		     int flags, int *pcr)
+int ima_match_policy(struct inode *inode, const struct cred *cred,
+		     enum ima_hooks func, int mask, int flags, int *pcr)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -373,7 +377,7 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
 		if (!(entry->action & actmask))
 			continue;
 
-		if (!ima_match_rules(entry, inode, func, mask))
+		if (!ima_match_rules(entry, inode, cred, func, mask))
 			continue;
 
 		action |= entry->flags & IMA_ACTION_FLAGS;
@@ -690,6 +694,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = MMAP_CHECK;
 			else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
 				entry->func = BPRM_CHECK;
+			else if (strcmp(args[0].from, "CREDS_CHECK") == 0)
+				entry->func = CREDS_CHECK;
 			else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
 				 0)
 				entry->func = KEXEC_KERNEL_CHECK;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e1bf040fb110..d70fd875d62f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -48,10 +48,14 @@
 #define IMA_BPRM_APPRAISED	0x00002000
 #define IMA_READ_APPRAISE	0x00004000
 #define IMA_READ_APPRAISED	0x00008000
+#define IMA_CREDS_APPRAISE	0x00010000
+#define IMA_CREDS_APPRAISED	0x00020000
 #define IMA_APPRAISE_SUBMASK	(IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
-				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
+				 IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
+				 IMA_CREDS_APPRAISE)
 #define IMA_APPRAISED_SUBMASK	(IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
-				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
+				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
+				 IMA_CREDS_APPRAISED)
 
 enum evm_ima_xattr_type {
 	IMA_XATTR_DIGEST = 0x01,
@@ -108,6 +112,7 @@ struct integrity_iint_cache {
 	enum integrity_status ima_mmap_status:4;
 	enum integrity_status ima_bprm_status:4;
 	enum integrity_status ima_read_status:4;
+	enum integrity_status ima_creds_status:4;
 	enum integrity_status evm_status:4;
 	struct ima_digest_data *ima_hash;
 };
-- 
2.15.1.620.gb9897f4670-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03  1:20   ` Matthew Garrett
@ 2018-01-03 15:54     ` Casey Schaufler
  -1 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 15:54 UTC (permalink / raw)
  To: Matthew Garrett, linux-integrity
  Cc: Paul Moore, Stephen Smalley, Eric Paris, selinux,
	linux-security-module, Mimi Zohar, Dmitry Kasatkin

On 1/2/2018 5:20 PM, Matthew Garrett wrote:
> Right now most of the IMA code is using current->creds, but the LSM
> checks are using security_task_getsecid() which ends up looking at
> real_creds. Switch to using security_cred_getsecid() in order to make
> this consistent.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Eric Paris <eparis@parisplace.org>
> Cc: selinux@tycho.nsa.gov
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: linux-security-module@vger.kernel.org
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: linux-integrity@vger.kernel.org
> ---
>  security/integrity/ima/ima_policy.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index ee4613fa5840..52951ac445ea 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -249,7 +249,6 @@ static void ima_lsm_update_rules(void)
>  static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  			    enum ima_hooks func, int mask)
>  {
> -	struct task_struct *tsk = current;
>  	const struct cred *cred = current_cred();
>  	int i;
>  
> @@ -305,7 +304,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_SUBJ_USER:
>  		case LSM_SUBJ_ROLE:
>  		case LSM_SUBJ_TYPE:
> -			security_task_getsecid(tsk, &sid);
> +			security_cred_getsecid(cred, &sid);
>  			rc = security_filter_rule_match(sid,

security_filter_rule_match() is security_audit_rule_match() in
sheep's clothing. Using the cred secid in this case, where the
task secid is used elsewhere is going to lead to tears. It's
going to make *me* cry as I work on untangling secids for
stacking/namespaces. I can't predict how else it's going to
bite us, but I'm betting on it.

 

>  							rule->lsm[i].type,
>  							Audit_equal,

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03 15:54     ` Casey Schaufler
  0 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 15:54 UTC (permalink / raw)
  To: linux-security-module

On 1/2/2018 5:20 PM, Matthew Garrett wrote:
> Right now most of the IMA code is using current->creds, but the LSM
> checks are using security_task_getsecid() which ends up looking at
> real_creds. Switch to using security_cred_getsecid() in order to make
> this consistent.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Eric Paris <eparis@parisplace.org>
> Cc: selinux at tycho.nsa.gov
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: linux-security-module at vger.kernel.org
> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
> Cc: linux-integrity at vger.kernel.org
> ---
>  security/integrity/ima/ima_policy.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index ee4613fa5840..52951ac445ea 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -249,7 +249,6 @@ static void ima_lsm_update_rules(void)
>  static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  			    enum ima_hooks func, int mask)
>  {
> -	struct task_struct *tsk = current;
>  	const struct cred *cred = current_cred();
>  	int i;
>  
> @@ -305,7 +304,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
>  		case LSM_SUBJ_USER:
>  		case LSM_SUBJ_ROLE:
>  		case LSM_SUBJ_TYPE:
> -			security_task_getsecid(tsk, &sid);
> +			security_cred_getsecid(cred, &sid);
>  			rc = security_filter_rule_match(sid,

security_filter_rule_match() is security_audit_rule_match() in
sheep's clothing. Using the cred secid in this case, where the
task secid is used elsewhere is going to lead to tears. It's
going to make *me* cry as I work on untangling secids for
stacking/namespaces. I can't predict how else it's going to
bite us, but I'm betting on it.

 

>  							rule->lsm[i].type,
>  							Audit_equal,

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03 15:54     ` Casey Schaufler
@ 2018-01-03 18:11       ` Matthew Garrett
  -1 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03 18:11 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-integrity, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, LSM List, Mimi Zohar, Dmitry Kasatkin

On Wed, Jan 3, 2018 at 7:54 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/2/2018 5:20 PM, Matthew Garrett wrote:
>> Right now most of the IMA code is using current->creds, but the LSM
>> checks are using security_task_getsecid() which ends up looking at
>> real_creds. Switch to using security_cred_getsecid() in order to make
>> this consistent.
> security_filter_rule_match() is security_audit_rule_match() in
> sheep's clothing. Using the cred secid in this case, where the
> task secid is used elsewhere is going to lead to tears. It's
> going to make *me* cry as I work on untangling secids for
> stacking/namespaces. I can't predict how else it's going to
> bite us, but I'm betting on it.

The problem here is that we don't *have* the task secid for one of the
cases I care about. Validating the task secid at execution time gives
us the security context of the spawning process, rather than the
spawned one - by the time it's committed to the task structure, it's
too late to block execution, so all we have is the secid associated
with the creds in the bprm structure. Obviously fixing this in a way
that doesn't break your work is important, so any suggestions on how I
should be fixing this? :)

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03 18:11       ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03 18:11 UTC (permalink / raw)
  To: linux-security-module

On Wed, Jan 3, 2018 at 7:54 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/2/2018 5:20 PM, Matthew Garrett wrote:
>> Right now most of the IMA code is using current->creds, but the LSM
>> checks are using security_task_getsecid() which ends up looking at
>> real_creds. Switch to using security_cred_getsecid() in order to make
>> this consistent.
> security_filter_rule_match() is security_audit_rule_match() in
> sheep's clothing. Using the cred secid in this case, where the
> task secid is used elsewhere is going to lead to tears. It's
> going to make *me* cry as I work on untangling secids for
> stacking/namespaces. I can't predict how else it's going to
> bite us, but I'm betting on it.

The problem here is that we don't *have* the task secid for one of the
cases I care about. Validating the task secid at execution time gives
us the security context of the spawning process, rather than the
spawned one - by the time it's committed to the task structure, it's
too late to block execution, so all we have is the secid associated
with the creds in the bprm structure. Obviously fixing this in a way
that doesn't break your work is important, so any suggestions on how I
should be fixing this? :)
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03 18:11       ` Matthew Garrett
@ 2018-01-03 19:32         ` Casey Schaufler
  -1 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 19:32 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, LSM List, Mimi Zohar, Dmitry Kasatkin

On 1/3/2018 10:11 AM, Matthew Garrett wrote:
> On Wed, Jan 3, 2018 at 7:54 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 1/2/2018 5:20 PM, Matthew Garrett wrote:
>>> Right now most of the IMA code is using current->creds, but the LSM
>>> checks are using security_task_getsecid() which ends up looking at
>>> real_creds. Switch to using security_cred_getsecid() in order to make
>>> this consistent.
>> security_filter_rule_match() is security_audit_rule_match() in
>> sheep's clothing. Using the cred secid in this case, where the
>> task secid is used elsewhere is going to lead to tears. It's
>> going to make *me* cry as I work on untangling secids for
>> stacking/namespaces. I can't predict how else it's going to
>> bite us, but I'm betting on it.
> The problem here is that we don't *have* the task secid for one of the
> cases I care about. Validating the task secid at execution time gives
> us the security context of the spawning process, rather than the
> spawned one - by the time it's committed to the task structure, it's
> too late to block execution, so all we have is the secid associated
> with the creds in the bprm structure. Obviously fixing this in a way
> that doesn't break your work is important, so any suggestions on how I
> should be fixing this? :)

A security module is allowed to manage either or both of
task and cred blobs. How a security module uses secids is
completely up to the module. So far, everyone is using the
secid to be an alias for the secctx, and the task and cred
are treated as (roughly) the same kind of thing. But that's
not guaranteed going forward. I don't know what someone
might want to do that would cause a problem, but people are
amazingly creative.

I'm actually more concerned with the IMA code using the audit
rule matching. There's an assumption that the secid from a
cred and a secid from a task are both acceptable to the audit
system. What if they aren't? It's possible that I'm just
being paranoid, but we're getting too many permutations
(audit/IMA + task/cred) for my liking. 

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03 19:32         ` Casey Schaufler
  0 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 19:32 UTC (permalink / raw)
  To: linux-security-module

On 1/3/2018 10:11 AM, Matthew Garrett wrote:
> On Wed, Jan 3, 2018 at 7:54 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 1/2/2018 5:20 PM, Matthew Garrett wrote:
>>> Right now most of the IMA code is using current->creds, but the LSM
>>> checks are using security_task_getsecid() which ends up looking at
>>> real_creds. Switch to using security_cred_getsecid() in order to make
>>> this consistent.
>> security_filter_rule_match() is security_audit_rule_match() in
>> sheep's clothing. Using the cred secid in this case, where the
>> task secid is used elsewhere is going to lead to tears. It's
>> going to make *me* cry as I work on untangling secids for
>> stacking/namespaces. I can't predict how else it's going to
>> bite us, but I'm betting on it.
> The problem here is that we don't *have* the task secid for one of the
> cases I care about. Validating the task secid at execution time gives
> us the security context of the spawning process, rather than the
> spawned one - by the time it's committed to the task structure, it's
> too late to block execution, so all we have is the secid associated
> with the creds in the bprm structure. Obviously fixing this in a way
> that doesn't break your work is important, so any suggestions on how I
> should be fixing this? :)

A security module is allowed to manage either or both of
task and cred blobs. How a security module uses secids is
completely up to the module. So far, everyone is using the
secid to be an alias for the secctx, and the task and cred
are treated as (roughly) the same kind of thing. But that's
not guaranteed going forward. I don't know what someone
might want to do that would cause a problem, but people are
amazingly creative.

I'm actually more concerned with the IMA code using the audit
rule matching. There's an assumption that the secid from a
cred and a secid from a task are both acceptable to the audit
system. What if they aren't? It's possible that I'm just
being paranoid, but we're getting too many permutations
(audit/IMA + task/cred) for my liking. 


--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03 19:32         ` Casey Schaufler
@ 2018-01-03 19:44           ` Matthew Garrett
  -1 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03 19:44 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-integrity, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, LSM List, Mimi Zohar, Dmitry Kasatkin

On Wed, Jan 3, 2018 at 11:32 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/3/2018 10:11 AM, Matthew Garrett wrote:
>> The problem here is that we don't *have* the task secid for one of the
>> cases I care about. Validating the task secid at execution time gives
>> us the security context of the spawning process, rather than the
>> spawned one - by the time it's committed to the task structure, it's
>> too late to block execution, so all we have is the secid associated
>> with the creds in the bprm structure. Obviously fixing this in a way
>> that doesn't break your work is important, so any suggestions on how I
>> should be fixing this? :)
>
> A security module is allowed to manage either or both of
> task and cred blobs. How a security module uses secids is
> completely up to the module. So far, everyone is using the
> secid to be an alias for the secctx, and the task and cred
> are treated as (roughly) the same kind of thing. But that's
> not guaranteed going forward. I don't know what someone
> might want to do that would cause a problem, but people are
> amazingly creative.
>
> I'm actually more concerned with the IMA code using the audit
> rule matching. There's an assumption that the secid from a
> cred and a secid from a task are both acceptable to the audit
> system. What if they aren't? It's possible that I'm just
> being paranoid, but we're getting too many permutations
> (audit/IMA + task/cred) for my liking.

The idea here is that we want to be able to trigger an IMA rule
conditionally based on the LSM context a process is running under at
exec time. The current code does so using the secid of current, which
means we're determining whether the new binary should be measured
based on the security context of the task that's executing it.
However, we want to be able to do so based on the security context of
the task that's being executed (usecase here is that I want to measure
anything that's executed in a privileged security context, but don't
care about anything that's running in an unprivileged context). The
child secid has been calculated and put in the creds that are present
in the bprm structure, but commit_creds() hasn't been called yet and
as a result they're not associated with the task struct. One of the
outcomes of measurement may be to block execution, and unfortunately
by the time commit_creds() has been called it's too late to do so.

If we want to be able to do something conditional on the LSM context
that a process is going to be executed under, *before* commit_creds()
is called, is there an existing way to do so? I can rework this so we
use the task secid for all running processes and the cred secid for
the not-yet-running child process, but I don't know if that's
sufficient to avoid problems in future.

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03 19:44           ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-03 19:44 UTC (permalink / raw)
  To: linux-security-module

On Wed, Jan 3, 2018 at 11:32 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/3/2018 10:11 AM, Matthew Garrett wrote:
>> The problem here is that we don't *have* the task secid for one of the
>> cases I care about. Validating the task secid at execution time gives
>> us the security context of the spawning process, rather than the
>> spawned one - by the time it's committed to the task structure, it's
>> too late to block execution, so all we have is the secid associated
>> with the creds in the bprm structure. Obviously fixing this in a way
>> that doesn't break your work is important, so any suggestions on how I
>> should be fixing this? :)
>
> A security module is allowed to manage either or both of
> task and cred blobs. How a security module uses secids is
> completely up to the module. So far, everyone is using the
> secid to be an alias for the secctx, and the task and cred
> are treated as (roughly) the same kind of thing. But that's
> not guaranteed going forward. I don't know what someone
> might want to do that would cause a problem, but people are
> amazingly creative.
>
> I'm actually more concerned with the IMA code using the audit
> rule matching. There's an assumption that the secid from a
> cred and a secid from a task are both acceptable to the audit
> system. What if they aren't? It's possible that I'm just
> being paranoid, but we're getting too many permutations
> (audit/IMA + task/cred) for my liking.

The idea here is that we want to be able to trigger an IMA rule
conditionally based on the LSM context a process is running under at
exec time. The current code does so using the secid of current, which
means we're determining whether the new binary should be measured
based on the security context of the task that's executing it.
However, we want to be able to do so based on the security context of
the task that's being executed (usecase here is that I want to measure
anything that's executed in a privileged security context, but don't
care about anything that's running in an unprivileged context). The
child secid has been calculated and put in the creds that are present
in the bprm structure, but commit_creds() hasn't been called yet and
as a result they're not associated with the task struct. One of the
outcomes of measurement may be to block execution, and unfortunately
by the time commit_creds() has been called it's too late to do so.

If we want to be able to do something conditional on the LSM context
that a process is going to be executed under, *before* commit_creds()
is called, is there an existing way to do so? I can rework this so we
use the task secid for all running processes and the cred secid for
the not-yet-running child process, but I don't know if that's
sufficient to avoid problems in future.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03 19:44           ` Matthew Garrett
@ 2018-01-03 20:08             ` Casey Schaufler
  -1 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 20:08 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-integrity, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, LSM List, Mimi Zohar, Dmitry Kasatkin

On 1/3/2018 11:44 AM, Matthew Garrett wrote:
> On Wed, Jan 3, 2018 at 11:32 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 1/3/2018 10:11 AM, Matthew Garrett wrote:
>>> The problem here is that we don't *have* the task secid for one of the
>>> cases I care about. Validating the task secid at execution time gives
>>> us the security context of the spawning process, rather than the
>>> spawned one - by the time it's committed to the task structure, it's
>>> too late to block execution, so all we have is the secid associated
>>> with the creds in the bprm structure. Obviously fixing this in a way
>>> that doesn't break your work is important, so any suggestions on how I
>>> should be fixing this? :)
>> A security module is allowed to manage either or both of
>> task and cred blobs. How a security module uses secids is
>> completely up to the module. So far, everyone is using the
>> secid to be an alias for the secctx, and the task and cred
>> are treated as (roughly) the same kind of thing. But that's
>> not guaranteed going forward. I don't know what someone
>> might want to do that would cause a problem, but people are
>> amazingly creative.
>>
>> I'm actually more concerned with the IMA code using the audit
>> rule matching. There's an assumption that the secid from a
>> cred and a secid from a task are both acceptable to the audit
>> system. What if they aren't? It's possible that I'm just
>> being paranoid, but we're getting too many permutations
>> (audit/IMA + task/cred) for my liking.
> The idea here is that we want to be able to trigger an IMA rule
> conditionally based on the LSM context a process is running under at
> exec time. The current code does so using the secid of current, which
> means we're determining whether the new binary should be measured
> based on the security context of the task that's executing it.
> However, we want to be able to do so based on the security context of
> the task that's being executed (usecase here is that I want to measure
> anything that's executed in a privileged security context, but don't
> care about anything that's running in an unprivileged context). The
> child secid has been calculated and put in the creds that are present
> in the bprm structure, but commit_creds() hasn't been called yet and
> as a result they're not associated with the task struct. One of the
> outcomes of measurement may be to block execution, and unfortunately
> by the time commit_creds() has been called it's too late to do so.
>
> If we want to be able to do something conditional on the LSM context
> that a process is going to be executed under, *before* commit_creds()
> is called, is there an existing way to do so? I can rework this so we
> use the task secid for all running processes and the cred secid for
> the not-yet-running child process, but I don't know if that's
> sufficient to avoid problems in future.

It's possible that converting all the existing calls of
security_task_getsecid() to security_cred_getsecid() is the
safe approach. No one is using the task blob today, and this
would disambiguate the situation.

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-03 20:08             ` Casey Schaufler
  0 siblings, 0 replies; 18+ messages in thread
From: Casey Schaufler @ 2018-01-03 20:08 UTC (permalink / raw)
  To: linux-security-module

On 1/3/2018 11:44 AM, Matthew Garrett wrote:
> On Wed, Jan 3, 2018 at 11:32 AM, Casey Schaufler <casey@schaufler-ca.com> wrote:
>> On 1/3/2018 10:11 AM, Matthew Garrett wrote:
>>> The problem here is that we don't *have* the task secid for one of the
>>> cases I care about. Validating the task secid at execution time gives
>>> us the security context of the spawning process, rather than the
>>> spawned one - by the time it's committed to the task structure, it's
>>> too late to block execution, so all we have is the secid associated
>>> with the creds in the bprm structure. Obviously fixing this in a way
>>> that doesn't break your work is important, so any suggestions on how I
>>> should be fixing this? :)
>> A security module is allowed to manage either or both of
>> task and cred blobs. How a security module uses secids is
>> completely up to the module. So far, everyone is using the
>> secid to be an alias for the secctx, and the task and cred
>> are treated as (roughly) the same kind of thing. But that's
>> not guaranteed going forward. I don't know what someone
>> might want to do that would cause a problem, but people are
>> amazingly creative.
>>
>> I'm actually more concerned with the IMA code using the audit
>> rule matching. There's an assumption that the secid from a
>> cred and a secid from a task are both acceptable to the audit
>> system. What if they aren't? It's possible that I'm just
>> being paranoid, but we're getting too many permutations
>> (audit/IMA + task/cred) for my liking.
> The idea here is that we want to be able to trigger an IMA rule
> conditionally based on the LSM context a process is running under at
> exec time. The current code does so using the secid of current, which
> means we're determining whether the new binary should be measured
> based on the security context of the task that's executing it.
> However, we want to be able to do so based on the security context of
> the task that's being executed (usecase here is that I want to measure
> anything that's executed in a privileged security context, but don't
> care about anything that's running in an unprivileged context). The
> child secid has been calculated and put in the creds that are present
> in the bprm structure, but commit_creds() hasn't been called yet and
> as a result they're not associated with the task struct. One of the
> outcomes of measurement may be to block execution, and unfortunately
> by the time commit_creds() has been called it's too late to do so.
>
> If we want to be able to do something conditional on the LSM context
> that a process is going to be executed under, *before* commit_creds()
> is called, is there an existing way to do so? I can rework this so we
> use the task secid for all running processes and the cred secid for
> the not-yet-running child process, but I don't know if that's
> sufficient to avoid problems in future.

It's possible that converting all the existing calls of
security_task_getsecid() to security_cred_getsecid() is the
safe approach. No one is using the task blob today, and this
would disambiguate the situation.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH V4 2/3] IMA: Use consistent creds
  2018-01-03 20:08             ` Casey Schaufler
@ 2018-01-04 19:17               ` Matthew Garrett
  -1 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-04 19:17 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: linux-integrity, Paul Moore, Stephen Smalley, Eric Paris,
	selinux, LSM List, Mimi Zohar, Dmitry Kasatkin

On Wed, Jan 3, 2018 at 12:08 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/3/2018 11:44 AM, Matthew Garrett wrote:
>> If we want to be able to do something conditional on the LSM context
>> that a process is going to be executed under, *before* commit_creds()
>> is called, is there an existing way to do so? I can rework this so we
>> use the task secid for all running processes and the cred secid for
>> the not-yet-running child process, but I don't know if that's
>> sufficient to avoid problems in future.
>
> It's possible that converting all the existing calls of
> security_task_getsecid() to security_cred_getsecid() is the
> safe approach. No one is using the task blob today, and this
> would disambiguate the situation.

Ok. Should we be looking at creds or real_creds?

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

* [PATCH V4 2/3] IMA: Use consistent creds
@ 2018-01-04 19:17               ` Matthew Garrett
  0 siblings, 0 replies; 18+ messages in thread
From: Matthew Garrett @ 2018-01-04 19:17 UTC (permalink / raw)
  To: linux-security-module

On Wed, Jan 3, 2018 at 12:08 PM, Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 1/3/2018 11:44 AM, Matthew Garrett wrote:
>> If we want to be able to do something conditional on the LSM context
>> that a process is going to be executed under, *before* commit_creds()
>> is called, is there an existing way to do so? I can rework this so we
>> use the task secid for all running processes and the cred secid for
>> the not-yet-running child process, but I don't know if that's
>> sufficient to avoid problems in future.
>
> It's possible that converting all the existing calls of
> security_task_getsecid() to security_cred_getsecid() is the
> safe approach. No one is using the task blob today, and this
> would disambiguate the situation.

Ok. Should we be looking at creds or real_creds?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2018-01-04 19:22 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-03  1:20 [PATCH V4 1/3] security: Add a cred_getsecid hook Matthew Garrett
2018-01-03  1:20 ` Matthew Garrett
2018-01-03  1:20 ` [PATCH V4 2/3] IMA: Use consistent creds Matthew Garrett
2018-01-03  1:20   ` Matthew Garrett
2018-01-03 15:54   ` Casey Schaufler
2018-01-03 15:54     ` Casey Schaufler
2018-01-03 18:11     ` Matthew Garrett
2018-01-03 18:11       ` Matthew Garrett
2018-01-03 19:32       ` Casey Schaufler
2018-01-03 19:32         ` Casey Schaufler
2018-01-03 19:44         ` Matthew Garrett
2018-01-03 19:44           ` Matthew Garrett
2018-01-03 20:08           ` Casey Schaufler
2018-01-03 20:08             ` Casey Schaufler
2018-01-04 19:17             ` Matthew Garrett
2018-01-04 19:17               ` Matthew Garrett
2018-01-03  1:20 ` [PATCH V4 3/3] IMA: Support using new creds in appraisal policy Matthew Garrett
2018-01-03  1:20   ` Matthew Garrett

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.