[PATCH v1 0/3] fixes for missing security.ima on new empty files

[PATCH v1 0/3] fixes for missing security.ima on new empty files

[Dmitry Kasatkin]

Hi,

This patchset fixes the problem of missing security.ima on new empty files.
Detailed descriptions of problems are in the following patch descriptions.
First two patches fixes the problem. Third patch makes use of FILE_CREATED
flag from VFS, which was not available at the time IMA appraisal came to the
kernel.

- Dmitry

Dmitry Kasatkin (3):
  ima: provide flag to identify new empty files
  evm: skip integrity verification for newly created files
  ima: pass 'opened' flag to identify newly created files

 fs/namei.c                            |  2 +-
 fs/nfsd/vfs.c                         |  2 +-
 include/linux/ima.h                   |  4 ++--
 security/integrity/evm/evm_main.c     |  6 +++++-
 security/integrity/ima/ima.h          |  4 ++--
 security/integrity/ima/ima_appraise.c |  9 ++++++---
 security/integrity/ima/ima_main.c     | 26 ++++++++++++++------------
 security/integrity/integrity.h        |  1 +
 8 files changed, 32 insertions(+), 22 deletions(-)

-- 
1.9.1

[PATCH v1 1/3] ima: provide flag to identify new empty files

[Dmitry Kasatkin]

Newly created empty files do not get initial security.ima
value because iversion does not change. It can be checked from
the shell as:

  $ (exec >foo)
  $ getfattr -h -e hex -d -m security foo

This patch defines IMA_NEW_FILE flag which is set when IMA detects that new
file is created. It is checked upon ima_file_free hook to set initial
security.ima value.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_appraise.c |  7 +++++--
 security/integrity/ima/ima_main.c     | 12 +++++++-----
 security/integrity/integrity.h        |  1 +
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 9dd18b5..3a4beb3 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -194,8 +194,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 			goto out;
 
 		cause = "missing-hash";
-		status =
-		    (inode->i_size == 0) ? INTEGRITY_PASS : INTEGRITY_NOLABEL;
+		status = INTEGRITY_NOLABEL;
+		if (inode->i_size == 0) {
+			iint->flags |= IMA_NEW_FILE;
+			status = INTEGRITY_PASS;
+		}
 		goto out;
 	}
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index e51e0d5..5a870e7 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -124,11 +124,13 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
 		return;
 
 	mutex_lock(&iint->mutex);
-	if (atomic_read(&inode->i_writecount) == 1 &&
-	    iint->version != inode->i_version) {
-		iint->flags &= ~IMA_DONE_MASK;
-		if (iint->flags & IMA_APPRAISE)
-			ima_update_xattr(iint, file);
+	if (atomic_read(&inode->i_writecount) == 1) {
+		if ((iint->version != inode->i_version) ||
+		    (iint->flags & IMA_NEW_FILE)) {
+			iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
+			if (iint->flags & IMA_APPRAISE)
+				ima_update_xattr(iint, file);
+		}
 	}
 	mutex_unlock(&iint->mutex);
 }
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 92c1083..7656d47 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -30,6 +30,7 @@
 #define IMA_ACTION_FLAGS	0xff000000
 #define IMA_DIGSIG_REQUIRED	0x01000000
 #define IMA_PERMIT_DIRECTIO	0x02000000
+#define IMA_NEW_FILE		0x04000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_APPRAISE_SUBMASK)
-- 
1.9.1

Re: [PATCH v1 1/3] ima: provide flag to identify new empty files

[Mimi Zohar]

On Fri, 2014-07-11 at 14:46 +0300, Dmitry Kasatkin wrote: 
> Newly created empty files do not get initial security.ima
> value because iversion does not change. It can be checked from
> the shell as:
> 
>   $ (exec >foo)
>   $ getfattr -h -e hex -d -m security foo
> 
> This patch defines IMA_NEW_FILE flag which is set when IMA detects that new
> file is created. It is checked upon ima_file_free hook to set initial
> security.ima value.

Other than rebasing on top of #next, this patch set looks good.

thanks,

Mimi

> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

> ---
>  security/integrity/ima/ima_appraise.c |  7 +++++--
>  security/integrity/ima/ima_main.c     | 12 +++++++-----
>  security/integrity/integrity.h        |  1 +
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 9dd18b5..3a4beb3 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -194,8 +194,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>  			goto out;
> 
>  		cause = "missing-hash";
> -		status =
> -		    (inode->i_size == 0) ? INTEGRITY_PASS : INTEGRITY_NOLABEL;
> +		status = INTEGRITY_NOLABEL;
> +		if (inode->i_size == 0) {
> +			iint->flags |= IMA_NEW_FILE;
> +			status = INTEGRITY_PASS;
> +		}
>  		goto out;
>  	}
> 
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index e51e0d5..5a870e7 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -124,11 +124,13 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
>  		return;
> 
>  	mutex_lock(&iint->mutex);
> -	if (atomic_read(&inode->i_writecount) == 1 &&
> -	    iint->version != inode->i_version) {
> -		iint->flags &= ~IMA_DONE_MASK;
> -		if (iint->flags & IMA_APPRAISE)
> -			ima_update_xattr(iint, file);
> +	if (atomic_read(&inode->i_writecount) == 1) {
> +		if ((iint->version != inode->i_version) ||
> +		    (iint->flags & IMA_NEW_FILE)) {
> +			iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> +			if (iint->flags & IMA_APPRAISE)
> +				ima_update_xattr(iint, file);
> +		}
>  	}
>  	mutex_unlock(&iint->mutex);
>  }
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 92c1083..7656d47 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -30,6 +30,7 @@
>  #define IMA_ACTION_FLAGS	0xff000000
>  #define IMA_DIGSIG_REQUIRED	0x01000000
>  #define IMA_PERMIT_DIRECTIO	0x02000000
> +#define IMA_NEW_FILE		0x04000000
> 
>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>  				 IMA_APPRAISE_SUBMASK)

