Re: [PATCH 6/9] security, overlayfs: Provide hook to correctly label newly created files

[Stephen Smalley]

On 07/13/2016 10:57 AM, Stephen Smalley wrote:
> On 07/13/2016 10:44 AM, Vivek Goyal wrote:
>> During a new file creation we need to make sure new file is created with the
>> right label. New file is created in upper/ so effectively file should get
>> label as if task had created file in upper/.
>>
>> We switched to mounter's creds for actual file creation. Also if there is a
>> whiteout present, then file will be created in work/ dir first and then
>> renamed in upper. In none of the cases file will be labeled as we want it to
>> be.
>>
>> This patch introduces a new hook dentry_create_files_as(), which determines
>> the label/context dentry will get if it had been created by task in upper
>> and modify passed set of creds appropriately. Caller makes use of these new
>> creds for file creation.
>>
>> Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
>> ---
>>  fs/overlayfs/dir.c        | 10 ++++++++++
>>  include/linux/lsm_hooks.h | 15 +++++++++++++++
>>  include/linux/security.h  | 12 ++++++++++++
>>  security/security.c       | 11 +++++++++++
>>  4 files changed, 48 insertions(+)
>>
>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> index 4cdeb74..f94872f 100644
>> --- a/fs/overlayfs/dir.c
>> +++ b/fs/overlayfs/dir.c
>> @@ -433,6 +433,15 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>>  	if (override_cred) {
>>  		override_cred->fsuid = inode->i_uid;
>>  		override_cred->fsgid = inode->i_gid;
>> +		if (!hardlink) {
>> +			err = security_dentry_create_files_as(dentry,
>> +					mode, &dentry->d_name, old_cred,
>> +					override_cred);
>> +			if (err) {
>> +				put_cred(override_cred);
> 
> Same principle here; on error the caller should do nothing with
> override_cred.

Sorry, never mind - not allocated by the hook so properly handled by the
caller.

> 
>> +				goto out_revert_creds;
>> +			}
>> +		}
>>  		put_cred(override_creds(override_cred));
>>  		put_cred(override_cred);
>>  
>> @@ -443,6 +452,7 @@ static int ovl_create_or_link(struct dentry *dentry, int mode, dev_t rdev,
>>  			err = ovl_create_over_whiteout(dentry, inode, &stat,
>>  							link, hardlink);
>>  	}
>> +out_revert_creds:
>>  	revert_creds(old_cred);
>>  	if (!err) {
>>  		struct inode *realinode = d_inode(ovl_dentry_upper(dentry));
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 84caead..95745fe 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -151,6 +151,16 @@
>>   *	@name name of the last path component used to create file
>>   *	@ctx pointer to place the pointer to the resulting context in.
>>   *	@ctxlen point to place the length of the resulting context.
>> + * @dentry_create_files_as:
>> + *	Compute a context for a dentry as the inode is not yet available
>> + *	and set that context in passed in creds so that new files are
>> + *	created using that context. Context is calculated using the
>> + *	passed in creds and not the creds of the caller.
>> + *	@dentry dentry to use in calculating the context.
>> + *	@mode mode used to determine resource type.
>> + *	@name name of the last path component used to create file
>> + * 	@old creds which should be used for context calculation
>> + * 	@new creds to modify
>>   *
>>   *
>>   * Security hooks for inode operations.
>> @@ -1375,6 +1385,10 @@ union security_list_options {
>>  	int (*dentry_init_security)(struct dentry *dentry, int mode,
>>  					struct qstr *name, void **ctx,
>>  					u32 *ctxlen);
>> +	int (*dentry_create_files_as)(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old,
>> +					struct cred *new);
>>  
>>  
>>  #ifdef CONFIG_SECURITY_PATH
>> @@ -1675,6 +1689,7 @@ struct security_hook_heads {
>>  	struct list_head sb_clone_mnt_opts;
>>  	struct list_head sb_parse_opts_str;
>>  	struct list_head dentry_init_security;
>> +	struct list_head dentry_create_files_as;
>>  #ifdef CONFIG_SECURITY_PATH
>>  	struct list_head path_unlink;
>>  	struct list_head path_mkdir;
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 4a3b8bc..1eb03dc 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -242,6 +242,10 @@ int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>>  int security_dentry_init_security(struct dentry *dentry, int mode,
>>  					struct qstr *name, void **ctx,
>>  					u32 *ctxlen);
>> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old,
>> +					struct cred *new);
>>  
>>  int security_inode_alloc(struct inode *inode);
>>  void security_inode_free(struct inode *inode);
>> @@ -600,6 +604,14 @@ static inline int security_dentry_init_security(struct dentry *dentry,
>>  	return -EOPNOTSUPP;
>>  }
>>  
>> +static inline int security_dentry_create_files_as(struct dentry *dentry,
>> +						  int mode, struct qstr *name,
>> +						  const struct cred *old,
>> +						  struct cred *new)
>> +{
>> +	return 0;
>> +}
>> +
>>  
>>  static inline int security_inode_init_security(struct inode *inode,
>>  						struct inode *dir,
>> diff --git a/security/security.c b/security/security.c
>> index 3321e31..38747d1 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -364,6 +364,15 @@ int security_dentry_init_security(struct dentry *dentry, int mode,
>>  }
>>  EXPORT_SYMBOL(security_dentry_init_security);
>>  
>> +int security_dentry_create_files_as(struct dentry *dentry, int mode,
>> +					struct qstr *name,
>> +					const struct cred *old, struct cred *new)
>> +{
>> +	return call_int_hook(dentry_create_files_as, 0, dentry, mode,
>> +				name, old, new);
>> +}
>> +EXPORT_SYMBOL(security_dentry_create_files_as);
>> +
>>  int security_inode_init_security(struct inode *inode, struct inode *dir,
>>  				 const struct qstr *qstr,
>>  				 const initxattrs initxattrs, void *fs_data)
>> @@ -1614,6 +1623,8 @@ struct security_hook_heads security_hook_heads = {
>>  		LIST_HEAD_INIT(security_hook_heads.sb_parse_opts_str),
>>  	.dentry_init_security =
>>  		LIST_HEAD_INIT(security_hook_heads.dentry_init_security),
>> +	.dentry_create_files_as =
>> +		LIST_HEAD_INIT(security_hook_heads.dentry_create_files_as),
>>  #ifdef CONFIG_SECURITY_PATH
>>  	.path_unlink =	LIST_HEAD_INIT(security_hook_heads.path_unlink),
>>  	.path_mkdir =	LIST_HEAD_INIT(security_hook_heads.path_mkdir),
>>
>