lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Mar 2008 20:26:34 +0200 From: "Ahmed S. Darwish" <darwish.07@...il.com> To: Casey Schaufler <casey@...aufler-ca.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, James Morris <jmorris@...ei.org>, Paul Moore <paul.moore@...com>, LKML <linux-kernel@...r.kernel.org>, LSM-ML <linux-security-module@...r.kernel.org>, Audit-ML <linux-audit@...hat.com> Subject: Re: [RFC][PATCH] Smack<->Audit integration On Mon, Mar 10, 2008 at 09:07:08AM -0700, Casey Schaufler wrote: > > --- "Ahmed S. Darwish" <darwish.07@...il.com> wrote: > ... > > > > diff --git a/security/smack/smack.h b/security/smack/smack.h > > index c444f48..2c8bb4c 100644 > > --- a/security/smack/smack.h > > +++ b/security/smack/smack.h > > @@ -57,6 +57,15 @@ struct inode_smack { > > char *smk_inode; /* label of the fso */ > > struct mutex smk_lock; /* initialization lock */ > > int smk_flags; /* smack inode flags */ > > + int secid; /* security identifier */ > > No. > > Secid's are horrid things and every effort should be made to > expunge them from the known universe. Under no circumstances > should thier use be expanded. The only reason Smack has them > at all is because certain interfaces that in my mind should > have known better use them. If you must deal with secids, > and for this round of audit I think that's a given, use > smack_to_secid(sp->smk_inode) where you need to. If there's a > real performance issue apply intelligence to smack_to_secid > instead of storing the secid. There ought to be a way to > use container_of to do smack_to_secid, but I had trouble with > that and moved along without figuring out what I had done > wrong. > mm .. I should have remembered the un-official Smack motto: "Everything is a label, and whenever possible, this label is allocated once through system lifetime" About performance, yes there'll be issues searching labels espicially in audit_rule_match() which got called at the end of every system call. I'll try it using container_of (it should work at the end). ... > > > @@ -1696,9 +1738,9 @@ static int smack_msg_queue_alloc_security(struct > > msg_queue *msq) > > */ > > static void smack_msg_queue_free_security(struct msg_queue *msq) > > { > > - struct kern_ipc_perm *kisp = &msq->q_perm; > > + struct kern_ipc_perm *ipcp = &msq->q_perm; > > > > - kisp->security = NULL; > > + kfree(ipcp->security); > > } > > Don't you just hate repetative reviewers? > Probably hating secids with passion :) ? Admittedly, after some thinking I felt now that they don't fit with the Smack model very well. ... > > + > > +/** > > + * smack_audit_rule_match - Audit given secid identified object ? > > + * @secid: Security id to test > > + * @field: Message flags given from user-space > > + * @op: Required operation (only equality is allowed) > > + * @vrule: Smack audit rule that will be checked against the secid object > > + * @actx: audit context associated with the check (used for Audit logging) > > + * > > + * This is the core Audit hook. It's used to identify objects like > > + * syscalls and inodes requested from user-space to be audited from > > + * remaining kernel objects. > > + */ > > +static int smack_audit_rule_match(u32 secid, u32 field, u32 op, void *vrule, > > + struct audit_context *actx) > > +{ > > + struct smack_known *smk_rule = vrule; > > char *smack; > More of "everything is a label". > > + > > + if (!smk_rule) { > > + audit_log(actx, GFP_KERNEL, AUDIT_SELINUX_ERR, > > + "Smack: missing rule\n"); > > + return -ENOENT; > > + } > > + > > + if (field != AUDIT_SUBJ_USER && field != AUDIT_OBJ_USER) > > + return 0; > > + > > smack = smack_from_secid(secid); > > > + if (op == AUDIT_EQUAL) > > + return (smk_rule->smk_secid == secid); > > + if (op == AUDIT_NOT_EQUAL) > > + return (smk_rule->smk_secid != secid); > > if (op == AUDIT_EQUAL) > return (smk_rule->smk_smack == smack); > if (op == AUDIT_NOT_EQUAL) > return (smk_rule->smk_smack == smack); > You've meant using the short-circuit: smk_rule->smk_smack == smack || strnmp(smack, ..., ..) Right ? ... > > + > > +/** > > + * smack_audit_rule_free - free internal audit rule representation > > + * @vrule: rule to be freed. > > + * > > + * No memory was allocated in audit_rule_init. > > + */ > > +static void smack_audit_rule_free(void *vrule) > > +{ > > + /* No-op */ > > +} This little no-op was the only thing that was agreed upon ;) ... > > Casey Schaufler > casey@...aufler-ca.com Regards, -- "Better to light a candle, than curse the darkness" Ahmed S. Darwish Homepage: http://darwish.07.googlepages.com Blog: http://darwish-07.blogspot.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists