[PATCH 0/7] Fix dereferencing payload of revoked keys

[PATCH 0/7] Fix dereferencing payload of revoked keys

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

This series fixes the various users of the keyrings service that access
a "user" or "logon" key's payload without first checking whether the
payload pointer is NULL, or calling key_validate() while holding the key
semaphore.  Without one of these two checks, a NULL pointer dereference
will occur if the key has been revoked concurrently.  Usually this is
pretty easy to reproduce (in most of the cases even as an unprivileged
user), although it may be unlikely to happen by accident.

Patch 6 also fixes the lack of key length validation in ecryptfs.

These fixes probably will need to be split up between a few different
maintainers, but initially I'm sending the full series so that people
can see the full context of the fixes.

Eric Biggers (7):
  KEYS: encrypted: fix dereference of NULL user_key_payload
  FS-Cache: fix dereference of NULL user_key_payload
  lib/digsig: fix dereference of NULL user_key_payload
  fscrypt: fix dereference of NULL user_key_payload
  ecryptfs: fix dereference of NULL user_key_payload
  ecryptfs: fix out-of-bounds read of key payload
  ecryptfs: move key payload accessor functions into keystore.c

 fs/crypto/keyinfo.c                      |  5 +++
 fs/ecryptfs/ecryptfs_kernel.h            | 44 -------------------
 fs/ecryptfs/keystore.c                   | 73 +++++++++++++++++++++++++++++++-
 fs/fscache/object-list.c                 |  7 +++
 lib/digsig.c                             |  6 +++
 security/keys/encrypted-keys/encrypted.c |  7 +++
 6 files changed, 97 insertions(+), 45 deletions(-)

-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] KEYS: encrypted: fix dereference of NULL user_key_payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

A key of type "encrypted" references a "master key" which is used to
encrypt and decrypt the encrypted key's payload.  However, when we
accessed the master key's payload, we failed to handle the case where
the master key has been revoked, which sets the payload pointer to NULL.
Note that request_key() *does* skip revoked keys, but there is still a
window where the key can be revoked before we acquire its semaphore.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