Re: [Linux-ima-devel] [PATCH v1 1/3] ima: provide flag to identify new empty files

[Mimi Zohar]

On Tue, 2014-07-15 at 10:00 -0400, Mimi Zohar wrote: 
> On Fri, 2014-07-11 at 14:46 +0300, Dmitry Kasatkin wrote: 
> > Newly created empty files do not get initial security.ima
> > value because iversion does not change. It can be checked from
> > the shell as:
> > 
> >   $ (exec >foo)
> >   $ getfattr -h -e hex -d -m security foo

This is a change in behavior.  Please include the commit number that
introduced this change in the patch description.

Mimi

> > This patch defines IMA_NEW_FILE flag which is set when IMA detects that new
> > file is created. It is checked upon ima_file_free hook to set initial
> > security.ima value.
> 
> Other than rebasing on top of #next, this patch set looks good.
> 
> thanks,
> 
> Mimi
> 
> > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> 
> > ---
> >  security/integrity/ima/ima_appraise.c |  7 +++++--
> >  security/integrity/ima/ima_main.c     | 12 +++++++-----
> >  security/integrity/integrity.h        |  1 +
> >  3 files changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> > index 9dd18b5..3a4beb3 100644
> > --- a/security/integrity/ima/ima_appraise.c
> > +++ b/security/integrity/ima/ima_appraise.c
> > @@ -194,8 +194,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> >  			goto out;
> > 
> >  		cause = "missing-hash";
> > -		status =
> > -		    (inode->i_size == 0) ? INTEGRITY_PASS : INTEGRITY_NOLABEL;
> > +		status = INTEGRITY_NOLABEL;
> > +		if (inode->i_size == 0) {
> > +			iint->flags |= IMA_NEW_FILE;
> > +			status = INTEGRITY_PASS;
> > +		}
> >  		goto out;
> >  	}
> > 
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index e51e0d5..5a870e7 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -124,11 +124,13 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
> >  		return;
> > 
> >  	mutex_lock(&iint->mutex);
> > -	if (atomic_read(&inode->i_writecount) == 1 &&
> > -	    iint->version != inode->i_version) {
> > -		iint->flags &= ~IMA_DONE_MASK;
> > -		if (iint->flags & IMA_APPRAISE)
> > -			ima_update_xattr(iint, file);
> > +	if (atomic_read(&inode->i_writecount) == 1) {
> > +		if ((iint->version != inode->i_version) ||
> > +		    (iint->flags & IMA_NEW_FILE)) {
> > +			iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
> > +			if (iint->flags & IMA_APPRAISE)
> > +				ima_update_xattr(iint, file);
> > +		}
> >  	}
> >  	mutex_unlock(&iint->mutex);
> >  }
> > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> > index 92c1083..7656d47 100644
> > --- a/security/integrity/integrity.h
> > +++ b/security/integrity/integrity.h
> > @@ -30,6 +30,7 @@
> >  #define IMA_ACTION_FLAGS	0xff000000
> >  #define IMA_DIGSIG_REQUIRED	0x01000000
> >  #define IMA_PERMIT_DIRECTIO	0x02000000
> > +#define IMA_NEW_FILE		0x04000000
> > 
> >  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
> >  				 IMA_APPRAISE_SUBMASK)
> 
> 
> 
> ------------------------------------------------------------------------------
> Want fast and easy access to all the code in your enterprise? Index and
> search up to 200,000 lines of code with a free copy of Black Duck
> Code Sight - the same software that powers the world's largest code
> search on Ohloh, the Black Duck Open Hub! Try it now.
> http://p.sf.net/sfu/bds
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
> 

Re: [Linux-ima-devel] [PATCH v1 1/3] ima: provide flag to identify new empty files

[Dmitry Kasatkin]

On 16/07/14 01:12, Mimi Zohar wrote:
> On Tue, 2014-07-15 at 10:00 -0400, Mimi Zohar wrote: 
>> On Fri, 2014-07-11 at 14:46 +0300, Dmitry Kasatkin wrote: 
>>> Newly created empty files do not get initial security.ima
>>> value because iversion does not change. It can be checked from
>>> the shell as:
>>>
>>>   $ (exec >foo)
>>>   $ getfattr -h -e hex -d -m security foo
> This is a change in behavior.  Please include the commit number that
> introduced this change in the patch description.

This commit might caused this.
dff6efc326a4d5f305797d4a6bba14f374fdd633 fs: fix iversion handling

- Dmitry

