This is a note to let you know that I've just added the patch titled fscrypt: fix context consistency check when key(s) unavailable to the 4.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: fscrypt-fix-context-consistency-check-when-key-s-unavailable.patch and it can be found in the queue-4.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From 272f98f6846277378e1758a49a49d7bf39343c02 Mon Sep 17 00:00:00 2001 From: Eric Biggers <ebiggers@xxxxxxxxxx> Date: Fri, 7 Apr 2017 10:58:37 -0700 Subject: fscrypt: fix context consistency check when key(s) unavailable From: Eric Biggers <ebiggers@xxxxxxxxxx> commit 272f98f6846277378e1758a49a49d7bf39343c02 upstream. To mitigate some types of offline attacks, filesystem encryption is designed to enforce that all files in an encrypted directory tree use the same encryption policy (i.e. the same encryption context excluding the nonce). However, the fscrypt_has_permitted_context() function which enforces this relies on comparing struct fscrypt_info's, which are only available when we have the encryption keys. This can cause two incorrect behaviors: 1. If we have the parent directory's key but not the child's key, or vice versa, then fscrypt_has_permitted_context() returned false, causing applications to see EPERM or ENOKEY. This is incorrect if the encryption contexts are in fact consistent. Although we'd normally have either both keys or neither key in that case since the master_key_descriptors would be the same, this is not guaranteed because keys can be added or removed from keyrings at any time. 2. If we have neither the parent's key nor the child's key, then fscrypt_has_permitted_context() returned true, causing applications to see no error (or else an error for some other reason). This is incorrect if the encryption contexts are in fact inconsistent, since in that case we should deny access. To fix this, retrieve and compare the fscrypt_contexts if we are unable to set up both fscrypt_infos. While this slightly hurts performance when accessing an encrypted directory tree without the key, this isn't a case we really need to be optimizing for; access *with* the key is much more important. Furthermore, the performance hit is barely noticeable given that we are already retrieving the fscrypt_context and doing two keyring searches in fscrypt_get_encryption_info(). If we ever actually wanted to optimize this case we might start by caching the fscrypt_contexts. Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx> Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- fs/ext4/crypto_policy.c | 66 ++++++++++++++++++++++++++++++++++-------------- fs/f2fs/crypto_policy.c | 65 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 96 insertions(+), 35 deletions(-) --- a/fs/ext4/crypto_policy.c +++ b/fs/ext4/crypto_policy.c @@ -148,26 +148,38 @@ int ext4_get_policy(struct inode *inode, int ext4_is_child_context_consistent_with_parent(struct inode *parent, struct inode *child) { - struct ext4_crypt_info *parent_ci, *child_ci; + const struct ext4_crypt_info *parent_ci, *child_ci; + struct ext4_encryption_context parent_ctx, child_ctx; int res; - if ((parent == NULL) || (child == NULL)) { - pr_err("parent %p child %p\n", parent, child); - WARN_ON(1); /* Should never happen */ - return 0; - } - /* No restrictions on file types which are never encrypted */ if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) && !S_ISLNK(child->i_mode)) return 1; - /* no restrictions if the parent directory is not encrypted */ + /* No restrictions if the parent directory is unencrypted */ if (!ext4_encrypted_inode(parent)) return 1; - /* if the child directory is not encrypted, this is always a problem */ + + /* Encrypted directories must not contain unencrypted files */ if (!ext4_encrypted_inode(child)) return 0; + + /* + * Both parent and child are encrypted, so verify they use the same + * encryption policy. Compare the fscrypt_info structs if the keys are + * available, otherwise retrieve and compare the fscrypt_contexts. + * + * Note that the fscrypt_context retrieval will be required frequently + * when accessing an encrypted directory tree without the key. + * Performance-wise this is not a big deal because we already don't + * really optimize for file access without the key (to the extent that + * such access is even possible), given that any attempted access + * already causes a fscrypt_context retrieval and keyring search. + * + * In any case, if an unexpected error occurs, fall back to "forbidden". + */ + res = ext4_get_encryption_info(parent); if (res) return 0; @@ -176,17 +188,35 @@ int ext4_is_child_context_consistent_wit return 0; parent_ci = EXT4_I(parent)->i_crypt_info; child_ci = EXT4_I(child)->i_crypt_info; - if (!parent_ci && !child_ci) - return 1; - if (!parent_ci || !child_ci) + if (parent_ci && child_ci) { + return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key, + EXT4_KEY_DESCRIPTOR_SIZE) == 0 && + (parent_ci->ci_data_mode == child_ci->ci_data_mode) && + (parent_ci->ci_filename_mode == + child_ci->ci_filename_mode) && + (parent_ci->ci_flags == child_ci->ci_flags); + } + + res = ext4_xattr_get(parent, EXT4_XATTR_INDEX_ENCRYPTION, + EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, + &parent_ctx, sizeof(parent_ctx)); + if (res != sizeof(parent_ctx)) + return 0; + + res = ext4_xattr_get(child, EXT4_XATTR_INDEX_ENCRYPTION, + EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, + &child_ctx, sizeof(child_ctx)); + if (res != sizeof(child_ctx)) return 0; - return (memcmp(parent_ci->ci_master_key, - child_ci->ci_master_key, - EXT4_KEY_DESCRIPTOR_SIZE) == 0 && - (parent_ci->ci_data_mode == child_ci->ci_data_mode) && - (parent_ci->ci_filename_mode == child_ci->ci_filename_mode) && - (parent_ci->ci_flags == child_ci->ci_flags)); + return memcmp(parent_ctx.master_key_descriptor, + child_ctx.master_key_descriptor, + EXT4_KEY_DESCRIPTOR_SIZE) == 0 && + (parent_ctx.contents_encryption_mode == + child_ctx.contents_encryption_mode) && + (parent_ctx.filenames_encryption_mode == + child_ctx.filenames_encryption_mode) && + (parent_ctx.flags == child_ctx.flags); } /** --- a/fs/f2fs/crypto_policy.c +++ b/fs/f2fs/crypto_policy.c @@ -141,25 +141,38 @@ int f2fs_get_policy(struct inode *inode, int f2fs_is_child_context_consistent_with_parent(struct inode *parent, struct inode *child) { - struct f2fs_crypt_info *parent_ci, *child_ci; + const struct f2fs_crypt_info *parent_ci, *child_ci; + struct f2fs_encryption_context parent_ctx, child_ctx; int res; - if ((parent == NULL) || (child == NULL)) { - pr_err("parent %p child %p\n", parent, child); - BUG_ON(1); - } - /* No restrictions on file types which are never encrypted */ if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) && !S_ISLNK(child->i_mode)) return 1; - /* no restrictions if the parent directory is not encrypted */ + /* No restrictions if the parent directory is unencrypted */ if (!f2fs_encrypted_inode(parent)) return 1; - /* if the child directory is not encrypted, this is always a problem */ + + /* Encrypted directories must not contain unencrypted files */ if (!f2fs_encrypted_inode(child)) return 0; + + /* + * Both parent and child are encrypted, so verify they use the same + * encryption policy. Compare the fscrypt_info structs if the keys are + * available, otherwise retrieve and compare the fscrypt_contexts. + * + * Note that the fscrypt_context retrieval will be required frequently + * when accessing an encrypted directory tree without the key. + * Performance-wise this is not a big deal because we already don't + * really optimize for file access without the key (to the extent that + * such access is even possible), given that any attempted access + * already causes a fscrypt_context retrieval and keyring search. + * + * In any case, if an unexpected error occurs, fall back to "forbidden". + */ + res = f2fs_get_encryption_info(parent); if (res) return 0; @@ -168,17 +181,35 @@ int f2fs_is_child_context_consistent_wit return 0; parent_ci = F2FS_I(parent)->i_crypt_info; child_ci = F2FS_I(child)->i_crypt_info; - if (!parent_ci && !child_ci) - return 1; - if (!parent_ci || !child_ci) + if (parent_ci && child_ci) { + return memcmp(parent_ci->ci_master_key, child_ci->ci_master_key, + F2FS_KEY_DESCRIPTOR_SIZE) == 0 && + (parent_ci->ci_data_mode == child_ci->ci_data_mode) && + (parent_ci->ci_filename_mode == + child_ci->ci_filename_mode) && + (parent_ci->ci_flags == child_ci->ci_flags); + } + + res = f2fs_getxattr(parent, F2FS_XATTR_INDEX_ENCRYPTION, + F2FS_XATTR_NAME_ENCRYPTION_CONTEXT, + &parent_ctx, sizeof(parent_ctx), NULL); + if (res != sizeof(parent_ctx)) + return 0; + + res = f2fs_getxattr(child, F2FS_XATTR_INDEX_ENCRYPTION, + F2FS_XATTR_NAME_ENCRYPTION_CONTEXT, + &child_ctx, sizeof(child_ctx), NULL); + if (res != sizeof(child_ctx)) return 0; - return (memcmp(parent_ci->ci_master_key, - child_ci->ci_master_key, - F2FS_KEY_DESCRIPTOR_SIZE) == 0 && - (parent_ci->ci_data_mode == child_ci->ci_data_mode) && - (parent_ci->ci_filename_mode == child_ci->ci_filename_mode) && - (parent_ci->ci_flags == child_ci->ci_flags)); + return memcmp(parent_ctx.master_key_descriptor, + child_ctx.master_key_descriptor, + F2FS_KEY_DESCRIPTOR_SIZE) == 0 && + (parent_ctx.contents_encryption_mode == + child_ctx.contents_encryption_mode) && + (parent_ctx.filenames_encryption_mode == + child_ctx.filenames_encryption_mode) && + (parent_ctx.flags == child_ctx.flags); } /** Patches currently in stable-queue which might be from ebiggers@xxxxxxxxxx are queue-4.4/fscrypt-avoid-collisions-when-presenting-long-encrypted-filenames.patch queue-4.4/fscrypt-fix-context-consistency-check-when-key-s-unavailable.patch queue-4.4/ext4-crypto-fix-some-error-handling.patch queue-4.4/ext4-crypto-don-t-let-data-integrity-writebacks-fail-with-enomem.patch queue-4.4/f2fs-check-entire-encrypted-bigname-when-finding-a-dentry.patch