* [PATCH v11 22/25] Audit: Include object data for all security modules
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
When there is more than one context displaying security
module extend what goes into the audit record by supplimenting
the "obj=" with an "obj_<lsm>=" for each such security
module.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
kernel/audit.h | 4 +-
kernel/auditsc.c | 110 ++++++++++++++++++++++++-----------------------
2 files changed, 58 insertions(+), 56 deletions(-)
diff --git a/kernel/audit.h b/kernel/audit.h
index af9bc09e656c..c9f1e1641542 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -78,7 +78,7 @@ struct audit_names {
kuid_t uid;
kgid_t gid;
dev_t rdev;
- u32 osid;
+ struct lsmblob oblob;
struct audit_cap_data fcap;
unsigned int fcap_ver;
unsigned char type; /* record type */
@@ -152,7 +152,7 @@ struct audit_context {
kuid_t uid;
kgid_t gid;
umode_t mode;
- u32 osid;
+ struct lsmblob oblob;
int has_perm;
uid_t perm_uid;
gid_t perm_gid;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e0dd643e9b13..0c071947c2b3 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -659,17 +659,15 @@ 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(
- &blob,
+ &name->oblob,
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(
- &blob,
+ &n->oblob,
f->type,
f->op,
f->lsm_rule)) {
@@ -681,8 +679,7 @@ static int audit_filter_rules(struct task_struct *tsk,
/* Find ipc objects that match */
if (!ctx || ctx->type != AUDIT_IPC)
break;
- lsmblob_init(&blob, ctx->ipc.osid);
- if (security_audit_rule_match(&blob,
+ if (security_audit_rule_match(&ctx->ipc.oblob,
f->type, f->op,
f->lsm_rule))
++result;
@@ -956,13 +953,57 @@ static inline void audit_free_context(struct audit_context *context)
kfree(context);
}
+static int audit_log_object_context(struct audit_buffer *ab,
+ struct lsmblob *blob)
+{
+ struct lsmcontext context;
+ const char *lsm;
+ int i;
+
+ /*
+ * None of the installed modules have object labels.
+ */
+ if (security_lsm_slot_name(0) == NULL)
+ return 0;
+
+ if (blob->secid[0] != 0) {
+ if (security_secid_to_secctx(blob, &context, 0)) {
+ audit_log_format(ab, " obj=?");
+ return 1;
+ }
+ audit_log_format(ab, " obj=%s", context.context);
+ security_release_secctx(&context);
+ }
+
+ /*
+ * Don't do anything more unless there is more than one LSM
+ * with a security context to report.
+ */
+ if (security_lsm_slot_name(1) == NULL)
+ return 0;
+
+ for (i = 0; i < LSMBLOB_ENTRIES; i++) {
+ lsm = security_lsm_slot_name(i);
+ if (lsm == NULL)
+ break;
+ if (blob->secid[i] == 0)
+ continue;
+ if (security_secid_to_secctx(blob, &context, i)) {
+ audit_log_format(ab, " obj_%s=?", lsm);
+ continue;
+ }
+ audit_log_format(ab, " obj_%s=%s", lsm, context.context);
+ security_release_secctx(&context);
+ }
+ return 0;
+}
+
static int audit_log_pid_context(struct audit_context *context, pid_t pid,
kuid_t auid, kuid_t uid,
unsigned int sessionid,
struct lsmblob *blob, char *comm)
{
struct audit_buffer *ab;
- struct lsmcontext lsmctx;
int rc = 0;
ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
@@ -972,15 +1013,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
from_kuid(&init_user_ns, auid),
from_kuid(&init_user_ns, uid), sessionid);
- if (lsmblob_is_set(blob)) {
- if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
- audit_log_format(ab, " obj=(none)");
- rc = 1;
- } else {
- audit_log_format(ab, " obj=%s", lsmctx.context);
- security_release_secctx(&lsmctx);
- }
- }
+ rc = audit_log_object_context(ab, blob);
audit_log_format(ab, " ocomm=");
audit_log_untrustedstring(ab, comm);
audit_log_end(ab);
@@ -1207,26 +1240,14 @@ static void show_special(struct audit_context *context, int *call_panic)
context->socketcall.args[i]);
break; }
case AUDIT_IPC: {
- u32 osid = context->ipc.osid;
+ struct lsmblob *oblob = & context->ipc.oblob;
audit_log_format(ab, "ouid=%u ogid=%u mode=%#ho",
from_kuid(&init_user_ns, context->ipc.uid),
from_kgid(&init_user_ns, context->ipc.gid),
context->ipc.mode);
- if (osid) {
- struct lsmcontext lsmcxt;
- struct lsmblob blob;
-
- lsmblob_init(&blob, osid);
- if (security_secid_to_secctx(&blob, &lsmcxt,
- LSMBLOB_FIRST)) {
- audit_log_format(ab, " osid=%u", osid);
- *call_panic = 1;
- } else {
- audit_log_format(ab, " obj=%s", lsmcxt.context);
- security_release_secctx(&lsmcxt);
- }
- }
+ if (audit_log_object_context(ab, oblob))
+ *call_panic = 1;
if (context->ipc.has_perm) {
audit_log_end(ab);
ab = audit_log_start(context, GFP_KERNEL,
@@ -1366,20 +1387,8 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
from_kgid(&init_user_ns, n->gid),
MAJOR(n->rdev),
MINOR(n->rdev));
- if (n->osid != 0) {
- struct lsmblob blob;
- struct lsmcontext lsmctx;
-
- lsmblob_init(&blob, n->osid);
- if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
- audit_log_format(ab, " osid=%u", n->osid);
- if (call_panic)
- *call_panic = 2;
- } else {
- audit_log_format(ab, " obj=%s", lsmctx.context);
- security_release_secctx(&lsmctx);
- }
- }
+ if (audit_log_object_context(ab, &n->oblob) && call_panic)
+ *call_panic = 2;
/* log the audit_names record type */
switch (n->type) {
@@ -1929,17 +1938,13 @@ static void audit_copy_inode(struct audit_names *name,
const struct dentry *dentry,
struct inode *inode, unsigned int flags)
{
- struct lsmblob blob;
-
name->ino = inode->i_ino;
name->dev = inode->i_sb->s_dev;
name->mode = inode->i_mode;
name->uid = inode->i_uid;
name->gid = inode->i_gid;
name->rdev = inode->i_rdev;
- security_inode_getsecid(inode, &blob);
- /* scaffolding until osid is updated */
- name->osid = blob.secid[0];
+ security_inode_getsecid(inode, &name->oblob);
if (flags & AUDIT_INODE_NOEVAL) {
name->fcap_ver = -1;
return;
@@ -2285,14 +2290,11 @@ void __audit_mq_getsetattr(mqd_t mqdes, struct mq_attr *mqstat)
void __audit_ipc_obj(struct kern_ipc_perm *ipcp)
{
struct audit_context *context = audit_context();
- struct lsmblob blob;
context->ipc.uid = ipcp->uid;
context->ipc.gid = ipcp->gid;
context->ipc.mode = ipcp->mode;
context->ipc.has_perm = 0;
- security_ipc_getsecid(ipcp, &blob);
- /* scaffolding on the [0] - change "osid" to a lsmblob */
- context->ipc.osid = blob.secid[0];
+ security_ipc_getsecid(ipcp, &context->ipc.oblob);
context->type = AUDIT_IPC;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v11 24/25] LSM: Add /proc attr entry for full LSM context
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
Add an entry /proc/.../attr/context which displays the full
process security "context" in compound format:'
lsm1\0value\0lsm2\0value\0...
This entry is not writable.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
cc: linux-api@vger.kernel.org
---
fs/proc/base.c | 1 +
security/security.c | 27 +++++++++++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 950c200cb9ad..d13c2cf50e4b 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2653,6 +2653,7 @@ static const struct pid_entry attr_dir_stuff[] = {
ATTR(NULL, "keycreate", 0666),
ATTR(NULL, "sockcreate", 0666),
ATTR(NULL, "display", 0666),
+ ATTR(NULL, "context", 0666),
#ifdef CONFIG_SECURITY_SMACK
DIR("smack", 0555,
proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
diff --git a/security/security.c b/security/security.c
index f1fefa187ef8..7e5353f98988 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2099,6 +2099,10 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
char **value)
{
struct security_hook_list *hp;
+ char *final = NULL;
+ char *cp;
+ int rc = 0;
+ int finallen = 0;
int display = lsm_task_display(current);
int slot = 0;
@@ -2126,6 +2130,29 @@ int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
return -ENOMEM;
}
+ if (!strcmp(name, "context")) {
+ hlist_for_each_entry(hp, &security_hook_heads.getprocattr,
+ list) {
+ rc = hp->hook.getprocattr(p, "current", &cp);
+ if (rc == -EINVAL || rc == -ENOPROTOOPT)
+ continue;
+ if (rc < 0) {
+ kfree(final);
+ return rc;
+ }
+ rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+ cp, rc);
+ if (rc < 0) {
+ kfree(final);
+ return rc;
+ }
+ }
+ if (final == NULL)
+ return -EINVAL;
+ *value = final;
+ return finallen;
+ }
+
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
--
2.20.1
^ permalink raw reply related
* [PATCH v11 20/25] LSM: Verify LSM display sanity in binder
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
Verify that the tasks on the ends of a binder transaction
use the same "display" security module. This prevents confusion
of security "contexts".
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/security.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/security/security.c b/security/security.c
index 101063b00aeb..0e8c61cceecd 100644
--- a/security/security.c
+++ b/security/security.c
@@ -738,9 +738,38 @@ int security_binder_set_context_mgr(struct task_struct *mgr)
return call_int_hook(binder_set_context_mgr, 0, mgr);
}
+/**
+ * security_binder_transaction - Binder driver transacton check
+ * @from: source of the transaction
+ * @to: destination of the transaction
+ *
+ * Verify that the tasks have the same LSM "display", then
+ * call the security module hooks.
+ *
+ * Returns -EINVAL if the displays don't match, or the
+ * result of the security module checks.
+ */
int security_binder_transaction(struct task_struct *from,
struct task_struct *to)
{
+ int from_display = lsm_task_display(from);
+ int to_display = lsm_task_display(to);
+
+ /*
+ * If the display is LSMBLOB_INVALID the first module that has
+ * an entry is used. This will be in the 0 slot.
+ *
+ * This is currently only required if the server has requested
+ * peer contexts, but it would be unwieldly to have too much of
+ * the binder driver detail here.
+ */
+ if (from_display == LSMBLOB_INVALID)
+ from_display = 0;
+ if (to_display == LSMBLOB_INVALID)
+ to_display = 0;
+ if (from_display != to_display)
+ return -EINVAL;
+
return call_int_hook(binder_transaction, 0, from, to);
}
--
2.20.1
^ permalink raw reply related
* [PATCH v11 25/25] AppArmor: Remove the exclusive flag
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
With the inclusion of the "display" process attribute
mechanism AppArmor no longer needs to be treated as an
"exclusive" security module. Remove the flag that indicates
it is exclusive. Remove the stub getpeersec_dgram AppArmor
hook as it has no effect in the single LSM case and
interferes in the multiple LSM case.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
---
security/apparmor/lsm.c | 20 +-------------------
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index b4c964fdc2f9..81bc4f773429 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1099,22 +1099,6 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock, char **optval,
return error;
}
-/**
- * apparmor_socket_getpeersec_dgram - get security label of packet
- * @sock: the peer socket
- * @skb: packet data
- * @secid: pointer to where to put the secid of the packet
- *
- * Sets the netlabel socket state on sk from parent
- */
-static int apparmor_socket_getpeersec_dgram(struct socket *sock,
- struct sk_buff *skb, u32 *secid)
-
-{
- /* TODO: requires secid support */
- return -ENOPROTOOPT;
-}
-
/**
* apparmor_sock_graft - Initialize newly created socket
* @sk: child sock
@@ -1218,8 +1202,6 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
#endif
LSM_HOOK_INIT(socket_getpeersec_stream,
apparmor_socket_getpeersec_stream),
- LSM_HOOK_INIT(socket_getpeersec_dgram,
- apparmor_socket_getpeersec_dgram),
LSM_HOOK_INIT(sock_graft, apparmor_sock_graft),
#ifdef CONFIG_NETWORK_SECMARK
LSM_HOOK_INIT(inet_conn_request, apparmor_inet_conn_request),
@@ -1777,7 +1759,7 @@ static int __init apparmor_init(void)
DEFINE_LSM(apparmor) = {
.name = "apparmor",
- .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
+ .flags = LSM_FLAG_LEGACY_MAJOR,
.enabled = &apparmor_enabled,
.blobs = &apparmor_blob_sizes,
.init = apparmor_init,
--
2.20.1
^ permalink raw reply related
* [PATCH v11 19/25] NET: Store LSM netlabel data in a lsmblob
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
Netlabel uses LSM interfaces requiring an lsmblob and
the internal storage is used to pass information between
these interfaces, so change the internal data from a secid
to a lsmblob. Update the netlabel interfaces and their
callers to accommodate the change. This requires that the
modules using netlabel use the lsm_id.slot to access the
correct secid when using netlabel.
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
cc: netdev@vger.kernel.com
---
include/net/netlabel.h | 8 ++--
net/ipv4/cipso_ipv4.c | 6 ++-
net/netlabel/netlabel_kapi.c | 6 +--
net/netlabel/netlabel_unlabeled.c | 57 +++++++++++------------------
net/netlabel/netlabel_unlabeled.h | 2 +-
security/selinux/hooks.c | 2 +-
security/selinux/include/security.h | 1 +
security/selinux/netlabel.c | 2 +-
security/selinux/ss/services.c | 4 +-
security/smack/smack.h | 1 +
security/smack/smack_lsm.c | 5 ++-
security/smack/smackfs.c | 10 +++--
12 files changed, 50 insertions(+), 54 deletions(-)
diff --git a/include/net/netlabel.h b/include/net/netlabel.h
index 43ae50337685..73fc25b4042b 100644
--- a/include/net/netlabel.h
+++ b/include/net/netlabel.h
@@ -166,7 +166,7 @@ struct netlbl_lsm_catmap {
* @attr.mls: MLS sensitivity label
* @attr.mls.cat: MLS category bitmap
* @attr.mls.lvl: MLS sensitivity level
- * @attr.secid: LSM specific secid token
+ * @attr.lsmblob: LSM specific data
*
* Description:
* This structure is used to pass security attributes between NetLabel and the
@@ -201,7 +201,7 @@ struct netlbl_lsm_secattr {
struct netlbl_lsm_catmap *cat;
u32 lvl;
} mls;
- u32 secid;
+ struct lsmblob lsmblob;
} attr;
};
@@ -415,7 +415,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info);
int netlbl_cfg_unlbl_static_del(struct net *net,
const char *dev_name,
@@ -523,7 +523,7 @@ static inline int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
return -ENOSYS;
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 376882215919..8ee7a804423e 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1467,7 +1467,8 @@ static int cipso_v4_gentag_loc(const struct cipso_v4_doi *doi_def,
buffer[0] = CIPSO_V4_TAG_LOCAL;
buffer[1] = CIPSO_V4_TAG_LOC_BLEN;
- *(u32 *)&buffer[2] = secattr->attr.secid;
+ /* only one netlabel user - the first */
+ *(u32 *)&buffer[2] = secattr->attr.lsmblob.secid[0];
return CIPSO_V4_TAG_LOC_BLEN;
}
@@ -1487,7 +1488,8 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
const unsigned char *tag,
struct netlbl_lsm_secattr *secattr)
{
- secattr->attr.secid = *(u32 *)&tag[2];
+ /* only one netlabel user - the first */
+ secattr->attr.lsmblob.secid[0] = *(u32 *)&tag[2];
secattr->flags |= NETLBL_SECATTR_SECID;
return 0;
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 409a3ae47ce2..f2ebd43a7992 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -196,7 +196,7 @@ int netlbl_cfg_unlbl_map_add(const char *domain,
* @addr: IP address in network byte order (struct in[6]_addr)
* @mask: address mask in network byte order (struct in[6]_addr)
* @family: address family
- * @secid: LSM secid value for the entry
+ * @lsmblob: LSM data value for the entry
* @audit_info: NetLabel audit information
*
* Description:
@@ -210,7 +210,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
const void *addr,
const void *mask,
u16 family,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
u32 addr_len;
@@ -230,7 +230,7 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
return netlbl_unlhsh_add(net,
dev_name, addr, mask, addr_len,
- secid, audit_info);
+ lsmblob, audit_info);
}
/**
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index c03fe9a4f7b9..3b0f07b59436 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -66,7 +66,7 @@ struct netlbl_unlhsh_tbl {
#define netlbl_unlhsh_addr4_entry(iter) \
container_of(iter, struct netlbl_unlhsh_addr4, list)
struct netlbl_unlhsh_addr4 {
- u32 secid;
+ struct lsmblob lsmblob;
struct netlbl_af4list list;
struct rcu_head rcu;
@@ -74,7 +74,7 @@ struct netlbl_unlhsh_addr4 {
#define netlbl_unlhsh_addr6_entry(iter) \
container_of(iter, struct netlbl_unlhsh_addr6, list)
struct netlbl_unlhsh_addr6 {
- u32 secid;
+ struct lsmblob lsmblob;
struct netlbl_af6list list;
struct rcu_head rcu;
@@ -219,7 +219,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
* @iface: the associated interface entry
* @addr: IPv4 address in network byte order
* @mask: IPv4 address mask in network byte order
- * @secid: LSM secid value for entry
+ * @lsmblob: LSM data value for entry
*
* Description:
* Add a new address entry into the unlabeled connection hash table using the
@@ -230,7 +230,7 @@ static struct netlbl_unlhsh_iface *netlbl_unlhsh_search_iface(int ifindex)
static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
const struct in_addr *addr,
const struct in_addr *mask,
- u32 secid)
+ struct lsmblob *lsmblob)
{
int ret_val;
struct netlbl_unlhsh_addr4 *entry;
@@ -242,7 +242,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
entry->list.addr = addr->s_addr & mask->s_addr;
entry->list.mask = mask->s_addr;
entry->list.valid = 1;
- entry->secid = secid;
+ entry->lsmblob = *lsmblob;
spin_lock(&netlbl_unlhsh_lock);
ret_val = netlbl_af4list_add(&entry->list, &iface->addr4_list);
@@ -259,7 +259,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
* @iface: the associated interface entry
* @addr: IPv6 address in network byte order
* @mask: IPv6 address mask in network byte order
- * @secid: LSM secid value for entry
+ * @lsmblob: LSM data value for entry
*
* Description:
* Add a new address entry into the unlabeled connection hash table using the
@@ -270,7 +270,7 @@ static int netlbl_unlhsh_add_addr4(struct netlbl_unlhsh_iface *iface,
static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
const struct in6_addr *addr,
const struct in6_addr *mask,
- u32 secid)
+ struct lsmblob *lsmblob)
{
int ret_val;
struct netlbl_unlhsh_addr6 *entry;
@@ -286,7 +286,7 @@ static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
entry->list.addr.s6_addr32[3] &= mask->s6_addr32[3];
entry->list.mask = *mask;
entry->list.valid = 1;
- entry->secid = secid;
+ entry->lsmblob = *lsmblob;
spin_lock(&netlbl_unlhsh_lock);
ret_val = netlbl_af6list_add(&entry->list, &iface->addr6_list);
@@ -365,7 +365,7 @@ int netlbl_unlhsh_add(struct net *net,
const void *addr,
const void *mask,
u32 addr_len,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info)
{
int ret_val;
@@ -374,7 +374,6 @@ int netlbl_unlhsh_add(struct net *net,
struct netlbl_unlhsh_iface *iface;
struct audit_buffer *audit_buf = NULL;
struct lsmcontext context;
- struct lsmblob blob;
if (addr_len != sizeof(struct in_addr) &&
addr_len != sizeof(struct in6_addr))
@@ -407,7 +406,7 @@ int netlbl_unlhsh_add(struct net *net,
const struct in_addr *addr4 = addr;
const struct in_addr *mask4 = mask;
- ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, secid);
+ ret_val = netlbl_unlhsh_add_addr4(iface, addr4, mask4, lsmblob);
if (audit_buf != NULL)
netlbl_af4list_audit_addr(audit_buf, 1,
dev_name,
@@ -420,7 +419,7 @@ int netlbl_unlhsh_add(struct net *net,
const struct in6_addr *addr6 = addr;
const struct in6_addr *mask6 = mask;
- ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, secid);
+ ret_val = netlbl_unlhsh_add_addr6(iface, addr6, mask6, lsmblob);
if (audit_buf != NULL)
netlbl_af6list_audit_addr(audit_buf, 1,
dev_name,
@@ -437,8 +436,7 @@ int netlbl_unlhsh_add(struct net *net,
unlhsh_add_return:
rcu_read_unlock();
if (audit_buf != NULL) {
- lsmblob_init(&blob, secid);
- if (security_secid_to_secctx(&blob, &context) == 0) {
+ if (security_secid_to_secctx(lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -473,7 +471,6 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
struct audit_buffer *audit_buf;
struct net_device *dev;
struct lsmcontext context;
- struct lsmblob blob;
spin_lock(&netlbl_unlhsh_lock);
list_entry = netlbl_af4list_remove(addr->s_addr, mask->s_addr,
@@ -493,10 +490,8 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
addr->s_addr, mask->s_addr);
if (dev != NULL)
dev_put(dev);
- if (entry != NULL)
- lsmblob_init(&blob, entry->secid);
if (entry != NULL &&
- security_secid_to_secctx(&blob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -537,7 +532,6 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
struct audit_buffer *audit_buf;
struct net_device *dev;
struct lsmcontext context;
- struct lsmblob blob;
spin_lock(&netlbl_unlhsh_lock);
list_entry = netlbl_af6list_remove(addr, mask, &iface->addr6_list);
@@ -556,10 +550,8 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
addr, mask);
if (dev != NULL)
dev_put(dev);
- if (entry != NULL)
- lsmblob_init(&blob, entry->secid);
if (entry != NULL &&
- security_secid_to_secctx(&blob, &context) == 0) {
+ security_secid_to_secctx(&entry->lsmblob, &context) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -913,9 +905,8 @@ static int netlbl_unlabel_staticadd(struct sk_buff *skb,
if (ret_val != 0)
return ret_val;
- /* scaffolding with the [0] */
return netlbl_unlhsh_add(&init_net,
- dev_name, addr, mask, addr_len, blob.secid[0],
+ dev_name, addr, mask, addr_len, &blob,
&audit_info);
}
@@ -963,10 +954,8 @@ static int netlbl_unlabel_staticadddef(struct sk_buff *skb,
if (ret_val != 0)
return ret_val;
- /* scaffolding with the [0] */
return netlbl_unlhsh_add(&init_net,
- NULL, addr, mask, addr_len, blob.secid[0],
- &audit_info);
+ NULL, addr, mask, addr_len, &blob, &audit_info);
}
/**
@@ -1078,8 +1067,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
struct net_device *dev;
struct lsmcontext context;
void *data;
- u32 secid;
- struct lsmblob blob;
+ struct lsmblob *lsmb;
data = genlmsg_put(cb_arg->skb, NETLINK_CB(cb_arg->nl_cb->skb).portid,
cb_arg->seq, &netlbl_unlabel_gnl_family,
@@ -1117,7 +1105,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
if (ret_val != 0)
goto list_cb_failure;
- secid = addr4->secid;
+ lsmb = (struct lsmblob *)&addr4->lsmblob;
} else {
ret_val = nla_put_in6_addr(cb_arg->skb,
NLBL_UNLABEL_A_IPV6ADDR,
@@ -1131,11 +1119,10 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
if (ret_val != 0)
goto list_cb_failure;
- secid = addr6->secid;
+ lsmb = (struct lsmblob *)&addr6->lsmblob;
}
- lsmblob_init(&blob, secid);
- ret_val = security_secid_to_secctx(&blob, &context);
+ ret_val = security_secid_to_secctx(lsmb, &context);
if (ret_val != 0)
goto list_cb_failure;
ret_val = nla_put(cb_arg->skb,
@@ -1487,7 +1474,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
&iface->addr4_list);
if (addr4 == NULL)
goto unlabel_getattr_nolabel;
- secattr->attr.secid = netlbl_unlhsh_addr4_entry(addr4)->secid;
+ secattr->attr.lsmblob = netlbl_unlhsh_addr4_entry(addr4)->lsmblob;
break;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -1500,7 +1487,7 @@ int netlbl_unlabel_getattr(const struct sk_buff *skb,
&iface->addr6_list);
if (addr6 == NULL)
goto unlabel_getattr_nolabel;
- secattr->attr.secid = netlbl_unlhsh_addr6_entry(addr6)->secid;
+ secattr->attr.lsmblob = netlbl_unlhsh_addr6_entry(addr6)->lsmblob;
break;
}
#endif /* IPv6 */
diff --git a/net/netlabel/netlabel_unlabeled.h b/net/netlabel/netlabel_unlabeled.h
index 058e3a285d56..168920780994 100644
--- a/net/netlabel/netlabel_unlabeled.h
+++ b/net/netlabel/netlabel_unlabeled.h
@@ -211,7 +211,7 @@ int netlbl_unlhsh_add(struct net *net,
const void *addr,
const void *mask,
u32 addr_len,
- u32 secid,
+ struct lsmblob *lsmblob,
struct netlbl_audit *audit_info);
int netlbl_unlhsh_remove(struct net *net,
const char *dev_name,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5f50dae7c107..16348270b98e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6802,7 +6802,7 @@ 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 = {
+struct lsm_id selinux_lsmid __lsm_ro_after_init = {
.lsm = "selinux",
.slot = LSMBLOB_NEEDED
};
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index 111121281c47..25ca805d74a2 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -69,6 +69,7 @@
struct netlbl_lsm_secattr;
extern int selinux_enabled;
+extern struct lsm_id selinux_lsmid;
/* Policy capabilities */
enum {
diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 6a94b31b5472..d8d7603ab14e 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -108,7 +108,7 @@ static struct netlbl_lsm_secattr *selinux_netlbl_sock_getattr(
return NULL;
if ((secattr->flags & NETLBL_SECATTR_SECID) &&
- (secattr->attr.secid == sid))
+ (secattr->attr.lsmblob.secid[selinux_lsmid.slot] == sid))
return secattr;
return NULL;
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index a5813c7629c1..2b7680903b6b 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -3599,7 +3599,7 @@ int security_netlbl_secattr_to_sid(struct selinux_state *state,
if (secattr->flags & NETLBL_SECATTR_CACHE)
*sid = *(u32 *)secattr->cache->data;
else if (secattr->flags & NETLBL_SECATTR_SECID)
- *sid = secattr->attr.secid;
+ *sid = secattr->attr.lsmblob.secid[selinux_lsmid.slot];
else if (secattr->flags & NETLBL_SECATTR_MLS_LVL) {
rc = -EIDRM;
ctx = sidtab_search(sidtab, SECINITSID_NETMSG);
@@ -3672,7 +3672,7 @@ int security_netlbl_sid_to_secattr(struct selinux_state *state,
if (secattr->domain == NULL)
goto out;
- secattr->attr.secid = sid;
+ secattr->attr.lsmblob.secid[selinux_lsmid.slot] = sid;
secattr->flags |= NETLBL_SECATTR_DOMAIN_CPY | NETLBL_SECATTR_SECID;
mls_export_netlbl_lvl(policydb, ctx, secattr);
rc = mls_export_netlbl_cat(policydb, ctx, secattr);
diff --git a/security/smack/smack.h b/security/smack/smack.h
index 2836540f9577..6e76b6b33063 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -316,6 +316,7 @@ void smk_destroy_label_list(struct list_head *list);
* Shared data.
*/
extern int smack_enabled;
+extern struct lsm_id smack_lsmid;
extern int smack_cipso_direct;
extern int smack_cipso_mapped;
extern struct smack_known *smack_net_ambient;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 4e464e5e942e..e23792dae35c 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3776,7 +3776,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
/*
* Looks like a fallback, which gives us a secid.
*/
- return smack_from_secid(sap->attr.secid);
+ return smack_from_secid(
+ sap->attr.lsmblob.secid[smack_lsmid.slot]);
/*
* Without guidance regarding the smack value
* for the packet fall back on the network
@@ -4598,7 +4599,7 @@ 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 = {
+struct lsm_id smack_lsmid __lsm_ro_after_init = {
.lsm = "smack",
.slot = LSMBLOB_NEEDED
};
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index e3e05c04dbd1..d10e9c96717e 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -1122,6 +1122,7 @@ static void smk_net4addr_insert(struct smk_net4addr *new)
static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
+ struct lsmblob lsmblob;
struct smk_net4addr *snp;
struct sockaddr_in newname;
char *smack;
@@ -1253,10 +1254,13 @@ static ssize_t smk_write_net4addr(struct file *file, const char __user *buf,
* this host so that incoming packets get labeled.
* but only if we didn't get the special CIPSO option
*/
- if (rc == 0 && skp != NULL)
+ if (rc == 0 && skp != NULL) {
+ lsmblob_init(&lsmblob, 0);
+ lsmblob.secid[smack_lsmid.slot] = snp->smk_label->smk_secid;
rc = netlbl_cfg_unlbl_static_add(&init_net, NULL,
- &snp->smk_host, &snp->smk_mask, PF_INET,
- snp->smk_label->smk_secid, &audit_info);
+ &snp->smk_host, &snp->smk_mask, PF_INET, &lsmblob,
+ &audit_info);
+ }
if (rc == 0)
rc = count;
--
2.20.1
^ permalink raw reply related
* [PATCH v11 23/25] NET: Add SO_PEERCONTEXT for multiple LSMs
From: Casey Schaufler @ 2019-11-13 18:25 UTC (permalink / raw)
To: casey.schaufler, jmorris, linux-security-module, selinux
Cc: keescook, john.johansen, penguin-kernel, paul, sds
In-Reply-To: <20191113182506.2580-1-casey@schaufler-ca.com>
The getsockopt SO_PEERSEC provides the LSM based security
information for a single module, but for reasons of backward
compatibility cannot include the information for multiple
modules. A new option SO_PEERCONTEXT is added to report the
security "context" of multiple modules using a "compound" format
lsm1\0value\0lsm2\0value\0
This is expected to be used by system services, including dbus-daemon.
The exact format of a compound context has been the subject of
considerable debate. This format was suggested by Simon McVittie,
a dbus maintainer with a significant stake in the format being
usable.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
cc: netdev@vger.kernel.org
---
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 +
include/linux/lsm_hooks.h | 9 +-
include/linux/security.h | 11 ++-
include/uapi/asm-generic/socket.h | 1 +
kernel/audit.c | 4 +-
net/core/sock.c | 7 +-
net/netlabel/netlabel_unlabeled.c | 9 +-
net/netlabel/netlabel_user.c | 2 +-
security/apparmor/lsm.c | 20 ++---
security/security.c | 118 +++++++++++++++++++++++---
security/selinux/hooks.c | 20 ++---
security/smack/smack_lsm.c | 31 +++----
15 files changed, 164 insertions(+), 72 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index de6c4df61082..b26fb34e4226 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -123,6 +123,7 @@
#define SO_SNDTIMEO_NEW 67
#define SO_DETACH_REUSEPORT_BPF 68
+#define SO_PEERCONTEXT 69
#if !defined(__KERNEL__)
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index d0a9ed2ca2d6..10e03507b1ed 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -134,6 +134,7 @@
#define SO_SNDTIMEO_NEW 67
#define SO_DETACH_REUSEPORT_BPF 68
+#define SO_PEERCONTEXT 69
#if !defined(__KERNEL__)
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 10173c32195e..e11df59a84d1 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -115,6 +115,7 @@
#define SO_SNDTIMEO_NEW 0x4041
#define SO_DETACH_REUSEPORT_BPF 0x4042
+#define SO_PEERCONTEXT 0x4043
#if !defined(__KERNEL__)
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 8029b681fc7c..5b41ef778040 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -116,6 +116,7 @@
#define SO_SNDTIMEO_NEW 0x0045
#define SO_DETACH_REUSEPORT_BPF 0x0047
+#define SO_PEERCONTEXT 0x0048
#if !defined(__KERNEL__)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index b2ec81fcd1e2..6740bc713f12 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -880,8 +880,8 @@
* SO_GETPEERSEC. For tcp sockets this can be meaningful if the
* socket is associated with an ipsec SA.
* @sock is the local socket.
- * @optval userspace memory where the security state is to be copied.
- * @optlen userspace int where the module should copy the actual length
+ * @optval memory where the security state is to be copied.
+ * @optlen int where the module should copy the actual length
* of the security state.
* @len as input is the maximum length to copy to userspace provided
* by the caller.
@@ -1724,9 +1724,8 @@ union security_list_options {
int (*socket_setsockopt)(struct socket *sock, int level, int optname);
int (*socket_shutdown)(struct socket *sock, int how);
int (*socket_sock_rcv_skb)(struct sock *sk, struct sk_buff *skb);
- int (*socket_getpeersec_stream)(struct socket *sock,
- char __user *optval,
- int __user *optlen, unsigned len);
+ int (*socket_getpeersec_stream)(struct socket *sock, char **optval,
+ int *optlen, unsigned len);
int (*socket_getpeersec_dgram)(struct socket *sock,
struct sk_buff *skb, u32 *secid);
int (*sk_alloc_security)(struct sock *sk, int family, gfp_t priority);
diff --git a/include/linux/security.h b/include/linux/security.h
index 79f5177a6b52..55bcb4ed8a21 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -179,7 +179,7 @@ struct lsmblob {
#define LSMBLOB_NEEDED -2 /* Slot requested on initialization */
#define LSMBLOB_NOT_NEEDED -3 /* Slot not requested */
#define LSMBLOB_DISPLAY -4 /* Use the "display" slot */
-#define LSMBLOB_FIRST -5 /* Use the default "display" slot */
+#define LSMBLOB_COMPOUND -5 /* A compound "display" */
/**
* lsmblob_init - initialize an lsmblob structure.
@@ -1398,7 +1398,8 @@ int security_socket_setsockopt(struct socket *sock, int level, int optname);
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 __user *optlen, unsigned len,
+ int display);
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);
@@ -1532,8 +1533,10 @@ static inline int security_sock_rcv_skb(struct sock *sk,
return 0;
}
-static inline int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
- int __user *optlen, unsigned len)
+static inline int security_socket_getpeersec_stream(struct socket *sock,
+ char __user *optval,
+ int __user *optlen,
+ unsigned len, int display)
{
return -ENOPROTOOPT;
}
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 77f7c1638eb1..e3a853d53705 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -118,6 +118,7 @@
#define SO_SNDTIMEO_NEW 67
#define SO_DETACH_REUSEPORT_BPF 68
+#define SO_PEERCONTEXT 69
#if !defined(__KERNEL__)
diff --git a/kernel/audit.c b/kernel/audit.c
index 77e5d54a3e30..f75db95e6a9e 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1424,7 +1424,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
len = 0;
if (lsmblob_is_set(&audit_sig_lsm)) {
err = security_secid_to_secctx(&audit_sig_lsm,
- &context, LSMBLOB_FIRST);
+ &context, 0);
if (err)
return err;
}
@@ -2099,7 +2099,7 @@ int audit_log_task_context(struct audit_buffer *ab)
if (!lsmblob_is_set(&blob))
return 0;
- error = security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST);
+ error = security_secid_to_secctx(&blob, &context, 0);
if (error) {
if (error != -EINVAL)
goto error_path;
diff --git a/net/core/sock.c b/net/core/sock.c
index ac78a570e43a..7a1c41a79b0b 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1413,7 +1413,12 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
break;
case SO_PEERSEC:
- return security_socket_getpeersec_stream(sock, optval, optlen, len);
+ return security_socket_getpeersec_stream(sock, optval, optlen,
+ len, LSMBLOB_DISPLAY);
+
+ case SO_PEERCONTEXT:
+ return security_socket_getpeersec_stream(sock, optval, optlen,
+ len, LSMBLOB_COMPOUND);
case SO_MARK:
v.val = sk->sk_mark;
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 60a7665de0e3..fefd1f2d26f8 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -436,8 +436,7 @@ int netlbl_unlhsh_add(struct net *net,
unlhsh_add_return:
rcu_read_unlock();
if (audit_buf != NULL) {
- if (security_secid_to_secctx(lsmblob, &context,
- LSMBLOB_FIRST) == 0) {
+ if (security_secid_to_secctx(lsmblob, &context, 0) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -493,7 +492,7 @@ static int netlbl_unlhsh_remove_addr4(struct net *net,
dev_put(dev);
if (entry != NULL &&
security_secid_to_secctx(&entry->lsmblob, &context,
- LSMBLOB_FIRST) == 0) {
+ 0) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -554,7 +553,7 @@ static int netlbl_unlhsh_remove_addr6(struct net *net,
dev_put(dev);
if (entry != NULL &&
security_secid_to_secctx(&entry->lsmblob, &context,
- LSMBLOB_FIRST) == 0) {
+ 0) == 0) {
audit_log_format(audit_buf, " sec_obj=%s",
context.context);
security_release_secctx(&context);
@@ -1125,7 +1124,7 @@ static int netlbl_unlabel_staticlist_gen(u32 cmd,
lsmb = (struct lsmblob *)&addr6->lsmblob;
}
- ret_val = security_secid_to_secctx(lsmb, &context, LSMBLOB_FIRST);
+ ret_val = security_secid_to_secctx(lsmb, &context, 0);
if (ret_val != 0)
goto list_cb_failure;
ret_val = nla_put(cb_arg->skb,
diff --git a/net/netlabel/netlabel_user.c b/net/netlabel/netlabel_user.c
index 1941877fd16f..537c0bf25e3c 100644
--- a/net/netlabel/netlabel_user.c
+++ b/net/netlabel/netlabel_user.c
@@ -100,7 +100,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
lsmblob_init(&blob, audit_info->secid);
if (audit_info->secid != 0 &&
- security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST) == 0) {
+ security_secid_to_secctx(&blob, &context, 0) == 0) {
audit_log_format(audit_buf, " subj=%s", context.context);
security_release_secctx(&context);
}
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index fefccd559541..b4c964fdc2f9 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1065,10 +1065,8 @@ static struct aa_label *sk_peer_label(struct sock *sk)
*
* Note: for tcp only valid if using ipsec or cipso on lan
*/
-static int apparmor_socket_getpeersec_stream(struct socket *sock,
- char __user *optval,
- int __user *optlen,
- unsigned int len)
+static int apparmor_socket_getpeersec_stream(struct socket *sock, char **optval,
+ int *optlen, unsigned int len)
{
char *name;
int slen, error = 0;
@@ -1088,17 +1086,11 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock,
if (slen < 0) {
error = -ENOMEM;
} else {
- if (slen > len) {
+ if (slen > len)
error = -ERANGE;
- } else if (copy_to_user(optval, name, slen)) {
- error = -EFAULT;
- goto out;
- }
- if (put_user(slen, optlen))
- error = -EFAULT;
-out:
- kfree(name);
-
+ else
+ *optval = name;
+ *optlen = slen;
}
done:
diff --git a/security/security.c b/security/security.c
index 0dce15d74cb5..f1fefa187ef8 100644
--- a/security/security.c
+++ b/security/security.c
@@ -723,6 +723,42 @@ static void __init lsm_early_task(struct task_struct *task)
panic("%s: Early task alloc failed.\n", __func__);
}
+/**
+ * append_ctx - append a lsm/context pair to a compound context
+ * @ctx: the existing compound context
+ * @ctxlen: size of the old context, including terminating nul byte
+ * @lsm: new lsm name, nul terminated
+ * @new: new context, possibly nul terminated
+ * @newlen: maximum size of @new
+ *
+ * replace @ctx with a new compound context, appending @newlsm and @new
+ * to @ctx. On exit the new data replaces the old, which is freed.
+ * @ctxlen is set to the new size, which includes a trailing nul byte.
+ *
+ * Returns 0 on success, -ENOMEM if no memory is available.
+ */
+static int append_ctx(char **ctx, int *ctxlen, const char *lsm, char *new,
+ int newlen)
+{
+ char *final;
+ int llen;
+
+ llen = strlen(lsm) + 1;
+ newlen = strnlen(new, newlen) + 1;
+
+ final = kzalloc(*ctxlen + llen + newlen, GFP_KERNEL);
+ if (final == NULL)
+ return -ENOMEM;
+ if (*ctxlen)
+ memcpy(final, *ctx, *ctxlen);
+ memcpy(final + *ctxlen, lsm, llen);
+ memcpy(final + *ctxlen + llen, new, newlen);
+ kfree(*ctx);
+ *ctx = final;
+ *ctxlen = *ctxlen + llen + newlen;
+ return 0;
+}
+
/*
* Hook list operation macros.
*
@@ -2164,8 +2200,8 @@ int security_setprocattr(const char *lsm, const char *name, void *value,
hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
if (lsm != NULL && strcmp(lsm, hp->lsmid->lsm))
continue;
- if (lsm == NULL && *display != LSMBLOB_INVALID &&
- *display != hp->lsmid->slot)
+ if (lsm == NULL && display != NULL &&
+ *display != LSMBLOB_INVALID && *display != hp->lsmid->slot)
continue;
return hp->hook.setprocattr(name, value, size);
}
@@ -2196,7 +2232,7 @@ int security_secid_to_secctx(struct lsmblob *blob, struct lsmcontext *cp,
*/
if (display == LSMBLOB_DISPLAY)
display = lsm_task_display(current);
- else if (display == LSMBLOB_FIRST)
+ else if (display == 0)
display = LSMBLOB_INVALID;
else if (display < 0) {
WARN_ONCE(true,
@@ -2246,6 +2282,15 @@ void security_release_secctx(struct lsmcontext *cp)
struct security_hook_list *hp;
bool found = false;
+ if (cp->slot == LSMBLOB_INVALID)
+ return;
+
+ if (cp->slot == LSMBLOB_COMPOUND) {
+ kfree(cp->context);
+ found = true;
+ goto clear_out;
+ }
+
hlist_for_each_entry(hp, &security_hook_heads.release_secctx, list)
if (cp->slot == hp->lsmid->slot) {
hp->hook.release_secctx(cp->context, cp->len);
@@ -2253,6 +2298,7 @@ void security_release_secctx(struct lsmcontext *cp)
break;
}
+clear_out:
memset(cp, 0, sizeof(*cp));
if (!found)
@@ -2389,17 +2435,67 @@ int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
EXPORT_SYMBOL(security_sock_rcv_skb);
int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
- int __user *optlen, unsigned len)
+ int __user *optlen, unsigned len,
+ int display)
{
- int display = lsm_task_display(current);
struct security_hook_list *hp;
+ char *final = NULL;
+ char *cp;
+ int rc = 0;
+ unsigned finallen = 0;
+ unsigned clen = 0;
- hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
- list)
- if (display == LSMBLOB_INVALID || display == hp->lsmid->slot)
- return hp->hook.socket_getpeersec_stream(sock, optval,
- optlen, len);
- return -ENOPROTOOPT;
+ switch (display) {
+ case LSMBLOB_DISPLAY:
+ rc = -ENOPROTOOPT;
+ display = lsm_task_display(current);
+ hlist_for_each_entry(hp,
+ &security_hook_heads.socket_getpeersec_stream,
+ list)
+ if (display == LSMBLOB_INVALID ||
+ display == hp->lsmid->slot) {
+ rc = hp->hook.socket_getpeersec_stream(sock,
+ &final, &finallen, len);
+ break;
+ }
+ break;
+ case LSMBLOB_COMPOUND:
+ /*
+ * A compound context, in the form [lsm\0value\0]...
+ */
+ hlist_for_each_entry(hp,
+ &security_hook_heads.socket_getpeersec_stream,
+ list) {
+ rc = hp->hook.socket_getpeersec_stream(sock, &cp, &clen,
+ len);
+ if (rc == -EINVAL || rc == -ENOPROTOOPT) {
+ rc = 0;
+ continue;
+ }
+ if (rc) {
+ kfree(final);
+ return rc;
+ }
+ rc = append_ctx(&final, &finallen, hp->lsmid->lsm,
+ cp, clen);
+ }
+ if (final == NULL)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (finallen > len)
+ rc = -ERANGE;
+ else if (copy_to_user(optval, final, finallen))
+ rc = -EFAULT;
+
+ if (put_user(finallen, optlen))
+ rc = -EFAULT;
+
+ kfree(final);
+ return rc;
}
int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb,
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 16348270b98e..ca7b32631636 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5048,10 +5048,8 @@ 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 int len)
+static int selinux_socket_getpeersec_stream(struct socket *sock, char **optval,
+ int *optlen, unsigned int len)
{
int err = 0;
char *scontext;
@@ -5071,18 +5069,12 @@ static int selinux_socket_getpeersec_stream(struct socket *sock,
if (err)
return err;
- if (scontext_len > len) {
+ if (scontext_len > len)
err = -ERANGE;
- goto out_len;
- }
-
- if (copy_to_user(optval, scontext, scontext_len))
- err = -EFAULT;
+ else
+ *optval = scontext;
-out_len:
- if (put_user(scontext_len, optlen))
- err = -EFAULT;
- kfree(scontext);
+ *optlen = scontext_len;
return err;
}
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e23792dae35c..d6983fb67d31 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3958,28 +3958,29 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
*
* returns zero on success, an error code otherwise
*/
-static int smack_socket_getpeersec_stream(struct socket *sock,
- char __user *optval,
- int __user *optlen, unsigned len)
+static int smack_socket_getpeersec_stream(struct socket *sock, char **optval,
+ int *optlen, unsigned len)
{
- struct socket_smack *ssp;
- char *rcp = "";
- int slen = 1;
+ struct socket_smack *ssp = smack_sock(sock->sk);
+ char *rcp;
+ int slen;
int rc = 0;
- ssp = smack_sock(sock->sk);
- if (ssp->smk_packet != NULL) {
- rcp = ssp->smk_packet->smk_known;
- slen = strlen(rcp) + 1;
+ if (ssp->smk_packet == NULL) {
+ *optlen = 0;
+ return -EINVAL;
}
+ rcp = ssp->smk_packet->smk_known;
+ slen = strlen(rcp) + 1;
if (slen > len)
rc = -ERANGE;
- else if (copy_to_user(optval, rcp, slen) != 0)
- rc = -EFAULT;
-
- if (put_user(slen, optlen) != 0)
- rc = -EFAULT;
+ else {
+ *optval = kstrdup(rcp, GFP_KERNEL);
+ if (*optval == NULL)
+ rc = -ENOMEM;
+ }
+ *optlen = slen;
return rc;
}
--
2.20.1
^ permalink raw reply related
* [PATCH v6 3/3] KEYS: Call the IMA hook to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191113184658.2862-1-nramas@linux.microsoft.com>
Call the IMA hook from key_create_or_update function to measure
the key when a new key is created or an existing key is updated.
This patch adds the call to the IMA hook from key_create_or_update
function to measure the key on key create or update.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
include/linux/ima.h | 13 +++++++++++++
security/keys/key.c | 9 +++++++++
2 files changed, 22 insertions(+)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6d904754d858..ec5afe319ab7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -25,6 +25,12 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
extern void ima_post_path_mknod(struct dentry *dentry);
extern void ima_kexec_cmdline(const void *buf, int size);
+#ifdef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
+extern void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags, bool create);
+#endif
+
#ifdef CONFIG_IMA_KEXEC
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
@@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
{}
#endif
+#ifndef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
+static inline void ima_post_key_create_or_update(struct key *keyring,
+ struct key *key,
+ unsigned long flags,
+ bool create) {}
+#endif
+
#ifdef CONFIG_IMA_APPRAISE
extern bool is_ima_appraise_enabled(void);
extern void ima_inode_post_setattr(struct dentry *dentry);
diff --git a/security/keys/key.c b/security/keys/key.c
index 764f4c57913e..9782d4d046fd 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -13,6 +13,7 @@
#include <linux/security.h>
#include <linux/workqueue.h>
#include <linux/random.h>
+#include <linux/ima.h>
#include <linux/err.h>
#include "internal.h"
@@ -936,6 +937,9 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_link_end;
}
+ /* let the ima module know about the created key. */
+ ima_post_key_create_or_update(keyring, key, flags, true);
+
key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
error_link_end:
@@ -965,6 +969,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
}
key_ref = __key_update(key_ref, &prep);
+
+ /* let the ima module know about the updated key. */
+ if (!IS_ERR(key_ref))
+ ima_post_key_create_or_update(keyring, key, flags, false);
+
goto error_free_prep;
}
EXPORT_SYMBOL(key_create_or_update);
--
2.17.1
^ permalink raw reply related
* [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
Keys created or updated in the system are currently not 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:
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.
A new IMA policy function KEY_CHECK has been added to measure keys.
# measure keys loaded onto any keyring
measure func=KEY_CHECK
Testing performed:
* Booted the kernel with this change.
* When KEY_CHECK policy is set IMA measures keys loaded
onto any keyring.
* Keys are not measured when KEY_CHECK is not set.
* Added a new key to a keyring.
=> Added keys to .ima and .evm keyrings.
* Added the same key again.
=> Add the same key to .ima and .evm keyrings.
Change Log:
v6:
=> Rebased the changes to v5.4-rc7
=> Renamed KEYRING_CHECK to KEY_CHECK per Mimi's suggestion.
=> Excluded the patches that add support for limiting key
measurement to specific keyrings ("keyrings=" option
for "measure func=KEY_CHECK" in the IMA policy).
Also, excluded the patches that add support for deferred
processing of keys (queue support).
These patches will be added in separate patch sets later.
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.
Lakshmi Ramasubramanian (3):
IMA: Add KEY_CHECK func to measure keys
IMA: Define an IMA hook to measure keys
KEYS: Call the IMA hook to measure keys
Documentation/ABI/testing/ima_policy | 6 ++-
include/linux/ima.h | 13 +++++
security/integrity/ima/Kconfig | 14 ++++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
security/integrity/ima/ima_main.c | 7 +++
security/integrity/ima/ima_policy.c | 4 +-
security/keys/key.c | 9 ++++
9 files changed, 104 insertions(+), 2 deletions(-)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
--
2.17.1
^ permalink raw reply
* [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191113184658.2862-1-nramas@linux.microsoft.com>
Measure keys loaded onto any keyring.
This patch defines a new IMA policy func namely KEY_CHECK to
measure keys. Updated ima_match_rules() to check for KEY_CHECK
and ima_parse_rule() to handle 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_main.c | 7 +++++++
security/integrity/ima/ima_policy.c | 4 +++-
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 29aaedf33246..066d32797500 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,7 +29,7 @@ Description:
base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
- [KEXEC_CMDLINE]
+ [KEXEC_CMDLINE] [KEY_CHECK]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
@@ -113,3 +113,7 @@ Description:
Example of appraise rule allowing modsig appended signatures:
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
+
+ Example of measure rule using KEY_CHECK to measure all keys:
+
+ measure func=KEY_CHECK
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df4ca482fb53..fe6c698617bd 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(KEY_CHECK) \
hook(MAX_CHECK)
#define __ima_hook_enumify(ENUM) ENUM,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..12684e8d7124 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
int action = 0;
u32 secid;
+ /*
+ * If IMA is not yet initialized or IMA policy is empty
+ * then there is no need to measure.
+ */
+ if (!ima_policy_flag)
+ return;
+
/*
* Both LSM hooks and auxilary based buffer measurements are
* based on policy. To avoid code duplication, differentiate
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index f19a895ad7cd..1525a28fd705 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 == KEY_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, "KEY_CHECK") == 0)
+ entry->func = KEY_CHECK;
else
result = -EINVAL;
if (!result)
--
2.17.1
^ permalink raw reply related
* [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 18:46 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191113184658.2862-1-nramas@linux.microsoft.com>
Measure asymmetric keys used for verifying file signatures,
certificates, etc.
This patch defines a new IMA hook namely ima_post_key_create_or_update()
to measure asymmetric keys.
Note that currently IMA subsystem can be enabled without
enabling KEYS subsystem.
Adding support for measuring asymmetric keys in IMA requires KEYS
subsystem to be enabled. To handle this dependency a new config
namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS has been added. Enabling
this config requires the following configs to be enabled:
CONFIG_IMA, CONFIG_KEYS, CONFIG_ASYMMETRIC_KEY_TYPE, and
CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE.
CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is off by default.
The IMA hook is defined in a new file namely ima_asymmetric_keys.c
which is built only if CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is enabled.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/Kconfig | 14 ++++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
3 files changed, 66 insertions(+)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 838476d780e5..c6d14884bc19 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -310,3 +310,17 @@ config IMA_APPRAISE_SIGNED_INIT
default n
help
This option requires user-space init to be signed.
+
+config IMA_MEASURE_ASYMMETRIC_KEYS
+ bool "Enable measuring asymmetric keys on key create or update"
+ depends on IMA
+ depends on KEYS
+ depends on ASYMMETRIC_KEY_TYPE
+ depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+ default n
+ help
+ This option enables measuring asymmetric keys when
+ the key is created or updated. Additionally, IMA policy
+ needs to be configured to either measure keys linked to
+ any keyring or only measure keys linked to the keyrings
+ specified in the IMA policy through the keyrings= option.
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 31d57cdf2421..3e9d0ad68c7b 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
+obj-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
new file mode 100644
index 000000000000..f6884641a622
--- /dev/null
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Microsoft Corporation
+ *
+ * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com)
+ *
+ * File: ima_asymmetric_keys.c
+ * Defines an IMA hook to measure asymmetric keys on key
+ * create or update.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
+#include "ima.h"
+
+/**
+ * ima_post_key_create_or_update - measure asymmetric keys
+ * @keyring: keyring to which the key is linked to
+ * @key: created or updated key
+ * @flags: key flags
+ * @create: flag indicating whether the key was created or updated
+ *
+ * Keys can only be measured, not appraised.
+ */
+void ima_post_key_create_or_update(struct key *keyring, struct key *key,
+ unsigned long flags, bool create)
+{
+ const struct public_key *pk;
+
+ /* Only asymmetric keys are handled by this hook. */
+ if (key->type != &key_type_asymmetric)
+ return;
+
+ /* Get the public_key of the given asymmetric key to measure. */
+ pk = key->payload.data[asym_crypto];
+
+ /*
+ * keyring->description points to the name of the keyring
+ * (such as ".builtin_trusted_keys", ".ima", etc.) to
+ * which the given key is linked to.
+ *
+ * The name of the keyring is passed in the "eventname"
+ * parameter to process_buffer_measurement() and is set
+ * in the "eventname" field in ima_event_data for
+ * the key measurement IMA event.
+ */
+ process_buffer_measurement(pk->key, pk->keylen,
+ keyring->description, KEY_CHECK, 0);
+}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
From: Mimi Zohar @ 2019-11-13 20:09 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191113184658.2862-3-nramas@linux.microsoft.com>
On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> Measure asymmetric keys used for verifying file signatures,
> certificates, etc.
>
> This patch defines a new IMA hook namely ima_post_key_create_or_update()
> to measure asymmetric keys.
>
> Note that currently IMA subsystem can be enabled without
> enabling KEYS subsystem.
>
> Adding support for measuring asymmetric keys in IMA requires KEYS
> subsystem to be enabled. To handle this dependency a new config
> namely CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS has been added. Enabling
> this config requires the following configs to be enabled:
> CONFIG_IMA, CONFIG_KEYS, CONFIG_ASYMMETRIC_KEY_TYPE, and
> CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE.
>
> CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is off by default.
>
> The IMA hook is defined in a new file namely ima_asymmetric_keys.c
> which is built only if CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS is enabled.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
All that is is needed is the key and public_key structures, which are
defined in include/linux/keys.h and include/crypto/public_key.h. If
the keys subsystem is disabled, then the new IMA hook won't be called.
There's no need for a new Kconfig option or a new file.
Please move the hook to just after ima_kexec_cmdline().
Mimi
> ---
> security/integrity/ima/Kconfig | 14 ++++++
> security/integrity/ima/Makefile | 1 +
> security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
> 3 files changed, 66 insertions(+)
> create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 838476d780e5..c6d14884bc19 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -310,3 +310,17 @@ config IMA_APPRAISE_SIGNED_INIT
> default n
> help
> This option requires user-space init to be signed.
> +
> +config IMA_MEASURE_ASYMMETRIC_KEYS
> + bool "Enable measuring asymmetric keys on key create or update"
> + depends on IMA
> + depends on KEYS
> + depends on ASYMMETRIC_KEY_TYPE
> + depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
> + default n
> + help
> + This option enables measuring asymmetric keys when
> + the key is created or updated. Additionally, IMA policy
> + needs to be configured to either measure keys linked to
> + any keyring or only measure keys linked to the keyrings
> + specified in the IMA policy through the keyrings= option.
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index 31d57cdf2421..3e9d0ad68c7b 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
> ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
> obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> +obj-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
> diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
> new file mode 100644
> index 000000000000..f6884641a622
> --- /dev/null
> +++ b/security/integrity/ima/ima_asymmetric_keys.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019 Microsoft Corporation
> + *
> + * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com)
> + *
> + * File: ima_asymmetric_keys.c
> + * Defines an IMA hook to measure asymmetric keys on key
> + * create or update.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <crypto/public_key.h>
> +#include <keys/asymmetric-type.h>
> +#include "ima.h"
> +
> +/**
> + * ima_post_key_create_or_update - measure asymmetric keys
> + * @keyring: keyring to which the key is linked to
> + * @key: created or updated key
> + * @flags: key flags
> + * @create: flag indicating whether the key was created or updated
> + *
> + * Keys can only be measured, not appraised.
> + */
> +void ima_post_key_create_or_update(struct key *keyring, struct key *key,
> + unsigned long flags, bool create)
> +{
> + const struct public_key *pk;
> +
> + /* Only asymmetric keys are handled by this hook. */
> + if (key->type != &key_type_asymmetric)
> + return;
> +
> + /* Get the public_key of the given asymmetric key to measure. */
> + pk = key->payload.data[asym_crypto];
> +
> + /*
> + * keyring->description points to the name of the keyring
> + * (such as ".builtin_trusted_keys", ".ima", etc.) to
> + * which the given key is linked to.
> + *
> + * The name of the keyring is passed in the "eventname"
> + * parameter to process_buffer_measurement() and is set
> + * in the "eventname" field in ima_event_data for
> + * the key measurement IMA event.
> + */
> + process_buffer_measurement(pk->key, pk->keylen,
> + keyring->description, KEY_CHECK, 0);
> +}
^ permalink raw reply
* Re: [PATCH v6 3/3] KEYS: Call the IMA hook to measure keys
From: Mimi Zohar @ 2019-11-13 20:09 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191113184658.2862-4-nramas@linux.microsoft.com>
On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> Call the IMA hook from key_create_or_update function to measure
> the key when a new key is created or an existing key is updated.
>
> This patch adds the call to the IMA hook from key_create_or_update
> function to measure the key on key create or update.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> ---
> include/linux/ima.h | 13 +++++++++++++
> security/keys/key.c | 9 +++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..ec5afe319ab7 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -25,6 +25,12 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> extern void ima_post_path_mknod(struct dentry *dentry);
> extern void ima_kexec_cmdline(const void *buf, int size);
>
> +#ifdef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
> +extern void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags, bool create);
> +#endif
> +
> #ifdef CONFIG_IMA_KEXEC
> extern void ima_add_kexec_buffer(struct kimage *image);
> #endif
> @@ -101,6 +107,13 @@ static inline void ima_add_kexec_buffer(struct kimage *image)
> {}
> #endif
>
> +#ifndef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS
> +static inline void ima_post_key_create_or_update(struct key *keyring,
> + struct key *key,
> + unsigned long flags,
> + bool create) {}
> +#endif
> +
> #ifdef CONFIG_IMA_APPRAISE
> extern bool is_ima_appraise_enabled(void);
> extern void ima_inode_post_setattr(struct dentry *dentry);
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 764f4c57913e..9782d4d046fd 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -13,6 +13,7 @@
> #include <linux/security.h>
> #include <linux/workqueue.h>
> #include <linux/random.h>
> +#include <linux/ima.h>
> #include <linux/err.h>
> #include "internal.h"
>
> @@ -936,6 +937,9 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> goto error_link_end;
> }
>
> + /* let the ima module know about the created key. */
This comment and the one below doesn't provide any additional
information. Please remove them.
> + ima_post_key_create_or_update(keyring, key, flags, true);
> +
> key_ref = make_key_ref(key, is_key_possessed(keyring_ref));
>
> error_link_end:
> @@ -965,6 +969,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
> }
>
> key_ref = __key_update(key_ref, &prep);
> +
> + /* let the ima module know about the updated key. */
> + if (!IS_ERR(key_ref))
> + ima_post_key_create_or_update(keyring, key, flags, false);
> +
> goto error_free_prep;
> }
> EXPORT_SYMBOL(key_create_or_update);
^ permalink raw reply
* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
From: Mimi Zohar @ 2019-11-13 20:14 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191113184658.2862-2-nramas@linux.microsoft.com>
On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> Measure keys loaded onto any keyring.
>
> This patch defines a new IMA policy func namely KEY_CHECK to
> measure keys. Updated ima_match_rules() to check for KEY_CHECK
> and ima_parse_rule() to handle 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_main.c | 7 +++++++
> security/integrity/ima/ima_policy.c | 4 +++-
> 4 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 29aaedf33246..066d32797500 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -29,7 +29,7 @@ Description:
> base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> [FIRMWARE_CHECK]
> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> - [KEXEC_CMDLINE]
> + [KEXEC_CMDLINE] [KEY_CHECK]
> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
> [[^]MAY_EXEC]
> fsmagic:= hex value
> @@ -113,3 +113,7 @@ Description:
> Example of appraise rule allowing modsig appended signatures:
>
> appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
> +
> + Example of measure rule using KEY_CHECK to measure all keys:
> +
> + measure func=KEY_CHECK
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index df4ca482fb53..fe6c698617bd 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(KEY_CHECK) \
> hook(MAX_CHECK)
> #define __ima_hook_enumify(ENUM) ENUM,
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..12684e8d7124 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
> int action = 0;
> u32 secid;
>
> + /*
> + * If IMA is not yet initialized or IMA policy is empty
> + * then there is no need to measure.
> + */
> + if (!ima_policy_flag)
> + return;
> +
This addition has nothing to do with defining a new IMA hook and
should be a separate patch. This can be posted independently of this
patch set.
Mimi
> /*
> * Both LSM hooks and auxilary based buffer measurements are
> * based on policy. To avoid code duplication, differentiate
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index f19a895ad7cd..1525a28fd705 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 == KEY_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, "KEY_CHECK") == 0)
> + entry->func = KEY_CHECK;
> else
> result = -EINVAL;
> if (!result)
^ permalink raw reply
* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 20:21 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573676066.4843.18.camel@linux.ibm.com>
On 11/13/2019 12:14 PM, Mimi Zohar wrote:
>> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
>> int action = 0;
>> u32 secid;
>>
>> + /*
>> + * If IMA is not yet initialized or IMA policy is empty
>> + * then there is no need to measure.
>> + */
>> + if (!ima_policy_flag)
>> + return;
>> +
>
> This addition has nothing to do with defining a new IMA hook and
> should be a separate patch. This can be posted independently of this
> patch set.
>
> Mimi
I'll move this change to a different patch,
but it has to be either part of this patch set or the above change alone
needs to be taken before this patch set for the following reason:
The IMA hook gets called early in the boot process (for example, when
builtin_trusted_keys are added). If the above check is not there,
ima_get_action() is called and causes kernel panic (since IMA is not yet
initialized).
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH v6 1/3] IMA: Add KEY_CHECK func to measure keys
From: Mimi Zohar @ 2019-11-13 20:24 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <320826aa-f744-f2ae-8693-a6ce9461d886@linux.microsoft.com>
On Wed, 2019-11-13 at 12:21 -0800, Lakshmi Ramasubramanian wrote:
> On 11/13/2019 12:14 PM, Mimi Zohar wrote:
>
> >> @@ -655,6 +655,13 @@ void process_buffer_measurement(const void *buf, int size,
> >> int action = 0;
> >> u32 secid;
> >>
> >> + /*
> >> + * If IMA is not yet initialized or IMA policy is empty
> >> + * then there is no need to measure.
> >> + */
> >> + if (!ima_policy_flag)
> >> + return;
> >> +
> >
> > This addition has nothing to do with defining a new IMA hook and
> > should be a separate patch. This can be posted independently of this
> > patch set.
> >
> > Mimi
>
> I'll move this change to a different patch,
> but it has to be either part of this patch set or the above change alone
> needs to be taken before this patch set for the following reason:
>
> The IMA hook gets called early in the boot process (for example, when
> builtin_trusted_keys are added). If the above check is not there,
> ima_get_action() is called and causes kernel panic (since IMA is not yet
> initialized).
It will be upstreamed prior to this patch set.
Mimi
^ permalink raw reply
* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 20:52 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573675761.4843.13.camel@linux.ibm.com>
On 11/13/19 12:09 PM, Mimi Zohar wrote:
>
> All that is is needed is the key and public_key structures, which are
> defined in include/linux/keys.h and include/crypto/public_key.h. If
> the keys subsystem is disabled, then the new IMA hook won't be called.
> There's no need for a new Kconfig option or a new file.
>
> Please move the hook to just after ima_kexec_cmdline().
>
> Mimi
Yes - IMA hook won't be called when KEYS subsystem is disabled.
But, build dependency is breaking since "struct key" is not defined
without CONFIG_KEYS.
Sasha was able to craft a .config that enabled IMA without enabling KEYS
and found the build break.
Please see the build output he'd shared.
***********************************************************************
In file included from security/integrity/ima/ima.h:25,
from security/integrity/ima/ima_fs.c:26:
./include/keys/asymmetric-type.h: In function ‘asymmetric_key_ids’:
./include/keys/asymmetric-type.h:72:12: error: dereferencing pointer to
incomplete type ‘const struct key’
return key->payload.data[asym_key_ids];
^~
make[3]: *** [scripts/Makefile.build:266:
security/integrity/ima/ima_fs.o] Error 1
make[3]: *** Waiting for unfinished jobs....
In file included from security/integrity/ima/ima.h:25,
from security/integrity/ima/ima_queue.c:22:
./include/keys/asymmetric-type.h: In function ‘asymmetric_key_ids’:
./include/keys/asymmetric-type.h:72:12: error: dereferencing pointer to
incomplete type ‘const struct key’
return key->payload.data[asym_key_ids];
***********************************************************************
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
From: Mimi Zohar @ 2019-11-13 21:18 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <8eba665e-637c-d341-c77d-4f2645d3b246@linux.microsoft.com>
On Wed, 2019-11-13 at 12:52 -0800, Lakshmi Ramasubramanian wrote:
> On 11/13/19 12:09 PM, Mimi Zohar wrote:
> >
> > All that is is needed is the key and public_key structures, which are
> > defined in include/linux/keys.h and include/crypto/public_key.h. If
> > the keys subsystem is disabled, then the new IMA hook won't be called.
> > There's no need for a new Kconfig option or a new file.
> >
> > Please move the hook to just after ima_kexec_cmdline().
> >
> > Mimi
>
> Yes - IMA hook won't be called when KEYS subsystem is disabled.
>
> But, build dependency is breaking since "struct key" is not defined
> without CONFIG_KEYS.
>
> Sasha was able to craft a .config that enabled IMA without enabling KEYS
> and found the build break.
Yes, thanks for pointing out the "#ifdef CONFIG_KEYS" in keys.h. A
separate file is needed, as you pointed out, but still no need for a
new Kconfig. The ima/Makefile can be based on CONFIG_KEYS.
Mimi
^ permalink raw reply
* Re: [PATCH] x86/mtrr: only administrator can read the configurations.
From: Thomas Gleixner @ 2019-11-13 21:47 UTC (permalink / raw)
To: Kees Cook
Cc: Borislav Petkov, Zhang Xiaoxu, mingo, hpa, x86, tyhicks,
colin.king, linux-kernel, linux-security-module, Matthew Garrett
In-Reply-To: <201911121434.FF26FF3FE@keescook>
On Tue, 12 Nov 2019, Kees Cook wrote:
> 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?
Makes sense.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v6 2/3] IMA: Define an IMA hook to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-13 22:01 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573679903.4517.5.camel@linux.ibm.com>
On 11/13/19 1:18 PM, Mimi Zohar wrote:
> Yes, thanks for pointing out the "#ifdef CONFIG_KEYS" in keys.h. A
> separate file is needed, as you pointed out, but still no need for a
> new Kconfig. The ima/Makefile can be based on CONFIG_KEYS.
>
> Mimi
Sure - i'll try it now and send an updated patch set.
thanks,
-lakshmi
^ permalink raw reply
* Re: [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
From: Mimi Zohar @ 2019-11-13 22:02 UTC (permalink / raw)
To: Lakshmi Ramasubramanian, dhowells, matthewgarrett, sashal,
jamorris, linux-integrity, linux-security-module, keyrings,
linux-kernel
In-Reply-To: <20191113184658.2862-1-nramas@linux.microsoft.com>
On Wed, 2019-11-13 at 10:46 -0800, Lakshmi Ramasubramanian wrote:
> Keys created or updated in the system are currently not 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:
>
> 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.
>
> A new IMA policy function KEY_CHECK has been added to measure keys.
>
> # measure keys loaded onto any keyring
> measure func=KEY_CHECK
When re-posting this patch set, please include the support for
specifying the "keyrings=" policy option, as an additional patch.
Mimi
^ permalink raw reply
* Re: [PATCH v6 0/3] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-13 22:04 UTC (permalink / raw)
To: Mimi Zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <1573682528.4793.3.camel@linux.ibm.com>
On 11/13/2019 2:02 PM, Mimi Zohar wrote:
>
> When re-posting this patch set, please include the support for
> specifying the "keyrings=" policy option, as an additional patch.
>
> Mimi
Sure - will do.
thanks,
-lakshmi
^ permalink raw reply
* [PATCH v7 5/5] IMA: Read keyrings= option from the IMA policy
From: Lakshmi Ramasubramanian @ 2019-11-14 3:12 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191114031202.18012-1-nramas@linux.microsoft.com>
Read "keyrings=" option, if specified in the IMA policy, and store in
the list of IMA rules when the configured IMA policy is read.
This patch defines a new policy token enum namely Opt_keyrings
and an option flag IMA_KEYRINGS for reading "keyrings=" option
from the IMA policy.
Updated ima_parse_rule() to parse "keyrings=" option in the policy.
Updated ima_policy_show() to display "keyrings=" option.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/ima_policy.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 76da4f17bc79..577a51a548fb 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 */
@@ -795,7 +796,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 = {
@@ -831,6 +833,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}
};
@@ -1080,6 +1083,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 != KEY_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);
@@ -1455,6 +1475,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 v7 4/5] IMA: Add support to limit measuring keys
From: Lakshmi Ramasubramanian @ 2019-11-14 3:12 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191114031202.18012-1-nramas@linux.microsoft.com>
Limit measuring keys to those keys being loaded onto a given set of
keyrings only.
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 "measure func=KEY_CHECK" then only the keys
loaded onto a keyring given in the "keyrings=" option are measured.
Added a new parameter namely "keyring" (name of the keyring) to
process_buffer_measurement(). The keyring name is passed to
ima_get_action() to determine the required action.
ima_match_rules() is updated to check keyring in the policy, if
specified, for KEY_CHECK function.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
Documentation/ABI/testing/ima_policy | 10 +++++-
security/integrity/ima/ima.h | 8 +++--
security/integrity/ima/ima_api.c | 8 +++--
security/integrity/ima/ima_appraise.c | 4 +--
security/integrity/ima/ima_asymmetric_keys.c | 8 ++++-
security/integrity/ima/ima_main.c | 9 ++---
security/integrity/ima/ima_policy.c | 37 +++++++++++++++++---
7 files changed, 66 insertions(+), 18 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 3823c27894c5..5a941ed20fa3 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]
@@ -42,6 +42,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 KEY_CHECK.
template:= name of a defined IMA template type
(eg, ima-ng). Only valid when action is "measure".
pcr:= decimal value
@@ -118,3 +121,8 @@ Description:
measure func=KEY_CHECK
+ Example of measure rule using KEY_CHECK to only measure
+ keys added to .builtin_trusted_keys or .ima keyring:
+
+ measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
+
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index fe6c698617bd..f06238e41a7c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -208,7 +208,8 @@ struct modsig;
/* LIM API function definitions */
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr,
- struct ima_template_desc **template_desc);
+ struct ima_template_desc **template_desc,
+ const char *keyring);
int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
int ima_collect_measurement(struct integrity_iint_cache *iint,
struct file *file, void *buf, loff_t size,
@@ -220,7 +221,7 @@ void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file,
struct ima_template_desc *template_desc);
void process_buffer_measurement(const void *buf, int size,
const char *eventname, enum ima_hooks func,
- int pcr);
+ int pcr, const char *keyring);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
int ima_alloc_init_template(struct ima_event_data *event_data,
@@ -235,7 +236,8 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
/* IMA policy related functions */
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,
+ const char *keyring);
void ima_init_policy(void);
void ima_update_policy(void);
void ima_update_policy_flag(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 610759fe63b8..f6bc00914aa5 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -169,12 +169,13 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* @func: caller identifier
* @pcr: pointer filled in if matched measure policy sets pcr=
* @template_desc: pointer filled in if matched measure policy sets template=
+ * @keyring: keyring name used to determine the action
*
* The policy is defined in terms of keypairs:
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
* func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
- * | KEXEC_CMDLINE
+ * | KEXEC_CMDLINE | KEY_CHECK
* mask: contains the permission mask
* fsmagic: hex value
*
@@ -183,14 +184,15 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
*/
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr,
- struct ima_template_desc **template_desc)
+ struct ima_template_desc **template_desc,
+ const char *keyring)
{
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
flags &= ima_policy_flag;
return ima_match_policy(inode, cred, secid, func, mask, flags, pcr,
- template_desc);
+ template_desc, keyring);
}
/*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 300c8d2943c5..a9649b04b9f1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -55,7 +55,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
security_task_getsecid(current, &secid);
return ima_match_policy(inode, current_cred(), secid, func, mask,
- IMA_APPRAISE | IMA_HASH, NULL, NULL);
+ IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
}
static int ima_fix_xattr(struct dentry *dentry,
@@ -330,7 +330,7 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
process_buffer_measurement(digest, digestsize,
"blacklisted-hash", NONE,
- pcr);
+ pcr, NULL);
}
return rc;
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
index f6884641a622..8c692eb08a0a 100644
--- a/security/integrity/ima/ima_asymmetric_keys.c
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -45,7 +45,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
* parameter to process_buffer_measurement() and is set
* in the "eventname" field in ima_event_data for
* the key measurement IMA event.
+ *
+ * The name of the keyring is also passed in the "keyring"
+ * parameter to process_buffer_measurement() to check
+ * if the IMA policy is configured to measure a key linked
+ * to the given keyring.
*/
process_buffer_measurement(pk->key, pk->keylen,
- keyring->description, KEY_CHECK, 0);
+ keyring->description, KEY_CHECK, 0,
+ keyring->description);
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..6d0bf241ebf8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -215,7 +215,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
* Included is the appraise submask.
*/
action = ima_get_action(inode, cred, secid, mask, func, &pcr,
- &template_desc);
+ &template_desc, NULL);
violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
(ima_policy_flag & IMA_MEASURE));
if (!action && !violation_check)
@@ -632,12 +632,13 @@ int ima_load_data(enum kernel_load_data_id id)
* @eventname: event name to be used for the buffer entry.
* @func: IMA hook
* @pcr: pcr to extend the measurement
+ * @keyring: keyring name to determine the action to be performed
*
* Based on policy, the buffer is measured into the ima log.
*/
void process_buffer_measurement(const void *buf, int size,
const char *eventname, enum ima_hooks func,
- int pcr)
+ int pcr, const char *keyring)
{
int ret = 0;
struct ima_template_entry *entry = NULL;
@@ -665,7 +666,7 @@ void process_buffer_measurement(const void *buf, int size,
if (func) {
security_task_getsecid(current, &secid);
action = ima_get_action(NULL, current_cred(), secid, 0, func,
- &pcr, &template);
+ &pcr, &template, keyring);
if (!(action & IMA_MEASURE))
return;
}
@@ -718,7 +719,7 @@ void ima_kexec_cmdline(const void *buf, int size)
{
if (buf && size != 0)
process_buffer_measurement(buf, size, "kexec-cmdline",
- KEXEC_CMDLINE, 0);
+ KEXEC_CMDLINE, 0, NULL);
}
static int __init init_ima(void)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 1525a28fd705..76da4f17bc79 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -79,6 +79,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;
};
@@ -356,6 +357,25 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
return NOTIFY_OK;
}
+/**
+ * ima_match_keyring - determine whether the keyring matches the measure rule
+ * @rule: a pointer to a rule
+ * @keyring: name of the keyring to match against the measure rule
+ *
+ * If the measure action for KEY_CHECK does not specify keyrings=
+ * option then return true (Measure all keys).
+ * Else, return true if the given keyring name is present in
+ * the keyrings= option. False, otherwise.
+ */
+static bool ima_match_keyring(struct ima_rule_entry *rule,
+ const char *keyring)
+{
+ if ((keyring == NULL) || (rule->keyrings == NULL))
+ return true;
+ else
+ return (strstr(rule->keyrings, keyring) != NULL);
+}
+
/**
* ima_match_rules - determine whether an inode matches the measure rule.
* @rule: a pointer to a rule
@@ -364,18 +384,23 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
* @secid: the secid of the task to be validated
* @func: LIM hook identifier
* @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
+ * @keyring: keyring name to check in policy for KEY_CHECK func
*
* Returns true on rule match, false on failure.
*/
static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
const struct cred *cred, u32 secid,
- enum ima_hooks func, int mask)
+ enum ima_hooks func, int mask,
+ const char *keyring)
{
int i;
if ((func == KEXEC_CMDLINE) || (func == KEY_CHECK)) {
- if ((rule->flags & IMA_FUNC) && (rule->func == func))
+ if ((rule->flags & IMA_FUNC) && (rule->func == func)) {
+ if (func == KEY_CHECK)
+ return ima_match_keyring(rule, keyring);
return true;
+ }
return false;
}
if ((rule->flags & IMA_FUNC) &&
@@ -479,6 +504,8 @@ 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
+ * @keyring: the keyring name, if given, to be used to check in the policy.
+ * keyring can be NULL if func is anything other than KEY_CHECK.
*
* Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
* conditions.
@@ -489,7 +516,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,
+ const char *keyring)
{
struct ima_rule_entry *entry;
int action = 0, actmask = flags | (flags << 1);
@@ -503,7 +531,8 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
if (!(entry->action & actmask))
continue;
- if (!ima_match_rules(entry, inode, cred, secid, func, mask))
+ if (!ima_match_rules(entry, inode, cred, secid, func, mask,
+ keyring))
continue;
action |= entry->flags & IMA_ACTION_FLAGS;
--
2.17.1
^ permalink raw reply related
* [PATCH v7 2/5] IMA: Define an IMA hook to measure keys
From: Lakshmi Ramasubramanian @ 2019-11-14 3:11 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <20191114031202.18012-1-nramas@linux.microsoft.com>
Measure asymmetric keys used for verifying file signatures,
certificates, etc.
This patch defines a new IMA hook namely ima_post_key_create_or_update()
to measure asymmetric keys.
The IMA hook is defined in a new file namely ima_asymmetric_keys.c
which is built only if CONFIG_KEYS is enabled.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima_asymmetric_keys.c | 51 ++++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 security/integrity/ima/ima_asymmetric_keys.c
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 31d57cdf2421..3c796664be8a 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -12,3 +12,4 @@ ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
+obj-$(CONFIG_KEYS) += ima_asymmetric_keys.o
diff --git a/security/integrity/ima/ima_asymmetric_keys.c b/security/integrity/ima/ima_asymmetric_keys.c
new file mode 100644
index 000000000000..f6884641a622
--- /dev/null
+++ b/security/integrity/ima/ima_asymmetric_keys.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Microsoft Corporation
+ *
+ * Author: Lakshmi Ramasubramanian (nramas@linux.microsoft.com)
+ *
+ * File: ima_asymmetric_keys.c
+ * Defines an IMA hook to measure asymmetric keys on key
+ * create or update.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
+#include "ima.h"
+
+/**
+ * ima_post_key_create_or_update - measure asymmetric keys
+ * @keyring: keyring to which the key is linked to
+ * @key: created or updated key
+ * @flags: key flags
+ * @create: flag indicating whether the key was created or updated
+ *
+ * Keys can only be measured, not appraised.
+ */
+void ima_post_key_create_or_update(struct key *keyring, struct key *key,
+ unsigned long flags, bool create)
+{
+ const struct public_key *pk;
+
+ /* Only asymmetric keys are handled by this hook. */
+ if (key->type != &key_type_asymmetric)
+ return;
+
+ /* Get the public_key of the given asymmetric key to measure. */
+ pk = key->payload.data[asym_crypto];
+
+ /*
+ * keyring->description points to the name of the keyring
+ * (such as ".builtin_trusted_keys", ".ima", etc.) to
+ * which the given key is linked to.
+ *
+ * The name of the keyring is passed in the "eventname"
+ * parameter to process_buffer_measurement() and is set
+ * in the "eventname" field in ima_event_data for
+ * the key measurement IMA event.
+ */
+ process_buffer_measurement(pk->key, pk->keylen,
+ keyring->description, KEY_CHECK, 0);
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v7 0/5] KEYS: Measure keys when they are created or updated
From: Lakshmi Ramasubramanian @ 2019-11-14 3:11 UTC (permalink / raw)
To: zohar, dhowells, matthewgarrett, sashal, jamorris,
linux-integrity, linux-security-module, keyrings, linux-kernel
Keys created or updated in the system are currently not 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.
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.
A new IMA policy function KEY_CHECK has been added to measure keys.
"keyrings=" option can be specified for KEY_CHECK to limit
measuring the keys loaded onto the specified keyrings only.
# measure keys loaded onto any keyring
measure func=KEY_CHECK
# measure keys loaded onto the IMA keyring only
measure func=KEY_CHECK keyring=".ima"
# measure keys on the BUILTIN and IMA keyrings into a different PCR
measure func=KEY_CHECK keyring=".builtin_trusted_keys|.ima" pcr=11
Testing performed:
* Booted the kernel with this change.
* When KEY_CHECK policy is set IMA measures keys loaded
onto any keyring (keyrings= option not specified).
* Keys are not measured when KEY_CHECK is not set.
* When keyrings= option is specified for KEY_CHECK then only the keys
loaded onto a keyring specified in the option is measured.
* Added a new key to a keyring.
=> Added keys to .ima and .evm keyrings.
* Added the same key again.
=> Add the same key to .ima and .evm keyrings.
Change Log:
v7:
=> Removed CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS option and used
CONFIG_KEYS instead for ima_asymmetric_keys.c
=> Added the patches related to "keyrings=" option support to
this patch set.
v6:
=> Rebased the changes to v5.4-rc7
=> Renamed KEYRING_CHECK to KEY_CHECK per Mimi's suggestion.
=> Excluded the patches that add support for limiting key
measurement to specific keyrings ("keyrings=" option
for "measure func=KEY_CHECK" in the IMA policy).
Also, excluded the patches that add support for deferred
processing of keys (queue support).
These patches will be added in separate patch sets later.
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.
Lakshmi Ramasubramanian (5):
IMA: Add KEY_CHECK func to measure keys
IMA: Define an IMA hook to measure keys
KEYS: Call the IMA hook to measure keys
IMA: Add support to limit measuring keys
IMA: Read keyrings= option from the IMA policy
Documentation/ABI/testing/ima_policy | 17 ++++-
include/linux/ima.h | 13 ++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 ++-
security/integrity/ima/ima_api.c | 8 ++-
security/integrity/ima/ima_appraise.c | 4 +-
security/integrity/ima/ima_asymmetric_keys.c | 57 ++++++++++++++++
security/integrity/ima/ima_main.c | 9 +--
security/integrity/ima/ima_policy.c | 70 ++++++++++++++++++--
security/keys/key.c | 7 ++
10 files changed, 175 insertions(+), 20 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