* [PATCH v1 0/5] ima: support fs-verity signatures stored as
@ 2021-12-02 21:55 Mimi Zohar
2021-12-02 21:55 ` [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw)
To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel
Support for fs-verity file digests in IMA was discussed from the beginning,
prior to fs-verity being upstreamed[1,2]. This patch set adds signature
verification support based on the fs-verity file digest. Both the
file digest and the signature must be included in the IMA measurement list
in order to disambiguate the type of file digest.
[1] https://events19.linuxfoundation.org/wp-content/uploads/2017/11/fs-verify_Mike-Halcrow_Eric-Biggers.pdf
[2] Documentation/filesystems/fsverity.rst
Changelog v1:
- Updated both fsverity and IMA documentation.
- Addressed both Eric Bigger's and Lakshmi's comments.
Mimi Zohar (5):
fs-verity: define a function to return the integrity protected file
digest
ima: define a new signature type named IMA_VERITY_DIGSIG
ima: limit including fs-verity's file digest in measurement list
ima: support fs-verity file digest based signatures
fsverity: update the documentation
Documentation/filesystems/fsverity.rst | 22 ++++++----
Documentation/security/IMA-templates.rst | 9 +++-
fs/verity/Kconfig | 1 +
fs/verity/fsverity_private.h | 7 ---
fs/verity/measure.c | 49 +++++++++++++++++++++
include/linux/fsverity.h | 18 ++++++++
security/integrity/ima/ima.h | 3 +-
security/integrity/ima/ima_api.c | 23 +++++++++-
security/integrity/ima/ima_appraise.c | 52 ++++++++++++++++++++++-
security/integrity/ima/ima_main.c | 7 ++-
security/integrity/ima/ima_template_lib.c | 3 +-
security/integrity/integrity.h | 1 +
12 files changed, 172 insertions(+), 23 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar @ 2021-12-02 21:55 ` Mimi Zohar 2021-12-02 22:15 ` Eric Biggers 2021-12-02 21:55 ` [PATCH v1 2/5] ima: define a new signature type named IMA_VERITY_DIGSIG Mimi Zohar ` (3 subsequent siblings) 4 siblings, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw) To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel Define a function named fsverity_collect_digest() to return the verity file digest and the associated hash algorithm (enum hash_algo). Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Changelog v1: - Renamed new function to fsverity_collect_digest(), based on discussion with Eric Biggers and Lakshmi. - Addressed Eric's suggestions: updated kernel doc variable and function description, removed unnecessary include file. fs/verity/Kconfig | 1 + fs/verity/fsverity_private.h | 7 ------ fs/verity/measure.c | 49 ++++++++++++++++++++++++++++++++++++ include/linux/fsverity.h | 18 +++++++++++++ 4 files changed, 68 insertions(+), 7 deletions(-) diff --git a/fs/verity/Kconfig b/fs/verity/Kconfig index 24d1b54de807..54598cd80145 100644 --- a/fs/verity/Kconfig +++ b/fs/verity/Kconfig @@ -3,6 +3,7 @@ config FS_VERITY bool "FS Verity (read-only file-based authenticity protection)" select CRYPTO + select CRYPTO_HASH_INFO # SHA-256 is implied as it's intended to be the default hash algorithm. # To avoid bloat, other wanted algorithms must be selected explicitly. # Note that CRYPTO_SHA256 denotes the generic C implementation, but diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h index a7920434bae5..c6fb62e0ef1a 100644 --- a/fs/verity/fsverity_private.h +++ b/fs/verity/fsverity_private.h @@ -14,7 +14,6 @@ #define pr_fmt(fmt) "fs-verity: " fmt -#include <crypto/sha2.h> #include <linux/fsverity.h> #include <linux/mempool.h> @@ -26,12 +25,6 @@ struct ahash_request; */ #define FS_VERITY_MAX_LEVELS 8 -/* - * Largest digest size among all hash algorithms supported by fs-verity. - * Currently assumed to be <= size of fsverity_descriptor::root_hash. - */ -#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE - /* A hash algorithm supported by fs-verity */ struct fsverity_hash_alg { struct crypto_ahash *tfm; /* hash tfm, allocated on demand */ diff --git a/fs/verity/measure.c b/fs/verity/measure.c index f0d7b30c62db..64fbfbd408d4 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c @@ -57,3 +57,52 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg) return 0; } EXPORT_SYMBOL_GPL(fsverity_ioctl_measure); + +/** + * fsverity_collect_digest() - get a verity file's digest + * @inode: inode to get digest of + * @digest: (out) pointer to the digest + * @alg: (out) pointer to the hash algorithm enumeration + * + * Return the file hash algorithm and digest of an fsverity protected file. + * + * Return: 0 on success, -errno on failure + */ +int fsverity_collect_digest(struct inode *inode, + u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg) +{ + const struct fsverity_info *vi; + const struct fsverity_hash_alg *hash_alg; + int i; + + vi = fsverity_get_info(inode); + if (!vi) + return -ENODATA; /* not a verity file */ + + hash_alg = vi->tree_params.hash_alg; + memset(digest, 0, FS_VERITY_MAX_DIGEST_SIZE); + *alg = HASH_ALGO__LAST; + + /* convert hash algorithm to hash_algo_name */ + for (i = 0; i < HASH_ALGO__LAST; i++) { + pr_debug("name %s hash_algo_name[%d] %s\n", + hash_alg->name, i, hash_algo_name[i]); + + if (!strcmp(hash_alg->name, hash_algo_name[i])) { + *alg = i; + break; + } + } + + /* Shouldn't happen */ + if (*alg == HASH_ALGO__LAST) + return -EINVAL; + + memcpy(digest, vi->file_digest, hash_alg->digest_size); + + pr_debug("file digest:%s %*phN\n", hash_algo_name[*alg], + hash_digest_size[*alg], digest); + + return 0; +} diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h index b568b3c7d095..2755e8bd80e5 100644 --- a/include/linux/fsverity.h +++ b/include/linux/fsverity.h @@ -12,8 +12,16 @@ #define _LINUX_FSVERITY_H #include <linux/fs.h> +#include <crypto/hash_info.h> +#include <crypto/sha2.h> #include <uapi/linux/fsverity.h> +/* + * Largest digest size among all hash algorithms supported by fs-verity. + * Currently assumed to be <= size of fsverity_descriptor::root_hash. + */ +#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE + /* Verity operations for filesystems */ struct fsverity_operations { @@ -131,6 +139,9 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *arg); /* measure.c */ int fsverity_ioctl_measure(struct file *filp, void __user *arg); +int fsverity_collect_digest(struct inode *inode, + u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg); /* open.c */ @@ -170,6 +181,13 @@ static inline int fsverity_ioctl_measure(struct file *filp, void __user *arg) return -EOPNOTSUPP; } +static inline int fsverity_collect_digest(struct inode *inode, + u8 digest[FS_VERITY_MAX_DIGEST_SIZE], + enum hash_algo *alg) +{ + return -EOPNOTSUPP; +} + /* open.c */ static inline int fsverity_file_open(struct inode *inode, struct file *filp) -- 2.27.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest 2021-12-02 21:55 ` [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar @ 2021-12-02 22:15 ` Eric Biggers 0 siblings, 0 replies; 27+ messages in thread From: Eric Biggers @ 2021-12-02 22:15 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, Dec 02, 2021 at 04:55:03PM -0500, Mimi Zohar wrote: > + > +/** > + * fsverity_collect_digest() - get a verity file's digest > + * @inode: inode to get digest of > + * @digest: (out) pointer to the digest > + * @alg: (out) pointer to the hash algorithm enumeration > + * > + * Return the file hash algorithm and digest of an fsverity protected file. > + * > + * Return: 0 on success, -errno on failure > + */ > +int fsverity_collect_digest(struct inode *inode, > + u8 digest[FS_VERITY_MAX_DIGEST_SIZE], > + enum hash_algo *alg) I'd still prefer that this be named fsverity_get_digest(), but this is fine too. > +{ > + const struct fsverity_info *vi; > + const struct fsverity_hash_alg *hash_alg; > + int i; > + > + vi = fsverity_get_info(inode); > + if (!vi) > + return -ENODATA; /* not a verity file */ > + > + hash_alg = vi->tree_params.hash_alg; > + memset(digest, 0, FS_VERITY_MAX_DIGEST_SIZE); > + *alg = HASH_ALGO__LAST; > + > + /* convert hash algorithm to hash_algo_name */ > + for (i = 0; i < HASH_ALGO__LAST; i++) { > + pr_debug("name %s hash_algo_name[%d] %s\n", > + hash_alg->name, i, hash_algo_name[i]); > + > + if (!strcmp(hash_alg->name, hash_algo_name[i])) { > + *alg = i; > + break; > + } > + } How about using match_string() here? > + pr_debug("file digest:%s %*phN\n", hash_algo_name[*alg], > + hash_digest_size[*alg], digest); Other log messages in fs/verity/ use the format alg:hash. How about using "file_digest %s:%*phN\n" as the format string here? - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v1 2/5] ima: define a new signature type named IMA_VERITY_DIGSIG 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar @ 2021-12-02 21:55 ` Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list Mimi Zohar ` (2 subsequent siblings) 4 siblings, 0 replies; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw) To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel 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(+) 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 }; -- 2.27.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 2/5] ima: define a new signature type named IMA_VERITY_DIGSIG Mimi Zohar @ 2021-12-02 21:55 ` Mimi Zohar 2021-12-02 22:22 ` Eric Biggers 2021-12-02 21:55 ` [PATCH v1 4/5] ima: support fs-verity file digest based signatures Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 5/5] fsverity: update the documentation Mimi Zohar 4 siblings, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw) To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel Without the file signature included in the IMA measurement list, the type of file digest is unclear. Set up the plumbing to limit including fs-verity's file digest in the IMA measurement list based on whether the template name is ima-sig. In the future, this could be relaxed to include any template format that includes the file signature. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Changelog v1: - Updated patch description to indicate this is a prepartory patch. - Addressed Eric's comment: use lowercase 'true'/'false'. - Fixed patch description based on Lakshmi's review. Documentation/security/IMA-templates.rst | 9 +++++++-- security/integrity/ima/ima.h | 3 ++- security/integrity/ima/ima_api.c | 3 ++- security/integrity/ima/ima_appraise.c | 3 ++- security/integrity/ima/ima_main.c | 7 ++++++- security/integrity/ima/ima_template_lib.c | 3 ++- 6 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst index 1a91d92950a7..28640b543340 100644 --- a/Documentation/security/IMA-templates.rst +++ b/Documentation/security/IMA-templates.rst @@ -70,8 +70,8 @@ descriptors by adding their identifier to the format string prefix is shown only if the hash algorithm is not SHA1 or MD5); - 'd-modsig': the digest of the event without the appended modsig; - '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; @@ -106,3 +106,8 @@ currently the following methods are supported: the ``ima_template=`` parameter; - register a new template descriptor with custom format through the kernel command line parameter ``ima_template_fmt=``. + + +References +========== +[1] Documentation/filesystems/fsverity.rst diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index be965a8715e4..ab257e404f8e 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -262,7 +262,8 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode, int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func); int ima_collect_measurement(struct integrity_iint_cache *iint, struct file *file, void *buf, loff_t size, - enum hash_algo algo, struct modsig *modsig); + enum hash_algo algo, struct modsig *modsig, + bool veritysig); void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index a64fb0130b01..7505563315cb 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -212,7 +212,8 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode, */ int ima_collect_measurement(struct integrity_iint_cache *iint, struct file *file, void *buf, loff_t size, - enum hash_algo algo, struct modsig *modsig) + enum hash_algo algo, struct modsig *modsig, + bool veritysig) { const char *audit_cause = "failed"; struct inode *inode = file_inode(file); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d43a27a9a9b6..549fe051269a 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -510,7 +510,8 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) !(iint->flags & IMA_HASH)) return; - rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo, NULL); + rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo, + NULL, false); if (rc < 0) return; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 465865412100..4b6b13becb16 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -216,6 +216,7 @@ static int process_measurement(struct file *file, const struct cred *cred, bool violation_check; enum hash_algo hash_algo; unsigned int allowed_algos = 0; + int veritysig = false; if (!ima_policy_flag || !S_ISREG(inode->i_mode)) return 0; @@ -333,8 +334,12 @@ static int process_measurement(struct file *file, const struct cred *cred, } hash_algo = ima_get_hash_algo(xattr_value, xattr_len); + if (xattr_value && xattr_value->type == IMA_VERITY_DIGSIG && + strcmp(template_desc->name, "ima-sig") == 0) + veritysig = true; - rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig); + rc = ima_collect_measurement(iint, file, buf, size, hash_algo, + modsig, veritysig); if (rc != 0 && rc != -EBADF && rc != -EINVAL) goto out_locked; diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index ca017cae73eb..5bad251f3b07 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -478,7 +478,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 || + xattr_value->type == IMA_VERITY_DIGSIG)) return ima_eventevmsig_init(event_data, field_data); return ima_write_template_field_data(xattr_value, event_data->xattr_len, -- 2.27.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list 2021-12-02 21:55 ` [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list Mimi Zohar @ 2021-12-02 22:22 ` Eric Biggers 2021-12-02 22:55 ` Mimi Zohar 0 siblings, 1 reply; 27+ messages in thread From: Eric Biggers @ 2021-12-02 22:22 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, Dec 02, 2021 at 04:55:05PM -0500, Mimi Zohar wrote: > Without the file signature included in the IMA measurement list, the type > of file digest is unclear. Set up the plumbing to limit including > fs-verity's file digest in the IMA measurement list based on whether the > template name is ima-sig. In the future, this could be relaxed to include > any template format that includes the file signature. > Does it make sense to tie IMA's fs-verity support to files having signatures? What about IMA audit mode? I thought that is just about collecting hashes, and has nothing to do with signatures. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list 2021-12-02 22:22 ` Eric Biggers @ 2021-12-02 22:55 ` Mimi Zohar 0 siblings, 0 replies; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 22:55 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, 2021-12-02 at 14:22 -0800, Eric Biggers wrote: > On Thu, Dec 02, 2021 at 04:55:05PM -0500, Mimi Zohar wrote: > > Without the file signature included in the IMA measurement list, the type > > of file digest is unclear. Set up the plumbing to limit including > > fs-verity's file digest in the IMA measurement list based on whether the > > template name is ima-sig. In the future, this could be relaxed to include > > any template format that includes the file signature. > > > > Does it make sense to tie IMA's fs-verity support to files having signatures? > What about IMA audit mode? I thought that is just about collecting hashes, and > has nothing to do with signatures. There's IMA-measurement, IMA-audit, and IMA-appraisal. IMA-audit refers to adding the file hash to the audit log record. IMA- measurement stores the collected hash in the IMA measurement list and extends the TPM with the measurement, if there's a TPM. Based on policy, determines whether the file is measured, audited, and/or appraised. I actually do think it makes sense to require a signature, but not necessarily enforce signature verification, in order to differentiate the type of measurement being included in the measurement list. thanks, Mimi ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar ` (2 preceding siblings ...) 2021-12-02 21:55 ` [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list Mimi Zohar @ 2021-12-02 21:55 ` Mimi Zohar 2021-12-02 22:07 ` Eric Biggers 2021-12-02 21:55 ` [PATCH v1 5/5] fsverity: update the documentation Mimi Zohar 4 siblings, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw) To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel Instead of calculating a file hash and verifying the signature stored in the security.ima xattr against the calculated file hash, verify the signature based on the fs-verity's file digest and other metadata. The fs-verity file digest is a hash that includes the Merkle tree root hash. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Changelog v1: - Based on Eric Bigger's review, instead of verifying the fsverity's file digest directly, sign a hash of it with other file metadata. security/integrity/ima/ima_api.c | 20 ++++++++++++ security/integrity/ima/ima_appraise.c | 45 ++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 7505563315cb..4fe7bc99378a 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -14,6 +14,7 @@ #include <linux/xattr.h> #include <linux/evm.h> #include <linux/iversion.h> +#include <linux/fsverity.h> #include "ima.h" @@ -200,6 +201,23 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode, allowed_algos); } +static int ima_collect_verity_digest(struct integrity_iint_cache *iint, + struct ima_digest_data *hash) +{ + u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE]; + enum hash_algo verity_alg; + int rc; + + rc = fsverity_collect_digest(iint->inode, verity_digest, &verity_alg); + if (rc) + return -EINVAL; + if (hash->algo != verity_alg) + return -EINVAL; + hash->length = hash_digest_size[verity_alg]; + memcpy(hash->digest, verity_digest, hash->length); + return 0; +} + /* * ima_collect_measurement - collect file measurement * @@ -251,6 +269,8 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, if (buf) result = ima_calc_buffer_hash(buf, size, &hash.hdr); + else if (veritysig) + result = ima_collect_verity_digest(iint, &hash.hdr); else result = ima_calc_file_hash(file, &hash.hdr); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 549fe051269a..53938aa0497a 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -240,6 +240,11 @@ 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) { + u8 verity_digest[IMA_MAX_DIGEST_SIZE + 1]; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash; int rc = -EINVAL, hash_start = 0; switch (xattr_value->type) { @@ -277,7 +282,45 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint, *status = INTEGRITY_PASS; break; case IMA_VERITY_DIGSIG: - fallthrough; + set_bit(IMA_DIGSIG, &iint->atomic_flags); + + /* + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG + * and the fs-verity file digest, not directly on the + * fs-verity file digest. Both digests should probably be + * included in the IMA measurement list, but for now this + * digest is only used for verifying the IMA signature. + */ + verity_digest[0] = IMA_VERITY_DIGSIG; + memcpy(verity_digest + 1, iint->ima_hash->digest, + iint->ima_hash->length); + + hash.hdr.algo = iint->ima_hash->algo; + hash.hdr.length = iint->ima_hash->length; + + rc = ima_calc_buffer_hash(verity_digest, + iint->ima_hash->length + 1, + &hash.hdr); + if (rc) { + *cause = "verity-hashing-error"; + *status = INTEGRITY_FAIL; + break; + } + + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, + (const char *)xattr_value, + xattr_len, + hash.hdr.digest, + hash.hdr.length); + + if (rc) { + *cause = "invalid-verity-signature"; + *status = INTEGRITY_FAIL; + } else { + *status = INTEGRITY_PASS; + } + + break; case EVM_IMA_XATTR_DIGSIG: set_bit(IMA_DIGSIG, &iint->atomic_flags); rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA, -- 2.27.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-02 21:55 ` [PATCH v1 4/5] ima: support fs-verity file digest based signatures Mimi Zohar @ 2021-12-02 22:07 ` Eric Biggers 2021-12-02 22:13 ` Mimi Zohar 2021-12-31 15:35 ` Mimi Zohar 0 siblings, 2 replies; 27+ messages in thread From: Eric Biggers @ 2021-12-02 22:07 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > case IMA_VERITY_DIGSIG: > - fallthrough; > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > + > + /* > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > + * and the fs-verity file digest, not directly on the > + * fs-verity file digest. Both digests should probably be > + * included in the IMA measurement list, but for now this > + * digest is only used for verifying the IMA signature. > + */ > + verity_digest[0] = IMA_VERITY_DIGSIG; > + memcpy(verity_digest + 1, iint->ima_hash->digest, > + iint->ima_hash->length); > + > + hash.hdr.algo = iint->ima_hash->algo; > + hash.hdr.length = iint->ima_hash->length; This is still wrong because the bytes being signed don't include the hash algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity supports SHA-512 too, and it may support other hash algorithms in the future. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-02 22:07 ` Eric Biggers @ 2021-12-02 22:13 ` Mimi Zohar 2021-12-02 22:18 ` Eric Biggers 2021-12-31 15:35 ` Mimi Zohar 1 sibling, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 22:13 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > case IMA_VERITY_DIGSIG: > > - fallthrough; > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > + > > + /* > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > + * and the fs-verity file digest, not directly on the > > + * fs-verity file digest. Both digests should probably be > > + * included in the IMA measurement list, but for now this > > + * digest is only used for verifying the IMA signature. > > + */ > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > + iint->ima_hash->length); > > + > > + hash.hdr.algo = iint->ima_hash->algo; > > + hash.hdr.length = iint->ima_hash->length; > > This is still wrong because the bytes being signed don't include the hash > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > supports SHA-512 too, and it may support other hash algorithms in the future. The signature stored in security.ima is prefixed with a header (signature_v2_hdr). Mimi ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-02 22:13 ` Mimi Zohar @ 2021-12-02 22:18 ` Eric Biggers 0 siblings, 0 replies; 27+ messages in thread From: Eric Biggers @ 2021-12-02 22:18 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, Dec 02, 2021 at 05:13:15PM -0500, Mimi Zohar wrote: > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > case IMA_VERITY_DIGSIG: > > > - fallthrough; > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > + > > > + /* > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > + * and the fs-verity file digest, not directly on the > > > + * fs-verity file digest. Both digests should probably be > > > + * included in the IMA measurement list, but for now this > > > + * digest is only used for verifying the IMA signature. > > > + */ > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > + iint->ima_hash->length); > > > + > > > + hash.hdr.algo = iint->ima_hash->algo; > > > + hash.hdr.length = iint->ima_hash->length; > > > > This is still wrong because the bytes being signed don't include the hash > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > supports SHA-512 too, and it may support other hash algorithms in the future. > > The signature stored in security.ima is prefixed with a header > (signature_v2_hdr). Yes, but the byte that identifies the hash algorithm is not included in the bytes that are actually signed, as far as I can tell. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-02 22:07 ` Eric Biggers 2021-12-02 22:13 ` Mimi Zohar @ 2021-12-31 15:35 ` Mimi Zohar 2022-01-05 23:37 ` Eric Biggers 1 sibling, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-31 15:35 UTC (permalink / raw) To: Eric Biggers; +Cc: linux-integrity, linux-fscrypt, linux-kernel Hi Eric, On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > case IMA_VERITY_DIGSIG: > > - fallthrough; > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > + > > + /* > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > + * and the fs-verity file digest, not directly on the > > + * fs-verity file digest. Both digests should probably be > > + * included in the IMA measurement list, but for now this > > + * digest is only used for verifying the IMA signature. > > + */ > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > + iint->ima_hash->length); > > + > > + hash.hdr.algo = iint->ima_hash->algo; > > + hash.hdr.length = iint->ima_hash->length; > > This is still wrong because the bytes being signed don't include the hash > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > supports SHA-512 too, and it may support other hash algorithms in the future. IMA assumes that the file hash algorithm and the signature algorithm are the same. If they're not the same, for whatever reason, the signature verification would simply fail. Based on the v2 signature header 'type' field, IMA can differentiate between regular IMA file hash based signatures and fs-verity file digest based signatures. The digest field (d-ng) in the IMA meausrement list prefixes the digest with the hash algorithm. I'm missing the reason for needing to hash fs-verity's file digest with other metadata, and sign that hash rather than fs-verity's file digest directly. thanks, Mimi ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2021-12-31 15:35 ` Mimi Zohar @ 2022-01-05 23:37 ` Eric Biggers 2022-01-09 20:45 ` Vitaly Chikunov 0 siblings, 1 reply; 27+ messages in thread From: Eric Biggers @ 2022-01-05 23:37 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > Hi Eric, > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > case IMA_VERITY_DIGSIG: > > > - fallthrough; > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > + > > > + /* > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > + * and the fs-verity file digest, not directly on the > > > + * fs-verity file digest. Both digests should probably be > > > + * included in the IMA measurement list, but for now this > > > + * digest is only used for verifying the IMA signature. > > > + */ > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > + iint->ima_hash->length); > > > + > > > + hash.hdr.algo = iint->ima_hash->algo; > > > + hash.hdr.length = iint->ima_hash->length; > > > > This is still wrong because the bytes being signed don't include the hash > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > supports SHA-512 too, and it may support other hash algorithms in the future. > > IMA assumes that the file hash algorithm and the signature algorithm > are the same. If they're not the same, for whatever reason, the > signature verification would simply fail. > > Based on the v2 signature header 'type' field, IMA can differentiate > between regular IMA file hash based signatures and fs-verity file > digest based signatures. The digest field (d-ng) in the IMA > meausrement list prefixes the digest with the hash algorithm. I'm > missing the reason for needing to hash fs-verity's file digest with > other metadata, and sign that hash rather than fs-verity's file digest > directly. Because if someone signs a raw hash, then they also implicitly sign the same hash value for all supported hash algorithms that produce the same length hash. Signing a raw hash is only appropriate when there is only 1 supported algorithm. All the other stuff you mentioned is irrelevant. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-05 23:37 ` Eric Biggers @ 2022-01-09 20:45 ` Vitaly Chikunov 2022-01-09 21:07 ` Eric Biggers 0 siblings, 1 reply; 27+ messages in thread From: Vitaly Chikunov @ 2022-01-09 20:45 UTC (permalink / raw) To: Eric Biggers; +Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel Eric, On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > case IMA_VERITY_DIGSIG: > > > > - fallthrough; > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > + > > > > + /* > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > + * and the fs-verity file digest, not directly on the > > > > + * fs-verity file digest. Both digests should probably be > > > > + * included in the IMA measurement list, but for now this > > > > + * digest is only used for verifying the IMA signature. > > > > + */ > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > + iint->ima_hash->length); > > > > + > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > This is still wrong because the bytes being signed don't include the hash > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > IMA assumes that the file hash algorithm and the signature algorithm > > are the same. If they're not the same, for whatever reason, the > > signature verification would simply fail. > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > between regular IMA file hash based signatures and fs-verity file > > digest based signatures. The digest field (d-ng) in the IMA > > meausrement list prefixes the digest with the hash algorithm. I'm > > missing the reason for needing to hash fs-verity's file digest with > > other metadata, and sign that hash rather than fs-verity's file digest > > directly. > > Because if someone signs a raw hash, then they also implicitly sign the same > hash value for all supported hash algorithms that produce the same length hash. Unless there is broken hash algorithm allowing for preimage attacks this is irrelevant. If there is two broken algorithms allowing for collisions, colliding hashes could be prepared even if algo id is hashed too. Thanks, > Signing a raw hash is only appropriate when there is only 1 supported algorithm. > > All the other stuff you mentioned is irrelevant. > > - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-09 20:45 ` Vitaly Chikunov @ 2022-01-09 21:07 ` Eric Biggers 2022-01-15 5:31 ` Vitaly Chikunov 0 siblings, 1 reply; 27+ messages in thread From: Eric Biggers @ 2022-01-09 21:07 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > Eric, > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > case IMA_VERITY_DIGSIG: > > > > > - fallthrough; > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > + > > > > > + /* > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > + * and the fs-verity file digest, not directly on the > > > > > + * fs-verity file digest. Both digests should probably be > > > > > + * included in the IMA measurement list, but for now this > > > > > + * digest is only used for verifying the IMA signature. > > > > > + */ > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > + iint->ima_hash->length); > > > > > + > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > are the same. If they're not the same, for whatever reason, the > > > signature verification would simply fail. > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > between regular IMA file hash based signatures and fs-verity file > > > digest based signatures. The digest field (d-ng) in the IMA > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > missing the reason for needing to hash fs-verity's file digest with > > > other metadata, and sign that hash rather than fs-verity's file digest > > > directly. > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > hash value for all supported hash algorithms that produce the same length hash. > > Unless there is broken hash algorithm allowing for preimage attacks this > is irrelevant. If there is two broken algorithms allowing for collisions, > colliding hashes could be prepared even if algo id is hashed too. > Only one algorithm needs to be broken. For example, SM3 has the same hash length as SHA-256. If SM3 support were to be added to fs-verity, and if someone were to find a way to find an input that has a specific SM3 digest, then they could also make it match a specific SHA-256 digest. Someone might intend to sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the digest, then they would also be signing the corresponding SM3 digest. That's why the digest that is signed *must* also include the algorithm used in the digest (not the algorithm(s) used in the signature, which is different). - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-09 21:07 ` Eric Biggers @ 2022-01-15 5:31 ` Vitaly Chikunov 2022-01-15 6:21 ` Eric Biggers 0 siblings, 1 reply; 27+ messages in thread From: Vitaly Chikunov @ 2022-01-15 5:31 UTC (permalink / raw) To: Eric Biggers; +Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel Eric, On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: > On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > > case IMA_VERITY_DIGSIG: > > > > > > - fallthrough; > > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > > + > > > > > > + /* > > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > > + * and the fs-verity file digest, not directly on the > > > > > > + * fs-verity file digest. Both digests should probably be > > > > > > + * included in the IMA measurement list, but for now this > > > > > > + * digest is only used for verifying the IMA signature. > > > > > > + */ > > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > > + iint->ima_hash->length); > > > > > > + > > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > > are the same. If they're not the same, for whatever reason, the > > > > signature verification would simply fail. > > > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > > between regular IMA file hash based signatures and fs-verity file > > > > digest based signatures. The digest field (d-ng) in the IMA > > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > > missing the reason for needing to hash fs-verity's file digest with > > > > other metadata, and sign that hash rather than fs-verity's file digest > > > > directly. > > > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > > hash value for all supported hash algorithms that produce the same length hash. > > > > Unless there is broken hash algorithm allowing for preimage attacks this > > is irrelevant. If there is two broken algorithms allowing for collisions, > > colliding hashes could be prepared even if algo id is hashed too. > > > > Only one algorithm needs to be broken. For example, SM3 has the same hash > length as SHA-256. If SM3 support were to be added to fs-verity, and if someone > were to find a way to find an input that has a specific SM3 digest, then they > could also make it match a specific SHA-256 digest. Someone might intend to > sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the > digest, then they would also be signing the corresponding SM3 digest. That's > why the digest that is signed *must* also include the algorithm used in the > digest (not the algorithm(s) used in the signature, which is different). I think it will be beneficial if we pass hash algo id to the akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family hashes. Thanks, > > - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-15 5:31 ` Vitaly Chikunov @ 2022-01-15 6:21 ` Eric Biggers 2022-01-16 3:31 ` Stefan Berger 2022-01-16 17:01 ` Mimi Zohar 0 siblings, 2 replies; 27+ messages in thread From: Eric Biggers @ 2022-01-15 6:21 UTC (permalink / raw) To: Vitaly Chikunov; +Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: > Eric, > > On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: > > On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > > > case IMA_VERITY_DIGSIG: > > > > > > > - fallthrough; > > > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > > > + > > > > > > > + /* > > > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > > > + * and the fs-verity file digest, not directly on the > > > > > > > + * fs-verity file digest. Both digests should probably be > > > > > > > + * included in the IMA measurement list, but for now this > > > > > > > + * digest is only used for verifying the IMA signature. > > > > > > > + */ > > > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > > > + iint->ima_hash->length); > > > > > > > + > > > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > > > are the same. If they're not the same, for whatever reason, the > > > > > signature verification would simply fail. > > > > > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > > > between regular IMA file hash based signatures and fs-verity file > > > > > digest based signatures. The digest field (d-ng) in the IMA > > > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > > > missing the reason for needing to hash fs-verity's file digest with > > > > > other metadata, and sign that hash rather than fs-verity's file digest > > > > > directly. > > > > > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > > > hash value for all supported hash algorithms that produce the same length hash. > > > > > > Unless there is broken hash algorithm allowing for preimage attacks this > > > is irrelevant. If there is two broken algorithms allowing for collisions, > > > colliding hashes could be prepared even if algo id is hashed too. > > > > > > > Only one algorithm needs to be broken. For example, SM3 has the same hash > > length as SHA-256. If SM3 support were to be added to fs-verity, and if someone > > were to find a way to find an input that has a specific SM3 digest, then they > > could also make it match a specific SHA-256 digest. Someone might intend to > > sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the > > digest, then they would also be signing the corresponding SM3 digest. That's > > why the digest that is signed *must* also include the algorithm used in the > > digest (not the algorithm(s) used in the signature, which is different). > > I think it will be beneficial if we pass hash algo id to the > akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. > And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family > hashes. > I was going to reply to this thread again, but I got a bit distracted by everything else being broken. Yes, the kernel needs to be restricting which hash algorithms can be used with each public key algorithm, along the lines of what you said. I asked the BoringSSL maintainers for advice, and they confirmed that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash for it to be secure. This is a design flaw in ECDSA, which was fixed in newer algorithms such as EdDSA and SM2 as those have a hash built-in to the signature scheme. To mitigate it, the allowed hash algorithms must be restricted; in the case of ECDSA, that means to the SHA family (preferably excluding SHA-1). akcipher_alg::verify doesn't actually know which hash algorithm is used, except in the case of rsa-pkcs1pad where it is built into the name of the algorithm. So it can't check the hash algorithm. I believe it needs to happen in public_key_verify_signature() (and I'm working on a patch for that). Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design that includes the hash into the signature algorithm. This means that it must be used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but SM2 will hash it again internally.) Currently, public_key_verify_signature() allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and IMA uses this. This is broken and must be removed, since it isn't actually the SM2 algorithm as specified anymore, but rather some homebrew thing with unknown security properties. (Well, I'm not confident about SM2, but homebrew is worse.) Adding fs-verity support to IMA also complicates things, as doing it naively would introduce an ambiguity about what is signed. Naively, the *data* that is signed (considering the hash as part of the signature algorithm) would be either the whole file, in the case of traditional IMA, or the fsverity_descriptor struct, in the case of IMA with fs-verity. However, a file could have contents which match an fsverity_descriptor struct; that would create an ambiguity. Assuming that it needs to be allowed that the same key can sign files for both traditional and fs-verity hashing, solving this problem will require a second hash. The easiest way to do this would be sign/verify the following struct: struct ima_file_id { u8 is_fsverity; u8 hash_algorithm; u8 hash[]; }; This would be the *data* that is signed/verified -- meaning that it would be hashed again as part of the signature algorithm (whether that hash is built-in to the signature algorithm, as is the case for modern algorithms, or handled by the caller as is the case for legacy algorithms). Note that both traditional and fs-verity hashes would need to use this same method for it to be secure; the kernel must not accept signatures using the old method at the same time. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-15 6:21 ` Eric Biggers @ 2022-01-16 3:31 ` Stefan Berger 2022-01-16 5:24 ` Stefan Berger 2022-01-19 0:49 ` Eric Biggers 2022-01-16 17:01 ` Mimi Zohar 1 sibling, 2 replies; 27+ messages in thread From: Stefan Berger @ 2022-01-16 3:31 UTC (permalink / raw) To: Eric Biggers, Vitaly Chikunov Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel On 1/15/22 01:21, Eric Biggers wrote: > On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: >> Eric, >> >> On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: >>> On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: >>>> On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: >>>>> On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: >>>>>> On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: >>>>>>> On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: >>>>>>>> case IMA_VERITY_DIGSIG: >>>>>>>> - fallthrough; >>>>>>>> + set_bit(IMA_DIGSIG, &iint->atomic_flags); >>>>>>>> + >>>>>>>> + /* >>>>>>>> + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG >>>>>>>> + * and the fs-verity file digest, not directly on the >>>>>>>> + * fs-verity file digest. Both digests should probably be >>>>>>>> + * included in the IMA measurement list, but for now this >>>>>>>> + * digest is only used for verifying the IMA signature. >>>>>>>> + */ >>>>>>>> + verity_digest[0] = IMA_VERITY_DIGSIG; >>>>>>>> + memcpy(verity_digest + 1, iint->ima_hash->digest, >>>>>>>> + iint->ima_hash->length); >>>>>>>> + >>>>>>>> + hash.hdr.algo = iint->ima_hash->algo; >>>>>>>> + hash.hdr.length = iint->ima_hash->length; >>>>>>> This is still wrong because the bytes being signed don't include the hash >>>>>>> algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity >>>>>>> supports SHA-512 too, and it may support other hash algorithms in the future. >>>>>> IMA assumes that the file hash algorithm and the signature algorithm >>>>>> are the same. If they're not the same, for whatever reason, the >>>>>> signature verification would simply fail. >>>>>> >>>>>> Based on the v2 signature header 'type' field, IMA can differentiate >>>>>> between regular IMA file hash based signatures and fs-verity file >>>>>> digest based signatures. The digest field (d-ng) in the IMA >>>>>> meausrement list prefixes the digest with the hash algorithm. I'm >>>>>> missing the reason for needing to hash fs-verity's file digest with >>>>>> other metadata, and sign that hash rather than fs-verity's file digest >>>>>> directly. >>>>> Because if someone signs a raw hash, then they also implicitly sign the same >>>>> hash value for all supported hash algorithms that produce the same length hash. >>>> Unless there is broken hash algorithm allowing for preimage attacks this >>>> is irrelevant. If there is two broken algorithms allowing for collisions, >>>> colliding hashes could be prepared even if algo id is hashed too. >>>> >>> Only one algorithm needs to be broken. For example, SM3 has the same hash >>> length as SHA-256. If SM3 support were to be added to fs-verity, and if someone >>> were to find a way to find an input that has a specific SM3 digest, then they >>> could also make it match a specific SHA-256 digest. Someone might intend to >>> sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the >>> digest, then they would also be signing the corresponding SM3 digest. That's >>> why the digest that is signed *must* also include the algorithm used in the >>> digest (not the algorithm(s) used in the signature, which is different). >> I think it will be beneficial if we pass hash algo id to the >> akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. >> And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family >> hashes. >> > I was going to reply to this thread again, but I got a bit distracted by > everything else being broken. Yes, the kernel needs to be restricting which > hash algorithms can be used with each public key algorithm, along the lines of > what you said. I asked the BoringSSL maintainers for advice, and they confirmed > that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash > for it to be secure. This is a design flaw in ECDSA, which was fixed in newer > algorithms such as EdDSA and SM2 as those have a hash built-in to the signature > scheme. To mitigate it, the allowed hash algorithms must be restricted; in the > case of ECDSA, that means to the SHA family (preferably excluding SHA-1). > > akcipher_alg::verify doesn't actually know which hash algorithm is used, except > in the case of rsa-pkcs1pad where it is built into the name of the algorithm. > So it can't check the hash algorithm. I believe it needs to happen in > public_key_verify_signature() (and I'm working on a patch for that). > > Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design > that includes the hash into the signature algorithm. This means that it must be > used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but > SM2 will hash it again internally.) Currently, public_key_verify_signature() > allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and > IMA uses this. This is broken and must be removed, since it isn't actually the > SM2 algorithm as specified anymore, but rather some homebrew thing with unknown > security properties. (Well, I'm not confident about SM2, but homebrew is worse.) > > Adding fs-verity support to IMA also complicates things, as doing it naively > would introduce an ambiguity about what is signed. Naively, the *data* that is > signed (considering the hash as part of the signature algorithm) would be either > the whole file, in the case of traditional IMA, or the fsverity_descriptor > struct, in the case of IMA with fs-verity. However, a file could have contents > which match an fsverity_descriptor struct; that would create an ambiguity. > > Assuming that it needs to be allowed that the same key can sign files for both > traditional and fs-verity hashing, solving this problem will require a second > hash. The easiest way to do this would be sign/verify the following struct: > > struct ima_file_id { > u8 is_fsverity; > u8 hash_algorithm; > u8 hash[]; > }; To calrify, I suppose that for ECDSA NIST P256 you would allow pairing with any of the SHA family hashes (also as defined by the existing OIDs) and as the standard allows today? And the same then applies for NIST p384 etc.? Further, I suppose similar restriction would apply for ECRDSA to pair it with Streebog only, as Vitaly said. What's happening now is that to verify a signature, IMA/integrity subsystem fills out the following structure: struct public_key_signature pks; pks.hash_algo = hash_algo_name[hdr->hash_algo]; // name of hash algo will go into this here, e.g., 'sha256' pks.pkey_algo = pk->pkey_algo; // this is either 'rsa', 'ecdsa-', 'ecrdsa-' or 'sm2' string It then calls: ret = verify_signature(key, &pks); IMO, in the call path down this function the pairing of public key and hash algo would have to be enforced in order to enforce the standards. Would this not be sufficient to be able to stay with the standards ? File hashes: IMA calculates the hash over a file itself by calling crypto functions, so at least the digest's bytes are trusted input in that respect and using the sha family type of hashes directly with ECDSA should work. Which algorithm IMA is supposed to use for the hashing is given in the xattr bytestream header. IMA could then take that type of hash, lookup the hash function, perform the hashing on the data, and let verify_signature enforce the pairing, rejecting file signatures with wrong pairing. This way the only thing that is needed is 'enforcement of pairing'. Fsverity: How much control does a user have over the hash family fsverity is using? Can IMA ECDSA/RSA users tell it to use a sha family hash and ECRDSA users make it use a Streebog hash so that also the pairing of hash and key type can work 'naturally' and we don't need the level of indirection via your structure above? Stefan > This would be the *data* that is signed/verified -- meaning that it would be > hashed again as part of the signature algorithm (whether that hash is built-in > to the signature algorithm, as is the case for modern algorithms, or handled by > the caller as is the case for legacy algorithms). Note that both traditional > and fs-verity hashes would need to use this same method for it to be secure; the > kernel must not accept signatures using the old method at the same time. > > - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-16 3:31 ` Stefan Berger @ 2022-01-16 5:24 ` Stefan Berger 2022-01-19 0:49 ` Eric Biggers 1 sibling, 0 replies; 27+ messages in thread From: Stefan Berger @ 2022-01-16 5:24 UTC (permalink / raw) To: Eric Biggers, Vitaly Chikunov Cc: Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel On 1/15/22 22:31, Stefan Berger wrote: > > On 1/15/22 01:21, Eric Biggers wrote: >> On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: >>> Eric, >>> >>> On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: >>>> On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: >>>>> On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: >>>>>> On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: >>>>>>> On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: >>>>>>>> On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: >>>>>>>>> case IMA_VERITY_DIGSIG: >>>>>>>>> - fallthrough; >>>>>>>>> + set_bit(IMA_DIGSIG, &iint->atomic_flags); >>>>>>>>> + >>>>>>>>> + /* >>>>>>>>> + * The IMA signature is based on a hash of >>>>>>>>> IMA_VERITY_DIGSIG >>>>>>>>> + * and the fs-verity file digest, not directly on the >>>>>>>>> + * fs-verity file digest. Both digests should >>>>>>>>> probably be >>>>>>>>> + * included in the IMA measurement list, but for now >>>>>>>>> this >>>>>>>>> + * digest is only used for verifying the IMA signature. >>>>>>>>> + */ >>>>>>>>> + verity_digest[0] = IMA_VERITY_DIGSIG; >>>>>>>>> + memcpy(verity_digest + 1, iint->ima_hash->digest, >>>>>>>>> + iint->ima_hash->length); >>>>>>>>> + >>>>>>>>> + hash.hdr.algo = iint->ima_hash->algo; >>>>>>>>> + hash.hdr.length = iint->ima_hash->length; >>>>>>>> This is still wrong because the bytes being signed don't >>>>>>>> include the hash >>>>>>>> algorithm. Unless you mean for it to be implicitly always >>>>>>>> SHA-256? fs-verity >>>>>>>> supports SHA-512 too, and it may support other hash algorithms >>>>>>>> in the future. >>>>>>> IMA assumes that the file hash algorithm and the signature >>>>>>> algorithm >>>>>>> are the same. If they're not the same, for whatever reason, the >>>>>>> signature verification would simply fail. >>>>>>> >>>>>>> Based on the v2 signature header 'type' field, IMA can >>>>>>> differentiate >>>>>>> between regular IMA file hash based signatures and fs-verity file >>>>>>> digest based signatures. The digest field (d-ng) in the IMA >>>>>>> meausrement list prefixes the digest with the hash algorithm. I'm >>>>>>> missing the reason for needing to hash fs-verity's file digest with >>>>>>> other metadata, and sign that hash rather than fs-verity's file >>>>>>> digest >>>>>>> directly. >>>>>> Because if someone signs a raw hash, then they also implicitly >>>>>> sign the same >>>>>> hash value for all supported hash algorithms that produce the >>>>>> same length hash. >>>>> Unless there is broken hash algorithm allowing for preimage >>>>> attacks this >>>>> is irrelevant. If there is two broken algorithms allowing for >>>>> collisions, >>>>> colliding hashes could be prepared even if algo id is hashed too. >>>>> >>>> Only one algorithm needs to be broken. For example, SM3 has the >>>> same hash >>>> length as SHA-256. If SM3 support were to be added to fs-verity, >>>> and if someone >>>> were to find a way to find an input that has a specific SM3 digest, >>>> then they >>>> could also make it match a specific SHA-256 digest. Someone might >>>> intend to >>>> sign a SHA-256 digest, but if they are only signing the raw 32 >>>> bytes of the >>>> digest, then they would also be signing the corresponding SM3 >>>> digest. That's >>>> why the digest that is signed *must* also include the algorithm >>>> used in the >>>> digest (not the algorithm(s) used in the signature, which is >>>> different). >>> I think it will be beneficial if we pass hash algo id to the >>> akcipher_alg::verify. In fact, ecrdsa should only be used with >>> streebog. >>> And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha >>> family >>> hashes. >>> >> I was going to reply to this thread again, but I got a bit distracted by >> everything else being broken. Yes, the kernel needs to be >> restricting which >> hash algorithms can be used with each public key algorithm, along the >> lines of >> what you said. I asked the BoringSSL maintainers for advice, and >> they confirmed >> that ECDSA just signs/verifies a raw hash, and in fact it *must* be a >> raw hash >> for it to be secure. This is a design flaw in ECDSA, which was fixed >> in newer >> algorithms such as EdDSA and SM2 as those have a hash built-in to the >> signature >> scheme. To mitigate it, the allowed hash algorithms must be >> restricted; in the >> case of ECDSA, that means to the SHA family (preferably excluding >> SHA-1). >> >> akcipher_alg::verify doesn't actually know which hash algorithm is >> used, except >> in the case of rsa-pkcs1pad where it is built into the name of the >> algorithm. >> So it can't check the hash algorithm. I believe it needs to happen in >> public_key_verify_signature() (and I'm working on a patch for that). >> >> Now, SM2 is different from ECDSA and ECRDSA in that it uses the >> modern design >> that includes the hash into the signature algorithm. This means that >> it must be >> used to sign/verify *data*, not a hash. (Well, you can sign/verify a >> hash, but >> SM2 will hash it again internally.) Currently, >> public_key_verify_signature() >> allows SM2 to be used to sign/verify a hash, skipping the SM2 >> internal hash, and >> IMA uses this. This is broken and must be removed, since it isn't >> actually the >> SM2 algorithm as specified anymore, but rather some homebrew thing >> with unknown >> security properties. (Well, I'm not confident about SM2, but homebrew >> is worse.) >> >> Adding fs-verity support to IMA also complicates things, as doing it >> naively >> would introduce an ambiguity about what is signed. Naively, the >> *data* that is >> signed (considering the hash as part of the signature algorithm) >> would be either >> the whole file, in the case of traditional IMA, or the >> fsverity_descriptor >> struct, in the case of IMA with fs-verity. However, a file could >> have contents >> which match an fsverity_descriptor struct; that would create an >> ambiguity. >> >> Assuming that it needs to be allowed that the same key can sign files >> for both >> traditional and fs-verity hashing, solving this problem will require >> a second >> hash. The easiest way to do this would be sign/verify the following >> struct: >> >> struct ima_file_id { >> u8 is_fsverity; >> u8 hash_algorithm; >> u8 hash[]; >> }; > > > To calrify, I suppose that for ECDSA NIST P256 you would allow pairing > with any of the SHA family hashes (also as defined by the existing > OIDs) and as the standard allows today? And the same then applies for > NIST p384 etc.? > > Further, I suppose similar restriction would apply for ECRDSA to pair > it with Streebog only, as Vitaly said. > > > What's happening now is that to verify a signature, IMA/integrity > subsystem fills out the following structure: > > struct public_key_signature pks; > > pks.hash_algo = hash_algo_name[hdr->hash_algo]; // name of hash algo > will go into this here, e.g., 'sha256' > pks.pkey_algo = pk->pkey_algo; // this is either 'rsa', 'ecdsa-', > 'ecrdsa-' or 'sm2' string > > It then calls: > > ret = verify_signature(key, &pks); > > IMO, in the call path down this function the pairing of public key and > hash algo would have to be enforced in order to enforce the standards. > Would this not be sufficient to be able to stay with the standards ? > > File hashes: IMA calculates the hash over a file itself by calling > crypto functions, so at least the digest's bytes are trusted input in > that respect and using the sha family type of hashes directly with > ECDSA should work. Which algorithm IMA is supposed to use for the > hashing is given in the xattr bytestream header. IMA could then take > that type of hash, lookup the hash function, perform the hashing on > the data, and let verify_signature enforce the pairing, rejecting file > signatures with wrong pairing. This way the only thing that is needed > is 'enforcement of pairing'. > > Fsverity: How much control does a user have over the hash family > fsverity is using? Can IMA ECDSA/RSA users tell it to use a sha family > hash and ECRDSA users make it use a Streebog hash so that also the > pairing of hash and key type can work 'naturally' and we don't need > the level of indirection via your structure above? > > Stefan > > I would propos probably 3 patches that combined result in the following for enforcement of pairing. I think RSA isn't necessary to enforce since the OID in the signature contains the hash that was used. For SM2 I am not so sure about. From 800f11c32f6e64d328404c9fdd7abb6057dbebb9 Mon Sep 17 00:00:00 2001 From: Stefan Berger <stefanb@linux.ibm.com> Date: Sun, 16 Jan 2022 00:07:00 -0500 Subject: [PATCH] keys: asymmetric: Enforce key type and hash algo pairing for signatures Enforce the pairing of ECRDSA with Streebog hash type, SM2 with SM3 hash, and ECDSA with the SHA family of hashes for the low level verify_signature() API. Note: X509 certificates already have enforcement through the availability of OIDs. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- crypto/asymmetric_keys/public_key.c | 59 +++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 4fefb219bfdc..760b541d2803 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -20,6 +20,7 @@ #include <crypto/akcipher.h> #include <crypto/sm2.h> #include <crypto/sm3_base.h> +#include <crypto/hash_info.h> MODULE_DESCRIPTION("In-software asymmetric public-key subtype"); MODULE_AUTHOR("Red Hat, Inc."); @@ -305,6 +306,60 @@ static inline int cert_sig_digest_update( } #endif /* ! IS_REACHABLE(CONFIG_CRYPTO_SM2) */ +/* ECDSA allowed hashes */ +static const enum hash_algo ecdsa_hashes[] = { + HASH_ALGO_SHA1, + HASH_ALGO_SHA224, + HASH_ALGO_SHA256, + HASH_ALGO_SHA384, + HASH_ALGO_SHA512, +}; + +/* SM2 allowed hashes */ +static const enum hash_algo sm2_hashes[] = { + HASH_ALGO_SM3_256, +}; + +/* ECRDSA allowed hashes */ +static const enum hash_algo ecrdsa_hashes[] = { + HASH_ALGO_STREEBOG_256, + HASH_ALGO_STREEBOG_512, +}; + +/* + * Check the pairing for key algorithm and hash for ECDSA + * and ECRDSA. + */ +static int check_pkey_hash_algo_pairing(const struct public_key_signature *sig) +{ + const enum hash_algo *array = NULL; + size_t array_size; + size_t i; + + if (!strcmp(sig->pkey_algo, "ecrdsa")) { + array = ecrdsa_hashes; + array_size = ARRAY_SIZE(ecrdsa_hashes); + } else if (!strcmp(sig->pkey_algo, "sm2")) { + array = sm2_hashes; + array_size = ARRAY_SIZE(sm2_hashes); + } else if (!strncmp(sig->pkey_algo, "ecdsa-", 6)) { + /* ecdsa-nist-p192 etc. */ + array = ecdsa_hashes; + array_size = ARRAY_SIZE(ecdsa_hashes); + } + + if (array) { + for (i = 0; i < array_size; i++) { + if (!strcmp(sig->hash_algo, + hash_algo_name[array[i]])) + return 0; + } + return -EINVAL; + } + + return 0; +} + /* * Verify a signature using a public key. */ @@ -325,6 +380,10 @@ int public_key_verify_signature(const struct public_key *pkey, BUG_ON(!sig); BUG_ON(!sig->s); + ret = check_pkey_hash_algo_pairing(sig); + if (ret < 0) + return ret; + ret = software_key_determine_akcipher(sig->encoding, sig->hash_algo, pkey, alg_name); -- 2.31.1 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-16 3:31 ` Stefan Berger 2022-01-16 5:24 ` Stefan Berger @ 2022-01-19 0:49 ` Eric Biggers 2022-01-19 15:41 ` Stefan Berger 1 sibling, 1 reply; 27+ messages in thread From: Eric Biggers @ 2022-01-19 0:49 UTC (permalink / raw) To: Stefan Berger Cc: Vitaly Chikunov, Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel On Sat, Jan 15, 2022 at 10:31:40PM -0500, Stefan Berger wrote: > > On 1/15/22 01:21, Eric Biggers wrote: > > On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: > > > Eric, > > > > > > On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: > > > > On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > > > > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > > > > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > > > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > > > > > case IMA_VERITY_DIGSIG: > > > > > > > > > - fallthrough; > > > > > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > > > > > + > > > > > > > > > + /* > > > > > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > > > > > + * and the fs-verity file digest, not directly on the > > > > > > > > > + * fs-verity file digest. Both digests should probably be > > > > > > > > > + * included in the IMA measurement list, but for now this > > > > > > > > > + * digest is only used for verifying the IMA signature. > > > > > > > > > + */ > > > > > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > > > > > + iint->ima_hash->length); > > > > > > > > > + > > > > > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > > > > > are the same. If they're not the same, for whatever reason, the > > > > > > > signature verification would simply fail. > > > > > > > > > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > > > > > between regular IMA file hash based signatures and fs-verity file > > > > > > > digest based signatures. The digest field (d-ng) in the IMA > > > > > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > > > > > missing the reason for needing to hash fs-verity's file digest with > > > > > > > other metadata, and sign that hash rather than fs-verity's file digest > > > > > > > directly. > > > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > > > > > hash value for all supported hash algorithms that produce the same length hash. > > > > > Unless there is broken hash algorithm allowing for preimage attacks this > > > > > is irrelevant. If there is two broken algorithms allowing for collisions, > > > > > colliding hashes could be prepared even if algo id is hashed too. > > > > > > > > > Only one algorithm needs to be broken. For example, SM3 has the same hash > > > > length as SHA-256. If SM3 support were to be added to fs-verity, and if someone > > > > were to find a way to find an input that has a specific SM3 digest, then they > > > > could also make it match a specific SHA-256 digest. Someone might intend to > > > > sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the > > > > digest, then they would also be signing the corresponding SM3 digest. That's > > > > why the digest that is signed *must* also include the algorithm used in the > > > > digest (not the algorithm(s) used in the signature, which is different). > > > I think it will be beneficial if we pass hash algo id to the > > > akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. > > > And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family > > > hashes. > > > > > I was going to reply to this thread again, but I got a bit distracted by > > everything else being broken. Yes, the kernel needs to be restricting which > > hash algorithms can be used with each public key algorithm, along the lines of > > what you said. I asked the BoringSSL maintainers for advice, and they confirmed > > that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash > > for it to be secure. This is a design flaw in ECDSA, which was fixed in newer > > algorithms such as EdDSA and SM2 as those have a hash built-in to the signature > > scheme. To mitigate it, the allowed hash algorithms must be restricted; in the > > case of ECDSA, that means to the SHA family (preferably excluding SHA-1). > > > > akcipher_alg::verify doesn't actually know which hash algorithm is used, except > > in the case of rsa-pkcs1pad where it is built into the name of the algorithm. > > So it can't check the hash algorithm. I believe it needs to happen in > > public_key_verify_signature() (and I'm working on a patch for that). > > > > Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design > > that includes the hash into the signature algorithm. This means that it must be > > used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but > > SM2 will hash it again internally.) Currently, public_key_verify_signature() > > allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and > > IMA uses this. This is broken and must be removed, since it isn't actually the > > SM2 algorithm as specified anymore, but rather some homebrew thing with unknown > > security properties. (Well, I'm not confident about SM2, but homebrew is worse.) > > > > Adding fs-verity support to IMA also complicates things, as doing it naively > > would introduce an ambiguity about what is signed. Naively, the *data* that is > > signed (considering the hash as part of the signature algorithm) would be either > > the whole file, in the case of traditional IMA, or the fsverity_descriptor > > struct, in the case of IMA with fs-verity. However, a file could have contents > > which match an fsverity_descriptor struct; that would create an ambiguity. > > > > Assuming that it needs to be allowed that the same key can sign files for both > > traditional and fs-verity hashing, solving this problem will require a second > > hash. The easiest way to do this would be sign/verify the following struct: > > > > struct ima_file_id { > > u8 is_fsverity; > > u8 hash_algorithm; > > u8 hash[]; > > }; > > > To calrify, I suppose that for ECDSA NIST P256 you would allow pairing with > any of the SHA family hashes (also as defined by the existing OIDs) and as > the standard allows today? And the same then applies for NIST p384 etc.? > > Further, I suppose similar restriction would apply for ECRDSA to pair it > with Streebog only, as Vitaly said. I don't have any better ideas. > What's happening now is that to verify a signature, IMA/integrity subsystem > fills out the following structure: > > struct public_key_signature pks; > > pks.hash_algo = hash_algo_name[hdr->hash_algo]; // name of hash algo will > go into this here, e.g., 'sha256' > pks.pkey_algo = pk->pkey_algo; // this is either 'rsa', 'ecdsa-', 'ecrdsa-' > or 'sm2' string > > It then calls: > > ret = verify_signature(key, &pks); > > IMO, in the call path down this function the pairing of public key and hash > algo would have to be enforced in order to enforce the standards. Would this > not be sufficient to be able to stay with the standards ? That sounds right, though there are a number of other issues including SM2 being implemented incorrectly, the "encoding" string isn't validated, and it not being enforced that public_key_signature::pkey_algo actually matches public_key::pkey_algo. > File hashes: IMA calculates the hash over a file itself by calling crypto > functions, so at least the digest's bytes are trusted input in that respect > and using the sha family type of hashes directly with ECDSA should work. > Which algorithm IMA is supposed to use for the hashing is given in the xattr > bytestream header. IMA could then take that type of hash, lookup the hash > function, perform the hashing on the data, and let verify_signature enforce > the pairing, rejecting file signatures with wrong pairing. This way the only > thing that is needed is 'enforcement of pairing'. > > Fsverity: How much control does a user have over the hash family fsverity is > using? Can IMA ECDSA/RSA users tell it to use a sha family hash and ECRDSA > users make it use a Streebog hash so that also the pairing of hash and key > type can work 'naturally' and we don't need the level of indirection via > your structure above? The hash algorithm used by fs-verity is configurable and is always returned along with the file digest. Currently, only SHA-256 and SHA-512 are supported. Keep in mind that if you sign the fs-verity file digest directly with RSA, ECDSA, or ECRDSA, the *data* you are actually signing is the fsverity_descriptor -- the struct which the hash is a hash of. That creates an ambiguity when full file hashes are also signed by the same key, as I previously mentioned. A level of indirection is needed to avoid that. In the naive method, the *data* being signed would also be different with SM2. The level of indirection would avoid that. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-19 0:49 ` Eric Biggers @ 2022-01-19 15:41 ` Stefan Berger 0 siblings, 0 replies; 27+ messages in thread From: Stefan Berger @ 2022-01-19 15:41 UTC (permalink / raw) To: Eric Biggers, Vitaly Chikunov Cc: Vitaly Chikunov, Mimi Zohar, linux-integrity, linux-fscrypt, linux-kernel, linux-crypto On 1/18/22 19:49, Eric Biggers wrote: > On Sat, Jan 15, 2022 at 10:31:40PM -0500, Stefan Berger wrote: >> On 1/15/22 01:21, Eric Biggers wrote: >>> On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: >>>> Eric, >>>> >>>> On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: >>>>> On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: >>>>>> On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: >>>>>>> On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: >>>>>>>> On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: >>>>>>>>> On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: >>>>>>>>>> case IMA_VERITY_DIGSIG: >>>>>>>>>> - fallthrough; >>>>>>>>>> + set_bit(IMA_DIGSIG, &iint->atomic_flags); >>>>>>>>>> + >>>>>>>>>> + /* >>>>>>>>>> + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG >>>>>>>>>> + * and the fs-verity file digest, not directly on the >>>>>>>>>> + * fs-verity file digest. Both digests should probably be >>>>>>>>>> + * included in the IMA measurement list, but for now this >>>>>>>>>> + * digest is only used for verifying the IMA signature. >>>>>>>>>> + */ >>>>>>>>>> + verity_digest[0] = IMA_VERITY_DIGSIG; >>>>>>>>>> + memcpy(verity_digest + 1, iint->ima_hash->digest, >>>>>>>>>> + iint->ima_hash->length); >>>>>>>>>> + >>>>>>>>>> + hash.hdr.algo = iint->ima_hash->algo; >>>>>>>>>> + hash.hdr.length = iint->ima_hash->length; >>>>>>>>> This is still wrong because the bytes being signed don't include the hash >>>>>>>>> algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity >>>>>>>>> supports SHA-512 too, and it may support other hash algorithms in the future. >>>>>>>> IMA assumes that the file hash algorithm and the signature algorithm >>>>>>>> are the same. If they're not the same, for whatever reason, the >>>>>>>> signature verification would simply fail. >>>>>>>> >>>>>>>> Based on the v2 signature header 'type' field, IMA can differentiate >>>>>>>> between regular IMA file hash based signatures and fs-verity file >>>>>>>> digest based signatures. The digest field (d-ng) in the IMA >>>>>>>> meausrement list prefixes the digest with the hash algorithm. I'm >>>>>>>> missing the reason for needing to hash fs-verity's file digest with >>>>>>>> other metadata, and sign that hash rather than fs-verity's file digest >>>>>>>> directly. >>>>>>> Because if someone signs a raw hash, then they also implicitly sign the same >>>>>>> hash value for all supported hash algorithms that produce the same length hash. >>>>>> Unless there is broken hash algorithm allowing for preimage attacks this >>>>>> is irrelevant. If there is two broken algorithms allowing for collisions, >>>>>> colliding hashes could be prepared even if algo id is hashed too. >>>>>> >>>>> Only one algorithm needs to be broken. For example, SM3 has the same hash >>>>> length as SHA-256. If SM3 support were to be added to fs-verity, and if someone >>>>> were to find a way to find an input that has a specific SM3 digest, then they >>>>> could also make it match a specific SHA-256 digest. Someone might intend to >>>>> sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the >>>>> digest, then they would also be signing the corresponding SM3 digest. That's >>>>> why the digest that is signed *must* also include the algorithm used in the >>>>> digest (not the algorithm(s) used in the signature, which is different). >>>> I think it will be beneficial if we pass hash algo id to the >>>> akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. >>>> And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family >>>> hashes. >>>> >>> I was going to reply to this thread again, but I got a bit distracted by >>> everything else being broken. Yes, the kernel needs to be restricting which >>> hash algorithms can be used with each public key algorithm, along the lines of >>> what you said. I asked the BoringSSL maintainers for advice, and they confirmed >>> that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash >>> for it to be secure. This is a design flaw in ECDSA, which was fixed in newer >>> algorithms such as EdDSA and SM2 as those have a hash built-in to the signature >>> scheme. To mitigate it, the allowed hash algorithms must be restricted; in the >>> case of ECDSA, that means to the SHA family (preferably excluding SHA-1). >>> >>> akcipher_alg::verify doesn't actually know which hash algorithm is used, except >>> in the case of rsa-pkcs1pad where it is built into the name of the algorithm. >>> So it can't check the hash algorithm. I believe it needs to happen in >>> public_key_verify_signature() (and I'm working on a patch for that). >>> >>> Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design >>> that includes the hash into the signature algorithm. This means that it must be >>> used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but >>> SM2 will hash it again internally.) Currently, public_key_verify_signature() >>> allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and >>> IMA uses this. This is broken and must be removed, since it isn't actually the >>> SM2 algorithm as specified anymore, but rather some homebrew thing with unknown >>> security properties. (Well, I'm not confident about SM2, but homebrew is worse.) >>> >>> Adding fs-verity support to IMA also complicates things, as doing it naively >>> would introduce an ambiguity about what is signed. Naively, the *data* that is >>> signed (considering the hash as part of the signature algorithm) would be either >>> the whole file, in the case of traditional IMA, or the fsverity_descriptor >>> struct, in the case of IMA with fs-verity. However, a file could have contents >>> which match an fsverity_descriptor struct; that would create an ambiguity. >>> >>> Assuming that it needs to be allowed that the same key can sign files for both >>> traditional and fs-verity hashing, solving this problem will require a second >>> hash. The easiest way to do this would be sign/verify the following struct: >>> >>> struct ima_file_id { >>> u8 is_fsverity; >>> u8 hash_algorithm; >>> u8 hash[]; >>> }; >> >> To calrify, I suppose that for ECDSA NIST P256 you would allow pairing with >> any of the SHA family hashes (also as defined by the existing OIDs) and as >> the standard allows today? And the same then applies for NIST p384 etc.? >> >> Further, I suppose similar restriction would apply for ECRDSA to pair it >> with Streebog only, as Vitaly said. > I don't have any better ideas. > >> What's happening now is that to verify a signature, IMA/integrity subsystem >> fills out the following structure: >> >> struct public_key_signature pks; >> >> pks.hash_algo = hash_algo_name[hdr->hash_algo]; // name of hash algo will >> go into this here, e.g., 'sha256' >> pks.pkey_algo = pk->pkey_algo; // this is either 'rsa', 'ecdsa-', 'ecrdsa-' >> or 'sm2' string >> >> It then calls: >> >> ret = verify_signature(key, &pks); >> >> IMO, in the call path down this function the pairing of public key and hash >> algo would have to be enforced in order to enforce the standards. Would this >> not be sufficient to be able to stay with the standards ? > That sounds right, though there are a number of other issues including SM2 being > implemented incorrectly, the "encoding" string isn't validated, and it not being > enforced that public_key_signature::pkey_algo actually matches > public_key::pkey_algo. I don't know enough about SM2. Which call path are you looking at for "encoding" ? For IMA's signature verification with public keys we will necessarily get into: public_key_verfiy_signature: https://elixir.bootlin.com/linux/v5.14.21/source/crypto/asymmetric_keys/public_key.c#L311 sig->encoding is at least then used in software_key_determine_akcipher: https://elixir.bootlin.com/linux/v5.14.21/source/crypto/asymmetric_keys/public_key.c#L66 It doesn't *seem* to be used elsewhere down this call path. Is this not enough of looking at 'encoding' that is used to form the alg_name? Regarding matching of public_key_signature::pkey_algo and public_key::pkey_algo: What could be the implications of this not matching? Does it matter? Could one accidentally succeed in verifying a signature with the wrong type of key? As for the proposed patch. I would need to split this up into 3 patches with their corresponding fixes tag, either SM2 or ECDRSA in the first depending on which one is oldest. But not knowing about SM2 I would probably skip this one. > >> File hashes: IMA calculates the hash over a file itself by calling crypto >> functions, so at least the digest's bytes are trusted input in that respect >> and using the sha family type of hashes directly with ECDSA should work. >> Which algorithm IMA is supposed to use for the hashing is given in the xattr >> bytestream header. IMA could then take that type of hash, lookup the hash >> function, perform the hashing on the data, and let verify_signature enforce >> the pairing, rejecting file signatures with wrong pairing. This way the only >> thing that is needed is 'enforcement of pairing'. >> >> Fsverity: How much control does a user have over the hash family fsverity is >> using? Can IMA ECDSA/RSA users tell it to use a sha family hash and ECRDSA >> users make it use a Streebog hash so that also the pairing of hash and key >> type can work 'naturally' and we don't need the level of indirection via >> your structure above? > The hash algorithm used by fs-verity is configurable and is always returned > along with the file digest. Currently, only SHA-256 and SHA-512 are supported. > > Keep in mind that if you sign the fs-verity file digest directly with RSA, > ECDSA, or ECRDSA, the *data* you are actually signing is the fsverity_descriptor > -- the struct which the hash is a hash of. > > That creates an ambiguity when full file hashes are also signed by the same key, > as I previously mentioned. A level of indirection is needed to avoid that. > > In the naive method, the *data* being signed would also be different with SM2. > The level of indirection would avoid that. So in the fsverity case that level of indirection is needed, for the existing file signatures I don't think we need it. Stefan > > - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-15 6:21 ` Eric Biggers 2022-01-16 3:31 ` Stefan Berger @ 2022-01-16 17:01 ` Mimi Zohar 2022-01-19 0:39 ` Eric Biggers 1 sibling, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2022-01-16 17:01 UTC (permalink / raw) To: Eric Biggers, Vitaly Chikunov Cc: linux-integrity, linux-fscrypt, linux-kernel On Fri, 2022-01-14 at 22:21 -0800, Eric Biggers wrote: > On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: > > Eric, > > > > On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: > > > On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > > > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > > > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > > > > case IMA_VERITY_DIGSIG: > > > > > > > > - fallthrough; > > > > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > > > > + > > > > > > > > + /* > > > > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > > > > + * and the fs-verity file digest, not directly on the > > > > > > > > + * fs-verity file digest. Both digests should probably be > > > > > > > > + * included in the IMA measurement list, but for now this > > > > > > > > + * digest is only used for verifying the IMA signature. > > > > > > > > + */ > > > > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > > > > + iint->ima_hash->length); > > > > > > > > + > > > > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > > > > are the same. If they're not the same, for whatever reason, the > > > > > > signature verification would simply fail. > > > > > > > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > > > > between regular IMA file hash based signatures and fs-verity file > > > > > > digest based signatures. The digest field (d-ng) in the IMA > > > > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > > > > missing the reason for needing to hash fs-verity's file digest with > > > > > > other metadata, and sign that hash rather than fs-verity's file digest > > > > > > directly. > > > > > > > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > > > > hash value for all supported hash algorithms that produce the same length hash. > > > > > > > > Unless there is broken hash algorithm allowing for preimage attacks this > > > > is irrelevant. If there is two broken algorithms allowing for collisions, > > > > colliding hashes could be prepared even if algo id is hashed too. > > > > > > > > > > Only one algorithm needs to be broken. For example, SM3 has the same hash > > > length as SHA-256. If SM3 support were to be added to fs-verity, and if someone > > > were to find a way to find an input that has a specific SM3 digest, then they > > > could also make it match a specific SHA-256 digest. Someone might intend to > > > sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the > > > digest, then they would also be signing the corresponding SM3 digest. That's > > > why the digest that is signed *must* also include the algorithm used in the > > > digest (not the algorithm(s) used in the signature, which is different). > > > > I think it will be beneficial if we pass hash algo id to the > > akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. > > And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family > > hashes. > > > > I was going to reply to this thread again, but I got a bit distracted by > everything else being broken. Yes, the kernel needs to be restricting which > hash algorithms can be used with each public key algorithm, along the lines of > what you said. I asked the BoringSSL maintainers for advice, and they confirmed > that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash > for it to be secure. This is a design flaw in ECDSA, which was fixed in newer > algorithms such as EdDSA and SM2 as those have a hash built-in to the signature > scheme. To mitigate it, the allowed hash algorithms must be restricted; in the > case of ECDSA, that means to the SHA family (preferably excluding SHA-1). > > akcipher_alg::verify doesn't actually know which hash algorithm is used, except > in the case of rsa-pkcs1pad where it is built into the name of the algorithm. > So it can't check the hash algorithm. I believe it needs to happen in > public_key_verify_signature() (and I'm working on a patch for that). > > Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design > that includes the hash into the signature algorithm. This means that it must be > used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but > SM2 will hash it again internally.) Currently, public_key_verify_signature() > allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and > IMA uses this. This is broken and must be removed, since it isn't actually the > SM2 algorithm as specified anymore, but rather some homebrew thing with unknown > security properties. (Well, I'm not confident about SM2, but homebrew is worse.) > > Adding fs-verity support to IMA also complicates things, as doing it naively > would introduce an ambiguity about what is signed. Naively, the *data* that is > signed (considering the hash as part of the signature algorithm) would be either > the whole file, in the case of traditional IMA, or the fsverity_descriptor > struct, in the case of IMA with fs-verity. However, a file could have contents > which match an fsverity_descriptor struct; that would create an ambiguity. > > Assuming that it needs to be allowed that the same key can sign files for both > traditional and fs-verity hashing, solving this problem will require a second > hash. The IMA fs-verity policy rule could require specifying the hash algorithm. If it would require specifying a particular key as well, would hashing the hash then not be needed? > The easiest way to do this would be sign/verify the following struct: > struct ima_file_id { > u8 is_fsverity; > u8 hash_algorithm; > u8 hash[]; > }; > The v2 version of this patch introduces the "ima_tbs_hash" structure, which is more generic, since it uses the IMA xattr record type. Other than that, I don't see a difference. > This would be the *data* that is signed/verified -- meaning that it would be > hashed again as part of the signature algorithm (whether that hash is built-in > to the signature algorithm, as is the case for modern algorithms, or handled by > the caller as is the case for legacy algorithms). There seems to be an inconsistency, here, with what you said above, "... ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash for it to be secure." > Note that both traditional > and fs-verity hashes would need to use this same method for it to be secure; the > kernel must not accept signatures using the old method at the same time. The v2 version of this patch set signed the hash of a hash just for fs- verity signatures. Adding the equivalent support for regular file hashes will require the version in the IMA signature_v2_hdr to be incremented. If the version is incremented now, both signatures versions should then be able to co-exist. thanks, Mimi ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-16 17:01 ` Mimi Zohar @ 2022-01-19 0:39 ` Eric Biggers 2022-01-20 16:39 ` Mimi Zohar 0 siblings, 1 reply; 27+ messages in thread From: Eric Biggers @ 2022-01-19 0:39 UTC (permalink / raw) To: Mimi Zohar; +Cc: Vitaly Chikunov, linux-integrity, linux-fscrypt, linux-kernel On Sun, Jan 16, 2022 at 12:01:28PM -0500, Mimi Zohar wrote: > On Fri, 2022-01-14 at 22:21 -0800, Eric Biggers wrote: > > On Sat, Jan 15, 2022 at 08:31:01AM +0300, Vitaly Chikunov wrote: > > > Eric, > > > > > > On Sun, Jan 09, 2022 at 01:07:18PM -0800, Eric Biggers wrote: > > > > On Sun, Jan 09, 2022 at 11:45:37PM +0300, Vitaly Chikunov wrote: > > > > > On Wed, Jan 05, 2022 at 03:37:39PM -0800, Eric Biggers wrote: > > > > > > On Fri, Dec 31, 2021 at 10:35:00AM -0500, Mimi Zohar wrote: > > > > > > > On Thu, 2021-12-02 at 14:07 -0800, Eric Biggers wrote: > > > > > > > > On Thu, Dec 02, 2021 at 04:55:06PM -0500, Mimi Zohar wrote: > > > > > > > > > case IMA_VERITY_DIGSIG: > > > > > > > > > - fallthrough; > > > > > > > > > + set_bit(IMA_DIGSIG, &iint->atomic_flags); > > > > > > > > > + > > > > > > > > > + /* > > > > > > > > > + * The IMA signature is based on a hash of IMA_VERITY_DIGSIG > > > > > > > > > + * and the fs-verity file digest, not directly on the > > > > > > > > > + * fs-verity file digest. Both digests should probably be > > > > > > > > > + * included in the IMA measurement list, but for now this > > > > > > > > > + * digest is only used for verifying the IMA signature. > > > > > > > > > + */ > > > > > > > > > + verity_digest[0] = IMA_VERITY_DIGSIG; > > > > > > > > > + memcpy(verity_digest + 1, iint->ima_hash->digest, > > > > > > > > > + iint->ima_hash->length); > > > > > > > > > + > > > > > > > > > + hash.hdr.algo = iint->ima_hash->algo; > > > > > > > > > + hash.hdr.length = iint->ima_hash->length; > > > > > > > > > > > > > > > > This is still wrong because the bytes being signed don't include the hash > > > > > > > > algorithm. Unless you mean for it to be implicitly always SHA-256? fs-verity > > > > > > > > supports SHA-512 too, and it may support other hash algorithms in the future. > > > > > > > > > > > > > > IMA assumes that the file hash algorithm and the signature algorithm > > > > > > > are the same. If they're not the same, for whatever reason, the > > > > > > > signature verification would simply fail. > > > > > > > > > > > > > > Based on the v2 signature header 'type' field, IMA can differentiate > > > > > > > between regular IMA file hash based signatures and fs-verity file > > > > > > > digest based signatures. The digest field (d-ng) in the IMA > > > > > > > meausrement list prefixes the digest with the hash algorithm. I'm > > > > > > > missing the reason for needing to hash fs-verity's file digest with > > > > > > > other metadata, and sign that hash rather than fs-verity's file digest > > > > > > > directly. > > > > > > > > > > > > Because if someone signs a raw hash, then they also implicitly sign the same > > > > > > hash value for all supported hash algorithms that produce the same length hash. > > > > > > > > > > Unless there is broken hash algorithm allowing for preimage attacks this > > > > > is irrelevant. If there is two broken algorithms allowing for collisions, > > > > > colliding hashes could be prepared even if algo id is hashed too. > > > > > > > > > > > > > Only one algorithm needs to be broken. For example, SM3 has the same hash > > > > length as SHA-256. If SM3 support were to be added to fs-verity, and if someone > > > > were to find a way to find an input that has a specific SM3 digest, then they > > > > could also make it match a specific SHA-256 digest. Someone might intend to > > > > sign a SHA-256 digest, but if they are only signing the raw 32 bytes of the > > > > digest, then they would also be signing the corresponding SM3 digest. That's > > > > why the digest that is signed *must* also include the algorithm used in the > > > > digest (not the algorithm(s) used in the signature, which is different). > > > > > > I think it will be beneficial if we pass hash algo id to the > > > akcipher_alg::verify. In fact, ecrdsa should only be used with streebog. > > > And perhaps, sm2 with sm3, pkcs1 with md/sha/sm3, and ecdsa with sha family > > > hashes. > > > > > > > I was going to reply to this thread again, but I got a bit distracted by > > everything else being broken. Yes, the kernel needs to be restricting which > > hash algorithms can be used with each public key algorithm, along the lines of > > what you said. I asked the BoringSSL maintainers for advice, and they confirmed > > that ECDSA just signs/verifies a raw hash, and in fact it *must* be a raw hash > > for it to be secure. This is a design flaw in ECDSA, which was fixed in newer > > algorithms such as EdDSA and SM2 as those have a hash built-in to the signature > > scheme. To mitigate it, the allowed hash algorithms must be restricted; in the > > case of ECDSA, that means to the SHA family (preferably excluding SHA-1). > > > > akcipher_alg::verify doesn't actually know which hash algorithm is used, except > > in the case of rsa-pkcs1pad where it is built into the name of the algorithm. > > So it can't check the hash algorithm. I believe it needs to happen in > > public_key_verify_signature() (and I'm working on a patch for that). > > > > Now, SM2 is different from ECDSA and ECRDSA in that it uses the modern design > > that includes the hash into the signature algorithm. This means that it must be > > used to sign/verify *data*, not a hash. (Well, you can sign/verify a hash, but > > SM2 will hash it again internally.) Currently, public_key_verify_signature() > > allows SM2 to be used to sign/verify a hash, skipping the SM2 internal hash, and > > IMA uses this. This is broken and must be removed, since it isn't actually the > > SM2 algorithm as specified anymore, but rather some homebrew thing with unknown > > security properties. (Well, I'm not confident about SM2, but homebrew is worse.) > > > > Adding fs-verity support to IMA also complicates things, as doing it naively > > would introduce an ambiguity about what is signed. Naively, the *data* that is > > signed (considering the hash as part of the signature algorithm) would be either > > the whole file, in the case of traditional IMA, or the fsverity_descriptor > > struct, in the case of IMA with fs-verity. However, a file could have contents > > which match an fsverity_descriptor struct; that would create an ambiguity. > > > > Assuming that it needs to be allowed that the same key can sign files for both > > traditional and fs-verity hashing, solving this problem will require a second > > hash. > > The IMA fs-verity policy rule could require specifying the hash > algorithm. If it would require specifying a particular key as well, > would hashing the hash then not be needed? If both of those were true, and if the key was never reused for other purposes (either for other hash algorithms or for full file hashes), that would be fine. Obviously, the IMA policy is trusted in this context, since it's what tells the kernel to verify signatures in the first place. But, I could see users getting this wrong and reusing keys. > > > The easiest way to do this would be sign/verify the following struct: > > struct ima_file_id { > > u8 is_fsverity; > > u8 hash_algorithm; > > u8 hash[]; > > }; > > > > The v2 version of this patch introduces the "ima_tbs_hash" structure, > which is more generic, since it uses the IMA xattr record type. Other > than that, I don't see a difference. > > > This would be the *data* that is signed/verified -- meaning that it would be > > hashed again as part of the signature algorithm (whether that hash is built-in > > to the signature algorithm, as is the case for modern algorithms, or handled by > > the caller as is the case for legacy algorithms). > > There seems to be an inconsistency, here, with what you said above, > "... ECDSA just signs/verifies a raw hash, and in fact it *must* be a > raw hash for it to be secure." There isn't an inconsistency. ECDSA is among the algorithms where the caller is expected to handle the hash. It is confusing dealing with all these different signature algorithms. I think the right way to think about this is in terms of what *data* is being signed/verified. Currently the data is the full file contents. I think it needs to be made into an annotated hash, e.g. the struct I gave. public_key_verify_signature() also needs to be fixed to support both types of signature algorithms (caller-provided hash and internal hash) in a logical way. Originally it only supported caller-provided hashes, but then SM2 support was added and now it is super broken. > > Note that both traditional > > and fs-verity hashes would need to use this same method for it to be secure; the > > kernel must not accept signatures using the old method at the same time. > > The v2 version of this patch set signed the hash of a hash just for fs- > verity signatures. Adding the equivalent support for regular file > hashes will require the version in the IMA signature_v2_hdr to be > incremented. If the version is incremented now, both signatures > versions should then be able to co-exist. That seems like a good thing, unless you want users to be responsible for only ever signing full file hashes *or* fs-verity file hashes with each key. That seems like something that users will get wrong. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-19 0:39 ` Eric Biggers @ 2022-01-20 16:39 ` Mimi Zohar 2022-01-20 21:05 ` Eric Biggers 0 siblings, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2022-01-20 16:39 UTC (permalink / raw) To: Eric Biggers, Vitaly Chikunov Cc: linux-integrity, linux-fscrypt, linux-kernel Eric, Vitaly, On Tue, 2022-01-18 at 16:39 -0800, Eric Biggers wrote: > > > The easiest way to do this would be sign/verify the following struct: > > > struct ima_file_id { > > > u8 is_fsverity; > > > u8 hash_algorithm; > > > u8 hash[]; > > > }; > > This would be the *data* that is signed/verified -- meaning that it would be > > > hashed again as part of the signature algorithm (whether that hash is built-in > > > to the signature algorithm, as is the case for modern algorithms, or handled by > > > the caller as is the case for legacy algorithms). > > > > There seems to be an inconsistency, here, with what you said above, > > "... ECDSA just signs/verifies a raw hash, and in fact it *must* be a > > raw hash for it to be secure." > > There isn't an inconsistency. ECDSA is among the algorithms where the caller is > expected to handle the hash. > > It is confusing dealing with all these different signature algorithms. I think > the right way to think about this is in terms of what *data* is being > signed/verified. Currently the data is the full file contents. I think it > needs to be made into an annotated hash, e.g. the struct I gave. > > public_key_verify_signature() also needs to be fixed to support both types of > signature algorithms (caller-provided hash and internal hash) in a logical way. > Originally it only supported caller-provided hashes, but then SM2 support was > added and now it is super broken. Eric, did you say you're working on fixes to address these problems? > > > > Note that both traditional > > > and fs-verity hashes would need to use this same method for it to be secure; the > > > kernel must not accept signatures using the old method at the same time. > > > > The v2 version of this patch set signed the hash of a hash just for fs- > > verity signatures. Adding the equivalent support for regular file > > hashes will require the version in the IMA signature_v2_hdr to be > > incremented. If the version is incremented now, both signatures > > versions should then be able to co-exist. > > That seems like a good thing, unless you want users to be responsible for only > ever signing full file hashes *or* fs-verity file hashes with each key. That > seems like something that users will get wrong. Instead of using a flexible array, Vitaly suggested defining the hash as FS_VERITY_MAX_DIGEST_SIZE, so that it could be allocated temporarily on stack instead of kalloc. As the above struct is not limited to fsverity, we could use MAX_HASH_DIGESTSIZE, if it was exported, but it isn't. Would the following work for you? /* * IMA signature header version 3 disambiguates the data that is signed by * indirectly signing the hash of this structure, containing either the * fsverity_descriptor struct digest or, in the future, the traditional IMA * file hash. */ struct ima_file_id { __u8 is_fsverity; /* set to 1 for IMA_VERITY_DIGSIG */ __u8 hash_algorithm; /* Digest algorithm [enum hash_algo] */ #ifdef __KERNEL__ __u8 hash[HASH_MAX_DIGESTSIZE]; #else __u8 hash[]; #endif }; thanks, Mimi ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v1 4/5] ima: support fs-verity file digest based signatures 2022-01-20 16:39 ` Mimi Zohar @ 2022-01-20 21:05 ` Eric Biggers 0 siblings, 0 replies; 27+ messages in thread From: Eric Biggers @ 2022-01-20 21:05 UTC (permalink / raw) To: Mimi Zohar; +Cc: Vitaly Chikunov, linux-integrity, linux-fscrypt, linux-kernel On Thu, Jan 20, 2022 at 11:39:56AM -0500, Mimi Zohar wrote: > Eric, Vitaly, > > On Tue, 2022-01-18 at 16:39 -0800, Eric Biggers wrote: > > > > > The easiest way to do this would be sign/verify the following struct: > > > > struct ima_file_id { > > > > u8 is_fsverity; > > > > u8 hash_algorithm; > > > > u8 hash[]; > > > > }; > > > > > This would be the *data* that is signed/verified -- meaning that it > would be > > > > hashed again as part of the signature algorithm (whether that hash is built-in > > > > to the signature algorithm, as is the case for modern algorithms, or handled by > > > > the caller as is the case for legacy algorithms). > > > > > > There seems to be an inconsistency, here, with what you said above, > > > "... ECDSA just signs/verifies a raw hash, and in fact it *must* be a > > > raw hash for it to be secure." > > > > There isn't an inconsistency. ECDSA is among the algorithms where the caller is > > expected to handle the hash. > > > > It is confusing dealing with all these different signature algorithms. I think > > the right way to think about this is in terms of what *data* is being > > signed/verified. Currently the data is the full file contents. I think it > > needs to be made into an annotated hash, e.g. the struct I gave. > > > > public_key_verify_signature() also needs to be fixed to support both types of > > signature algorithms (caller-provided hash and internal hash) in a logical way. > > Originally it only supported caller-provided hashes, but then SM2 support was > > added and now it is super broken. > > Eric, did you say you're working on fixes to address these problems? Yes, I was working on some patches at https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=wip-keys-fixes but got distracted by other things, as well as finding the problems with SM2 which will take some time to decide what to do with. I'll try to get some patches ready, but there are a lot of things to fix. > > Instead of using a flexible array, Vitaly suggested defining the hash > as FS_VERITY_MAX_DIGEST_SIZE, so that it could be allocated temporarily > on stack > instead of kalloc. > > As the above struct is not limited to fsverity, we could use > MAX_HASH_DIGESTSIZE, if it was exported, but it isn't. Would the > following work for you? > > /* > * IMA signature header version 3 disambiguates the data that is signed > by > * indirectly signing the hash of this structure, containing either the > * fsverity_descriptor struct digest or, in the future, the traditional > IMA > * file hash. > */ > struct ima_file_id { > __u8 is_fsverity; /* set to 1 for IMA_VERITY_DIGSIG */ > __u8 hash_algorithm; /* Digest algorithm [enum hash_algo] */ > #ifdef __KERNEL__ > __u8 hash[HASH_MAX_DIGESTSIZE]; > #else > __u8 hash[]; > #endif > }; You could certainly declare a fixed-length struct, but only sign/verify the used portion of it. The fixed-length struct would just be an implementation detail. Alternatively, you could always sign/verify a fixed-length struct, with any shorter hashes zero-padded to HASH_MAX_DIGESTSIZE (64 bytes). This would be fine if you're confident that hashes longer than 64 bytes will never be used. (They don't make sense cryptographically, but who knows.) For future extensibility, you might want to call the 'is_fsverity' field something like 'hash_type', so it would be like an enum rather than a bool. I just used 'is_fsverity' as a minimal example. - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v1 5/5] fsverity: update the documentation 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar ` (3 preceding siblings ...) 2021-12-02 21:55 ` [PATCH v1 4/5] ima: support fs-verity file digest based signatures Mimi Zohar @ 2021-12-02 21:55 ` Mimi Zohar 2021-12-02 22:09 ` Eric Biggers 4 siblings, 1 reply; 27+ messages in thread From: Mimi Zohar @ 2021-12-02 21:55 UTC (permalink / raw) To: linux-integrity; +Cc: Mimi Zohar, Eric Biggers, linux-fscrypt, linux-kernel Update the fsverity documentation related to IMA signature support. Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- Documentation/filesystems/fsverity.rst | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst index 1d831e3cbcb3..c71f6e365df5 100644 --- a/Documentation/filesystems/fsverity.rst +++ b/Documentation/filesystems/fsverity.rst @@ -74,8 +74,12 @@ authenticating the files is up to userspace. However, to meet some users' needs, fs-verity optionally supports a simple signature verification mechanism where users can configure the kernel to require that all fs-verity files be signed by a key loaded into a keyring; see -`Built-in signature verification`_. Support for fs-verity file hashes -in IMA (Integrity Measurement Architecture) policies is also planned. +`Built-in signature verification`_. + +IMA (Integrity Measurement Architecture) supports fs-verity file +digests based signatures stored as security.ima xattrs, which are +identified by the signature type IMA_VERITY_DIGSIG. + User API ======== @@ -653,13 +657,13 @@ weren't already directly answered in other parts of this document. hashed and what to do with those hashes, such as log them, authenticate them, or add them to a measurement list. - IMA is planned to support the fs-verity hashing mechanism as an - alternative to doing full file hashes, for people who want the - performance and security benefits of the Merkle tree based hash. - But it doesn't make sense to force all uses of fs-verity to be - through IMA. As a standalone filesystem feature, fs-verity - already meets many users' needs, and it's testable like other - filesystem features e.g. with xfstests. + IMA supports the fs-verity hashing mechanism as an alternative + to doing full file hashes, for people who want the performance + and security benefits of the Merkle tree based hash. But it + doesn't make sense to force all uses of fs-verity to be through + IMA. As a standalone filesystem feature, fs-verity already meets + many users' needs, and it's testable like other filesystem + features e.g. with xfstests. :Q: Isn't fs-verity useless because the attacker can just modify the hashes in the Merkle tree, which is stored on-disk? -- 2.27.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH v1 5/5] fsverity: update the documentation 2021-12-02 21:55 ` [PATCH v1 5/5] fsverity: update the documentation Mimi Zohar @ 2021-12-02 22:09 ` Eric Biggers 0 siblings, 0 replies; 27+ messages in thread From: Eric Biggers @ 2021-12-02 22:09 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity, linux-fscrypt, linux-kernel On Thu, Dec 02, 2021 at 04:55:07PM -0500, Mimi Zohar wrote: > Update the fsverity documentation related to IMA signature support. > > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> > --- > Documentation/filesystems/fsverity.rst | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst > index 1d831e3cbcb3..c71f6e365df5 100644 > --- a/Documentation/filesystems/fsverity.rst > +++ b/Documentation/filesystems/fsverity.rst > @@ -74,8 +74,12 @@ authenticating the files is up to userspace. However, to meet some > users' needs, fs-verity optionally supports a simple signature > verification mechanism where users can configure the kernel to require > that all fs-verity files be signed by a key loaded into a keyring; see > -`Built-in signature verification`_. Support for fs-verity file hashes > -in IMA (Integrity Measurement Architecture) policies is also planned. > +`Built-in signature verification`_. > + > +IMA (Integrity Measurement Architecture) supports fs-verity file > +digests based signatures stored as security.ima xattrs, which are > +identified by the signature type IMA_VERITY_DIGSIG. > + > > User API > ======== > @@ -653,13 +657,13 @@ weren't already directly answered in other parts of this document. > hashed and what to do with those hashes, such as log them, > authenticate them, or add them to a measurement list. > > - IMA is planned to support the fs-verity hashing mechanism as an > - alternative to doing full file hashes, for people who want the > - performance and security benefits of the Merkle tree based hash. > - But it doesn't make sense to force all uses of fs-verity to be > - through IMA. As a standalone filesystem feature, fs-verity > - already meets many users' needs, and it's testable like other > - filesystem features e.g. with xfstests. > + IMA supports the fs-verity hashing mechanism as an alternative > + to doing full file hashes, for people who want the performance > + and security benefits of the Merkle tree based hash. But it > + doesn't make sense to force all uses of fs-verity to be through > + IMA. As a standalone filesystem feature, fs-verity already meets > + many users' needs, and it's testable like other filesystem > + features e.g. with xfstests. > Isn't there IMA documentation that can be linked to? - Eric ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2022-01-20 21:05 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-02 21:55 [PATCH v1 0/5] ima: support fs-verity signatures stored as Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 1/5] fs-verity: define a function to return the integrity protected file digest Mimi Zohar 2021-12-02 22:15 ` Eric Biggers 2021-12-02 21:55 ` [PATCH v1 2/5] ima: define a new signature type named IMA_VERITY_DIGSIG Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 3/5] ima: limit including fs-verity's file digest in measurement list Mimi Zohar 2021-12-02 22:22 ` Eric Biggers 2021-12-02 22:55 ` Mimi Zohar 2021-12-02 21:55 ` [PATCH v1 4/5] ima: support fs-verity file digest based signatures Mimi Zohar 2021-12-02 22:07 ` Eric Biggers 2021-12-02 22:13 ` Mimi Zohar 2021-12-02 22:18 ` Eric Biggers 2021-12-31 15:35 ` Mimi Zohar 2022-01-05 23:37 ` Eric Biggers 2022-01-09 20:45 ` Vitaly Chikunov 2022-01-09 21:07 ` Eric Biggers 2022-01-15 5:31 ` Vitaly Chikunov 2022-01-15 6:21 ` Eric Biggers 2022-01-16 3:31 ` Stefan Berger 2022-01-16 5:24 ` Stefan Berger 2022-01-19 0:49 ` Eric Biggers 2022-01-19 15:41 ` Stefan Berger 2022-01-16 17:01 ` Mimi Zohar 2022-01-19 0:39 ` Eric Biggers 2022-01-20 16:39 ` Mimi Zohar 2022-01-20 21:05 ` Eric Biggers 2021-12-02 21:55 ` [PATCH v1 5/5] fsverity: update the documentation Mimi Zohar 2021-12-02 22:09 ` Eric Biggers
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.