eCryptfs has file encryption keys (FEK), file encryption key encryption
keys (FEKEK), and filename encryption keys (FNEK). The per-file FEK is
encrypted with one or more FEKEKs and stored in the header of the
encrypted file. I noticed that the FEK is also being encrypted by the
FNEK. This is a problem if a user wants to use a different FNEK than
their FEKEK, as their file contents will still be accessible with the
FNEK.
This is a minimalistic patch which prevents the FNEKs signatures from
being copied to the inode signatures list. Ultimately, it keeps the FEK
from being encrypted with a FNEK.
Signed-off-by: Tyler Hicks <[email protected]>
---
fs/ecryptfs/crypto.c | 2 ++
fs/ecryptfs/ecryptfs_kernel.h | 3 ++-
fs/ecryptfs/keystore.c | 3 ++-
fs/ecryptfs/main.c | 5 +++--
4 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index f6caeb1..bdca1f4 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -946,6 +946,8 @@ static int ecryptfs_copy_mount_wide_sigs_to_inode_sigs(
list_for_each_entry(global_auth_tok,
&mount_crypt_stat->global_auth_tok_list,
mount_crypt_stat_list) {
+ if (global_auth_tok->flags & ECRYPTFS_AUTH_TOK_FNEK)
+ continue;
rc = ecryptfs_add_keysig(crypt_stat, global_auth_tok->sig);
if (rc) {
printk(KERN_ERR "Error adding keysig; rc = [%d]\n", rc);
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
index c11fc95..eb2267e 100644
--- a/fs/ecryptfs/ecryptfs_kernel.h
+++ b/fs/ecryptfs/ecryptfs_kernel.h
@@ -328,6 +328,7 @@ struct ecryptfs_dentry_info {
*/
struct ecryptfs_global_auth_tok {
#define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
+#define ECRYPTFS_AUTH_TOK_FNEK 0x00000002
u32 flags;
struct list_head mount_crypt_stat_list;
struct key *global_auth_tok_key;
@@ -696,7 +697,7 @@ ecryptfs_write_header_metadata(char *virt,
int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig);
int
ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
- char *sig);
+ char *sig, u32 global_auth_tok_flags);
int ecryptfs_get_global_auth_tok_for_sig(
struct ecryptfs_global_auth_tok **global_auth_tok,
struct ecryptfs_mount_crypt_stat *mount_crypt_stat, char *sig);
diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
index ff53942..e4a6223 100644
--- a/fs/ecryptfs/keystore.c
+++ b/fs/ecryptfs/keystore.c
@@ -2375,7 +2375,7 @@ struct kmem_cache *ecryptfs_global_auth_tok_cache;
int
ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
- char *sig)
+ char *sig, u32 global_auth_tok_flags)
{
struct ecryptfs_global_auth_tok *new_auth_tok;
int rc = 0;
@@ -2389,6 +2389,7 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
goto out;
}
memcpy(new_auth_tok->sig, sig, ECRYPTFS_SIG_SIZE_HEX);
+ new_auth_tok->flags = global_auth_tok_flags;
new_auth_tok->sig[ECRYPTFS_SIG_SIZE_HEX] = '\0';
mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
list_add(&new_auth_tok->mount_crypt_stat_list,
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
index 789cf2e..aed56c2 100644
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -319,7 +319,7 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
case ecryptfs_opt_ecryptfs_sig:
sig_src = args[0].from;
rc = ecryptfs_add_global_auth_tok(mount_crypt_stat,
- sig_src);
+ sig_src, 0);
if (rc) {
printk(KERN_ERR "Error attempting to register "
"global sig; rc = [%d]\n", rc);
@@ -370,7 +370,8 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
ECRYPTFS_SIG_SIZE_HEX] = '\0';
rc = ecryptfs_add_global_auth_tok(
mount_crypt_stat,
- mount_crypt_stat->global_default_fnek_sig);
+ mount_crypt_stat->global_default_fnek_sig,
+ ECRYPTFS_AUTH_TOK_FNEK);
if (rc) {
printk(KERN_ERR "Error attempting to register "
"global fnek sig [%s]; rc = [%d]\n",
--
1.5.3.7
Quoting Tyler Hicks ([email protected]):
> eCryptfs has file encryption keys (FEK), file encryption key encryption
> keys (FEKEK), and filename encryption keys (FNEK). The per-file FEK is
> encrypted with one or more FEKEKs and stored in the header of the
> encrypted file. I noticed that the FEK is also being encrypted by the
> FNEK. This is a problem if a user wants to use a different FNEK than
> their FEKEK, as their file contents will still be accessible with the
> FNEK.
>
> This is a minimalistic patch which prevents the FNEKs signatures from
> being copied to the inode signatures list. Ultimately, it keeps the FEK
> from being encrypted with a FNEK.
Right, so the file name encryption key is the same for all the files,
whereas you can have multiple file encryption key encryption keys.
So this bug means that the ability to have multiple FEKEKs becomes
completely worthless.
This makes me wonder if it's not worth doing a complete code-vs-design
comparison to make sure there are no other such gems hidden away.
Tyler, do you have a user-space (hopefully easier-to-read) parser for
encrypted ecryptfs files? (ISTR they were closely following a gpg
format)
> Signed-off-by: Tyler Hicks <[email protected]>
> ---
> fs/ecryptfs/crypto.c | 2 ++
> fs/ecryptfs/ecryptfs_kernel.h | 3 ++-
> fs/ecryptfs/keystore.c | 3 ++-
> fs/ecryptfs/main.c | 5 +++--
> 4 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index f6caeb1..bdca1f4 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -946,6 +946,8 @@ static int ecryptfs_copy_mount_wide_sigs_to_inode_sigs(
> list_for_each_entry(global_auth_tok,
> &mount_crypt_stat->global_auth_tok_list,
> mount_crypt_stat_list) {
> + if (global_auth_tok->flags & ECRYPTFS_AUTH_TOK_FNEK)
> + continue;
> rc = ecryptfs_add_keysig(crypt_stat, global_auth_tok->sig);
> if (rc) {
> printk(KERN_ERR "Error adding keysig; rc = [%d]\n", rc);
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index c11fc95..eb2267e 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -328,6 +328,7 @@ struct ecryptfs_dentry_info {
> */
> struct ecryptfs_global_auth_tok {
> #define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
> +#define ECRYPTFS_AUTH_TOK_FNEK 0x00000002
> u32 flags;
> struct list_head mount_crypt_stat_list;
> struct key *global_auth_tok_key;
> @@ -696,7 +697,7 @@ ecryptfs_write_header_metadata(char *virt,
> int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig);
> int
> ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
> - char *sig);
> + char *sig, u32 global_auth_tok_flags);
> int ecryptfs_get_global_auth_tok_for_sig(
> struct ecryptfs_global_auth_tok **global_auth_tok,
> struct ecryptfs_mount_crypt_stat *mount_crypt_stat, char *sig);
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index ff53942..e4a6223 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -2375,7 +2375,7 @@ struct kmem_cache *ecryptfs_global_auth_tok_cache;
>
> int
> ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
> - char *sig)
> + char *sig, u32 global_auth_tok_flags)
> {
> struct ecryptfs_global_auth_tok *new_auth_tok;
> int rc = 0;
> @@ -2389,6 +2389,7 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
> goto out;
> }
> memcpy(new_auth_tok->sig, sig, ECRYPTFS_SIG_SIZE_HEX);
> + new_auth_tok->flags = global_auth_tok_flags;
> new_auth_tok->sig[ECRYPTFS_SIG_SIZE_HEX] = '\0';
> mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
> list_add(&new_auth_tok->mount_crypt_stat_list,
> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
> index 789cf2e..aed56c2 100644
> --- a/fs/ecryptfs/main.c
> +++ b/fs/ecryptfs/main.c
> @@ -319,7 +319,7 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
> case ecryptfs_opt_ecryptfs_sig:
> sig_src = args[0].from;
> rc = ecryptfs_add_global_auth_tok(mount_crypt_stat,
> - sig_src);
> + sig_src, 0);
> if (rc) {
> printk(KERN_ERR "Error attempting to register "
> "global sig; rc = [%d]\n", rc);
> @@ -370,7 +370,8 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
> ECRYPTFS_SIG_SIZE_HEX] = '\0';
> rc = ecryptfs_add_global_auth_tok(
> mount_crypt_stat,
> - mount_crypt_stat->global_default_fnek_sig);
> + mount_crypt_stat->global_default_fnek_sig,
> + ECRYPTFS_AUTH_TOK_FNEK);
> if (rc) {
> printk(KERN_ERR "Error attempting to register "
> "global fnek sig [%s]; rc = [%d]\n",
> --
> 1.5.3.7
Serge E. Hallyn wrote:
> Quoting Tyler Hicks ([email protected]):
>> eCryptfs has file encryption keys (FEK), file encryption key encryption
>> keys (FEKEK), and filename encryption keys (FNEK). The per-file FEK is
>> encrypted with one or more FEKEKs and stored in the header of the
>> encrypted file. I noticed that the FEK is also being encrypted by the
>> FNEK. This is a problem if a user wants to use a different FNEK than
>> their FEKEK, as their file contents will still be accessible with the
>> FNEK.
>>
>> This is a minimalistic patch which prevents the FNEKs signatures from
>> being copied to the inode signatures list. Ultimately, it keeps the FEK
>> from being encrypted with a FNEK.
>
> Right, so the file name encryption key is the same for all the files,
> whereas you can have multiple file encryption key encryption keys.
> So this bug means that the ability to have multiple FEKEKs becomes
> completely worthless.
>
> This makes me wonder if it's not worth doing a complete code-vs-design
> comparison to make sure there are no other such gems hidden away.
After becoming the primary contact for the eCryptfs kernel code, I've
slowly been doing this. That's sort of how I came across this problem.
Keep in mind that filename encryption is pretty new and 2.6.29 is the
first release it will be in.
>
> Tyler, do you have a user-space (hopefully easier-to-read) parser for
> encrypted ecryptfs files? (ISTR they were closely following a gpg
> format)
This is very much needed! Especially with filename encryption.
ecryptfs-stat exists in the ecryptfs-utils package, but it doesn't have
too much functionality yet. I'll open a couple bugs against
ecryptfs-stat and subscribe you so that you can stay updated.
>
>> Signed-off-by: Tyler Hicks <[email protected]>
>> ---
>> fs/ecryptfs/crypto.c | 2 ++
>> fs/ecryptfs/ecryptfs_kernel.h | 3 ++-
>> fs/ecryptfs/keystore.c | 3 ++-
>> fs/ecryptfs/main.c | 5 +++--
>> 4 files changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
>> index f6caeb1..bdca1f4 100644
>> --- a/fs/ecryptfs/crypto.c
>> +++ b/fs/ecryptfs/crypto.c
>> @@ -946,6 +946,8 @@ static int ecryptfs_copy_mount_wide_sigs_to_inode_sigs(
>> list_for_each_entry(global_auth_tok,
>> &mount_crypt_stat->global_auth_tok_list,
>> mount_crypt_stat_list) {
>> + if (global_auth_tok->flags & ECRYPTFS_AUTH_TOK_FNEK)
>> + continue;
>> rc = ecryptfs_add_keysig(crypt_stat, global_auth_tok->sig);
>> if (rc) {
>> printk(KERN_ERR "Error adding keysig; rc = [%d]\n", rc);
>> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
>> index c11fc95..eb2267e 100644
>> --- a/fs/ecryptfs/ecryptfs_kernel.h
>> +++ b/fs/ecryptfs/ecryptfs_kernel.h
>> @@ -328,6 +328,7 @@ struct ecryptfs_dentry_info {
>> */
>> struct ecryptfs_global_auth_tok {
>> #define ECRYPTFS_AUTH_TOK_INVALID 0x00000001
>> +#define ECRYPTFS_AUTH_TOK_FNEK 0x00000002
>> u32 flags;
>> struct list_head mount_crypt_stat_list;
>> struct key *global_auth_tok_key;
>> @@ -696,7 +697,7 @@ ecryptfs_write_header_metadata(char *virt,
>> int ecryptfs_add_keysig(struct ecryptfs_crypt_stat *crypt_stat, char *sig);
>> int
>> ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
>> - char *sig);
>> + char *sig, u32 global_auth_tok_flags);
>> int ecryptfs_get_global_auth_tok_for_sig(
>> struct ecryptfs_global_auth_tok **global_auth_tok,
>> struct ecryptfs_mount_crypt_stat *mount_crypt_stat, char *sig);
>> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
>> index ff53942..e4a6223 100644
>> --- a/fs/ecryptfs/keystore.c
>> +++ b/fs/ecryptfs/keystore.c
>> @@ -2375,7 +2375,7 @@ struct kmem_cache *ecryptfs_global_auth_tok_cache;
>>
>> int
>> ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
>> - char *sig)
>> + char *sig, u32 global_auth_tok_flags)
>> {
>> struct ecryptfs_global_auth_tok *new_auth_tok;
>> int rc = 0;
>> @@ -2389,6 +2389,7 @@ ecryptfs_add_global_auth_tok(struct ecryptfs_mount_crypt_stat *mount_crypt_stat,
>> goto out;
>> }
>> memcpy(new_auth_tok->sig, sig, ECRYPTFS_SIG_SIZE_HEX);
>> + new_auth_tok->flags = global_auth_tok_flags;
>> new_auth_tok->sig[ECRYPTFS_SIG_SIZE_HEX] = '\0';
>> mutex_lock(&mount_crypt_stat->global_auth_tok_list_mutex);
>> list_add(&new_auth_tok->mount_crypt_stat_list,
>> diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c
>> index 789cf2e..aed56c2 100644
>> --- a/fs/ecryptfs/main.c
>> +++ b/fs/ecryptfs/main.c
>> @@ -319,7 +319,7 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
>> case ecryptfs_opt_ecryptfs_sig:
>> sig_src = args[0].from;
>> rc = ecryptfs_add_global_auth_tok(mount_crypt_stat,
>> - sig_src);
>> + sig_src, 0);
>> if (rc) {
>> printk(KERN_ERR "Error attempting to register "
>> "global sig; rc = [%d]\n", rc);
>> @@ -370,7 +370,8 @@ static int ecryptfs_parse_options(struct super_block *sb, char *options)
>> ECRYPTFS_SIG_SIZE_HEX] = '\0';
>> rc = ecryptfs_add_global_auth_tok(
>> mount_crypt_stat,
>> - mount_crypt_stat->global_default_fnek_sig);
>> + mount_crypt_stat->global_default_fnek_sig,
>> + ECRYPTFS_AUTH_TOK_FNEK);
>> if (rc) {
>> printk(KERN_ERR "Error attempting to register "
>> "global fnek sig [%s]; rc = [%d]\n",
>> --
>> 1.5.3.7
On Fri, 2009-03-13 at 08:39 -0500, Serge E. Hallyn wrote:
> Right, so the file name encryption key is the same for all the files,
> whereas you can have multiple file encryption key encryption keys.
> So this bug means that the ability to have multiple FEKEKs becomes
> completely worthless.
True, but only for files created up until this point with eCryptfs
filename encryption enabled.
Considering 2.6.29 is in RC, and Ubuntu Jaunty is still in Alpha (which
is carrying a backport of eCryptfs against 2.6.28), this should be a
relatively controlled set of affected individuals who should be at least
somewhat aware that they're running pre-release code.
+1, ACK on Tyler's patch. It's a good, simple fix. We're going to
carry that against Ubuntu's kernel. I certainly hope that it will make
it into 2.6.29 which should land on a lot more systems.
> This makes me wonder if it's not worth doing a complete code-vs-design
> comparison to make sure there are no other such gems hidden away.
Definitely a good idea.
> Tyler, do you have a user-space (hopefully easier-to-read) parser for
> encrypted ecryptfs files? (ISTR they were closely following a gpg
> format)
I'll take the to-do to fix this in userspace. I've file a bug for my
own tracking purposes. I'll update this as I enhance the ecryptfs-stat
utility:
* https://bugs.launchpad.net/ecryptfs/+bug/342398
--
:-Dustin
Dustin Kirkland
Ubuntu Server Developer
Canonical, LTD
[email protected]
GPG: 1024D/83A61194