f2fs: check entire encrypted bigname when finding a dentry
Subsystems (3)
PATCH 1/2 f2fs: check entire encrypted bigname when finding a dentry
From: Jaegeuk Kim <[email protected]>
commit 6332cd32c8290a80e929fc044dc5bdba77396e33 upstream. Please apply to 4.4-stable.
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
(fixed f2fs_dentry_hash() to work even when the hash is 0)
--- fs/f2fs/dir.c | 32 +++++++++++++++++++++----------- fs/f2fs/f2fs.h | 3 ++- fs/f2fs/hash.c | 7 ++++++- fs/f2fs/inline.c | 4 ++-- 4 files changed, 31 insertions(+), 15 deletions(-)
fs/f2fs/dir.c +21 -11
@@ -124,19 +124,29 @@ struct f2fs_dir_entry *find_target_dentry(struct f2fs_filename *fname, de = &d->dentry[bit_pos]; -/* 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 == 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,7 +180,7 @@ static struct f2fs_dir_entry *find_in_level(struct inode *dir,int max_slots;f2fs_hash_t namehash; - namehash =f2fs_dentry_hash (&name); + namehash =f2fs_dentry_hash (&name, fname);f2fs_bug_on (F2FS_I_SB (dir), level >MAX_DIR_HASH_DEPTH );
@@ -547,7 +557,7 @@ int __f2fs_add_link(struct inode *dir, const struct qstr *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) {
fs/f2fs/f2fs.h +2 -1
@@ -1722,7 +1722,8 @@ void f2fs_msg(struct super_block *, const char *, const char *, ...);/* * hash.c */ -f2fs_hash_t f2fs_dentry_hash (const struct qstr *); +f2fs_hash_t f2fs_dentry_hash (const struct qstr *name_info, +struct f2fs_filename *fname);/* * node.c
fs/f2fs/hash.c +6 -1
@@ -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 f2fs_filename *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 ;
fs/f2fs/inline.c +2 -2
@@ -303,7 +303,7 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode *dir,if (IS_ERR (ipage))return NULL ; - namehash =f2fs_dentry_hash (&name); + namehash =f2fs_dentry_hash (&name, fname); inline_dentry =inline_data_addr (ipage);
@@ -468,7 +468,7 @@ int f2fs_add_inline_entry(struct inode *dir, const struct qstr *name,f2fs_wait_on_page_writeback (ipage,NODE ); - name_hash =f2fs_dentry_hash (name); + name_hash =f2fs_dentry_hash (name,NULL );make_dentry_ptr (NULL , &d, (void *)dentry_blk,2 );f2fs_update_dentry (ino, mode, &d, name, name_hash, bit_pos);
Signature · 1 line
2.13.0.303.g4ebf302169-goog
PATCH 2/2 fscrypt: avoid collisions when presenting long encrypted filenames
From: Eric Biggers <[email protected]>
commit 6b06cdee81d68a8a829ad8e8d0f31d6836744af9 upstream. Please apply to 4.4-stable.
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.
--- fs/ext4/crypto_fname.c | 2 +- fs/ext4/namei.c | 4 ++-- fs/f2fs/crypto_fname.c | 2 +- fs/f2fs/dir.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-)
fs/ext4/crypto_fname.c +1 -1
@@ -343,7 +343,7 @@ int _ext4_fname_disk_to_usr(struct inode *inode,memcpy (buf+4 , &hinfo->minor_hash,4 ); }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 ] ='_' ; ret =digest_encode (buf,24 , oname->name+1 ); oname->len = ret +1 ;
fs/ext4/namei.c +2 -2
@@ -1243,9 +1243,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 ; }
fs/f2fs/crypto_fname.c +1 -1
@@ -333,7 +333,7 @@ int f2fs_fname_disk_to_usr(struct inode *inode,memset (buf +4 ,0 ,4 ); }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 ] ='_' ; ret =digest_encode (buf,24 , oname->name +1 ); oname->len = ret +1 ;
fs/f2fs/dir.c +2 -2
@@ -133,8 +133,8 @@ struct f2fs_dir_entry *find_target_dentry(struct f2fs_filename *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;
Signature · 1 line
2.13.0.303.g4ebf302169-goog