| Message ID | 20220126000658.138345-8-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ima: support fs-verity digests and signatures | expand |
On Tue, Jan 25, 2022 at 07:06:57PM -0500, Mimi Zohar wrote: > Instead of calculating a file hash and verifying the signature stored > in the security.ima xattr against the calculated file hash, verify > fs-verity's signature (version 3). > > To differentiate between a regular file hash and an fs-verity file digest > based signature stored as security.ima xattr, define a new signature type > named IMA_VERITY_DIGSIG. > > Update the 'ima-sig' template field to display the new fs-verity signature > type as well. > > For example: > appraise func=BPRM_CHECK digest_type=hash|verity > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > Documentation/ABI/testing/ima_policy | 10 +++++ > Documentation/security/IMA-templates.rst | 4 +- > security/integrity/ima/ima_appraise.c | 49 ++++++++++++++++++++++- > security/integrity/ima/ima_template_lib.c | 3 +- > security/integrity/integrity.h | 5 ++- > 5 files changed, 65 insertions(+), 6 deletions(-) All this IMA-specific stuff is confusing to me, so let me ask a question about what the end result actually is. Let's say I want to use IMA to authenticate ("appraise") a file. I've signed its fs-verity digest with a key. I put only that one key in the IMA keyring, and that key was only ever used to sign that one fs-verity digest. Can an attacker (who controls the file's contents and IMA xattr) replace the file with one with a different contents and still pass the IMA check? For example, could they replace the file's contents with the ima_file_id of the authentic file, and then downgrade the signature version to v2? If they can do that, then the goal of authentication wasn't met. It might be necessary to enforce that only one signature version is used at a time, to avoid this kind of ambiguity. - Eric
On Mon, 2022-01-31 at 17:06 -0800, Eric Biggers wrote: > On Tue, Jan 25, 2022 at 07:06:57PM -0500, Mimi Zohar wrote: > > Instead of calculating a file hash and verifying the signature stored > > in the security.ima xattr against the calculated file hash, verify > > fs-verity's signature (version 3). > > > > To differentiate between a regular file hash and an fs-verity file digest > > based signature stored as security.ima xattr, define a new signature type > > named IMA_VERITY_DIGSIG. > > > > Update the 'ima-sig' template field to display the new fs-verity signature > > type as well. > > > > For example: > > appraise func=BPRM_CHECK digest_type=hash|verity > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > --- > > Documentation/ABI/testing/ima_policy | 10 +++++ > > Documentation/security/IMA-templates.rst | 4 +- > > security/integrity/ima/ima_appraise.c | 49 ++++++++++++++++++++++- > > security/integrity/ima/ima_template_lib.c | 3 +- > > security/integrity/integrity.h | 5 ++- > > 5 files changed, 65 insertions(+), 6 deletions(-) > > All this IMA-specific stuff is confusing to me, so let me ask a question about > what the end result actually is. Let's say I want to use IMA to authenticate > ("appraise") a file. I've signed its fs-verity digest with a key. I put only > that one key in the IMA keyring, and that key was only ever used to sign that > one fs-verity digest. Can an attacker (who controls the file's contents and IMA > xattr) replace the file with one with a different contents and still pass the > IMA check? For example, could they replace the file's contents with the > ima_file_id of the authentic file, and then downgrade the signature version to > v2? If they can do that, then the goal of authentication wasn't met. It might > be necessary to enforce that only one signature version is used at a time, to > avoid this kind of ambiguity. Instead of only allowing a single signature version, the signature verification could be based on policy rules. "ima: include fsverity's file digests in the IMA measurement list" defines the new policy rule 'digest_type=' option, which currently permits either IMA or fsverity signatures to match. Instead only allow IMA or fsverity signatures, not both, on a per policy rule basis. From an IMA perspective, this would be safe since the builtin policies do not support fs-verity signatures. After loading a custom policy, additional rules can only extend the custom policy. thanks, Mimi
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 444bb7ccbe03..fadf90dde289 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -151,6 +151,16 @@ Description: appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512 + Example of measure and appraise rules allowing fs-verity + signed digests on a particular filesystem identified by + it's fsuuid: + + measure func=BPRM_CHECK digest_type=hash|verity \ + fsuuid=... template=ima-sig + appraise func=BPRM_CHECK digest_type=hash|verity \ + fsuuid=... + + Example of measure rule allowing fs-verity's digests on a particular filesystem with indication of type of digest. diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst index 5e31513e8ec4..390936810ebc 100644 --- a/Documentation/security/IMA-templates.rst +++ b/Documentation/security/IMA-templates.rst @@ -71,8 +71,8 @@ descriptors by adding their identifier to the format string - 'd-modsig': the digest of the event without the appended modsig; - 'd-type': the type of file digest (e.g. hash, verity[1]); - 'n-ng': the name of the event, without size limitations; - - 'sig': the file signature, or the EVM portable signature if the file - signature is not found; + - 'sig': the file signature, based on either the file's/fsverity's digest[1], + or the EVM portable signature if the file signature is not found; - 'modsig' the appended file signature; - 'buf': the buffer data that was used to generate the hash without size limitations; - 'evmsig': the EVM portable signature; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 7bc180bd808e..68376c56feff 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -13,7 +13,9 @@ #include <linux/magic.h> #include <linux/ima.h> #include <linux/evm.h> +#include <linux/fsverity.h> #include <keys/system_keyring.h> +#include <uapi/linux/fsverity.h> #include "ima.h" @@ -183,13 +185,18 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value, return ima_hash_algo; switch (xattr_value->type) { + case IMA_VERITY_DIGSIG: + sig = (typeof(sig))xattr_value; + if (sig->version != 3 || xattr_len <= sizeof(*sig) || + sig->hash_algo >= HASH_ALGO__LAST) + return ima_hash_algo; + return sig->hash_algo; case EVM_IMA_XATTR_DIGSIG: sig = (typeof(sig))xattr_value; if (sig->version != 2 || xattr_len <= sizeof(*sig) || sig->hash_algo >= HASH_ALGO__LAST) return ima_hash_algo; return sig->hash_algo; - break; case IMA_XATTR_DIGEST_NG: /* first byte contains algorithm id */ ret = xattr_value->data[0]; @@ -235,15 +242,22 @@ int ima_read_xattr(struct dentry *dentry, * IMA signature version 3 disambiguates the data that is signed by * indirectly signing the hash of the ima_file_id structure data. * + * Signing the ima_file_id struct is currently only supported for + * IMA_VERITY_DIGSIG type xattrs. + * * Return 0 on success, error code otherwise. */ static int calc_file_id_hash(enum evm_ima_xattr_type type, enum hash_algo algo, const u8 *digest, struct ima_max_digest_data *hash) { - struct ima_file_id file_id = {.hash_algorithm = algo}; + struct ima_file_id file_id = { + .hash_type = IMA_VERITY_DIGSIG, .hash_algorithm = algo}; uint unused = HASH_MAX_DIGESTSIZE - hash_digest_size[algo]; + if (type != IMA_VERITY_DIGSIG) + return -EINVAL; + memcpy(file_id.hash, digest, hash_digest_size[algo]); hash->algo = algo; @@ -264,6 +278,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, struct evm_ima_xattr_data *xattr_value, int xattr_len, enum integrity_status *status, const char **cause) { + struct ima_max_digest_data hash; struct signature_v2_hdr *sig; int rc = -EINVAL, hash_start = 0; @@ -332,6 +347,36 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, } else { *status = INTEGRITY_PASS; } + break; + case IMA_VERITY_DIGSIG: + set_bit(IMA_DIGSIG, &iint->atomic_flags); + + sig = (typeof(sig))xattr_value; + if (sig->version != 3) { + *cause = "invalid-verity-version"; + *status = INTEGRITY_FAIL; + break; + } + + rc = calc_file_id_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo, + iint->ima_hash->digest, &hash); + if (rc) { + *cause = "verity-hashing-error"; + *status = INTEGRITY_FAIL; + break; + } + + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, + (const char *)xattr_value, + xattr_len, hash.digest, + hash.length); + if (rc) { + *cause = "invalid-verity-signature"; + *status = INTEGRITY_FAIL; + } else { + *status = INTEGRITY_PASS; + } + break; default: *status = INTEGRITY_UNKNOWN; diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 44e57d7e5fed..0d4bbb4da59a 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -499,7 +499,8 @@ int ima_eventsig_init(struct ima_event_data *event_data, { struct evm_ima_xattr_data *xattr_value = event_data->xattr_value; - if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG)) + if (!xattr_value || + !(xattr_value->type & (EVM_IMA_XATTR_DIGSIG | IMA_VERITY_DIGSIG))) return ima_eventevmsig_init(event_data, field_data); return ima_write_template_field_data(xattr_value, event_data->xattr_len, diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index ed4966d943e9..5b1aa8b7d61c 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -80,6 +80,7 @@ enum evm_ima_xattr_type { EVM_IMA_XATTR_DIGSIG, IMA_XATTR_DIGEST_NG, EVM_XATTR_PORTABLE_DIGSIG, + IMA_VERITY_DIGSIG, IMA_XATTR_LAST }; @@ -154,7 +155,9 @@ struct signature_v2_hdr { /* * IMA signature version 3 disambiguates the data that is signed, by - * indirectly signing the hash of the ima_file_id structure data. + * indirectly signing the hash of the ima_file_id structure data, + * containing either the fsverity_descriptor struct digest or, in the + * future, the regular IMA file hash. * * (The hash of the ima_file_id structure is only of the portion used.) */
Instead of calculating a file hash and verifying the signature stored in the security.ima xattr against the calculated file hash, verify fs-verity's signature (version 3). To differentiate between a regular file hash and an fs-verity file digest based signature stored as security.ima xattr, define a new signature type named IMA_VERITY_DIGSIG. Update the 'ima-sig' template field to display the new fs-verity signature type as well. For example: appraise func=BPRM_CHECK digest_type=hash|verity Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Documentation/ABI/testing/ima_policy | 10 +++++ Documentation/security/IMA-templates.rst | 4 +- security/integrity/ima/ima_appraise.c | 49 ++++++++++++++++++++++- security/integrity/ima/ima_template_lib.c | 3 +- security/integrity/integrity.h | 5 ++- 5 files changed, 65 insertions(+), 6 deletions(-)