> Mimi
>
>>> This patch defines IMA_NEW_FILE flag which is set when IMA detects that new
>>> file is created. It is checked upon ima_file_free hook to set initial
>>> security.ima value.
>> Other than rebasing on top of #next, this patch set looks good.
>>
>> thanks,
>>
>> Mimi
>>
>>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>>> ---
>>>  security/integrity/ima/ima_appraise.c |  7 +++++--
>>>  security/integrity/ima/ima_main.c     | 12 +++++++-----
>>>  security/integrity/integrity.h        |  1 +
>>>  3 files changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>>> index 9dd18b5..3a4beb3 100644
>>> --- a/security/integrity/ima/ima_appraise.c
>>> +++ b/security/integrity/ima/ima_appraise.c
>>> @@ -194,8 +194,11 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>>>  			goto out;
>>>
>>>  		cause = "missing-hash";
>>> -		status =
>>> -		    (inode->i_size == 0) ? INTEGRITY_PASS : INTEGRITY_NOLABEL;
>>> +		status = INTEGRITY_NOLABEL;
>>> +		if (inode->i_size == 0) {
>>> +			iint->flags |= IMA_NEW_FILE;
>>> +			status = INTEGRITY_PASS;
>>> +		}
>>>  		goto out;
>>>  	}
>>>
>>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>>> index e51e0d5..5a870e7 100644
>>> --- a/security/integrity/ima/ima_main.c
>>> +++ b/security/integrity/ima/ima_main.c
>>> @@ -124,11 +124,13 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
>>>  		return;
>>>
>>>  	mutex_lock(&iint->mutex);
>>> -	if (atomic_read(&inode->i_writecount) == 1 &&
>>> -	    iint->version != inode->i_version) {
>>> -		iint->flags &= ~IMA_DONE_MASK;
>>> -		if (iint->flags & IMA_APPRAISE)
>>> -			ima_update_xattr(iint, file);
>>> +	if (atomic_read(&inode->i_writecount) == 1) {
>>> +		if ((iint->version != inode->i_version) ||
>>> +		    (iint->flags & IMA_NEW_FILE)) {
>>> +			iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
>>> +			if (iint->flags & IMA_APPRAISE)
>>> +				ima_update_xattr(iint, file);
>>> +		}
>>>  	}
>>>  	mutex_unlock(&iint->mutex);
>>>  }
>>> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
>>> index 92c1083..7656d47 100644
>>> --- a/security/integrity/integrity.h
>>> +++ b/security/integrity/integrity.h
>>> @@ -30,6 +30,7 @@
>>>  #define IMA_ACTION_FLAGS	0xff000000
>>>  #define IMA_DIGSIG_REQUIRED	0x01000000
>>>  #define IMA_PERMIT_DIRECTIO	0x02000000
>>> +#define IMA_NEW_FILE		0x04000000
>>>
>>>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>>>  				 IMA_APPRAISE_SUBMASK)
>>
>>
>> ------------------------------------------------------------------------------
>> Want fast and easy access to all the code in your enterprise? Index and
>> search up to 200,000 lines of code with a free copy of Black Duck
>> Code Sight - the same software that powers the world's largest code
>> search on Ohloh, the Black Duck Open Hub! Try it now.
>> http://p.sf.net/sfu/bds
>> _______________________________________________
>> Linux-ima-devel mailing list
>> Linux-ima-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
>>
>
>

[PATCH v1 2/3] evm: skip integrity verification for newly created files

[Dmitry Kasatkin]

Newly created files do not get initial security.ima until close.
Setting xattr value before close fails because EVM fails to verify
integrity due to missing xattrs. Following is the example when it
happens:
  fd = open("foo", O_CREAT | O_WRONLY, 0644);
  setxattr("foo", "security.SMACK64", value, sizeof(value), 0);
  close(fd);

This patch skips integrity verification if IMA_NEW_FILE flag is set.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/evm_main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 3bcb80d..682e640 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -248,10 +248,14 @@ EXPORT_SYMBOL_GPL(evm_verifyxattr);
 static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
+	struct integrity_iint_cache *iint;
 
 	if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode)
 		return 0;
