Eric Biggers <ebiggers@xxxxxxxxxx> writes: > On Wed, Mar 23, 2022 at 02:33:17PM +0000, Luís Henriques wrote: >> Hi Eric, >> >> Jeff Layton <jlayton@xxxxxxxxxx> writes: >> >> > Ceph is going to add fscrypt support, but we still want encrypted >> > filenames to be composed of printable characters, so we can maintain >> > compatibility with clients that don't support fscrypt. >> > >> > We could just adopt fscrypt's current nokey name format, but that is >> > subject to change in the future, and it also contains dirhash fields >> > that we don't need for cephfs. Because of this, we're going to concoct >> > our own scheme for encoding encrypted filenames. It's very similar to >> > fscrypt's current scheme, but doesn't bother with the dirhash fields. >> > >> > The ceph encoding scheme will use base64 encoding as well, and we also >> > want it to avoid characters that are illegal in filenames. Export the >> > fscrypt base64 encoding/decoding routines so we can use them in ceph's >> > fscrypt implementation. >> > >> > Acked-by: Eric Biggers <ebiggers@xxxxxxxxxx> >> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> >> > --- >> > fs/crypto/fname.c | 8 ++++---- >> > include/linux/fscrypt.h | 5 +++++ >> > 2 files changed, 9 insertions(+), 4 deletions(-) >> > >> > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c >> > index a9be4bc74a94..1e4233c95005 100644 >> > --- a/fs/crypto/fname.c >> > +++ b/fs/crypto/fname.c >> > @@ -182,8 +182,6 @@ static int fname_decrypt(const struct inode *inode, >> > static const char base64url_table[65] = >> > "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"; >> > >> > -#define FSCRYPT_BASE64URL_CHARS(nbytes) DIV_ROUND_UP((nbytes) * 4, 3) >> > - >> > /** >> > * fscrypt_base64url_encode() - base64url-encode some binary data >> > * @src: the binary data to encode >> > @@ -198,7 +196,7 @@ static const char base64url_table[65] = >> > * Return: the length of the resulting base64url-encoded string in bytes. >> > * This will be equal to FSCRYPT_BASE64URL_CHARS(srclen). >> > */ >> > -static int fscrypt_base64url_encode(const u8 *src, int srclen, char *dst) >> > +int fscrypt_base64url_encode(const u8 *src, int srclen, char *dst) >> >> I know you've ACK'ed this patch already, but I was wondering if you'd be >> open to change these encode/decode interfaces so that they could be used >> for non-url base64 too. >> >> My motivation is that ceph has this odd limitation where snapshot names >> can not start with the '_' character. And I've an RFC that adds snapshot >> names encryption support which, unfortunately, can end up starting with >> this char after base64 encoding. >> >> So, my current proposal is to use a different encoding table. I was >> thinking about the IMAP mailboxes naming which uses '+' and ',' instead of >> the '-' and '_', but any other charset would be OK (except those that >> include '/' of course). So, instead of adding yet another base64 >> implementation to the kernel, I was wondering if you'd be OK accepting a >> patch to add an optional arg to these encoding/decoding functions to pass >> an alternative table. Or, if you'd prefer, keep the existing interface >> but turning these functions into wrappers to more generic functions. >> >> Obviously, Jeff, please feel free to comment too if you have any reserves >> regarding this approach. >> >> Cheers, >> -- >> Luís >> > > Base64 encoding/decoding is trivial enough that I think you should just add your > own functions to fs/ceph/ for now if you need yet another Base64 variant. If we > were to add general functions that allow "building your own" Base64 variant, I > think they'd belong in lib/, not fs/crypto/. (I objected to lib/ in the first > version of Jeff's patchset because that patchset proposed adding just the old, > idiosyncratic fscrypt Base64 variant to lib/ and just calling it "base64", which > was misleading. But, if there were to be properly documented functions to > "build your own" Base64 variant, allowing control over both the character set > and whether padding is done, lib/ would be the place...) OK, that makes sense. I agree that the right place for a generic implementation would be somewhere out of the fs/crypto/ directory. I guess that, for now, I'll follow your advice and keep a local implementation (in fact, the libceph *has* already an implementation!). But adding a generic implementation and clean-up all the different implementations in the kernel tree is probably a nice project. For the future. Maybe. *sigh* Cheers, -- Luís
