From patchwork Mon Dec 19 22:20:12 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 9480795 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 7980C601C2 for ; Mon, 19 Dec 2016 22:21:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6A2E42837F for ; Mon, 19 Dec 2016 22:21:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5C9E3284FC; Mon, 19 Dec 2016 22:21:08 +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.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, 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 B295B2837F for ; Mon, 19 Dec 2016 22:21:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754316AbcLSWVF (ORCPT ); Mon, 19 Dec 2016 17:21:05 -0500 Received: from mail-it0-f66.google.com ([209.85.214.66]:33908 "EHLO mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753904AbcLSWVE (ORCPT ); Mon, 19 Dec 2016 17:21:04 -0500 Received: by mail-it0-f66.google.com with SMTP id 75so11936039ite.1 for ; Mon, 19 Dec 2016 14:21:03 -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=//zziEoPJYn6/MS89vcrZd3a/5Q5V0VxBtVTPEb6lJQ=; b=UlwSC0xy/6byZT5J/dIZSFlJ4ywrz4dUX5eoNWdpkcjB/BQXfexe1KbyMDA38UxRKT nk5GoC58oMgICjzqonqRN5KflE/o6yaZ7onlQHz+g6/tJkd5Fpwvkv5+HF79nx/gPLUy jsQU78tiKiBJ2LQXYWTptRLlGJkfhp2QcpDaSVGJHAC6reM2V9FaUfmlYzKW81eVtua0 IYcBrPvAAAbm9qqqTlJOmgY1dFSPXJ/aGREeue/aTdmZjxbQ3x14K8qowlXzNM00d9U7 97jIgHqVftS94nBbVIMH2F9Vjm7aIJMmNhtcn9q2E0yP/vuPhE+fXBiGGayqfSQ5xH2H L6wQ== 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=//zziEoPJYn6/MS89vcrZd3a/5Q5V0VxBtVTPEb6lJQ=; b=m4j0pLZujGhpWhJWsiDkbovFb9ID/WAls+2Equ7g2BvGsXXC/SskSGas+6q1ZHt9k7 dw7bkxW9b8D1NGaQPfhw0p1ZLYEb4UlYHnOPehkYynYU8CCOgYk2mXnbyQRTaHPV1Csy Cnpg9QVPyWYBvcAt3xeRq0okmgJLwkEGYhlezjse/1X78gjL/CMUDlc5g2GYovb0x0Pb hOZ18ShF3LnLlOy30eCGx4xI/RRGivkEz4dST9xGFI4UOdm4D2VRdC+vWfOfcmzYFHjb DXjkqqbo0yquJPza0JrajT8cwKXYZRA+ySH3wUs4a77krhogmzrVhv7HZv7/oW28TqjK jqyA== X-Gm-Message-State: AIkVDXIJHV5hvIXzaPnAoKczUxC8Ys4w5mTvMlkb4ctwIlAwR8dym2yY23QIgklCZ+mxyQ== X-Received: by 10.36.149.196 with SMTP id m187mr4810109itd.34.1482186062998; Mon, 19 Dec 2016 14:21:02 -0800 (PST) Received: from ebiggers-linuxstation.kir.corp.google.com ([100.119.30.131]) by smtp.gmail.com with ESMTPSA id g186sm7687106itb.21.2016.12.19.14.21.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 19 Dec 2016 14:21:02 -0800 (PST) From: Eric Biggers To: linux-fsdevel@vger.kernel.org Cc: "Theodore Y . Ts'o" , Jaegeuk Kim , Richard Weinberger , Eric Biggers Subject: [PATCH v2 1/5] fscrypt: fix loophole in one-encryption-policy-per-tree enforcement Date: Mon, 19 Dec 2016 14:20:12 -0800 Message-Id: <1482186016-107643-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 Cc: stable@vger.kernel.org --- 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);