All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: linux-fscrypt@vger.kernel.org
Cc: Satya Tangirala <satyat@google.com>,
	linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org,
	Paul Crowley <paulcrowley@google.com>
Subject: [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy
Date: Mon, 20 May 2019 10:25:48 -0700	[thread overview]
Message-ID: <20190520172552.217253-13-ebiggers@kernel.org> (raw)
In-Reply-To: <20190520172552.217253-1-ebiggers@kernel.org>

From: Eric Biggers <ebiggers@google.com>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key.  Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/keyring.c         | 47 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          |  6 +++++
 3 files changed, 56 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
 fscrypt_find_master_key(struct super_block *sb,
 			const struct fscrypt_key_specifier *mk_spec);
 
+extern int fscrypt_verify_key_added(struct super_block *sb,
+				    const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
 extern int __init fscrypt_init_keyring(void);
 extern void fscrypt_exit_keyring(void);
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0a92f2998b437..92eadda419278 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
 
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not).  This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+	struct fscrypt_key_specifier mk_spec;
+	struct key *key, *mk_user;
+	struct fscrypt_master_key *mk;
+	int err;
+
+	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+	key = fscrypt_find_master_key(sb, &mk_spec);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out;
+	}
+	mk = key->payload.data[0];
+	mk_user = find_master_key_user(mk);
+	if (IS_ERR(mk_user)) {
+		err = PTR_ERR(mk_user);
+	} else {
+		key_put(mk_user);
+		err = 0;
+	}
+	key_put(key);
+out:
+	if (err == -ENOKEY && capable(CAP_FOWNER))
+		err = 0;
+	return err;
+}
+
 static void shrink_dcache_inode(struct inode *inode)
 {
 	struct dentry *dentry;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3643156a511a7..8fb9f5ac5f8c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -211,6 +211,7 @@ static int set_encryption_policy(struct inode *inode,
 {
 	union fscrypt_context ctx;
 	int ctxsize;
+	int err;
 
 	if (!fscrypt_supported_policy(policy))
 		return -EINVAL;
@@ -229,6 +230,11 @@ static int set_encryption_policy(struct inode *inode,
 		 */
 		pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
+	} else {
+		err = fscrypt_verify_key_added(inode->i_sb,
+					       policy->v2.master_key_identifier);
+		if (err)
+			return err;
 	}
 
 	ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
-- 
2.21.0.1020.gf2820cf01a-goog


WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-fscrypt@vger.kernel.org
Cc: linux-ext4@vger.kernel.org, linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Satya Tangirala <satyat@google.com>,
	Paul Crowley <paulcrowley@google.com>
Subject: [f2fs-dev] [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy
Date: Mon, 20 May 2019 10:25:48 -0700	[thread overview]
Message-ID: <20190520172552.217253-13-ebiggers@kernel.org> (raw)
In-Reply-To: <20190520172552.217253-1-ebiggers@kernel.org>

From: Eric Biggers <ebiggers@google.com>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key.  Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/keyring.c         | 47 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          |  6 +++++
 3 files changed, 56 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
 fscrypt_find_master_key(struct super_block *sb,
 			const struct fscrypt_key_specifier *mk_spec);
 
+extern int fscrypt_verify_key_added(struct super_block *sb,
+				    const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
 extern int __init fscrypt_init_keyring(void);
 extern void fscrypt_exit_keyring(void);
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0a92f2998b437..92eadda419278 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
 
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not).  This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+	struct fscrypt_key_specifier mk_spec;
+	struct key *key, *mk_user;
+	struct fscrypt_master_key *mk;
+	int err;
+
+	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+	key = fscrypt_find_master_key(sb, &mk_spec);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out;
+	}
+	mk = key->payload.data[0];
+	mk_user = find_master_key_user(mk);
+	if (IS_ERR(mk_user)) {
+		err = PTR_ERR(mk_user);
+	} else {
+		key_put(mk_user);
+		err = 0;
+	}
+	key_put(key);
+out:
+	if (err == -ENOKEY && capable(CAP_FOWNER))
+		err = 0;
+	return err;
+}
+
 static void shrink_dcache_inode(struct inode *inode)
 {
 	struct dentry *dentry;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3643156a511a7..8fb9f5ac5f8c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -211,6 +211,7 @@ static int set_encryption_policy(struct inode *inode,
 {
 	union fscrypt_context ctx;
 	int ctxsize;
+	int err;
 
 	if (!fscrypt_supported_policy(policy))
 		return -EINVAL;
@@ -229,6 +230,11 @@ static int set_encryption_policy(struct inode *inode,
 		 */
 		pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
+	} else {
+		err = fscrypt_verify_key_added(inode->i_sb,
+					       policy->v2.master_key_identifier);
+		if (err)
+			return err;
 	}
 
 	ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
-- 
2.21.0.1020.gf2820cf01a-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-fscrypt@vger.kernel.org
Cc: linux-ext4@vger.kernel.org, linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Satya Tangirala <satyat@google.com>,
	Paul Crowley <paulcrowley@google.com>
Subject: [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy
Date: Mon, 20 May 2019 17:25:48 +0000	[thread overview]
Message-ID: <20190520172552.217253-13-ebiggers@kernel.org> (raw)
In-Reply-To: <20190520172552.217253-1-ebiggers@kernel.org>

From: Eric Biggers <ebiggers@google.com>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key.  Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/keyring.c         | 47 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          |  6 +++++
 3 files changed, 56 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
 fscrypt_find_master_key(struct super_block *sb,
 			const struct fscrypt_key_specifier *mk_spec);
 
+extern int fscrypt_verify_key_added(struct super_block *sb,
+				    const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
 extern int __init fscrypt_init_keyring(void);
 extern void fscrypt_exit_keyring(void);
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0a92f2998b437..92eadda419278 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
 
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not).  This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+	struct fscrypt_key_specifier mk_spec;
+	struct key *key, *mk_user;
+	struct fscrypt_master_key *mk;
+	int err;
+
+	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+	key = fscrypt_find_master_key(sb, &mk_spec);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out;
+	}
+	mk = key->payload.data[0];
+	mk_user = find_master_key_user(mk);
+	if (IS_ERR(mk_user)) {
+		err = PTR_ERR(mk_user);
+	} else {
+		key_put(mk_user);
+		err = 0;
+	}
+	key_put(key);
+out:
+	if (err = -ENOKEY && capable(CAP_FOWNER))
+		err = 0;
+	return err;
+}
+
 static void shrink_dcache_inode(struct inode *inode)
 {
 	struct dentry *dentry;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3643156a511a7..8fb9f5ac5f8c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -211,6 +211,7 @@ static int set_encryption_policy(struct inode *inode,
 {
 	union fscrypt_context ctx;
 	int ctxsize;
+	int err;
 
 	if (!fscrypt_supported_policy(policy))
 		return -EINVAL;
@@ -229,6 +230,11 @@ static int set_encryption_policy(struct inode *inode,
 		 */
 		pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
+	} else {
+		err = fscrypt_verify_key_added(inode->i_sb,
+					       policy->v2.master_key_identifier);
+		if (err)
+			return err;
 	}
 
 	ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
-- 
2.21.0.1020.gf2820cf01a-goog

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-fscrypt@vger.kernel.org
Cc: linux-ext4@vger.kernel.org, linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Satya Tangirala <satyat@google.com>,
	Paul Crowley <paulcrowley@google.com>
Subject: [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy
Date: Mon, 20 May 2019 10:25:48 -0700	[thread overview]
Message-ID: <20190520172552.217253-13-ebiggers@kernel.org> (raw)
In-Reply-To: <20190520172552.217253-1-ebiggers@kernel.org>

From: Eric Biggers <ebiggers@google.com>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key.  Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/keyring.c         | 47 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          |  6 +++++
 3 files changed, 56 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
 fscrypt_find_master_key(struct super_block *sb,
 			const struct fscrypt_key_specifier *mk_spec);
 
+extern int fscrypt_verify_key_added(struct super_block *sb,
+				    const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
 extern int __init fscrypt_init_keyring(void);
 extern void fscrypt_exit_keyring(void);
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0a92f2998b437..92eadda419278 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
 
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not).  This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+	struct fscrypt_key_specifier mk_spec;
+	struct key *key, *mk_user;
+	struct fscrypt_master_key *mk;
+	int err;
+
+	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+	key = fscrypt_find_master_key(sb, &mk_spec);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out;
+	}
+	mk = key->payload.data[0];
+	mk_user = find_master_key_user(mk);
+	if (IS_ERR(mk_user)) {
+		err = PTR_ERR(mk_user);
+	} else {
+		key_put(mk_user);
+		err = 0;
+	}
+	key_put(key);
+out:
+	if (err == -ENOKEY && capable(CAP_FOWNER))
+		err = 0;
+	return err;
+}
+
 static void shrink_dcache_inode(struct inode *inode)
 {
 	struct dentry *dentry;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3643156a511a7..8fb9f5ac5f8c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -211,6 +211,7 @@ static int set_encryption_policy(struct inode *inode,
 {
 	union fscrypt_context ctx;
 	int ctxsize;
+	int err;
 
 	if (!fscrypt_supported_policy(policy))
 		return -EINVAL;
@@ -229,6 +230,11 @@ static int set_encryption_policy(struct inode *inode,
 		 */
 		pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
+	} else {
+		err = fscrypt_verify_key_added(inode->i_sb,
+					       policy->v2.master_key_identifier);
+		if (err)
+			return err;
 	}
 
 	ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
-- 
2.21.0.1020.gf2820cf01a-goog

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: linux-fscrypt@vger.kernel.org
Cc: linux-ext4@vger.kernel.org, linux-api@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net, keyrings@vger.kernel.org,
	linux-mtd@lists.infradead.org, linux-crypto@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Satya Tangirala <satyat@google.com>,
	Paul Crowley <paulcrowley@google.com>
Subject: [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy
Date: Mon, 20 May 2019 10:25:48 -0700	[thread overview]
Message-ID: <20190520172552.217253-13-ebiggers@kernel.org> (raw)
In-Reply-To: <20190520172552.217253-1-ebiggers@kernel.org>

From: Eric Biggers <ebiggers@google.com>

By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key.  Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).

We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h |  3 +++
 fs/crypto/keyring.c         | 47 +++++++++++++++++++++++++++++++++++++
 fs/crypto/policy.c          |  6 +++++
 3 files changed, 56 insertions(+)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
 fscrypt_find_master_key(struct super_block *sb,
 			const struct fscrypt_key_specifier *mk_spec);
 
+extern int fscrypt_verify_key_added(struct super_block *sb,
+				    const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
 extern int __init fscrypt_init_keyring(void);
 extern void fscrypt_exit_keyring(void);
 
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0a92f2998b437..92eadda419278 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
 }
 EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
 
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not).  This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+			     const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+	struct fscrypt_key_specifier mk_spec;
+	struct key *key, *mk_user;
+	struct fscrypt_master_key *mk;
+	int err;
+
+	mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+	memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+	key = fscrypt_find_master_key(sb, &mk_spec);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out;
+	}
+	mk = key->payload.data[0];
+	mk_user = find_master_key_user(mk);
+	if (IS_ERR(mk_user)) {
+		err = PTR_ERR(mk_user);
+	} else {
+		key_put(mk_user);
+		err = 0;
+	}
+	key_put(key);
+out:
+	if (err == -ENOKEY && capable(CAP_FOWNER))
+		err = 0;
+	return err;
+}
+
 static void shrink_dcache_inode(struct inode *inode)
 {
 	struct dentry *dentry;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 3643156a511a7..8fb9f5ac5f8c0 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -211,6 +211,7 @@ static int set_encryption_policy(struct inode *inode,
 {
 	union fscrypt_context ctx;
 	int ctxsize;
+	int err;
 
 	if (!fscrypt_supported_policy(policy))
 		return -EINVAL;
@@ -229,6 +230,11 @@ static int set_encryption_policy(struct inode *inode,
 		 */
 		pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
 			     current->comm, current->pid);
+	} else {
+		err = fscrypt_verify_key_added(inode->i_sb,
+					       policy->v2.master_key_identifier);
+		if (err)
+			return err;
 	}
 
 	ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
-- 
2.21.0.1020.gf2820cf01a-goog


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  parent reply	other threads:[~2019-05-20 17:29 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-20 17:25 [PATCH v6 00/16] fscrypt: key management improvements Eric Biggers
2019-05-20 17:25 ` Eric Biggers
2019-05-20 17:25 ` Eric Biggers
2019-05-20 17:25 ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 01/16] fs, fscrypt: move uapi definitions to new header <linux/fscrypt.h> Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 02/16] fscrypt: use FSCRYPT_ prefix for uapi constants Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 03/16] fscrypt: use FSCRYPT_* definitions, not FS_* Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 04/16] fscrypt: add ->ci_inode to fscrypt_info Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 05/16] fscrypt: refactor v1 policy key setup into keysetup_legacy.c Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 06/16] fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 07/16] fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 08/16] fscrypt: add FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 09/16] fscrypt: add an HKDF-SHA512 implementation Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 10/16] fscrypt: v2 encryption policy support Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 11/16] fscrypt: allow unprivileged users to add/remove keys for v2 policies Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` Eric Biggers [this message]
2019-05-20 17:25   ` [PATCH v6 12/16] fscrypt: require that key be added when setting a v2 encryption policy Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 13/16] ext4: wire up new fscrypt ioctls Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 14/16] f2fs: " Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 15/16] ubifs: " Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-20 17:25 ` [PATCH v6 16/16] fscrypt: document the new ioctls and policy version Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` Eric Biggers
2019-05-20 17:25   ` [f2fs-dev] " Eric Biggers
2019-05-21  0:16 ` [PATCH v6 00/16] fscrypt: key management improvements Theodore Ts'o
2019-05-21  0:16   ` Theodore Ts'o
2019-05-21  0:16   ` Theodore Ts'o
2019-05-21  0:16   ` Theodore Ts'o
2019-05-21  0:41   ` Eric Biggers
2019-05-21  0:41     ` Eric Biggers
2019-05-21  0:41     ` Eric Biggers
2019-05-21  3:29     ` Theodore Ts'o
2019-05-21  3:29       ` Theodore Ts'o
2019-05-21  3:29       ` Theodore Ts'o
2019-05-21  3:29       ` Theodore Ts'o

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=20190520172552.217253-13-ebiggers@kernel.org \
    --to=ebiggers@kernel.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=paulcrowley@google.com \
    --cc=satyat@google.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.