| From foo@baz Sun Jun 17 12:07:33 CEST 2018 |
| From: Tyler Hicks <tyhicks@canonical.com> |
| Date: Wed, 28 Mar 2018 23:41:52 +0000 |
| Subject: eCryptfs: don't pass up plaintext names when using filename encryption |
| |
| From: Tyler Hicks <tyhicks@canonical.com> |
| |
| [ Upstream commit e86281e700cca8a773f9a572fa406adf2784ba5c ] |
| |
| Both ecryptfs_filldir() and ecryptfs_readlink_lower() use |
| ecryptfs_decode_and_decrypt_filename() to translate lower filenames to |
| upper filenames. The function correctly passes up lower filenames, |
| unchanged, when filename encryption isn't in use. However, it was also |
| passing up lower filenames when the filename wasn't encrypted or |
| when decryption failed. Since 88ae4ab9802e, eCryptfs refuses to lookup |
| lower plaintext names when filename encryption is enabled so this |
| resulted in a situation where userspace would see lower plaintext |
| filenames in calls to getdents(2) but then not be able to lookup those |
| filenames. |
| |
| An example of this can be seen when enabling filename encryption on an |
| eCryptfs mount at the root directory of an Ext4 filesystem: |
| |
| $ ls -1i /lower |
| 12 ECRYPTFS_FNEK_ENCRYPTED.FWYZD8TcW.5FV-TKTEYOHsheiHX9a-w.NURCCYIMjI8pn5BDB9-h3fXwrE-- |
| 11 lost+found |
| $ ls -1i /upper |
| ls: cannot access '/upper/lost+found': No such file or directory |
| ? lost+found |
| 12 test |
| |
| With this change, the lower lost+found dentry is ignored: |
| |
| $ ls -1i /lower |
| 12 ECRYPTFS_FNEK_ENCRYPTED.FWYZD8TcW.5FV-TKTEYOHsheiHX9a-w.NURCCYIMjI8pn5BDB9-h3fXwrE-- |
| 11 lost+found |
| $ ls -1i /upper |
| 12 test |
| |
| Additionally, some potentially noisy error/info messages in the related |
| code paths are turned into debug messages so that the logs can't be |
| easily filled. |
| |
| Fixes: 88ae4ab9802e ("ecryptfs_lookup(): try either only encrypted or plaintext name") |
| Reported-by: Guenter Roeck <linux@roeck-us.net> |
| Cc: Al Viro <viro@zeniv.linux.org.uk> |
| Signed-off-by: Tyler Hicks <tyhicks@canonical.com> |
| Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| fs/ecryptfs/crypto.c | 41 ++++++++++++++++++++++++++++------------- |
| fs/ecryptfs/file.c | 21 ++++++++++++++++----- |
| 2 files changed, 44 insertions(+), 18 deletions(-) |
| |
| --- a/fs/ecryptfs/crypto.c |
| +++ b/fs/ecryptfs/crypto.c |
| @@ -1997,6 +1997,16 @@ out: |
| return rc; |
| } |
| |
| +static bool is_dot_dotdot(const char *name, size_t name_size) |
| +{ |
| + if (name_size == 1 && name[0] == '.') |
| + return true; |
| + else if (name_size == 2 && name[0] == '.' && name[1] == '.') |
| + return true; |
| + |
| + return false; |
| +} |
| + |
| /** |
| * ecryptfs_decode_and_decrypt_filename - converts the encoded cipher text name to decoded plaintext |
| * @plaintext_name: The plaintext name |
| @@ -2021,13 +2031,21 @@ int ecryptfs_decode_and_decrypt_filename |
| size_t packet_size; |
| int rc = 0; |
| |
| - if ((mount_crypt_stat->flags & ECRYPTFS_GLOBAL_ENCRYPT_FILENAMES) |
| - && !(mount_crypt_stat->flags & ECRYPTFS_ENCRYPTED_VIEW_ENABLED) |
| - && (name_size > ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE) |
| - && (strncmp(name, ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX, |
| - ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE) == 0)) { |
| - const char *orig_name = name; |
| - size_t orig_name_size = name_size; |
| + if ((mount_crypt_stat->flags & ECRYPTFS_GLOBAL_ENCRYPT_FILENAMES) && |
| + !(mount_crypt_stat->flags & ECRYPTFS_ENCRYPTED_VIEW_ENABLED)) { |
| + if (is_dot_dotdot(name, name_size)) { |
| + rc = ecryptfs_copy_filename(plaintext_name, |
| + plaintext_name_size, |
| + name, name_size); |
| + goto out; |
| + } |
| + |
| + if (name_size <= ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE || |
| + strncmp(name, ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX, |
| + ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE)) { |
| + rc = -EINVAL; |
| + goto out; |
| + } |
| |
| name += ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE; |
| name_size -= ECRYPTFS_FNEK_ENCRYPTED_FILENAME_PREFIX_SIZE; |
| @@ -2047,12 +2065,9 @@ int ecryptfs_decode_and_decrypt_filename |
| decoded_name, |
| decoded_name_size); |
| if (rc) { |
| - printk(KERN_INFO "%s: Could not parse tag 70 packet " |
| - "from filename; copying through filename " |
| - "as-is\n", __func__); |
| - rc = ecryptfs_copy_filename(plaintext_name, |
| - plaintext_name_size, |
| - orig_name, orig_name_size); |
| + ecryptfs_printk(KERN_DEBUG, |
| + "%s: Could not parse tag 70 packet from filename\n", |
| + __func__); |
| goto out_free; |
| } |
| } else { |
| --- a/fs/ecryptfs/file.c |
| +++ b/fs/ecryptfs/file.c |
| @@ -82,17 +82,28 @@ ecryptfs_filldir(struct dir_context *ctx |
| buf->sb, lower_name, |
| lower_namelen); |
| if (rc) { |
| - printk(KERN_ERR "%s: Error attempting to decode and decrypt " |
| - "filename [%s]; rc = [%d]\n", __func__, lower_name, |
| - rc); |
| - goto out; |
| + if (rc != -EINVAL) { |
| + ecryptfs_printk(KERN_DEBUG, |
| + "%s: Error attempting to decode and decrypt filename [%s]; rc = [%d]\n", |
| + __func__, lower_name, rc); |
| + return rc; |
| + } |
| + |
| + /* Mask -EINVAL errors as these are most likely due a plaintext |
| + * filename present in the lower filesystem despite filename |
| + * encryption being enabled. One unavoidable example would be |
| + * the "lost+found" dentry in the root directory of an Ext4 |
| + * filesystem. |
| + */ |
| + return 0; |
| } |
| + |
| buf->caller->pos = buf->ctx.pos; |
| rc = !dir_emit(buf->caller, name, name_size, ino, d_type); |
| kfree(name); |
| if (!rc) |
| buf->entries_written++; |
| -out: |
| + |
| return rc; |
| } |
| |