-	return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
+	iint = integrity_iint_find(inode);
+	if (iint && (iint->flags & IMA_NEW_FILE))
+		return 0;
+	return evm_verify_hmac(dentry, NULL, NULL, 0, iint);
 }
 
 /*
-- 
1.9.1

Re: [PATCH v1 2/3] evm: skip integrity verification for newly created files

[Mimi Zohar]

On Fri, 2014-07-11 at 14:47 +0300, Dmitry Kasatkin wrote: 
> Newly created files do not get initial security.ima until close.
> Setting xattr value before close fails because EVM fails to verify
> integrity due to missing xattrs. Following is the example when it
> happens:
>   fd = open("foo", O_CREAT | O_WRONLY, 0644);
>   setxattr("foo", "security.SMACK64", value, sizeof(value), 0);
>   close(fd);
> 
> This patch skips integrity verification if IMA_NEW_FILE flag is set.

Right, unless the LSM initially created the security xattr at
d_instantiate(), security.evm will not have been created.

Mimi

> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  security/integrity/evm/evm_main.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 3bcb80d..682e640 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -248,10 +248,14 @@ EXPORT_SYMBOL_GPL(evm_verifyxattr);
>  static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
>  {
>  	struct inode *inode = dentry->d_inode;
> +	struct integrity_iint_cache *iint;
> 
>  	if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode)
>  		return 0;
> -	return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
> +	iint = integrity_iint_find(inode);
> +	if (iint && (iint->flags & IMA_NEW_FILE))
> +		return 0;
> +	return evm_verify_hmac(dentry, NULL, NULL, 0, iint);
>  }
> 
>  /*

[PATCH v1 3/3] ima: pass 'opened' flag to identify newly created files

[Dmitry Kasatkin]

Empty file size and missing xattrs do not guaranty that file
was just created. It could be originally made empty and labeled
with needed LSM labels. Current implementation makes it possible
to remove security.ima, and set arbitrary LSM related attribute.
On open, IMA would be forced to update security.evm to 'fake' LSM
xattrs.

This patch passes FILE_CREATED flag to IMA to reliably identify new
files.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 fs/namei.c                            |  2 +-
 fs/nfsd/vfs.c                         |  2 +-
 include/linux/ima.h                   |  4 ++--
 security/integrity/ima/ima.h          |  4 ++--
 security/integrity/ima/ima_appraise.c |  4 ++--
 security/integrity/ima/ima_main.c     | 14 +++++++-------
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 985c6f3..005771f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3058,7 +3058,7 @@ opened:
 	error = open_check_o_direct(file);
 	if (error)
 		goto exit_fput;
-	error = ima_file_check(file, op->acc_mode);
+	error = ima_file_check(file, op->acc_mode, *opened);
 	if (error)
 		goto exit_fput;
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 140c496..d49c778 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -709,7 +709,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
 		host_err = PTR_ERR(*filp);
 		*filp = NULL;
 	} else {
-		host_err = ima_file_check(*filp, may_flags);
+		host_err = ima_file_check(*filp, may_flags, 0);
 
 		if (may_flags & NFSD_MAY_64BIT_COOKIE)
 			(*filp)->f_mode |= FMODE_64BITHASH;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 1b7f268..23a87a4 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -15,7 +15,7 @@ struct linux_binprm;
 
 #ifdef CONFIG_IMA
 extern int ima_bprm_check(struct linux_binprm *bprm);
-extern int ima_file_check(struct file *file, int mask);
+extern int ima_file_check(struct file *file, int mask, int opened);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_module_check(struct file *file);
@@ -26,7 +26,7 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
 	return 0;
 }
 
-static inline int ima_file_check(struct file *file, int mask)
+static inline int ima_file_check(struct file *file, int mask, int opened)
 {
 	return 0;
 }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3e9be3d..9337aa9 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -175,7 +175,7 @@ void ima_delete_rules(void);
 int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len);
+			     int xattr_len, int opened);
 int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -191,7 +191,7 @@ static inline int ima_appraise_measurement(int func,
 					   struct file *file,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
-					   int xattr_len)
+					   int xattr_len, int opened)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 3a4beb3..10679c8 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -175,7 +175,7 @@ int ima_read_xattr(struct dentry *dentry,
 int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len)
+			     int xattr_len, int opened)
 {
 	static const char op[] = "appraise_data";
 	char *cause = "unknown";
@@ -195,7 +195,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
 
 		cause = "missing-hash";
 		status = INTEGRITY_NOLABEL;
-		if (inode->i_size == 0) {
+		if (opened & FILE_CREATED) {
 			iint->flags |= IMA_NEW_FILE;
 			status = INTEGRITY_PASS;
 		}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5a870e7..3384036 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -157,7 +157,7 @@ void ima_file_free(struct file *file)
 }
 
 static int process_measurement(struct file *file, const char *filename,
-			       int mask, int function)
+			       int mask, int function, int opened)
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint;
@@ -238,7 +238,7 @@ static int process_measurement(struct file *file, const char *filename,
 	if (action & IMA_APPRAISE_SUBMASK) {
 		mutex_lock(&inode->i_mutex);
 		rc = ima_appraise_measurement(_func, iint, file, pathname,
-					      xattr_value, xattr_len);
+					      xattr_value, xattr_len, opened);
 		mutex_unlock(&inode->i_mutex);
 	}
 	if (action & IMA_AUDIT)
@@ -269,7 +269,7 @@ out_unlocked:
 int ima_file_mmap(struct file *file, unsigned long prot)
 {
 	if (file && (prot & PROT_EXEC))
-		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK);
+		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0);
 	return 0;
 }
 
@@ -291,7 +291,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
 	return process_measurement(bprm->file,
 				   (strcmp(bprm->filename, bprm->interp) == 0) ?
 				   bprm->filename : bprm->interp,
-				   MAY_EXEC, BPRM_CHECK);
+				   MAY_EXEC, BPRM_CHECK, 0);
 }
 
 /**
@@ -304,12 +304,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
  * On success return 0.  On integrity appraisal error, assuming the file
  * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
  */
-int ima_file_check(struct file *file, int mask)
+int ima_file_check(struct file *file, int mask, int opened)
 {
 	ima_rdwr_violation_check(file);
 	return process_measurement(file, NULL,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
-				   FILE_CHECK);
+				   FILE_CHECK, opened);
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
 
@@ -332,7 +332,7 @@ int ima_module_check(struct file *file)
 #endif
 		return 0;	/* We rely on module signature checking */
 	}
-	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK);
+	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0);
 }
 
 static int __init init_ima(void)
-- 
1.9.1

Re: [PATCH v1 3/3] ima: pass 'opened' flag to identify newly created files

[Mimi Zohar]

On Fri, 2014-07-11 at 14:47 +0300, Dmitry Kasatkin wrote: 
> Empty file size and missing xattrs do not guaranty that file

^guarantee

> was just created. It could be originally made empty and labeled
> with needed LSM labels. Current implementation makes it possible
> to remove security.ima, and set arbitrary LSM related attribute.
> On open, IMA would be forced to update security.evm to 'fake' LSM
> xattrs.

Only in 'fix' mode, is the security.ima value written out on file
open.  The previous patch introduced the ability to set "arbitrary LSM
related attributes" without a security.evm label.

The patch itself is fine.  Please update the patch description.

thanks,

Mimi

