Re: [PATCH v35 21/29] LSM: Extend security_secid_to_secctx to include module selection

John Johansen
2022-04-25T23:32:34Z

On 4/18/22 07:59, Casey Schaufler wrote:
> Add a parameter to security_secid_to_secctx() to identify
> which of the security modules that may be active should
> provide the security context. If the parameter is greater
> than or equal to zero, the security module associated with
> that LSM "slot" is used. If the value is LSMBLOB_DISPLAY
> the "interface lsm" is used. If the value is LSMBLOB_FIRST
> the first security module providing a hook is used.
>

So the patch does change behavior from previously doing
effectively LSMBLOB_DISPLAY everywhere to using LSMBLOB_FIRST
in certain cases. I think the reason for the change needs
to called out. I think a note in the patch description
would do.

> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
> drivers/android/binder.c | 2 +-
> include/linux/security.h | 7 +++++--
> include/net/scm.h | 2 +-
> kernel/audit.c | 4 ++--
> kernel/auditsc.c | 7 ++++---
> net/ipv4/ip_sockglue.c | 2 +-
> net/netfilter/nf_conntrack_netlink.c | 4 ++--
> net/netfilter/nf_conntrack_standalone.c | 2 +-
> net/netfilter/nfnetlink_queue.c | 2 +-
> net/netlabel/netlabel_unlabeled.c | 11 +++++++----
> net/netlabel/netlabel_user.c | 2 +-
> security/security.c | 20 ++++++++++++++++++--
> 12 files changed, 44 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 259f5e38e6ba..d59c4ebf7e22 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2983,7 +2983,7 @@ static void binder_transaction(struct binder_proc *proc,
> size_t added_size;
>
> security_cred_getsecid(proc->cred, &blob);
> - ret = security_secid_to_secctx(&blob, &lsmctx);
> + ret = security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_DISPLAY);
> if (ret) {
> return_error = BR_FAILED_REPLY;
> return_error_param = ret;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index dc66f3f48456..2150016492be 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -184,6 +184,8 @@ struct lsmblob {
> #define LSMBLOB_INVALID -1 /* Not a valid LSM slot number */
> #define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
> #define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
> +#define LSMBLOB_DISPLAY -4 /* Use the "interface_lsm" slot */
> +#define LSMBLOB_FIRST -5 /* Use the first slot */
>
> /**
> * lsmblob_init - initialize a lsmblob structure
> @@ -615,7 +617,8 @@ int security_setprocattr(const char *lsm, 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(struct lsmblob *blob, struct lsmcontext *cp);
> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp,
> + int ilsm);
> int security_secctx_to_secid(const char *secdata, u32 seclen,
> struct lsmblob *blob);
> void security_release_secctx(struct lsmcontext *cp);
> @@ -1470,7 +1473,7 @@ static inline int security_ismaclabel(const char *name)
> }
>
> static inline int security_secid_to_secctx(struct lsmblob *blob,
> - struct lsmcontext *cp)
> + struct lsmcontext *cp, int ilsm)
> {
> return -EOPNOTSUPP;
> }
> diff --git a/include/net/scm.h b/include/net/scm.h
> index b77a52f93389..f4d567d4885e 100644
> --- a/include/net/scm.h
> +++ b/include/net/scm.h
> @@ -101,7 +101,7 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
> * and the infrastructure will know which it is.
> */
> lsmblob_init(&lb, scm->secid);
> - err = security_secid_to_secctx(&lb, &context);
> + err = security_secid_to_secctx(&lb, &context, LSMBLOB_DISPLAY);
>
> if (!err) {
> put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, context.len,
> diff --git a/kernel/audit.c b/kernel/audit.c
> index a885ebdbb91e..28ff7a5f90bd 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1464,7 +1464,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>
> if (lsmblob_is_set(&audit_sig_lsm)) {
> err = security_secid_to_secctx(&audit_sig_lsm,
> - &context);
> + &context, LSMBLOB_FIRST);
> if (err)
> return err;
> }
> @@ -2176,7 +2176,7 @@ int audit_log_task_context(struct audit_buffer *ab)
> if (!lsmblob_is_set(&blob))
> return 0;
>
> - error = security_secid_to_secctx(&blob, &context);
> + error = security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST);
>
> if (error) {
> if (error != -EINVAL)
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 802de65259d8..231631f61550 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1132,7 +1132,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
> from_kuid(&init_user_ns, auid),
> from_kuid(&init_user_ns, uid), sessionid);
> if (lsmblob_is_set(blob)) {
> - if (security_secid_to_secctx(blob, &lsmctx)) {
> + if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
> audit_log_format(ab, " obj=(none)");
> rc = 1;
> } else {
> @@ -1425,7 +1425,8 @@ static void show_special(struct audit_context *context, int *call_panic)
> struct lsmblob blob;
>
> lsmblob_init(&blob, osid);
> - if (security_secid_to_secctx(&blob, &lsmcxt)) {
> + if (security_secid_to_secctx(&blob, &lsmcxt,
> + LSMBLOB_FIRST)) {
> audit_log_format(ab, " osid=%u", osid);
> *call_panic = 1;
> } else {
> @@ -1593,7 +1594,7 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
> struct lsmcontext lsmctx;
>
> lsmblob_init(&blob, n->osid);
> - if (security_secid_to_secctx(&blob, &lsmctx)) {
> + if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
> audit_log_format(ab, " osid=%u", n->osid);
> if (call_panic)
> *call_panic = 2;
> diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
> index ad5be7707bca..9b5c44dec1e9 100644
> --- a/net/ipv4/ip_sockglue.c
> +++ b/net/ipv4/ip_sockglue.c
> @@ -140,7 +140,7 @@ static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
> return;
>
> lsmblob_init(&lb, secid);
> - err = security_secid_to_secctx(&lb, &context);
> + err = security_secid_to_secctx(&lb, &context, LSMBLOB_DISPLAY);
> if (err)
> return;
>
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 07660c7dd342..5d72d2f41562 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -353,7 +353,7 @@ static int ctnetlink_dump_secctx(struct sk_buff *skb, const struct nf_conn *ct)
> * security_secid_to_secctx() will know which security module
> * to use to create the secctx. */
> lsmblob_init(&blob, ct->secmark);
> - ret = security_secid_to_secctx(&blob, &context);
> + ret = security_secid_to_secctx(&blob, &context, LSMBLOB_DISPLAY);
> if (ret)
> return 0;
>
> @@ -663,7 +663,7 @@ static inline int ctnetlink_secctx_size(const struct nf_conn *ct)
> int len;
> struct lsmblob blob;
>
> - len = security_secid_to_secctx(&blob, NULL);
> + len = security_secid_to_secctx(&blob, NULL, LSMBLOB_DISPLAY);
> if (len <= 0)
> return 0;
>
> diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
> index 36338660df3c..cb4b8b636f6a 100644
> --- a/net/netfilter/nf_conntrack_standalone.c
> +++ b/net/netfilter/nf_conntrack_standalone.c
> @@ -180,7 +180,7 @@ static void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
> struct lsmcontext context;
>
> lsmblob_init(&blob, ct->secmark);
> - ret = security_secid_to_secctx(&blob, &context);
> + ret = security_secid_to_secctx(&blob, &context, LSMBLOB_DISPLAY);
> if (ret)
> return;
>
> diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
> index f60a0b6240ff..844955b2e163 100644
> --- a/net/netfilter/nfnetlink_queue.c
> +++ b/net/netfilter/nfnetlink_queue.c
> @@ -316,7 +316,7 @@ static void nfqnl_get_sk_secctx(struct sk_buff *skb, struct lsmcontext *context)
> * blob. security_secid_to_secctx() will know which security
> * module to use to create the secctx. */
> lsmblob_init(&blob, skb->secmark);
> - security_secid_to_secctx(&blob, context);
> + security_secid_to_secctx(&blob, context, LSMBLOB_DISPLAY);
> }
>
> read_unlock_bh(&skb->sk->sk_callback_lock);
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index 910a03f15b0d..8deee7e176a9 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -437,7 +437,8 @@ int netlbl_unlhsh_add(struct net *net,
> unlhsh_add_return:
> rcu_read_unlock();
> if (audit_buf != NULL) {
> - if (security_secid_to_secctx(lsmblob, &context) == 0) {
> + if (security_secid_to_secctx(lsmblob, &context,
> + LSMBLOB_FIRST) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s",
> context.context);
> security_release_secctx(&context);
> @@ -491,7 +492,8 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
> addr->s_addr, mask->s_addr);
> dev_put(dev);
> if (entry != NULL &&
> - security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
> + security_secid_to_secctx(&entry->lsmblob, &context,
> + LSMBLOB_FIRST) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s",
> context.context);
> security_release_secctx(&context);
> @@ -550,7 +552,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
> addr, mask);
> dev_put(dev);
> if (entry != NULL &&
> - security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
> + security_secid_to_secctx(&entry->lsmblob, &context,
> + LSMBLOB_FIRST) == 0) {
> audit_log_format(audit_buf, " sec_obj=%s",
> context.context);
> security_release_secctx(&context);
> @@ -1120,7 +1123,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
> lsmb = (struct lsmblob *)&addr6->lsmblob;
> }
>
> - ret_val = security_secid_to_secctx(lsmb, &context);
> + ret_val = security_secid_to_secctx(lsmb, &context, LSMBLOB_FIRST);
> if (ret_val != 0)
> goto list_cb_failure;
> ret_val = nla_put(cb_arg->skb,
> diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
> index 951ba0639d20..1941877fd16f 100644
> --- a/net/netlabel/netlabel_user.c
> +++ b/net/netlabel/netlabel_user.c
> @@ -100,7 +100,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
>
> lsmblob_init(&blob, audit_info->secid);
> if (audit_info->secid != 0 &&
> - security_secid_to_secctx(&blob, &context) == 0) {
> + security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST) == 0) {
> audit_log_format(audit_buf, " subj=%s", context.context);
> security_release_secctx(&context);
> }
> diff --git a/security/security.c b/security/security.c
> index 64073d807240..be6682768760 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2347,20 +2347,36 @@ EXPORT_SYMBOL(security_ismaclabel);
> * security_secid_to_secctx - convert secid to secctx
> * @blob: set of secids
> * @cp: lsm context into which result is put
> + * @ilsm: which security module to report
> *
> * Translate secid information into a secctx string.
> * Return a negative value on error.
> * If cp is NULL return the length of the string.
> * Otherwise, return 0.
> */
> -int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp)
> +int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp,
> + int ilsm)
> {
> struct security_hook_list *hp;
> - int ilsm = lsm_task_ilsm(current);
>
> if (cp)
> memset(cp, 0, sizeof(*cp));
>
> + /*
> + * ilsm either is the slot number use for formatting
> + * or an instruction on which relative slot to use.
> + */
> + if (ilsm == LSMBLOB_DISPLAY)
> + ilsm = lsm_task_ilsm(current);
> + else if (ilsm == LSMBLOB_FIRST)
> + ilsm = LSMBLOB_INVALID;
> + else if (ilsm < 0) {
> + WARN_ONCE(true, "LSM: %s unknown interface LSM\n", __func__);
> + ilsm = LSMBLOB_INVALID;
> + } else if (ilsm >= lsm_slot) {
> + WARN_ONCE(true, "LSM: %s invalid interface LSM\n", __func__);
> + ilsm = LSMBLOB_INVALID;
> + }
> hlist_for_each_entry(hp, &security_hook_heads.secid_to_secctx, list) {
> if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
> continue;