| Message ID | 20220104170416.1923685-19-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Namespace IMA with audit support in IMA-ns | expand |
On Tue, Jan 04, 2022 at 12:04:15PM -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.ibm.com> > > Show the uid and gid values of the owning user namespace when displaying > the IMA policy rather than the kernel uid and gid values. Now the same uid > and gid values are shown in the policy as those that were used when the > policy was set. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > security/integrity/ima/ima_policy.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 15c68dc5da9e..b7dbc687b6ff 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -1997,6 +1997,7 @@ static void ima_policy_show_appraise_algos(struct seq_file *m, > > int ima_policy_show(struct seq_file *m, void *v) > { > + struct user_namespace *user_ns = ima_user_ns_from_file(m->file); Hm, so when looking at the policy entries via seq_file's .show method and displaying the {g,u}id values of the rules we don't want the values resolved according to the user namespace the securityfs instances was mounted in. That would be misleading for callers that are in an ancestor userns (which we allow in .permission). So we want to make sure that we see the values as the opener of the file would see them. This is similar to e.g. looking at a task's ids through /proc/<pid>/status. So this should be seq_user_ns(m) instead of ima_user_ns_from_file(). > struct ima_rule_entry *entry = v; > int i; > char tbuf[64] = {0,}; > @@ -2074,7 +2075,8 @@ int ima_policy_show(struct seq_file *m, void *v) > } > > if (entry->flags & IMA_UID) { > - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->uid)); > + snprintf(tbuf, sizeof(tbuf), > + "%d", from_kuid(user_ns, entry->uid)); This should be from_k{g,u}id_munged().
On 1/14/22 08:45, Christian Brauner wrote: > On Tue, Jan 04, 2022 at 12:04:15PM -0500, Stefan Berger wrote: >> From: Stefan Berger <stefanb@linux.ibm.com> >> >> Show the uid and gid values of the owning user namespace when displaying >> the IMA policy rather than the kernel uid and gid values. Now the same uid >> and gid values are shown in the policy as those that were used when the >> policy was set. >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> --- >> security/integrity/ima/ima_policy.c | 19 +++++++++++++------ >> 1 file changed, 13 insertions(+), 6 deletions(-) >> >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index 15c68dc5da9e..b7dbc687b6ff 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -1997,6 +1997,7 @@ static void ima_policy_show_appraise_algos(struct seq_file *m, >> >> int ima_policy_show(struct seq_file *m, void *v) >> { >> + struct user_namespace *user_ns = ima_user_ns_from_file(m->file); > Hm, so when looking at the policy entries via seq_file's .show method > and displaying the {g,u}id values of the rules we don't want the values > resolved according to the user namespace the securityfs instances was > mounted in. That would be misleading for callers that are in an > ancestor userns (which we allow in .permission). > > So we want to make sure that we see the values as the opener of the file > would see them. This is similar to e.g. looking at a task's ids through > /proc/<pid>/status. So this should be seq_user_ns(m) instead of > ima_user_ns_from_file(). >> struct ima_rule_entry *entry = v; >> int i; >> char tbuf[64] = {0,}; >> @@ -2074,7 +2075,8 @@ int ima_policy_show(struct seq_file *m, void *v) >> } >> >> if (entry->flags & IMA_UID) { >> - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->uid)); >> + snprintf(tbuf, sizeof(tbuf), >> + "%d", from_kuid(user_ns, entry->uid)); > This should be from_k{g,u}id_munged(). Thanks, fixed. When I run a runc container as uid=1000 I see uid = 0 when inside the container and when entering its mount namespace from root account via nsenter it shows 'uid = 1000' while before it was showing 'uid = 0'.
On Tue, Jan 18, 2022 at 11:31:29AM -0500, Stefan Berger wrote: > > On 1/14/22 08:45, Christian Brauner wrote: > > On Tue, Jan 04, 2022 at 12:04:15PM -0500, Stefan Berger wrote: > > > From: Stefan Berger <stefanb@linux.ibm.com> > > > > > > Show the uid and gid values of the owning user namespace when displaying > > > the IMA policy rather than the kernel uid and gid values. Now the same uid > > > and gid values are shown in the policy as those that were used when the > > > policy was set. > > > > > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > > --- > > > security/integrity/ima/ima_policy.c | 19 +++++++++++++------ > > > 1 file changed, 13 insertions(+), 6 deletions(-) > > > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > > index 15c68dc5da9e..b7dbc687b6ff 100644 > > > --- a/security/integrity/ima/ima_policy.c > > > +++ b/security/integrity/ima/ima_policy.c > > > @@ -1997,6 +1997,7 @@ static void ima_policy_show_appraise_algos(struct seq_file *m, > > > int ima_policy_show(struct seq_file *m, void *v) > > > { > > > + struct user_namespace *user_ns = ima_user_ns_from_file(m->file); > > Hm, so when looking at the policy entries via seq_file's .show method > > and displaying the {g,u}id values of the rules we don't want the values > > resolved according to the user namespace the securityfs instances was > > mounted in. That would be misleading for callers that are in an > > ancestor userns (which we allow in .permission). > > > > So we want to make sure that we see the values as the opener of the file > > would see them. This is similar to e.g. looking at a task's ids through > > /proc/<pid>/status. So this should be seq_user_ns(m) instead of > > ima_user_ns_from_file(). > > > struct ima_rule_entry *entry = v; > > > int i; > > > char tbuf[64] = {0,}; > > > @@ -2074,7 +2075,8 @@ int ima_policy_show(struct seq_file *m, void *v) > > > } > > > if (entry->flags & IMA_UID) { > > > - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->uid)); > > > + snprintf(tbuf, sizeof(tbuf), > > > + "%d", from_kuid(user_ns, entry->uid)); > > This should be from_k{g,u}id_munged(). > > Thanks, fixed. > > When I run a runc container as uid=1000 I see uid = 0 when inside the > container and when entering its mount namespace from root account via > nsenter it shows 'uid = 1000' while before it was showing 'uid = 0'. Yes, when you're only entering the mountns you should see uid 1000 as that's what that {g,u}id is mapped to in your namespace and you've opened __and read__ that file from the same namespace. (Now, if you were to open that fd and send it back to a process running in the container and that process does the read it would still see 1000. But that's ok, because we care about the opener's creds.)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 15c68dc5da9e..b7dbc687b6ff 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1997,6 +1997,7 @@ static void ima_policy_show_appraise_algos(struct seq_file *m, int ima_policy_show(struct seq_file *m, void *v) { + struct user_namespace *user_ns = ima_user_ns_from_file(m->file); struct ima_rule_entry *entry = v; int i; char tbuf[64] = {0,}; @@ -2074,7 +2075,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_UID) { - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->uid)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kuid(user_ns, entry->uid)); if (entry->uid_op == &uid_gt) seq_printf(m, pt(Opt_uid_gt), tbuf); else if (entry->uid_op == &uid_lt) @@ -2085,7 +2087,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_EUID) { - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->uid)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kuid(user_ns, entry->uid)); if (entry->uid_op == &uid_gt) seq_printf(m, pt(Opt_euid_gt), tbuf); else if (entry->uid_op == &uid_lt) @@ -2096,7 +2099,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_GID) { - snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->gid)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kgid(user_ns, entry->gid)); if (entry->gid_op == &gid_gt) seq_printf(m, pt(Opt_gid_gt), tbuf); else if (entry->gid_op == &gid_lt) @@ -2107,7 +2111,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_EGID) { - snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->gid)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kgid(user_ns, entry->gid)); if (entry->gid_op == &gid_gt) seq_printf(m, pt(Opt_egid_gt), tbuf); else if (entry->gid_op == &gid_lt) @@ -2118,7 +2123,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_FOWNER) { - snprintf(tbuf, sizeof(tbuf), "%d", __kuid_val(entry->fowner)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kuid(user_ns, entry->fowner)); if (entry->fowner_op == &uid_gt) seq_printf(m, pt(Opt_fowner_gt), tbuf); else if (entry->fowner_op == &uid_lt) @@ -2129,7 +2135,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_FGROUP) { - snprintf(tbuf, sizeof(tbuf), "%d", __kgid_val(entry->fgroup)); + snprintf(tbuf, sizeof(tbuf), + "%d", from_kgid(user_ns, entry->fgroup)); if (entry->fgroup_op == &gid_gt) seq_printf(m, pt(Opt_fgroup_gt), tbuf); else if (entry->fgroup_op == &gid_lt)