* [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry
[not found] <20170424170013.85175-1-ebiggers3@gmail.com>
@ 2017-04-24 17:00 ` Eric Biggers
2017-04-25 0:10 ` Jaegeuk Kim
2017-04-30 6:19 ` [1/6] " Theodore Ts'o
2017-04-24 17:00 ` [PATCH 2/6] fscrypt: avoid collisions when presenting long encrypted filenames Eric Biggers
1 sibling, 2 replies; 7+ messages in thread
From: Eric Biggers @ 2017-04-24 17:00 UTC (permalink / raw)
To: linux-fscrypt
Cc: Theodore Y . Ts'o, Jaegeuk Kim, linux-f2fs-devel, linux-ext4,
linux-mtd, Gwendal Grignou, hashimoto, kinaba, stable,
Eric Biggers
From: Jaegeuk Kim <jaegeuk@kernel.org>
If user has no key under an encrypted dir, fscrypt gives digested dentries.
Previously, when looking up a dentry, f2fs only checks its hash value with
first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
This patch enhances to check entire dentry bytes likewise ext4.
Eric reported how to reproduce this issue by:
# seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
# find edir -type f | xargs stat -c %i | sort | uniq | wc -l
100000
# sync
# echo 3 > /proc/sys/vm/drop_caches
# keyctl new_session
# find edir -type f | xargs stat -c %i | sort | uniq | wc -l
99999
Cc: <stable@vger.kernel.org>
Reported-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
(fixed f2fs_dentry_hash() to work even when the hash is 0)
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/f2fs/dir.c | 37 +++++++++++++++++++++----------------
fs/f2fs/f2fs.h | 3 ++-
fs/f2fs/hash.c | 7 ++++++-
fs/f2fs/inline.c | 4 ++--
4 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index c143dffcae6e..374e4b8f9b70 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -130,19 +130,29 @@ struct f2fs_dir_entry *find_target_dentry(struct fscrypt_name *fname,
continue;
}
- /* encrypted case */
+ if (de->hash_code != namehash)
+ goto not_match;
+
de_name.name = d->filename[bit_pos];
de_name.len = le16_to_cpu(de->name_len);
- /* show encrypted name */
- if (fname->hash) {
- if (de->hash_code == cpu_to_le32(fname->hash))
- goto found;
- } else if (de_name.len == name->len &&
- de->hash_code == namehash &&
- !memcmp(de_name.name, name->name, name->len))
+#ifdef CONFIG_F2FS_FS_ENCRYPTION
+ if (unlikely(!name->name)) {
+ if (fname->usr_fname->name[0] == '_') {
+ if (de_name.len >= 16 &&
+ !memcmp(de_name.name + de_name.len - 16,
+ fname->crypto_buf.name + 8, 16))
+ goto found;
+ goto not_match;
+ }
+ name->name = fname->crypto_buf.name;
+ name->len = fname->crypto_buf.len;
+ }
+#endif
+ if (de_name.len == name->len &&
+ !memcmp(de_name.name, name->name, name->len))
goto found;
-
+not_match:
if (max_slots && max_len > *max_slots)
*max_slots = max_len;
max_len = 0;
@@ -170,12 +180,7 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
struct f2fs_dir_entry *de = NULL;
bool room = false;
int max_slots;
- f2fs_hash_t namehash;
-
- if(fname->hash)
- namehash = cpu_to_le32(fname->hash);
- else
- namehash = f2fs_dentry_hash(&name);
+ f2fs_hash_t namehash = f2fs_dentry_hash(&name, fname);
nbucket = dir_buckets(level, F2FS_I(dir)->i_dir_level);
nblock = bucket_blocks(level);
@@ -527,7 +532,7 @@ int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name,
level = 0;
slots = GET_DENTRY_SLOTS(new_name->len);
- dentry_hash = f2fs_dentry_hash(new_name);
+ dentry_hash = f2fs_dentry_hash(new_name, NULL);
current_depth = F2FS_I(dir)->i_current_depth;
if (F2FS_I(dir)->chash == dentry_hash) {
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 562db8989a4e..5bc232e21a6e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2145,7 +2145,8 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi);
/*
* hash.c
*/
-f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info);
+f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
+ struct fscrypt_name *fname);
/*
* node.c
diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
index 71b7206c431e..eb2e031ea887 100644
--- a/fs/f2fs/hash.c
+++ b/fs/f2fs/hash.c
@@ -70,7 +70,8 @@ static void str2hashbuf(const unsigned char *msg, size_t len,
*buf++ = pad;
}
-f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
+f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
+ struct fscrypt_name *fname)
{
__u32 hash;
f2fs_hash_t f2fs_hash;
@@ -79,6 +80,10 @@ f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
const unsigned char *name = name_info->name;
size_t len = name_info->len;
+ /* encrypted bigname case */
+ if (fname && !fname->disk_name.name)
+ return cpu_to_le32(fname->hash);
+
if (is_dot_dotdot(name_info))
return 0;
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0ccdefe9fdba..e4c527c4e7d0 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -298,7 +298,7 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
return NULL;
}
- namehash = f2fs_dentry_hash(&name);
+ namehash = f2fs_dentry_hash(&name, fname);
inline_dentry = inline_data_addr(ipage);
@@ -533,7 +533,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
f2fs_wait_on_page_writeback(ipage, NODE, true);
- name_hash = f2fs_dentry_hash(new_name);
+ name_hash = f2fs_dentry_hash(new_name, NULL);
make_dentry_ptr_inline(NULL, &d, dentry_blk);
f2fs_update_dentry(ino, mode, &d, new_name, name_hash, bit_pos);
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/6] fscrypt: avoid collisions when presenting long encrypted filenames
[not found] <20170424170013.85175-1-ebiggers3@gmail.com>
2017-04-24 17:00 ` [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry Eric Biggers
@ 2017-04-24 17:00 ` Eric Biggers
2017-04-30 6:19 ` [2/6] " Theodore Ts'o
1 sibling, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2017-04-24 17:00 UTC (permalink / raw)
To: linux-fscrypt
Cc: Theodore Y . Ts'o, Jaegeuk Kim, linux-f2fs-devel, linux-ext4,
linux-mtd, Gwendal Grignou, hashimoto, kinaba, Eric Biggers,
stable
From: Eric Biggers <ebiggers@google.com>
When accessing an encrypted directory without the key, userspace must
operate on filenames derived from the ciphertext names, which contain
arbitrary bytes. Since we must support filenames as long as NAME_MAX,
we can't always just base64-encode the ciphertext, since that may make
it too long. Currently, this is solved by presenting long names in an
abbreviated form containing any needed filesystem-specific hashes (e.g.
to identify a directory block), then the last 16 bytes of ciphertext.
This needs to be sufficient to identify the actual name on lookup.
However, there is a bug. It seems to have been assumed that due to the
use of a CBC (ciphertext block chaining)-based encryption mode, the last
16 bytes (i.e. the AES block size) of ciphertext would depend on the
full plaintext, preventing collisions. However, we actually use CBC
with ciphertext stealing (CTS), which handles the last two blocks
specially, causing them to appear "flipped". Thus, it's actually the
second-to-last block which depends on the full plaintext.
This caused long filenames that differ only near the end of their
plaintexts to, when observed without the key, point to the wrong inode
and be undeletable. For example, with ext4:
# echo pass | e4crypt add_key -p 16 edir/
# seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
# find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
100000
# sync
# echo 3 > /proc/sys/vm/drop_caches
# keyctl new_session
# find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
2004
# rm -rf edir/
rm: cannot remove 'edir/_A7nNFi3rhkEQlJ6P,hdzluhODKOeWx5V': Structure needs cleaning
...
To fix this, when presenting long encrypted filenames, encode the
second-to-last block of ciphertext rather than the last 16 bytes.
Although it would be nice to solve this without depending on a specific
encryption mode, that would mean doing a cryptographic hash like SHA-256
which would be much less efficient. This way is sufficient for now, and
it's still compatible with encryption modes like HEH which are strong
pseudorandom permutations. Also, changing the presented names is still
allowed at any time because they are only provided to allow applications
to do things like delete encrypted directories. They're not designed to
be used to persistently identify files --- which would be hard to do
anyway, given that they're encrypted after all.
For ease of backports, this patch only makes the minimal fix to both
ext4 and f2fs. It leaves ubifs as-is, since ubifs doesn't compare the
ciphertext block yet. Follow-on patches will clean things up properly
and make the filesystems use a shared helper function.
Fixes: 5de0b4d0cd15 ("ext4 crypto: simplify and speed up filename encryption")
Reported-by: Gwendal Grignou <gwendal@chromium.org>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/fname.c | 2 +-
fs/ext4/namei.c | 4 ++--
fs/f2fs/dir.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 13052b85c393..932881f27f2f 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -300,7 +300,7 @@ int fscrypt_fname_disk_to_usr(struct inode *inode,
} else {
memset(buf, 0, 8);
}
- memcpy(buf + 8, iname->name + iname->len - 16, 16);
+ memcpy(buf + 8, iname->name + ((iname->len - 17) & ~15), 16);
oname->name[0] = '_';
oname->len = 1 + digest_encode(buf, 24, oname->name + 1);
return 0;
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 6ad612c576fc..e6301b6933fc 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1255,9 +1255,9 @@ static inline int ext4_match(struct ext4_filename *fname,
if (unlikely(!name)) {
if (fname->usr_fname->name[0] == '_') {
int ret;
- if (de->name_len < 16)
+ if (de->name_len <= 32)
return 0;
- ret = memcmp(de->name + de->name_len - 16,
+ ret = memcmp(de->name + ((de->name_len - 17) & ~15),
fname->crypto_buf.name + 8, 16);
return (ret == 0) ? 1 : 0;
}
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 374e4b8f9b70..5df3596a667a 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -139,8 +139,8 @@ struct f2fs_dir_entry *find_target_dentry(struct fscrypt_name *fname,
#ifdef CONFIG_F2FS_FS_ENCRYPTION
if (unlikely(!name->name)) {
if (fname->usr_fname->name[0] == '_') {
- if (de_name.len >= 16 &&
- !memcmp(de_name.name + de_name.len - 16,
+ if (de_name.len > 32 &&
+ !memcmp(de_name.name + ((de_name.len - 17) & ~15),
fname->crypto_buf.name + 8, 16))
goto found;
goto not_match;
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry
2017-04-24 17:00 ` [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry Eric Biggers
@ 2017-04-25 0:10 ` Jaegeuk Kim
2017-05-03 2:56 ` Eric Biggers
2017-04-30 6:19 ` [1/6] " Theodore Ts'o
1 sibling, 1 reply; 7+ messages in thread
From: Jaegeuk Kim @ 2017-04-25 0:10 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, Theodore Y . Ts'o, linux-f2fs-devel,
linux-ext4, linux-mtd, Gwendal Grignou, hashimoto, kinaba, stable,
Eric Biggers
Hi Eric,
This looks good to me.
I'll drop it from my tree, so please move forward through fscrypt.
Thanks,
On 04/24, Eric Biggers wrote:
> From: Jaegeuk Kim <jaegeuk@kernel.org>
>
> If user has no key under an encrypted dir, fscrypt gives digested dentries.
> Previously, when looking up a dentry, f2fs only checks its hash value with
> first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
> This patch enhances to check entire dentry bytes likewise ext4.
>
> Eric reported how to reproduce this issue by:
>
> # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
> # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
> # sync
> # echo 3 > /proc/sys/vm/drop_caches
> # keyctl new_session
> # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 99999
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> (fixed f2fs_dentry_hash() to work even when the hash is 0)
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/f2fs/dir.c | 37 +++++++++++++++++++++----------------
> fs/f2fs/f2fs.h | 3 ++-
> fs/f2fs/hash.c | 7 ++++++-
> fs/f2fs/inline.c | 4 ++--
> 4 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index c143dffcae6e..374e4b8f9b70 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -130,19 +130,29 @@ struct f2fs_dir_entry *find_target_dentry(struct fscrypt_name *fname,
> continue;
> }
>
> - /* encrypted case */
> + if (de->hash_code != namehash)
> + goto not_match;
> +
> de_name.name = d->filename[bit_pos];
> de_name.len = le16_to_cpu(de->name_len);
>
> - /* show encrypted name */
> - if (fname->hash) {
> - if (de->hash_code == cpu_to_le32(fname->hash))
> - goto found;
> - } else if (de_name.len == name->len &&
> - de->hash_code == namehash &&
> - !memcmp(de_name.name, name->name, name->len))
> +#ifdef CONFIG_F2FS_FS_ENCRYPTION
> + if (unlikely(!name->name)) {
> + if (fname->usr_fname->name[0] == '_') {
> + if (de_name.len >= 16 &&
> + !memcmp(de_name.name + de_name.len - 16,
> + fname->crypto_buf.name + 8, 16))
> + goto found;
> + goto not_match;
> + }
> + name->name = fname->crypto_buf.name;
> + name->len = fname->crypto_buf.len;
> + }
> +#endif
> + if (de_name.len == name->len &&
> + !memcmp(de_name.name, name->name, name->len))
> goto found;
> -
> +not_match:
> if (max_slots && max_len > *max_slots)
> *max_slots = max_len;
> max_len = 0;
> @@ -170,12 +180,7 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,
> struct f2fs_dir_entry *de = NULL;
> bool room = false;
> int max_slots;
> - f2fs_hash_t namehash;
> -
> - if(fname->hash)
> - namehash = cpu_to_le32(fname->hash);
> - else
> - namehash = f2fs_dentry_hash(&name);
> + f2fs_hash_t namehash = f2fs_dentry_hash(&name, fname);
>
> nbucket = dir_buckets(level, F2FS_I(dir)->i_dir_level);
> nblock = bucket_blocks(level);
> @@ -527,7 +532,7 @@ int f2fs_add_regular_entry(struct inode *dir, const struct qstr *new_name,
>
> level = 0;
> slots = GET_DENTRY_SLOTS(new_name->len);
> - dentry_hash = f2fs_dentry_hash(new_name);
> + dentry_hash = f2fs_dentry_hash(new_name, NULL);
>
> current_depth = F2FS_I(dir)->i_current_depth;
> if (F2FS_I(dir)->chash == dentry_hash) {
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 562db8989a4e..5bc232e21a6e 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2145,7 +2145,8 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi);
> /*
> * hash.c
> */
> -f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info);
> +f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> + struct fscrypt_name *fname);
>
> /*
> * node.c
> diff --git a/fs/f2fs/hash.c b/fs/f2fs/hash.c
> index 71b7206c431e..eb2e031ea887 100644
> --- a/fs/f2fs/hash.c
> +++ b/fs/f2fs/hash.c
> @@ -70,7 +70,8 @@ static void str2hashbuf(const unsigned char *msg, size_t len,
> *buf++ = pad;
> }
>
> -f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
> +f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info,
> + struct fscrypt_name *fname)
> {
> __u32 hash;
> f2fs_hash_t f2fs_hash;
> @@ -79,6 +80,10 @@ f2fs_hash_t f2fs_dentry_hash(const struct qstr *name_info)
> const unsigned char *name = name_info->name;
> size_t len = name_info->len;
>
> + /* encrypted bigname case */
> + if (fname && !fname->disk_name.name)
> + return cpu_to_le32(fname->hash);
> +
> if (is_dot_dotdot(name_info))
> return 0;
>
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 0ccdefe9fdba..e4c527c4e7d0 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -298,7 +298,7 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,
> return NULL;
> }
>
> - namehash = f2fs_dentry_hash(&name);
> + namehash = f2fs_dentry_hash(&name, fname);
>
> inline_dentry = inline_data_addr(ipage);
>
> @@ -533,7 +533,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *new_name,
>
> f2fs_wait_on_page_writeback(ipage, NODE, true);
>
> - name_hash = f2fs_dentry_hash(new_name);
> + name_hash = f2fs_dentry_hash(new_name, NULL);
> make_dentry_ptr_inline(NULL, &d, dentry_blk);
> f2fs_update_dentry(ino, mode, &d, new_name, name_hash, bit_pos);
>
> --
> 2.12.2.816.g2cccc81164-goog
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [1/6] f2fs: check entire encrypted bigname when finding a dentry
2017-04-24 17:00 ` [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry Eric Biggers
2017-04-25 0:10 ` Jaegeuk Kim
@ 2017-04-30 6:19 ` Theodore Ts'o
1 sibling, 0 replies; 7+ messages in thread
From: Theodore Ts'o @ 2017-04-30 6:19 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, Jaegeuk Kim, linux-f2fs-devel, linux-ext4,
linux-mtd, Gwendal Grignou, hashimoto, kinaba, stable,
Eric Biggers
On Mon, Apr 24, 2017 at 10:00:08AM -0700, Eric Biggers wrote:
> From: Jaegeuk Kim <jaegeuk@kernel.org>
>
> If user has no key under an encrypted dir, fscrypt gives digested dentries.
> Previously, when looking up a dentry, f2fs only checks its hash value with
> first 4 bytes of the digested dentry, which didn't handle hash collisions fully.
> This patch enhances to check entire dentry bytes likewise ext4.
>
> Eric reported how to reproduce this issue by:
>
> # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
> # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
> # sync
> # echo 3 > /proc/sys/vm/drop_caches
> # keyctl new_session
> # find edir -type f | xargs stat -c %i | sort | uniq | wc -l
> 99999
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> (fixed f2fs_dentry_hash() to work even when the hash is 0)
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Thanks, applied to the fscrypt tree.
- Ted
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [2/6] fscrypt: avoid collisions when presenting long encrypted filenames
2017-04-24 17:00 ` [PATCH 2/6] fscrypt: avoid collisions when presenting long encrypted filenames Eric Biggers
@ 2017-04-30 6:19 ` Theodore Ts'o
0 siblings, 0 replies; 7+ messages in thread
From: Theodore Ts'o @ 2017-04-30 6:19 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, Jaegeuk Kim, linux-f2fs-devel, linux-ext4,
linux-mtd, Gwendal Grignou, hashimoto, kinaba, Eric Biggers,
stable
On Mon, Apr 24, 2017 at 10:00:09AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> When accessing an encrypted directory without the key, userspace must
> operate on filenames derived from the ciphertext names, which contain
> arbitrary bytes. Since we must support filenames as long as NAME_MAX,
> we can't always just base64-encode the ciphertext, since that may make
> it too long. Currently, this is solved by presenting long names in an
> abbreviated form containing any needed filesystem-specific hashes (e.g.
> to identify a directory block), then the last 16 bytes of ciphertext.
> This needs to be sufficient to identify the actual name on lookup.
>
> However, there is a bug. It seems to have been assumed that due to the
> use of a CBC (ciphertext block chaining)-based encryption mode, the last
> 16 bytes (i.e. the AES block size) of ciphertext would depend on the
> full plaintext, preventing collisions. However, we actually use CBC
> with ciphertext stealing (CTS), which handles the last two blocks
> specially, causing them to appear "flipped". Thus, it's actually the
> second-to-last block which depends on the full plaintext.
>
> This caused long filenames that differ only near the end of their
> plaintexts to, when observed without the key, point to the wrong inode
> and be undeletable. For example, with ext4:
>
> # echo pass | e4crypt add_key -p 16 edir/
> # seq -f "edir/abcdefghijklmnopqrstuvwxyz012345%.0f" 100000 | xargs touch
> # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
> 100000
> # sync
> # echo 3 > /proc/sys/vm/drop_caches
> # keyctl new_session
> # find edir/ -type f | xargs stat -c %i | sort | uniq | wc -l
> 2004
> # rm -rf edir/
> rm: cannot remove 'edir/_A7nNFi3rhkEQlJ6P,hdzluhODKOeWx5V': Structure needs cleaning
> ...
>
> To fix this, when presenting long encrypted filenames, encode the
> second-to-last block of ciphertext rather than the last 16 bytes.
>
> Although it would be nice to solve this without depending on a specific
> encryption mode, that would mean doing a cryptographic hash like SHA-256
> which would be much less efficient. This way is sufficient for now, and
> it's still compatible with encryption modes like HEH which are strong
> pseudorandom permutations. Also, changing the presented names is still
> allowed at any time because they are only provided to allow applications
> to do things like delete encrypted directories. They're not designed to
> be used to persistently identify files --- which would be hard to do
> anyway, given that they're encrypted after all.
>
> For ease of backports, this patch only makes the minimal fix to both
> ext4 and f2fs. It leaves ubifs as-is, since ubifs doesn't compare the
> ciphertext block yet. Follow-on patches will clean things up properly
> and make the filesystems use a shared helper function.
>
> Fixes: 5de0b4d0cd15 ("ext4 crypto: simplify and speed up filename encryption")
> Reported-by: Gwendal Grignou <gwendal@chromium.org>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Thanks, applied.
- Ted
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry
2017-04-25 0:10 ` Jaegeuk Kim
@ 2017-05-03 2:56 ` Eric Biggers
2017-05-03 4:21 ` Jaegeuk Kim
0 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2017-05-03 2:56 UTC (permalink / raw)
To: Jaegeuk Kim
Cc: linux-fscrypt, Theodore Y . Ts'o, linux-f2fs-devel,
linux-ext4, linux-mtd, Gwendal Grignou, hashimoto, kinaba, stable,
Eric Biggers
Hi Jaegeuk,
On Mon, Apr 24, 2017 at 05:10:23PM -0700, Jaegeuk Kim wrote:
> Hi Eric,
>
> This looks good to me.
> I'll drop it from my tree, so please move forward through fscrypt.
>
> Thanks,
This is in fscrypt/master now (along with the other patches in the series), but
it's also in f2fs/dev. Are you still planning to drop it from the f2fs tree?
- Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry
2017-05-03 2:56 ` Eric Biggers
@ 2017-05-03 4:21 ` Jaegeuk Kim
0 siblings, 0 replies; 7+ messages in thread
From: Jaegeuk Kim @ 2017-05-03 4:21 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-fscrypt, Theodore Y . Ts'o, linux-f2fs-devel,
linux-ext4, linux-mtd, Gwendal Grignou, hashimoto, kinaba, stable,
Eric Biggers
Hi Eric,
On 05/02, Eric Biggers wrote:
> Hi Jaegeuk,
>
> On Mon, Apr 24, 2017 at 05:10:23PM -0700, Jaegeuk Kim wrote:
> > Hi Eric,
> >
> > This looks good to me.
> > I'll drop it from my tree, so please move forward through fscrypt.
> >
> > Thanks,
>
> This is in fscrypt/master now (along with the other patches in the series), but
> it's also in f2fs/dev. Are you still planning to drop it from the f2fs tree?
Oh, yup. I dropped it.
Thank you for pointing this out.
Thanks,
>
> - Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-05-03 4:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20170424170013.85175-1-ebiggers3@gmail.com>
2017-04-24 17:00 ` [PATCH 1/6] f2fs: check entire encrypted bigname when finding a dentry Eric Biggers
2017-04-25 0:10 ` Jaegeuk Kim
2017-05-03 2:56 ` Eric Biggers
2017-05-03 4:21 ` Jaegeuk Kim
2017-04-30 6:19 ` [1/6] " Theodore Ts'o
2017-04-24 17:00 ` [PATCH 2/6] fscrypt: avoid collisions when presenting long encrypted filenames Eric Biggers
2017-04-30 6:19 ` [2/6] " Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).