Jeff Layton <jlayton@xxxxxxxxxx> writes:
> On Fri, 2022-03-04 at 16:26 +0000, Luís Henriques wrote:
>> Luís Henriques <lhenriques@xxxxxxx> 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
