Re: [PATCH] fscrypt: align Base64 encoding with RFC 4648 base64url

[Hannes Reinecke <hare@suse.de>]

On 7/18/21 2:01 AM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> fscrypt uses a Base64 encoding to encode no-key filenames (the filenames
> that are presented to userspace when a directory is listed without its
> encryption key).  There are many variants of Base64, but the most common
> ones are specified by RFC 4648.  fscrypt can't use the regular RFC 4648
> "base64" variant because "base64" uses the '/' character, which isn't
> allowed in filenames.  However, RFC 4648 also specifies a "base64url"
> variant for use in URLs and filenames.  "base64url" is less common than
> "base64", but it's still implemented in many programming libraries.
> 
> Unfortunately, what fscrypt actually uses is a custom Base64 variant
> that differs from "base64url" in several ways:
> 
> - The binary data is divided into 6-bit chunks differently.
> 
> - Values 62 and 63 are encoded with '+' and ',' instead of '-' and '_'.
> 
> - '='-padding isn't used.  This isn't a problem per se, as the padding
>   isn't technically necessary, and RFC 4648 doesn't strictly require it.
>   But it needs to be properly documented.
> 
> There have been two attempts to copy the fscrypt Base64 code into lib/
> (https://lkml.kernel.org/r/20200821182813.52570-6-jlayton@kernel.org and
> https://lkml.kernel.org/r/20210716110428.9727-5-hare@suse.de), and both
> have been caught up by the fscrypt Base64 variant being nonstandard and
> not properly documented.  Also, the planned use of the fscrypt Base64
> code in the CephFS storage back-end will prevent it from being changed
> later (whereas currently it can still be changed), so we need to choose
> an encoding that we're happy with before it's too late.
> 
> Therefore, switch the fscrypt Base64 variant to base64url, in order to
> align more closely with RFC 4648 and other implementations and uses of
> Base64.  However, I opted not to implement '='-padding, as '='-padding
> adds complexity, is unnecessary, and isn't required by the RFC.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  Documentation/filesystems/fscrypt.rst |  10 +--
>  fs/crypto/fname.c                     | 106 ++++++++++++++++----------
>  2 files changed, 70 insertions(+), 46 deletions(-)
> 
Thanks for doing it. I got confused by it myself, and having it named
'base64encode' while not _actually_ being base64 didn't help, either.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@suse.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer