On Fri, Mar 11, 2011 at 8:28 AM, Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx> wrote: > On Wed Mar 09, 2011 at 10:43:45AM +0800, Ethan Du <ethan.too@xxxxxxxxx> wrote: >> On Wed, Mar 9, 2011 at 7:21 AM, Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxx> wrote: >> > On Wed Mar 02, 2011 at 11:09:25AM +0800, Ethan Du <ethan.too@xxxxxxxxx> wrote: >> >> With ecryptfs_create_plain mount option, newly created file will be >> >> plain lower fs file >> >> For those who want to read encrypted files, but want no more encrypted files >> > >> > Hi Ethan - Lets get the process issues out of the way. The gmail web >> > client seems to break patches. See Documentation/email-clients.txt for >> > details. Also, it is a good idea to include the maintainers and project >> > mailing list in your distribution list to make sure that the right >> > people see the patch. scripts/get_maintainer.pl works or you can >> > manually take a look at the MAINTAINERS file. >> >> Thanks for the info. I git-send-email to the mail list >> ecryptfs-devel@xxxxxxxxxxxxxxxxxxxx And then lazily forwarded it to >> here, maybe this is the reason. > > ecryptfs-devel is moderated for all non-members, so that's probably to > blame. No worries, though. Just something to keep in mind to make sure > your future patches don't get lost. > >> >> > >> > For the patch itself, I'm not sure that this mount option would get used >> > by many people, yet would add to the eCryptfs test burden. How do you >> > see this being used? I can't imagine many users wanting to disable >> > encrypted file creation at a mount wide level. >> >> I think for most users, when they want to disable encryption on a >> mount point, they will copy the files to elsewhere, and then unmount >> ecryptfs, and move them back. > > Can you explain why you want to disable encryption on a mount point? It is not I want, it is user wants. I am adding an option for user to enable/disable encryption on the device. If user decide to disable encryption (which was enabled before), currently I will not convert those already encrypted files, but user should be able to read them through the device. So... > >> However, I am on a phone, the internal storage is limited, and I am >> using ecryptfs on external micro SD card, the card could be 8/16/32GB, >> which may be a lot larger then the internal storage. >> >> So I can't do a one time copy. Another option is to convert those >> files one by one, however, even I can do it, if the micro SD card is >> unfortunately full, the process could last for hours, and phone may >> run out of battery. It is still hard to control. >> >> So adding this mount option is the lazy way for me. >> >> > >> > I see the usefulness of optionally creating plaintext files at a more >> > granular level. It has always been a goal to create an eCryptfs >> > encryption policy language that could define things like, "User tyhicks >> > uses key with sig deadbeefdeadbeef and aes-128", or "Files with the >> > svirt_image_t type get encrypted by key with sig feedbeeffeedbeef and >> > aes-256", or "Files under ~/public receive no encryption". >> >> Won't ecryptfs header exist in the lower file in such case? > > These decisions would have to be made at file creation time. If the > decision was to not encrypt a file, then no eCryptfs header would be > written out during file creation. I see > >> >> > >> > Another idea that popped up somewhat recently is to have per-mount >> > read and write keyrings. A process can clear their keyrings if they want >> > to do things like write plaintext or read ciphertext. >> > >> > However, I'll need a little more convincing before I see the value of >> > doing this with a mount option. >> >> I've been using the patch for a while, finally decide to send out in >> case someone else has the same requirement. > > I really do appreciate you sending it out. I'm just hesitant to add > another mount opt which modifies the eCryptfs read and/or write paths > unless it is something very useful. It isn't easy to remove mount > options after the fact, so I'd rather err on the side of caution until I > hear of some more interest in this feature. I agree, that's my thought too. Thanks, -Ethan > >> >> Regards, >> -Ethan >> >> > >> > Tyler >> > >> >> >> >> Signed-off-by: Ethan.Du <ethan.too@xxxxxxxxx> >> >> --- >> >> fs/ecryptfs/ecryptfs_kernel.h | 1 + >> >> fs/ecryptfs/inode.c | 8 ++++++++ >> >> fs/ecryptfs/main.c | 5 +++++ >> >> fs/ecryptfs/mmap.c | 33 +++++++++++++++++++++++++-------- >> >> fs/ecryptfs/super.c | 2 ++ >> >> 5 files changed, 41 insertions(+), 8 deletions(-) >> >> >> >> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h >> >> index e007534..4d0c5c4 100644 >> >> --- a/fs/ecryptfs/ecryptfs_kernel.h >> >> +++ b/fs/ecryptfs/ecryptfs_kernel.h >> >> @@ -377,6 +377,7 @@ struct ecryptfs_mount_crypt_stat { >> >> #define ECRYPTFS_GLOBAL_ENCFN_USE_MOUNT_FNEK 0x00000020 >> >> #define ECRYPTFS_GLOBAL_ENCFN_USE_FEK 0x00000040 >> >> #define ECRYPTFS_GLOBAL_MOUNT_AUTH_TOK_ONLY 0x00000080 >> >> +#define ECRYPTFS_CREATE_PLAIN_FILE 0x00000100 >> >> u32 flags; >> >> struct list_head global_auth_tok_list; >> >> struct mutex global_auth_tok_list_mutex; >> >> diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c >> >> index b592938..a6ffe89 100644 >> >> --- a/fs/ecryptfs/inode.c >> >> +++ b/fs/ecryptfs/inode.c >> >> @@ -174,6 +174,9 @@ static int ecryptfs_initialize_file(struct dentry >> >> *ecryptfs_dentry) >> >> { >> >> struct ecryptfs_crypt_stat *crypt_stat = >> >> &ecryptfs_inode_to_private(ecryptfs_dentry->d_inode)->crypt_stat; >> >> + struct ecryptfs_mount_crypt_stat *mount_crypt_stat = >> >> + &ecryptfs_superblock_to_private(ecryptfs_dentry->d_sb)-> >> >> + mount_crypt_stat; >> >> int rc = 0; >> >> >> >> if (S_ISDIR(ecryptfs_dentry->d_inode->i_mode)) { >> >> @@ -182,6 +185,11 @@ static int ecryptfs_initialize_file(struct dentry >> >> *ecryptfs_dentry) >> >> goto out; >> >> } >> >> crypt_stat->flags |= ECRYPTFS_NEW_FILE; >> >> + if (mount_crypt_stat && (mount_crypt_stat->flags >> >> + & ECRYPTFS_CREATE_PLAIN_FILE)) { >> >> + crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED); >> >> + goto out; >> >> + } >> >> ecryptfs_printk(KERN_DEBUG, "Initializing crypto context\n"); >> >> rc = ecryptfs_new_file_context(ecryptfs_dentry); >> >> if (rc) { >> >> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c >> >> index 758323a..ee4e286 100644 >> >> --- a/fs/ecryptfs/main.c >> >> +++ b/fs/ecryptfs/main.c >> >> @@ -218,6 +218,7 @@ enum { ecryptfs_opt_sig, ecryptfs_opt_ecryptfs_sig, >> >> ecryptfs_opt_encrypted_view, ecryptfs_opt_fnek_sig, >> >> ecryptfs_opt_fn_cipher, ecryptfs_opt_fn_cipher_key_bytes, >> >> ecryptfs_opt_unlink_sigs, ecryptfs_opt_mount_auth_tok_only, >> >> + ecryptfs_opt_create_plain, >> >> ecryptfs_opt_err }; >> >> >> >> static const match_table_t tokens = { >> >> @@ -234,6 +235,7 @@ static const match_table_t tokens = { >> >> {ecryptfs_opt_fn_cipher_key_bytes, "ecryptfs_fn_key_bytes=%u"}, >> >> {ecryptfs_opt_unlink_sigs, "ecryptfs_unlink_sigs"}, >> >> {ecryptfs_opt_mount_auth_tok_only, "ecryptfs_mount_auth_tok_only"}, >> >> + {ecryptfs_opt_create_plain, "ecryptfs_create_plain"}, >> >> {ecryptfs_opt_err, NULL} >> >> }; >> >> >> >> @@ -421,6 +423,9 @@ static int ecryptfs_parse_options(struct >> >> ecryptfs_sb_info *sbi, char *options) >> >> mount_crypt_stat->flags |= >> >> ECRYPTFS_GLOBAL_MOUNT_AUTH_TOK_ONLY; >> >> break; >> >> + case ecryptfs_opt_create_plain: >> >> + mount_crypt_stat->flags |= ECRYPTFS_CREATE_PLAIN_FILE; >> >> + break; >> >> case ecryptfs_opt_err: >> >> default: >> >> printk(KERN_WARNING >> >> diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c >> >> index cc64fca..6c5786b 100644 >> >> --- a/fs/ecryptfs/mmap.c >> >> +++ b/fs/ecryptfs/mmap.c >> >> @@ -60,18 +60,35 @@ struct page *ecryptfs_get_locked_page(struct inode >> >> *inode, loff_t index) >> >> */ >> >> static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc) >> >> { >> >> - int rc; >> >> + struct inode *ecryptfs_inode; >> >> + struct ecryptfs_crypt_stat *crypt_stat; >> >> + int rc = 0; >> >> >> >> - rc = ecryptfs_encrypt_page(page); >> >> - if (rc) { >> >> - ecryptfs_printk(KERN_WARNING, "Error encrypting " >> >> + ecryptfs_inode = page->mapping->host; >> >> + crypt_stat = >> >> + &(ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat); >> >> + >> >> + if (!crypt_stat >> >> + || !(crypt_stat->flags & ECRYPTFS_ENCRYPTED) >> >> + || (crypt_stat->flags & ECRYPTFS_NEW_FILE)) { >> >> + ecryptfs_printk(KERN_DEBUG, >> >> + "Passing through unencrypted page\n"); >> >> + rc = ecryptfs_write_lower_page_segment(ecryptfs_inode, page, >> >> + 0, PAGE_CACHE_SIZE); >> >> + } else { >> >> + rc = ecryptfs_encrypt_page(page); >> >> + if (rc) >> >> + ecryptfs_printk(KERN_ERR, "Error encrypting " >> >> "page (upper index [0x%.16lx])\n", page->index); >> >> + } >> >> + >> >> + if (rc) >> >> ClearPageUptodate(page); >> >> - goto out; >> >> + else { >> >> + SetPageUptodate(page); >> >> + unlock_page(page); >> >> } >> >> - SetPageUptodate(page); >> >> - unlock_page(page); >> >> -out: >> >> + >> >> return rc; >> >> } >> >> >> >> diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c >> >> index 3042fe1..dd19570 100644 >> >> --- a/fs/ecryptfs/super.c >> >> +++ b/fs/ecryptfs/super.c >> >> @@ -191,6 +191,8 @@ static int ecryptfs_show_options(struct seq_file >> >> *m, struct vfsmount *mnt) >> >> seq_printf(m, ",ecryptfs_unlink_sigs"); >> >> if (mount_crypt_stat->flags & ECRYPTFS_GLOBAL_MOUNT_AUTH_TOK_ONLY) >> >> seq_printf(m, ",ecryptfs_mount_auth_tok_only"); >> >> + if (mount_crypt_stat->flags & ECRYPTFS_CREATE_PLAIN_FILE) >> >> + seq_printf(m, ",ecryptfs_create_plain"); >> >> >> >> return 0; >> >> } >> >> -- >> >> 1.7.2.3 >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in >> >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html