From bd4f0fe8bb7c73c738e1e11bc90d6e2cf9c6e20e Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Wed, 23 Feb 2011 00:14:19 -0600 Subject: eCryptfs: Remove unnecessary grow_file() function When creating a new eCryptfs file, the crypto metadata is written out and then the lower file was being "grown" with 4 kB of encrypted zeroes. I suspect that growing the encrypted file was to prevent an information leak that the unencrypted file was empty. However, the unencrypted file size is stored, in plaintext, in the metadata so growing the file is unnecessary. Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 1 + fs/ecryptfs/inode.c | 23 ----------------------- 2 files changed, 1 insertion(+), 23 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index bfd8b68..98a2a31 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1389,6 +1389,7 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry) rc = -ENOMEM; goto out; } + /* Zeroed page ensures the in-header unencrypted i_size is set to 0 */ rc = ecryptfs_write_headers_virt(virt, virt_len, &size, crypt_stat, ecryptfs_dentry); if (unlikely(rc)) { diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index b592938..8fa365a 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -143,26 +143,6 @@ out: } /** - * grow_file - * @ecryptfs_dentry: the eCryptfs dentry - * - * This is the code which will grow the file to its correct size. - */ -static int grow_file(struct dentry *ecryptfs_dentry) -{ - struct inode *ecryptfs_inode = ecryptfs_dentry->d_inode; - char zero_virt[] = { 0x00 }; - int rc = 0; - - rc = ecryptfs_write(ecryptfs_inode, zero_virt, 0, 1); - i_size_write(ecryptfs_inode, 0); - rc = ecryptfs_write_inode_size_to_metadata(ecryptfs_inode); - ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat.flags |= - ECRYPTFS_NEW_FILE; - return rc; -} - -/** * ecryptfs_initialize_file * * Cause the file to be changed from a basic empty file to an ecryptfs @@ -202,9 +182,6 @@ static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry) printk(KERN_ERR "Error writing headers; rc = [%d]\n", rc); goto out; } - rc = grow_file(ecryptfs_dentry); - if (rc) - printk(KERN_ERR "Error growing file; rc = [%d]\n", rc); out: return rc; } -- cgit v1.1 From fed8859b3ab94274c986cbdf7d27130e0545f02c Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Wed, 23 Feb 2011 00:54:20 -0600 Subject: eCryptfs: Remove ECRYPTFS_NEW_FILE crypt stat flag Now that grow_file() is not called in the ecryptfs_create() path, the ECRYPTFS_NEW_FILE flag is no longer needed. It helped ecryptfs_readpage() know not to decrypt zeroes that were read from the lower file in the grow_file() path. Signed-off-by: Tyler Hicks --- fs/ecryptfs/ecryptfs_kernel.h | 25 ++++++++++++------------- fs/ecryptfs/inode.c | 1 - fs/ecryptfs/mmap.c | 15 ++------------- 3 files changed, 14 insertions(+), 27 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index e007534..427478e 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -257,19 +257,18 @@ struct ecryptfs_filename { struct ecryptfs_crypt_stat { #define ECRYPTFS_STRUCT_INITIALIZED 0x00000001 #define ECRYPTFS_POLICY_APPLIED 0x00000002 -#define ECRYPTFS_NEW_FILE 0x00000004 -#define ECRYPTFS_ENCRYPTED 0x00000008 -#define ECRYPTFS_SECURITY_WARNING 0x00000010 -#define ECRYPTFS_ENABLE_HMAC 0x00000020 -#define ECRYPTFS_ENCRYPT_IV_PAGES 0x00000040 -#define ECRYPTFS_KEY_VALID 0x00000080 -#define ECRYPTFS_METADATA_IN_XATTR 0x00000100 -#define ECRYPTFS_VIEW_AS_ENCRYPTED 0x00000200 -#define ECRYPTFS_KEY_SET 0x00000400 -#define ECRYPTFS_ENCRYPT_FILENAMES 0x00000800 -#define ECRYPTFS_ENCFN_USE_MOUNT_FNEK 0x00001000 -#define ECRYPTFS_ENCFN_USE_FEK 0x00002000 -#define ECRYPTFS_UNLINK_SIGS 0x00004000 +#define ECRYPTFS_ENCRYPTED 0x00000004 +#define ECRYPTFS_SECURITY_WARNING 0x00000008 +#define ECRYPTFS_ENABLE_HMAC 0x00000010 +#define ECRYPTFS_ENCRYPT_IV_PAGES 0x00000020 +#define ECRYPTFS_KEY_VALID 0x00000040 +#define ECRYPTFS_METADATA_IN_XATTR 0x00000080 +#define ECRYPTFS_VIEW_AS_ENCRYPTED 0x00000100 +#define ECRYPTFS_KEY_SET 0x00000200 +#define ECRYPTFS_ENCRYPT_FILENAMES 0x00000400 +#define ECRYPTFS_ENCFN_USE_MOUNT_FNEK 0x00000800 +#define ECRYPTFS_ENCFN_USE_FEK 0x00001000 +#define ECRYPTFS_UNLINK_SIGS 0x00002000 u32 flags; unsigned int file_version; size_t iv_bytes; diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 8fa365a..f99051b 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -161,7 +161,6 @@ static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry) crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED); goto out; } - crypt_stat->flags |= ECRYPTFS_NEW_FILE; ecryptfs_printk(KERN_DEBUG, "Initializing crypto context\n"); rc = ecryptfs_new_file_context(ecryptfs_dentry); if (rc) { diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index cc64fca..519af27 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -193,11 +193,7 @@ static int ecryptfs_readpage(struct file *file, struct page *page) &ecryptfs_inode_to_private(page->mapping->host)->crypt_stat; int rc = 0; - if (!crypt_stat - || !(crypt_stat->flags & ECRYPTFS_ENCRYPTED) - || (crypt_stat->flags & ECRYPTFS_NEW_FILE)) { - ecryptfs_printk(KERN_DEBUG, - "Passing through unencrypted page\n"); + if (!crypt_stat || !(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) { rc = ecryptfs_read_lower_page_segment(page, page->index, 0, PAGE_CACHE_SIZE, page->mapping->host); @@ -295,8 +291,7 @@ static int ecryptfs_write_begin(struct file *file, struct ecryptfs_crypt_stat *crypt_stat = &ecryptfs_inode_to_private(mapping->host)->crypt_stat; - if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED) - || (crypt_stat->flags & ECRYPTFS_NEW_FILE)) { + if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) { rc = ecryptfs_read_lower_page_segment( page, index, 0, PAGE_CACHE_SIZE, mapping->host); if (rc) { @@ -487,12 +482,6 @@ static int ecryptfs_write_end(struct file *file, &ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat; int rc; - if (crypt_stat->flags & ECRYPTFS_NEW_FILE) { - ecryptfs_printk(KERN_DEBUG, "ECRYPTFS_NEW_FILE flag set in " - "crypt_stat at memory location [%p]\n", crypt_stat); - crypt_stat->flags &= ~(ECRYPTFS_NEW_FILE); - } else - ecryptfs_printk(KERN_DEBUG, "Not a new file\n"); ecryptfs_printk(KERN_DEBUG, "Calling fill_zeros_to_end_of_page" "(page w/ index = [0x%.16lx], to = [%d])\n", index, to); if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) { -- cgit v1.1 From 57db4e8d73ef2b5e94a3f412108dff2576670a8a Mon Sep 17 00:00:00 2001 From: Thieu Le Date: Tue, 8 Mar 2011 16:26:03 -0800 Subject: ecryptfs: modify write path to encrypt page in writepage Change the write path to encrypt the data only when the page is written to disk in ecryptfs_writepage. Previously, ecryptfs encrypts the page in ecryptfs_write_end which means that if there are multiple write requests to the same page, ecryptfs ends up re-encrypting that page over and over again. This patch minimizes the number of encryptions needed. Signed-off-by: Thieu Le [tyhicks: Changed NULL .drop_inode sop pointer to generic_drop_inode] Signed-off-by: Tyler Hicks --- fs/ecryptfs/ecryptfs_kernel.h | 1 - fs/ecryptfs/file.c | 9 ++++++++- fs/ecryptfs/main.c | 2 -- fs/ecryptfs/mmap.c | 41 +++++++++++++++++++++++++++-------------- fs/ecryptfs/read_write.c | 12 ++---------- fs/ecryptfs/super.c | 3 +-- 6 files changed, 38 insertions(+), 30 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 427478e..3ef5b00 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -296,7 +296,6 @@ struct ecryptfs_inode_info { struct inode vfs_inode; struct inode *wii_inode; struct file *lower_file; - struct mutex lower_file_mutex; struct ecryptfs_crypt_stat crypt_stat; }; diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 7d1050e..cedc913 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -273,7 +273,14 @@ static int ecryptfs_release(struct inode *inode, struct file *file) static int ecryptfs_fsync(struct file *file, int datasync) { - return vfs_fsync(ecryptfs_file_to_lower(file), datasync); + int rc = 0; + + rc = generic_file_fsync(file, datasync); + if (rc) + goto out; + rc = vfs_fsync(ecryptfs_file_to_lower(file), datasync); +out: + return rc; } static int ecryptfs_fasync(int fd, struct file *file, int flag) diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 758323a..63e412c 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -122,7 +122,6 @@ int ecryptfs_init_persistent_file(struct dentry *ecryptfs_dentry) ecryptfs_inode_to_private(ecryptfs_dentry->d_inode); int rc = 0; - mutex_lock(&inode_info->lower_file_mutex); if (!inode_info->lower_file) { struct dentry *lower_dentry; struct vfsmount *lower_mnt = @@ -138,7 +137,6 @@ int ecryptfs_init_persistent_file(struct dentry *ecryptfs_dentry) inode_info->lower_file = NULL; } } - mutex_unlock(&inode_info->lower_file_mutex); return rc; } diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 519af27..5e15013 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -62,6 +62,18 @@ static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc) { int rc; + /* + * Refuse to write the page out if we are called from reclaim context + * since our writepage() path may potentially allocate memory when + * calling into the lower fs vfs_write() which may in turn invoke + * us again. + */ + if (current->flags & PF_MEMALLOC) { + redirty_page_for_writepage(wbc, page); + rc = 0; + goto out; + } + rc = ecryptfs_encrypt_page(page); if (rc) { ecryptfs_printk(KERN_WARNING, "Error encrypting " @@ -70,8 +82,8 @@ static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc) goto out; } SetPageUptodate(page); - unlock_page(page); out: + unlock_page(page); return rc; } @@ -481,6 +493,7 @@ static int ecryptfs_write_end(struct file *file, struct ecryptfs_crypt_stat *crypt_stat = &ecryptfs_inode_to_private(ecryptfs_inode)->crypt_stat; int rc; + int need_unlock_page = 1; ecryptfs_printk(KERN_DEBUG, "Calling fill_zeros_to_end_of_page" "(page w/ index = [0x%.16lx], to = [%d])\n", index, to); @@ -501,26 +514,26 @@ static int ecryptfs_write_end(struct file *file, "zeros in page with index = [0x%.16lx]\n", index); goto out; } - rc = ecryptfs_encrypt_page(page); - if (rc) { - ecryptfs_printk(KERN_WARNING, "Error encrypting page (upper " - "index [0x%.16lx])\n", index); - goto out; - } + set_page_dirty(page); + unlock_page(page); + need_unlock_page = 0; if (pos + copied > i_size_read(ecryptfs_inode)) { i_size_write(ecryptfs_inode, pos + copied); ecryptfs_printk(KERN_DEBUG, "Expanded file size to " "[0x%.16llx]\n", (unsigned long long)i_size_read(ecryptfs_inode)); + balance_dirty_pages_ratelimited(mapping); + rc = ecryptfs_write_inode_size_to_metadata(ecryptfs_inode); + if (rc) { + printk(KERN_ERR "Error writing inode size to metadata; " + "rc = [%d]\n", rc); + goto out; + } } - rc = ecryptfs_write_inode_size_to_metadata(ecryptfs_inode); - if (rc) - printk(KERN_ERR "Error writing inode size to metadata; " - "rc = [%d]\n", rc); - else - rc = copied; + rc = copied; out: - unlock_page(page); + if (need_unlock_page) + unlock_page(page); page_cache_release(page); return rc; } diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c index db184ef..85d4309 100644 --- a/fs/ecryptfs/read_write.c +++ b/fs/ecryptfs/read_write.c @@ -44,15 +44,11 @@ int ecryptfs_write_lower(struct inode *ecryptfs_inode, char *data, ssize_t rc; inode_info = ecryptfs_inode_to_private(ecryptfs_inode); - mutex_lock(&inode_info->lower_file_mutex); BUG_ON(!inode_info->lower_file); - inode_info->lower_file->f_pos = offset; fs_save = get_fs(); set_fs(get_ds()); - rc = vfs_write(inode_info->lower_file, data, size, - &inode_info->lower_file->f_pos); + rc = vfs_write(inode_info->lower_file, data, size, &offset); set_fs(fs_save); - mutex_unlock(&inode_info->lower_file_mutex); mark_inode_dirty_sync(ecryptfs_inode); return rc; } @@ -234,15 +230,11 @@ int ecryptfs_read_lower(char *data, loff_t offset, size_t size, mm_segment_t fs_save; ssize_t rc; - mutex_lock(&inode_info->lower_file_mutex); BUG_ON(!inode_info->lower_file); - inode_info->lower_file->f_pos = offset; fs_save = get_fs(); set_fs(get_ds()); - rc = vfs_read(inode_info->lower_file, data, size, - &inode_info->lower_file->f_pos); + rc = vfs_read(inode_info->lower_file, data, size, &offset); set_fs(fs_save); - mutex_unlock(&inode_info->lower_file_mutex); return rc; } diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c index 3042fe1..bacc882 100644 --- a/fs/ecryptfs/super.c +++ b/fs/ecryptfs/super.c @@ -55,7 +55,6 @@ static struct inode *ecryptfs_alloc_inode(struct super_block *sb) if (unlikely(!inode_info)) goto out; ecryptfs_init_crypt_stat(&inode_info->crypt_stat); - mutex_init(&inode_info->lower_file_mutex); inode_info->lower_file = NULL; inode = &inode_info->vfs_inode; out: @@ -198,7 +197,7 @@ static int ecryptfs_show_options(struct seq_file *m, struct vfsmount *mnt) const struct super_operations ecryptfs_sops = { .alloc_inode = ecryptfs_alloc_inode, .destroy_inode = ecryptfs_destroy_inode, - .drop_inode = generic_delete_inode, + .drop_inode = generic_drop_inode, .statfs = ecryptfs_statfs, .remount_fs = NULL, .evict_inode = ecryptfs_evict_inode, -- cgit v1.1 From 50f198ae16ac66508d4b8d5a40967a8507ad19ee Mon Sep 17 00:00:00 2001 From: Tyler Hicks Date: Wed, 9 Mar 2011 11:49:13 -0600 Subject: eCryptfs: Unlock page in write_begin error path Unlock the page in error path of ecryptfs_write_begin(). This may happen, for example, if decryption fails while bring the page up-to-date. Cc: Signed-off-by: Tyler Hicks --- fs/ecryptfs/mmap.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs') diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 5e15013..6a44148 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -381,6 +381,11 @@ static int ecryptfs_write_begin(struct file *file, && (pos != 0)) zero_user(page, 0, PAGE_CACHE_SIZE); out: + if (unlikely(rc)) { + unlock_page(page); + page_cache_release(page); + *pagep = NULL; + } return rc; } -- cgit v1.1 From 1821df040ac3cd6a57518739f345da6d50ea9d3f Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Thu, 17 Mar 2011 12:48:50 +0100 Subject: eCryptfs: ecryptfs_keyring_auth_tok_for_sig() bug fix The pointer '(*auth_tok_key)' is set to NULL in case request_key() fails, in order to prevent its use by functions calling ecryptfs_keyring_auth_tok_for_sig(). Signed-off-by: Roberto Sassu Cc: Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs') diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index c1436cf..4feb78c 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -1563,6 +1563,7 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, printk(KERN_ERR "Could not find key with description: [%s]\n", sig); rc = process_request_key_err(PTR_ERR(*auth_tok_key)); + (*auth_tok_key) = NULL; goto out; } (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key); -- cgit v1.1 From cf35ca69131d5fc8febb74629d173e0731bf49c0 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 21 Mar 2011 16:00:51 +0100 Subject: eCryptfs: removed num_global_auth_toks from ecryptfs_mount_crypt_stat This patch removes the 'num_global_auth_toks' field of the ecryptfs_mount_crypt_stat structure, used to count the number of items in the 'global_auth_tok_list' list. This variable is not needed because there are no checks based upon it. Signed-off-by: Roberto Sassu Signed-off-by: Tyler Hicks --- fs/ecryptfs/crypto.c | 1 - fs/ecryptfs/ecryptfs_kernel.h | 1 - fs/ecryptfs/keystore.c | 1 - 3 files changed, 3 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 98a2a31..d2a70a4 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -266,7 +266,6 @@ void ecryptfs_destroy_mount_crypt_stat( &mount_crypt_stat->global_auth_tok_list, mount_crypt_stat_list) { list_del(&auth_tok->mount_crypt_stat_list); - mount_crypt_stat->num_global_auth_toks--; if (auth_tok->global_auth_tok_key && !(auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID)) key_put(auth_tok->global_auth_tok_key); diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 3ef5b00..6c6b826 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -378,7 +378,6 @@ struct ecryptfs_mount_crypt_stat { u32 flags; struct list_head global_auth_tok_list; struct mutex global_auth_tok_list_mutex; - size_t num_global_auth_toks; size_t global_default_cipher_key_size; size_t global_default_fn_cipher_key_bytes; unsigned char global_default_cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 4feb78c..523e51d 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -2454,7 +2454,6 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat, mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex); list_add(&new_auth_tok->mount_crypt_stat_list, &mount_crypt_stat->global_auth_tok_list); - mount_crypt_stat->num_global_auth_toks++; mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex); out: return rc; -- cgit v1.1 From 7762e230fd31fcc1abc03ba32ee957fadc8eafb4 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 21 Mar 2011 16:00:52 +0100 Subject: eCryptfs: modified size of keysig in the ecryptfs_key_sig structure The size of the 'keysig' array is incremented of one byte in order to make room for the NULL character. The 'keysig' variable is used, in the function ecryptfs_generate_key_packet_set(), to find an authentication token with the given signature and is printed a debug message if it cannot be retrieved. Signed-off-by: Roberto Sassu Signed-off-by: Tyler Hicks --- fs/ecryptfs/ecryptfs_kernel.h | 2 +- fs/ecryptfs/keystore.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 6c6b826..40c93fe 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -233,7 +233,7 @@ ecryptfs_get_key_payload_data(struct key *key) struct ecryptfs_key_sig { struct list_head crypt_stat_list; - char keysig[ECRYPTFS_SIG_SIZE_HEX]; + char keysig[ECRYPTFS_SIG_SIZE_HEX + 1]; }; struct ecryptfs_filename { diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 523e51d..bd139df 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -2425,6 +2425,7 @@ int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig) return -ENOMEM; } memcpy(new_key_sig->keysig, sig, ECRYPTFS_SIG_SIZE_HEX); + new_key_sig->keysig[ECRYPTFS_SIG_SIZE_HEX] = '\0'; /* Caller must hold keysig_list_mutex */ list_add(&new_key_sig->crypt_stat_list, &crypt_stat->keysig_list); -- cgit v1.1 From 0e1fc5ef470cc1d157005c437a434868d59fead4 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 21 Mar 2011 16:00:53 +0100 Subject: eCryptfs: verify authentication tokens before their use Authentication tokens content may change if another requestor calls the update() method of the corresponding key. The new function ecryptfs_verify_auth_tok_from_key() retrieves the authentication token from the provided key and verifies if it is still valid before being used to encrypt or decrypt an eCryptfs file. Signed-off-by: Roberto Sassu [tyhicks: Minor formatting changes] Signed-off-by: Tyler Hicks --- fs/ecryptfs/ecryptfs_kernel.h | 1 - fs/ecryptfs/keystore.c | 211 ++++++++++++++++++++++++++---------------- fs/ecryptfs/main.c | 4 +- 3 files changed, 135 insertions(+), 81 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 40c93fe..bd3cafd 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -331,7 +331,6 @@ struct ecryptfs_global_auth_tok { u32 flags; struct list_head mount_crypt_stat_list; struct key *global_auth_tok_key; - struct ecryptfs_auth_tok *global_auth_tok; unsigned char sig[ECRYPTFS_SIG_SIZE_HEX + 1]; }; diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index bd139df..3c4039c 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -65,6 +65,24 @@ static int process_request_key_err(long err_code) return rc; } +static int process_find_global_auth_tok_for_sig_err(int err_code) +{ + int rc = err_code; + + switch (err_code) { + case -ENOENT: + ecryptfs_printk(KERN_WARNING, "Missing auth tok\n"); + break; + case -EINVAL: + ecryptfs_printk(KERN_WARNING, "Invalid auth tok\n"); + break; + default: + rc = process_request_key_err(err_code); + break; + } + return rc; +} + /** * ecryptfs_parse_packet_length * @data: Pointer to memory containing length at offset @@ -403,27 +421,117 @@ out: return rc; } +/** + * ecryptfs_verify_version + * @version: The version number to confirm + * + * Returns zero on good version; non-zero otherwise + */ +static int ecryptfs_verify_version(u16 version) +{ + int rc = 0; + unsigned char major; + unsigned char minor; + + major = ((version >> 8) & 0xFF); + minor = (version & 0xFF); + if (major != ECRYPTFS_VERSION_MAJOR) { + ecryptfs_printk(KERN_ERR, "Major version number mismatch. " + "Expected [%d]; got [%d]\n", + ECRYPTFS_VERSION_MAJOR, major); + rc = -EINVAL; + goto out; + } + if (minor != ECRYPTFS_VERSION_MINOR) { + ecryptfs_printk(KERN_ERR, "Minor version number mismatch. " + "Expected [%d]; got [%d]\n", + ECRYPTFS_VERSION_MINOR, minor); + rc = -EINVAL; + goto out; + } +out: + return rc; +} + +/** + * ecryptfs_verify_auth_tok_from_key + * @auth_tok_key: key containing the authentication token + * @auth_tok: authentication token + * + * Returns zero on valid auth tok; -EINVAL otherwise + */ +static int +ecryptfs_verify_auth_tok_from_key(struct key *auth_tok_key, + struct ecryptfs_auth_tok **auth_tok) +{ + int rc = 0; + + (*auth_tok) = ecryptfs_get_key_payload_data(auth_tok_key); + if (ecryptfs_verify_version((*auth_tok)->version)) { + printk(KERN_ERR "Data structure version mismatch. Userspace " + "tools must match eCryptfs kernel module with major " + "version [%d] and minor version [%d]\n", + ECRYPTFS_VERSION_MAJOR, ECRYPTFS_VERSION_MINOR); + rc = -EINVAL; + goto out; + } + if ((*auth_tok)->token_type != ECRYPTFS_PASSWORD + && (*auth_tok)->token_type != ECRYPTFS_PRIVATE_KEY) { + printk(KERN_ERR "Invalid auth_tok structure " + "returned from key query\n"); + rc = -EINVAL; + goto out; + } +out: + return rc; +} + static int ecryptfs_find_global_auth_tok_for_sig( - struct ecryptfs_global_auth_tok **global_auth_tok, + struct key **auth_tok_key, + struct ecryptfs_auth_tok **auth_tok, struct ecryptfs_mount_crypt_stat *mount_crypt_stat, char *sig) { struct ecryptfs_global_auth_tok *walker; int rc = 0; - (*global_auth_tok) = NULL; + (*auth_tok_key) = NULL; + (*auth_tok) = NULL; mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex); list_for_each_entry(walker, &mount_crypt_stat->global_auth_tok_list, mount_crypt_stat_list) { - if (memcmp(walker->sig, sig, ECRYPTFS_SIG_SIZE_HEX) == 0) { - rc = key_validate(walker->global_auth_tok_key); - if (!rc) - (*global_auth_tok) = walker; + if (memcmp(walker->sig, sig, ECRYPTFS_SIG_SIZE_HEX)) + continue; + + if (walker->flags & ECRYPTFS_AUTH_TOK_INVALID) { + rc = -EINVAL; goto out; } + + rc = key_validate(walker->global_auth_tok_key); + if (rc) { + if (rc == -EKEYEXPIRED) + goto out; + goto out_invalid_auth_tok; + } + + rc = ecryptfs_verify_auth_tok_from_key( + walker->global_auth_tok_key, auth_tok); + if (rc) + goto out_invalid_auth_tok; + + (*auth_tok_key) = walker->global_auth_tok_key; + key_get(*auth_tok_key); + goto out; } - rc = -EINVAL; + rc = -ENOENT; + goto out; +out_invalid_auth_tok: + printk(KERN_WARNING "Invalidating auth tok with sig = [%s]\n", sig); + walker->flags |= ECRYPTFS_AUTH_TOK_INVALID; + key_put(walker->global_auth_tok_key); + walker->global_auth_tok_key = NULL; out: mutex_unlock(&mount_crypt_stat->global_auth_tok_list_mutex); return rc; @@ -451,14 +559,11 @@ ecryptfs_find_auth_tok_for_sig( struct ecryptfs_mount_crypt_stat *mount_crypt_stat, char *sig) { - struct ecryptfs_global_auth_tok *global_auth_tok; int rc = 0; - (*auth_tok_key) = NULL; - (*auth_tok) = NULL; - if (ecryptfs_find_global_auth_tok_for_sig(&global_auth_tok, - mount_crypt_stat, sig)) { - + rc = ecryptfs_find_global_auth_tok_for_sig(auth_tok_key, auth_tok, + mount_crypt_stat, sig); + if (rc == -ENOENT) { /* if the flag ECRYPTFS_GLOBAL_MOUNT_AUTH_TOK_ONLY is set in the * mount_crypt_stat structure, we prevent to use auth toks that * are not inserted through the ecryptfs_add_global_auth_tok @@ -470,8 +575,7 @@ ecryptfs_find_auth_tok_for_sig( rc = ecryptfs_keyring_auth_tok_for_sig(auth_tok_key, auth_tok, sig); - } else - (*auth_tok) = global_auth_tok->global_auth_tok; + } return rc; } @@ -1520,38 +1624,6 @@ out: return rc; } -/** - * ecryptfs_verify_version - * @version: The version number to confirm - * - * Returns zero on good version; non-zero otherwise - */ -static int ecryptfs_verify_version(u16 version) -{ - int rc = 0; - unsigned char major; - unsigned char minor; - - major = ((version >> 8) & 0xFF); - minor = (version & 0xFF); - if (major != ECRYPTFS_VERSION_MAJOR) { - ecryptfs_printk(KERN_ERR, "Major version number mismatch. " - "Expected [%d]; got [%d]\n", - ECRYPTFS_VERSION_MAJOR, major); - rc = -EINVAL; - goto out; - } - if (minor != ECRYPTFS_VERSION_MINOR) { - ecryptfs_printk(KERN_ERR, "Minor version number mismatch. " - "Expected [%d]; got [%d]\n", - ECRYPTFS_VERSION_MINOR, minor); - rc = -EINVAL; - goto out; - } -out: - return rc; -} - int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, struct ecryptfs_auth_tok **auth_tok, char *sig) @@ -1566,29 +1638,12 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, (*auth_tok_key) = NULL; goto out; } - (*auth_tok) = ecryptfs_get_key_payload_data(*auth_tok_key); - if (ecryptfs_verify_version((*auth_tok)->version)) { - printk(KERN_ERR - "Data structure version mismatch. " - "Userspace tools must match eCryptfs " - "kernel module with major version [%d] " - "and minor version [%d]\n", - ECRYPTFS_VERSION_MAJOR, - ECRYPTFS_VERSION_MINOR); - rc = -EINVAL; - goto out_release_key; - } - if ((*auth_tok)->token_type != ECRYPTFS_PASSWORD - && (*auth_tok)->token_type != ECRYPTFS_PRIVATE_KEY) { - printk(KERN_ERR "Invalid auth_tok structure " - "returned from key query\n"); - rc = -EINVAL; - goto out_release_key; - } -out_release_key: + + rc = ecryptfs_verify_auth_tok_from_key(*auth_tok_key, auth_tok); if (rc) { key_put(*auth_tok_key); (*auth_tok_key) = NULL; + goto out; } out: return rc; @@ -2325,7 +2380,7 @@ ecryptfs_generate_key_packet_set(char *dest_base, size_t max) { struct ecryptfs_auth_tok *auth_tok; - struct ecryptfs_global_auth_tok *global_auth_tok; + struct key *auth_tok_key = NULL; struct ecryptfs_mount_crypt_stat *mount_crypt_stat = &ecryptfs_superblock_to_private( ecryptfs_dentry->d_sb)->mount_crypt_stat; @@ -2344,21 +2399,16 @@ ecryptfs_generate_key_packet_set(char *dest_base, list_for_each_entry(key_sig, &crypt_stat->keysig_list, crypt_stat_list) { memset(key_rec, 0, sizeof(*key_rec)); - rc = ecryptfs_find_global_auth_tok_for_sig(&global_auth_tok, + rc = ecryptfs_find_global_auth_tok_for_sig(&auth_tok_key, + &auth_tok, mount_crypt_stat, key_sig->keysig); if (rc) { - printk(KERN_ERR "Error attempting to get the global " - "auth_tok; rc = [%d]\n", rc); + printk(KERN_WARNING "Unable to retrieve auth tok with " + "sig = [%s]\n", key_sig->keysig); + rc = process_find_global_auth_tok_for_sig_err(rc); goto out_free; } - if (global_auth_tok->flags & ECRYPTFS_AUTH_TOK_INVALID) { - printk(KERN_WARNING - "Skipping invalid auth tok with sig = [%s]\n", - global_auth_tok->sig); - continue; - } - auth_tok = global_auth_tok->global_auth_tok; if (auth_tok->token_type == ECRYPTFS_PASSWORD) { rc = write_tag_3_packet((dest_base + (*len)), &max, auth_tok, @@ -2396,6 +2446,8 @@ ecryptfs_generate_key_packet_set(char *dest_base, rc = -EINVAL; goto out_free; } + key_put(auth_tok_key); + auth_tok_key = NULL; } if (likely(max > 0)) { dest_base[(*len)] = 0x00; @@ -2408,6 +2460,9 @@ out_free: out: if (rc) (*len) = 0; + if (auth_tok_key) + key_put(auth_tok_key); + mutex_unlock(&crypt_stat->keysig_list_mutex); return rc; } diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 63e412c..520d05f 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -239,14 +239,14 @@ static int ecryptfs_init_global_auth_toks( struct ecryptfs_mount_crypt_stat *mount_crypt_stat) { struct ecryptfs_global_auth_tok *global_auth_tok; + struct ecryptfs_auth_tok *auth_tok; int rc = 0; list_for_each_entry(global_auth_tok, &mount_crypt_stat->global_auth_tok_list, mount_crypt_stat_list) { rc = ecryptfs_keyring_auth_tok_for_sig( - &global_auth_tok->global_auth_tok_key, - &global_auth_tok->global_auth_tok, + &global_auth_tok->global_auth_tok_key, &auth_tok, global_auth_tok->sig); if (rc) { printk(KERN_ERR "Could not find valid key in user " -- cgit v1.1 From 950983fc04e02232e0d25717903461578a755ebb Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 21 Mar 2011 16:00:54 +0100 Subject: eCryptfs: move ecryptfs_find_auth_tok_for_sig() call before mutex_lock The ecryptfs_find_auth_tok_for_sig() call is moved before the mutex_lock(s->tfm_mutex) instruction in order to avoid possible deadlocks that may occur by holding the lock on the two semaphores 'key->sem' and 's->tfm_mutex' in reverse order. Signed-off-by: Roberto Sassu Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index 3c4039c..d95dd50 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -635,6 +635,16 @@ ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes, } s->desc.flags = CRYPTO_TFM_REQ_MAY_SLEEP; (*packet_size) = 0; + rc = ecryptfs_find_auth_tok_for_sig( + &auth_tok_key, + &s->auth_tok, mount_crypt_stat, + mount_crypt_stat->global_default_fnek_sig); + if (rc) { + printk(KERN_ERR "%s: Error attempting to find auth tok for " + "fnek sig [%s]; rc = [%d]\n", __func__, + mount_crypt_stat->global_default_fnek_sig, rc); + goto out; + } rc = ecryptfs_get_tfm_and_mutex_for_cipher_name( &s->desc.tfm, &s->tfm_mutex, mount_crypt_stat->global_default_fn_cipher_name); @@ -720,16 +730,6 @@ ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes, goto out_free_unlock; } dest[s->i++] = s->cipher_code; - rc = ecryptfs_find_auth_tok_for_sig( - &auth_tok_key, - &s->auth_tok, mount_crypt_stat, - mount_crypt_stat->global_default_fnek_sig); - if (rc) { - printk(KERN_ERR "%s: Error attempting to find auth tok for " - "fnek sig [%s]; rc = [%d]\n", __func__, - mount_crypt_stat->global_default_fnek_sig, rc); - goto out_free_unlock; - } /* TODO: Support other key modules than passphrase for * filename encryption */ if (s->auth_tok->token_type != ECRYPTFS_PASSWORD) { @@ -983,6 +983,15 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size, __func__, s->cipher_code); goto out; } + rc = ecryptfs_find_auth_tok_for_sig(&auth_tok_key, + &s->auth_tok, mount_crypt_stat, + s->fnek_sig_hex); + if (rc) { + printk(KERN_ERR "%s: Error attempting to find auth tok for " + "fnek sig [%s]; rc = [%d]\n", __func__, s->fnek_sig_hex, + rc); + goto out; + } rc = ecryptfs_get_tfm_and_mutex_for_cipher_name(&s->desc.tfm, &s->tfm_mutex, s->cipher_string); @@ -1029,15 +1038,6 @@ ecryptfs_parse_tag_70_packet(char **filename, size_t *filename_size, * >= ECRYPTFS_MAX_IV_BYTES. */ memset(s->iv, 0, ECRYPTFS_MAX_IV_BYTES); s->desc.info = s->iv; - rc = ecryptfs_find_auth_tok_for_sig(&auth_tok_key, - &s->auth_tok, mount_crypt_stat, - s->fnek_sig_hex); - if (rc) { - printk(KERN_ERR "%s: Error attempting to find auth tok for " - "fnek sig [%s]; rc = [%d]\n", __func__, s->fnek_sig_hex, - rc); - goto out_free_unlock; - } /* TODO: Support other key modules than passphrase for * filename encryption */ if (s->auth_tok->token_type != ECRYPTFS_PASSWORD) { -- cgit v1.1 From b5695d04634fa4ccca7dcbc05bb4a66522f02e0b Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Mon, 21 Mar 2011 16:00:55 +0100 Subject: eCryptfs: write lock requested keys A requested key is write locked in order to prevent modifications on the authentication token while it is being used. Signed-off-by: Roberto Sassu Signed-off-by: Tyler Hicks --- fs/ecryptfs/keystore.c | 26 ++++++++++++++++++++------ fs/ecryptfs/main.c | 4 +++- 2 files changed, 23 insertions(+), 7 deletions(-) (limited to 'fs') diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c index d95dd50..03e609c 100644 --- a/fs/ecryptfs/keystore.c +++ b/fs/ecryptfs/keystore.c @@ -516,10 +516,11 @@ ecryptfs_find_global_auth_tok_for_sig( goto out_invalid_auth_tok; } + down_write(&(walker->global_auth_tok_key->sem)); rc = ecryptfs_verify_auth_tok_from_key( walker->global_auth_tok_key, auth_tok); if (rc) - goto out_invalid_auth_tok; + goto out_invalid_auth_tok_unlock; (*auth_tok_key) = walker->global_auth_tok_key; key_get(*auth_tok_key); @@ -527,6 +528,8 @@ ecryptfs_find_global_auth_tok_for_sig( } rc = -ENOENT; goto out; +out_invalid_auth_tok_unlock: + up_write(&(walker->global_auth_tok_key->sem)); out_invalid_auth_tok: printk(KERN_WARNING "Invalidating auth tok with sig = [%s]\n", sig); walker->flags |= ECRYPTFS_AUTH_TOK_INVALID; @@ -869,8 +872,10 @@ out_free_unlock: out_unlock: mutex_unlock(s->tfm_mutex); out: - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } kfree(s); return rc; } @@ -1106,8 +1111,10 @@ out: (*filename_size) = 0; (*filename) = NULL; } - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } kfree(s); return rc; } @@ -1638,9 +1645,10 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, (*auth_tok_key) = NULL; goto out; } - + down_write(&(*auth_tok_key)->sem); rc = ecryptfs_verify_auth_tok_from_key(*auth_tok_key, auth_tok); if (rc) { + up_write(&(*auth_tok_key)->sem); key_put(*auth_tok_key); (*auth_tok_key) = NULL; goto out; @@ -1865,6 +1873,7 @@ int ecryptfs_parse_packet_set(struct ecryptfs_crypt_stat *crypt_stat, find_next_matching_auth_tok: found_auth_tok = 0; if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); auth_tok_key = NULL; } @@ -1951,8 +1960,10 @@ found_matching_auth_tok: out_wipe_list: wipe_auth_tok_list(&auth_tok_list); out: - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } return rc; } @@ -2446,6 +2457,7 @@ ecryptfs_generate_key_packet_set(char *dest_base, rc = -EINVAL; goto out_free; } + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); auth_tok_key = NULL; } @@ -2460,8 +2472,10 @@ out_free: out: if (rc) (*len) = 0; - if (auth_tok_key) + if (auth_tok_key) { + up_write(&(auth_tok_key->sem)); key_put(auth_tok_key); + } mutex_unlock(&crypt_stat->keysig_list_mutex); return rc; diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 520d05f..c27c0ec 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -254,8 +254,10 @@ static int ecryptfs_init_global_auth_toks( "option: [%s]\n", global_auth_tok->sig); global_auth_tok->flags |= ECRYPTFS_AUTH_TOK_INVALID; goto out; - } else + } else { global_auth_tok->flags &= ~ECRYPTFS_AUTH_TOK_INVALID; + up_write(&(global_auth_tok->global_auth_tok_key)->sem); + } } out: return rc; -- cgit v1.1