Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1702338 > unrolled thread

Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal

Started byThiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
First post2017-08-02 19:50 +0200
Last post2017-08-03 23:10 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-08-02 19:50 +0200
    Re: [PATCH v3 7/7] ima: Support module-style appended signatures  for appraisal Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-08-03 01:00 +0200
      Re: [PATCH v3 7/7] ima: Support module-style appended signatures  for appraisal Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-08-03 16:40 +0200
        Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-08-03 23:10 +0200

#1702338 — Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal

FromThiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Date2017-08-02 19:50 +0200
SubjectRe: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal
Message-ID<ua5Wq-4AY-31@gated-at.bofh.it>
Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> On Thu, 2017-07-06 at 19:17 -0300, Thiago Jung Bauermann wrote:
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -200,18 +200,40 @@ int ima_read_xattr(struct dentry *dentry,
>>   */
>>  int ima_appraise_measurement(enum ima_hooks func,
>>  			     struct integrity_iint_cache *iint,
>> -			     struct file *file, const unsigned char *filename,
>> -			     struct evm_ima_xattr_data *xattr_value,
>> -			     int xattr_len, int opened)
>> +			     struct file *file, const void *buf, loff_t size,
>> +			     const unsigned char *filename,
>> +			     struct evm_ima_xattr_data **xattr_value_,
>> +			     int *xattr_len_, int opened)
>>  {
>>  	static const char op[] = "appraise_data";
>>  	char *cause = "unknown";
>>  	struct dentry *dentry = file_dentry(file);
>>  	struct inode *inode = d_backing_inode(dentry);
>>  	enum integrity_status status = INTEGRITY_UNKNOWN;
>> -	int rc = xattr_len, hash_start = 0;
>> +	struct evm_ima_xattr_data *xattr_value = *xattr_value_;
>> +	int xattr_len = *xattr_len_, rc = xattr_len, hash_start = 0;
>> +	bool appraising_modsig = false;
>> +	void *xattr_value_evm;
>> +	size_t xattr_len_evm;
>> +
>> +	if (iint->flags & IMA_MODSIG_ALLOWED) {
>> +		/*
>> +		 * Not supposed to happen. Hooks that support modsig are
>> +		 * whitelisted when parsing the policy using
>> +		 * ima_hooks_supports_modsig.
>> +		 */
>> +		if (!buf || !size)
>> +			WARN_ONCE(true, "%s doesn't support modsig\n",
>> +				  func_tokens[func]);
>
> ima _appraise_measurement() is getting kind of long. Is there any
> reason we can't move this comment and test to ima_read_modsig()?

I didn't do that because then I would need to pass func as an argument
to ima_read_modsig just to print the warning above. But it does simplify
the code so it may be worth it. I'll make that change in v4.

>> @@ -229,8 +251,24 @@ int ima_appraise_measurement(enum ima_hooks func,
>>  		goto out;
>>  	}
>> 
>> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
>> -	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
>> +	/*
>> +	 * Appended signatures aren't protected by EVM but we still call
>> +	 * evm_verifyxattr to check other security xattrs, if they exist.
>> +	 */
>> +	if (appraising_modsig) {
>> +		xattr_value_evm = NULL;
>> +		xattr_len_evm = 0;
>> +	} else {
>> +		xattr_value_evm = xattr_value;
>> +		xattr_len_evm = xattr_len;
>> +	}
>> +
>> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
>> +				 xattr_len_evm, iint);
>> +	if (appraising_modsig && status == INTEGRITY_FAIL) {
>> +		cause = "invalid-HMAC";
>> +		goto out;
>
> "modsig" is special, because having any security xattrs is not
> required. This test doesn't prevent status from being set to
> "missing-HMAC". This test is redundant with the original tests below.

Indeed, that is wrong. I'm still a bit fuzzy about how EVM works and how
it interacts with IMA. The only way I can think of singling out modsig
without reintroduced the complex expression you didn't like in v2 is as
below. What do you think?

@@ -229,8 +241,25 @@ int ima_appraise_measurement(enum ima_hooks func,
 		goto out;
 	}
 
-	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
-	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
+	/*
+	 * Appended signatures aren't protected by EVM but we still call
+	 * evm_verifyxattr to check other security xattrs, if they exist.
+	 */
+	if (appraising_modsig) {
+		xattr_value_evm = NULL;
+		xattr_len_evm = 0;
+	} else {
+		xattr_value_evm = xattr_value;
+		xattr_len_evm = xattr_len;
+	}
+
+	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
+				 xattr_len_evm, iint);
+	if (appraising_modsig && (status == INTEGRITY_NOLABEL
+				  || status == INTEGRITY_NOXATTRS))
+		/* It's ok if there's no xattr in the case of modsig. */
+		;
+	else if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
 		if ((status == INTEGRITY_NOLABEL)
 		    || (status == INTEGRITY_NOXATTRS))
 			cause = "missing-HMAC";

>> +	} else if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
>>  		if ((status == INTEGRITY_NOLABEL)
>>  		    || (status == INTEGRITY_NOXATTRS))
>>  			cause = "missing-HMAC";
>> @@ -281,6 +319,43 @@ int ima_appraise_measurement(enum ima_hooks func,
>>  			status = INTEGRITY_PASS;
>>  		}
>
> Calling evm_verifyxattr() with the IMA xattr value prevents EVM from
> having to re-read the IMA xattr, but isn't necessary.On modsig
> signature verification failure, calling evm_verifyxattr() a second
> time isn't necessary.