> 
> This patch passes FILE_CREATED flag to IMA to reliably identify new
> files.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  fs/namei.c                            |  2 +-
>  fs/nfsd/vfs.c                         |  2 +-
>  include/linux/ima.h                   |  4 ++--
>  security/integrity/ima/ima.h          |  4 ++--
>  security/integrity/ima/ima_appraise.c |  4 ++--
>  security/integrity/ima/ima_main.c     | 14 +++++++-------
>  6 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 985c6f3..005771f 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -3058,7 +3058,7 @@ opened:
>  	error = open_check_o_direct(file);
>  	if (error)
>  		goto exit_fput;
> -	error = ima_file_check(file, op->acc_mode);
> +	error = ima_file_check(file, op->acc_mode, *opened);
>  	if (error)
>  		goto exit_fput;
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 140c496..d49c778 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -709,7 +709,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
>  		host_err = PTR_ERR(*filp);
>  		*filp = NULL;
>  	} else {
> -		host_err = ima_file_check(*filp, may_flags);
> +		host_err = ima_file_check(*filp, may_flags, 0);
> 
>  		if (may_flags & NFSD_MAY_64BIT_COOKIE)
>  			(*filp)->f_mode |= FMODE_64BITHASH;
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 1b7f268..23a87a4 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -15,7 +15,7 @@ struct linux_binprm;
> 
>  #ifdef CONFIG_IMA
>  extern int ima_bprm_check(struct linux_binprm *bprm);
> -extern int ima_file_check(struct file *file, int mask);
> +extern int ima_file_check(struct file *file, int mask, int opened);
>  extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
>  extern int ima_module_check(struct file *file);
> @@ -26,7 +26,7 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
>  	return 0;
>  }
> 
> -static inline int ima_file_check(struct file *file, int mask)
> +static inline int ima_file_check(struct file *file, int mask, int opened)
>  {
>  	return 0;
>  }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 3e9be3d..9337aa9 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -175,7 +175,7 @@ void ima_delete_rules(void);
>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>  			     struct file *file, const unsigned char *filename,
>  			     struct evm_ima_xattr_data *xattr_value,
> -			     int xattr_len);
> +			     int xattr_len, int opened);
>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
>  void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
>  enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> @@ -191,7 +191,7 @@ static inline int ima_appraise_measurement(int func,
>  					   struct file *file,
>  					   const unsigned char *filename,
>  					   struct evm_ima_xattr_data *xattr_value,
> -					   int xattr_len)
> +					   int xattr_len, int opened)
>  {
>  	return INTEGRITY_UNKNOWN;
>  }
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 3a4beb3..10679c8 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -175,7 +175,7 @@ int ima_read_xattr(struct dentry *dentry,
>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>  			     struct file *file, const unsigned char *filename,
>  			     struct evm_ima_xattr_data *xattr_value,
> -			     int xattr_len)
> +			     int xattr_len, int opened)
>  {
>  	static const char op[] = "appraise_data";
>  	char *cause = "unknown";
> @@ -195,7 +195,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> 
>  		cause = "missing-hash";
>  		status = INTEGRITY_NOLABEL;
> -		if (inode->i_size == 0) {
> +		if (opened & FILE_CREATED) {
>  			iint->flags |= IMA_NEW_FILE;
>  			status = INTEGRITY_PASS;
>  		}
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5a870e7..3384036 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -157,7 +157,7 @@ void ima_file_free(struct file *file)
>  }
> 
>  static int process_measurement(struct file *file, const char *filename,
> -			       int mask, int function)
> +			       int mask, int function, int opened)
>  {
>  	struct inode *inode = file_inode(file);
>  	struct integrity_iint_cache *iint;
> @@ -238,7 +238,7 @@ static int process_measurement(struct file *file, const char *filename,
>  	if (action & IMA_APPRAISE_SUBMASK) {
>  		mutex_lock(&inode->i_mutex);
>  		rc = ima_appraise_measurement(_func, iint, file, pathname,
> -					      xattr_value, xattr_len);
> +					      xattr_value, xattr_len, opened);
>  		mutex_unlock(&inode->i_mutex);
>  	}
>  	if (action & IMA_AUDIT)
> @@ -269,7 +269,7 @@ out_unlocked:
>  int ima_file_mmap(struct file *file, unsigned long prot)
>  {
>  	if (file && (prot & PROT_EXEC))
> -		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK);
> +		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0);
>  	return 0;
>  }
> 
> @@ -291,7 +291,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
>  	return process_measurement(bprm->file,
>  				   (strcmp(bprm->filename, bprm->interp) == 0) ?
>  				   bprm->filename : bprm->interp,
> -				   MAY_EXEC, BPRM_CHECK);
> +				   MAY_EXEC, BPRM_CHECK, 0);
>  }
> 
>  /**
> @@ -304,12 +304,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
>   * On success return 0.  On integrity appraisal error, assuming the file
>   * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
>   */
> -int ima_file_check(struct file *file, int mask)
> +int ima_file_check(struct file *file, int mask, int opened)
>  {
>  	ima_rdwr_violation_check(file);
>  	return process_measurement(file, NULL,
>  				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
> -				   FILE_CHECK);
> +				   FILE_CHECK, opened);
>  }
>  EXPORT_SYMBOL_GPL(ima_file_check);
> 
> @@ -332,7 +332,7 @@ int ima_module_check(struct file *file)
>  #endif
>  		return 0;	/* We rely on module signature checking */
>  	}
> -	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK);
> +	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0);
>  }
> 
>  static int __init init_ima(void)

Re: [PATCH v1 3/3] ima: pass 'opened' flag to identify newly created files

[Dmitry Kasatkin]

On 16/07/14 01:12, Mimi Zohar wrote:
> On Fri, 2014-07-11 at 14:47 +0300, Dmitry Kasatkin wrote: 
>> Empty file size and missing xattrs do not guaranty that file
> ^guarantee
>
>> was just created. It could be originally made empty and labeled
>> with needed LSM labels. Current implementation makes it possible
>> to remove security.ima, and set arbitrary LSM related attribute.
>> On open, IMA would be forced to update security.evm to 'fake' LSM
>> xattrs.
> Only in 'fix' mode, is the security.ima value written out on file
> open.  The previous patch introduced the ability to set "arbitrary LSM
> related attributes" without a security.evm label.

