| Message ID | 20211129170057.243127-3-zohar@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ima: support fs-verity signatures stored as | expand |
On Mon, Nov 29, 2021 at 12:00:55PM -0500, Mimi Zohar wrote: > 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. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> For this new signature type, what bytes are actually signed? It looks like it's just the raw digest, which isn't sufficient since it is ambiguous. It needs to include information that makes it clear what the signer is actually signing, such as "this is an fs-verity SHA-256 file digest". See 'struct fsverity_formatted_digest' for an example of this (but it isn't necessary to use that exact structure). I think the existing IMA signatures have the same problem (but it is hard for me to understand the code). However, a new signature type doesn't have backwards-compatibility concerns, so it could be done right. - Eric
On Mon, 2021-11-29 at 18:33 -0800, Eric Biggers wrote: > On Mon, Nov 29, 2021 at 12:00:55PM -0500, Mimi Zohar wrote: > > 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. > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > For this new signature type, what bytes are actually signed? It looks like it's > just the raw digest, which isn't sufficient since it is ambiguous. It needs to > include information that makes it clear what the signer is actually signing, > such as "this is an fs-verity SHA-256 file digest". See > 'struct fsverity_formatted_digest' for an example of this (but it isn't > necessary to use that exact structure). > > I think the existing IMA signatures have the same problem (but it is hard for me > to understand the code). However, a new signature type doesn't have > backwards-compatibility concerns, so it could be done right. As this change should probably be applicable to all signature types, the signature version in the signature_v2_hdr should be bumped. The existing signature version could co-exist with the new signature version. thanks, Mimi
Hi Eric, On Tue, 2021-11-30 at 13:14 -0500, Mimi Zohar wrote: > On Mon, 2021-11-29 at 18:33 -0800, Eric Biggers wrote: > > On Mon, Nov 29, 2021 at 12:00:55PM -0500, Mimi Zohar wrote: > > > 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. > > > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > > > For this new signature type, what bytes are actually signed? It looks like it's > > just the raw digest, which isn't sufficient since it is ambiguous. It needs to > > include information that makes it clear what the signer is actually signing, > > such as "this is an fs-verity SHA-256 file digest". See > > 'struct fsverity_formatted_digest' for an example of this (but it isn't > > necessary to use that exact structure). > > > > I think the existing IMA signatures have the same problem (but it is hard for me > > to understand the code). However, a new signature type doesn't have > > backwards-compatibility concerns, so it could be done right. > > As this change should probably be applicable to all signature types, > the signature version in the signature_v2_hdr should be bumped. The > existing signature version could co-exist with the new signature > version. By signing the file hash, the sig field in the IMA measurement list can be directly verified against the digest field. For appended signatures, we defined a new template named ima-modsig which contains two file hashes, with and without the appended signature. Similarly, by signing a digest containing other metadata and fs- verity's file digest, the measurement list should include both digests. Otherwise the consumer of the measurement list would first need to calculate the signed digest before verifying the signature. Options: - Include just fs-verity's file digest and the signature in the measurement list. Leave it to the consumer of the measurement list to deal with. - Define a new template format to include both digests, add a new field in the iint for the signed digest. (Much more work.) - As originally posted, directly sign fs-verity's file digest. thanks, Mimi
On Thu, Dec 02, 2021 at 11:25:05AM -0500, Mimi Zohar wrote: > Hi Eric, > > On Tue, 2021-11-30 at 13:14 -0500, Mimi Zohar wrote: > > On Mon, 2021-11-29 at 18:33 -0800, Eric Biggers wrote: > > > On Mon, Nov 29, 2021 at 12:00:55PM -0500, Mimi Zohar wrote: > > > > 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. > > > > > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > > > > > For this new signature type, what bytes are actually signed? It looks like it's > > > just the raw digest, which isn't sufficient since it is ambiguous. It needs to > > > include information that makes it clear what the signer is actually signing, > > > such as "this is an fs-verity SHA-256 file digest". See > > > 'struct fsverity_formatted_digest' for an example of this (but it isn't > > > necessary to use that exact structure). > > > > > > I think the existing IMA signatures have the same problem (but it is hard for me > > > to understand the code). However, a new signature type doesn't have > > > backwards-compatibility concerns, so it could be done right. > > > > As this change should probably be applicable to all signature types, > > the signature version in the signature_v2_hdr should be bumped. The > > existing signature version could co-exist with the new signature > > version. > > By signing the file hash, the sig field in the IMA measurement list can > be directly verified against the digest field. For appended > signatures, we defined a new template named ima-modsig which contains > two file hashes, with and without the appended signature. > > Similarly, by signing a digest containing other metadata and fs- > verity's file digest, the measurement list should include both digests. > Otherwise the consumer of the measurement list would first need to > calculate the signed digest before verifying the signature. > > Options: > - Include just fs-verity's file digest and the signature in the > measurement list. Leave it to the consumer of the measurement list to > deal with. > - Define a new template format to include both digests, add a new field > in the iint for the signed digest. (Much more work.) > - As originally posted, directly sign fs-verity's file digest. I don't really have enough knowledge about IMA and how it is used to decide on one approach or the other. Note that earlier I mentioned that it would be possible to have an fs-verity setting that makes a full file digest be included in the fsverity_descriptor, so it gets covered by the fs-verity file digest and is also retrievable in constant time like the fs-verity file digest is. If you'd like to solve this problem at the IMA layer instead, by storing the full file digest in an xattr and signing both the full file digest and fs-verity file digest together, that would achieve the same goal of making the full file digest available, and wouldn't require any changes to fs-verity. This would assume that the file would be signed, though. What about audit-only mode without signatures; is that something you care about? Alternatively, maybe this problem doesn't need to be solved at all and IMA would be fine with the fs-verity file digest only, and not need the full file hash too. I don't know the answer to that; I think it's up to you to decide. - Eric
On Thu, 2021-12-02 at 13:17 -0800, Eric Biggers wrote: > On Thu, Dec 02, 2021 at 11:25:05AM -0500, Mimi Zohar wrote: > > Hi Eric, > > > > On Tue, 2021-11-30 at 13:14 -0500, Mimi Zohar wrote: > > > On Mon, 2021-11-29 at 18:33 -0800, Eric Biggers wrote: > > > > On Mon, Nov 29, 2021 at 12:00:55PM -0500, Mimi Zohar wrote: > > > > > 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. > > > > > > > > > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > > > > > > > > For this new signature type, what bytes are actually signed? It looks like it's > > > > just the raw digest, which isn't sufficient since it is ambiguous. It needs to > > > > include information that makes it clear what the signer is actually signing, > > > > such as "this is an fs-verity SHA-256 file digest". See > > > > 'struct fsverity_formatted_digest' for an example of this (but it isn't > > > > necessary to use that exact structure). > > > > > > > > I think the existing IMA signatures have the same problem (but it is hard for me > > > > to understand the code). However, a new signature type doesn't have > > > > backwards-compatibility concerns, so it could be done right. > > > > > > As this change should probably be applicable to all signature types, > > > the signature version in the signature_v2_hdr should be bumped. The > > > existing signature version could co-exist with the new signature > > > version. > > > > By signing the file hash, the sig field in the IMA measurement list can > > be directly verified against the digest field. For appended > > signatures, we defined a new template named ima-modsig which contains > > two file hashes, with and without the appended signature. > > > > Similarly, by signing a digest containing other metadata and fs- > > verity's file digest, the measurement list should include both digests. > > Otherwise the consumer of the measurement list would first need to > > calculate the signed digest before verifying the signature. > > > > Options: > > - Include just fs-verity's file digest and the signature in the > > measurement list. Leave it to the consumer of the measurement list to > > deal with. > > - Define a new template format to include both digests, add a new field > > in the iint for the signed digest. (Much more work.) > > - As originally posted, directly sign fs-verity's file digest. > > I don't really have enough knowledge about IMA and how it is used to decide on > one approach or the other. Note that earlier I mentioned that it would be > possible to have an fs-verity setting that makes a full file digest be included > in the fsverity_descriptor, so it gets covered by the fs-verity file digest and > is also retrievable in constant time like the fs-verity file digest is. > > If you'd like to solve this problem at the IMA layer instead, by storing the > full file digest in an xattr and signing both the full file digest and fs-verity > file digest together, that would achieve the same goal of making the full file > digest available, and wouldn't require any changes to fs-verity. This would > assume that the file would be signed, though. What about audit-only mode > without signatures; is that something you care about? > > Alternatively, maybe this problem doesn't need to be solved at all and IMA would > be fine with the fs-verity file digest only, and not need the full file hash > too. I don't know the answer to that; I think it's up to you to decide. I just posted v1 which implements option 1, including the fsverity file digest in the measurement list. Both option 2 or including the actual file hash, will require a new template format with two digests. Mimi
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index dbba51583e7c..d43a27a9a9b6 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,6 +185,8 @@ 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: + fallthrough; case EVM_IMA_XATTR_DIGSIG: sig = (typeof(sig))xattr_value; if (sig->version != 2 || xattr_len <= sizeof(*sig) @@ -272,6 +276,8 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, } *status = INTEGRITY_PASS; break; + case IMA_VERITY_DIGSIG: + fallthrough; case EVM_IMA_XATTR_DIGSIG: set_bit(IMA_DIGSIG, &iint->atomic_flags); rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 547425c20e11..94f9ba55e840 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -77,6 +77,7 @@ enum evm_ima_xattr_type { EVM_IMA_XATTR_DIGSIG, IMA_XATTR_DIGEST_NG, EVM_XATTR_PORTABLE_DIGSIG, + IMA_VERITY_DIGSIG, IMA_XATTR_LAST };
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. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- security/integrity/ima/ima_appraise.c | 6 ++++++ security/integrity/integrity.h | 1 + 2 files changed, 7 insertions(+)