Re: [f2fs-dev] [PATCH] f2fs: don't allow rename unencrypted file to encrypted directory
On 2017/3/8 21:35, Kinglong Mee wrote: > On 3/8/2017 20:08, Chao Yu wrote: >> In commit d9cdc9033181 ("ext4 crypto: enforce context consistency") we >> declared that: >> >> 2) All files or directories in a directory must be protected using the >> same key as their containing directory. >> >> But in f2fs_cross_rename there is a vulnerability that allow to cross >> rename unencrypted file into encrypted directory, it needs to be refused. > > fscrypt_has_permitted_context has do the checking as this patch, > > 168 /* no restrictions if the parent directory is not encrypted */ > 169 if (!parent->i_sb->s_cop->is_encrypted(parent)) > 170 return 1; > 171 /* if the child directory is not encrypted, this is always a > problem */ > 172 if (!parent->i_sb->s_cop->is_encrypted(child)) > 173 return 0; > > So, the cross rename unencrypted file into encrypted directory is permitted > right now. > > I have a encrypted directory "ncry", "new" is unencrypted file. > > [root@nfstestnic f2fs]# renameat2 -x encry/hello new > Operation not permitted > [root@nfstestnic f2fs]# renameat2 -x encry/hello new > Operation not permitted I've tried this, the same result as yours, so this patch is wrong, please ignore it, sorry about the noise. Thanks, > > How do you test it? > > thanks, > Kinglong Mee >> >> Signed-off-by: Chao Yu>> --- >> fs/f2fs/namei.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index 25c073f6c7d4..8de684b84cb9 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -855,6 +855,10 @@ static int f2fs_cross_rename(struct inode *old_dir, >> struct dentry *old_dentry, >> !fscrypt_has_encryption_key(new_dir))) >> return -ENOKEY; >> >> +if (f2fs_encrypted_inode(old_dir) && !f2fs_encrypted_inode(new_inode) || >> +f2fs_encrypted_inode(new_dir) && >> !f2fs_encrypted_inode(old_inode)) >> +return -EPERM; >> + >> if ((f2fs_encrypted_inode(old_dir) || f2fs_encrypted_inode(new_dir)) && >> (old_dir != new_dir) && >> (!fscrypt_has_permitted_context(new_dir, old_inode) || >> > > . >
Re: [f2fs-dev] [PATCH] f2fs: don't allow rename unencrypted file to encrypted directory
On 2017/3/8 21:35, Kinglong Mee wrote: > On 3/8/2017 20:08, Chao Yu wrote: >> In commit d9cdc9033181 ("ext4 crypto: enforce context consistency") we >> declared that: >> >> 2) All files or directories in a directory must be protected using the >> same key as their containing directory. >> >> But in f2fs_cross_rename there is a vulnerability that allow to cross >> rename unencrypted file into encrypted directory, it needs to be refused. > > fscrypt_has_permitted_context has do the checking as this patch, > > 168 /* no restrictions if the parent directory is not encrypted */ > 169 if (!parent->i_sb->s_cop->is_encrypted(parent)) > 170 return 1; > 171 /* if the child directory is not encrypted, this is always a > problem */ > 172 if (!parent->i_sb->s_cop->is_encrypted(child)) > 173 return 0; > > So, the cross rename unencrypted file into encrypted directory is permitted > right now. > > I have a encrypted directory "ncry", "new" is unencrypted file. > > [root@nfstestnic f2fs]# renameat2 -x encry/hello new > Operation not permitted > [root@nfstestnic f2fs]# renameat2 -x encry/hello new > Operation not permitted I've tried this, the same result as yours, so this patch is wrong, please ignore it, sorry about the noise. Thanks, > > How do you test it? > > thanks, > Kinglong Mee >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/namei.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c >> index 25c073f6c7d4..8de684b84cb9 100644 >> --- a/fs/f2fs/namei.c >> +++ b/fs/f2fs/namei.c >> @@ -855,6 +855,10 @@ static int f2fs_cross_rename(struct inode *old_dir, >> struct dentry *old_dentry, >> !fscrypt_has_encryption_key(new_dir))) >> return -ENOKEY; >> >> +if (f2fs_encrypted_inode(old_dir) && !f2fs_encrypted_inode(new_inode) || >> +f2fs_encrypted_inode(new_dir) && >> !f2fs_encrypted_inode(old_inode)) >> +return -EPERM; >> + >> if ((f2fs_encrypted_inode(old_dir) || f2fs_encrypted_inode(new_dir)) && >> (old_dir != new_dir) && >> (!fscrypt_has_permitted_context(new_dir, old_inode) || >> > > . >
Re: [f2fs-dev] [PATCH] f2fs: don't allow rename unencrypted file to encrypted directory
On 3/8/2017 20:08, Chao Yu wrote: > In commit d9cdc9033181 ("ext4 crypto: enforce context consistency") we > declared that: > > 2) All files or directories in a directory must be protected using the > same key as their containing directory. > > But in f2fs_cross_rename there is a vulnerability that allow to cross > rename unencrypted file into encrypted directory, it needs to be refused. fscrypt_has_permitted_context has do the checking as this patch, 168 /* no restrictions if the parent directory is not encrypted */ 169 if (!parent->i_sb->s_cop->is_encrypted(parent)) 170 return 1; 171 /* if the child directory is not encrypted, this is always a problem */ 172 if (!parent->i_sb->s_cop->is_encrypted(child)) 173 return 0; So, the cross rename unencrypted file into encrypted directory is permitted right now. I have a encrypted directory "ncry", "new" is unencrypted file. [root@nfstestnic f2fs]# renameat2 -x encry/hello new Operation not permitted [root@nfstestnic f2fs]# renameat2 -x encry/hello new Operation not permitted How do you test it? thanks, Kinglong Mee > > Signed-off-by: Chao Yu> --- > fs/f2fs/namei.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 25c073f6c7d4..8de684b84cb9 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -855,6 +855,10 @@ static int f2fs_cross_rename(struct inode *old_dir, > struct dentry *old_dentry, > !fscrypt_has_encryption_key(new_dir))) > return -ENOKEY; > > + if (f2fs_encrypted_inode(old_dir) && !f2fs_encrypted_inode(new_inode) || > + f2fs_encrypted_inode(new_dir) && > !f2fs_encrypted_inode(old_inode)) > + return -EPERM; > + > if ((f2fs_encrypted_inode(old_dir) || f2fs_encrypted_inode(new_dir)) && > (old_dir != new_dir) && > (!fscrypt_has_permitted_context(new_dir, old_inode) || >
Re: [f2fs-dev] [PATCH] f2fs: don't allow rename unencrypted file to encrypted directory
On 3/8/2017 20:08, Chao Yu wrote: > In commit d9cdc9033181 ("ext4 crypto: enforce context consistency") we > declared that: > > 2) All files or directories in a directory must be protected using the > same key as their containing directory. > > But in f2fs_cross_rename there is a vulnerability that allow to cross > rename unencrypted file into encrypted directory, it needs to be refused. fscrypt_has_permitted_context has do the checking as this patch, 168 /* no restrictions if the parent directory is not encrypted */ 169 if (!parent->i_sb->s_cop->is_encrypted(parent)) 170 return 1; 171 /* if the child directory is not encrypted, this is always a problem */ 172 if (!parent->i_sb->s_cop->is_encrypted(child)) 173 return 0; So, the cross rename unencrypted file into encrypted directory is permitted right now. I have a encrypted directory "ncry", "new" is unencrypted file. [root@nfstestnic f2fs]# renameat2 -x encry/hello new Operation not permitted [root@nfstestnic f2fs]# renameat2 -x encry/hello new Operation not permitted How do you test it? thanks, Kinglong Mee > > Signed-off-by: Chao Yu > --- > fs/f2fs/namei.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c > index 25c073f6c7d4..8de684b84cb9 100644 > --- a/fs/f2fs/namei.c > +++ b/fs/f2fs/namei.c > @@ -855,6 +855,10 @@ static int f2fs_cross_rename(struct inode *old_dir, > struct dentry *old_dentry, > !fscrypt_has_encryption_key(new_dir))) > return -ENOKEY; > > + if (f2fs_encrypted_inode(old_dir) && !f2fs_encrypted_inode(new_inode) || > + f2fs_encrypted_inode(new_dir) && > !f2fs_encrypted_inode(old_inode)) > + return -EPERM; > + > if ((f2fs_encrypted_inode(old_dir) || f2fs_encrypted_inode(new_dir)) && > (old_dir != new_dir) && > (!fscrypt_has_permitted_context(new_dir, old_inode) || >