Linux-FSCrypt Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name"
@ 2020-09-24  4:26 Eric Biggers
  2020-09-24  4:26 ` [PATCH 1/2] fscrypt: don't call no-key names "ciphertext names" Eric Biggers
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Biggers @ 2020-09-24  4:26 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-f2fs-devel, Daniel Rosenberg, Jeff Layton

This series fixes overloading of the terms "ciphertext name" and
"encrypted name" to also sometimes mean "no-key name".
The overloading of these terms has caused some confusion.

No change in behavior.

Eric Biggers (2):
  fscrypt: don't call no-key names "ciphertext names"
  fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME

 fs/crypto/fname.c       | 16 ++++++++--------
 fs/crypto/hooks.c       | 13 ++++++-------
 fs/f2fs/dir.c           |  2 +-
 include/linux/dcache.h  |  2 +-
 include/linux/fscrypt.h | 25 ++++++++++++-------------
 5 files changed, 28 insertions(+), 30 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] fscrypt: don't call no-key names "ciphertext names"
  2020-09-24  4:26 [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
@ 2020-09-24  4:26 ` Eric Biggers
  2020-09-24  4:26 ` [PATCH 2/2] fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME Eric Biggers
  2020-09-28 21:55 ` [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2020-09-24  4:26 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-f2fs-devel, Daniel Rosenberg, Jeff Layton

From: Eric Biggers <ebiggers@google.com>

Currently we're using the term "ciphertext name" ambiguously because it
can mean either the actual ciphertext filename, or the encoded filename
that is shown when an encrypted directory is listed without its key.
The latter we're now usually calling the "no-key name"; and while it's
derived from the ciphertext name, it's not the same thing.

To avoid this ambiguity, rename fscrypt_name::is_ciphertext_name to
fscrypt_name::is_nokey_name, and update comments that say "ciphertext
name" (or "encrypted name") to say "no-key name" instead when warranted.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fname.c       | 16 ++++++++--------
 fs/crypto/hooks.c       |  6 +++---
 fs/f2fs/dir.c           |  2 +-
 include/linux/fscrypt.h | 15 +++++++--------
 4 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index f47b581d8a94..391acea4bc96 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -382,9 +382,9 @@ EXPORT_SYMBOL(fscrypt_fname_disk_to_usr);
  * directory's encryption key, then @iname is the plaintext, so we encrypt it to
  * get the disk_name.
  *
- * Else, for keyless @lookup operations, @iname is the presented ciphertext, so
- * we decode it to get the fscrypt_nokey_name.  Non-@lookup operations will be
- * impossible in this case, so we fail them with ENOKEY.
+ * Else, for keyless @lookup operations, @iname should be a no-key name, so we
+ * decode it to get the struct fscrypt_nokey_name.  Non-@lookup operations will
+ * be impossible in this case, so we fail them with ENOKEY.
  *
  * If successful, fscrypt_free_filename() must be called later to clean up.
  *
@@ -429,7 +429,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 	}
 	if (!lookup)
 		return -ENOKEY;
-	fname->is_ciphertext_name = true;
+	fname->is_nokey_name = true;
 
 	/*
 	 * We don't have the key and we are doing a lookup; decode the
@@ -538,17 +538,17 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 
 	/*
 	 * Plaintext names are always valid, since fscrypt doesn't support
-	 * reverting to ciphertext names without evicting the directory's inode
+	 * reverting to no-key names without evicting the directory's inode
 	 * -- which implies eviction of the dentries in the directory.
 	 */
 	if (!(dentry->d_flags & DCACHE_ENCRYPTED_NAME))
 		return 1;
 
 	/*
-	 * Ciphertext name; valid if the directory's key is still unavailable.
+	 * No-key name; valid if the directory's key is still unavailable.
 	 *
-	 * Although fscrypt forbids rename() on ciphertext names, we still must
-	 * use dget_parent() here rather than use ->d_parent directly.  That's
+	 * Although fscrypt forbids rename() on no-key names, we still must use
+	 * dget_parent() here rather than use ->d_parent directly.  That's
 	 * because a corrupted fs image may contain directory hard links, which
 	 * the VFS handles by moving the directory's dentry tree in the dcache
 	 * each time ->lookup() finds the directory and it already has a dentry
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 42f5ee9f592d..ca996e1c92d9 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -60,7 +60,7 @@ int __fscrypt_prepare_link(struct inode *inode, struct inode *dir,
 	if (err)
 		return err;
 
-	/* ... in case we looked up ciphertext name before key was added */
+	/* ... in case we looked up no-key name before key was added */
 	if (dentry->d_flags & DCACHE_ENCRYPTED_NAME)
 		return -ENOKEY;
 
@@ -85,7 +85,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (err)
 		return err;
 