Comment is a bit unclear to me...

Previous patch does not allow to set arbitrary LSM value,
but if runtime permission allows, it allows to set "initial" xattrs for
newly created files...

I think description I made is a bit unclear..

What I wanted to tell is that..

"Assuming that empty file is a newly created file, IMA skips EVM
verification which allows "offline" removing security.ima and set
arbitrary security xattrs. Updating and closing file will make EVM to
update security.evm with forged secirity xattrs.

The question is if making file empty on purpose, clearing security.ima
and changing xattrs will allow any attack as file is empty.
If so, then using FILE_CREATED flag is safe choice.

- Dmitry


> The patch itself is fine.  Please update the patch description.
>
> thanks,
>
> Mimi
>
>> This patch passes FILE_CREATED flag to IMA to reliably identify new
>> files.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  fs/namei.c                            |  2 +-
>>  fs/nfsd/vfs.c                         |  2 +-
>>  include/linux/ima.h                   |  4 ++--
>>  security/integrity/ima/ima.h          |  4 ++--
>>  security/integrity/ima/ima_appraise.c |  4 ++--
>>  security/integrity/ima/ima_main.c     | 14 +++++++-------
>>  6 files changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 985c6f3..005771f 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -3058,7 +3058,7 @@ opened:
>>  	error = open_check_o_direct(file);
>>  	if (error)
>>  		goto exit_fput;
>> -	error = ima_file_check(file, op->acc_mode);
>> +	error = ima_file_check(file, op->acc_mode, *opened);
>>  	if (error)
>>  		goto exit_fput;
>>
>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>> index 140c496..d49c778 100644
>> --- a/fs/nfsd/vfs.c
>> +++ b/fs/nfsd/vfs.c
>> @@ -709,7 +709,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
>>  		host_err = PTR_ERR(*filp);
>>  		*filp = NULL;
>>  	} else {
>> -		host_err = ima_file_check(*filp, may_flags);
>> +		host_err = ima_file_check(*filp, may_flags, 0);
>>
>>  		if (may_flags & NFSD_MAY_64BIT_COOKIE)
>>  			(*filp)->f_mode |= FMODE_64BITHASH;
>> diff --git a/include/linux/ima.h b/include/linux/ima.h
>> index 1b7f268..23a87a4 100644
>> --- a/include/linux/ima.h
>> +++ b/include/linux/ima.h
>> @@ -15,7 +15,7 @@ struct linux_binprm;
>>
>>  #ifdef CONFIG_IMA
>>  extern int ima_bprm_check(struct linux_binprm *bprm);
>> -extern int ima_file_check(struct file *file, int mask);
>> +extern int ima_file_check(struct file *file, int mask, int opened);
>>  extern void ima_file_free(struct file *file);
>>  extern int ima_file_mmap(struct file *file, unsigned long prot);
>>  extern int ima_module_check(struct file *file);
>> @@ -26,7 +26,7 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
>>  	return 0;
>>  }
>>
>> -static inline int ima_file_check(struct file *file, int mask)
>> +static inline int ima_file_check(struct file *file, int mask, int opened)
>>  {
>>  	return 0;
>>  }
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 3e9be3d..9337aa9 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -175,7 +175,7 @@ void ima_delete_rules(void);
>>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>>  			     struct file *file, const unsigned char *filename,
>>  			     struct evm_ima_xattr_data *xattr_value,
>> -			     int xattr_len);
>> +			     int xattr_len, int opened);
>>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
>>  void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
>>  enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
>> @@ -191,7 +191,7 @@ static inline int ima_appraise_measurement(int func,
>>  					   struct file *file,
>>  					   const unsigned char *filename,
>>  					   struct evm_ima_xattr_data *xattr_value,
>> -					   int xattr_len)
>> +					   int xattr_len, int opened)
>>  {
>>  	return INTEGRITY_UNKNOWN;
>>  }
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index 3a4beb3..10679c8 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -175,7 +175,7 @@ int ima_read_xattr(struct dentry *dentry,
>>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>>  			     struct file *file, const unsigned char *filename,
>>  			     struct evm_ima_xattr_data *xattr_value,
>> -			     int xattr_len)
>> +			     int xattr_len, int opened)
>>  {
>>  	static const char op[] = "appraise_data";
>>  	char *cause = "unknown";
>> @@ -195,7 +195,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
>>
>>  		cause = "missing-hash";
>>  		status = INTEGRITY_NOLABEL;
>> -		if (inode->i_size == 0) {
>> +		if (opened & FILE_CREATED) {
>>  			iint->flags |= IMA_NEW_FILE;
>>  			status = INTEGRITY_PASS;
>>  		}
>> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
>> index 5a870e7..3384036 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -157,7 +157,7 @@ void ima_file_free(struct file *file)
>>  }
>>
>>  static int process_measurement(struct file *file, const char *filename,
>> -			       int mask, int function)
>> +			       int mask, int function, int opened)
>>  {
>>  	struct inode *inode = file_inode(file);
>>  	struct integrity_iint_cache *iint;
>> @@ -238,7 +238,7 @@ static int process_measurement(struct file *file, const char *filename,
>>  	if (action & IMA_APPRAISE_SUBMASK) {
>>  		mutex_lock(&inode->i_mutex);
>>  		rc = ima_appraise_measurement(_func, iint, file, pathname,
>> -					      xattr_value, xattr_len);
>> +					      xattr_value, xattr_len, opened);
>>  		mutex_unlock(&inode->i_mutex);
>>  	}
>>  	if (action & IMA_AUDIT)
>> @@ -269,7 +269,7 @@ out_unlocked:
>>  int ima_file_mmap(struct file *file, unsigned long prot)
>>  {
>>  	if (file && (prot & PROT_EXEC))
>> -		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK);
>> +		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0);
>>  	return 0;
>>  }
>>
>> @@ -291,7 +291,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
>>  	return process_measurement(bprm->file,
>>  				   (strcmp(bprm->filename, bprm->interp) == 0) ?
>>  				   bprm->filename : bprm->interp,
>> -				   MAY_EXEC, BPRM_CHECK);
>> +				   MAY_EXEC, BPRM_CHECK, 0);
>>  }
>>
>>  /**
>> @@ -304,12 +304,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
>>   * On success return 0.  On integrity appraisal error, assuming the file
>>   * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
>>   */
>> -int ima_file_check(struct file *file, int mask)
>> +int ima_file_check(struct file *file, int mask, int opened)
>>  {
>>  	ima_rdwr_violation_check(file);
>>  	return process_measurement(file, NULL,
>>  				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
>> -				   FILE_CHECK);
>> +				   FILE_CHECK, opened);
>>  }
>>  EXPORT_SYMBOL_GPL(ima_file_check);
>>
>> @@ -332,7 +332,7 @@ int ima_module_check(struct file *file)
>>  #endif
>>  		return 0;	/* We rely on module signature checking */
>>  	}
>> -	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK);
>> +	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0);
>>  }
>>
>>  static int __init init_ima(void)
>
>

