linux-ext4 - Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20161003180340.GA54410@google.com>
Date:   Mon, 3 Oct 2016 11:03:40 -0700
From:   Eric Biggers <ebiggers@...gle.com>
To:     Richard Weinberger <richard.weinberger@...il.com>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        Theodore Ts'o <tytso@....edu>, jaegeuk@...nel.org
Subject: Re: [PATCH] fscrypto: make XTS tweak initialization
 endian-independent

On Sat, Oct 01, 2016 at 06:03:31PM +0200, Richard Weinberger wrote:
> Eric,
> 
> On Fri, Sep 30, 2016 at 7:58 PM, Eric Biggers <ebiggers@...gle.com> wrote:
> > The XTS tweak (or IV) was initialized differently on little endian and
> > big endian systems.  Because the ciphertext depends on the XTS tweak, it
> > was not possible to use an encrypted filesystem created by a little
> > endian system on a big endian system and vice versa, even if they shared
> > the same PAGE_SIZE.  Fix this by always using little endian.
> >
> > This will break hypothetical big endian users of ext4 or f2fs
> > encryption.  However, all users we are aware of are little endian, and
> > it's believed that "real" big endian users are unlikely to exist yet.
> > So this might as well be fixed now before it's too late.
> >
> > Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> > ---
> >  fs/crypto/crypto.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 61057b7d..98f87fe 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -151,7 +151,10 @@ static int do_page_crypto(struct inode *inode,
> >                         struct page *src_page, struct page *dest_page,
> >                         gfp_t gfp_flags)
> >  {
> > -       u8 xts_tweak[FS_XTS_TWEAK_SIZE];
> > +       struct {
> > +               __le64 index;
> > +               u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
> > +       } xts_tweak;
> 
> While we are here, wouldn't it make sense to rename the variable to "iv"?
> In aes-xts mode the IV is used as tweak. But it is still an IV and passed
> as IV parameter to the crypto API.
> 
> Especially when other cipher modes are used this is confusing.
> 

Good idea --- I agree that "iv" is a better name, so as to not tie the code to
XTS specifically.  But I think the renaming should be a separate patch.

Also, currently this code *is* only supposed to be used for XTS.  There's a bug
where a specially crafted filesystem can cause this code path to be entered with
CTS, but I have a patch pending in the ext4 tree to fix that.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html