| 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
| ||
|
Message-ID: <4ea89fdba649142dcebc05e115d6e5aefb7f9430.camel@canonical.com>
Date: Tue, 27 Aug 2024 11:45:57 -0300
From: Georgia Garcia <georgia.garcia@...onical.com>
To: Casey Schaufler <casey@...aufler-ca.com>, paul@...l-moore.com,
linux-security-module@...r.kernel.org
Cc: jmorris@...ei.org, serge@...lyn.com, keescook@...omium.org,
john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
mic@...ikod.net
Subject: Re: [PATCH 03/13] LSM: Add lsmblob_to_secctx hook
On Mon, 2024-08-26 at 11:45 -0700, Casey Schaufler wrote:
> On 8/26/2024 10:43 AM, Georgia Garcia wrote:
> > Hi Casey
> >
> > On Sun, 2024-08-25 at 12:00 -0700, Casey Schaufler wrote:
> > > Add a new hook security_lsmblob_to_secctx() and its LSM specific
> > > implementations. The LSM specific code will use the lsmblob element
> > > allocated for that module. This allows for the possibility that more
> > > than one module may be called upon to translate a secid to a string,
> > > as can occur in the audit code.
> > >
> > > Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> > > ---
> > > include/linux/lsm_hook_defs.h | 2 ++
> > > include/linux/security.h | 11 +++++++++-
> > > security/apparmor/include/secid.h | 2 ++
> > > security/apparmor/lsm.c | 1 +
> > > security/apparmor/secid.c | 36 +++++++++++++++++++++++++++++++
> > > security/security.c | 30 ++++++++++++++++++++++++++
> > > security/selinux/hooks.c | 16 ++++++++++++--
> > > security/smack/smack_lsm.c | 31 +++++++++++++++++++++-----
> > > 8 files changed, 121 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> > > index 1d3bdf71109e..3e5f6baa7b9f 100644
> > > --- a/include/linux/lsm_hook_defs.h
> > > +++ b/include/linux/lsm_hook_defs.h
> > > @@ -291,6 +291,8 @@ LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
> > > LSM_HOOK(int, 0, ismaclabel, const char *name)
> > > LSM_HOOK(int, -EOPNOTSUPP, secid_to_secctx, u32 secid, char **secdata,
> > > u32 *seclen)
> > > +LSM_HOOK(int, -EOPNOTSUPP, lsmblob_to_secctx, struct lsmblob *blob,
> > > + char **secdata, u32 *seclen)
> > > LSM_HOOK(int, 0, secctx_to_secid, const char *secdata, u32 seclen, u32 *secid)
> > > LSM_HOOK(void, LSM_RET_VOID, release_secctx, char *secdata, u32 seclen)
> > > LSM_HOOK(void, LSM_RET_VOID, inode_invalidate_secctx, struct inode *inode)
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index c0ed2119a622..457fafc32fb0 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -520,6 +520,8 @@ int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
> > > int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> > > int security_ismaclabel(const char *name);
> > > int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> > > +int security_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen);
> > > int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
> > > void security_release_secctx(char *secdata, u32 seclen);
> > > void security_inode_invalidate_secctx(struct inode *inode);
> > > @@ -1461,7 +1463,14 @@ static inline int security_ismaclabel(const char *name)
> > > return 0;
> > > }
> > >
> > > -static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> > > +static inline int security_secid_to_secctx(u32 secid, char **secdata,
> > > + u32 *seclen)
> > > +{
> > > + return -EOPNOTSUPP;
> > > +}
> > > +
> > > +static inline int security_lsmblob_to_secctx(struct lsmblob *blob,
> > > + char **secdata, u32 *seclen)
> > > {
> > > return -EOPNOTSUPP;
> > > }
> > > diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h
> > > index a912a5d5d04f..816a425e2023 100644
> > > --- a/security/apparmor/include/secid.h
> > > +++ b/security/apparmor/include/secid.h
> > > @@ -26,6 +26,8 @@ extern int apparmor_display_secid_mode;
> > >
> > > struct aa_label *aa_secid_to_label(u32 secid);
> > > int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> > > +int apparmor_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen);
> > > int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
> > > void apparmor_release_secctx(char *secdata, u32 seclen);
> > >
> > > diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> > > index 808060f9effb..050d103f5ca5 100644
> > > --- a/security/apparmor/lsm.c
> > > +++ b/security/apparmor/lsm.c
> > > @@ -1532,6 +1532,7 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
> > > #endif
> > >
> > > LSM_HOOK_INIT(secid_to_secctx, apparmor_secid_to_secctx),
> > > + LSM_HOOK_INIT(lsmblob_to_secctx, apparmor_lsmblob_to_secctx),
> > > LSM_HOOK_INIT(secctx_to_secid, apparmor_secctx_to_secid),
> > > LSM_HOOK_INIT(release_secctx, apparmor_release_secctx),
> > >
> > > diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
> > > index 83d3d1e6d9dc..3c389e5810cd 100644
> > > --- a/security/apparmor/secid.c
> > > +++ b/security/apparmor/secid.c
> > > @@ -90,6 +90,42 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> > > return 0;
> > > }
> > >
> > > +int apparmor_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen)
> > > +{
> > > + /* TODO: cache secctx and ref count so we don't have to recreate */
> > > + struct aa_label *label;
> > > + int flags = FLAG_VIEW_SUBNS | FLAG_HIDDEN_UNCONFINED | FLAG_ABS_ROOT;
> > > + int len;
> > > +
> > > + AA_BUG(!seclen);
> > > +
> > > + /* scaffolding */
> > > + if (!blob->apparmor.label && blob->scaffold.secid)
> > > + label = aa_secid_to_label(blob->scaffold.secid);
> > > + else
> > > + label = blob->apparmor.label;
> > > +
> > It would improve maintainability if the rest of the function was
> > refactored into label_to_secctx(...), for example, so it could be
> > shared by apparmor_secid_to_secctx and apparmor_lsmblob_to_secctx.
>
> All uses of scaffold.secid disappear by patch 13/13 of this set.
> This code, being temporary and short lived, would not benefit much
> from being maintainable.
I was referring to the rest of the function which does not include the
scaffolding and remains duplicated by the end of the patch set.
>From this section:
> >
> > > + if (!label)
> > > + return -EINVAL;
> > > +
> > > + if (apparmor_display_secid_mode)
> > > + flags |= FLAG_SHOW_MODE;
> > > +
> > > + if (secdata)
> > > + len = aa_label_asxprint(secdata, root_ns, label,
> > > + flags, GFP_ATOMIC);
> > > + else
> > > + len = aa_label_snxprint(NULL, 0, root_ns, label, flags);
> > > +
> > > + if (len < 0)
> > > + return -ENOMEM;
> > > +
> > > + *seclen = len;
> > > +
> > > + return 0;
> > > +}
To here.
> > > +
> > > int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> > > {
> > > struct aa_label *label;
> > > diff --git a/security/security.c b/security/security.c
> > > index 64a6d6bbd1f4..bb541a3be410 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -4192,6 +4192,36 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> > > }
> > > EXPORT_SYMBOL(security_secid_to_secctx);
> > >
> > > +/**
> > > + * security_lsmblob_to_secctx() - Convert a lsmblob to a secctx
> > > + * @blob: lsm specific information
> > > + * @secdata: secctx
> > > + * @seclen: secctx length
> > > + *
> > > + * Convert a @blob entry to security context. If @secdata is NULL the
> > > + * length of the result will be returned in @seclen, but no @secdata
> > > + * will be returned. This does mean that the length could change between
> > > + * calls to check the length and the next call which actually allocates
> > > + * and returns the @secdata.
> > > + *
> > > + * Return: Return 0 on success, error on failure.
> > > + */
> > > +int security_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen)
> > > +{
> > > + struct security_hook_list *hp;
> > > + int rc;
> > > +
> > > + hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> > > + rc = hp->hook.lsmblob_to_secctx(blob, secdata, seclen);
> > > + if (rc != LSM_RET_DEFAULT(secid_to_secctx))
> > > + return rc;
> > > + }
> > > +
> > > + return LSM_RET_DEFAULT(secid_to_secctx);
> > > +}
> > > +EXPORT_SYMBOL(security_lsmblob_to_secctx);
> > > +
> > > /**
> > > * security_secctx_to_secid() - Convert a secctx to a secid
> > > * @secdata: secctx
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 55c78c318ccd..102489e6d579 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -6610,8 +6610,19 @@ static int selinux_ismaclabel(const char *name)
> > >
> > > static int selinux_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> > > {
> > > - return security_sid_to_context(secid,
> > > - secdata, seclen);
> > > + return security_sid_to_context(secid, secdata, seclen);
> > > +}
> > > +
> > > +static int selinux_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen)
> > > +{
> > > + u32 secid = blob->selinux.secid;
> > > +
> > > + /* scaffolding */
> > > + if (!secid)
> > > + secid = blob->scaffold.secid;
> > > +
> > > + return security_sid_to_context(secid, secdata, seclen);
> > > }
> > >
> > > static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
> > > @@ -7388,6 +7399,7 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
> > > LSM_HOOK_INIT(inode_alloc_security, selinux_inode_alloc_security),
> > > LSM_HOOK_INIT(sem_alloc_security, selinux_sem_alloc_security),
> > > LSM_HOOK_INIT(secid_to_secctx, selinux_secid_to_secctx),
> > > + LSM_HOOK_INIT(lsmblob_to_secctx, selinux_lsmblob_to_secctx),
> > > LSM_HOOK_INIT(inode_getsecctx, selinux_inode_getsecctx),
> > > LSM_HOOK_INIT(sk_alloc_security, selinux_sk_alloc_security),
> > > LSM_HOOK_INIT(tun_dev_alloc_security, selinux_tun_dev_alloc_security),
> > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> > > index 52d5ef986db8..5d74d8590862 100644
> > > --- a/security/smack/smack_lsm.c
> > > +++ b/security/smack/smack_lsm.c
> > > @@ -4787,7 +4787,7 @@ static int smack_audit_rule_known(struct audit_krule *krule)
> > > static int smack_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
> > > void *vrule)
> > > {
> > > - struct smack_known *skp;
> > > + struct smack_known *skp = blob->smack.skp;
> > > char *rule = vrule;
> > >
> > > if (unlikely(!rule)) {
> > > @@ -4799,10 +4799,8 @@ static int smack_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
> > > return 0;
> > >
> > > /* scaffolding */
> > > - if (!blob->smack.skp && blob->scaffold.secid)
> > > + if (!skp && blob->scaffold.secid)
> > > skp = smack_from_secid(blob->scaffold.secid);
> > > - else
> > > - skp = blob->smack.skp;
> > >
> > > /*
> > > * No need to do string comparisons. If a match occurs,
> > > @@ -4833,7 +4831,6 @@ static int smack_ismaclabel(const char *name)
> > > return (strcmp(name, XATTR_SMACK_SUFFIX) == 0);
> > > }
> > >
> > > -
> > > /**
> > > * smack_secid_to_secctx - return the smack label for a secid
> > > * @secid: incoming integer
> > > @@ -4852,6 +4849,29 @@ static int smack_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
> > > return 0;
> > > }
> > >
> > > +/**
> > > + * smack_lsmblob_to_secctx - return the smack label
> > > + * @blob: includes incoming Smack data
> > > + * @secdata: destination
> > > + * @seclen: how long it is
> > > + *
> > > + * Exists for audit code.
> > > + */
> > > +static int smack_lsmblob_to_secctx(struct lsmblob *blob, char **secdata,
> > > + u32 *seclen)
> > > +{
> > > + struct smack_known *skp = blob->smack.skp;
> > > +
> > > + /* scaffolding */
> > > + if (!skp && blob->scaffold.secid)
> > > + skp = smack_from_secid(blob->scaffold.secid);
> > > +
> > > + if (secdata)
> > > + *secdata = skp->smk_known;
> > > + *seclen = strlen(skp->smk_known);
> > > + return 0;
> > > +}
> > > +
> > > /**
> > > * smack_secctx_to_secid - return the secid for a smack label
> > > * @secdata: smack label
> > > @@ -5208,6 +5228,7 @@ static struct security_hook_list smack_hooks[] __ro_after_init = {
> > >
> > > LSM_HOOK_INIT(ismaclabel, smack_ismaclabel),
> > > LSM_HOOK_INIT(secid_to_secctx, smack_secid_to_secctx),
> > > + LSM_HOOK_INIT(lsmblob_to_secctx, smack_lsmblob_to_secctx),
> > > LSM_HOOK_INIT(secctx_to_secid, smack_secctx_to_secid),
> > > LSM_HOOK_INIT(inode_notifysecctx, smack_inode_notifysecctx),
> > > LSM_HOOK_INIT(inode_setsecctx, smack_inode_setsecctx),
Powered by blists - more mailing lists