So even for the IMA xattr sig case, the evm_verifyxattr call in
ima_appraise_measurement is an optimization and can be skipped?

>> +	case IMA_MODSIG:
>> +		/*
>> +		 * To avoid being tricked into recursion, we don't allow a
>> +		 * modsig stored in the xattr.
>> +		 */
>> +		if (!appraising_modsig) {
>> +			status = INTEGRITY_UNKNOWN;
>> +			cause = "unknown-ima-data";
>> +
>> +			break;
>> +		}
>> +
>> +		rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA, xattr_value);
>> +		if (!rc) {
>> +			iint->flags |= IMA_DIGSIG;
>> +			status = 
>> +
>> +			kfree(*xattr_value_);
>> +			*xattr_value_ = xattr_value;
>> +			*xattr_len_ = xattr_len;
>> +
>> +			break;
>> +		}
>
> When including the appended signature in the measurement list, the
> corresponding file hash needs to be included in the measurement list,
> which might be different than the previously calculated file hash
> based on the hash algorithm as defined in the IMA xattr.
>
> Including the file hash and signature in the measurement list allows
> the attestation server, with just a public key, to verify the file
> signature against the file hash. No need for a white list.
>
> ima_modsig_verify() must calculate the file hash in order to verify
> the file signature. This file hash value somehow needs to be returned
> in order for it to be included in the measurement list.

In that case, patch 6/7 "ima: Store measurement after appraisal" isn't
enough and we have to go back to v2's change in ima_main.c which ties
together the collect and appraise steps in process_measurement (In that
version I called the function measure_and_appraise but it should be
called collect_and_appraise instead). That is because if the modsig
verification fails, the hash needs to be recalculated for the xattr
signature verification.

Either that, or I add another call to ima_collect_measurement inside
ima_appraise_measurement if the modsig verification fails. Which do you
prefer?

>> +		/*
>> +		 * The appended signature failed verification. Let's try
>> +		 * reading a signature from the extended attribute instead.
>> +		 */
>> +
>> +		pr_debug("modsig didn't verify, trying the xattr signature\n");
>> +
>> +		ima_free_xattr_data(xattr_value);
>> +		iint->flags &= ~IMA_MODSIG_ALLOWED;
>> +
>> +		return ima_appraise_measurement(func, iint, file, buf, size,
>> +						filename, xattr_value_,
>> +						xattr_len_, opened);
>
> Most of the code before "switch" needs to be done only once. Is
> recursion necessary? Or can we just retry the "switch" using the IMA
> xattr, assuming there is an IMA xattr?

I used recursion to avoid duplicating two blocks of code: the logic in
the "if (rc <= 0)" block which needs to be done again when verifying the
xattr sig, and also the logic of interpreting the return value of
evm_verifyxattr which I also thought needed to be done again, but you
seem to be saying that is just an optimization and can be skipped.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[toc] | [next] | [standalone]

#1702543 — Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal

FromMimi Zohar <zohar@linux.vnet.ibm.com>
Date2017-08-03 01:00 +0200
SubjectRe: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal
Message-ID<uaaMq-7QI-13@gated-at.bofh.it>
In reply to#1702338
On Wed, 2017-08-02 at 14:42 -0300, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > On Thu, 2017-07-06 at 19:17 -0300, Thiago Jung Bauermann wrote:
> >> --- a/security/integrity/ima/ima_appraise.c
> >> +++ b/security/integrity/ima/ima_appraise.c
> >> @@ -200,18 +200,40 @@ int ima_read_xattr(struct dentry *dentry,
> >>   */
> >>  int ima_appraise_measurement(enum ima_hooks func,
> >>  			     struct integrity_iint_cache *iint,
> >> -			     struct file *file, const unsigned char *filename,
> >> -			     struct evm_ima_xattr_data *xattr_value,
> >> -			     int xattr_len, int opened)
> >> +			     struct file *file, const void *buf, loff_t size,
> >> +			     const unsigned char *filename,
> >> +			     struct evm_ima_xattr_data **xattr_value_,
> >> +			     int *xattr_len_, int opened)
> >>  {
> >>  	static const char op[] = "appraise_data";
> >>  	char *cause = "unknown";
> >>  	struct dentry *dentry = file_dentry(file);
> >>  	struct inode *inode = d_backing_inode(dentry);
> >>  	enum integrity_status status = INTEGRITY_UNKNOWN;
> >> -	int rc = xattr_len, hash_start = 0;
> >> +	struct evm_ima_xattr_data *xattr_value = *xattr_value_;
> >> +	int xattr_len = *xattr_len_, rc = xattr_len, hash_start = 0;
> >> +	bool appraising_modsig = false;
> >> +	void *xattr_value_evm;
> >> +	size_t xattr_len_evm;
> >> +
> >> +	if (iint->flags & IMA_MODSIG_ALLOWED) {
> >> +		/*
> >> +		 * Not supposed to happen. Hooks that support modsig are
> >> +		 * whitelisted when parsing the policy using
> >> +		 * ima_hooks_supports_modsig.
> >> +		 */
> >> +		if (!buf || !size)
> >> +			WARN_ONCE(true, "%s doesn't support modsig\n",
> >> +				  func_tokens[func]);
> >
> > ima _appraise_measurement() is getting kind of long. Is there any
> > reason we can't move this comment and test to ima_read_modsig()?
> 
> I didn't do that because then I would need to pass func as an argument
> to ima_read_modsig just to print the warning above. But it does simplify
> the code so it may be worth it. I'll make that change in v4.

Makes sense.

> >> @@ -229,8 +251,24 @@ int ima_appraise_measurement(enum ima_hooks func,
> >>  		goto out;
> >>  	}
> >> 
> >> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
> >> -	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
> >> +	/*
> >> +	 * Appended signatures aren't protected by EVM but we still call
> >> +	 * evm_verifyxattr to check other security xattrs, if they exist.
> >> +	 */
> >> +	if (appraising_modsig) {
> >> +		xattr_value_evm = NULL;
> >> +		xattr_len_evm = 0;
> >> +	} else {
> >> +		xattr_value_evm = xattr_value;
> >> +		xattr_len_evm = xattr_len;
> >> +	}
> >> +
> >> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
> >> +				 xattr_len_evm, iint);
> >> +	if (appraising_modsig && status == INTEGRITY_FAIL) {
> >> +		cause = "invalid-HMAC";
> >> +		goto out;
> >
> > "modsig" is special, because having any security xattrs is not
> > required. This test doesn't prevent status from being set to
> > "missing-HMAC". This test is redundant with the original tests below.
> 
> Indeed, that is wrong. I'm still a bit fuzzy about how EVM works and how
> it interacts with IMA. The only way I can think of singling out modsig
> without reintroduced the complex expression you didn't like in v2 is as
> below. What do you think?

The original code, without any extra tests, should be fine.

> 
> @@ -229,8 +241,25 @@ int ima_appraise_measurement(enum ima_hooks func,
>  		goto out;
>  	}
> 
> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
> -	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
> +	/*
> +	 * Appended signatures aren't protected by EVM but we still call
> +	 * evm_verifyxattr to check other security xattrs, if they exist.
> +	 */
> +	if (appraising_modsig) {
> +		xattr_value_evm = NULL;
> +		xattr_len_evm = 0;
> +	} else {
> +		xattr_value_evm = xattr_value;
> +		xattr_len_evm = xattr_len;
> +	}
> +
> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
> +				 xattr_len_evm, iint);
> +	if (appraising_modsig && (status == INTEGRITY_NOLABEL
> +				  || status == INTEGRITY_NOXATTRS))
> +		/* It's ok if there's no xattr in the case of modsig. */
> +		;
> +	else if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
>  		if ((status == INTEGRITY_NOLABEL)
>  		    || (status == INTEGRITY_NOXATTRS))
>  			cause = "missing-HMAC";
> 
> >> +	} else if (status != INTEGRITY_PASS && status != INTEGRITY_UNKNOWN) {
> >>  		if ((status == INTEGRITY_NOLABEL)
> >>  		    || (status == INTEGRITY_NOXATTRS))
> >>  			cause = "missing-HMAC";
> >> @@ -281,6 +319,43 @@ int ima_appraise_measurement(enum ima_hooks func,
> >>  			status = INTEGRITY_PASS;
> >>  		}
> >
> > Calling evm_verifyxattr() with the IMA xattr value prevents EVM from
> > having to re-read the IMA xattr, but isn't necessary.On modsig
> > signature verification failure, calling evm_verifyxattr() a second
> > time isn't necessary.
> 
> So even for the IMA xattr sig case, the evm_verifyxattr call in
> ima_appraise_measurement is an optimization and can be skipped?

Right, it is just an optimization.  The evm xattr needs to be verified
just once.

> >> +	case IMA_MODSIG:
> >> +		/*
> >> +		 * To avoid being tricked into recursion, we don't allow a
> >> +		 * modsig stored in the xattr.
> >> +		 */
> >> +		if (!appraising_modsig) {
> >> +			status = INTEGRITY_UNKNOWN;
> >> +			cause = "unknown-ima-data";
> >> +
> >> +			break;
> >> +		}
> >> +
> >> +		rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA, xattr_value);
> >> +		if (!rc) {
> >> +			iint->flags |= IMA_DIGSIG;
> >> +			status = 
> >> +
> >> +			kfree(*xattr_value_);
> >> +			*xattr_value_ = xattr_value;
> >> +			*xattr_len_ = xattr_len;
> >> +
> >> +			break;
> >> +		}
> >
> > When including the appended signature in the measurement list, the
> > corresponding file hash needs to be included in the measurement list,
> > which might be different than the previously calculated file hash
> > based on the hash algorithm as defined in the IMA xattr.
> >
> > Including the file hash and signature in the measurement list allows
> I> the attestation server, with just a public key, to verify the file
> > signature against the file hash. No need for a white list.
> >
> > ima_modsig_verify() must calculate the file hash in order to verify
> > the file signature. This file hash value somehow needs to be returned
> > in order for it to be included in the measurement list.
> 
> In that case, patch 6/7 "ima: Store measurement after appraisal" isn't
> enough and we have to go back to v2's change in ima_main.c which ties
> together the collect and appraise steps in process_measurement (In that
> version I called the function measure_and_appraise but it should be
> called collect_and_appraise instead). That is because if the modsig
> verification fails, the hash needs to be recalculated for the xattr
> signature verification.

