Re: [PATCH] eCryptfs: support creating plain files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux