From patchwork Thu Dec 15 19:19:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 9476763 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 462DA6047D for ; Thu, 15 Dec 2016 19:39:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 34A1D2877A for ; Thu, 15 Dec 2016 19:39:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 29394287D8; Thu, 15 Dec 2016 19:39:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9E5E42877A for ; Thu, 15 Dec 2016 19:39:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753997AbcLOTjS (ORCPT ); Thu, 15 Dec 2016 14:39:18 -0500 Received: from mail-pg0-f67.google.com ([74.125.83.67]:36109 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753819AbcLOTjO (ORCPT ); Thu, 15 Dec 2016 14:39:14 -0500 Received: by mail-pg0-f67.google.com with SMTP id a1so49239pgf.3; Thu, 15 Dec 2016 11:39:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=r64DW2RRFAF7h/WpejXEVkbIDxSk2fMVn/SCUcbL/W8=; b=V6aOstGMn1vte2kV+0iFgrdCWeOcRGMMGL9jpDiPS40G3Svh3vuc9MhaylU+UkHGAw mnKtljVkWRWEK2AjKEVpYAHeYJB2obW2lLs8vjOLPALg/FI28mEISlLuGqR0Fn43odDn DqL91O7lIQB1M6tSa2vxGnqaOdaeD4eg74I5KWNAnZdvmIR0hJ83Tlg3lpj2TIG1gQta Nnx9kQYYzSmrOzK89cRjrSSStWckBOWwk9MeRrEEjQjE2dugP4q6amjcJWzFMrMwn7Es W3wPTXhXuiMp7H02UxnUKVmhBV5ciXFsiIlQGdY2NVi78TedYbWBj2HcaZyk5PQtw+As p9WQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=r64DW2RRFAF7h/WpejXEVkbIDxSk2fMVn/SCUcbL/W8=; b=bzLVw9kgUZaLgJOZfAnpvftefa6kNv5afjGjKe1tOXooibHxfSse3wyTM8FRoQS1sE Vovw3WkKSDjKPbxKgt+tHGu3hg4l/zKn5gq+1nKMpzCkcLZIJIpR+tevLKExuHTHNHbE udkA9wQMjCB18sBv8kseA5XwKiZMQe+ZDSF1SAtJlprG/7jFaaqmPzw0GSHcoh0TkcrK R+AmJlmPRliTiMY2rXWEzWVJuksYp0hWtvVVdaRWq+Tya6y8wwF2GWIHwwNdvDlI+VCZ Nig2dLWzmYNyST9Uzb5N215eoHFKsYwDFunGoQXM4jUSHYd9MuOMpVQMj41ZEfDSnPCS FShg== X-Gm-Message-State: AKaTC00UTft8Z5wimjbbNq0ALJZVRbWeZcikScEkSaxmILE+02qUMKne5PbE6FcfYauZjg== X-Received: by 10.84.131.165 with SMTP id d34mr5632378pld.41.1481829837115; Thu, 15 Dec 2016 11:23:57 -0800 (PST) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.119.30.131]) by smtp.gmail.com with ESMTPSA id c22sm6390976pgn.12.2016.12.15.11.23.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 15 Dec 2016 11:23:56 -0800 (PST) From: Eric Biggers To: linux-fsdevel@vger.kernel.org Cc: "Theodore Y . Ts'o" , Jaegeuk Kim , linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, Richard Weinberger , David Gstir , Eric Biggers Subject: [PATCH 1/3] fscrypt: fix loophole in one-encryption-policy-per-tree enforcement Date: Thu, 15 Dec 2016 11:19:42 -0800 Message-Id: <1481829584-50218-1-git-send-email-ebiggers3@gmail.com> X-Mailer: git-send-email 2.8.0.rc3.226.g39d4020 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Biggers Filesystem encryption is designed to enforce that all files in an encrypted directory tree use the same encryption policy. Operations that violate this constraint are supposed to fail with EPERM. There was one case that was missed, however: the cross-rename operation (i.e. renameat2 with RENAME_EXCHANGE) allowed two files with different encryption policies to be exchanged, provided that neither encryption key was available. To fix this, when we can't compare the fscrypt_info structs because the key is unavailable, compare the fscrypt_context structs instead. This will be covered by a test in my encryption xfstests patchset. Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode") Signed-off-by: Eric Biggers Reviewed-by: Richard Weinberger --- fs/crypto/policy.c | 76 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index 6ed7c2e..5de0633 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -169,22 +169,46 @@ int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg) } EXPORT_SYMBOL(fscrypt_ioctl_get_policy); +/** + * fscrypt_has_permitted_context() - is a file's encryption policy permitted + * within its parent directory? + * + * @parent: inode for parent directory + * @child: inode for file being looked up or linked into the directory + * + * Filesystems must call this function during lookup in an encrypted directory + * and before any operation that involves linking a file into an encrypted + * directory, including link, rename, and cross rename. It enforces the + * constraint that within a given encrypted directory tree, all files use the + * same encryption policy. The lookup check is needed to detect offline + * violations of this constraint, whereas the other checks are needed to prevent + * online violations of this constraint. + * + * Return: 1 if permitted, 0 if forbidden. If forbidden, the caller must fail + * the filesystem operation with EPERM. + */ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child) { - struct fscrypt_info *parent_ci, *child_ci; + const struct fscrypt_operations *cops = parent->i_sb->s_cop; + const struct fscrypt_info *parent_ci, *child_ci; + struct fscrypt_context parent_ctx, child_ctx; int res; - if ((parent == NULL) || (child == NULL)) { - printk(KERN_ERR "parent %p child %p\n", parent, child); - BUG_ON(1); - } - - /* no restrictions if the parent directory is not encrypted */ - if (!parent->i_sb->s_cop->is_encrypted(parent)) + /* No restrictions if the parent directory is unencrypted */ + if (!cops->is_encrypted(parent)) return 1; - /* if the child directory is not encrypted, this is always a problem */ - if (!parent->i_sb->s_cop->is_encrypted(child)) + + /* Encrypted directories must not contain unencrypted files */ + if (!cops->is_encrypted(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 compare the fscrypt_context structs directly. + * In any case, if an unexpected error occurs, fall back to "forbidden". + */ + res = fscrypt_get_encryption_info(parent); if (res) return 0; @@ -193,17 +217,31 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child) return 0; parent_ci = parent->i_crypt_info; child_ci = 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, + FS_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 = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx)); + if (res != sizeof(parent_ctx)) return 0; - return (memcmp(parent_ci->ci_master_key, - child_ci->ci_master_key, - FS_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 = cops->get_context(child, &child_ctx, sizeof(child_ctx)); + if (res != sizeof(child_ctx)) + return 0; + + return memcmp(parent_ctx.master_key_descriptor, + child_ctx.master_key_descriptor, + FS_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); } EXPORT_SYMBOL(fscrypt_has_permitted_context);