> Either that, or I add another call to ima_collect_measurement inside
> ima_appraise_measurement if the modsig verification fails. Which do you
> prefer?

The file hash (without the appended signature) is already being
calculated by verify_pkcs7_message_sig().  There's no reason to
calculate it twice.

If the appended signature verification succeeds, that means the file
hash stored in the appended signature was valid.  Somehow we need
access to sig->digest, sig->digest_size and sig->hash_algo, which was
compared against the calculated hash.  Refer to
public_key_verify_signature().

Mimi

[toc] | [prev] | [next] | [standalone]

#1703169 — Re: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal

FromMimi Zohar <zohar@linux.vnet.ibm.com>
Date2017-08-03 16:40 +0200
SubjectRe: [PATCH v3 7/7] ima: Support module-style appended signatures for appraisal
Message-ID<uaps6-1gd-23@gated-at.bofh.it>
In reply to#1702543
On Wed, 2017-08-02 at 18:52 -0400, Mimi Zohar wrote:
> On Wed, 2017-08-02 at 14:42 -0300, Thiago Jung Bauermann wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> > >> @@ -229,8 +251,24 @@ int ima_appraise_measurement(enum ima_hooks func,
> > >>  		goto out;
> > >>  	}
> > >> 
> > >> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
> > >> -	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
> > >> +	/*
> > >> +	 * Appended signatures aren't protected by EVM but we still call
> > >> +	 * evm_verifyxattr to check other security xattrs, if they exist.
> > >> +	 */
> > >> +	if (appraising_modsig) {
> > >> +		xattr_value_evm = NULL;
> > >> +		xattr_len_evm = 0;
> > >> +	} else {
> > >> +		xattr_value_evm = xattr_value;
> > >> +		xattr_len_evm = xattr_len;
> > >> +	}
> > >> +
> > >> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
> > >> +				 xattr_len_evm, iint);
> > >> +	if (appraising_modsig && status == INTEGRITY_FAIL) {
> > >> +		cause = "invalid-HMAC";
> > >> +		goto out;
> > >
> > > "modsig" is special, because having any security xattrs is not
> > > required. This test doesn't prevent status from being set to
> > > "missing-HMAC". This test is redundant with the original tests below.
> > 
> > Indeed, that is wrong. I'm still a bit fuzzy about how EVM works and how
> > it interacts with IMA. The only way I can think of singling out modsig
> > without reintroduced the complex expression you didn't like in v2 is as
> > below. What do you think?
> 
> The original code, without any extra tests, should be fine.

There is one major difference.

EVM verifies a file's metadata has not been modified based on either
an HMAC or signature stored as security.evm.  Prior to the appended
signatures patch set, all files in policy required a security.evm
xattr. With IMA enabled we could guarantee that at least one security
xattr existed.  The only exception were new files, which hadn't yet
been labeled. 

With appended signatures, there is now no guarantee that at least one
security xattr exists.

Perhaps the code snippet below will help clarify the meaning of the
integrity_status results. 

        switch (status) {
        case INTEGRITY_PASS:
        case INTEGRITY_UNKNOWN:      
              break;		
        case INTEGRITY_NOXATTRS:        /* no EVM protected xattrs */
                if (appraising_modsig)
                        break;
        case INTEGRITY_NOLABEL:         /* no security.evm xattr */
                cause = "missing-HMAC";
                fail = 1;
                break;
        case INTEGRITY_FAIL:            /* invalid HMAC/signature */
        default:
                cause = "invalid-HMAC";
                fail = 1;
                break;
        }

Mimi

[toc] | [prev] | [next] | [standalone]

#1703415

FromThiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Date2017-08-03 23:10 +0200
Message-ID<uavxv-5BE-7@gated-at.bofh.it>
In reply to#1703169
Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> On Wed, 2017-08-02 at 18:52 -0400, Mimi Zohar wrote:
>> On Wed, 2017-08-02 at 14:42 -0300, Thiago Jung Bauermann wrote:
>> > Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>
>> > >> @@ -229,8 +251,24 @@ int ima_appraise_measurement(enum ima_hooks func,
>> > >>  		goto out;
>> > >>  	}
>> > >> 
>> > >> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
>> > >> -	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
>> > >> +	/*
>> > >> +	 * Appended signatures aren't protected by EVM but we still call
>> > >> +	 * evm_verifyxattr to check other security xattrs, if they exist.
>> > >> +	 */
>> > >> +	if (appraising_modsig) {
>> > >> +		xattr_value_evm = NULL;
>> > >> +		xattr_len_evm = 0;
>> > >> +	} else {
>> > >> +		xattr_value_evm = xattr_value;
>> > >> +		xattr_len_evm = xattr_len;
>> > >> +	}
>> > >> +
>> > >> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value_evm,
>> > >> +				 xattr_len_evm, iint);
>> > >> +	if (appraising_modsig && status == INTEGRITY_FAIL) {
>> > >> +		cause = "invalid-HMAC";
>> > >> +		goto out;
>> > >
>> > > "modsig" is special, because having any security xattrs is not
>> > > required. This test doesn't prevent status from being set to
>> > > "missing-HMAC". This test is redundant with the original tests below.
>> > 
>> > Indeed, that is wrong. I'm still a bit fuzzy about how EVM works and how
>> > it interacts with IMA. The only way I can think of singling out modsig
>> > without reintroduced the complex expression you didn't like in v2 is as
>> > below. What do you think?
>> 
>> The original code, without any extra tests, should be fine.
>
> There is one major difference.
>
> EVM verifies a file's metadata has not been modified based on either
> an HMAC or signature stored as security.evm. Prior to the appended
> signatures patch set, all files in policy required a security.evm
> xattr. With IMA enabled we could guarantee that at least one security
> xattr existed. The only exception were new files, which hadn't yet
> been labeled.
>
> With appended signatures, there is now no guarantee that at least one
> security xattr exists.
>
> Perhaps the code snippet below will help clarify the meaning of the
> integrity_status results.
>
>     switch (status) {
> case INTEGRITY_PASS:
> case INTEGRITY_UNKNOWN:   
>        break;		
>     case INTEGRITY_NOXATTRS:/* no EVM protected xattrs */
> if (appraising_modsig)
> break;
> case INTEGRITY_NOLABEL:/* no security.evm xattr */
> cause = "missing-HMAC";
> fail = 1;
> break;
> case INTEGRITY_FAIL:/* invalid HMAC/signature */
> default:
> cause = "invalid-HMAC";
> fail = 1;
> break;
> }

Thanks! I'll use the switch above in the next version of the patch.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web