diff mbox series

[08/11] ima: Allow imasig requirement to be satisfied by EVM portable signatures

Message ID 20200618160458.1579-8-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show
Series [01/11] evm: Execute evm_inode_init_security() only when the HMAC key is loaded | expand

Commit Message

Roberto Sassu June 18, 2020, 4:04 p.m. UTC
System administrators can require that all accessed files have a signature
by specifying appraise_type=imasig in a policy rule.

Currently, only IMA signatures satisfy this requirement. IMA signatures
ensure data source authentication for file content and prevent any change.
EVM signatures instead ensure data source authentication for file metadata.
Given that the digest or signature of the file content must be included in
the metadata, EVM signatures provide at least the same guarantees of IMA
signatures.

This patch lets systems protected with EVM signatures pass appraisal
verification if the appraise_type=imasig requirement is specified in the
policy. This facilitates deployment in the scenarios where only EVM
signatures are available.

The patch makes the following changes:

file xattr types:
security.ima: IMA_XATTR_DIGEST/IMA_XATTR_DIGEST_NG
security.evm: EVM_XATTR_PORTABLE_DIGSIG

execve(), mmap(), open() behavior (with appraise_type=imasig):
before: denied (file without IMA signature, imasig requirement not met)
after: allowed (file with EVM portable signature, imasig requirement met)

open(O_WRONLY) behavior (without appraise_type=imasig):
before: allowed (file without IMA signature, not immutable)
after: denied (file with EVM portable signature, immutable)

In addition, similarly to IMA signatures, this patch temporarily allows
new files without or with incomplete metadata to be opened so that content
can be written.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_appraise.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

Comments

Mimi Zohar Aug. 24, 2020, 1:02 p.m. UTC | #1
On Thu, 2020-06-18 at 18:04 +0200, Roberto Sassu wrote:
> System administrators can require that all accessed files have a signature
> by specifying appraise_type=imasig in a policy rule.
> 
> Currently, only IMA signatures satisfy this requirement.

Appended signatures may also satisfy this requirement, but are not
applicable as ...

> IMA signatures
> ensure data source authentication for file content and prevent any change.
> EVM signatures instead ensure data source authentication for file metadata.
> Given that the digest or signature of the file content must be included in
> the metadata, EVM signatures provide at least the same guarantees of IMA
> signatures.

^provide the same file data guarantees of IMA signatures, as well as
providing file metadata guarantees.

> 
> This patch lets systems protected with EVM signatures pass appraisal
> verification if the appraise_type=imasig requirement is specified in the
> policy. This facilitates deployment in the scenarios where only EVM
> signatures are available.
> 
> The patch makes the following changes:
> 
> file xattr types:
> security.ima: IMA_XATTR_DIGEST/IMA_XATTR_DIGEST_NG
> security.evm: EVM_XATTR_PORTABLE_DIGSIG
> 
> execve(), mmap(), open() behavior (with appraise_type=imasig):
> before: denied (file without IMA signature, imasig requirement not met)
> after: allowed (file with EVM portable signature, imasig requirement met)
> 
> open(O_WRONLY) behavior (without appraise_type=imasig):
> before: allowed (file without IMA signature, not immutable)
> after: denied (file with EVM portable signature, immutable)
> 
> In addition, similarly to IMA signatures, this patch temporarily allows
> new files without or with incomplete metadata to be opened so that content
> can be written.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

After addressing the comments above and below,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

> ---
>  security/integrity/ima/ima_appraise.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 21bda264fc30..9505bb390d90 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -219,12 +219,16 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
>  		hash_start = 1;
>  		/* fall through */
>  	case IMA_XATTR_DIGEST:
> -		if (iint->flags & IMA_DIGSIG_REQUIRED) {
> -			*cause = "IMA-signature-required";
> -			*status = INTEGRITY_FAIL;
> -			break;
> +		if (*status != INTEGRITY_PASS_IMMUTABLE) {
> +			if (iint->flags & IMA_DIGSIG_REQUIRED) {
> +				*cause = "IMA-signature-required";
> +				*status = INTEGRITY_FAIL;
> +				break;
> +			}
> +			clear_bit(IMA_DIGSIG, &iint->atomic_flags);
> +		} else {
> +			set_bit(IMA_DIGSIG, &iint->atomic_flags);
>  		}
> -		clear_bit(IMA_DIGSIG, &iint->atomic_flags);
>  		if (xattr_len - sizeof(xattr_value->type) - hash_start >=
>  				iint->ima_hash->length)
>  			/*
> @@ -394,6 +398,8 @@ int ima_appraise_measurement(enum ima_hooks func,
>  		cause = "missing-HMAC";
>  		goto out;
>  	case INTEGRITY_FAIL_IMMUTABLE:
> +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> +		fallthrough;
>  	case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
>  		cause = "invalid-HMAC";
>  		goto out;
> @@ -437,9 +443,9 @@ int ima_appraise_measurement(enum ima_hooks func,
>  				status = INTEGRITY_PASS;
>  		}
>  
> -		/* Permit new files with file signatures, but without data. */
> +		/* Permit new files marked as immutable, but without data. */

This comment isn't quite right.

>  		if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
> -		    xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
> +		    test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
>  			status = INTEGRITY_PASS;
>  		}
>
diff mbox series

Patch

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 21bda264fc30..9505bb390d90 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -219,12 +219,16 @@  static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 		hash_start = 1;
 		/* fall through */
 	case IMA_XATTR_DIGEST:
-		if (iint->flags & IMA_DIGSIG_REQUIRED) {
-			*cause = "IMA-signature-required";
-			*status = INTEGRITY_FAIL;
-			break;
+		if (*status != INTEGRITY_PASS_IMMUTABLE) {
+			if (iint->flags & IMA_DIGSIG_REQUIRED) {
+				*cause = "IMA-signature-required";
+				*status = INTEGRITY_FAIL;
+				break;
+			}
+			clear_bit(IMA_DIGSIG, &iint->atomic_flags);
+		} else {
+			set_bit(IMA_DIGSIG, &iint->atomic_flags);
 		}
-		clear_bit(IMA_DIGSIG, &iint->atomic_flags);
 		if (xattr_len - sizeof(xattr_value->type) - hash_start >=
 				iint->ima_hash->length)
 			/*
@@ -394,6 +398,8 @@  int ima_appraise_measurement(enum ima_hooks func,
 		cause = "missing-HMAC";
 		goto out;
 	case INTEGRITY_FAIL_IMMUTABLE:
+		set_bit(IMA_DIGSIG, &iint->atomic_flags);
+		fallthrough;
 	case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
 		cause = "invalid-HMAC";
 		goto out;
@@ -437,9 +443,9 @@  int ima_appraise_measurement(enum ima_hooks func,
 				status = INTEGRITY_PASS;
 		}
 
-		/* Permit new files with file signatures, but without data. */
+		/* Permit new files marked as immutable, but without data. */
 		if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
-		    xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
+		    test_bit(IMA_DIGSIG, &iint->atomic_flags)) {
 			status = INTEGRITY_PASS;
 		}