linux-kernel - Re: [f2fs-dev] [PATCH] fscrypto: fix to null-terminate encrypted filename in fname

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <81758b43-a82c-0526-8921-cb34d6da1c50@kernel.org>
Date:   Mon, 29 Aug 2016 22:55:47 +0800
From:   Chao Yu <chao@...nel.org>
To:     Theodore Ts'o <tytso@....edu>, jaegeuk@...nel.org,
        linux-f2fs-devel@...ts.sourceforge.net, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, Chao Yu <yuchao0@...wei.com>
Subject: Re: [f2fs-dev] [PATCH] fscrypto: fix to null-terminate encrypted
 filename in fname_encrypt

Hi Ted, Jaegeuk,

On 2016/8/28 14:16, Chao Yu wrote:
> Hi Ted,
> 
> On 2016/8/28 13:13, Theodore Ts'o wrote:
>> On Sun, Aug 28, 2016 at 09:13:28AM +0800, Chao Yu wrote:
>>> From: Chao Yu <yuchao0@...wei.com>
>>>
>>> This patch fixes to add null character at the end of encrypted filename

Since encryption functionality in ext4/f2fs was exported to vfs as fscrypot
module, more filesystems can use it, I'm not sure, maybe other fs will traverse
encrypted filename directly.

So, could we set this null character in fname_encrypt in advance in order to
avoid hitting random characters behind target filename when traversing it?

Thanks,

>>> in fname_encrypt, in order to avoid incorrectly traversing random data
>>> located after target filename. The call stack is as below:
>>>
>>> - f2fs_add_link
>>>  - __f2fs_add_link
>>>   - fscrypt_setup_filename
>>>    - fscrypt_fname_alloc_buffer		allocate buffer for @fname
>>>    - fname_encrypt			didn't set null character for @fname
>>>   - f2fs_add_regular_entry		init qstr with @fname
>>>    - init_inode_metadata
>>>     - f2fs_init_security
>>>      - security_inode_init_security
>>>       - selinux_inode_init_security
>>>        - selinux_determine_inode_label
>>>         - security_transition_sid
>>> 	 - security_compute_sid
>>> 	  - filename_compute_type
>>> 	   - hashtab_search
>>> 	    - filenametr_hash		traverse @fname as one which has null character
>>
>> The problem is not in fname_encrypt(), but rather that
>> security_inode_init_security() should be given the _unencrypted_
>> filename.
>>
>> In ext4 security_inode_init_security() is called with the qstr from
>> the dentry, not the encrypted qstr --- in fact we call
>> security_inode_init_security before we call fname_encrypt.
>>
>> SELinux needs the unencrypted filename in order to decide which
>> SELinux rules / labels should apply.
> 
> You're right, I missed this mistake. So actually, this is a bug of f2fs.
> Let me figure out the fixing patch.
> 
> Thanks for your review! :)
> 
> Thanks,
> 
>>
>> 					- Ted
>>
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@...ts.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>