-	/* ... in case we looked up ciphertext name(s) before key was added */
+	/* ... in case we looked up no-key name(s) before key was added */
 	if ((old_dentry->d_flags | new_dentry->d_flags) &
 	    DCACHE_ENCRYPTED_NAME)
 		return -ENOKEY;
@@ -114,7 +114,7 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 	if (err && err != -ENOENT)
 		return err;
 
-	if (fname->is_ciphertext_name) {
+	if (fname->is_nokey_name) {
 		spin_lock(&dentry->d_lock);
 		dentry->d_flags |= DCACHE_ENCRYPTED_NAME;
 		spin_unlock(&dentry->d_lock);
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 414bc94fbd54..53fbc4dd6e48 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -111,7 +111,7 @@ static int __f2fs_setup_filename(const struct inode *dir,
 #ifdef CONFIG_FS_ENCRYPTION
 	fname->crypto_buf = crypt_name->crypto_buf;
 #endif
-	if (crypt_name->is_ciphertext_name) {
+	if (crypt_name->is_nokey_name) {
 		/* hash was decoded from the no-key name */
 		fname->hash = cpu_to_le32(crypt_name->hash);
 	} else {
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index fc67c4cbaa96..bc9ec727e993 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -35,7 +35,7 @@ struct fscrypt_name {
 	u32 hash;
 	u32 minor_hash;
 	struct fscrypt_str crypto_buf;
-	bool is_ciphertext_name;
+	bool is_nokey_name;
 };
 
 #define FSTR_INIT(n, l)		{ .name = n, .len = l }
@@ -730,17 +730,16 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
  * @fname: (output) the name to use to search the on-disk directory
  *
  * Prepare for ->lookup() in a directory which may be encrypted by determining
- * the name that will actually be used to search the directory on-disk.  Lookups
- * can be done with or without the directory's encryption key; without the key,
- * filenames are presented in encrypted form.  Therefore, we'll try to set up
- * the directory's encryption key, but even without it the lookup can continue.
+ * the name that will actually be used to search the directory on-disk.  If the
+ * directory's encryption key is available, then the lookup is assumed to be by
+ * plaintext name; otherwise, it is assumed to be by no-key name.
  *
  * This also installs a custom ->d_revalidate() method which will invalidate the
  * dentry if it was created without the key and the key is later added.
  *
- * Return: 0 on success; -ENOENT if key is unavailable but the filename isn't a
- * correctly formed encoded ciphertext name, so a negative dentry should be
- * created; or another -errno code.
+ * Return: 0 on success; -ENOENT if the directory's key is unavailable but the
+ * filename isn't a valid no-key name, so a negative dentry should be created;
+ * or another -errno code.
  */
 static inline int fscrypt_prepare_lookup(struct inode *dir,
 					 struct dentry *dentry,
-- 
2.28.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME
  2020-09-24  4:26 [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
  2020-09-24  4:26 ` [PATCH 1/2] fscrypt: don't call no-key names "ciphertext names" Eric Biggers
@ 2020-09-24  4:26 ` Eric Biggers
  2020-09-28 21:55 ` [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2020-09-24  4:26 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-f2fs-devel, Daniel Rosenberg, Jeff Layton

From: Eric Biggers <ebiggers@google.com>

Originally we used the term "encrypted name" or "ciphertext name" to
mean the encoded filename that is shown when an encrypted directory is
listed without its key.  But these terms are ambiguous since they also
mean the filename stored on-disk.  "Encrypted name" is especially
ambiguous since it could also be understood to mean "this filename is
encrypted on-disk", similar to "encrypted file".

So we've started calling these encoded names "no-key names" instead.

Therefore, rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME to avoid
confusion about what this flag means.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fname.c       |  2 +-
 fs/crypto/hooks.c       |  7 +++----
 include/linux/dcache.h  |  2 +-
 include/linux/fscrypt.h | 12 ++++++------
 4 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 391acea4bc96..c65979452844 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -541,7 +541,7 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 	 * reverting to no-key names without evicting the directory's inode
 	 * -- which implies eviction of the dentries in the directory.
 	 */
-	if (!(dentry->d_flags & DCACHE_ENCRYPTED_NAME))
+	if (!(dentry->d_flags & DCACHE_NOKEY_NAME))
 		return 1;
 
 	/*
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index ca996e1c92d9..20b0df47fe6a 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -61,7 +61,7 @@ int __fscrypt_prepare_link(struct inode *inode, struct inode *dir,
 		return err;
 
 	/* ... in case we looked up no-key name before key was added */
-	if (dentry->d_flags & DCACHE_ENCRYPTED_NAME)
+	if (dentry->d_flags & DCACHE_NOKEY_NAME)
 		return -ENOKEY;
 
 	if (!fscrypt_has_permitted_context(dir, inode))
@@ -86,8 +86,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
 		return err;
 
 	/* ... in case we looked up no-key name(s) before key was added */
-	if ((old_dentry->d_flags | new_dentry->d_flags) &
-	    DCACHE_ENCRYPTED_NAME)
+	if ((old_dentry->d_flags | new_dentry->d_flags) & DCACHE_NOKEY_NAME)
 		return -ENOKEY;
 
 	if (old_dir != new_dir) {
@@ -116,7 +115,7 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 
 	if (fname->is_nokey_name) {
 		spin_lock(&dentry->d_lock);
-		dentry->d_flags |= DCACHE_ENCRYPTED_NAME;
+		dentry->d_flags |= DCACHE_NOKEY_NAME;
 		spin_unlock(&dentry->d_lock);
 		d_set_d_op(dentry, &fscrypt_d_ops);
 	}
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 65d975bf9390..6f95c3300cbb 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -213,7 +213,7 @@ struct dentry_operations {
 
 #define DCACHE_MAY_FREE			0x00800000
 #define DCACHE_FALLTHRU			0x01000000 /* Fall through to lower layer */
-#define DCACHE_ENCRYPTED_NAME		0x02000000 /* Encrypted name (dir key was unavailable) */
+#define DCACHE_NOKEY_NAME		0x02000000 /* Encrypted name encoded without key */
 #define DCACHE_OP_REAL			0x04000000
 
 #define DCACHE_PAR_LOOKUP		0x10000000 /* being looked up (with parent locked shared) */
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index bc9ec727e993..f1757e73162d 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -100,15 +100,15 @@ static inline bool fscrypt_needs_contents_encryption(const struct inode *inode)
 }
 
 /*
- * When d_splice_alias() moves a directory's encrypted alias to its decrypted
- * alias as a result of the encryption key being added, DCACHE_ENCRYPTED_NAME
- * must be cleared.  Note that we don't have to support arbitrary moves of this
- * flag because fscrypt doesn't allow encrypted aliases to be the source or
- * target of a rename().
+ * When d_splice_alias() moves a directory's no-key alias to its plaintext alias
+ * as a result of the encryption key being added, DCACHE_NOKEY_NAME must be
+ * cleared.  Note that we don't have to support arbitrary moves of this flag
+ * because fscrypt doesn't allow no-key names to be the source or target of a
+ * rename().
  */
 static inline void fscrypt_handle_d_move(struct dentry *dentry)
 {
-	dentry->d_flags &= ~DCACHE_ENCRYPTED_NAME;
+	dentry->d_flags &= ~DCACHE_NOKEY_NAME;
 }
 
 /* crypto.c */
-- 
2.28.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name"
  2020-09-24  4:26 [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
  2020-09-24  4:26 ` [PATCH 1/2] fscrypt: don't call no-key names "ciphertext names" Eric Biggers
  2020-09-24  4:26 ` [PATCH 2/2] fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME Eric Biggers
@ 2020-09-28 21:55 ` Eric Biggers
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Biggers @ 2020-09-28 21:55 UTC (permalink / raw)
  To: linux-fscrypt
  Cc: linux-fsdevel, linux-f2fs-devel, Daniel Rosenberg, Jeff Layton

On Wed, Sep 23, 2020 at 09:26:22PM -0700, Eric Biggers wrote:
> This series fixes overloading of the terms "ciphertext name" and
> "encrypted name" to also sometimes mean "no-key name".
> The overloading of these terms has caused some confusion.
> 
> No change in behavior.
> 
> Eric Biggers (2):
>   fscrypt: don't call no-key names "ciphertext names"
>   fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME
> 
>  fs/crypto/fname.c       | 16 ++++++++--------
>  fs/crypto/hooks.c       | 13 ++++++-------
>  fs/f2fs/dir.c           |  2 +-
>  include/linux/dcache.h  |  2 +-
>  include/linux/fscrypt.h | 25 ++++++++++++-------------
>  5 files changed, 28 insertions(+), 30 deletions(-)

Applied to fscrypt.git#master for 5.10.

- Eric

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-24  4:26 [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers
2020-09-24  4:26 ` [PATCH 1/2] fscrypt: don't call no-key names "ciphertext names" Eric Biggers
2020-09-24  4:26 ` [PATCH 2/2] fscrypt: rename DCACHE_ENCRYPTED_NAME to DCACHE_NOKEY_NAME Eric Biggers
2020-09-28 21:55 ` [PATCH 0/2] fscrypt: avoid ambiguous terms for "no-key name" Eric Biggers

Linux-FSCrypt Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fscrypt/0 linux-fscrypt/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fscrypt linux-fscrypt/ https://lore.kernel.org/linux-fscrypt \
		linux-fscrypt@vger.kernel.org
	public-inbox-index linux-fscrypt

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fscrypt


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git