* linux-next: manual merge of the userns tree with the security tree
@ 2012-09-24 11:52 Stephen Rothwell
2012-09-24 15:31 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2012-09-24 11:52 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-next, linux-kernel, Mimi Zohar, James Morris, Dmitry Kasatkin
[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]
Hi Eric,
Today's linux-next merge of the userns tree got a conflict in
security/integrity/ima/ima_policy.c between commit 07f6a79415d7 ("ima:
add appraise action keywords and default rules") from the security tree
and commit 8b94eea4bfb8 ("userns: Add user namespace support to IMA") from
the userns tree.
I fixed it up (see below) but it probably needs more and can carry the
fix as necessary (no action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc security/integrity/ima/ima_policy.c
index cda9031,c84df05..0000000
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@@ -45,8 -39,7 +45,8 @@@ struct ima_rule_entry
enum ima_hooks func;
int mask;
unsigned long fsmagic;
- uid_t uid;
+ kuid_t uid;
+ uid_t fowner;
struct {
void *rule; /* LSM file metadata specific */
int type; /* audit type */
@@@ -141,10 -112,8 +141,10 @@@ static bool ima_match_rules(struct ima_
if ((rule->flags & IMA_FSMAGIC)
&& rule->fsmagic != inode->i_sb->s_magic)
return false;
- if ((rule->flags & IMA_UID) && rule->uid != cred->uid)
+ if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
return false;
+ if ((rule->flags & IMA_FOWNER) && rule->fowner != inode->i_uid)
+ return false;
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid, sid;
@@@ -336,8 -277,7 +336,8 @@@ static int ima_parse_rule(char *rule, s
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_RULE);
- entry->uid = -1;
+ entry->uid = INVALID_UID;
+ entry->fowner = -1;
entry->action = UNKNOWN;
while ((p = strsep(&rule, " \t")) != NULL) {
substring_t args[MAX_OPT_ARGS];
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the userns tree with the security tree
2012-09-24 11:52 linux-next: manual merge of the userns tree with the security tree Stephen Rothwell
@ 2012-09-24 15:31 ` Eric W. Biederman
2012-09-25 0:46 ` Stephen Rothwell
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2012-09-24 15:31 UTC (permalink / raw)
To: Stephen Rothwell
Cc: linux-next, linux-kernel, Mimi Zohar, James Morris, Dmitry Kasatkin
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi Eric,
>
> Today's linux-next merge of the userns tree got a conflict in
> security/integrity/ima/ima_policy.c between commit 07f6a79415d7 ("ima:
> add appraise action keywords and default rules") from the security tree
> and commit 8b94eea4bfb8 ("userns: Add user namespace support to IMA") from
> the userns tree.
>
> I fixed it up (see below) but it probably needs more and can carry the
> fix as necessary (no action is required).
Right. To work when user namespace support is enabled fowner needs be
converted to a kuid_t as well.
When I did a trial earlier this is what I wound up with. As long as
user namespaces are not enabled what you wound up with should be fine.
Eric
diff --cc security/integrity/ima/ima_policy.c
index c84df05,cda9031..346fe8f
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@@ -39,7 -45,8 +45,8 @@@ struct ima_rule_entry
enum ima_hooks func;
int mask;
unsigned long fsmagic;
- uid_t uid;
- uid_t fowner;
+ kuid_t uid;
++ kuid_t fowner;
struct {
void *rule; /* LSM file metadata specific */
int type; /* audit type */
@@@ -75,14 -82,28 +82,28 @@@ static struct ima_rule_entry default_ru
.flags = IMA_FUNC | IMA_MASK | IMA_UID},
};
- static LIST_HEAD(measure_default_rules);
- static LIST_HEAD(measure_policy_rules);
- static struct list_head *ima_measure;
+ static struct ima_rule_entry default_appraise_rules[] = {
+ {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC},
+ {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC},
- {.action = APPRAISE,.fowner = 0,.flags = IMA_FOWNER},
++ {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER},
+ };
+
+ static LIST_HEAD(ima_default_rules);
+ static LIST_HEAD(ima_policy_rules);
+ static struct list_head *ima_rules;
- static DEFINE_MUTEX(ima_measure_mutex);
+ static DEFINE_MUTEX(ima_rules_mutex);
static bool ima_use_tcb __initdata;
- static int __init default_policy_setup(char *str)
+ static int __init default_measure_policy_setup(char *str)
{
ima_use_tcb = 1;
return 1;
@@@ -112,8 -141,10 +141,10 @@@ static bool ima_match_rules(struct ima_
if ((rule->flags & IMA_FSMAGIC)
&& rule->fsmagic != inode->i_sb->s_magic)
return false;
- if ((rule->flags & IMA_UID) && rule->uid != cred->uid)
+ if ((rule->flags & IMA_UID) && !uid_eq(rule->uid, cred->uid))
return false;
- if ((rule->flags & IMA_FOWNER) && rule->fowner != inode->i_uid)
++ if ((rule->flags & IMA_FOWNER) && !uid_eq(rule->fowner, inode->i_uid))
+ return false;
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid, sid;
@@@ -277,7 -336,8 +336,8 @@@ static int ima_parse_rule(char *rule, s
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_RULE);
- entry->uid = -1;
- entry->fowner = -1;
+ entry->uid = INVALID_UID;
++ entry->fowner = INVALID_UID;
entry->action = UNKNOWN;
while ((p = strsep(&rule, " \t")) != NULL) {
substring_t args[MAX_OPT_ARGS];
@@@ -375,6 -459,23 +459,23 @@@
entry->flags |= IMA_UID;
}
break;
+ case Opt_fowner:
+ ima_log_string(ab, "fowner", args[0].from);
+
- if (entry->fowner != -1) {
++ if (uid_valid(entry->fowner)) {
+ result = -EINVAL;
+ break;
+ }
+
+ result = strict_strtoul(args[0].from, 10, &lnum);
+ if (!result) {
- entry->fowner = (uid_t) lnum;
- if (entry->fowner != lnum)
++ entry->fowner = make_kuid(current_user_ns(), (uid_t) lnum);
++ if (!uid_valid(entry->fowner) || ((uid_t)lnum != lnum))
+ result = -EINVAL;
+ else
+ entry->flags |= IMA_FOWNER;
+ }
+ break;
case Opt_obj_user:
ima_log_string(ab, "obj_user", args[0].from);
result = ima_lsm_rule_init(entry, args[0].from,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the userns tree with the security tree
2012-09-24 15:31 ` Eric W. Biederman
@ 2012-09-25 0:46 ` Stephen Rothwell
0 siblings, 0 replies; 6+ messages in thread
From: Stephen Rothwell @ 2012-09-25 0:46 UTC (permalink / raw)
To: Eric W. Biederman
Cc: linux-next, linux-kernel, Mimi Zohar, James Morris, Dmitry Kasatkin
[-- Attachment #1: Type: text/plain, Size: 480 bytes --]
Hi Eric,
On Mon, 24 Sep 2012 08:31:07 -0700 ebiederm@xmission.com (Eric W. Biederman) wrote:
>
> Right. To work when user namespace support is enabled fowner needs be
> converted to a kuid_t as well.
>
> When I did a trial earlier this is what I wound up with. As long as
> user namespaces are not enabled what you wound up with should be fine.
I will use your resolution today.
Thanks.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the userns tree with the security tree
2012-09-24 12:17 ` Stephen Rothwell
@ 2012-09-24 15:36 ` Peter Moody
0 siblings, 0 replies; 6+ messages in thread
From: Peter Moody @ 2012-09-24 15:36 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: Eric W. Biederman, linux-next, linux-kernel, Eric Paris
Hey Eric (Paris),
this is the second time I've been notified of a merge issue with this
audit patch; Is there something I need to do (or should have done
earlier) to keep this from continuing to be an issue?
Cheers,
peter
On Mon, Sep 24, 2012 at 5:17 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi,
>
> On Mon, 24 Sep 2012 21:41:16 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> diff --cc kernel/auditsc.c
>> index 37f52f2,ff4798f..0000000
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@@ -1160,32 -1151,8 +1152,38 @@@ void audit_log_task_info(struct audit_b
>> char name[sizeof(tsk->comm)];
>> struct mm_struct *mm = tsk->mm;
>> struct vm_area_struct *vma;
>> + char *tty;
>> +
>> + if (!ab)
>> + return;
>>
>> /* tsk == current */
>> + cred = current_cred();
>> +
>> + spin_lock_irq(&tsk->sighand->siglock);
>> + if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
>> + tty = tsk->signal->tty->name;
>> + else
>> + tty = "(none)";
>> + spin_unlock_irq(&tsk->sighand->siglock);
>> +
>> +
>> + audit_log_format(ab,
>> + " ppid=%ld pid=%d auid=%u uid=%u gid=%u"
>> + " euid=%u suid=%u fsuid=%u"
>> + " egid=%u sgid=%u fsgid=%u ses=%u tty=%s",
>> + sys_getppid(),
>> + tsk->pid,
>> - tsk->loginuid, cred->uid, cred->gid,
>> - cred->euid, cred->suid, cred->fsuid,
>> - cred->egid, cred->sgid, cred->fsgid,
>> ++ from_kuid(&init_user_ns, tsk->loginuid),
>> ++ from_kuid(&init_user_ns, context->uid),
>> ++ from_kgid(&init_user_ns, context->gid),
>> ++ from_kuid(&init_user_ns, context->euid),
>> ++ from_kuid(&init_user_ns, context->suid),
>> ++ from_kuid(&init_user_ns, context->fsuid),
>> ++ from_kgid(&init_user_ns, context->egid),
>> ++ from_kgid(&init_user_ns, context->sgid),
>> ++ from_kgid(&init_user_ns, context->fsgid),
>
> These should all be "cred" not "context", of course. I fixed this in my
> tree.
>
> --
> Cheers,
> Stephen Rothwell sfr@canb.auug.org.au
--
Peter Moody Google 1.650.253.7306
Security Engineer pgp:0xC3410038
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: linux-next: manual merge of the userns tree with the security tree
2012-09-24 11:41 Stephen Rothwell
@ 2012-09-24 12:17 ` Stephen Rothwell
2012-09-24 15:36 ` Peter Moody
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2012-09-24 12:17 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-next, linux-kernel, Peter Moody
[-- Attachment #1: Type: text/plain, Size: 1713 bytes --]
Hi,
On Mon, 24 Sep 2012 21:41:16 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> diff --cc kernel/auditsc.c
> index 37f52f2,ff4798f..0000000
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@@ -1160,32 -1151,8 +1152,38 @@@ void audit_log_task_info(struct audit_b
> char name[sizeof(tsk->comm)];
> struct mm_struct *mm = tsk->mm;
> struct vm_area_struct *vma;
> + char *tty;
> +
> + if (!ab)
> + return;
>
> /* tsk == current */
> + cred = current_cred();
> +
> + spin_lock_irq(&tsk->sighand->siglock);
> + if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
> + tty = tsk->signal->tty->name;
> + else
> + tty = "(none)";
> + spin_unlock_irq(&tsk->sighand->siglock);
> +
> +
> + audit_log_format(ab,
> + " ppid=%ld pid=%d auid=%u uid=%u gid=%u"
> + " euid=%u suid=%u fsuid=%u"
> + " egid=%u sgid=%u fsgid=%u ses=%u tty=%s",
> + sys_getppid(),
> + tsk->pid,
> - tsk->loginuid, cred->uid, cred->gid,
> - cred->euid, cred->suid, cred->fsuid,
> - cred->egid, cred->sgid, cred->fsgid,
> ++ from_kuid(&init_user_ns, tsk->loginuid),
> ++ from_kuid(&init_user_ns, context->uid),
> ++ from_kgid(&init_user_ns, context->gid),
> ++ from_kuid(&init_user_ns, context->euid),
> ++ from_kuid(&init_user_ns, context->suid),
> ++ from_kuid(&init_user_ns, context->fsuid),
> ++ from_kgid(&init_user_ns, context->egid),
> ++ from_kgid(&init_user_ns, context->sgid),
> ++ from_kgid(&init_user_ns, context->fsgid),
These should all be "cred" not "context", of course. I fixed this in my
tree.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* linux-next: manual merge of the userns tree with the security tree
@ 2012-09-24 11:41 Stephen Rothwell
2012-09-24 12:17 ` Stephen Rothwell
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2012-09-24 11:41 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-next, linux-kernel, Peter Moody
[-- Attachment #1: Type: text/plain, Size: 2392 bytes --]
Hi Eric,
Today's linux-next merge of the userns tree got a conflict in
kernel/auditsc.c between commit e23eb920b0f3 ("audit: export
audit_log_task_info") from the security tree and commits e1760bd5ffae
("userns: Convert the audit loginuid to be a kuid") and cca080d9b622
("userns: Convert audit to work with user namespaces enabled") from the
userns tree.
I fixed it up (I think - see below) and can carry the fix as necessary (no
action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --cc kernel/auditsc.c
index 37f52f2,ff4798f..0000000
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@@ -1160,32 -1151,8 +1152,38 @@@ void audit_log_task_info(struct audit_b
char name[sizeof(tsk->comm)];
struct mm_struct *mm = tsk->mm;
struct vm_area_struct *vma;
+ char *tty;
+
+ if (!ab)
+ return;
/* tsk == current */
+ cred = current_cred();
+
+ spin_lock_irq(&tsk->sighand->siglock);
+ if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name)
+ tty = tsk->signal->tty->name;
+ else
+ tty = "(none)";
+ spin_unlock_irq(&tsk->sighand->siglock);
+
+
+ audit_log_format(ab,
+ " ppid=%ld pid=%d auid=%u uid=%u gid=%u"
+ " euid=%u suid=%u fsuid=%u"
+ " egid=%u sgid=%u fsgid=%u ses=%u tty=%s",
+ sys_getppid(),
+ tsk->pid,
- tsk->loginuid, cred->uid, cred->gid,
- cred->euid, cred->suid, cred->fsuid,
- cred->egid, cred->sgid, cred->fsgid,
++ from_kuid(&init_user_ns, tsk->loginuid),
++ from_kuid(&init_user_ns, context->uid),
++ from_kgid(&init_user_ns, context->gid),
++ from_kuid(&init_user_ns, context->euid),
++ from_kuid(&init_user_ns, context->suid),
++ from_kuid(&init_user_ns, context->fsuid),
++ from_kgid(&init_user_ns, context->egid),
++ from_kgid(&init_user_ns, context->sgid),
++ from_kgid(&init_user_ns, context->fsgid),
+ tsk->sessionid, tty);
get_task_comm(name, tsk);
audit_log_format(ab, " comm=");
@@@ -1208,10 -1175,8 +1206,10 @@@
audit_log_task_context(ab);
}
+EXPORT_SYMBOL(audit_log_task_info);
+
static int audit_log_pid_context(struct audit_context *context, pid_t pid,
- uid_t auid, uid_t uid, unsigned int sessionid,
+ kuid_t auid, kuid_t uid, unsigned int sessionid,
u32 sid, char *comm)
{
struct audit_buffer *ab;
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-25 0:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-24 11:52 linux-next: manual merge of the userns tree with the security tree Stephen Rothwell
2012-09-24 15:31 ` Eric W. Biederman
2012-09-25 0:46 ` Stephen Rothwell
-- strict thread matches above, loose matches on Subject: below --
2012-09-24 11:41 Stephen Rothwell
2012-09-24 12:17 ` Stephen Rothwell
2012-09-24 15:36 ` Peter Moody
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).