Re: [PATCH v1 3/3] ima: pass 'opened' flag to identify newly created files

[Mimi Zohar]

On Wed, 2014-07-16 at 11:25 +0300, Dmitry Kasatkin wrote: 
> On 16/07/14 01:12, Mimi Zohar wrote:
> > On Fri, 2014-07-11 at 14:47 +0300, Dmitry Kasatkin wrote: 
> >> Empty file size and missing xattrs do not guaranty that file
> > ^guarantee
> >
> >> was just created. It could be originally made empty and labeled
> >> with needed LSM labels. Current implementation makes it possible
> >> to remove security.ima, and set arbitrary LSM related attribute.
> >> On open, IMA would be forced to update security.evm to 'fake' LSM
> >> xattrs.
> > Only in 'fix' mode, is the security.ima value written out on file
> > open.  The previous patch introduced the ability to set "arbitrary LSM
> > related attributes" without a security.evm label.
> 
> Comment is a bit unclear to me...
> 
> Previous patch does not allow to set arbitrary LSM value,
> but if runtime permission allows, it allows to set "initial" xattrs for
> newly created files...
> 
> I think description I made is a bit unclear..
> 
> What I wanted to tell is that..
> 
> "Assuming that empty file is a newly created file, IMA skips EVM
> verification which allows "offline" removing security.ima and set
> arbitrary security xattrs. Updating and closing file will make EVM to
> update security.evm with forged secirity xattrs.

Sure the security IMA and EVM xattrs can be removed offline, but can not
be replaced one without the other.  The file size indicates  a "new"
file, only if there aren't any existing xattrs.  The question is whether
just removing an xattr provides a benefit.

> The question is if making file empty on purpose, clearing security.ima
> and changing xattrs will allow any attack as file is empty.
> If so, then using FILE_CREATED flag is safe choice.
> 
> - Dmitry

Can we say, "Clearly identifying new files further limits possible
attacks."

Mimi