This was an issue for master keys of type "user" only.  Master keys can
also be of type "trusted", but those cannot be revoked.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Cc: <stable@vger.kernel.org>    [v2.6.38+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 security/keys/encrypted-keys/encrypted.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index f54b92868bc3..d92cbf9687c3 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -309,6 +309,13 @@ static struct key *request_user_key(const char *master_desc, const u8 **master_k
 
 	down_read(&ukey->sem);
 	upayload = user_key_payload_locked(ukey);
+	if (!upayload) {
+		/* key was revoked before we acquired its semaphore */
+		up_read(&ukey->sem);
+		key_put(ukey);
+		ukey = ERR_PTR(-EKEYREVOKED);
+		goto error;
+	}
 	*master_key = upayload->data;
 	*master_keylen = upayload->datalen;
 error:
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] FS-Cache: fix dereference of NULL user_key_payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

When the file /proc/fs/fscache/objects (available with
CONFIG_FSCACHE_OBJECT_LIST=y) is opened, we request a user key with
description "fscache:objlist", then access its payload.  However, a
revoked key has a NULL payload, and we failed to check for this.
request_key() *does* skip revoked keys, but there is still a window
where the key can be revoked before we access its payload.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

Fixes: 4fbf4291aa15 ("FS-Cache: Allow the current state of all objects to be dumped")
Cc: <stable@vger.kernel.org>    [v2.6.32+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/fscache/object-list.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/fscache/object-list.c b/fs/fscache/object-list.c
index b5ab06fabc60..0438d4cd91ef 100644
--- a/fs/fscache/object-list.c
+++ b/fs/fscache/object-list.c
@@ -331,6 +331,13 @@ static void fscache_objlist_config(struct fscache_objlist_data *data)
 	rcu_read_lock();
 
 	confkey = user_key_payload_rcu(key);
+	if (!confkey) {
+		/* key was revoked */
+		rcu_read_unlock();
+		key_put(key);
+		goto no_config;
+	}
+
 	buf = confkey->data;
 
 	for (len = confkey->datalen - 1; len >= 0; len--) {
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] lib/digsig: fix dereference of NULL user_key_payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

digsig_verify() requests a user key, then accesses its payload.
However, a revoked key has a NULL payload, and we failed to check for
this.  request_key() *does* skip revoked keys, but there is still a
window where the key can be revoked before we acquire its semaphore.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

Fixes: 051dbb918c7f ("crypto: digital signature verification support")
Cc: <stable@vger.kernel.org>    [v3.3+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 lib/digsig.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/digsig.c b/lib/digsig.c
index 03d7c63837ae..6ba6fcd92dd1 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -87,6 +87,12 @@ static int digsig_verify_rsa(struct key *key,
 	down_read(&key->sem);
 	ukp = user_key_payload_locked(key);
 
+	if (!ukp) {
+		/* key was revoked before we acquired its semaphore */
+		err = -EKEYREVOKED;
+		goto err1;
+	}
+
 	if (ukp->datalen < sizeof(*pkh))
 		goto err1;
 
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info@ http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] fscrypt: fix dereference of NULL user_key_payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

When an fscrypt-encrypted file is opened, we request the file's master
key from the keyrings service as a logon key, then access its payload.
However, a revoked key has a NULL payload, and we failed to check for
this.  request_key() *does* skip revoked keys, but there is still a
window where the key can be revoked before we acquire its semaphore.

Fix it by checking for a NULL payload, treating it like a key which was
already revoked at the time it was requested.

Fixes: 88bd6ccdcdd6 ("ext4 crypto: add encryption key management facilities")
Cc: <stable@vger.kernel.org>    [v4.1+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/keyinfo.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 018c588c7ac3..8e704d12a1cf 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -109,6 +109,11 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		goto out;
 	}
 	ukp = user_key_payload_locked(keyring_key);
+	if (!ukp) {
+		/* key was revoked before we acquired its semaphore */
+		res = -EKEYREVOKED;
+		goto out;
+	}
 	if (ukp->datalen != sizeof(struct fscrypt_key)) {
 		res = -EINVAL;
 		goto out;
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] ecryptfs: fix dereference of NULL user_key_payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

In eCryptfs, we failed to verify that the authentication token keys are
not revoked before dereferencing their payloads, which is problematic
because the payload of a revoked key is NULL.  request_key() *does* skip
revoked keys, but there is still a window where the key can be revoked
before we acquire the key semaphore.

Fix it by updating ecryptfs_get_key_payload_data() to return
-EKEYREVOKED if the key payload is NULL.  For completeness we check this
for "encrypted" keys as well as "user" keys, although encrypted keys
cannot be revoked currently.

Alternatively we could use key_validate(), but since we'll also need to
fix ecryptfs_get_key_payload_data() to validate the payload length, it
seems appropriate to just check the payload pointer.

Fixes: 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
Cc: <stable@vger.kernel.org>    [v2.6.19+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ecryptfs/ecryptfs_kernel.h | 24 +++++++++++++++++-------
 fs/ecryptfs/keystore.c        |  9 ++++++++-
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 9c351bf757b2..3fbc0ff79699 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -84,11 +84,16 @@ struct ecryptfs_page_crypt_context {
 static inline struct ecryptfs_auth_tok *
 ecryptfs_get_encrypted_key_payload_data(struct key *key)
 {
-	if (key->type == &key_type_encrypted)
-		return (struct ecryptfs_auth_tok *)
-			(&((struct encrypted_key_payload *)key->payload.data[0])->payload_data);
-	else
+	struct encrypted_key_payload *payload;
+
+	if (key->type != &key_type_encrypted)
 		return NULL;
+
+	payload = key->payload.data[0];
+	if (!payload)
+		return ERR_PTR(-EKEYREVOKED);
+
+	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
 static inline struct key *ecryptfs_get_encrypted_key(char *sig)
@@ -114,12 +119,17 @@ static inline struct ecryptfs_auth_tok *
 ecryptfs_get_key_payload_data(struct key *key)
 {
 	struct ecryptfs_auth_tok *auth_tok;
+	struct user_key_payload *ukp;
 
 	auth_tok = ecryptfs_get_encrypted_key_payload_data(key);
-	if (!auth_tok)
-		return (struct ecryptfs_auth_tok *)user_key_payload_locked(key)->data;
-	else
+	if (auth_tok)
 		return auth_tok;
+
+	ukp = user_key_payload_locked(key);
+	if (!ukp)
+		return ERR_PTR(-EKEYREVOKED);
+
+	return (struct ecryptfs_auth_tok *)ukp->data;
 }
 
 #define ECRYPTFS_MAX_KEYSET_SIZE 1024
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index 3cf1546dca82..fa218cd64f74 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -459,7 +459,8 @@ static int ecryptfs_verify_version(u16 version)
  * @auth_tok_key: key containing the authentication token
  * @auth_tok: authentication token
  *
- * Returns zero on valid auth tok; -EINVAL otherwise
+ * Returns zero on valid auth tok; -EINVAL if the payload is invalid; or
+ * -EKEYREVOKED if the key was revoked before we acquired its semaphore.
  */
 static int
 ecryptfs_verify_auth_tok_from_key(struct key *auth_tok_key,
@@ -468,6 +469,12 @@ ecryptfs_verify_auth_tok_from_key(struct key *auth_tok_key,
 	int rc = 0;
 
 	(*auth_tok) = ecryptfs_get_key_payload_data(auth_tok_key);
+	if (IS_ERR(*auth_tok)) {
+		rc = PTR_ERR(*auth_tok);
+		*auth_tok = NULL;
+		goto out;
+	}
+
 	if (ecryptfs_verify_version((*auth_tok)->version)) {
 		printk(KERN_ERR "Data structure version mismatch. Userspace "
 		       "tools must match eCryptfs kernel module with major "
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] ecryptfs: fix out-of-bounds read of key payload

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

eCryptfs blindly casts the user-supplied key payload to a
'struct ecryptfs_auth_tok' without validating that the payload does, in
fact, have the size of a 'struct ecryptfs_auth_tok'.  Fix it.

Fixes: 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
Cc: <stable@vger.kernel.org>    [v2.6.19+]
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ecryptfs/ecryptfs_kernel.h | 6 ++++++
 fs/ecryptfs/keystore.c        | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 3fbc0ff79699..945844d5f0ef 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -93,6 +93,9 @@ ecryptfs_get_encrypted_key_payload_data(struct key *key)
 	if (!payload)
 		return ERR_PTR(-EKEYREVOKED);
 
+	if (payload->payload_datalen != sizeof(struct ecryptfs_auth_tok))
+		return ERR_PTR(-EINVAL);
+
 	return (struct ecryptfs_auth_tok *)payload->payload_data;
 }
 
@@ -129,6 +132,9 @@ ecryptfs_get_key_payload_data(struct key *key)
 	if (!ukp)
 		return ERR_PTR(-EKEYREVOKED);
 
+	if (ukp->datalen != sizeof(struct ecryptfs_auth_tok))
+		return ERR_PTR(-EINVAL);
+
 	return (struct ecryptfs_auth_tok *)ukp->data;
 }
 
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index fa218cd64f74..95e20ab67df3 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -471,6 +471,10 @@ ecryptfs_verify_auth_tok_from_key(struct key *auth_tok_key,
 	(*auth_tok) = ecryptfs_get_key_payload_data(auth_tok_key);
 	if (IS_ERR(*auth_tok)) {
 		rc = PTR_ERR(*auth_tok);
+		if (rc == -EINVAL) {
+			ecryptfs_printk(KERN_ERR,
+					"Authentication token payload has wrong length\n");
+		}
 		*auth_tok = NULL;
 		goto out;
 	}
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] ecryptfs: move key payload accessor functions into keystore.c

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

As experience has shown, accessing the 'struct key' payload is very
error-prone, since we need to hold the key semaphore and properly
validate everything.  Fortunately eCryptfs only does it from one place,
in ecryptfs_verify_auth_tok_from_key() in keystore.c.  Therefore, move
the payload accessor functions like ecryptfs_get_key_payload_data() out
of ecryptfs_kernel.h and into keystore.c so that people might be less
tempted to use them directly.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ecryptfs/ecryptfs_kernel.h | 60 -------------------------------------------
 fs/ecryptfs/keystore.c        | 60 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index 945844d5f0ef..f2e339a6f9e9 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -29,8 +29,6 @@
 #define ECRYPTFS_KERNEL_H
 
 #include <crypto/skcipher.h>
-#include <keys/user-type.h>
-#include <keys/encrypted-type.h>
 #include <linux/fs.h>
 #include <linux/fs_stack.h>
 #include <linux/namei.h>
@@ -80,64 +78,6 @@ struct ecryptfs_page_crypt_context {
 	} param;
 };
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	struct encrypted_key_payload *payload;
-
-	if (key->type != &key_type_encrypted)
-		return NULL;
-
-	payload = key->payload.data[0];
-	if (!payload)
-		return ERR_PTR(-EKEYREVOKED);
-
-	if (payload->payload_datalen != sizeof(struct ecryptfs_auth_tok))
-		return ERR_PTR(-EINVAL);
-
-	return (struct ecryptfs_auth_tok *)payload->payload_data;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return request_key(&key_type_encrypted, sig, NULL);
-}
-
-#else
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_encrypted_key_payload_data(struct key *key)
-{
-	return NULL;
-}
-
-static inline struct key *ecryptfs_get_encrypted_key(char *sig)
-{
-	return ERR_PTR(-ENOKEY);
-}
-
-#endif /* CONFIG_ENCRYPTED_KEYS */
-
-static inline struct ecryptfs_auth_tok *
-ecryptfs_get_key_payload_data(struct key *key)
-{
-	struct ecryptfs_auth_tok *auth_tok;
-	struct user_key_payload *ukp;
-
-	auth_tok = ecryptfs_get_encrypted_key_payload_data(key);
-	if (auth_tok)
-		return auth_tok;
-
-	ukp = user_key_payload_locked(key);
-	if (!ukp)
-		return ERR_PTR(-EKEYREVOKED);
-
-	if (ukp->datalen != sizeof(struct ecryptfs_auth_tok))
-		return ERR_PTR(-EINVAL);
-
-	return (struct ecryptfs_auth_tok *)ukp->data;
-}
-
 #define ECRYPTFS_MAX_KEYSET_SIZE 1024
 #define ECRYPTFS_MAX_CIPHER_NAME_SIZE 31
 #define ECRYPTFS_MAX_NUM_ENC_KEYS 64
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index 95e20ab67df3..cb801bdcbae2 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -27,6 +27,8 @@
 
 #include <crypto/hash.h>
 #include <crypto/skcipher.h>
+#include <keys/user-type.h>
+#include <keys/encrypted-type.h>
 #include <linux/string.h>
 #include <linux/pagemap.h>
 #include <linux/key.h>
@@ -454,6 +456,64 @@ static int ecryptfs_verify_version(u16 version)
 	return rc;
 }
 
+#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
+static inline struct ecryptfs_auth_tok *
+ecryptfs_get_encrypted_key_payload_data(struct key *key)
+{
+	struct encrypted_key_payload *payload;
+
+	if (key->type != &key_type_encrypted)
+		return NULL;
+
+	payload = key->payload.data[0];
+	if (!payload)
+		return ERR_PTR(-EKEYREVOKED);
+
+	if (payload->payload_datalen != sizeof(struct ecryptfs_auth_tok))
+		return ERR_PTR(-EINVAL);
+
+	return (struct ecryptfs_auth_tok *)payload->payload_data;
+}
+
+static inline struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	return request_key(&key_type_encrypted, sig, NULL);
+}
+
+#else
+static inline struct ecryptfs_auth_tok *
+ecryptfs_get_encrypted_key_payload_data(struct key *key)
+{
+	return NULL;
+}
+
+static inline struct key *ecryptfs_get_encrypted_key(char *sig)
+{
+	return ERR_PTR(-ENOKEY);
+}
+
+#endif /* CONFIG_ENCRYPTED_KEYS */
+
+static struct ecryptfs_auth_tok *
+ecryptfs_get_key_payload_data(struct key *key)
+{
+	struct ecryptfs_auth_tok *auth_tok;
+	struct user_key_payload *ukp;
+
+	auth_tok = ecryptfs_get_encrypted_key_payload_data(key);
+	if (auth_tok)
+		return auth_tok;
+
+	ukp = user_key_payload_locked(key);
+	if (!ukp)
+		return ERR_PTR(-EKEYREVOKED);
+
+	if (ukp->datalen != sizeof(struct ecryptfs_auth_tok))
+		return ERR_PTR(-EINVAL);
+
+	return (struct ecryptfs_auth_tok *)ukp->data;
+}
+
 /**
  * ecryptfs_verify_auth_tok_from_key
  * @auth_tok_key: key containing the authentication token
-- 
2.14.2.822.g60be5d43e6-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] KEYS: encrypted: fix dereference of NULL user_key_payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> A key of type "encrypted" references a "master key" which is used to
> encrypt and decrypt the encrypted key's payload.  However, when we
> accessed the master key's payload, we failed to handle the case where
> the master key has been revoked, which sets the payload pointer to NULL.
> Note that request_key() *does* skip revoked keys, but there is still a
> window where the key can be revoked before we acquire its semaphore.
> 
> Fix it by checking for a NULL payload, treating it like a key which was
> already revoked at the time it was requested.
> 
> This was an issue for master keys of type "user" only.  Master keys can
> also be of type "trusted", but those cannot be revoked.
> 
> Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
> Cc: <stable@vger.kernel.org>    [v2.6.38+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/7] FS-Cache: fix dereference of NULL user_key_payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> When the file /proc/fs/fscache/objects (available with
> CONFIG_FSCACHE_OBJECT_LIST=y) is opened, we request a user key with
> description "fscache:objlist", then access its payload.  However, a
> revoked key has a NULL payload, and we failed to check for this.
> request_key() *does* skip revoked keys, but there is still a window
> where the key can be revoked before we access its payload.
> 
> Fix it by checking for a NULL payload, treating it like a key which was
> already revoked at the time it was requested.
> 
> Fixes: 4fbf4291aa15 ("FS-Cache: Allow the current state of all objects to be dumped")
> Cc: <stable@vger.kernel.org>    [v2.6.32+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/7] lib/digsig: fix dereference of NULL user_key_payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> digsig_verify() requests a user key, then accesses its payload.
> However, a revoked key has a NULL payload, and we failed to check for
> this.  request_key() *does* skip revoked keys, but there is still a
> window where the key can be revoked before we acquire its semaphore.
> 
> Fix it by checking for a NULL payload, treating it like a key which was
> already revoked at the time it was requested.
> 
> Fixes: 051dbb918c7f ("crypto: digital signature verification support")
> Cc: <stable@vger.kernel.org>    [v3.3+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Reviewed-by: James Morris <james.l.morris@oracle.com>

-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/7] fscrypt: fix dereference of NULL user_key_payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> When an fscrypt-encrypted file is opened, we request the file's master
> key from the keyrings service as a logon key, then access its payload.
> However, a revoked key has a NULL payload, and we failed to check for
> this.  request_key() *does* skip revoked keys, but there is still a
> window where the key can be revoked before we acquire its semaphore.
> 
> Fix it by checking for a NULL payload, treating it like a key which was
> already revoked at the time it was requested.
> 
> Fixes: 88bd6ccdcdd6 ("ext4 crypto: add encryption key management facilities")
> Cc: <stable@vger.kernel.org>    [v4.1+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/7] ecryptfs: fix dereference of NULL user_key_payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> In eCryptfs, we failed to verify that the authentication token keys are
> not revoked before dereferencing their payloads, which is problematic
> because the payload of a revoked key is NULL.  request_key() *does* skip
> revoked keys, but there is still a window where the key can be revoked
> before we acquire the key semaphore.
> 
> Fix it by updating ecryptfs_get_key_payload_data() to return
> -EKEYREVOKED if the key payload is NULL.  For completeness we check this
> for "encrypted" keys as well as "user" keys, although encrypted keys
> cannot be revoked currently.
> 
> Alternatively we could use key_validate(), but since we'll also need to
> fix ecryptfs_get_key_payload_data() to validate the payload length, it
> seems appropriate to just check the payload pointer.
> 
> Fixes: 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
> Cc: <stable@vger.kernel.org>    [v2.6.19+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>

(A further cleanup might add some inline accessor functions for key data, 
but it's not necessary now).

Reviewed-by: James Morris <james.l.morris@oracle.com>

-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/7] ecryptfs: fix out-of-bounds read of key payload

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> eCryptfs blindly casts the user-supplied key payload to a
> 'struct ecryptfs_auth_tok' without validating that the payload does, in
> fact, have the size of a 'struct ecryptfs_auth_tok'.  Fix it.
> 
> Fixes: 237fead61998 ("[PATCH] ecryptfs: fs/Makefile and fs/Kconfig")
> Cc: <stable@vger.kernel.org>    [v2.6.19+]
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/7] ecryptfs: move key payload accessor functions into keystore.c

[James Morris]

On Thu, 28 Sep 2017, Eric Biggers wrote:

> From: Eric Biggers <ebiggers@google.com>
> 
> As experience has shown, accessing the 'struct key' payload is very
> error-prone, since we need to hold the key semaphore and properly
> validate everything.  Fortunately eCryptfs only does it from one place,
> in ecryptfs_verify_auth_tok_from_key() in keystore.c.  Therefore, move
> the payload accessor functions like ecryptfs_get_key_payload_data() out
> of ecryptfs_kernel.h and into keystore.c so that people might be less
> tempted to use them directly.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/ecryptfs/ecryptfs_kernel.h | 60 -------------------------------------------
>  fs/ecryptfs/keystore.c        | 60 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 60 insertions(+), 60 deletions(-)

Ok, I should have started at this patch :)

Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html