All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: keyrings@vger.kernel.org
Cc: linux-security-module@vger.kernel.org, linux-nfs@vger.kernel.org,
	linux-cifs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	rgb@redhat.com, dhowells@redhat.com,
	linux-kernel@vger.kernel.org
Subject: [RFC PATCH 5/9] keys: Include target namespace in match criteria
Date: Thu, 14 Feb 2019 16:57:47 +0000	[thread overview]
Message-ID: <155016346784.11489.7639781267844538576.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <155016339876.11489.901851271827069026.stgit@warthog.procyon.org.uk>

Currently a key has a standard matching criteria of { type, description } and
this is used to only allow keys with unique criteria in a keyring.  This
means, however, that you cannot have keys with the same type and description
but a different target namespace in the same keyring.

This is a potential problem for a containerised environment where, say, a
container is made up of some parts of its mount space involving netfs
superblocks from two different network namespaces.

This is also a problem for shared system management keyrings such as the DNS
records keyring or the NFS idmapper keyring that might contain keys from
different network namespaces.

Fix this by including a namespace component in a key's matching criteria.
Keyring types are marked to indicate which, if any, namespace is relevant to
keys of that type, and that namespace is set when the key is created from the
current task's namespace set.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h        |   11 +++++++++++
 security/keys/gc.c         |    2 +-
 security/keys/key.c        |    1 +
 security/keys/keyring.c    |   36 ++++++++++++++++++++++++++++++++++--
 security/keys/persistent.c |    1 +
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index b39f5876b66d..bb716c33bffe 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -82,9 +82,16 @@ struct cred;
 
 struct key_type;
 struct key_owner;
+struct key_tag;
 struct keyring_list;
 struct keyring_name;
 
+struct key_tag {
+	struct rcu_head		rcu;
+	refcount_t		usage;
+	bool			removed;	/* T when subject removed */
+};
+
 struct keyring_index_key {
 	/* [!] If this structure is altered, the union in struct key must change too! */
 	unsigned long		hash;			/* Hash value */
@@ -101,6 +108,7 @@ struct keyring_index_key {
 		unsigned long x;
 	};
 	struct key_type		*type;
+	struct key_tag		*domain_tag;	/* Domain of operation */
 	const char		*description;
 };
 
@@ -218,6 +226,7 @@ struct key {
 			unsigned long	hash;
 			unsigned long	len_desc;
 			struct key_type	*type;		/* type of key */
+			struct key_tag	*domain_tag;	/* Domain of operation */
 			char		*description;
 		};
 	};
@@ -268,6 +277,7 @@ extern struct key *key_alloc(struct key_type *type,
 extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);
 extern void key_put(struct key *key);
+extern bool key_put_tag(struct key_tag *tag);
 
 static inline struct key *__key_get(struct key *key)
 {
@@ -425,6 +435,7 @@ extern void key_init(void);
 #define key_fsuid_changed(t)		do { } while(0)
 #define key_fsgid_changed(t)		do { } while(0)
 #define key_init()			do { } while(0)
+#define key_put_subject(s)		do { } while(0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 634e96b380e8..83d279fb7793 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -154,7 +154,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 			atomic_dec(&key->user->nikeys);
 
 		key_user_put(key->user);
-
+		key_put_tag(key->domain_tag);
 		kfree(key->description);
 
 		memzero_explicit(key, sizeof(*key));
diff --git a/security/keys/key.c b/security/keys/key.c
index 1568b0028ba4..8cd0f98e4fa3 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -317,6 +317,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto security_error;
 
 	/* publish the key by giving it a serial number */
+	refcount_inc(&key->domain_tag->usage);
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 11c44ad07b5b..0df10221c037 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -181,6 +181,9 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 	type = (unsigned long)index_key->type;
 	acc = mult_64x32_and_fold(type, desc_len + 13);
 	acc = mult_64x32_and_fold(acc, 9207);
+	piece = (unsigned long)index_key->domain_tag;
+	acc = mult_64x32_and_fold(acc, piece);
+	acc = mult_64x32_and_fold(acc, 9207);
 
 	for (;;) {
 		n = desc_len;
@@ -214,16 +217,36 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 
 /*
  * Finalise an index key to include a part of the description actually in the
- * index key and to add in the hash too.
+ * index key, to set the domain tag and to calculate the hash.
  */
 void key_set_index_key(struct keyring_index_key *index_key)
 {
+	static struct key_tag default_domain_tag = { .usage = REFCOUNT_INIT(1), };
 	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+
 	memcpy(index_key->desc, index_key->description, n);
 
+	index_key->domain_tag = &default_domain_tag;
 	hash_key_type_and_desc(index_key);
 }
 
+/**
+ * key_put_tag - Release a ref on a tag.
+ * @tag: The tag to release.
+ *
+ * This releases a reference the given tag and returns true if that ref was the
+ * last one.
+ */
+bool key_put_tag(struct key_tag *tag)
+{
+	if (refcount_dec_and_test(&tag->usage)) {
+		kfree_rcu(tag, rcu);
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * Build the next index key chunk.
  *
@@ -244,8 +267,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 		return index_key->x;
 	case 2:
 		return (unsigned long)index_key->type;
+	case 3:
+		return (unsigned long)index_key->domain_tag;
 	default:
-		level -= 3;
+		level -= 4;
 		if (desc_len <= sizeof(index_key->desc))
 			return 0;
 
@@ -274,6 +299,7 @@ static bool keyring_compare_object(const void *object, const void *data)
 	const struct key *key = keyring_ptr_to_key(object);
 
 	return key->index_key.type = index_key->type &&
+		key->index_key.domain_tag = index_key->domain_tag &&
 		key->index_key.desc_len = index_key->desc_len &&
 		memcmp(key->index_key.description, index_key->description,
 		       index_key->desc_len) = 0;
@@ -315,6 +341,12 @@ static int keyring_diff_objects(const void *object, const void *data)
 		goto differ;
 	level += sizeof(unsigned long);
 
+	seg_a = (unsigned long)a->domain_tag;
+	seg_b = (unsigned long)b->domain_tag;
+	if ((seg_a ^ seg_b) != 0)
+		goto differ;
+	level += sizeof(unsigned long);
+
 	i = sizeof(a->desc);
 	if (a->desc_len <= i)
 		goto same;
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index fc29ec59efa7..c9fbe63adc58 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -84,6 +84,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	long ret;
 
 	/* Look in the register if it exists */
+	memset(&index_key, 0, sizeof(index_key));
 	index_key.type = &key_type_keyring;
 	index_key.description = buf;
 	index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));

WARNING: multiple messages have this Message-ID (diff)
From: David Howells <dhowells@redhat.com>
To: keyrings@vger.kernel.org
Cc: linux-security-module@vger.kernel.org, linux-nfs@vger.kernel.org,
	linux-cifs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	rgb@redhat.com, dhowells@redhat.com,
	linux-kernel@vger.kernel.org
Subject: [RFC PATCH 5/9] keys: Include target namespace in match criteria
Date: Thu, 14 Feb 2019 16:57:47 +0000	[thread overview]
Message-ID: <155016346784.11489.7639781267844538576.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <155016339876.11489.901851271827069026.stgit@warthog.procyon.org.uk>

Currently a key has a standard matching criteria of { type, description } and
this is used to only allow keys with unique criteria in a keyring.  This
means, however, that you cannot have keys with the same type and description
but a different target namespace in the same keyring.

This is a potential problem for a containerised environment where, say, a
container is made up of some parts of its mount space involving netfs
superblocks from two different network namespaces.

This is also a problem for shared system management keyrings such as the DNS
records keyring or the NFS idmapper keyring that might contain keys from
different network namespaces.

Fix this by including a namespace component in a key's matching criteria.
Keyring types are marked to indicate which, if any, namespace is relevant to
keys of that type, and that namespace is set when the key is created from the
current task's namespace set.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/key.h        |   11 +++++++++++
 security/keys/gc.c         |    2 +-
 security/keys/key.c        |    1 +
 security/keys/keyring.c    |   36 ++++++++++++++++++++++++++++++++++--
 security/keys/persistent.c |    1 +
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/include/linux/key.h b/include/linux/key.h
index b39f5876b66d..bb716c33bffe 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -82,9 +82,16 @@ struct cred;
 
 struct key_type;
 struct key_owner;
+struct key_tag;
 struct keyring_list;
 struct keyring_name;
 
+struct key_tag {
+	struct rcu_head		rcu;
+	refcount_t		usage;
+	bool			removed;	/* T when subject removed */
+};
+
 struct keyring_index_key {
 	/* [!] If this structure is altered, the union in struct key must change too! */
 	unsigned long		hash;			/* Hash value */
@@ -101,6 +108,7 @@ struct keyring_index_key {
 		unsigned long x;
 	};
 	struct key_type		*type;
+	struct key_tag		*domain_tag;	/* Domain of operation */
 	const char		*description;
 };
 
@@ -218,6 +226,7 @@ struct key {
 			unsigned long	hash;
 			unsigned long	len_desc;
 			struct key_type	*type;		/* type of key */
+			struct key_tag	*domain_tag;	/* Domain of operation */
 			char		*description;
 		};
 	};
@@ -268,6 +277,7 @@ extern struct key *key_alloc(struct key_type *type,
 extern void key_revoke(struct key *key);
 extern void key_invalidate(struct key *key);
 extern void key_put(struct key *key);
+extern bool key_put_tag(struct key_tag *tag);
 
 static inline struct key *__key_get(struct key *key)
 {
@@ -425,6 +435,7 @@ extern void key_init(void);
 #define key_fsuid_changed(t)		do { } while(0)
 #define key_fsgid_changed(t)		do { } while(0)
 #define key_init()			do { } while(0)
+#define key_put_subject(s)		do { } while(0)
 
 #endif /* CONFIG_KEYS */
 #endif /* __KERNEL__ */
diff --git a/security/keys/gc.c b/security/keys/gc.c
index 634e96b380e8..83d279fb7793 100644
--- a/security/keys/gc.c
+++ b/security/keys/gc.c
@@ -154,7 +154,7 @@ static noinline void key_gc_unused_keys(struct list_head *keys)
 			atomic_dec(&key->user->nikeys);
 
 		key_user_put(key->user);
-
+		key_put_tag(key->domain_tag);
 		kfree(key->description);
 
 		memzero_explicit(key, sizeof(*key));
diff --git a/security/keys/key.c b/security/keys/key.c
index 1568b0028ba4..8cd0f98e4fa3 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -317,6 +317,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 		goto security_error;
 
 	/* publish the key by giving it a serial number */
+	refcount_inc(&key->domain_tag->usage);
 	atomic_inc(&user->nkeys);
 	key_alloc_serial(key);
 
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 11c44ad07b5b..0df10221c037 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -181,6 +181,9 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 	type = (unsigned long)index_key->type;
 	acc = mult_64x32_and_fold(type, desc_len + 13);
 	acc = mult_64x32_and_fold(acc, 9207);
+	piece = (unsigned long)index_key->domain_tag;
+	acc = mult_64x32_and_fold(acc, piece);
+	acc = mult_64x32_and_fold(acc, 9207);
 
 	for (;;) {
 		n = desc_len;
@@ -214,16 +217,36 @@ static void hash_key_type_and_desc(struct keyring_index_key *index_key)
 
 /*
  * Finalise an index key to include a part of the description actually in the
- * index key and to add in the hash too.
+ * index key, to set the domain tag and to calculate the hash.
  */
 void key_set_index_key(struct keyring_index_key *index_key)
 {
+	static struct key_tag default_domain_tag = { .usage = REFCOUNT_INIT(1), };
 	size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+
 	memcpy(index_key->desc, index_key->description, n);
 
+	index_key->domain_tag = &default_domain_tag;
 	hash_key_type_and_desc(index_key);
 }
 
+/**
+ * key_put_tag - Release a ref on a tag.
+ * @tag: The tag to release.
+ *
+ * This releases a reference the given tag and returns true if that ref was the
+ * last one.
+ */
+bool key_put_tag(struct key_tag *tag)
+{
+	if (refcount_dec_and_test(&tag->usage)) {
+		kfree_rcu(tag, rcu);
+		return true;
+	}
+
+	return false;
+}
+
 /*
  * Build the next index key chunk.
  *
@@ -244,8 +267,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
 		return index_key->x;
 	case 2:
 		return (unsigned long)index_key->type;
+	case 3:
+		return (unsigned long)index_key->domain_tag;
 	default:
-		level -= 3;
+		level -= 4;
 		if (desc_len <= sizeof(index_key->desc))
 			return 0;
 
@@ -274,6 +299,7 @@ static bool keyring_compare_object(const void *object, const void *data)
 	const struct key *key = keyring_ptr_to_key(object);
 
 	return key->index_key.type == index_key->type &&
+		key->index_key.domain_tag == index_key->domain_tag &&
 		key->index_key.desc_len == index_key->desc_len &&
 		memcmp(key->index_key.description, index_key->description,
 		       index_key->desc_len) == 0;
@@ -315,6 +341,12 @@ static int keyring_diff_objects(const void *object, const void *data)
 		goto differ;
 	level += sizeof(unsigned long);
 
+	seg_a = (unsigned long)a->domain_tag;
+	seg_b = (unsigned long)b->domain_tag;
+	if ((seg_a ^ seg_b) != 0)
+		goto differ;
+	level += sizeof(unsigned long);
+
 	i = sizeof(a->desc);
 	if (a->desc_len <= i)
 		goto same;
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index fc29ec59efa7..c9fbe63adc58 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -84,6 +84,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
 	long ret;
 
 	/* Look in the register if it exists */
+	memset(&index_key, 0, sizeof(index_key));
 	index_key.type = &key_type_keyring;
 	index_key.description = buf;
 	index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));


  parent reply	other threads:[~2019-02-14 16:57 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-14 16:56 [RFC PATCH 0/9] keys: Namespacing David Howells
2019-02-14 16:56 ` [RFC PATCH 1/9] keys: Invalidate used request_key authentication keys David Howells
2019-02-14 16:57 ` [RFC PATCH 2/9] keys: Kill off request_key_async{,_with_auxdata} David Howells
2019-02-14 16:57 ` [RFC PATCH 3/9] keys: Simplify key description management David Howells
2019-02-14 16:57   ` David Howells
2019-02-14 16:57 ` [RFC PATCH 4/9] keys: Cache the hash value to avoid lots of recalculation David Howells
2019-02-14 16:57   ` David Howells
2019-02-14 16:57 ` David Howells [this message]
2019-02-14 16:57   ` [RFC PATCH 5/9] keys: Include target namespace in match criteria David Howells
2019-02-14 16:58 ` [RFC PATCH 6/9] keys: Garbage collect keys for which the domain has been removed David Howells
2019-02-14 16:58 ` [RFC PATCH 7/9] keys: Network namespace domain tag David Howells
2019-02-14 16:58 ` [RFC PATCH 8/9] keys: Pass the network namespace into request_key mechanism David Howells
2019-02-14 16:58   ` David Howells
2019-02-14 16:58 ` [RFC PATCH 9/9] KEYS: Namespace keyring names David Howells
2019-02-14 16:58   ` David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=155016346784.11489.7639781267844538576.stgit@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=rgb@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.