> 
> > The patch itself is fine.  Please update the patch description.
> >
> > thanks,
> >
> > Mimi
> >
> >> This patch passes FILE_CREATED flag to IMA to reliably identify new
> >> files.
> >>
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >> ---
> >>  fs/namei.c                            |  2 +-
> >>  fs/nfsd/vfs.c                         |  2 +-
> >>  include/linux/ima.h                   |  4 ++--
> >>  security/integrity/ima/ima.h          |  4 ++--
> >>  security/integrity/ima/ima_appraise.c |  4 ++--
> >>  security/integrity/ima/ima_main.c     | 14 +++++++-------
> >>  6 files changed, 15 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/fs/namei.c b/fs/namei.c
> >> index 985c6f3..005771f 100644
> >> --- a/fs/namei.c
> >> +++ b/fs/namei.c
> >> @@ -3058,7 +3058,7 @@ opened:
> >>  	error = open_check_o_direct(file);
> >>  	if (error)
> >>  		goto exit_fput;
> >> -	error = ima_file_check(file, op->acc_mode);
> >> +	error = ima_file_check(file, op->acc_mode, *opened);
> >>  	if (error)
> >>  		goto exit_fput;
> >>
> >> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> >> index 140c496..d49c778 100644
> >> --- a/fs/nfsd/vfs.c
> >> +++ b/fs/nfsd/vfs.c
> >> @@ -709,7 +709,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
> >>  		host_err = PTR_ERR(*filp);
> >>  		*filp = NULL;
> >>  	} else {
> >> -		host_err = ima_file_check(*filp, may_flags);
> >> +		host_err = ima_file_check(*filp, may_flags, 0);
> >>
> >>  		if (may_flags & NFSD_MAY_64BIT_COOKIE)
> >>  			(*filp)->f_mode |= FMODE_64BITHASH;
> >> diff --git a/include/linux/ima.h b/include/linux/ima.h
> >> index 1b7f268..23a87a4 100644
> >> --- a/include/linux/ima.h
> >> +++ b/include/linux/ima.h
> >> @@ -15,7 +15,7 @@ struct linux_binprm;
> >>
> >>  #ifdef CONFIG_IMA
> >>  extern int ima_bprm_check(struct linux_binprm *bprm);
> >> -extern int ima_file_check(struct file *file, int mask);
> >> +extern int ima_file_check(struct file *file, int mask, int opened);
> >>  extern void ima_file_free(struct file *file);
> >>  extern int ima_file_mmap(struct file *file, unsigned long prot);
> >>  extern int ima_module_check(struct file *file);
> >> @@ -26,7 +26,7 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
> >>  	return 0;
> >>  }
> >>
> >> -static inline int ima_file_check(struct file *file, int mask)
> >> +static inline int ima_file_check(struct file *file, int mask, int opened)
> >>  {
> >>  	return 0;
> >>  }
> >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> >> index 3e9be3d..9337aa9 100644
> >> --- a/security/integrity/ima/ima.h
> >> +++ b/security/integrity/ima/ima.h
> >> @@ -175,7 +175,7 @@ void ima_delete_rules(void);
> >>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> >>  			     struct file *file, const unsigned char *filename,
> >>  			     struct evm_ima_xattr_data *xattr_value,
> >> -			     int xattr_len);
> >> +			     int xattr_len, int opened);
> >>  int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
> >>  void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
> >>  enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> >> @@ -191,7 +191,7 @@ static inline int ima_appraise_measurement(int func,
> >>  					   struct file *file,
> >>  					   const unsigned char *filename,
> >>  					   struct evm_ima_xattr_data *xattr_value,
> >> -					   int xattr_len)
> >> +					   int xattr_len, int opened)
> >>  {
> >>  	return INTEGRITY_UNKNOWN;
> >>  }
> >> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> >> index 3a4beb3..10679c8 100644
> >> --- a/security/integrity/ima/ima_appraise.c
> >> +++ b/security/integrity/ima/ima_appraise.c
> >> @@ -175,7 +175,7 @@ int ima_read_xattr(struct dentry *dentry,
> >>  int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> >>  			     struct file *file, const unsigned char *filename,
> >>  			     struct evm_ima_xattr_data *xattr_value,
> >> -			     int xattr_len)
> >> +			     int xattr_len, int opened)
> >>  {
> >>  	static const char op[] = "appraise_data";
> >>  	char *cause = "unknown";
> >> @@ -195,7 +195,7 @@ int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> >>
> >>  		cause = "missing-hash";
> >>  		status = INTEGRITY_NOLABEL;
> >> -		if (inode->i_size == 0) {
> >> +		if (opened & FILE_CREATED) {
> >>  			iint->flags |= IMA_NEW_FILE;
> >>  			status = INTEGRITY_PASS;
> >>  		}
> >> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> >> index 5a870e7..3384036 100644
> >> --- a/security/integrity/ima/ima_main.c
> >> +++ b/security/integrity/ima/ima_main.c
> >> @@ -157,7 +157,7 @@ void ima_file_free(struct file *file)
> >>  }
> >>
> >>  static int process_measurement(struct file *file, const char *filename,
> >> -			       int mask, int function)
> >> +			       int mask, int function, int opened)
> >>  {
> >>  	struct inode *inode = file_inode(file);
> >>  	struct integrity_iint_cache *iint;
> >> @@ -238,7 +238,7 @@ static int process_measurement(struct file *file, const char *filename,
> >>  	if (action & IMA_APPRAISE_SUBMASK) {
> >>  		mutex_lock(&inode->i_mutex);
> >>  		rc = ima_appraise_measurement(_func, iint, file, pathname,
> >> -					      xattr_value, xattr_len);
> >> +					      xattr_value, xattr_len, opened);
> >>  		mutex_unlock(&inode->i_mutex);
> >>  	}
> >>  	if (action & IMA_AUDIT)
> >> @@ -269,7 +269,7 @@ out_unlocked:
> >>  int ima_file_mmap(struct file *file, unsigned long prot)
> >>  {
> >>  	if (file && (prot & PROT_EXEC))
> >> -		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK);
> >> +		return process_measurement(file, NULL, MAY_EXEC, MMAP_CHECK, 0);
> >>  	return 0;
> >>  }
> >>
> >> @@ -291,7 +291,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
> >>  	return process_measurement(bprm->file,
> >>  				   (strcmp(bprm->filename, bprm->interp) == 0) ?
> >>  				   bprm->filename : bprm->interp,
> >> -				   MAY_EXEC, BPRM_CHECK);
> >> +				   MAY_EXEC, BPRM_CHECK, 0);
> >>  }
> >>
> >>  /**
> >> @@ -304,12 +304,12 @@ int ima_bprm_check(struct linux_binprm *bprm)
> >>   * On success return 0.  On integrity appraisal error, assuming the file
> >>   * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
> >>   */
> >> -int ima_file_check(struct file *file, int mask)
> >> +int ima_file_check(struct file *file, int mask, int opened)
> >>  {
> >>  	ima_rdwr_violation_check(file);
> >>  	return process_measurement(file, NULL,
> >>  				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
> >> -				   FILE_CHECK);
> >> +				   FILE_CHECK, opened);
> >>  }
> >>  EXPORT_SYMBOL_GPL(ima_file_check);
> >>
> >> @@ -332,7 +332,7 @@ int ima_module_check(struct file *file)
> >>  #endif
> >>  		return 0;	/* We rely on module signature checking */
> >>  	}
> >> -	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK);
> >> +	return process_measurement(file, NULL, MAY_EXEC, MODULE_CHECK, 0);
> >>  }
> >>
> >>  static int __init init_ima(void)
> >
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>