linux-kernel - Re: [PATCH 0/3] ceph: minor fixes and encrypted snapshot names

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: <878rtoo3bi.fsf@brahms.olymp>
Date:   Sat, 05 Mar 2022 14:56:49 +0000
From:   Luís Henriques <lhenriques@...e.de>
To:     Jeff Layton <jlayton@...nel.org>
Cc:     Xiubo Li <xiubli@...hat.com>, Ilya Dryomov <idryomov@...il.com>,
        ceph-devel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] ceph: minor fixes and encrypted snapshot names

Jeff Layton <jlayton@...nel.org> writes:

> On Fri, 2022-03-04 at 16:26 +0000, Luís Henriques wrote:
>> Luís Henriques <lhenriques@...e.de> writes:
>> 
>> > Hi!
>> > 
>> > I'm sending another iteration of the encrypted snapshot names patch.  This
>> > patch assumes PR#45224 [1] to be merged as it adds support for the
>> > alternate names.
>> > 
>> > Two notes:
>> > 
>> > 1. Patch 0001 is just a small fix from another fscrypt patch.  It's
>> >    probably better to simply squash it.
>> > 
>> > 2. I'm not sure how easy it is to hit the UAF fixed by patch 0002.  I can
>> >    reproduce it easily by commenting the code that adds the
>> >    DCACHE_NOKEY_NAME flag in patch 0003.
>> 
>> Obviously, immediately after sending this patchset I realized I failed to
>> mention a very (*VERY*) important note:
>> 
>> Snapshot names can not start with a '_'.  I think the reason is related
>> with the 'long snapshot names', but I can't really remember the details
>> anymore.  The point is that an encrypted snapshot name base64-encoded
>> *may* end-up starting with an '_' as we're using the base64-url variant.
>> 
>> I really don't know if it's possible to fix that.  I guess that in that
>> case the user will get an error and fail to create the snapshot but he'll
>> be clueless because the reason.  Probably a warning can be added to the
>> kernel logs, but maybe there are other ideas.
>> 
>
>
> Ouch. Is that imposed by the MDS? It'd be best if we could remove that
> limitation from it altogether if we can.

I do remember hitting this limitation in the past, but a quick grep didn't
show anything in the documentation about it.  This seems to have been
added to the MDS a *long* time ago, with commit 068553473c82 ("mds: adjust
trace encoding, clean up snap naming") but (as usual) there aren't a lot
of details.

>
> If we can't, then we might be able to get away with prepending all the
> encrypted names with some legal characte. Then when we go to decrypt it
> we just strip that off.

This is probably the best way to fix it, but it's worth trying to find
out the origins of this limitation.  I do seem to remember some obscure
reasons, related with the long snap names (for which Xiubo has a patch),
which will start with '_'.  But yeah I'll have to go dig deeper.

> We could also consider changing the base64 routine to use something else
> in lieu of '_' but that's more of a hassle.

Cheers,
-- 
Luís