* [PATCH v10 06/25] LSM: Use lsmblob in security_secctx_to_secid
From: Casey Schaufler @ 2019-11-13 0:00 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
Change security_secctx_to_secid() to fill in a lsmblob instead
of a u32 secid. Multiple LSMs may be able to interpret the
string, and this allows for setting whichever secid is
appropriate. In some cases there is scaffolding where other
interfaces have yet to be converted.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/security.h | 5 +++--
kernel/cred.c | 4 +---
net/netfilter/nft_meta.c | 13 ++++++-------
net/netfilter/xt_SECMARK.c | 5 ++++-
net/netlabel/netlabel_unlabeled.c | 14 ++++++++------
security/security.c | 18 +++++++++++++++---
6 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 02ff6250bf2b..2cc0588311b9 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -439,7 +439,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
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_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
+int security_secctx_to_secid(const char *secdata, u32 seclen,
+ struct lsmblob *blob);
void security_release_secctx(char *secdata, u32 seclen);
void security_inode_invalidate_secctx(struct inode *inode);
@@ -1222,7 +1223,7 @@ static inline int security_secid_to_secctx(u32 secid, char **secdata, u32 *secle
static inline int security_secctx_to_secid(const char *secdata,
u32 seclen,
- u32 *secid)
+ struct lsmblob *blob)
{
return -EOPNOTSUPP;
}
diff --git a/kernel/cred.c b/kernel/cred.c
index 71c14dda107e..d70a2c02ced4 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -725,14 +725,12 @@ EXPORT_SYMBOL(set_security_override);
int set_security_override_from_ctx(struct cred *new, const char *secctx)
{
struct lsmblob blob;
- u32 secid;
int ret;
- ret = security_secctx_to_secid(secctx, strlen(secctx), &secid);
+ ret = security_secctx_to_secid(secctx, strlen(secctx), &blob);
if (ret < 0)
return ret;
- lsmblob_init(&blob, secid);
return set_security_override(new, &blob);
}
EXPORT_SYMBOL(set_security_override_from_ctx);
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 987d2d6ce624..054fb4b48d51 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -576,21 +576,20 @@ static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
static int nft_secmark_compute_secid(struct nft_secmark *priv)
{
- u32 tmp_secid = 0;
+ struct lsmblob blob;
int err;
- err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &tmp_secid);
+ err = security_secctx_to_secid(priv->ctx, strlen(priv->ctx), &blob);
if (err)
return err;
- if (!tmp_secid)
- return -ENOENT;
-
- err = security_secmark_relabel_packet(tmp_secid);
+ /* Using le[0] is scaffolding */
+ err = security_secmark_relabel_packet(blob.secid[0]);
if (err)
return err;
- priv->secid = tmp_secid;
+ /* Using le[0] is scaffolding */
+ priv->secid = blob.secid[0];
return 0;
}
diff --git a/net/netfilter/xt_SECMARK.c b/net/netfilter/xt_SECMARK.c
index f16202d26c20..8081fadc30e9 100644
--- a/net/netfilter/xt_SECMARK.c
+++ b/net/netfilter/xt_SECMARK.c
@@ -49,13 +49,14 @@ secmark_tg(struct sk_buff *skb, const struct xt_action_param *par)
static int checkentry_lsm(struct xt_secmark_target_info *info)
{
+ struct lsmblob blob;
int err;
info->secctx[SECMARK_SECCTX_MAX - 1] = '\0';
info->secid = 0;
err = security_secctx_to_secid(info->secctx, strlen(info->secctx),
- &info->secid);
+ &blob);
if (err) {
if (err == -EINVAL)
pr_info_ratelimited("invalid security context \'%s\'\n",
@@ -63,6 +64,8 @@ static int checkentry_lsm(struct xt_secmark_target_info *info)
return err;
}
+ /* scaffolding during the transition */
+ info->secid = blob.secid[0];
if (!info->secid) {
pr_info_ratelimited("unable to map security context \'%s\'\n",
info->secctx);
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index c92894c3e40a..2976370e41aa 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -895,7 +895,7 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
void *addr;
void *mask;
u32 addr_len;
- u32 secid;
+ struct lsmblob blob;
struct netlbl_audit audit_info;
/* Don't allow users to add both IPv4 and IPv6 addresses for a
@@ -919,12 +919,13 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
ret_val = security_secctx_to_secid(
nla_data(info->attrs[NLBL_UNLABEL_A_SECCTX]),
nla_len(info->attrs[NLBL_UNLABEL_A_SECCTX]),
- &secid);
+ &blob);
if (ret_val != 0)
return ret_val;
+ /* scaffolding with the [0] */
return netlbl_unlhsh_add(&init_net,
- dev_name, addr, mask, addr_len, secid,
+ dev_name, addr, mask, addr_len, blob.secid[0],
&audit_info);
}
@@ -946,7 +947,7 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
void *addr;
void *mask;
u32 addr_len;
- u32 secid;
+ struct lsmblob blob;
struct netlbl_audit audit_info;
/* Don't allow users to add both IPv4 and IPv6 addresses for a
@@ -968,12 +969,13 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
ret_val = security_secctx_to_secid(
nla_data(info->attrs[NLBL_UNLABEL_A_SECCTX]),
nla_len(info->attrs[NLBL_UNLABEL_A_SECCTX]),
- &secid);
+ &blob);
if (ret_val != 0)
return ret_val;
+ /* scaffolding with the [0] */
return netlbl_unlhsh_add(&init_net,
- NULL, addr, mask, addr_len, secid,
+ NULL, addr, mask, addr_len, blob.secid[0],
&audit_info);
}
diff --git a/security/security.c b/security/security.c
index bd685be33b56..8e4f41d9af0f 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1921,10 +1921,22 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
}
EXPORT_SYMBOL(security_secid_to_secctx);
-int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
+int security_secctx_to_secid(const char *secdata, u32 seclen,
+ struct lsmblob *blob)
{
- *secid = 0;
- return call_int_hook(secctx_to_secid, 0, secdata, seclen, secid);
+ struct security_hook_list *hp;
+ int rc;
+
+ lsmblob_init(blob, 0);
+ hlist_for_each_entry(hp, &security_hook_heads.secctx_to_secid, list) {
+ if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
+ continue;
+ rc = hp->hook.secctx_to_secid(secdata, seclen,
+ &blob->secid[hp->lsmid->slot]);
+ if (rc != 0)
+ return rc;
+ }
+ return 0;
}
EXPORT_SYMBOL(security_secctx_to_secid);
--
2.20.1
^ permalink raw reply related
* [PATCH v10 05/25] net: Prepare UDS for security module stacking
From: Casey Schaufler @ 2019-11-13 0:00 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
Change the data used in UDS SO_PEERSEC processing from a
secid to a more general struct lsmblob. Update the
security_socket_getpeersec_dgram() interface to use the
lsmblob. There is a small amount of scaffolding code
that will come out when the security_secid_to_secctx()
code is brought in line with the lsmblob.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/security.h | 7 +++++--
include/net/af_unix.h | 2 +-
include/net/scm.h | 8 +++++---
net/ipv4/ip_sockglue.c | 8 +++++---
net/unix/af_unix.c | 6 +++---
security/security.c | 18 +++++++++++++++---
6 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index cd09e7b1df9f..02ff6250bf2b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1272,7 +1272,8 @@ int security_socket_shutdown(struct socket *sock, int how);
int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb);
int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
int __user *optlen, unsigned len);
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
+int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+ struct lsmblob *blob);
int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
void security_sk_free(struct sock *sk);
void security_sk_clone(const struct sock *sk, struct sock *newsk);
@@ -1410,7 +1411,9 @@ static inline int security_socket_getpeersec_stream(struct socket *sock, char __
return -ENOPROTOOPT;
}
-static inline int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
+static inline int security_socket_getpeersec_dgram(struct socket *sock,
+ struct sk_buff *skb,
+ struct lsmblob *blob)
{
return -ENOPROTOOPT;
}
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 3426d6dacc45..933492c08b8c 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -36,7 +36,7 @@ struct unix_skb_parms {
kgid_t gid;
struct scm_fp_list *fp; /* Passed files */
#ifdef CONFIG_SECURITY_NETWORK
- u32 secid; /* Security ID */
+ struct lsmblob lsmblob; /* Security LSM data */
#endif
u32 consumed;
} __randomize_layout;
diff --git a/include/net/scm.h b/include/net/scm.h
index 1ce365f4c256..e2e71c4bf9d0 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -33,7 +33,7 @@ struct scm_cookie {
struct scm_fp_list *fp; /* Passed files */
struct scm_creds creds; /* Skb credentials */
#ifdef CONFIG_SECURITY_NETWORK
- u32 secid; /* Passed security ID */
+ struct lsmblob lsmblob; /* Passed LSM data */
#endif
};
@@ -46,7 +46,7 @@ struct scm_fp_list *scm_fp_dup(struct scm_fp_list *fpl);
#ifdef CONFIG_SECURITY_NETWORK
static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
{
- security_socket_getpeersec_dgram(sock, NULL, &scm->secid);
+ security_socket_getpeersec_dgram(sock, NULL, &scm->lsmblob);
}
#else
static __inline__ void unix_get_peersec_dgram(struct socket *sock, struct scm_cookie *scm)
@@ -97,7 +97,9 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
int err;
if (test_bit(SOCK_PASSSEC, &sock->flags)) {
- err = security_secid_to_secctx(scm->secid, &secdata, &seclen);
+ /* Scaffolding - it has to be element 0 for now */
+ err = security_secid_to_secctx(scm->lsmblob.secid[0],
+ &secdata, &seclen);
if (!err) {
put_cmsg(msg, SOL_SOCKET, SCM_SECURITY, seclen, secdata);
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 82f341e84fae..2a5c868ce135 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -130,15 +130,17 @@ static void ip_cmsg_recv_checksum(struct msghdr *msg, struct sk_buff *skb,
static void ip_cmsg_recv_security(struct msghdr *msg, struct sk_buff *skb)
{
+ struct lsmblob lb;
char *secdata;
- u32 seclen, secid;
+ u32 seclen;
int err;
- err = security_socket_getpeersec_dgram(NULL, skb, &secid);
+ err = security_socket_getpeersec_dgram(NULL, skb, &lb);
if (err)
return;
- err = security_secid_to_secctx(secid, &secdata, &seclen);
+ /* Scaffolding - it has to be element 0 */
+ err = security_secid_to_secctx(lb.secid[0], &secdata, &seclen);
if (err)
return;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index ddb838a1b74c..c50a004a1389 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -143,17 +143,17 @@ static struct hlist_head *unix_sockets_unbound(void *addr)
#ifdef CONFIG_SECURITY_NETWORK
static void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
{
- UNIXCB(skb).secid = scm->secid;
+ UNIXCB(skb).lsmblob = scm->lsmblob;
}
static inline void unix_set_secdata(struct scm_cookie *scm, struct sk_buff *skb)
{
- scm->secid = UNIXCB(skb).secid;
+ scm->lsmblob = UNIXCB(skb).lsmblob;
}
static inline bool unix_secdata_eq(struct scm_cookie *scm, struct sk_buff *skb)
{
- return (scm->secid == UNIXCB(skb).secid);
+ return lsmblob_equal(&scm->lsmblob, &(UNIXCB(skb).lsmblob));
}
#else
static inline void unix_get_secdata(struct scm_cookie *scm, struct sk_buff *skb)
diff --git a/security/security.c b/security/security.c
index 7879da7025d2..bd685be33b56 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2059,10 +2059,22 @@ int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
optval, optlen, len);
}
-int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)
+int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
+ struct lsmblob *blob)
{
- return call_int_hook(socket_getpeersec_dgram, -ENOPROTOOPT, sock,
- skb, secid);
+ struct security_hook_list *hp;
+ int rc = -ENOPROTOOPT;
+
+ hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_dgram,
+ list) {
+ if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
+ continue;
+ rc = hp->hook.socket_getpeersec_dgram(sock, skb,
+ &blob->secid[hp->lsmid->slot]);
+ if (rc != 0)
+ break;
+ }
+ return rc;
}
EXPORT_SYMBOL(security_socket_getpeersec_dgram);
--
2.20.1
^ permalink raw reply related
* [PATCH v10 04/25] LSM: Use lsmblob in security_kernel_act_as
From: Casey Schaufler @ 2019-11-13 0:00 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
Change the security_kernel_act_as interface to use a lsmblob
structure in place of the single u32 secid in support of
module stacking. Change it's only caller, set_security_override,
to do the same. Change that one's only caller,
set_security_override_from_ctx, to call it with the new
parameter type.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/cred.h | 3 ++-
include/linux/security.h | 5 +++--
kernel/cred.c | 10 ++++++----
security/security.c | 14 ++++++++++++--
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index efb6edf32de7..9a21c376ed97 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -22,6 +22,7 @@
struct cred;
struct inode;
+struct lsmblob;
/*
* COW Supplementary groups list
@@ -165,7 +166,7 @@ extern const struct cred *override_creds(const struct cred *);
extern void revert_creds(const struct cred *);
extern struct cred *prepare_kernel_cred(struct task_struct *);
extern int change_create_files_as(struct cred *, struct inode *);
-extern int set_security_override(struct cred *, u32);
+extern int set_security_override(struct cred *, struct lsmblob *);
extern int set_security_override_from_ctx(struct cred *, const char *);
extern int set_create_files_as(struct cred *, struct inode *);
extern int cred_fscmp(const struct cred *, const struct cred *);
diff --git a/include/linux/security.h b/include/linux/security.h
index c22fa5bfaad8..cd09e7b1df9f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -380,7 +380,7 @@ void security_cred_free(struct cred *cred);
int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
void security_transfer_creds(struct cred *new, const struct cred *old);
void security_cred_getsecid(const struct cred *c, u32 *secid);
-int security_kernel_act_as(struct cred *new, u32 secid);
+int security_kernel_act_as(struct cred *new, struct lsmblob *blob);
int security_kernel_create_files_as(struct cred *new, struct inode *inode);
int security_kernel_module_request(char *kmod_name);
int security_kernel_load_data(enum kernel_load_data_id id);
@@ -963,7 +963,8 @@ static inline void security_transfer_creds(struct cred *new,
{
}
-static inline int security_kernel_act_as(struct cred *cred, u32 secid)
+static inline int security_kernel_act_as(struct cred *cred,
+ struct lsmblob *blob)
{
return 0;
}
diff --git a/kernel/cred.c b/kernel/cred.c
index 45d77284aed0..71c14dda107e 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -701,14 +701,14 @@ EXPORT_SYMBOL(prepare_kernel_cred);
/**
* set_security_override - Set the security ID in a set of credentials
* @new: The credentials to alter
- * @secid: The LSM security ID to set
+ * @blob: The LSM security information to set
*
* Set the LSM security ID in a set of credentials so that the subjective
* security is overridden when an alternative set of credentials is used.
*/
-int set_security_override(struct cred *new, u32 secid)
+int set_security_override(struct cred *new, struct lsmblob *blob)
{
- return security_kernel_act_as(new, secid);
+ return security_kernel_act_as(new, blob);
}
EXPORT_SYMBOL(set_security_override);
@@ -724,6 +724,7 @@ EXPORT_SYMBOL(set_security_override);
*/
int set_security_override_from_ctx(struct cred *new, const char *secctx)
{
+ struct lsmblob blob;
u32 secid;
int ret;
@@ -731,7 +732,8 @@ int set_security_override_from_ctx(struct cred *new, const char *secctx)
if (ret < 0)
return ret;
- return set_security_override(new, secid);
+ lsmblob_init(&blob, secid);
+ return set_security_override(new, &blob);
}
EXPORT_SYMBOL(set_security_override_from_ctx);
diff --git a/security/security.c b/security/security.c
index 8fd5c8cd4f50..7879da7025d2 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1566,9 +1566,19 @@ void security_cred_getsecid(const struct cred *c, u32 *secid)
}
EXPORT_SYMBOL(security_cred_getsecid);
-int security_kernel_act_as(struct cred *new, u32 secid)
+int security_kernel_act_as(struct cred *new, struct lsmblob *blob)
{
- return call_int_hook(kernel_act_as, 0, new, secid);
+ struct security_hook_list *hp;
+ int rc;
+
+ hlist_for_each_entry(hp, &security_hook_heads.kernel_act_as, list) {
+ if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
+ continue;
+ rc = hp->hook.kernel_act_as(new, blob->secid[hp->lsmid->slot]);
+ if (rc != 0)
+ return rc;
+ }
+ return 0;
}
int security_kernel_create_files_as(struct cred *new, struct inode *inode)
--
2.20.1
^ permalink raw reply related
* [PATCH v10 02/25] LSM: Create and manage the lsmblob data structure.
From: Casey Schaufler @ 2019-11-12 23:59 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
When more than one security module is exporting data to
audit and networking sub-systems a single 32 bit integer
is no longer sufficient to represent the data. Add a
structure to be used instead.
The lsmblob structure is currently an array of
u32 "secids". There is an entry for each of the
security modules built into the system that would
use secids if active. The system assigns the module
a "slot" when it registers hooks. If modules are
compiled in but not registered there will be unused
slots.
A new lsm_id structure, which contains the name
of the LSM and its slot number, is created. There
is an instance for each LSM, which assigns the name
and passes it to the infrastructure to set the slot.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/lsm_hooks.h | 12 ++++++--
include/linux/security.h | 58 ++++++++++++++++++++++++++++++++++++++
security/apparmor/lsm.c | 7 ++++-
security/commoncap.c | 7 ++++-
security/loadpin/loadpin.c | 8 +++++-
security/safesetid/lsm.c | 8 +++++-
security/security.c | 31 +++++++++++++++-----
security/selinux/hooks.c | 8 +++++-
security/smack/smack_lsm.c | 7 ++++-
security/tomoyo/tomoyo.c | 8 +++++-
security/yama/yama_lsm.c | 7 ++++-
11 files changed, 144 insertions(+), 17 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 9a80c7e94785..24b7d78a36b2 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2029,6 +2029,14 @@ struct security_hook_heads {
#endif /* CONFIG_BPF_SYSCALL */
} __randomize_layout;
+/*
+ * Information that identifies a security module.
+ */
+struct lsm_id {
+ const char *lsm; /* Name of the LSM */
+ int slot; /* Slot in lsmblob if one is allocated */
+};
+
/*
* Security module hook list structure.
* For use with generic list macros for common operations.
@@ -2037,7 +2045,7 @@ struct security_hook_list {
struct hlist_node list;
struct hlist_head *head;
union security_list_options hook;
- char *lsm;
+ struct lsm_id *lsmid;
} __randomize_layout;
/*
@@ -2066,7 +2074,7 @@ extern struct security_hook_heads security_hook_heads;
extern char *lsm_names;
extern void security_add_hooks(struct security_hook_list *hooks, int count,
- char *lsm);
+ struct lsm_id *lsmid);
#define LSM_FLAG_LEGACY_MAJOR BIT(0)
#define LSM_FLAG_EXCLUSIVE BIT(1)
diff --git a/include/linux/security.h b/include/linux/security.h
index 49f2685324b0..260760a6f6db 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -76,6 +76,64 @@ enum lsm_event {
LSM_POLICY_CHANGE,
};
+/*
+ * Data exported by the security modules
+ *
+ * Any LSM that provides secid or secctx based hooks must be included.
+ */
+#define LSMBLOB_ENTRIES ( \
+ (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \
+ (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0))
+
+struct lsmblob {
+ u32 secid[LSMBLOB_ENTRIES];
+};
+
+#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 */
+
+/**
+ * lsmblob_init - initialize an lsmblob structure.
+ * @blob: Pointer to the data to initialize
+ * @secid: The initial secid value
+ *
+ * Set all secid for all modules to the specified value.
+ */
+static inline void lsmblob_init(struct lsmblob *blob, u32 secid)
+{
+ int i;
+
+ for (i = 0; i < LSMBLOB_ENTRIES; i++)
+ blob->secid[i] = secid;
+}
+
+/**
+ * lsmblob_is_set - report if there is an value in the lsmblob
+ * @blob: Pointer to the exported LSM data
+ *
+ * Returns true if there is a secid set, false otherwise
+ */
+static inline bool lsmblob_is_set(struct lsmblob *blob)
+{
+ struct lsmblob empty = {};
+
+ return !!memcmp(blob, &empty, sizeof(*blob));
+}
+
+/**
+ * lsmblob_equal - report if the two lsmblob's are equal
+ * @bloba: Pointer to one LSM data
+ * @blobb: Pointer to the other LSM data
+ *
+ * Returns true if all entries in the two are equal, false otherwise
+ */
+static inline bool lsmblob_equal(struct lsmblob *bloba, struct lsmblob *blobb)
+{
+ return !memcmp(bloba, blobb, sizeof(*bloba));
+}
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
int cap, unsigned int opts);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 2716e7731279..ec2e39aa9a84 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1138,6 +1138,11 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
.lbs_sock = sizeof(struct aa_sk_ctx),
};
+static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
+ .lsm = "apparmor",
+ .slot = LSMBLOB_NEEDED
+};
+
static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
@@ -1679,7 +1684,7 @@ static int __init apparmor_init(void)
goto buffers_out;
}
security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
- "apparmor");
+ &apparmor_lsmid);
/* Report that AppArmor successfully initialized */
apparmor_initialized = 1;
diff --git a/security/commoncap.c b/security/commoncap.c
index afd9679ca866..973e6c7009d0 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1344,6 +1344,11 @@ int cap_mmap_file(struct file *file, unsigned long reqprot,
#ifdef CONFIG_SECURITY
+static struct lsm_id capability_lsmid __lsm_ro_after_init = {
+ .lsm = "capability",
+ .slot = LSMBLOB_NOT_NEEDED
+};
+
static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(capable, cap_capable),
LSM_HOOK_INIT(settime, cap_settime),
@@ -1368,7 +1373,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = {
static int __init capability_init(void)
{
security_add_hooks(capability_hooks, ARRAY_SIZE(capability_hooks),
- "capability");
+ &capability_lsmid);
return 0;
}
diff --git a/security/loadpin/loadpin.c b/security/loadpin/loadpin.c
index 055fb0a64169..7b23fdf24e27 100644
--- a/security/loadpin/loadpin.c
+++ b/security/loadpin/loadpin.c
@@ -181,6 +181,11 @@ static int loadpin_load_data(enum kernel_load_data_id id)
return loadpin_read_file(NULL, (enum kernel_read_file_id) id);
}
+static struct lsm_id loadpin_lsmid __lsm_ro_after_init = {
+ .lsm = "loadpin",
+ .slot = LSMBLOB_NOT_NEEDED
+};
+
static struct security_hook_list loadpin_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(sb_free_security, loadpin_sb_free_security),
LSM_HOOK_INIT(kernel_read_file, loadpin_read_file),
@@ -191,7 +196,8 @@ static int __init loadpin_init(void)
{
pr_info("ready to pin (currently %senforcing)\n",
enforce ? "" : "not ");
- security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks), "loadpin");
+ security_add_hooks(loadpin_hooks, ARRAY_SIZE(loadpin_hooks),
+ &loadpin_lsmid);
return 0;
}
diff --git a/security/safesetid/lsm.c b/security/safesetid/lsm.c
index cecd38e2ac80..4a96cd8c0d15 100644
--- a/security/safesetid/lsm.c
+++ b/security/safesetid/lsm.c
@@ -255,6 +255,11 @@ void flush_safesetid_whitelist_entries(void)
}
}
+static struct lsm_id safesetid_lsmid __lsm_ro_after_init = {
+ .lsm = "safesetid",
+ .slot = LSMBLOB_NOT_NEEDED
+};
+
static struct security_hook_list safesetid_security_hooks[] = {
LSM_HOOK_INIT(task_fix_setuid, safesetid_task_fix_setuid),
LSM_HOOK_INIT(capable, safesetid_security_capable)
@@ -263,7 +268,8 @@ static struct security_hook_list safesetid_security_hooks[] = {
static int __init safesetid_security_init(void)
{
security_add_hooks(safesetid_security_hooks,
- ARRAY_SIZE(safesetid_security_hooks), "safesetid");
+ ARRAY_SIZE(safesetid_security_hooks),
+ &safesetid_lsmid);
/* Report that SafeSetID successfully initialized */
safesetid_initialized = 1;
diff --git a/security/security.c b/security/security.c
index 757a8ee4da65..1cae4faaa279 100644
--- a/security/security.c
+++ b/security/security.c
@@ -309,6 +309,7 @@ static void __init ordered_lsm_init(void)
init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
init_debug("task blob size = %d\n", blob_sizes.lbs_task);
+ init_debug("lsmblob size = %lu\n", sizeof(struct lsmblob));
/*
* Create any kmem_caches needed for blobs
@@ -391,7 +392,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
return !strcmp(last, lsm);
}
-static int lsm_append(char *new, char **result)
+static int lsm_append(const char *new, char **result)
{
char *cp;
@@ -412,24 +413,40 @@ static int lsm_append(char *new, char **result)
return 0;
}
+/*
+ * Current index to use while initializing the lsmblob secid list.
+ */
+static int lsm_slot __initdata;
+
/**
* security_add_hooks - Add a modules hooks to the hook lists.
* @hooks: the hooks to add
* @count: the number of hooks to add
- * @lsm: the name of the security module
+ * @lsmid: the identification information for the security module
*
* Each LSM has to register its hooks with the infrastructure.
+ * If the LSM is using hooks that export secids allocate a slot
+ * for it in the lsmblob.
*/
void __init security_add_hooks(struct security_hook_list *hooks, int count,
- char *lsm)
+ struct lsm_id *lsmid)
{
int i;
+ if (lsmid->slot == LSMBLOB_NEEDED) {
+ if (lsm_slot >= LSMBLOB_ENTRIES)
+ panic("%s Too many LSMs registered.\n", __func__);
+ lsmid->slot = lsm_slot++;
+ init_debug("%s assigned lsmblob slot %d\n", lsmid->lsm,
+ lsmid->slot);
+ }
+
for (i = 0; i < count; i++) {
- hooks[i].lsm = lsm;
+ hooks[i].lsmid = lsmid;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
}
- if (lsm_append(lsm, &lsm_names) < 0)
+
+ if (lsm_append(lsmid->lsm, &lsm_names) < 0)
panic("%s - Cannot get early memory.\n", __func__);
}
@@ -1856,7 +1873,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.getprocattr(p, name, value);
}
@@ -1869,7 +1886,7 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsm))
+ if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
return hp->hook.setprocattr(name, value, size);
}
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d4a1304f1e99..7592b95b43c4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6645,6 +6645,11 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
.lbs_sock = sizeof(struct sk_security_struct),
};
+static struct lsm_id selinux_lsmid __lsm_ro_after_init = {
+ .lsm = "selinux",
+ .slot = LSMBLOB_NEEDED
+};
+
static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
@@ -6902,7 +6907,8 @@ static __init int selinux_init(void)
hashtab_cache_init();
- security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks), "selinux");
+ security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks),
+ &selinux_lsmid);
if (avc_add_callback(selinux_netcache_avc_callback, AVC_CALLBACK_RESET))
panic("SELinux: Unable to register AVC netcache callback\n");
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e64eb558334a..61e05fe86013 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4571,6 +4571,11 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_sock = sizeof(struct socket_smack),
};
+static struct lsm_id smack_lsmid __lsm_ro_after_init = {
+ .lsm = "smack",
+ .slot = LSMBLOB_NEEDED
+};
+
static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, smack_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, smack_ptrace_traceme),
@@ -4763,7 +4768,7 @@ static __init int smack_init(void)
/*
* Register with LSM
*/
- security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), "smack");
+ security_add_hooks(smack_hooks, ARRAY_SIZE(smack_hooks), &smack_lsmid);
smack_enabled = 1;
pr_info("Smack: Initializing.\n");
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 716c92ec941a..f1968e80f06d 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -529,6 +529,11 @@ static void tomoyo_task_free(struct task_struct *task)
}
}
+static struct lsm_id tomoyo_lsmid __lsm_ro_after_init = {
+ .lsm = "tomoyo",
+ .slot = LSMBLOB_NOT_NEEDED
+};
+
/*
* tomoyo_security_ops is a "struct security_operations" which is used for
* registering TOMOYO.
@@ -581,7 +586,8 @@ static int __init tomoyo_init(void)
struct tomoyo_task *s = tomoyo_task(current);
/* register ourselves with the security framework */
- security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks), "tomoyo");
+ security_add_hooks(tomoyo_hooks, ARRAY_SIZE(tomoyo_hooks),
+ &tomoyo_lsmid);
pr_info("TOMOYO Linux initialized\n");
s->domain_info = &tomoyo_kernel_domain;
atomic_inc(&tomoyo_kernel_domain.users);
diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
index efac68556b45..0529ecc86954 100644
--- a/security/yama/yama_lsm.c
+++ b/security/yama/yama_lsm.c
@@ -425,6 +425,11 @@ static int yama_ptrace_traceme(struct task_struct *parent)
return rc;
}
+static struct lsm_id yama_lsmid __lsm_ro_after_init = {
+ .lsm = "yama",
+ .slot = LSMBLOB_NOT_NEEDED
+};
+
static struct security_hook_list yama_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(ptrace_access_check, yama_ptrace_access_check),
LSM_HOOK_INIT(ptrace_traceme, yama_ptrace_traceme),
@@ -482,7 +487,7 @@ static inline void yama_init_sysctl(void) { }
static int __init yama_init(void)
{
pr_info("Yama: becoming mindful.\n");
- security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), "yama");
+ security_add_hooks(yama_hooks, ARRAY_SIZE(yama_hooks), &yama_lsmid);
yama_init_sysctl();
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v10 03/25] LSM: Use lsmblob in security_audit_rule_match
From: Casey Schaufler @ 2019-11-13 0:00 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
Change the secid parameter of security_audit_rule_match
to a lsmblob structure pointer. Pass the entry from the
lsmblob structure for the approprite slot to the LSM hook.
Change the users of security_audit_rule_match to use the
lsmblob instead of a u32. In some cases this requires a
temporary conversion using lsmblob_init() that will go
away when other interfaces get converted.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/security.h | 7 ++++---
kernel/auditfilter.c | 7 +++++--
kernel/auditsc.c | 14 ++++++++++----
security/integrity/ima/ima.h | 4 ++--
security/integrity/ima/ima_policy.c | 7 +++++--
security/security.c | 18 +++++++++++++++---
6 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 260760a6f6db..c22fa5bfaad8 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1753,7 +1753,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer)
#ifdef CONFIG_SECURITY
int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule);
int security_audit_rule_known(struct audit_krule *krule);
-int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule);
+int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+ void *lsmrule);
void security_audit_rule_free(void *lsmrule);
#else
@@ -1769,8 +1770,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule)
return 0;
}
-static inline int security_audit_rule_match(u32 secid, u32 field, u32 op,
- void *lsmrule)
+static inline int security_audit_rule_match(struct lsmblob *blob, u32 field,
+ u32 op, void *lsmrule)
{
return 0;
}
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 63f8b3f26fab..8786b95b60bd 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1324,6 +1324,7 @@ int audit_filter(int msgtype, unsigned int listtype)
struct audit_field *f = &e->rule.fields[i];
pid_t pid;
u32 sid;
+ struct lsmblob blob;
switch (f->type) {
case AUDIT_PID:
@@ -1354,8 +1355,10 @@ int audit_filter(int msgtype, unsigned int listtype)
case AUDIT_SUBJ_CLR:
if (f->lsm_rule) {
security_task_getsecid(current, &sid);
- result = security_audit_rule_match(sid,
- f->type, f->op, f->lsm_rule);
+ lsmblob_init(&blob, sid);
+ result = security_audit_rule_match(
+ &blob, f->type,
+ f->op, f->lsm_rule);
}
break;
case AUDIT_EXE:
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index d1eab1d4a930..18ee5556c086 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk,
const struct cred *cred;
int i, need_sid = 1;
u32 sid;
+ struct lsmblob blob;
unsigned int sessionid;
cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation);
@@ -630,7 +631,9 @@ static int audit_filter_rules(struct task_struct *tsk,
security_task_getsecid(tsk, &sid);
need_sid = 0;
}
- result = security_audit_rule_match(sid, f->type,
+ lsmblob_init(&blob, sid);
+ result = security_audit_rule_match(&blob,
+ f->type,
f->op,
f->lsm_rule);
}
@@ -645,15 +648,17 @@ static int audit_filter_rules(struct task_struct *tsk,
if (f->lsm_rule) {
/* Find files that match */
if (name) {
+ lsmblob_init(&blob, name->osid);
result = security_audit_rule_match(
- name->osid,
+ &blob,
f->type,
f->op,
f->lsm_rule);
} else if (ctx) {
list_for_each_entry(n, &ctx->names_list, list) {
+ lsmblob_init(&blob, n->osid);
if (security_audit_rule_match(
- n->osid,
+ &blob,
f->type,
f->op,
f->lsm_rule)) {
@@ -665,7 +670,8 @@ static int audit_filter_rules(struct task_struct *tsk,
/* Find ipc objects that match */
if (!ctx || ctx->type != AUDIT_IPC)
break;
- if (security_audit_rule_match(ctx->ipc.osid,
+ lsmblob_init(&blob, ctx->ipc.osid);
+ if (security_audit_rule_match(&blob,
f->type, f->op,
f->lsm_rule))
++result;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..5a337239d9e4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -307,8 +307,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
return -EINVAL;
}
-static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
- void *lsmrule)
+static inline int security_filter_rule_match(struct lsmblob *blob, u32 field,
+ u32 op, void *lsmrule)
{
return -EINVAL;
}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e0cc323f948f..e7b8ce942950 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -327,6 +327,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
for (i = 0; i < MAX_LSM_RULES; i++) {
int rc = 0;
u32 osid;
+ struct lsmblob blob;
int retried = 0;
if (!rule->lsm[i].rule)
@@ -337,7 +338,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
case LSM_OBJ_ROLE:
case LSM_OBJ_TYPE:
security_inode_getsecid(inode, &osid);
- rc = security_filter_rule_match(osid,
+ lsmblob_init(&blob, osid);
+ rc = security_filter_rule_match(&blob,
rule->lsm[i].type,
Audit_equal,
rule->lsm[i].rule);
@@ -345,7 +347,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
case LSM_SUBJ_USER:
case LSM_SUBJ_ROLE:
case LSM_SUBJ_TYPE:
- rc = security_filter_rule_match(secid,
+ lsmblob_init(&blob, secid);
+ rc = security_filter_rule_match(&blob,
rule->lsm[i].type,
Audit_equal,
rule->lsm[i].rule);
diff --git a/security/security.c b/security/security.c
index 1cae4faaa279..8fd5c8cd4f50 100644
--- a/security/security.c
+++ b/security/security.c
@@ -416,7 +416,7 @@ static int lsm_append(const char *new, char **result)
/*
* Current index to use while initializing the lsmblob secid list.
*/
-static int lsm_slot __initdata;
+static int lsm_slot __lsm_ro_after_init;
/**
* security_add_hooks - Add a modules hooks to the hook lists.
@@ -2363,9 +2363,21 @@ void security_audit_rule_free(void *lsmrule)
call_void_hook(audit_rule_free, lsmrule);
}
-int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule)
+int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
+ void *lsmrule)
{
- return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule);
+ struct security_hook_list *hp;
+ int rc;
+
+ hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) {
+ if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot))
+ continue;
+ rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot],
+ field, op, lsmrule);
+ if (rc != 0)
+ return rc;
+ }
+ return 0;
}
#endif /* CONFIG_AUDIT */
--
2.20.1
^ permalink raw reply related
* [PATCH v10 01/25] LSM: Infrastructure management of the sock security
From: Casey Schaufler @ 2019-11-12 23:59 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey@schaufler-ca.com>
Move management of the sock->sk_security blob out
of the individual security modules and into the security
infrastructure. Instead of allocating the blobs from within
the modules the modules tell the infrastructure how much
space is required, and the space is allocated there.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
include/linux/lsm_hooks.h | 1 +
security/apparmor/include/net.h | 6 ++-
security/apparmor/lsm.c | 38 ++++-----------
security/security.c | 36 +++++++++++++-
security/selinux/hooks.c | 78 +++++++++++++++----------------
security/selinux/include/objsec.h | 5 ++
security/selinux/netlabel.c | 23 ++++-----
security/smack/smack.h | 5 ++
security/smack/smack_lsm.c | 64 ++++++++++++-------------
security/smack/smack_netfilter.c | 8 ++--
10 files changed, 144 insertions(+), 120 deletions(-)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a240a3fc5fc4..9a80c7e94785 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2047,6 +2047,7 @@ struct lsm_blob_sizes {
int lbs_cred;
int lbs_file;
int lbs_inode;
+ int lbs_sock;
int lbs_ipc;
int lbs_msg_msg;
int lbs_task;
diff --git a/security/apparmor/include/net.h b/security/apparmor/include/net.h
index 7334ac966d01..adac04e3b3cc 100644
--- a/security/apparmor/include/net.h
+++ b/security/apparmor/include/net.h
@@ -55,7 +55,11 @@ struct aa_sk_ctx {
struct aa_label *peer;
};
-#define SK_CTX(X) ((X)->sk_security)
+static inline struct aa_sk_ctx *aa_sock(const struct sock *sk)
+{
+ return sk->sk_security + apparmor_blob_sizes.lbs_sock;
+}
+
#define SOCK_ctx(X) SOCK_INODE(X)->i_security
#define DEFINE_AUDIT_NET(NAME, OP, SK, F, T, P) \
struct lsm_network_audit NAME ## _net = { .sk = (SK), \
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 49d664ddff44..2716e7731279 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -757,33 +757,15 @@ static int apparmor_task_kill(struct task_struct *target, struct kernel_siginfo
return error;
}
-/**
- * apparmor_sk_alloc_security - allocate and attach the sk_security field
- */
-static int apparmor_sk_alloc_security(struct sock *sk, int family, gfp_t flags)
-{
- struct aa_sk_ctx *ctx;
-
- ctx = kzalloc(sizeof(*ctx), flags);
- if (!ctx)
- return -ENOMEM;
-
- SK_CTX(sk) = ctx;
-
- return 0;
-}
-
/**
* apparmor_sk_free_security - free the sk_security field
*/
static void apparmor_sk_free_security(struct sock *sk)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
- SK_CTX(sk) = NULL;
aa_put_label(ctx->label);
aa_put_label(ctx->peer);
- kfree(ctx);
}
/**
@@ -792,8 +774,8 @@ static void apparmor_sk_free_security(struct sock *sk)
static void apparmor_sk_clone_security(const struct sock *sk,
struct sock *newsk)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
- struct aa_sk_ctx *new = SK_CTX(newsk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
+ struct aa_sk_ctx *new = aa_sock(newsk);
new->label = aa_get_label(ctx->label);
new->peer = aa_get_label(ctx->peer);
@@ -844,7 +826,7 @@ static int apparmor_socket_post_create(struct socket *sock, int family,
label = aa_get_current_label();
if (sock->sk) {
- struct aa_sk_ctx *ctx = SK_CTX(sock->sk);
+ struct aa_sk_ctx *ctx = aa_sock(sock->sk);
aa_put_label(ctx->label);
ctx->label = aa_get_label(label);
@@ -1029,7 +1011,7 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
*/
static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
if (!skb->secmark)
return 0;
@@ -1042,7 +1024,7 @@ static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
static struct aa_label *sk_peer_label(struct sock *sk)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
if (ctx->peer)
return ctx->peer;
@@ -1126,7 +1108,7 @@ static int apparmor_socket_getpeersec_dgram(struct socket *sock,
*/
static void apparmor_sock_graft(struct sock *sk, struct socket *parent)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
if (!ctx->label)
ctx->label = aa_get_current_label();
@@ -1136,7 +1118,7 @@ static void apparmor_sock_graft(struct sock *sk, struct socket *parent)
static int apparmor_inet_conn_request(struct sock *sk, struct sk_buff *skb,
struct request_sock *req)
{
- struct aa_sk_ctx *ctx = SK_CTX(sk);
+ struct aa_sk_ctx *ctx = aa_sock(sk);
if (!skb->secmark)
return 0;
@@ -1153,6 +1135,7 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
.lbs_cred = sizeof(struct aa_task_ctx *),
.lbs_file = sizeof(struct aa_file_ctx),
.lbs_task = sizeof(struct aa_task_ctx),
+ .lbs_sock = sizeof(struct aa_sk_ctx),
};
static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
@@ -1189,7 +1172,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(getprocattr, apparmor_getprocattr),
LSM_HOOK_INIT(setprocattr, apparmor_setprocattr),
- LSM_HOOK_INIT(sk_alloc_security, apparmor_sk_alloc_security),
LSM_HOOK_INIT(sk_free_security, apparmor_sk_free_security),
LSM_HOOK_INIT(sk_clone_security, apparmor_sk_clone_security),
@@ -1581,7 +1563,7 @@ static unsigned int apparmor_ip_postroute(void *priv,
if (sk == NULL)
return NF_ACCEPT;
- ctx = SK_CTX(sk);
+ ctx = aa_sock(sk);
if (!apparmor_secmark_check(ctx->label, OP_SENDMSG, AA_MAY_SEND,
skb->secmark, sk))
return NF_ACCEPT;
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..757a8ee4da65 100644
--- a/security/security.c
+++ b/security/security.c
@@ -32,6 +32,7 @@
#include <linux/string.h>
#include <linux/msg.h>
#include <net/flow.h>
+#include <net/sock.h>
#define MAX_LSM_EVM_XATTR 2
@@ -172,6 +173,7 @@ static void __init lsm_set_blob_sizes(struct lsm_blob_sizes *needed)
lsm_set_blob_size(&needed->lbs_inode, &blob_sizes.lbs_inode);
lsm_set_blob_size(&needed->lbs_ipc, &blob_sizes.lbs_ipc);
lsm_set_blob_size(&needed->lbs_msg_msg, &blob_sizes.lbs_msg_msg);
+ lsm_set_blob_size(&needed->lbs_sock, &blob_sizes.lbs_sock);
lsm_set_blob_size(&needed->lbs_task, &blob_sizes.lbs_task);
}
@@ -305,6 +307,7 @@ static void __init ordered_lsm_init(void)
init_debug("inode blob size = %d\n", blob_sizes.lbs_inode);
init_debug("ipc blob size = %d\n", blob_sizes.lbs_ipc);
init_debug("msg_msg blob size = %d\n", blob_sizes.lbs_msg_msg);
+ init_debug("sock blob size = %d\n", blob_sizes.lbs_sock);
init_debug("task blob size = %d\n", blob_sizes.lbs_task);
/*
@@ -589,6 +592,28 @@ static int lsm_msg_msg_alloc(struct msg_msg *mp)
return 0;
}
+/**
+ * lsm_sock_alloc - allocate a composite sock blob
+ * @sock: the sock that needs a blob
+ * @priority: allocation mode
+ *
+ * Allocate the sock blob for all the modules
+ *
+ * Returns 0, or -ENOMEM if memory can't be allocated.
+ */
+static int lsm_sock_alloc(struct sock *sock, gfp_t priority)
+{
+ if (blob_sizes.lbs_sock == 0) {
+ sock->sk_security = NULL;
+ return 0;
+ }
+
+ sock->sk_security = kzalloc(blob_sizes.lbs_sock, priority);
+ if (sock->sk_security == NULL)
+ return -ENOMEM;
+ return 0;
+}
+
/**
* lsm_early_task - during initialization allocate a composite task blob
* @task: the task that needs a blob
@@ -2016,12 +2041,21 @@ EXPORT_SYMBOL(security_socket_getpeersec_dgram);
int security_sk_alloc(struct sock *sk, int family, gfp_t priority)
{
- return call_int_hook(sk_alloc_security, 0, sk, family, priority);
+ int rc = lsm_sock_alloc(sk, priority);
+
+ if (unlikely(rc))
+ return rc;
+ rc = call_int_hook(sk_alloc_security, 0, sk, family, priority);
+ if (unlikely(rc))
+ security_sk_free(sk);
+ return rc;
}
void security_sk_free(struct sock *sk)
{
call_void_hook(sk_free_security, sk);
+ kfree(sk->sk_security);
+ sk->sk_security = NULL;
}
void security_sk_clone(const struct sock *sk, struct sock *newsk)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1d0b37af2444..d4a1304f1e99 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4333,7 +4333,7 @@ static int socket_sockcreate_sid(const struct task_security_struct *tsec,
static int sock_has_perm(struct sock *sk, u32 perms)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
@@ -4390,7 +4390,7 @@ static int selinux_socket_post_create(struct socket *sock, int family,
isec->initialized = LABEL_INITIALIZED;
if (sock->sk) {
- sksec = sock->sk->sk_security;
+ sksec = selinux_sock(sock->sk);
sksec->sclass = sclass;
sksec->sid = sid;
/* Allows detection of the first association on this socket */
@@ -4406,8 +4406,8 @@ static int selinux_socket_post_create(struct socket *sock, int family,
static int selinux_socket_socketpair(struct socket *socka,
struct socket *sockb)
{
- struct sk_security_struct *sksec_a = socka->sk->sk_security;
- struct sk_security_struct *sksec_b = sockb->sk->sk_security;
+ struct sk_security_struct *sksec_a = selinux_sock(socka->sk);
+ struct sk_security_struct *sksec_b = selinux_sock(sockb->sk);
sksec_a->peer_sid = sksec_b->sid;
sksec_b->peer_sid = sksec_a->sid;
@@ -4422,7 +4422,7 @@ static int selinux_socket_socketpair(struct socket *socka,
static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, int addrlen)
{
struct sock *sk = sock->sk;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
u16 family;
int err;
@@ -4554,7 +4554,7 @@ static int selinux_socket_connect_helper(struct socket *sock,
struct sockaddr *address, int addrlen)
{
struct sock *sk = sock->sk;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
int err;
err = sock_has_perm(sk, SOCKET__CONNECT);
@@ -4725,9 +4725,9 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
struct sock *other,
struct sock *newsk)
{
- struct sk_security_struct *sksec_sock = sock->sk_security;
- struct sk_security_struct *sksec_other = other->sk_security;
- struct sk_security_struct *sksec_new = newsk->sk_security;
+ struct sk_security_struct *sksec_sock = selinux_sock(sock);
+ struct sk_security_struct *sksec_other = selinux_sock(other);
+ struct sk_security_struct *sksec_new = selinux_sock(newsk);
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
int err;
@@ -4759,8 +4759,8 @@ static int selinux_socket_unix_stream_connect(struct sock *sock,
static int selinux_socket_unix_may_send(struct socket *sock,
struct socket *other)
{
- struct sk_security_struct *ssec = sock->sk->sk_security;
- struct sk_security_struct *osec = other->sk->sk_security;
+ struct sk_security_struct *ssec = selinux_sock(sock->sk);
+ struct sk_security_struct *osec = selinux_sock(other->sk);
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
@@ -4802,7 +4802,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
u16 family)
{
int err = 0;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
u32 sk_sid = sksec->sid;
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
@@ -4835,7 +4835,7 @@ static int selinux_sock_rcv_skb_compat(struct sock *sk, struct sk_buff *skb,
static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
int err;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
u16 family = sk->sk_family;
u32 sk_sid = sksec->sid;
struct common_audit_data ad;
@@ -4903,13 +4903,15 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
return err;
}
-static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *optval,
- int __user *optlen, unsigned len)
+static int selinux_socket_getpeersec_stream(struct socket *sock,
+ char __user *optval,
+ int __user *optlen,
+ unsigned int len)
{
int err = 0;
char *scontext;
u32 scontext_len;
- struct sk_security_struct *sksec = sock->sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sock->sk);
u32 peer_sid = SECSID_NULL;
if (sksec->sclass == SECCLASS_UNIX_STREAM_SOCKET ||
@@ -4969,34 +4971,27 @@ static int selinux_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *
static int selinux_sk_alloc_security(struct sock *sk, int family, gfp_t priority)
{
- struct sk_security_struct *sksec;
-
- sksec = kzalloc(sizeof(*sksec), priority);
- if (!sksec)
- return -ENOMEM;
+ struct sk_security_struct *sksec = selinux_sock(sk);
sksec->peer_sid = SECINITSID_UNLABELED;
sksec->sid = SECINITSID_UNLABELED;
sksec->sclass = SECCLASS_SOCKET;
selinux_netlbl_sk_security_reset(sksec);
- sk->sk_security = sksec;
return 0;
}
static void selinux_sk_free_security(struct sock *sk)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
- sk->sk_security = NULL;
selinux_netlbl_sk_security_free(sksec);
- kfree(sksec);
}
static void selinux_sk_clone_security(const struct sock *sk, struct sock *newsk)
{
- struct sk_security_struct *sksec = sk->sk_security;
- struct sk_security_struct *newsksec = newsk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
+ struct sk_security_struct *newsksec = selinux_sock(newsk);
newsksec->sid = sksec->sid;
newsksec->peer_sid = sksec->peer_sid;
@@ -5010,7 +5005,7 @@ static void selinux_sk_getsecid(struct sock *sk, u32 *secid)
if (!sk)
*secid = SECINITSID_ANY_SOCKET;
else {
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
*secid = sksec->sid;
}
@@ -5020,7 +5015,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
{
struct inode_security_struct *isec =
inode_security_novalidate(SOCK_INODE(parent));
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
if (sk->sk_family == PF_INET || sk->sk_family == PF_INET6 ||
sk->sk_family == PF_UNIX)
@@ -5035,7 +5030,7 @@ static void selinux_sock_graft(struct sock *sk, struct socket *parent)
static int selinux_sctp_assoc_request(struct sctp_endpoint *ep,
struct sk_buff *skb)
{
- struct sk_security_struct *sksec = ep->base.sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(ep->base.sk);
struct common_audit_data ad;
struct lsm_network_audit net = {0,};
u8 peerlbl_active;
@@ -5186,8 +5181,8 @@ static int selinux_sctp_bind_connect(struct sock *sk, int optname,
static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
struct sock *newsk)
{
- struct sk_security_struct *sksec = sk->sk_security;
- struct sk_security_struct *newsksec = newsk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
+ struct sk_security_struct *newsksec = selinux_sock(newsk);
/* If policy does not support SECCLASS_SCTP_SOCKET then call
* the non-sctp clone version.
@@ -5204,7 +5199,7 @@ static void selinux_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
struct request_sock *req)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
int err;
u16 family = req->rsk_ops->family;
u32 connsid;
@@ -5225,7 +5220,7 @@ static int selinux_inet_conn_request(struct sock *sk, struct sk_buff *skb,
static void selinux_inet_csk_clone(struct sock *newsk,
const struct request_sock *req)
{
- struct sk_security_struct *newsksec = newsk->sk_security;
+ struct sk_security_struct *newsksec = selinux_sock(newsk);
newsksec->sid = req->secid;
newsksec->peer_sid = req->peer_secid;
@@ -5242,7 +5237,7 @@ static void selinux_inet_csk_clone(struct sock *newsk,
static void selinux_inet_conn_established(struct sock *sk, struct sk_buff *skb)
{
u16 family = sk->sk_family;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
/* handle mapped IPv4 packets arriving via IPv6 sockets */
if (family == PF_INET6 && skb->protocol == htons(ETH_P_IP))
@@ -5326,7 +5321,7 @@ static int selinux_tun_dev_attach_queue(void *security)
static int selinux_tun_dev_attach(struct sock *sk, void *security)
{
struct tun_security_struct *tunsec = security;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
/* we don't currently perform any NetLabel based labeling here and it
* isn't clear that we would want to do so anyway; while we could apply
@@ -5367,7 +5362,7 @@ static int selinux_nlmsg_perm(struct sock *sk, struct sk_buff *skb)
int err = 0;
u32 perm;
struct nlmsghdr *nlh;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
if (skb->len < NLMSG_HDRLEN) {
err = -EINVAL;
@@ -5508,7 +5503,7 @@ static unsigned int selinux_ip_output(struct sk_buff *skb,
return NF_ACCEPT;
/* standard practice, label using the parent socket */
- sksec = sk->sk_security;
+ sksec = selinux_sock(sk);
sid = sksec->sid;
} else
sid = SECINITSID_KERNEL;
@@ -5547,7 +5542,7 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
if (sk == NULL)
return NF_ACCEPT;
- sksec = sk->sk_security;
+ sksec = selinux_sock(sk);
ad.type = LSM_AUDIT_DATA_NET;
ad.u.net = &net;
@@ -5639,7 +5634,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
u32 skb_sid;
struct sk_security_struct *sksec;
- sksec = sk->sk_security;
+ sksec = selinux_sock(sk);
if (selinux_skb_peerlbl_sid(skb, family, &skb_sid))
return NF_DROP;
/* At this point, if the returned skb peerlbl is SECSID_NULL
@@ -5668,7 +5663,7 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb,
} else {
/* Locally generated packet, fetch the security label from the
* associated socket. */
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
peer_sid = sksec->sid;
secmark_perm = PACKET__SEND;
}
@@ -6647,6 +6642,7 @@ struct lsm_blob_sizes selinux_blob_sizes __lsm_ro_after_init = {
.lbs_inode = sizeof(struct inode_security_struct),
.lbs_ipc = sizeof(struct ipc_security_struct),
.lbs_msg_msg = sizeof(struct msg_security_struct),
+ .lbs_sock = sizeof(struct sk_security_struct),
};
static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 231262d8eac9..137494cb2e2c 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -188,4 +188,9 @@ static inline struct ipc_security_struct *selinux_ipc(
return ipc->security + selinux_blob_sizes.lbs_ipc;
}
+static inline struct sk_security_struct *selinux_sock(const struct sock *sock)
+{
+ return sock->sk_security + selinux_blob_sizes.lbs_sock;
+}
+
#endif /* _SELINUX_OBJSEC_H_ */
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 186e727b737b..c40914a157b7 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -31,6 +31,7 @@
#include <linux/gfp.h>
#include <linux/ip.h>
#include <linux/ipv6.h>
+#include <linux/lsm_hooks.h>
#include <net/sock.h>
#include <net/netlabel.h>
#include <net/ip.h>
@@ -81,7 +82,7 @@ static int selinux_netlbl_sidlookup_cached(struct sk_buff *skb,
static struct netlbl_lsm_secattr *selinux_netlbl_sock_genattr(struct sock *sk)
{
int rc;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct netlbl_lsm_secattr *secattr;
if (sksec->nlbl_secattr != NULL)
@@ -114,7 +115,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
const struct sock *sk,
u32 sid)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct netlbl_lsm_secattr *secattr = sksec->nlbl_secattr;
if (secattr == NULL)
@@ -249,7 +250,7 @@ int selinux_netlbl_skbuff_setsid(struct sk_buff *skb,
* being labeled by it's parent socket, if it is just exit */
sk = skb_to_full_sk(skb);
if (sk != NULL) {
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
if (sksec->nlbl_state != NLBL_REQSKB)
return 0;
@@ -287,7 +288,7 @@ int selinux_netlbl_sctp_assoc_request(struct sctp_endpoint *ep,
{
int rc;
struct netlbl_lsm_secattr secattr;
- struct sk_security_struct *sksec = ep->base.sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(ep->base.sk);
struct sockaddr *addr;
struct sockaddr_in addr4;
#if IS_ENABLED(CONFIG_IPV6)
@@ -370,7 +371,7 @@ int selinux_netlbl_inet_conn_request(struct request_sock *req, u16 family)
*/
void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
if (family == PF_INET)
sksec->nlbl_state = NLBL_LABELED;
@@ -388,8 +389,8 @@ void selinux_netlbl_inet_csk_clone(struct sock *sk, u16 family)
*/
void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
{
- struct sk_security_struct *sksec = sk->sk_security;
- struct sk_security_struct *newsksec = newsk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
+ struct sk_security_struct *newsksec = selinux_sock(newsk);
newsksec->nlbl_state = sksec->nlbl_state;
}
@@ -407,7 +408,7 @@ void selinux_netlbl_sctp_sk_clone(struct sock *sk, struct sock *newsk)
int selinux_netlbl_socket_post_create(struct sock *sk, u16 family)
{
int rc;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct netlbl_lsm_secattr *secattr;
if (family != PF_INET && family != PF_INET6)
@@ -522,7 +523,7 @@ int selinux_netlbl_socket_setsockopt(struct socket *sock,
{
int rc = 0;
struct sock *sk = sock->sk;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct netlbl_lsm_secattr secattr;
if (selinux_netlbl_option(level, optname) &&
@@ -560,7 +561,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
struct sockaddr *addr)
{
int rc;
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
struct netlbl_lsm_secattr *secattr;
/* connected sockets are allowed to disconnect when the address family
@@ -599,7 +600,7 @@ static int selinux_netlbl_socket_connect_helper(struct sock *sk,
int selinux_netlbl_socket_connect_locked(struct sock *sk,
struct sockaddr *addr)
{
- struct sk_security_struct *sksec = sk->sk_security;
+ struct sk_security_struct *sksec = selinux_sock(sk);
if (sksec->nlbl_state != NLBL_REQSKB &&
sksec->nlbl_state != NLBL_CONNLABELED)
diff --git a/security/smack/smack.h b/security/smack/smack.h
index cf52af77d15e..0a5a2a296c1a 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -375,6 +375,11 @@ static inline struct smack_known **smack_ipc(const struct kern_ipc_perm *ipc)
return ipc->security + smack_blob_sizes.lbs_ipc;
}
+static inline struct socket_smack *smack_sock(const struct sock *sock)
+{
+ return sock->sk_security + smack_blob_sizes.lbs_sock;
+}
+
/*
* Is the directory transmuting?
*/
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 5c1613519d5a..e64eb558334a 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1456,7 +1456,7 @@ static int smack_inode_getsecurity(struct inode *inode,
if (sock == NULL || sock->sk == NULL)
return -EOPNOTSUPP;
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
if (strcmp(name, XATTR_SMACK_IPIN) == 0)
isp = ssp->smk_in;
@@ -1838,7 +1838,7 @@ static int smack_file_receive(struct file *file)
if (inode->i_sb->s_magic == SOCKFS_MAGIC) {
sock = SOCKET_I(inode);
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
tsp = smack_cred(current_cred());
/*
* If the receiving process can't write to the
@@ -2248,11 +2248,7 @@ static void smack_task_to_inode(struct task_struct *p, struct inode *inode)
static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
{
struct smack_known *skp = smk_of_current();
- struct socket_smack *ssp;
-
- ssp = kzalloc(sizeof(struct socket_smack), gfp_flags);
- if (ssp == NULL)
- return -ENOMEM;
+ struct socket_smack *ssp = smack_sock(sk);
/*
* Sockets created by kernel threads receive web label.
@@ -2266,11 +2262,10 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
}
ssp->smk_packet = NULL;
- sk->sk_security = ssp;
-
return 0;
}
+#ifdef SMACK_IPV6_PORT_LABELING
/**
* smack_sk_free_security - Free a socket blob
* @sk: the socket
@@ -2279,7 +2274,6 @@ static int smack_sk_alloc_security(struct sock *sk, int family, gfp_t gfp_flags)
*/
static void smack_sk_free_security(struct sock *sk)
{
-#ifdef SMACK_IPV6_PORT_LABELING
struct smk_port_label *spp;
if (sk->sk_family == PF_INET6) {
@@ -2292,9 +2286,8 @@ static void smack_sk_free_security(struct sock *sk)
}
rcu_read_unlock();
}
-#endif
- kfree(sk->sk_security);
}
+#endif
/**
* smack_ipv4host_label - check host based restrictions
@@ -2412,7 +2405,7 @@ static struct smack_known *smack_ipv6host_label(struct sockaddr_in6 *sip)
static int smack_netlabel(struct sock *sk, int labeled)
{
struct smack_known *skp;
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
int rc = 0;
/*
@@ -2457,7 +2450,7 @@ static int smack_netlabel_send(struct sock *sk, struct sockaddr_in *sap)
int rc;
int sk_lbl;
struct smack_known *hkp;
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
struct smk_audit_info ad;
rcu_read_lock();
@@ -2533,7 +2526,7 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
{
struct sock *sk = sock->sk;
struct sockaddr_in6 *addr6;
- struct socket_smack *ssp = sock->sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sock->sk);
struct smk_port_label *spp;
unsigned short port = 0;
@@ -2620,7 +2613,7 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
int act)
{
struct smk_port_label *spp;
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
struct smack_known *skp = NULL;
unsigned short port;
struct smack_known *object;
@@ -2714,7 +2707,7 @@ static int smack_inode_setsecurity(struct inode *inode, const char *name,
if (sock == NULL || sock->sk == NULL)
return -EOPNOTSUPP;
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
if (strcmp(name, XATTR_SMACK_IPIN) == 0)
ssp->smk_in = skp;
@@ -2762,7 +2755,7 @@ static int smack_socket_post_create(struct socket *sock, int family,
* Sockets created by kernel threads receive web label.
*/
if (unlikely(current->flags & PF_KTHREAD)) {
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
ssp->smk_in = &smack_known_web;
ssp->smk_out = &smack_known_web;
}
@@ -2787,8 +2780,8 @@ static int smack_socket_post_create(struct socket *sock, int family,
static int smack_socket_socketpair(struct socket *socka,
struct socket *sockb)
{
- struct socket_smack *asp = socka->sk->sk_security;
- struct socket_smack *bsp = sockb->sk->sk_security;
+ struct socket_smack *asp = smack_sock(socka->sk);
+ struct socket_smack *bsp = smack_sock(sockb->sk);
asp->smk_packet = bsp->smk_out;
bsp->smk_packet = asp->smk_out;
@@ -2842,7 +2835,7 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
return 0;
#ifdef SMACK_IPV6_SECMARK_LABELING
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
#endif
switch (sock->sk->sk_family) {
@@ -3583,9 +3576,9 @@ static int smack_unix_stream_connect(struct sock *sock,
{
struct smack_known *skp;
struct smack_known *okp;
- struct socket_smack *ssp = sock->sk_security;
- struct socket_smack *osp = other->sk_security;
- struct socket_smack *nsp = newsk->sk_security;
+ struct socket_smack *ssp = smack_sock(sock);
+ struct socket_smack *osp = smack_sock(other);
+ struct socket_smack *nsp = smack_sock(newsk);
struct smk_audit_info ad;
int rc = 0;
#ifdef CONFIG_AUDIT
@@ -3631,8 +3624,8 @@ static int smack_unix_stream_connect(struct sock *sock,
*/
static int smack_unix_may_send(struct socket *sock, struct socket *other)
{
- struct socket_smack *ssp = sock->sk->sk_security;
- struct socket_smack *osp = other->sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sock->sk);
+ struct socket_smack *osp = smack_sock(other->sk);
struct smk_audit_info ad;
int rc;
@@ -3669,7 +3662,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;
#endif
#ifdef SMACK_IPV6_SECMARK_LABELING
- struct socket_smack *ssp = sock->sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sock->sk);
struct smack_known *rsp;
#endif
int rc = 0;
@@ -3834,7 +3827,7 @@ static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
{
struct netlbl_lsm_secattr secattr;
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
struct smack_known *skp = NULL;
int rc = 0;
struct smk_audit_info ad;
@@ -3951,7 +3944,7 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
int slen = 1;
int rc = 0;
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
if (ssp->smk_packet != NULL) {
rcp = ssp->smk_packet->smk_known;
slen = strlen(rcp) + 1;
@@ -4001,7 +3994,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
switch (family) {
case PF_UNIX:
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
s = ssp->smk_out->smk_secid;
break;
case PF_INET:
@@ -4014,7 +4007,7 @@ static int smack_socket_getpeersec_dgram(struct socket *sock,
* Translate what netlabel gave us.
*/
if (sock != NULL && sock->sk != NULL)
- ssp = sock->sk->sk_security;
+ ssp = smack_sock(sock->sk);
netlbl_secattr_init(&secattr);
rc = netlbl_skbuff_getattr(skb, family, &secattr);
if (rc == 0) {
@@ -4052,7 +4045,7 @@ static void smack_sock_graft(struct sock *sk, struct socket *parent)
(sk->sk_family != PF_INET && sk->sk_family != PF_INET6))
return;
- ssp = sk->sk_security;
+ ssp = smack_sock(sk);
ssp->smk_in = skp;
ssp->smk_out = skp;
/* cssp->smk_packet is already set in smack_inet_csk_clone() */
@@ -4072,7 +4065,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
{
u16 family = sk->sk_family;
struct smack_known *skp;
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
struct netlbl_lsm_secattr secattr;
struct sockaddr_in addr;
struct iphdr *hdr;
@@ -4171,7 +4164,7 @@ static int smack_inet_conn_request(struct sock *sk, struct sk_buff *skb,
static void smack_inet_csk_clone(struct sock *sk,
const struct request_sock *req)
{
- struct socket_smack *ssp = sk->sk_security;
+ struct socket_smack *ssp = smack_sock(sk);
struct smack_known *skp;
if (req->peer_secid != 0) {
@@ -4575,6 +4568,7 @@ struct lsm_blob_sizes smack_blob_sizes __lsm_ro_after_init = {
.lbs_inode = sizeof(struct inode_smack),
.lbs_ipc = sizeof(struct smack_known *),
.lbs_msg_msg = sizeof(struct smack_known *),
+ .lbs_sock = sizeof(struct socket_smack),
};
static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
@@ -4684,7 +4678,9 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(socket_getpeersec_stream, smack_socket_getpeersec_stream),
LSM_HOOK_INIT(socket_getpeersec_dgram, smack_socket_getpeersec_dgram),
LSM_HOOK_INIT(sk_alloc_security, smack_sk_alloc_security),
+#ifdef SMACK_IPV6_PORT_LABELING
LSM_HOOK_INIT(sk_free_security, smack_sk_free_security),
+#endif
LSM_HOOK_INIT(sock_graft, smack_sock_graft),
LSM_HOOK_INIT(inet_conn_request, smack_inet_conn_request),
LSM_HOOK_INIT(inet_csk_clone, smack_inet_csk_clone),
diff --git a/security/smack/smack_netfilter.c b/security/smack/smack_netfilter.c
index e36d17835d4f..701a1cc1bdcc 100644
--- a/security/smack/smack_netfilter.c
+++ b/security/smack/smack_netfilter.c
@@ -31,8 +31,8 @@ static unsigned int smack_ipv6_output(void *priv,
struct socket_smack *ssp;
struct smack_known *skp;
- if (sk && sk->sk_security) {
- ssp = sk->sk_security;
+ if (sk && smack_sock(sk)) {
+ ssp = smack_sock(sk);
skp = ssp->smk_out;
skb->secmark = skp->smk_secid;
}
@@ -49,8 +49,8 @@ static unsigned int smack_ipv4_output(void *priv,
struct socket_smack *ssp;
struct smack_known *skp;
- if (sk && sk->sk_security) {
- ssp = sk->sk_security;
+ if (sk && smack_sock(sk)) {
+ ssp = smack_sock(sk);
skp = ssp->smk_out;
skb->secmark = skp->smk_secid;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v10 00/25] LSM: Module stacking for AppArmor
From: Casey Schaufler @ 2019-11-12 23:59 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: casey, keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113000022.5300-1-casey.ref@schaufler-ca.com>
This patchset provides the changes required for
the AppArmor security module to stack safely with any other.
v10: Ask the security modules if the display can be changed.
v9: There is no version 9
v8: Incorporate feedback from v7
- Minor clean-up in display value management
- refactor "compound" context creation to use a common
append_ctx() function.
v7: Incorporate feedback from v6
- Make setting the display a privileged operation. The
availability of compound contexts reduces the need for
setting the display.
v6: Incorporate feedback from v5
- Add subj_<lsm>= and obj_<lsm>= fields to audit records
- Add /proc/.../attr/context to get the full context in
lsmname\0value\0... format as suggested by Simon McVittie
- Add SO_PEERCONTEXT for getsockopt() to get the full context
in the same format, also suggested by Simon McVittie.
- Add /sys/kernel/security/lsm_display_default to provide
the display default value.
v5: Incorporate feedback from v4
- Initialize the lsmcontext in security_secid_to_secctx()
- Clear the lsmcontext in all security_release_secctx() cases
- Don't use the "display" on strictly internal context
interfaces.
- The SELinux binder hooks check for cases where the context
"display" isn't compatible with SELinux.
v4: Incorporate feedback from v3
- Mark new lsm_<blob>_alloc functions static
- Replace the lsm and slot fields of the security_hook_list
with a pointer to a LSM allocated lsm_id structure. The
LSM identifies if it needs a slot explicitly. Use the
lsm_id rather than make security_add_hooks return the
slot value.
- Validate slot values used in security.c
- Reworked the "display" process attribute handling so that
it works right and doesn't use goofy list processing.
- fix display value check in dentry_init_security
- Replace audit_log of secids with '?' instead of deleting
the audit log
v3: Incorporate feedback from v2
- Make lsmblob parameter and variable names more
meaningful, changing "le" and "l" to "blob".
- Improve consistency of constant naming.
- Do more sanity checking during LSM initialization.
- Be a bit clearer about what is temporary scaffolding.
- Rather than clutter security_getpeersec_dgram with
otherwise unnecessary checks remove the apparmor
stub, which does nothing useful.
Patche 0001 moves management of the sock security blob from the individual
modules to the infrastructure.
Patches 0002-0012 replace system use of a "secid" with
a structure "lsmblob" containing information from the
security modules to be held and reused later. At this
point lsmblob contains an array of u32 secids, one "slot"
for each of the security modules compiled into the
kernel that used secids. A "slot" is allocated when
a security module requests one.
The infrastructure is changed to use the slot number
to pass the correct secid to or from the security module
hooks.
It is important that the lsmblob be a fixed size entity
that does not have to be allocated. Several of the places
where it is used would have performance and/or locking
issues with dynamic allocation.
Patch 0013 provides a mechanism for a process to
identify which security module's hooks should be used
when displaying or converting a security context string.
A new interface /proc/.../attr/display contains the name
of the security module to show. Reading from this file
will present the name of the module, while writing to
it will set the value. Only names of active security
modules are accepted. Internally, the name is translated
to the appropriate "slot" number for the module which
is then stored in the task security blob. Setting the
display requires that all modules using the /proc interfaces
allow the transition.
Patch 0014 Starts the process of changing how a security
context is represented. Since it is possible for a
security context to have been generated by more than one
security module it is now necessary to note which module
created a security context so that the correct "release"
hook can be called. There are several places where the
module that created a security context cannot be inferred.
This is achieved by introducing a "lsmcontext" structure
which contains the context string, its length and the
"slot" number of the security module that created it.
The security_release_secctx() interface is changed,
replacing the (string,len) pointer pair with a lsmcontext
pointer.
Patches 0015-0017 convert the security interfaces from
(string,len) pointer pairs to a lsmcontext pointer.
The slot number identifying the creating module is
added by the infrastructure. Where the security context
is stored for extended periods the data type is changed.
The Netlabel code is converted to save lsmblob structures
instead of secids in Patches 0018-0019.
Patch 0020 adds checks to the binder hooks which verify
that if both ends of a transaction use the same "display".
Patches 0021-0022 add addition data to the audit records
to identify the LSM specific data for all active modules.
Patches 0023-0024 add new interfaces for getting the
compound security contexts.
Finally, with all interference on the AppArmor hooks
removed, Patch 0025 removes the exclusive bit from
AppArmor. An unnecessary stub hook was also removed.
The Ubuntu project is using an earlier version of
this patchset in their distribution to enable stacking
for containers.
Performance measurements to date have the change
within the "noise". The sockperf and dbench results
are on the order of 0.2% to 0.8% difference, with
better performance being as common as worse. The
benchmarks were run with AppArmor and Smack on Ubuntu.
https://github.com/cschaufler/lsm-stacking.git#stack-5.2-v10-apparmor
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
arch/alpha/include/uapi/asm/socket.h | 1 +
arch/mips/include/uapi/asm/socket.h | 1 +
arch/parisc/include/uapi/asm/socket.h | 1 +
arch/sparc/include/uapi/asm/socket.h | 1 +
drivers/android/binder.c | 24 +-
fs/kernfs/dir.c | 5 +-
fs/kernfs/inode.c | 35 +-
fs/kernfs/kernfs-internal.h | 3 +-
fs/nfs/nfs4proc.c | 22 +-
fs/nfsd/nfs4xdr.c | 20 +-
fs/proc/base.c | 2 +
include/linux/audit.h | 1 +
include/linux/cred.h | 3 +-
include/linux/lsm_hooks.h | 37 +-
include/linux/security.h | 175 ++++++++--
include/net/af_unix.h | 2 +-
include/net/netlabel.h | 8 +-
include/net/scm.h | 15 +-
include/uapi/asm-generic/socket.h | 1 +
kernel/audit.c | 70 +++-
kernel/audit.h | 9 +-
kernel/audit_fsnotify.c | 1 +
kernel/auditfilter.c | 10 +-
kernel/auditsc.c | 129 ++++---
kernel/cred.c | 12 +-
net/core/sock.c | 7 +-
net/ipv4/cipso_ipv4.c | 6 +-
net/ipv4/ip_sockglue.c | 12 +-
net/netfilter/nf_conntrack_netlink.c | 20 +-
net/netfilter/nf_conntrack_standalone.c | 11 +-
net/netfilter/nfnetlink_queue.c | 26 +-
net/netfilter/nft_meta.c | 13 +-
net/netfilter/xt_SECMARK.c | 5 +-
net/netlabel/netlabel_kapi.c | 6 +-
net/netlabel/netlabel_unlabeled.c | 97 +++---
net/netlabel/netlabel_unlabeled.h | 2 +-
net/netlabel/netlabel_user.c | 13 +-
net/netlabel/netlabel_user.h | 6 +-
net/unix/af_unix.c | 6 +-
net/xfrm/xfrm_policy.c | 2 +
net/xfrm/xfrm_state.c | 2 +
security/apparmor/include/apparmor.h | 3 +-
security/apparmor/include/net.h | 6 +-
security/apparmor/lsm.c | 121 ++++---
security/commoncap.c | 7 +-
security/integrity/ima/ima.h | 14 +-
security/integrity/ima/ima_api.c | 10 +-
security/integrity/ima/ima_appraise.c | 6 +-
security/integrity/ima/ima_main.c | 36 +-
security/integrity/ima/ima_policy.c | 19 +-
security/integrity/integrity_audit.c | 1 +
security/loadpin/loadpin.c | 8 +-
security/safesetid/lsm.c | 8 +-
security/security.c | 586 +++++++++++++++++++++++++++++---
security/selinux/hooks.c | 109 +++---
security/selinux/include/classmap.h | 2 +-
security/selinux/include/objsec.h | 5 +
security/selinux/include/security.h | 1 +
security/selinux/netlabel.c | 25 +-
security/selinux/ss/services.c | 4 +-
security/smack/smack.h | 6 +
security/smack/smack_lsm.c | 124 ++++---
security/smack/smack_netfilter.c | 8 +-
security/smack/smackfs.c | 10 +-
security/tomoyo/tomoyo.c | 8 +-
security/yama/yama_lsm.c | 7 +-
66 files changed, 1376 insertions(+), 580 deletions(-)
^ permalink raw reply
* Re: [PATCH] x86/mtrr: only administrator can read the configurations.
From: Kees Cook @ 2019-11-12 22:35 UTC (permalink / raw)
To: Borislav Petkov
Cc: Zhang Xiaoxu, mingo, hpa, x86, tyhicks, colin.king, tglx,
linux-kernel, linux-security-module, Matthew Garrett
In-Reply-To: <20191112174956.GB32336@zn.tnic>
On Tue, Nov 12, 2019 at 06:49:56PM +0100, Borislav Petkov wrote:
> On Mon, Nov 11, 2019 at 09:56:16AM -0800, Kees Cook wrote:
> > Some recap from being accidentally offlist:
> >
> > - this patch should check capabilities at open time (or retain the
> > checks on the opener's permissions for later checks).
> >
> > - changing the DAC permissions might break something that expects to
> > read mtrr when not uid 0.
> >
> > - if we leave the DAC permissions alone and just move the capable check
> > to the opener, we should get the intent of the original patch. (i.e.
> > check against CAP_SYS_ADMIN not just the wider uid 0.)
> >
> > - *this may still break things* if userspace expects to be able to
> > read other parts of the file as non-uid-0 and non-CAP_SYS_ADMIN.
> > If *that* is the case, then we need to censor the contents using
> > the opener's permissions (as done in other /proc cases).
> >
> > I think the most cautious way forward is something like
> > 51d7b120418e ("/proc/iomem: only expose physical resource addresses to
> > privileged users"). Untested (and should likely be expanded to know
> > about read vs write for lockdown interaction):
>
> I'm back'n'forth on this.
>
> So tglx and I agree that it doesn't make a whole lotta sense for
> non-privileged luserspace to be able to read /proc/mtrr because it is a
> small leak and normal users shouldn't care about the caching attributes
> of memory regions in the first place.
>
> So maybe we should do the second variant.
>
> But then we're not supposed to break luserspace.
>
> But then we can revert it if we do...
>
> Ugh.
Shall I send a patch for just moving the capable() checks into open()
and if someone yells we switch to the other option on the assumption
that then we'll have a real-world case we can test the other solution
against?
--
Kees Cook
^ permalink raw reply
* Re: WARNING in aa_sock_msg_perm (2)
From: Tetsuo Handa @ 2019-11-12 21:00 UTC (permalink / raw)
To: syzbot, syzkaller-bugs
Cc: jmorris, john.johansen, linux-kernel, linux-security-module,
serge
In-Reply-To: <00000000000004471505972a9432@google.com>
net/rxrpc/output.c:655 was previously reported as "#syz dup: KASAN: use-after-free Read in rxrpc_send_keepalive".
Now reported as different subject?
#syz dup: KMSAN: use-after-free in rxrpc_send_keepalive
^ permalink raw reply
* [PATCH] tpm_tis: Move setting of TPM_CHIP_FLAG_IRQ into tpm_tis_probe_irq_single
From: Stefan Berger @ 2019-11-12 20:27 UTC (permalink / raw)
To: linux-integrity, jsnitsel, jarkko.sakkinen
Cc: linux-kernel, linux-security-module, Stefan Berger
From: Stefan Berger <stefanb@linux.ibm.com>
Move the setting of the TPM_CHIP_FLAG_IRQ for irq probing into
tpm_tis_probe_irq_single before calling tpm_tis_gen_interrupt.
This move handles error conditions better that may arise if anything
before fails in tpm_tis_probe_irq_single.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Suggested-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
drivers/char/tpm/tpm_tis_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 8af2cee1a762..6b6605890c7d 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -790,6 +790,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
return rc;
priv->irq_tested = false;
+ chip->flags |= TPM_CHIP_FLAG_IRQ;
/* Generate an interrupt by having the core call through to
* tpm_tis_send
@@ -1060,7 +1061,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
}
tpm_chip_start(chip);
- chip->flags |= TPM_CHIP_FLAG_IRQ;
if (irq) {
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
irq);
--
2.14.5
^ permalink raw reply related
* WARNING in aa_sock_msg_perm (2)
From: syzbot @ 2019-11-12 18:40 UTC (permalink / raw)
To: jmorris, john.johansen, linux-kernel, linux-security-module,
serge, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 31f4f5b4 Linux 5.4-rc7
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12887074e00000
kernel config: https://syzkaller.appspot.com/x/.config?x=83de638b2a862933
dashboard link: https://syzkaller.appspot.com/bug?extid=e3b328542d4adc02a975
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e3b328542d4adc02a975@syzkaller.appspotmail.com
------------[ cut here ]------------
AppArmor WARN aa_sock_msg_perm: ((!sock)):
WARNING: CPU: 0 PID: 12781 at security/apparmor/lsm.c:920
aa_sock_msg_perm.isra.0+0xdd/0x170 security/apparmor/lsm.c:920
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 12781 Comm: kworker/0:1 Not tainted 5.4.0-rc7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: krxrpcd rxrpc_peer_keepalive_worker
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x197/0x210 lib/dump_stack.c:118
panic+0x2e3/0x75c kernel/panic.c:221
__warn.cold+0x2f/0x35 kernel/panic.c:582
report_bug+0x289/0x300 lib/bug.c:195
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:272
do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:291
invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1028
RIP: 0010:aa_sock_msg_perm.isra.0+0xdd/0x170 security/apparmor/lsm.c:920
Code: 89 ef e8 26 e4 02 00 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 16 3a 60 fe
48 c7 c6 e0 39 e1 87 48 c7 c7 e0 24 e1 87 e8 4b 6c 31 fe <0f> 0b e9 43 ff
ff ff e8 f7 39 60 fe 48 c7 c6 e0 39 e1 87 48 c7 c7
RSP: 0018:ffff88809c3d79b0 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815cc846 RDI: ffffed101387af28
RBP: ffff88809c3d79d8 R08: ffff88805b02c600 R09: fffffbfff14efd54
R10: fffffbfff14efd53 R11: ffffffff8a77ea9f R12: ffff88809c3d7b20
R13: ffffffff87e12920 R14: 0000000000000002 R15: 000000000000001d
apparmor_socket_sendmsg+0x2a/0x30 security/apparmor/lsm.c:936
security_socket_sendmsg+0x77/0xc0 security/security.c:2013
sock_sendmsg+0x45/0x130 net/socket.c:654
kernel_sendmsg+0x44/0x50 net/socket.c:677
rxrpc_send_keepalive+0x1ff/0x940 net/rxrpc/output.c:655
rxrpc_peer_keepalive_dispatch net/rxrpc/peer_event.c:376 [inline]
rxrpc_peer_keepalive_worker+0x7be/0xd02 net/rxrpc/peer_event.c:437
process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
worker_thread+0x98/0xe40 kernel/workqueue.c:2415
kthread+0x361/0x430 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [PATCH v5 04/10] IMA: Updated IMA policy functions to return keyrings option read from the policy
From: Mimi Zohar @ 2019-11-12 18:06 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <407b93e1-f474-7b01-816f-62b45690f417@linux.microsoft.com>
On Tue, 2019-11-12 at 09:47 -0800, Lakshmi Ramasubramanian wrote:
> On 11/12/2019 9:05 AM, Mimi Zohar wrote:
>
> >> int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> >> enum ima_hooks func, int mask, int flags, int *pcr,
> >> - struct ima_template_desc **template_desc)
> >> + struct ima_template_desc **template_desc,
> >> + char **keyrings)
> >> {
> >> struct ima_rule_entry *entry;
> >> int action = 0, actmask = flags | (flags << 1);
> >> @@ -527,6 +529,9 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> >> if ((pcr) && (entry->flags & IMA_PCR))
> >> *pcr = entry->pcr;
> >>
> >> + if ((keyrings) && (entry->flags & IMA_KEYRINGS))
> >> + *keyrings = entry->keyrings;
> >
> > ima_match_rules() determines whether the rule is in policy or not. It
> > returns true on rule match, false on failure. There's no need to
> > return the list of keyrings.
>
> But the above code change is in ima_match_policy() - not in
> ima_match_rules() function.
>
> ima_match_rules() function is updated in Patch #1 -
> [PATCH v5 01/10] IMA: Added KEYRING_CHECK func in IMA policy to measure keys
>
> I've updated that function to check if func is "KEYRING_CHECK" and
> return true\false as appropriate.
>
> Am I missing something?
The first patch adds basic support for the new "func". This allows
measuring all keys. ima_match_rules() then needs to be updated in the
patch that adds the "keyrings=" or "keyring=" support to limit it to a
specific keyring.
Mimi
^ permalink raw reply
* Re: [PATCH v5 02/10] IMA: Added keyrings= option in IMA policy to only measure keys added to the specified keyrings.
From: Mimi Zohar @ 2019-11-12 17:58 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <1f940f9d-fb1b-a0a4-b5f4-3f3532dbc041@linux.microsoft.com>
On Tue, 2019-11-12 at 09:43 -0800, Lakshmi Ramasubramanian wrote:
> On 11/12/2019 9:05 AM, Mimi Zohar wrote:
>
> > The C maximum line length is 80 characters. The subject line is even
> > less than that (~50). The Subject line here could be "ima: add
> > support to limit measuring keys".
> I'll update the subject line for the patches - limit to max 50 chars.
>
> >
> > On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
> >> IMA policy needs to support measuring only those keys linked to
> >> a specific set of keyrings.
> >
> > Patch descriptions should be written in the imperative. For example,
> > "Limit measuring keys to those keys being loaded onto a specific
> > keyring."
> Will update.
>
> >
> >>
> >> This patch defines a new IMA policy option namely "keyrings=" that
> >> can be used to specify a set of keyrings. If this option is specified
> >> in the policy for func=KEYRING_CHECK then only the keys linked to
> >> the keyrings given in "keyrings=" option are measured.
> >
> > This description does not seem to match the code, which for some
> > reason isn't included in this patch, nor in 3/10. Please
> > combine/squash patches 2 & 3. Missing from the combined patch is the
> > keyring matching code in ima_match_rules().
>
> This patch defines "keyrings=" option in the IMA policy and adds the
> related field in ima_rule_entry struct.
>
> The code for updating the new field in ima_rule_entry is in patch #4
> [PATCH v5 04/10] IMA: Updated IMA policy functions to return keyrings
> option read from the policy
That's the problem. The keyrings doesn't need to be returned, but
processed in ima_match_rules().
Mimi
^ permalink raw reply
* Re: [PATCH v5 0/10] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-12 17:52 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573578526.17949.47.camel@linux.ibm.com>
On 11/12/2019 9:08 AM, Mimi Zohar wrote:
> On Mon, 2019-11-11 at 11:41 -0800, Lakshmi Ramasubramanian wrote:
>> On 11/11/2019 11:32 AM, Lakshmi Ramasubramanian wrote:
>>
>> Hi Mimi,
>>
>>> Problem Statement:
>
> The above line isn't needed.
Will update.
> I've commented on patches 1 - 4. There's still so much wrong with
> this patch set. Limiting the scope of the patch set sounds like
> really a good idea.
>
> Mimi
I'll address your comments and send an update - I'll split this into 2
patch sets.
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH] x86/mtrr: only administrator can read the configurations.
From: Borislav Petkov @ 2019-11-12 17:49 UTC (permalink / raw)
To: Kees Cook
Cc: Zhang Xiaoxu, zhangxiaoxu, mingo, hpa, x86, tyhicks, colin.king,
tglx, linux-kernel, linux-security-module, Matthew Garrett
In-Reply-To: <201911110934.AC5BA313@keescook>
On Mon, Nov 11, 2019 at 09:56:16AM -0800, Kees Cook wrote:
> Some recap from being accidentally offlist:
>
> - this patch should check capabilities at open time (or retain the
> checks on the opener's permissions for later checks).
>
> - changing the DAC permissions might break something that expects to
> read mtrr when not uid 0.
>
> - if we leave the DAC permissions alone and just move the capable check
> to the opener, we should get the intent of the original patch. (i.e.
> check against CAP_SYS_ADMIN not just the wider uid 0.)
>
> - *this may still break things* if userspace expects to be able to
> read other parts of the file as non-uid-0 and non-CAP_SYS_ADMIN.
> If *that* is the case, then we need to censor the contents using
> the opener's permissions (as done in other /proc cases).
>
> I think the most cautious way forward is something like
> 51d7b120418e ("/proc/iomem: only expose physical resource addresses to
> privileged users"). Untested (and should likely be expanded to know
> about read vs write for lockdown interaction):
I'm back'n'forth on this.
So tglx and I agree that it doesn't make a whole lotta sense for
non-privileged luserspace to be able to read /proc/mtrr because it is a
small leak and normal users shouldn't care about the caching attributes
of memory regions in the first place.
So maybe we should do the second variant.
But then we're not supposed to break luserspace.
But then we can revert it if we do...
Ugh.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v5 04/10] IMA: Updated IMA policy functions to return keyrings option read from the policy
From: Lakshmi Ramasubramanian @ 2019-11-12 17:47 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573578316.17949.43.camel@linux.ibm.com>
On 11/12/2019 9:05 AM, Mimi Zohar wrote:
>> int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
>> enum ima_hooks func, int mask, int flags, int *pcr,
>> - struct ima_template_desc **template_desc)
>> + struct ima_template_desc **template_desc,
>> + char **keyrings)
>> {
>> struct ima_rule_entry *entry;
>> int action = 0, actmask = flags | (flags << 1);
>> @@ -527,6 +529,9 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
>> if ((pcr) && (entry->flags & IMA_PCR))
>> *pcr = entry->pcr;
>>
>> + if ((keyrings) && (entry->flags & IMA_KEYRINGS))
>> + *keyrings = entry->keyrings;
>
> ima_match_rules() determines whether the rule is in policy or not. It
> returns true on rule match, false on failure. There's no need to
> return the list of keyrings.
But the above code change is in ima_match_policy() - not in
ima_match_rules() function.
ima_match_rules() function is updated in Patch #1 -
[PATCH v5 01/10] IMA: Added KEYRING_CHECK func in IMA policy to measure keys
I've updated that function to check if func is "KEYRING_CHECK" and
return true\false as appropriate.
Am I missing something?
-lakshmi
^ permalink raw reply
* Re: [PATCH v5 02/10] IMA: Added keyrings= option in IMA policy to only measure keys added to the specified keyrings.
From: Lakshmi Ramasubramanian @ 2019-11-12 17:43 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573578305.17949.42.camel@linux.ibm.com>
On 11/12/2019 9:05 AM, Mimi Zohar wrote:
> The C maximum line length is 80 characters. The subject line is even
> less than that (~50). The Subject line here could be "ima: add
> support to limit measuring keys".
I'll update the subject line for the patches - limit to max 50 chars.
>
> On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
>> IMA policy needs to support measuring only those keys linked to
>> a specific set of keyrings.
>
> Patch descriptions should be written in the imperative. For example,
> "Limit measuring keys to those keys being loaded onto a specific
> keyring."
Will update.
>
>>
>> This patch defines a new IMA policy option namely "keyrings=" that
>> can be used to specify a set of keyrings. If this option is specified
>> in the policy for func=KEYRING_CHECK then only the keys linked to
>> the keyrings given in "keyrings=" option are measured.
>
> This description does not seem to match the code, which for some
> reason isn't included in this patch, nor in 3/10. Please
> combine/squash patches 2 & 3. Missing from the combined patch is the
> keyring matching code in ima_match_rules().
This patch defines "keyrings=" option in the IMA policy and adds the
related field in ima_rule_entry struct.
The code for updating the new field in ima_rule_entry is in patch #4
[PATCH v5 04/10] IMA: Updated IMA policy functions to return keyrings
option read from the policy
I'll update the description for this patch (#2).
-lakshmi
^ permalink raw reply
* Re: [PATCH v5 01/10] IMA: Added KEYRING_CHECK func in IMA policy to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-12 17:37 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573578296.17949.41.camel@linux.ibm.com>
On 11/12/2019 9:04 AM, Mimi Zohar wrote:
> On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
>> IMA policy needs to support a func to enable measurement of
>> asymmetric keys.
>>
>> This patch defines a new IMA policy func namely KEYRING_CHECK to
>> measure asymmetric keys.
>
> This new feature measures "keys" based on policy, not "keyrings".
> Please change the name to KEY_CHECK.
Good point - I will change the func name to KEY_CHECK.
-lakshmi
^ permalink raw reply
* Re: [PATCH v5 0/10] KEYS: Measure keys when they are created or updated
From: Mimi Zohar @ 2019-11-12 17:08 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <b135b1ac-add6-aea4-cab3-3e9c12796b6a@linux.microsoft.com>
On Mon, 2019-11-11 at 11:41 -0800, Lakshmi Ramasubramanian wrote:
> On 11/11/2019 11:32 AM, Lakshmi Ramasubramanian wrote:
>
> Hi Mimi,
>
> > Problem Statement:
The above line isn't needed.
> >
> > Keys created or updated in the system are currently not being measured.
> >
> > This change aims to address measuring keys created or updated
> > in the system:
> >
> > => Patches #1 through #5 update IMA policy functions to handle
> > measurement of keys based on configured IMA policy.
> >
> > => Patches #6 and #7 add IMA hook for measuring keys and the call
> > to the IMA hook from key_create_or_update function.
> > Keys are processed immediately - no support for
> > deferred processing.
> >
> > => Patches #8 through #10 add support for queuing keys if
> > custom IMA policies have not been applied yet and process
> > the queued keys when custom IMA policies are applied.
>
> I was wondering if it'd be better to split this patch set into two sets:
>
> 1st set including the patches for measuring keys without queuing support
> (Patches #1 through #7)
I've commented on patches 1 - 4. There's still so much wrong with
this patch set. Limiting the scope of the patch set sounds like
really a good idea.
Mimi
>
> 2nd set including the patches that add queuing support (Patches #8
> through #10).
^ permalink raw reply
* Re: [PATCH v5 04/10] IMA: Updated IMA policy functions to return keyrings option read from the policy
From: Mimi Zohar @ 2019-11-12 17:05 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191111193303.12781-5-nramas@linux.microsoft.com>
On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 1aee3c8b9cf6..d1889eee9287 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -481,6 +481,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
> * @pcr: set the pcr to extend
> * @template_desc: the template that should be used for this rule
> + * @keyrings: set the keyrings for this rule, if specified
> *
> * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
> * conditions.
> @@ -491,7 +492,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> */
> int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> enum ima_hooks func, int mask, int flags, int *pcr,
> - struct ima_template_desc **template_desc)
> + struct ima_template_desc **template_desc,
> + char **keyrings)
> {
> struct ima_rule_entry *entry;
> int action = 0, actmask = flags | (flags << 1);
> @@ -527,6 +529,9 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
> if ((pcr) && (entry->flags & IMA_PCR))
> *pcr = entry->pcr;
>
> + if ((keyrings) && (entry->flags & IMA_KEYRINGS))
> + *keyrings = entry->keyrings;
ima_match_rules() determines whether the rule is in policy or not. It
returns true on rule match, false on failure. There's no need to
return the list of keyrings.
Mimi
> +
> if (template_desc && entry->template)
> *template_desc = entry->template;
>
^ permalink raw reply
* Re: [PATCH v5 02/10] IMA: Added keyrings= option in IMA policy to only measure keys added to the specified keyrings.
From: Mimi Zohar @ 2019-11-12 17:05 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191111193303.12781-3-nramas@linux.microsoft.com>
The C maximum line length is 80 characters. The subject line is even
less than that (~50). The Subject line here could be "ima: add
support to limit measuring keys".
On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
> IMA policy needs to support measuring only those keys linked to
> a specific set of keyrings.
Patch descriptions should be written in the imperative. For example,
"Limit measuring keys to those keys being loaded onto a specific
keyring."
>
> This patch defines a new IMA policy option namely "keyrings=" that
> can be used to specify a set of keyrings. If this option is specified
> in the policy for func=KEYRING_CHECK then only the keys linked to
> the keyrings given in "keyrings=" option are measured.
This description does not seem to match the code, which for some
reason isn't included in this patch, nor in 3/10. Please
combine/squash patches 2 & 3. Missing from the combined patch is the
keyring matching code in ima_match_rules().
>
> If "keyrings=" option is not specified for func=KEYRING_CHECK then
> all keys are measured.
The last sentence is unnecessary. Please remove.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
> Documentation/ABI/testing/ima_policy | 10 +++++++++-
> security/integrity/ima/ima_policy.c | 2 ++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 341df49b5ad1..be2874fa3928 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -25,7 +25,7 @@ Description:
> lsm: [[subj_user=] [subj_role=] [subj_type=]
> [obj_user=] [obj_role=] [obj_type=]]
> option: [[appraise_type=]] [template=] [permit_directio]
> - [appraise_flag=]
> + [appraise_flag=] [keyrings=]
> base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> [FIRMWARE_CHECK]
> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> @@ -43,6 +43,9 @@ Description:
> appraise_flag:= [check_blacklist]
> Currently, blacklist check is only for files signed with appended
> signature.
> + keyrings:= list of keyrings
> + (eg, .builtin_trusted_keys|.ima). Only valid
> + when action is "measure" and func is KEYRING_CHECK.
> template:= name of a defined IMA template type
> (eg, ima-ng). Only valid when action is "measure".
> pcr:= decimal value
> @@ -119,3 +122,8 @@ Description:
> all keys:
>
> measure func=KEYRING_CHECK
> +
> + Example of measure rule using KEYRING_CHECK to only measure
> + keys added to .builtin_trusted_keys or .ima keyring:
> +
> + measure func=KEYRING_CHECK keyrings=.builtin_trusted_keys|.ima
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 9ca32ffaaa9d..a0f7ffa80736 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -34,6 +34,7 @@
> #define IMA_EUID 0x0080
> #define IMA_PCR 0x0100
> #define IMA_FSNAME 0x0200
> +#define IMA_KEYRINGS 0x0400
>
> #define UNKNOWN 0
> #define MEASURE 0x0001 /* same as IMA_MEASURE */
> @@ -79,6 +80,7 @@ struct ima_rule_entry {
> int type; /* audit type */
> } lsm[MAX_LSM_RULES];
> char *fsname;
> + char *keyrings; /* Measure keys added to these keyrings */
> struct ima_template_desc *template;
> };
>
^ permalink raw reply
* Re: [PATCH v5 01/10] IMA: Added KEYRING_CHECK func in IMA policy to measure keys
From: Mimi Zohar @ 2019-11-12 17:04 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191111193303.12781-2-nramas@linux.microsoft.com>
On Mon, 2019-11-11 at 11:32 -0800, Lakshmi Ramasubramanian wrote:
> IMA policy needs to support a func to enable measurement of
> asymmetric keys.
>
> This patch defines a new IMA policy func namely KEYRING_CHECK to
> measure asymmetric keys.
This new feature measures "keys" based on policy, not "keyrings".
Please change the name to KEY_CHECK.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
> Documentation/ABI/testing/ima_policy | 6 ++++++
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_policy.c | 4 +++-
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 29aaedf33246..341df49b5ad1 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -30,6 +30,7 @@ Description:
> [FIRMWARE_CHECK]
> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> [KEXEC_CMDLINE]
> + [KEYRING_CHECK]
> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
> [[^]MAY_EXEC]
> fsmagic:= hex value
> @@ -113,3 +114,8 @@ Description:
> Example of appraise rule allowing modsig appended signatures:
>
> appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
> +
> + Example of measure rule using KEYRING_CHECK to measure
> + all keys:
> +
> + measure func=KEYRING_CHECK
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df4ca482fb53..7f23405b2718 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -193,6 +193,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
> hook(KEXEC_INITRAMFS_CHECK) \
> hook(POLICY_CHECK) \
> hook(KEXEC_CMDLINE) \
> + hook(KEYRING_CHECK) \
> hook(MAX_CHECK)
> #define __ima_hook_enumify(ENUM) ENUM,
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index f19a895ad7cd..9ca32ffaaa9d 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -373,7 +373,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> {
> int i;
>
> - if (func == KEXEC_CMDLINE) {
> + if ((func == KEXEC_CMDLINE) || (func == KEYRING_CHECK)) {
> if ((rule->flags & IMA_FUNC) && (rule->func == func))
> return true;
> return false;
> @@ -997,6 +997,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> entry->func = POLICY_CHECK;
> else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0)
> entry->func = KEXEC_CMDLINE;
> + else if (strcmp(args[0].from, "KEYRING_CHECK") == 0)
> + entry->func = KEYRING_CHECK;
> else
> result = -EINVAL;
> if (!result)
^ permalink raw reply
* Re: [PATCH v5 0/10] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-11 19:41 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191111193303.12781-1-nramas@linux.microsoft.com>
On 11/11/2019 11:32 AM, Lakshmi Ramasubramanian wrote:
Hi Mimi,
> Problem Statement:
>
> Keys created or updated in the system are currently not being measured.
>
> This change aims to address measuring keys created or updated
> in the system:
>
> => Patches #1 through #5 update IMA policy functions to handle
> measurement of keys based on configured IMA policy.
>
> => Patches #6 and #7 add IMA hook for measuring keys and the call
> to the IMA hook from key_create_or_update function.
> Keys are processed immediately - no support for
> deferred processing.
>
> => Patches #8 through #10 add support for queuing keys if
> custom IMA policies have not been applied yet and process
> the queued keys when custom IMA policies are applied.
I was wondering if it'd be better to split this patch set into two sets:
1st set including the patches for measuring keys without queuing support
(Patches #1 through #7)
2nd set including the patches that add queuing support (Patches #8
through #10).
thanks,
-lakshmi
^ permalink raw reply
* [PATCH v5 03/10] IMA: Read keyrings= option from the IMA policy into ima_rule_entry
From: Lakshmi Ramasubramanian @ 2019-11-11 19:32 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191111193303.12781-1-nramas@linux.microsoft.com>
"keyrings=" option, if specified in the IMA policy, needs to be
stored in the list of IMA rules when the configured IMA policy is read.
This patch defines a new policy token enum namely Opt_keyrings
for reading "keyrings=" option from the IMA policy.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/ima_policy.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a0f7ffa80736..1aee3c8b9cf6 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -768,7 +768,8 @@ enum {
Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
Opt_appraise_type, Opt_appraise_flag,
- Opt_permit_directio, Opt_pcr, Opt_template, Opt_err
+ Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
+ Opt_err
};
static const match_table_t policy_tokens = {
@@ -804,6 +805,7 @@ static const match_table_t policy_tokens = {
{Opt_permit_directio, "permit_directio"},
{Opt_pcr, "pcr=%s"},
{Opt_template, "template=%s"},
+ {Opt_keyrings, "keyrings=%s"},
{Opt_err, NULL}
};
@@ -1053,6 +1055,23 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
result = 0;
entry->flags |= IMA_FSNAME;
break;
+ case Opt_keyrings:
+ ima_log_string(ab, "keyrings", args[0].from);
+
+ if ((entry->keyrings) ||
+ (entry->action != MEASURE) ||
+ (entry->func != KEYRING_CHECK)) {
+ result = -EINVAL;
+ break;
+ }
+ entry->keyrings = kstrdup(args[0].from, GFP_KERNEL);
+ if (!entry->keyrings) {
+ result = -ENOMEM;
+ break;
+ }
+ result = 0;
+ entry->flags |= IMA_KEYRINGS;
+ break;
case Opt_fsuuid:
ima_log_string(ab, "fsuuid", args[0].from);
@@ -1428,6 +1447,13 @@ int ima_policy_show(struct seq_file *m, void *v)
seq_puts(m, " ");
}
+ if (entry->flags & IMA_KEYRINGS) {
+ if (entry->keyrings != NULL)
+ snprintf(tbuf, sizeof(tbuf), "%s", entry->keyrings);
+ seq_printf(m, pt(Opt_keyrings), tbuf);
+ seq_puts(m, " ");
+ }
+
if (entry->flags & IMA_PCR) {
snprintf(tbuf, sizeof(tbuf), "%d", entry->pcr);
seq_printf(m, pt(Opt_pcr), tbuf);
--
2.17.1
^ permalink raw reply related
* [PATCH v5 0/10] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-11 19:32 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
Problem Statement:
Keys created or updated in the system are currently not being measured.
Therefore an attestation service, for instance, would not be able to
attest whether or not the trusted keys keyring(s), for instance, contain
only known good (trusted) keys.
IMA measures system files, command line arguments passed to kexec,
boot aggregate, etc. It can be used to measure keys as well.
But there is no mechanism available in the kernel for IMA to
know when a key is created or updated.
This change aims to address measuring keys created or updated
in the system:
=> Patches #1 through #5 update IMA policy functions to handle
measurement of keys based on configured IMA policy.
=> Patches #6 and #7 add IMA hook for measuring keys and the call
to the IMA hook from key_create_or_update function.
Keys are processed immediately - no support for
deferred processing.
=> Patches #8 through #10 add support for queuing keys if
custom IMA policies have not been applied yet and process
the queued keys when custom IMA policies are applied.
To achieve the above the following changes have been made:
- Added a new IMA hook namely, ima_post_key_create_or_update, which
measures the key. This IMA hook is called from key_create_or_update
function. The key measurement can be controlled through IMA policy.
In this change set a new IMA policy function KEYRING_CHECK has been
added to measure keys. The policy can optionally specify a set of
keyrings to measure. By default all keyrings are included in
the measurement when KEYRING_CHECK policy is specified.
# measure all keys
measure func=KEYRING_CHECK
# measure keys on the IMA keyring
measure func=KEYRING_CHECK keyring=".ima"
# measure keys on the BUILTIN and IMA keyrings into a different PCR
measure func=KEYRING_CHECK keyring=".builtin_trusted_keys|.ima" pcr=11
Testing performed:
* Booted the kernel with this change.
* Executed keyctl tests from the Linux Test Project (LTP)
* All keys are measured when only KEYRING_CHECK is set.
* Only keys added to the given keyrings are measured
when keyrings option is set.
* Keys are not measured when KEYRING_CHECK is not set.
* Key is queued for measurement if IMA is not yet initialized
and processed when IMA is initialized.
* Key is measured rightaway when IMA is initialized.
* Added a new key to a keyring and verified "key create" code path.
=> In this case added a key to .ima keyring.
* Added the same key again and verified "key update" code path.
=> Add the same key to .ima keyring.
Change Log:
v5:
=> Reorganized the patches to add measurement of keys through
the IMA hook without any queuing and then added queuing support.
=> Updated the queuing functions to minimize code executed inside mutex.
=> Process queued keys after custom IMA policies have been applied.
v4:
=> Rebased the changes to v5.4-rc3
=> Applied the following dependent patch set first
and then added new changes.
https://lore.kernel.org/linux-integrity/1572492694-6520-1-git-send-email-zohar@linux.ibm.com
=> Refactored the patch set to separate out changes related to
func KEYRING_CHECK and options keyrings into different patches.
=> Moved the functions to queue and dequeue keys for measurement
from ima_queue.c to a new file ima_asymmetric_keys.c.
=> Added a new config namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
to compile ima_asymmetric_keys.c
v3:
=> Added KEYRING_CHECK for measuring keys. This can optionally specify
keyrings to measure.
=> Updated ima_get_action() and related functions to return
the keyrings if specified in the policy.
=> process_buffer_measurement() function is updated to take keyring
as a parameter. The key will be measured if the policy includes
the keyring in the list of measured keyrings. If the policy does not
specify any keyrings then all keys are measured.
v2:
=> Per suggestion from Mimi reordered the patch set to first
enable measuring keys added or updated in the system.
And, then scope the measurement to keys added to
builtin_trusted_keys keyring through ima policy.
=> Removed security_key_create_or_update function and instead
call ima hook, to measure the key, directly from
key_create_or_update function.
v1:
=> LSM function for key_create_or_update. It calls ima.
=> Added ima hook for measuring keys
=> ima measures keys based on ima policy.
v0:
=> Added LSM hook for key_create_or_update.
=> Measure keys added to builtin or secondary trusted keys keyring.
Background:
Currently ima measures file hashes and .ima signatures. ima signatures
are validated against keys in the ".ima" keyring. If the kernel is built
with CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY enabled,
then all keys in ".ima" keyring must be signed by a key in
".builtin_trusted_keys" or ".secondary_trusted_keys" keyrings.
Although ima supports the above configuration, not having an insight
into what keys are present in these trusted keys keyrings would prevent
an attestation service from validating a client machine.
On systems with CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
enabled, measuring keys in the ".builtin_trusted_keys" keyring provides
a mechanism to attest that the client's system binaries are indeed signed
by signers that chain to known trusted keys.
Without this change, to attest the clients one needs to maintain
an "allowed list" of file hashes of all versions of all client binaries
that are deployed on the clients in the enterprise. That is a huge
operational challenge in a large scale environment of clients with
heterogenous builds. This also limits scalability and agility of
rolling out frequent client binary updates.
Questions and concerns raised by reviewers on this patch set:
Question 1:
Is "Signed with a trusted key" equal to "Trusted file"?
Doesn't the service need the hashes of the system files to determine
whether a file is trusted or not?
"Signed with a trusted key" does not equal "Trusted"
Answer:
Agree "Signed with a trusted key" may not equal "Trusted".
To address this, the attesting service can maintain a small
manageable set of bad hashes (a "Blocked list") and a list of
trusted keys expected in client's .builtin_trusted_keys" keyring.
Using this data, the service can detect the presence of
"Disallowed (untrusted) version of client binaries".
Question 2:
Providing more data to the service (such as ".builtin_trusted_keys"),
empowers the service to deny access to clients (block clients).
IMA walks a fine line in enforcing and measuring file integrity.
This patchset breaches that fine line and in doing so brings back
the fears of trusted computing.
Answer:
Any new measurement we add in IMA will provide more data to service
and can enable it to deny access to clients. It is not clear why this patch
set would breach the fine line between measuring and enforcing.
Since this patch set is disabled by default and enabled through
CONFIG_IMA_MEASURE_TRUSTED_KEYS, only those enterprises that
require this new measurement can opt-in for it. Since it is disabled
by default, it does not restrict the autonomy of independent users
who are unaffected by attestation.
Question 3:
IMA log already contains a pointer to the IMA keys used for signature
verification. Why does the service need to care what keys were used
to sign (install) the IMA keys? What is gained by measuring the keys
in the ".builtin_trusted_keys"
Answer:
To attest the clients using the current IMA log, service needs to maintain
hashes of all the deployed versions of all the system binaries for their
enterprise. This will introduce a very high operational overhead in
a large scale environment of clients with heterogenous builds.
This limits scalability and agility of rolling out frequent client
binary updates.
On the other hand, with the current patch set, we will have IMA
validate the file signature on the clients and the service validate
that the IMA keys were installed using trusted keys.
This provides a chain of trust:
=> IMA Key validates file signature on the client
=> Built-In trusted key attests IMA key on the client
=> Attestation service attests the Built-In trusted keys
reported by the client in the IMA log
This approach, therefore, would require the service to maintain
a manageble set of trusted keys that it receives from a trusted source.
And, verify if the clients only have keys from that set of trusted keys.
Question 4:
Where will the attestation service receive the keys to validate against?
Answer:
Attestation service will receive the keys from a trusted source such as
the enterprise build services that provides the client builds.
The service will use this set of keys to verify that the keys reported by
the clients in the IMA log contains only keys from this trusted list.
Question 5:
What is changing in the IMA log through this patch set?
Answer:
This patch set does not remove any data that is currently included
in the IMA log. It only adds more data to the IMA log - the data on
".builtin_trusted_keys"
Lakshmi Ramasubramanian (10):
IMA: Added KEYRING_CHECK func in IMA policy to measure keys
IMA: Added keyrings= option in IMA policy to only measure keys added
to the specified keyrings.
IMA: Read keyrings= option from the IMA policy into ima_rule_entry
IMA: Updated IMA policy functions to return keyrings option read from
the policy
IMA: Measure key if the IMA policy allows measurement for the keyring
to which the key is linked to
IMA: Defined an IMA hook to measure keys on key create or update
KEYS: Call the IMA hook to measure key when a new key is created or an
existing key is updated
IMA: Added a flag to determine whether IMA hook can process the key
now or has to queue for processing later
IMA: Defined functions to queue and dequeue keys for measurement
IMA: Call queue and dequeue functions to measure keys.
Documentation/ABI/testing/ima_policy | 16 +-
include/linux/ima.h | 13 ++
security/integrity/ima/Kconfig | 14 ++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 32 ++-
security/integrity/ima/ima_api.c | 8 +-
security/integrity/ima/ima_appraise.c | 4 +-
security/integrity/ima/ima_asymmetric_keys.c | 201 +++++++++++++++++++
security/integrity/ima/ima_main.c | 31 ++-
security/integrity/ima/ima_policy.c | 53 ++++-
security/keys/key.c | 9 +
11 files changed, 366 insertions(+), 16 deletions(-)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
--
2.17.1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox