| Message ID | 20200904160537.76663-7-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ceph+fscrypt: context, filename and symlink support | expand |
On Fri, Sep 04, 2020 at 12:05:25PM -0400, Jeff Layton wrote: > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/crypto/fname.c | 71 +++++++++++++++++++++++------------------ > include/linux/fscrypt.h | 3 ++ > 2 files changed, 43 insertions(+), 31 deletions(-) > > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c > index 9440a44e24ac..09f09def87fc 100644 > --- a/fs/crypto/fname.c > +++ b/fs/crypto/fname.c > @@ -300,6 +300,45 @@ void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str) > } > EXPORT_SYMBOL(fscrypt_fname_free_buffer); > > +void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash, > + const struct fscrypt_str *iname, > + struct fscrypt_str *oname) > +{ > + struct fscrypt_nokey_name nokey_name; > + u32 size; /* size of the unencoded no-key name */ > + > + /* > + * Sanity check that struct fscrypt_nokey_name doesn't have padding > + * between fields and that its encoded size never exceeds NAME_MAX. > + */ > + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) != > + offsetof(struct fscrypt_nokey_name, bytes)); > + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) != > + offsetof(struct fscrypt_nokey_name, sha256)); > + BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX); > + > + if (hash) { > + nokey_name.dirhash[0] = hash; > + nokey_name.dirhash[1] = minor_hash; > + } else { > + nokey_name.dirhash[0] = 0; > + nokey_name.dirhash[1] = 0; > + } > + if (iname->len <= sizeof(nokey_name.bytes)) { > + memcpy(nokey_name.bytes, iname->name, iname->len); > + size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]); > + } else { > + memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes)); > + /* Compute strong hash of remaining part of name. */ > + fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)], > + iname->len - sizeof(nokey_name.bytes), > + nokey_name.sha256); > + size = FSCRYPT_NOKEY_NAME_MAX; > + } > + oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name); > +} > +EXPORT_SYMBOL(fscrypt_encode_nokey_name); Why does this need to be exported? There's no user of this function introduced in this patchset. - Eric
On Mon, 2020-09-07 at 20:55 -0700, Eric Biggers wrote: > On Fri, Sep 04, 2020 at 12:05:25PM -0400, Jeff Layton wrote: > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/crypto/fname.c | 71 +++++++++++++++++++++++------------------ > > include/linux/fscrypt.h | 3 ++ > > 2 files changed, 43 insertions(+), 31 deletions(-) > > > > diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c > > index 9440a44e24ac..09f09def87fc 100644 > > --- a/fs/crypto/fname.c > > +++ b/fs/crypto/fname.c > > @@ -300,6 +300,45 @@ void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str) > > } > > EXPORT_SYMBOL(fscrypt_fname_free_buffer); > > > > +void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash, > > + const struct fscrypt_str *iname, > > + struct fscrypt_str *oname) > > +{ > > + struct fscrypt_nokey_name nokey_name; > > + u32 size; /* size of the unencoded no-key name */ > > + > > + /* > > + * Sanity check that struct fscrypt_nokey_name doesn't have padding > > + * between fields and that its encoded size never exceeds NAME_MAX. > > + */ > > + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) != > > + offsetof(struct fscrypt_nokey_name, bytes)); > > + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) != > > + offsetof(struct fscrypt_nokey_name, sha256)); > > + BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX); > > + > > + if (hash) { > > + nokey_name.dirhash[0] = hash; > > + nokey_name.dirhash[1] = minor_hash; > > + } else { > > + nokey_name.dirhash[0] = 0; > > + nokey_name.dirhash[1] = 0; > > + } > > + if (iname->len <= sizeof(nokey_name.bytes)) { > > + memcpy(nokey_name.bytes, iname->name, iname->len); > > + size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]); > > + } else { > > + memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes)); > > + /* Compute strong hash of remaining part of name. */ > > + fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)], > > + iname->len - sizeof(nokey_name.bytes), > > + nokey_name.sha256); > > + size = FSCRYPT_NOKEY_NAME_MAX; > > + } > > + oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name); > > +} > > +EXPORT_SYMBOL(fscrypt_encode_nokey_name); > > Why does this need to be exported? > > There's no user of this function introduced in this patchset. > > - Eric Yeah, I probably should have dropped this from the series for now as nothing uses it yet, but eventually we may need this. I did a fairly detailed writeup of the problem here: https://tracker.ceph.com/issues/47162 Basically, we still need to allow clients to look up dentries in the MDS even when they don't have the key. There are a couple of different approaches, but the simplest is to just have the client always store long dentry names using the nokey_name, and then keep the full name in a new field in the dentry representation that is sent across the wire. This requires some changes to the Ceph MDS (which is what that tracker bug is about), and will mean enshrining the nokey name in perpetuity. We're still looking at this for now though, and we're open to other approaches if you've got any to suggest.
On Tue, Sep 08, 2020 at 08:50:04AM -0400, Jeff Layton wrote: > > > +EXPORT_SYMBOL(fscrypt_encode_nokey_name); > > > > Why does this need to be exported? > > > > There's no user of this function introduced in this patchset. > > > > - Eric > > Yeah, I probably should have dropped this from the series for now as > nothing uses it yet, but eventually we may need this. I did a fairly > detailed writeup of the problem here: > > https://tracker.ceph.com/issues/47162 > > Basically, we still need to allow clients to look up dentries in the MDS > even when they don't have the key. > > There are a couple of different approaches, but the simplest is to just > have the client always store long dentry names using the nokey_name, and > then keep the full name in a new field in the dentry representation that > is sent across the wire. > > This requires some changes to the Ceph MDS (which is what that tracker > bug is about), and will mean enshrining the nokey name in perpetuity. > We're still looking at this for now though, and we're open to other > approaches if you've got any to suggest. The (persistent) directory entries have to include the full ciphertext filenames. If they only included the no-key names, then it wouldn't always be possible to translate them back into the original plaintext filenames. It's also required that the filesystem can find a specific directory entry given its corresponding no-key name. For a network filesystem, that can be done either on the client (request all filenames in the directory, then check all of them...), or on the server (give it the no-key name and have it do the matching; it would need to know the specifics of how the no-key names work). The no-key names aren't currently stable, and it would be nice to keep them that way since we might want to improve how they work later. But if you need to stabilize a version of the no-key name format for use in the ceph protocol so that the server can do the matching, it would be possible to do that. It wouldn't even necessarily have to be what fscrypt currently uses; e.g. if it were to simplify things a lot for you to simply use SHA-256(ciphertext_name) instead of the current 'struct fscrypt_nokey_name', you could do that. - Eric
On Tue, 2020-09-08 at 15:53 -0700, Eric Biggers wrote: > On Tue, Sep 08, 2020 at 08:50:04AM -0400, Jeff Layton wrote: > > > > +EXPORT_SYMBOL(fscrypt_encode_nokey_name); > > > > > > Why does this need to be exported? > > > > > > There's no user of this function introduced in this patchset. > > > > > > - Eric > > > > Yeah, I probably should have dropped this from the series for now as > > nothing uses it yet, but eventually we may need this. I did a fairly > > detailed writeup of the problem here: > > > > https://tracker.ceph.com/issues/47162 > > > > Basically, we still need to allow clients to look up dentries in the MDS > > even when they don't have the key. > > > > There are a couple of different approaches, but the simplest is to just > > have the client always store long dentry names using the nokey_name, and > > then keep the full name in a new field in the dentry representation that > > is sent across the wire. > > > > This requires some changes to the Ceph MDS (which is what that tracker > > bug is about), and will mean enshrining the nokey name in perpetuity. > > We're still looking at this for now though, and we're open to other > > approaches if you've got any to suggest. > > The (persistent) directory entries have to include the full ciphertext > filenames. If they only included the no-key names, then it wouldn't always be > possible to translate them back into the original plaintext filenames. > > It's also required that the filesystem can find a specific directory entry given > its corresponding no-key name. For a network filesystem, that can be done > either on the client (request all filenames in the directory, then check all of > them...), or on the server (give it the no-key name and have it do the matching; > it would need to know the specifics of how the no-key names work). > > The no-key names aren't currently stable, and it would be nice to keep them that > way since we might want to improve how they work later. But if you need to > stabilize a version of the no-key name format for use in the ceph protocol so > that the server can do the matching, it would be possible to do that. It > wouldn't even necessarily have to be what fscrypt currently uses; e.g. if it > were to simplify things a lot for you to simply use SHA-256(ciphertext_name) > instead of the current 'struct fscrypt_nokey_name', you could do that. > (cc'ing Xiubo since he's working on the MDS part) We probably will need to make a stable representation. I think the nokey name scheme as it stands would be fine for this purpose, particularly as the representation is only different for really long filenames. We'd only need to carry an alternate name for dentries with names longer than ~150 chars, and those are somewhat rare. Much of this depends on the MDS though, and I'm a lot less familiar with that part. So for now, no need to do anything. We'll reach out once we have a more solid plan of how we want to handle this. Thanks!
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c index 9440a44e24ac..09f09def87fc 100644 --- a/fs/crypto/fname.c +++ b/fs/crypto/fname.c @@ -300,6 +300,45 @@ void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str) } EXPORT_SYMBOL(fscrypt_fname_free_buffer); +void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash, + const struct fscrypt_str *iname, + struct fscrypt_str *oname) +{ + struct fscrypt_nokey_name nokey_name; + u32 size; /* size of the unencoded no-key name */ + + /* + * Sanity check that struct fscrypt_nokey_name doesn't have padding + * between fields and that its encoded size never exceeds NAME_MAX. + */ + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) != + offsetof(struct fscrypt_nokey_name, bytes)); + BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) != + offsetof(struct fscrypt_nokey_name, sha256)); + BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX); + + if (hash) { + nokey_name.dirhash[0] = hash; + nokey_name.dirhash[1] = minor_hash; + } else { + nokey_name.dirhash[0] = 0; + nokey_name.dirhash[1] = 0; + } + if (iname->len <= sizeof(nokey_name.bytes)) { + memcpy(nokey_name.bytes, iname->name, iname->len); + size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]); + } else { + memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes)); + /* Compute strong hash of remaining part of name. */ + fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)], + iname->len - sizeof(nokey_name.bytes), + nokey_name.sha256); + size = FSCRYPT_NOKEY_NAME_MAX; + } + oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name); +} +EXPORT_SYMBOL(fscrypt_encode_nokey_name); + /** * fscrypt_fname_disk_to_usr() - convert an encrypted filename to * user-presentable form @@ -327,8 +366,6 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode, struct fscrypt_str *oname) { const struct qstr qname = FSTR_TO_QSTR(iname); - struct fscrypt_nokey_name nokey_name; - u32 size; /* size of the unencoded no-key name */ if (fscrypt_is_dot_dotdot(&qname)) { oname->name[0] = '.'; @@ -343,35 +380,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode, if (fscrypt_has_encryption_key(inode)) return fname_decrypt(inode, iname, oname); - /* - * Sanity check that struct fscrypt_nokey_name doesn't have padding - * between fields and that its encoded size never exceeds NAME_MAX. - */ - BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, dirhash) != - offsetof(struct fscrypt_nokey_name, bytes)); - BUILD_BUG_ON(offsetofend(struct fscrypt_nokey_name, bytes) != - offsetof(struct fscrypt_nokey_name, sha256)); - BUILD_BUG_ON(BASE64_CHARS(FSCRYPT_NOKEY_NAME_MAX) > NAME_MAX); - - if (hash) { - nokey_name.dirhash[0] = hash; - nokey_name.dirhash[1] = minor_hash; - } else { - nokey_name.dirhash[0] = 0; - nokey_name.dirhash[1] = 0; - } - if (iname->len <= sizeof(nokey_name.bytes)) { - memcpy(nokey_name.bytes, iname->name, iname->len); - size = offsetof(struct fscrypt_nokey_name, bytes[iname->len]); - } else { - memcpy(nokey_name.bytes, iname->name, sizeof(nokey_name.bytes)); - /* Compute strong hash of remaining part of name. */ - fscrypt_do_sha256(&iname->name[sizeof(nokey_name.bytes)], - iname->len - sizeof(nokey_name.bytes), - nokey_name.sha256); - size = FSCRYPT_NOKEY_NAME_MAX; - } - oname->len = base64_encode((const u8 *)&nokey_name, size, oname->name); + fscrypt_encode_nokey_name(hash, minor_hash, iname, oname); return 0; } EXPORT_SYMBOL(fscrypt_fname_disk_to_usr); diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h index 0ddbd27a2e58..57146f9f70e7 100644 --- a/include/linux/fscrypt.h +++ b/include/linux/fscrypt.h @@ -202,6 +202,9 @@ static inline void fscrypt_free_filename(struct fscrypt_name *fname) int fscrypt_fname_alloc_buffer(u32 max_encrypted_len, struct fscrypt_str *crypto_str); void fscrypt_fname_free_buffer(struct fscrypt_str *crypto_str); +void fscrypt_encode_nokey_name(u32 hash, u32 minor_hash, + const struct fscrypt_str *iname, + struct fscrypt_str *oname); int fscrypt_fname_disk_to_usr(const struct inode *inode, u32 hash, u32 minor_hash, const struct fscrypt_str *iname,
Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/crypto/fname.c | 71 +++++++++++++++++++++++------------------ include/linux/fscrypt.h | 3 ++ 2 files changed, 43 insertions(+), 31 deletions(-)