From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from imap.thunk.org ([74.207.234.97]:49942 "EHLO imap.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750705AbcL1DsS (ORCPT ); Tue, 27 Dec 2016 22:48:18 -0500 Date: Tue, 27 Dec 2016 22:48:12 -0500 From: Theodore Ts'o To: Eric Biggers Cc: linux-fsdevel@vger.kernel.org, Jaegeuk Kim , Richard Weinberger , Eric Biggers Subject: Re: [PATCH v2 1/5] fscrypt: fix loophole in one-encryption-policy-per-tree enforcement Message-ID: <20161228034812.ikoat5x3e7ucnac7@thunk.org> References: <1482186016-107643-1-git-send-email-ebiggers3@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1482186016-107643-1-git-send-email-ebiggers3@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Dec 19, 2016 at 02:20:12PM -0800, Eric Biggers wrote: > 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. I'm actually not sure this is the best way to address this issue. What I think is better would be to forbid any renames if we are missing the encryption key for the directory. I had actually noticed a problem here early when this worked: root@kvm-xfstests:/# mount -o test_dummy_encryption /dev/vdc /vdc root@kvm-xfstests:/# mkdir /vdc/test root@kvm-xfstests:/# cp /etc/motd /vdc/test root@kvm-xfstests:/# touch /vdc/test/empty root@kvm-xfstests:/# umount /vdc root@kvm-xfstests:/# mount /dev/vdc /vdc root@kvm-xfstests:/# cd /vdc/test root@kvm-xfstests:/vdc/test# ls -l total 4 -rw-r--r-- 1 root root 0 Dec 27 22:35 6,LKNRJsp209FbXoSvJWzB -rw-r--r-- 1 root root 286 Dec 27 22:35 uRJ5vJh9gE7vcomYMqTAyD root@kvm-xfstests:/vdc/test# mv uRJ5vJh9gE7vcomYMqTAyD 6,LKNRJsp209FbXoSvJWzB root@kvm-xfstests:/vdc/test# ls -l total 4 -rw-r--r-- 1 root root 286 Dec 27 22:35 6,LKNRJsp209FbXoSvJWzB root@kvm-xfstests:/vdc/test# While there's no cryptographic reason why this can't be done (obviously), and while a bad guy can always do something like this via debugfs, I can't see any legitimate reason why we should allow this to work. It's one thing to allow a process without the encryption key (generally with root privileges) to delete a file, but to rename two files in an encrypted directory? Or as you pointed out, allowing an exchange of two files via RENAME_EXCHANGE? Even if encryption policies match, I don't think we should be allowing this at all. - Ted