fscrypt: remove broken support for detecting keyring key revocation (CVE-2017-7374)
This commit is contained in:
parent
43f7156d3a
commit
5547db97a6
|
@ -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 <ben@decadent.org.uk> Thu, 30 Mar 2017 18:27:30 +0100
|
||||
|
||||
|
|
253
debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch
vendored
Normal file
253
debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch
vendored
Normal file
|
@ -0,0 +1,253 @@
|
|||
From: Eric Biggers <ebiggers@google.com>
|
||||
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 <ebiggers@google.com>
|
||||
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
|
||||
Acked-by: Michael Halcrow <mhalcrow@google.com>
|
||||
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
||||
[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
|
||||
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue