From 5547db97a6bff7bdf24fa79fba93899e2f3c16ff Mon Sep 17 00:00:00 2001 From: Salvatore Bonaccorso Date: Sat, 8 Apr 2017 09:32:12 +0200 Subject: [PATCH] fscrypt: remove broken support for detecting keyring key revocation (CVE-2017-7374) --- debian/changelog | 2 + ...roken-support-for-detecting-keyring-.patch | 253 ++++++++++++++++++ debian/patches/series | 1 + 3 files changed, 256 insertions(+) create mode 100644 debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch diff --git a/debian/changelog b/debian/changelog index b223cf0e8..0473f9563 100644 --- a/debian/changelog +++ b/debian/changelog @@ -13,6 +13,8 @@ linux (4.9.18-2) UNRELEASED; urgency=medium [ Salvatore Bonaccorso ] * ping: implement proper locking (CVE-2017-2671) + * fscrypt: remove broken support for detecting keyring key revocation + (CVE-2017-7374) -- Ben Hutchings Thu, 30 Mar 2017 18:27:30 +0100 diff --git a/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch b/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch new file mode 100644 index 000000000..0e58294da --- /dev/null +++ b/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch @@ -0,0 +1,253 @@ +From: Eric Biggers +Date: Tue, 21 Feb 2017 15:07:11 -0800 +Subject: fscrypt: remove broken support for detecting keyring key revocation +Origin: https://git.kernel.org/linus/1b53cf9815bb4744958d41f3795d5d5a1d365e2d +Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-7374 + +Filesystem encryption ostensibly supported revoking a keyring key that +had been used to "unlock" encrypted files, causing those files to become +"locked" again. This was, however, buggy for several reasons, the most +severe of which was that when key revocation happened to be detected for +an inode, its fscrypt_info was immediately freed, even while other +threads could be using it for encryption or decryption concurrently. +This could be exploited to crash the kernel or worse. + +This patch fixes the use-after-free by removing the code which detects +the keyring key having been revoked, invalidated, or expired. Instead, +an encrypted inode that is "unlocked" now simply remains unlocked until +it is evicted from memory. Note that this is no worse than the case for +block device-level encryption, e.g. dm-crypt, and it still remains +possible for a privileged user to evict unused pages, inodes, and +dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by +simply unmounting the filesystem. In fact, one of those actions was +already needed anyway for key revocation to work even somewhat sanely. +This change is not expected to break any applications. + +In the future I'd like to implement a real API for fscrypt key +revocation that interacts sanely with ongoing filesystem operations --- +waiting for existing operations to complete and blocking new operations, +and invalidating and sanitizing key material and plaintext from the VFS +caches. But this is a hard problem, and for now this bug must be fixed. + +This bug affected almost all versions of ext4, f2fs, and ubifs +encryption, and it was potentially reachable in any kernel configured +with encryption support (CONFIG_EXT4_ENCRYPTION=y, +CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or +CONFIG_UBIFS_FS_ENCRYPTION=y). Note that older kernels did not use the +shared fs/crypto/ code, but due to the potential security implications +of this bug, it may still be worthwhile to backport this fix to them. + +Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode") +Signed-off-by: Eric Biggers +Signed-off-by: Theodore Ts'o +Acked-by: Michael Halcrow +Signed-off-by: Greg Kroah-Hartman +[carnil: backport synced with 2984e52c75c657db7901f6189f02e0251ca963c2 in 4.9.20] +--- + fs/crypto/crypto.c | 10 +--------- + fs/crypto/fname.c | 2 +- + fs/crypto/keyinfo.c | 52 +++++++++--------------------------------------- + include/linux/fscrypto.h | 2 -- + 4 files changed, 11 insertions(+), 55 deletions(-) + +diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c +index 98f87fe8f186..61cfccea77bc 100644 +--- a/fs/crypto/crypto.c ++++ b/fs/crypto/crypto.c +@@ -352,7 +352,6 @@ EXPORT_SYMBOL(fscrypt_zeroout_range); + static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) + { + struct dentry *dir; +- struct fscrypt_info *ci; + int dir_has_key, cached_with_key; + + if (flags & LOOKUP_RCU) +@@ -364,18 +363,11 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags) + return 0; + } + +- ci = d_inode(dir)->i_crypt_info; +- if (ci && ci->ci_keyring_key && +- (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) | +- (1 << KEY_FLAG_REVOKED) | +- (1 << KEY_FLAG_DEAD)))) +- ci = NULL; +- + /* this should eventually be an flag in d_flags */ + spin_lock(&dentry->d_lock); + cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY; + spin_unlock(&dentry->d_lock); +- dir_has_key = (ci != NULL); ++ dir_has_key = (d_inode(dir)->i_crypt_info != NULL); + dput(dir); + + /* +diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c +index 9b774f4b50c8..80bb956e14e5 100644 +--- a/fs/crypto/fname.c ++++ b/fs/crypto/fname.c +@@ -350,7 +350,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname, + fname->disk_name.len = iname->len; + return 0; + } +- ret = get_crypt_info(dir); ++ ret = fscrypt_get_encryption_info(dir); + if (ret && ret != -EOPNOTSUPP) + return ret; + +diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c +index 67fb6d8876d0..bb4606368eb1 100644 +--- a/fs/crypto/keyinfo.c ++++ b/fs/crypto/keyinfo.c +@@ -99,6 +99,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info, + kfree(full_key_descriptor); + if (IS_ERR(keyring_key)) + return PTR_ERR(keyring_key); ++ down_read(&keyring_key->sem); + + if (keyring_key->type != &key_type_logon) { + printk_once(KERN_WARNING +@@ -106,11 +107,9 @@ static int validate_user_key(struct fscrypt_info *crypt_info, + res = -ENOKEY; + goto out; + } +- down_read(&keyring_key->sem); + ukp = user_key_payload(keyring_key); + if (ukp->datalen != sizeof(struct fscrypt_key)) { + res = -EINVAL; +- up_read(&keyring_key->sem); + goto out; + } + master_key = (struct fscrypt_key *)ukp->data; +@@ -121,17 +120,11 @@ static int validate_user_key(struct fscrypt_info *crypt_info, + "%s: key size incorrect: %d\n", + __func__, master_key->size); + res = -ENOKEY; +- up_read(&keyring_key->sem); + goto out; + } + res = derive_key_aes(ctx->nonce, master_key->raw, raw_key); +- up_read(&keyring_key->sem); +- if (res) +- goto out; +- +- crypt_info->ci_keyring_key = keyring_key; +- return 0; + out: ++ up_read(&keyring_key->sem); + key_put(keyring_key); + return res; + } +@@ -173,12 +166,11 @@ static void put_crypt_info(struct fscrypt_info *ci) + if (!ci) + return; + +- key_put(ci->ci_keyring_key); + crypto_free_skcipher(ci->ci_ctfm); + kmem_cache_free(fscrypt_info_cachep, ci); + } + +-int get_crypt_info(struct inode *inode) ++int fscrypt_get_encryption_info(struct inode *inode) + { + struct fscrypt_info *crypt_info; + struct fscrypt_context ctx; +@@ -188,21 +180,15 @@ int get_crypt_info(struct inode *inode) + u8 *raw_key = NULL; + int res; + ++ if (inode->i_crypt_info) ++ return 0; ++ + res = fscrypt_initialize(); + if (res) + return res; + + if (!inode->i_sb->s_cop->get_context) + return -EOPNOTSUPP; +-retry: +- crypt_info = ACCESS_ONCE(inode->i_crypt_info); +- if (crypt_info) { +- if (!crypt_info->ci_keyring_key || +- key_validate(crypt_info->ci_keyring_key) == 0) +- return 0; +- fscrypt_put_encryption_info(inode, crypt_info); +- goto retry; +- } + + res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx)); + if (res < 0) { +@@ -230,7 +216,6 @@ int get_crypt_info(struct inode *inode) + crypt_info->ci_data_mode = ctx.contents_encryption_mode; + crypt_info->ci_filename_mode = ctx.filenames_encryption_mode; + crypt_info->ci_ctfm = NULL; +- crypt_info->ci_keyring_key = NULL; + memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor, + sizeof(crypt_info->ci_master_key)); + +@@ -285,14 +270,8 @@ int get_crypt_info(struct inode *inode) + if (res) + goto out; + +- kzfree(raw_key); +- raw_key = NULL; +- if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) != NULL) { +- put_crypt_info(crypt_info); +- goto retry; +- } +- return 0; +- ++ if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL) ++ crypt_info = NULL; + out: + if (res == -ENOKEY) + res = 0; +@@ -300,6 +279,7 @@ int get_crypt_info(struct inode *inode) + kzfree(raw_key); + return res; + } ++EXPORT_SYMBOL(fscrypt_get_encryption_info); + + void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci) + { +@@ -317,17 +297,3 @@ void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci) + put_crypt_info(ci); + } + EXPORT_SYMBOL(fscrypt_put_encryption_info); +- +-int fscrypt_get_encryption_info(struct inode *inode) +-{ +- struct fscrypt_info *ci = inode->i_crypt_info; +- +- if (!ci || +- (ci->ci_keyring_key && +- (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) | +- (1 << KEY_FLAG_REVOKED) | +- (1 << KEY_FLAG_DEAD))))) +- return get_crypt_info(inode); +- return 0; +-} +-EXPORT_SYMBOL(fscrypt_get_encryption_info); +diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h +index ff8b11b26f31..f6dfc2950f76 100644 +--- a/include/linux/fscrypto.h ++++ b/include/linux/fscrypto.h +@@ -79,7 +79,6 @@ struct fscrypt_info { + u8 ci_filename_mode; + u8 ci_flags; + struct crypto_skcipher *ci_ctfm; +- struct key *ci_keyring_key; + u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE]; + }; + +@@ -256,7 +255,6 @@ extern int fscrypt_has_permitted_context(struct inode *, struct inode *); + extern int fscrypt_inherit_context(struct inode *, struct inode *, + void *, bool); + /* keyinfo.c */ +-extern int get_crypt_info(struct inode *); + extern int fscrypt_get_encryption_info(struct inode *); + extern void fscrypt_put_encryption_info(struct inode *, struct fscrypt_info *); + +-- +2.11.0 + diff --git a/debian/patches/series b/debian/patches/series index c10d9fd81..4b8473c14 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -131,6 +131,7 @@ bugfix/all/net-packet-fix-overflow-in-check-for-priv-area-size.patch bugfix/all/net-packet-fix-overflow-in-check-for-tp_frame_nr.patch bugfix/all/net-packet-fix-overflow-in-check-for-tp_reserve.patch bugfix/all/ping-implement-proper-locking.patch +bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch # Fix exported symbol versions bugfix/ia64/revert-ia64-move-exports-to-definitions.patch