All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers3@gmail.com>
To: ecryptfs@vger.kernel.org, Tyler Hicks <tyhicks@canonical.com>
Cc: keyrings@vger.kernel.org, linux-security-module@vger.kernel.org,
	Eric Biggers <ebiggers@google.com>,
	Michael Halcrow <mhalcrow@google.com>
Subject: [PATCH v2 3/3] ecryptfs: move key payload accessor functions into keystore.c
Date: Mon,  9 Oct 2017 12:51:29 -0700	[thread overview]
Message-ID: <20171009195129.68610-3-ebiggers3@gmail.com> (raw)
In-Reply-To: <20171009195129.68610-1-ebiggers3@gmail.com>

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.

Reviewed-by: James Morris <james.l.morris@oracle.com>
Cc: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

Changed since v1: added Reviewed-by and resent in series with just the
ecryptfs patches.  Can this please be taken through the ecryptfs tree?

 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.920.gcf0c67979c-goog


WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers3@gmail.com>
To: linux-security-module@vger.kernel.org
Subject: [PATCH v2 3/3] ecryptfs: move key payload accessor functions into keystore.c
Date: Mon, 09 Oct 2017 19:51:29 +0000	[thread overview]
Message-ID: <20171009195129.68610-3-ebiggers3@gmail.com> (raw)
In-Reply-To: <20171009195129.68610-1-ebiggers3@gmail.com>

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.

Reviewed-by: James Morris <james.l.morris@oracle.com>
Cc: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

Changed since v1: added Reviewed-by and resent in series with just the
ecryptfs patches.  Can this please be taken through the ecryptfs tree?

 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.920.gcf0c67979c-goog


WARNING: multiple messages have this Message-ID (diff)
From: ebiggers3@gmail.com (Eric Biggers)
To: linux-security-module@vger.kernel.org
Subject: [PATCH v2 3/3] ecryptfs: move key payload accessor functions into keystore.c
Date: Mon,  9 Oct 2017 12:51:29 -0700	[thread overview]
Message-ID: <20171009195129.68610-3-ebiggers3@gmail.com> (raw)
In-Reply-To: <20171009195129.68610-1-ebiggers3@gmail.com>

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.

Reviewed-by: James Morris <james.l.morris@oracle.com>
Cc: Michael Halcrow <mhalcrow@google.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

Changed since v1: added Reviewed-by and resent in series with just the
ecryptfs patches.  Can this please be taken through the ecryptfs tree?

 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.920.gcf0c67979c-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

  parent reply	other threads:[~2017-10-09 19:51 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-09 19:51 [PATCH v2 1/3] ecryptfs: fix dereference of NULL user_key_payload Eric Biggers
2017-10-09 19:51 ` Eric Biggers
2017-10-09 19:51 ` Eric Biggers
2017-10-09 19:51 ` [PATCH v2 2/3] ecryptfs: fix out-of-bounds read of key payload Eric Biggers
2017-10-09 19:51   ` Eric Biggers
2017-10-09 19:51   ` Eric Biggers
2017-10-09 19:51 ` Eric Biggers [this message]
2017-10-09 19:51   ` [PATCH v2 3/3] ecryptfs: move key payload accessor functions into keystore.c Eric Biggers
2017-10-09 19:51   ` Eric Biggers

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=20171009195129.68610-3-ebiggers3@gmail.com \
    --to=ebiggers3@gmail.com \
    --cc=ebiggers@google.com \
    --cc=ecryptfs@vger.kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mhalcrow@google.com \
    --cc=tyhicks@canonical.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.