Re: [PATCH V6 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Mon, Jan 22, 2018 at 5:14 PM, Casey Schaufler  wrote:
> On 1/21/2018 9:27 PM, Matthew Garrett wrote:
>> On Tue, Jan 9, 2018 at 8:36 AM, Matthew Garrett  wrote:
>>> 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.
>> Any feedback on this version of the set?
>
>
> Sorry for the delay. I'm having a mindset crisis on secids just
> now, and I'm not completely sure if I have any issue with this
> particular hook. Don't wait for me. If everyone else is OK with
> it, go ahead.

Thanks Casey - Mimi, are you ok with the IMA changes? If so, which
tree should these go through?

Re: [PATCH V6 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Tue, Jan 9, 2018 at 8:36 AM, Matthew Garrett  wrote:
> 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.

Any feedback on this version of the set?

[PATCH V6 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

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.

After this patch, an IMA policy rule along the lines of:

measure func=CREDS_CHECK subj_type=unconfined_t

will trigger if a process is executed and runs as unconfined_t, ignoring
the context of the parent process. This is in contrast to:

measure func=BPRM_CHECK subj_type=unconfined_t

which will trigger if the process that calls exec() is already executing
in unconfined_t, ignoring the context that the child process executes
into.

Signed-off-by: Matthew Garrett 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 security/integrity/iint.c |  1 +
 security/integrity/ima/ima.h  |  9 
 security/integrity/ima/ima_api.c  |  9 +---
 security/integrity/ima/ima_appraise.c | 13 ++-
 security/integrity/ima/ima_main.c | 42 ++-
 security/integrity/ima/ima_policy.c   | 23 ---
 security/integrity/integrity.h|  9 ++--
 8 files changed, 79 insertions(+), 29 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..35fe91aa1fc9 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,8 +192,8 @@ enum ima_hooks {
 };
 
 /* LIM API function definitions */
-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, u32 secid,
+  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,
struct file *file, void *buf, loff_t size,
@@ -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, u32 secid,
+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 

[PATCH V6 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

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 
Acked-by: Paul Moore 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@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 

Re: [PATCH V5 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Mon, Jan 8, 2018 at 4:18 AM, Mimi Zohar  wrote:
> On Fri, 2018-01-05 at 13:15 -0800, Matthew Garrett wrote:
>> 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.
>
> Refactoring IMA to pass the creds structure all the way down is a
> generic solution, but if the CREDS_CHECK rule is only being called
> from ima_bprm_check(), "container_of" the bprm->file returns a pointer
> to the bprm structure.  Perhaps you could limit the amount of
> refactoring needed based on the func.

Hm. This would avoid adding an argument to process_measurement(), but
we'd still need to pass additional information down through
ima_get_action() in order to get the creds and secid right. It feels a
little ugly to have process_measurement() recreate information rather
than having the caller pass it in, but I'm not going to object.

> Could you include in the patch description a simple method for testing
> this change?

Certainly.

[PATCH V5 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

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 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---
 Documentation/ABI/testing/ima_policy  |  2 +-
 security/integrity/iint.c |  1 +
 security/integrity/ima/ima.h  |  9 
 security/integrity/ima/ima_api.c  |  9 +---
 security/integrity/ima/ima_appraise.c | 13 ++-
 security/integrity/ima/ima_main.c | 42 ++-
 security/integrity/ima/ima_policy.c   | 23 ---
 security/integrity/integrity.h|  9 ++--
 8 files changed, 79 insertions(+), 29 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..35fe91aa1fc9 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,8 +192,8 @@ enum ima_hooks {
 };
 
 /* LIM API function definitions */
-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, u32 secid,
+  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,
struct file *file, void *buf, loff_t size,
@@ -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, u32 secid,
+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..09a8da070162 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -157,6 +157,8 @@ 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
+ * @secid: secid of the task being 

[PATCH V5 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

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 
Acked-by: Paul Moore 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---

Ok, I'm taking a slightly different approach and just passing in the secid -
for everything other than the CREDS_CHECK case this will be the task secid,
for CREDS_CHECK it'll be the one associated with the BPRM structure. I believe
that this results in zero functional change other than when dealing with the
new functionality, and we can come back and rework this once we have a better
idea about how IMA should be using secids in the first place.

 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, 

Re: [PATCH V4 2/3] IMA: Use consistent creds

[Matthew Garrett via Selinux]

On Wed, Jan 3, 2018 at 12:08 PM, Casey Schaufler  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?

Re: [PATCH V4 2/3] IMA: Use consistent creds

[Matthew Garrett via Selinux]

On Wed, Jan 3, 2018 at 11:32 AM, Casey Schaufler  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.

Re: [PATCH V4 2/3] IMA: Use consistent creds

[Matthew Garrett via Selinux]

On Wed, Jan 3, 2018 at 7:54 AM, Casey Schaufler  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? :)

[PATCH V4 2/3] IMA: Use consistent creds

[Matthew Garrett via Selinux]

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 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@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, );
+   security_cred_getsecid(cred, );
rc = security_filter_rule_match(sid,
rule->lsm[i].type,
Audit_equal,
-- 
2.15.1.620.gb9897f4670-goog

[PATCH V4 3/3] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

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 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@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 

[PATCH V4 1/3] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

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 
Acked-by: Paul Moore 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@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 

Re: [PATCH V3 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Fri, Dec 15, 2017 at 2:24 PM, Matthew Garrett  wrote:
> Hm, sorry, missed this mail.
>
> On Tue, Nov 28, 2017 at 2:33 PM, Mimi Zohar  wrote:
>> On Tue, 2017-11-28 at 13:37 -0800, Matthew Garrett wrote:
>>> security_task_getsecid(current) will give the same results as
>>> security_cred_getsecid(current_creds())
>>
>> Unwinding security_task_getsecid(current) looks like it is using
>> real_cred, while current_cred() is using cred.
>
> Good question, and there's a current_real_cred() macro, so I should
> just use that instead.

Hm. Actually, I'm not sure. For most checks we were using cred, and
only using real_cred for the secid lookup. This feels somewhat
inconsistent.

Re: [PATCH V3 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

Hm, sorry, missed this mail.

On Tue, Nov 28, 2017 at 2:33 PM, Mimi Zohar  wrote:
> On Tue, 2017-11-28 at 13:37 -0800, Matthew Garrett wrote:
>> security_task_getsecid(current) will give the same results as
>> security_cred_getsecid(current_creds())
>
> Unwinding security_task_getsecid(current) looks like it is using
> real_cred, while current_cred() is using cred.

Good question, and there's a current_real_cred() macro, so I should
just use that instead.

Re: [PATCH V3 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Tue, Nov 28, 2017 at 1:35 PM, Mimi Zohar  wrote:
> On Tue, 2017-11-28 at 13:22 -0800, Matthew Garrett wrote:
>> We need to check against the appropriate credentials structure, and
>> since we're doing this before commit_creds() has been called we can't
>> just do it against the one in the task structure.  For BPRM_CHECK
>> that'll be current_cred(), which means there's no change in
>> functionality, whereas for CREDS_CHECK it'll be the new credentials
>> structure.
>
> The existing code calls security_task_getsecid() with "current" not
> "current_cred".  Will replacing security_task_getsecid() with
> security_cred_getsecid() return the same info for the original
> BRPM_CHECK?

security_task_getsecid(current) will give the same results as
security_cred_getsecid(current_creds())

Re: [PATCH V3 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Tue, Nov 28, 2017 at 12:48 PM, Mimi Zohar  wrote:
> Hi Matthew,
>
> On Thu, 2017-10-26 at 01:40 -0700, Matthew Garrett wrote:
> > 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.
>
> < snip >
>
> > @@ -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, );
> > + security_cred_getsecid(cred, );
> >   rc = security_filter_rule_match(sid,
> >   rule->lsm[i].type,
> >   Audit_equal,
>
> Based on the patch description, I wouldn't expect to see any changes
> here, unless this is wrong to begin with.  In which case, it should be
> a separate patch.

We need to check against the appropriate credentials structure, and
since we're doing this before commit_creds() has been called we can't
just do it against the one in the task structure. For BPRM_CHECK
that'll be current_cred(), which means there's no change in
functionality, whereas for CREDS_CHECK it'll be the new credentials
structure.

Re: [PATCH V3 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Mon, Oct 30, 2017 at 10:03 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On Mon, 2017-10-30 at 10:57 +, Matthew Garrett via Selinux wrote:
>> On Thu, Oct 26, 2017 at 3:20 PM, Stephen Smalley <s...@tycho.nsa.gov>
>> wrote:
>> > On Thu, 2017-10-26 at 01:40 -0700, Matthew Garrett via Selinux
>> > wrote:
>> > > +static void selinux_cred_getsecid(const struct cred *c, u32
>> > > *secid)
>> > > +{
>> > > + rcu_read_lock();
>> > > + *secid = cred_sid(c);
>> > > + rcu_read_unlock();
>> >
>> > Is rcu_read_lock() necessary here? Seems like we use cred_sid() in
>> > many
>> > places without it.
>>
>> Ah, I thought it was based on task_sid(), but I guess that's actually
>> protecting the __task_cred()?
>
> It appears to me that in all other cases, we are either dealing with
> the current cred, or something in the call chain of cred_sid() is
> holding a reference to the cred, or something in the call chain of
> cred_sid() has called rcu_read_lock() already.  I might have missed
> something though, and I don't know how safe it is to assume that all
> future callers will do this.  cc'd David for his thoughts.

Hi David,

Any opinion on this?

Thanks!

Re: [PATCH V3 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Thu, Oct 26, 2017 at 3:20 PM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On Thu, 2017-10-26 at 01:40 -0700, Matthew Garrett via Selinux wrote:
>> +static void selinux_cred_getsecid(const struct cred *c, u32 *secid)
>> +{
>> + rcu_read_lock();
>> + *secid = cred_sid(c);
>> + rcu_read_unlock();
>
> Is rcu_read_lock() necessary here? Seems like we use cred_sid() in many
> places without it.

Ah, I thought it was based on task_sid(), but I guess that's actually
protecting the __task_cred()?

Re: [PATCH V3 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Thu, Oct 26, 2017 at 2:21 PM, Casey Schaufler  wrote:
> On 10/26/2017 1:40 AM, Matthew Garrett wrote:
>>  V3: Fix smack_cred_getsecid()
>
> Much better. Have you tried this with Smack?

I'm afraid not - I have zero expertise with Smack and no easy way to
set it up. I can do so later in the week if you like?

[PATCH V3 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

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 
Acked-by: Paul Moore 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---
 V3: Fix smack_cred_getsecid()

 include/linux/lsm_hooks.h  |  6 ++
 include/linux/security.h   |  1 +
 security/security.c|  7 +++
 security/selinux/hooks.c   |  8 
 security/smack/smack_lsm.c | 18 ++
 5 files changed, 40 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index c9258124e417..c28c6f8b65dc 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.
@@ -1507,6 +1511,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);
@@ -1779,6 +1784,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 ce6265960d6c..14848fef8f62 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 4bf0f571b4ef..02d217597400 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1004,6 +1004,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 f5d304736852..1d11679674a6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3836,6 +3836,13 @@ 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)
+{
+   rcu_read_lock();
+   *secid = cred_sid(c);
+   rcu_read_unlock();
+}
+
 /*
  * set the security data for a kernel service
  * - all the creation contexts are set to unlabelled
@@ -6338,6 +6345,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 286171a16ed2..37c35aaa6955 100644
--- 

[PATCH 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

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.

Signed-off-by: Matthew Garrett 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---
 V2: Fix the IMA_CRED_CHECK defines
 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   | 19 ---
 security/integrity/integrity.h|  9 +++--
 8 files changed, 56 insertions(+), 26 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 6fc888ca468e..ad30094a58b4 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 c2edba8de35e..ff33b7e65a07 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=
  * 

Re: [PATCH 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

On Mon, Oct 16, 2017 at 2:58 PM, Casey Schaufler  wrote:
> On 10/16/2017 1:37 PM, Matthew Garrett wrote:
>> 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.
>
> Why do you want the secid? What are you planning to do with it?

See the following patch - IMA policy allows the admin to restrict
appraisal to executables running in specific security contexts.
However, right now the check at application execution time ends up
using the current task creds before the new creds are committed.

Re: [PATCH 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Tue, Oct 17, 2017 at 12:07 PM, Mimi Zohar  wrote:
> On Mon, 2017-10-16 at 13:37 -0700, Matthew Garrett wrote:
>>   case LSM_SUBJ_TYPE:
>> - security_task_getsecid(tsk, );
>> + security_cred_getsecid(cred, );
>>   rc = security_filter_rule_match(sid,
>>   rule->lsm[i].type,
>>   Audit_equal,
>
> By replacing the call from security_task_getsec() to
> security_cred_getsecid(), I assume you're expecting different results.
>  Will this change break existing IMA policies?

No, for BPRM_CHECK they'll use the same creds that were previously
checked. CREDS_CHECK will behave differently to BPRM_CHECK.

Re: [PATCH 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

On Mon, Oct 16, 2017 at 2:03 PM, Mikhail Kurinnoi
 wrote:
> В Mon, 16 Oct 2017 13:37:09 -0700
> Matthew Garrett  пишет:
>>  #define IMA_BPRM_APPRAISED   0x2000
>>  #define IMA_READ_APPRAISE0x4000
>>  #define IMA_READ_APPRAISED   0x8000
>> +#define IMA_CREDS_APPRAISE   0x4000
>> +#define IMA_CREDS_APPRAISED  0x8000
>
> Is this correct, that the IMA_CREDS_APPRAISE and IMA_CREDS_APPRAISED
> same as IMA_READ_APPRAISE and IMA_READ_APPRAISED?

Definitely not correct, good catch. I'll resend with that fixed if
people feel this approach is reasonable.

[PATCH 2/2] IMA: Support using new creds in appraisal policy

[Matthew Garrett via Selinux]

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.

Signed-off-by: Matthew Garrett 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@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   | 19 ---
 security/integrity/integrity.h|  9 +++--
 8 files changed, 56 insertions(+), 26 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 6fc888ca468e..ad30094a58b4 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 c2edba8de35e..ff33b7e65a07 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.

[PATCH 1/2] security: Add a cred_getsecid hook

[Matthew Garrett via Selinux]

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 
Cc: Paul Moore 
Cc: Stephen Smalley 
Cc: Eric Paris 
Cc: selinux@tycho.nsa.gov
Cc: Casey Schaufler 
Cc: linux-security-mod...@vger.kernel.org
Cc: Mimi Zohar 
Cc: Dmitry Kasatkin 
Cc: linux-integr...@vger.kernel.org
---
 include/linux/lsm_hooks.h  |  6 ++
 include/linux/security.h   |  1 +
 security/security.c|  7 +++
 security/selinux/hooks.c   |  8 
 security/smack/smack.h | 10 ++
 security/smack/smack_lsm.c | 14 ++
 6 files changed, 46 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ce02f76a6188..48a929fd47e6 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -556,6 +556,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
+ * @p 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.
@@ -1510,6 +1514,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);
@@ -1783,6 +1788,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 458e24bea2d4..8d969958c25e 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 55b5997e4b72..0f5784880c94 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1009,6 +1009,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 33fd061305c4..e0828e9130c7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3829,6 +3829,13 @@ 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)
+{
+   rcu_read_lock();
+   *secid = cred_sid(c);
+   rcu_read_unlock();
+}
+
 /*
  * set the security data for a kernel service
  * - all the creation contexts are set to unlabelled
@@ -6332,6 +6339,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.h b/security/smack/smack.h
index 6a71fc7831ab..5af7b7e709bc 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -391,6