| Message ID | 1431402663-24809-1-git-send-email-jaegeuk@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 11, 2015 at 08:51:03PM -0700, Jaegeuk Kim wrote: > Previoulsy when making xts_tweak, page->index was used. > But, when it supports fcollapse, the block address was moved, so that we can > lose the original page->index, which causes decrytion failure. > > In order to avoid that, let's use the inode->i_ino for xfs_tweak hint. I'm afraid that's a really bad idea. We need to have a different xts tweak for each block, and if we use the inode number, then every single block will have the same XTS tweak, which is a cryptographic disaster. Basically, we currently can't support either collapse range or insert range for encrypted files. In ext4 we explicitly return EOPNOTSUPP if there is an attempt to call collapse range on an encrypted file. Personally, I don't think this is a major restriction, so I haven't lost any sleep over this. Eventually, Michael and I hope to add support for Galois Counter Mode, but that requires the file system to be able to store per-block cryptrographic information, which can be used for the GCM authentication tag as well as a per-block IV. The per-block IV being stored in a separate data structure would also allow insert range/collapse range to work, at the cost of needing to do a lookup to fetch the per-block cryptographic information. (And to set the per-block cryptographic information when writing the information, in a way where we can atomically write the data block as well as the per-block authenticaiton tag, which gets a bit tricky....) In any case, I believe support for data integrity is a far more compelling reason for adding pre-block crypto information, and supporting collapse/insert range is at best a fortunate side effect. Cheers, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, May 12, 2015 at 07:35:57PM -0400, Theodore Ts'o wrote: > On Mon, May 11, 2015 at 08:51:03PM -0700, Jaegeuk Kim wrote: > > Previoulsy when making xts_tweak, page->index was used. > > But, when it supports fcollapse, the block address was moved, so that we can > > lose the original page->index, which causes decrytion failure. > > > > In order to avoid that, let's use the inode->i_ino for xfs_tweak hint. > > I'm afraid that's a really bad idea. We need to have a different xts > tweak for each block, and if we use the inode number, then every > single block will have the same XTS tweak, which is a cryptographic > disaster. Thank you for the kind explanation. I just thought that inode number was enough for encryption, since user can easily retrieve any inode number and its block offsets/addresses as well. > > Basically, we currently can't support either collapse range or insert > range for encrypted files. In ext4 we explicitly return EOPNOTSUPP if > there is an attempt to call collapse range on an encrypted file. > Personally, I don't think this is a major restriction, so I haven't > lost any sleep over this. Ok, I see. It should not be a major concern. I'll deactivate collapse/insert in f2fs instead. > > Eventually, Michael and I hope to add support for Galois Counter Mode, > but that requires the file system to be able to store per-block > cryptrographic information, which can be used for the GCM > authentication tag as well as a per-block IV. The per-block IV being > stored in a separate data structure would also allow insert > range/collapse range to work, at the cost of needing to do a lookup to > fetch the per-block cryptographic information. (And to set the > per-block cryptographic information when writing the information, in a > way where we can atomically write the data block as well as the > per-block authenticaiton tag, which gets a bit tricky....) > > In any case, I believe support for data integrity is a far more > compelling reason for adding pre-block crypto information, and > supporting collapse/insert range is at best a fortunate side effect. Got it, thanks, :) > > Cheers, > > - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/f2fs/crypto.c b/fs/f2fs/crypto.c index 1dd0835..35986a5 100644 --- a/fs/f2fs/crypto.c +++ b/fs/f2fs/crypto.c @@ -375,7 +375,6 @@ typedef enum { static int f2fs_page_crypto(struct f2fs_crypto_ctx *ctx, struct inode *inode, f2fs_direction_t rw, - pgoff_t index, struct page *src_page, struct page *dest_page) @@ -420,10 +419,10 @@ static int f2fs_page_crypto(struct f2fs_crypto_ctx *ctx, req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP, f2fs_crypt_complete, &ecr); - BUILD_BUG_ON(F2FS_XTS_TWEAK_SIZE < sizeof(index)); - memcpy(xts_tweak, &index, sizeof(index)); - memset(&xts_tweak[sizeof(index)], 0, - F2FS_XTS_TWEAK_SIZE - sizeof(index)); + BUILD_BUG_ON(F2FS_XTS_TWEAK_SIZE < sizeof(inode->i_ino)); + memcpy(xts_tweak, &inode->i_ino, sizeof(inode->i_ino)); + memset(&xts_tweak[sizeof(inode->i_ino)], 0, + F2FS_XTS_TWEAK_SIZE - sizeof(inode->i_ino)); sg_init_table(&dst, 1); sg_set_page(&dst, dest_page, PAGE_CACHE_SIZE, 0); @@ -496,7 +495,7 @@ struct page *f2fs_encrypt(struct inode *inode, } ctx->bounce_page = ciphertext_page; ctx->control_page = plaintext_page; - err = f2fs_page_crypto(ctx, inode, F2FS_ENCRYPT, plaintext_page->index, + err = f2fs_page_crypto(ctx, inode, F2FS_ENCRYPT, plaintext_page, ciphertext_page); if (err) { f2fs_release_crypto_ctx(ctx); @@ -524,7 +523,7 @@ int f2fs_decrypt(struct f2fs_crypto_ctx *ctx, struct page *page) BUG_ON(!PageLocked(page)); return f2fs_page_crypto(ctx, page->mapping->host, - F2FS_DECRYPT, page->index, page, page); + F2FS_DECRYPT, page, page); } /*
Previoulsy when making xts_tweak, page->index was used. But, when it supports fcollapse, the block address was moved, so that we can lose the original page->index, which causes decrytion failure. In order to avoid that, let's use the inode->i_ino for xfs_tweak hint. Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/crypto.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)