[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-integrity
Subject:    Re: [RFC][PATCH 2/2] ima: don't measure files in the TCB if Biba strict policy is enforced
From:       Roberto Sassu <roberto.sassu () huawei ! com>
Date:       2017-10-24 12:38:14
Message-ID: 5045bc1d-0547-0895-9874-c837167c4b75 () huawei ! com
[Download RAW message or body]


On 10/23/2017 10:40 PM, Mimi Zohar wrote:
> 
> On Fri, 2017-10-20 at 17:41 +0200, Roberto Sassu wrote:
> > 
> > The Biba strict policy prevents processes outside the TCB from modifying
> > appraised files. Then, since the integrity of those files is preserved,
> > because only processes in the TCB can write appraised files, it is not
> > necessary to measure them each time they are accessed by the TCB.
> 
> 
> The builtin appraise_tcb appraises all files owned by root.  With this
> patch you've redefined TCB to be any currently loaded IMA policy.


Root processes are part of the TCB. Since a policy can be uploaded only
by a root process, it will be always appraised. Digital signature could
be required specifically for the POLICY_CHECK hook.


> 
> This solves one of the main problems of binary attestation: when a
> modified file is accessed by the TCB, it was necessary to measure it
> because verifiers cannot determine from the measurement list if the
> writer belong or not to the TCB. Verifiers find an unknown digest
> and have to consider the whole system as compromised.
> 
> If the Biba strict policy has been selected, and appraisal is in enforce
> mode, IMA measures files at first access, if they have a digital signature.
> Then, for subsequent accesses, files are not measured again, unless the
> appraisal status changes.


Signed files aren't changing, so there should only be one file
measurement in the measurement list.  So this only affects mutable
files.  We're going through a lot of effort to re-measure mutable
files after they change.


If appraised files can be written only by processes in the TCB, it is
not necessary to report the new digest, after their content changes,
because their integrity is preserved. It is sufficient to correctly
update the extended attribute.

Roberto



Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
---
  security/integrity/ima/ima_main.c | 17 +++++++++++++++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 6e85ea8e2373..16c2da0e32d9 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -200,10 +200,16 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
  			goto out;
  	}

-	if (ima_integrity_policy)
+	if (ima_integrity_policy) {
  		policy_violation = ima_appraise_biba_check(file, iint,
  						must_appraise, &pathbuf,
  						&pathname, filename);
+		/* do not measure mutable files, if they are appraised */
+		if (ima_integrity_policy == BIBA_STRICT &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+			if (iint && (iint->flags & IMA_APPRAISED))
+				action &= ~IMA_MEASURE;
+	}
  	if (violation_check)
  		ima_rdwr_violation_check(file, iint, action & IMA_MEASURE,
  					 &pathbuf, &pathname);
@@ -246,9 +252,16 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
  	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
  		pathname = ima_d_path(&file->f_path, &pathbuf, filename);

-	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK))
+	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
  		rc = ima_appraise_measurement(func, iint, file, pathname,
  					      xattr_value, xattr_len, opened);
+		if (!rc && ima_integrity_policy == BIBA_STRICT &&
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			iint->flags &= ~IMA_MEASURE;
+			if (!(iint->flags & IMA_DIGSIG))
+				action &= ~IMA_MEASURE;
+		}
+	}
  	if (action & IMA_MEASURE)
  		ima_store_measurement(iint, file, pathname,
  				      xattr_value, xattr_len, pcr);




--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG




[prev in list] [next in list] [prev in thread] [next in thread]