Re: [PATCH] ima: instantiate the bprm_creds_for_exec() hook

[Stefan Berger <stefanb@xxxxxxxxxxxxx>]

On 11/27/24 4:02 PM, Mimi Zohar wrote:
> Like direct file execution (e.g. ./script.sh), indirect file execution
> (e.g. sh script.sh) needs to be measured and appraised.  Instantiate
> the new security_bprm_creds_for_exec() hook to measure and verify the
> indirect file's integrity.  Unlike direct file execution, indirect file
> execution integrity is optionally enforced by the interpreter.
>
> Update the audit messages to differentiate between kernel and userspace
> enforced integrity.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> ---
>   security/integrity/ima/ima_appraise.c | 84 ++++++++++++++++++++-------
>   security/integrity/ima/ima_main.c     | 22 +++++++
>   2 files changed, 86 insertions(+), 20 deletions(-)
>
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 656c709b974f..b5f8e49cde9d 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -8,6 +8,7 @@
>   #include <linux/module.h>
>   #include <linux/init.h>
>   #include <linux/file.h>
> +#include <linux/binfmts.h>
>   #include <linux/fs.h>
>   #include <linux/xattr.h>
>   #include <linux/magic.h>
> @@ -16,6 +17,7 @@
>   #include <linux/fsverity.h>
>   #include <keys/system_keyring.h>
>   #include <uapi/linux/fsverity.h>
> +#include <linux/securebits.h>
>   
>   #include "ima.h"
>   
> @@ -276,7 +278,8 @@ static int calc_file_id_hash(enum evm_ima_xattr_type type,
>    */
>   static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   			struct evm_ima_xattr_data *xattr_value, int xattr_len,
> -			enum integrity_status *status, const char **cause)
> +			enum integrity_status *status, const char **cause,
> +			bool is_check)
>   {
>   	struct ima_max_digest_data hash;
>   	struct signature_v2_hdr *sig;
> @@ -292,9 +295,11 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   		if (*status != INTEGRITY_PASS_IMMUTABLE) {
>   			if (iint->flags & IMA_DIGSIG_REQUIRED) {
>   				if (iint->flags & IMA_VERITY_REQUIRED)
> -					*cause = "verity-signature-required";
> +					*cause = !is_check ? "verity-signature-required" :
> +						"verity-signature-required(userspace)";
>   				else
> -					*cause = "IMA-signature-required";
> +					*cause = !is_check ? "IMA-signature-required" :
> +						"IMA-signature-required(userspace)";
>   				*status = INTEGRITY_FAIL;
>   				break;
>   			}
> @@ -314,7 +319,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   		else
>   			rc = -EINVAL;
>   		if (rc) {
> -			*cause = "invalid-hash";
> +			*cause = !is_check ? "invalid-hash" :
> +				"invalid-hash(userspace)";
>   			*status = INTEGRITY_FAIL;
>   			break;
>   		}
> @@ -325,14 +331,16 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   
>   		mask = IMA_DIGSIG_REQUIRED | IMA_VERITY_REQUIRED;
>   		if ((iint->flags & mask) == mask) {
> -			*cause = "verity-signature-required";
> +			*cause = !is_check ? "verity-signature-required" :
> +				"verity-signature-required(userspace)";
>   			*status = INTEGRITY_FAIL;
>   			break;
>   		}
>   
>   		sig = (typeof(sig))xattr_value;
>   		if (sig->version >= 3) {
> -			*cause = "invalid-signature-version";
> +			*cause = !is_check ? "invalid-signature-version" :
> +				"invalid-signature-version(userspace)";
>   			*status = INTEGRITY_FAIL;
>   			break;
>   		}
> @@ -353,7 +361,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   						     iint->ima_hash->digest,
>   						     iint->ima_hash->length);
>   		if (rc) {
> -			*cause = "invalid-signature";
> +			*cause = !is_check ? "invalid-signature" :
> +				"invalid-signature(userspace)";
>   			*status = INTEGRITY_FAIL;
>   		} else {
>   			*status = INTEGRITY_PASS;
> @@ -364,7 +373,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   
>   		if (iint->flags & IMA_DIGSIG_REQUIRED) {
>   			if (!(iint->flags & IMA_VERITY_REQUIRED)) {
> -				*cause = "IMA-signature-required";
> +				*cause = !is_check ? "IMA-signature-required" :
> +					"IMA-signature-required(userspace)";
>   				*status = INTEGRITY_FAIL;
>   				break;
>   			}
> @@ -372,7 +382,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   
>   		sig = (typeof(sig))xattr_value;
>   		if (sig->version != 3) {
> -			*cause = "invalid-signature-version";
> +			*cause = !is_check ? "invalid-signature-version" :
> +				"invalid-signature-version(userspace)";
>   			*status = INTEGRITY_FAIL;
>   			break;
>   		}
> @@ -382,7 +393,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   				       container_of(&hash.hdr,
>   					       struct ima_digest_data, hdr));
>   		if (rc) {
> -			*cause = "sigv3-hashing-error";
> +			*cause = !is_check ? "sigv3-hashing-error" :
> +				"sigv3-hashing-error(userspace)";
>   			*status = INTEGRITY_FAIL;
>   			break;
>   		}
> @@ -392,7 +404,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   					     xattr_len, hash.digest,
>   					     hash.hdr.length);
>   		if (rc) {
> -			*cause = "invalid-verity-signature";
> +			*cause = !is_check ? "invalid-verity-signature" :
> +				"invalid-verify-signature(userspace)";
>   			*status = INTEGRITY_FAIL;
>   		} else {
>   			*status = INTEGRITY_PASS;
> @@ -401,7 +414,8 @@ static int xattr_verify(enum ima_hooks func, struct ima_iint_cache *iint,
>   		break;
>   	default:
>   		*status = INTEGRITY_UNKNOWN;
> -		*cause = "unknown-ima-data";
> +		*cause = !is_check ? "unknown-ima-data" :
> +			"unknown-ima-data(userspace)";
>   		break;
>   	}
>   
> @@ -469,6 +483,18 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
>   	return rc;
>   }
>   
> +static int is_bprm_creds_for_exec(enum ima_hooks func, struct file *file)

is_check is bool, so this should probably also return bool

> +{
> +	struct linux_binprm *bprm = NULL;
> +
> +	if (func == BPRM_CHECK) {
> +		bprm = container_of(&file, struct linux_binprm, file);
> +		if (bprm->is_check)
> +			return 1;
> +	}
> +	return 0;
> +}
> +
>   /*
>    * ima_appraise_measurement - appraise file measurement
>    *
> @@ -489,11 +515,24 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
>   	enum integrity_status status = INTEGRITY_UNKNOWN;
>   	int rc = xattr_len;
>   	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
> +	bool is_check = false;

no need to initialize it