| Message ID | 1675119451-23180-11-git-send-email-wufan@linux.microsoft.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | Integrity Policy Enforcement LSM (IPE) | expand |
On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote: > From: Deven Bowers <deven.desai@linux.microsoft.com> > > dm-verity provides a strong guarantee of a block device's integrity. As > a generic way to check the integrity of a block device, it provides > those integrity guarantees to its higher layers, including the filesystem > level. I think you could reuse most of is_trusted_verity_target(), in particular dm_verity_get_root_digest(). And probably, the previous patch is not necessary. Roberto > An LSM that control access to a resource on the system based on the > available integrity claims can use this transitive property of > dm-verity, by querying the underlying block_device of a particular > file. > > The digest and signature information need to be stored in the block > device to fulfill the next requirement of authorization via LSM policy. > This will enable the LSM to perform revocation of devices that are still > mounted, prohibiting execution of files that are no longer authorized > by the LSM in question. > > Signed-off-by: Deven Bowers <deven.desai@linux.microsoft.com> > Signed-off-by: Fan Wu <wufan@linux.microsoft.com> > --- > v2: > + No Changes > > v3: > + No changes > > v4: > + No changes > > v5: > + No changes > > v6: > + Fix an improper cleanup that can result in > a leak > > v7: > + Squash patch 08/12, 10/12 to [11/16] > + Use part0 for block_device, to retrieve the block_device, when > calling security_bdev_setsecurity > > v8: > + Undo squash of 08/12, 10/12 - separating drivers/md/ from > security/ & block/ > + Use common-audit function for dmverity_signature. > + Change implementation for storing the dm-verity digest to use the > newly introduced dm_verity_digest structure introduced in patch > 14/20. > + Create new structure, dm_verity_digest, containing digest algorithm, > size, and digest itself to pass to the LSM layer. V7 was missing the > algorithm. > + Create an associated public header containing this new structure and > the key values for the LSM hook, specific to dm-verity. > + Additional information added to commit, discussing the layering of > the changes and how the information passed will be used. > > v9: > + No changes > --- > drivers/md/dm-verity-target.c | 25 +++++++++++++++++++++++-- > drivers/md/dm-verity-verify-sig.c | 16 +++++++++++++--- > drivers/md/dm-verity-verify-sig.h | 10 ++++++---- > include/linux/dm-verity.h | 19 +++++++++++++++++++ > 4 files changed, 61 insertions(+), 9 deletions(-) > create mode 100644 include/linux/dm-verity.h > > diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c > index ccf5b852fbf7..afea61eed4ec 100644 > --- a/drivers/md/dm-verity-target.c > +++ b/drivers/md/dm-verity-target.c > @@ -13,6 +13,7 @@ > * access behavior. > */ > > +#include "dm-core.h" > #include "dm-verity.h" > #include "dm-verity-fec.h" > #include "dm-verity-verify-sig.h" > @@ -21,6 +22,9 @@ > #include <linux/scatterlist.h> > #include <linux/string.h> > #include <linux/jump_label.h> > +#include <linux/security.h> > +#include <linux/dm-verity.h> > +#include <crypto/hash_info.h> > > #define DM_MSG_PREFIX "verity" > > @@ -1169,6 +1173,8 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) > sector_t hash_position; > char dummy; > char *root_hash_digest_to_validate; > + struct block_device *bdev; > + struct dm_verity_digest root_digest; > > v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL); > if (!v) { > @@ -1211,6 +1217,13 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) > } > v->version = num; > > + bdev = dm_table_get_md(ti->table)->disk->part0; > + if (!bdev) { > + ti->error = "Mapped device lookup failed"; > + r = -ENOMEM; > + goto bad; > + } > + > r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev); > if (r) { > ti->error = "Data device lookup failed"; > @@ -1343,7 +1356,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) > } > > /* Root hash signature is a optional parameter*/ > - r = verity_verify_root_hash(root_hash_digest_to_validate, > + r = verity_verify_root_hash(bdev, root_hash_digest_to_validate, > strlen(root_hash_digest_to_validate), > verify_args.sig, > verify_args.sig_size); > @@ -1428,12 +1441,20 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) > ti->per_io_data_size = roundup(ti->per_io_data_size, > __alignof__(struct dm_verity_io)); > > + root_digest.digest = v->root_digest; > + root_digest.digest_len = v->digest_size; > + root_digest.algo = v->alg_name; > + > + r = security_bdev_setsecurity(bdev, DM_VERITY_ROOTHASH_SEC_NAME, &root_digest, > + sizeof(root_digest)); > + if (r) > + goto bad; > + > verity_verify_sig_opts_cleanup(&verify_args); > > return 0; > > bad: > - > verity_verify_sig_opts_cleanup(&verify_args); > verity_dtr(ti); > > diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c > index db61a1f43ae9..5a73b91157d5 100644 > --- a/drivers/md/dm-verity-verify-sig.c > +++ b/drivers/md/dm-verity-verify-sig.c > @@ -9,6 +9,9 @@ > #include <linux/verification.h> > #include <keys/user-type.h> > #include <linux/module.h> > +#include <linux/security.h> > +#include <linux/dm-verity.h> > +#include "dm-core.h" > #include "dm-verity.h" > #include "dm-verity-verify-sig.h" > > @@ -97,14 +100,17 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, > * verify_verify_roothash - Verify the root hash of the verity hash device > * using builtin trusted keys. > * > + * @bdev: block_device representing the device-mapper created block device. > + * Used by the security hook, to set information about the block_device. > * @root_hash: For verity, the roothash/data to be verified. > * @root_hash_len: Size of the roothash/data to be verified. > * @sig_data: The trusted signature that verifies the roothash/data. > * @sig_len: Size of the signature. > * > */ > -int verity_verify_root_hash(const void *root_hash, size_t root_hash_len, > - const void *sig_data, size_t sig_len) > +int verity_verify_root_hash(struct block_device *bdev, const void *root_hash, > + size_t root_hash_len, const void *sig_data, > + size_t sig_len) > { > int ret; > > @@ -126,8 +132,12 @@ int verity_verify_root_hash(const void *root_hash, size_t root_hash_len, > NULL, > #endif > VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL); > + if (ret) > + return ret; > > - return ret; > + return security_bdev_setsecurity(bdev, > + DM_VERITY_SIGNATURE_SEC_NAME, > + sig_data, sig_len); > } > > void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts) > diff --git a/drivers/md/dm-verity-verify-sig.h b/drivers/md/dm-verity-verify-sig.h > index 3987c7141f79..31692fff92e4 100644 > --- a/drivers/md/dm-verity-verify-sig.h > +++ b/drivers/md/dm-verity-verify-sig.h > @@ -20,8 +20,9 @@ struct dm_verity_sig_opts { > > #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 2 > > -int verity_verify_root_hash(const void *data, size_t data_len, > - const void *sig_data, size_t sig_len); > +int verity_verify_root_hash(struct block_device *bdev, const void *data, > + size_t data_len, const void *sig_data, > + size_t sig_len); > bool verity_verify_is_sig_opt_arg(const char *arg_name); > > int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, > @@ -34,8 +35,9 @@ void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts); > > #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 0 > > -static inline int verity_verify_root_hash(const void *data, size_t data_len, > - const void *sig_data, size_t sig_len) > +int verity_verify_root_hash(struct block_device *bdev, const void *data, > + size_t data_len, const void *sig_data, > + size_t sig_len) > { > return 0; > } > diff --git a/include/linux/dm-verity.h b/include/linux/dm-verity.h > new file mode 100644 > index 000000000000..bb0413d55d72 > --- /dev/null > +++ b/include/linux/dm-verity.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +#ifndef _LINUX_DM_VERITY_H > +#define _LINUX_DM_VERITY_H > + > +#include <linux/types.h> > +#include <crypto/hash_info.h> > +#include <linux/device-mapper.h> > + > +struct dm_verity_digest { > + const char *algo; > + const u8 *digest; > + size_t digest_len; > +}; > + > +#define DM_VERITY_SIGNATURE_SEC_NAME DM_NAME ".verity-signature" > +#define DM_VERITY_ROOTHASH_SEC_NAME DM_NAME ".verity-roothash" > + > +#endif /* _LINUX_DM_VERITY_H */
On Tue, Jan 31, 2023 at 02:22:01PM +0100, Roberto Sassu wrote: > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote: > > From: Deven Bowers <deven.desai@linux.microsoft.com> > > > > dm-verity provides a strong guarantee of a block device's integrity. As > > a generic way to check the integrity of a block device, it provides > > those integrity guarantees to its higher layers, including the filesystem > > level. > > I think you could reuse most of is_trusted_verity_target(), in > particular dm_verity_get_root_digest(). > > And probably, the previous patch is not necessary. > > Roberto > Thanks for the info. This function seems could be used to get the roothash but for saving the signature we still need the hook function in the previous patch. -Fan
On Wed, 2023-02-01 at 15:26 -0800, Fan Wu wrote: > On Tue, Jan 31, 2023 at 02:22:01PM +0100, Roberto Sassu wrote: > > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote: > > > From: Deven Bowers <deven.desai@linux.microsoft.com> > > > > > > dm-verity provides a strong guarantee of a block device's integrity. As > > > a generic way to check the integrity of a block device, it provides > > > those integrity guarantees to its higher layers, including the filesystem > > > level. > > > > I think you could reuse most of is_trusted_verity_target(), in > > particular dm_verity_get_root_digest(). > > > > And probably, the previous patch is not necessary. > > > > Roberto > > > Thanks for the info. This function seems could be used to get the roothash > but for saving the signature we still need the hook function in the previous > patch. Uhm, look at the LoadPin case. It does not need to temporarily store the root digest in a security blob. It evaluates it directly. Well, ok, dm_verity_loadpin_is_bdev_trusted() looks for trusted digests in the dm_verity_loadpin_trusted_root_digests list. So, something equivalent needs to be made for IPE (or you just get the digest). However, I find not introducing new hooks and evaluating the information directly more efficient. Roberto
On Thu, Feb 02, 2023 at 09:21:24AM +0100, Roberto Sassu wrote: > On Wed, 2023-02-01 at 15:26 -0800, Fan Wu wrote: > > On Tue, Jan 31, 2023 at 02:22:01PM +0100, Roberto Sassu wrote: > > > On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote: > > > > From: Deven Bowers <deven.desai@linux.microsoft.com> > > > > > > > > dm-verity provides a strong guarantee of a block device's integrity. As > > > > a generic way to check the integrity of a block device, it provides > > > > those integrity guarantees to its higher layers, including the filesystem > > > > level. > > > > > > I think you could reuse most of is_trusted_verity_target(), in > > > particular dm_verity_get_root_digest(). > > > > > > And probably, the previous patch is not necessary. > > > > > > Roberto > > > > > Thanks for the info. This function seems could be used to get the roothash > > but for saving the signature we still need the hook function in the previous > > patch. > > Uhm, look at the LoadPin case. It does not need to temporarily store > the root digest in a security blob. It evaluates it directly. > > Well, ok, dm_verity_loadpin_is_bdev_trusted() looks for trusted digests > in the dm_verity_loadpin_trusted_root_digests list. So, something > equivalent needs to be made for IPE (or you just get the digest). > However, I find not introducing new hooks and evaluating the > information directly more efficient. > > Roberto Thanks for the input. I did a deeper dive into the source code and did some experiements, my conclusion is the hook is still the preferred way for us. For the root digest part, dm_verity_loadpin_is_bdev_trusted is able to query the root digest is because the root digest is saved in struct dm_verity. Specifically it will call dm_verity_get_root_digest to kmemdup the digest. If every binary execution will trigger a kmemdup to copy a digest, the overhead will be noticeable. Using a hook can let us copy the root digest exactly once when the block device is created and free the copied digest when the block device is unmounted. For the signature, it is currently an optional parameter and it is not saved in struct dm_verity. But even if we let struct dm_verity saves the signature it will still have the kmemdup problem above. So using the hook will be the cleanest way. -Fan
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index ccf5b852fbf7..afea61eed4ec 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -13,6 +13,7 @@ * access behavior. */ +#include "dm-core.h" #include "dm-verity.h" #include "dm-verity-fec.h" #include "dm-verity-verify-sig.h" @@ -21,6 +22,9 @@ #include <linux/scatterlist.h> #include <linux/string.h> #include <linux/jump_label.h> +#include <linux/security.h> +#include <linux/dm-verity.h> +#include <crypto/hash_info.h> #define DM_MSG_PREFIX "verity" @@ -1169,6 +1173,8 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) sector_t hash_position; char dummy; char *root_hash_digest_to_validate; + struct block_device *bdev; + struct dm_verity_digest root_digest; v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL); if (!v) { @@ -1211,6 +1217,13 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) } v->version = num; + bdev = dm_table_get_md(ti->table)->disk->part0; + if (!bdev) { + ti->error = "Mapped device lookup failed"; + r = -ENOMEM; + goto bad; + } + r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev); if (r) { ti->error = "Data device lookup failed"; @@ -1343,7 +1356,7 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) } /* Root hash signature is a optional parameter*/ - r = verity_verify_root_hash(root_hash_digest_to_validate, + r = verity_verify_root_hash(bdev, root_hash_digest_to_validate, strlen(root_hash_digest_to_validate), verify_args.sig, verify_args.sig_size); @@ -1428,12 +1441,20 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv) ti->per_io_data_size = roundup(ti->per_io_data_size, __alignof__(struct dm_verity_io)); + root_digest.digest = v->root_digest; + root_digest.digest_len = v->digest_size; + root_digest.algo = v->alg_name; + + r = security_bdev_setsecurity(bdev, DM_VERITY_ROOTHASH_SEC_NAME, &root_digest, + sizeof(root_digest)); + if (r) + goto bad; + verity_verify_sig_opts_cleanup(&verify_args); return 0; bad: - verity_verify_sig_opts_cleanup(&verify_args); verity_dtr(ti); diff --git a/drivers/md/dm-verity-verify-sig.c b/drivers/md/dm-verity-verify-sig.c index db61a1f43ae9..5a73b91157d5 100644 --- a/drivers/md/dm-verity-verify-sig.c +++ b/drivers/md/dm-verity-verify-sig.c @@ -9,6 +9,9 @@ #include <linux/verification.h> #include <keys/user-type.h> #include <linux/module.h> +#include <linux/security.h> +#include <linux/dm-verity.h> +#include "dm-core.h" #include "dm-verity.h" #include "dm-verity-verify-sig.h" @@ -97,14 +100,17 @@ int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, * verify_verify_roothash - Verify the root hash of the verity hash device * using builtin trusted keys. * + * @bdev: block_device representing the device-mapper created block device. + * Used by the security hook, to set information about the block_device. * @root_hash: For verity, the roothash/data to be verified. * @root_hash_len: Size of the roothash/data to be verified. * @sig_data: The trusted signature that verifies the roothash/data. * @sig_len: Size of the signature. * */ -int verity_verify_root_hash(const void *root_hash, size_t root_hash_len, - const void *sig_data, size_t sig_len) +int verity_verify_root_hash(struct block_device *bdev, const void *root_hash, + size_t root_hash_len, const void *sig_data, + size_t sig_len) { int ret; @@ -126,8 +132,12 @@ int verity_verify_root_hash(const void *root_hash, size_t root_hash_len, NULL, #endif VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL); + if (ret) + return ret; - return ret; + return security_bdev_setsecurity(bdev, + DM_VERITY_SIGNATURE_SEC_NAME, + sig_data, sig_len); } void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts) diff --git a/drivers/md/dm-verity-verify-sig.h b/drivers/md/dm-verity-verify-sig.h index 3987c7141f79..31692fff92e4 100644 --- a/drivers/md/dm-verity-verify-sig.h +++ b/drivers/md/dm-verity-verify-sig.h @@ -20,8 +20,9 @@ struct dm_verity_sig_opts { #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 2 -int verity_verify_root_hash(const void *data, size_t data_len, - const void *sig_data, size_t sig_len); +int verity_verify_root_hash(struct block_device *bdev, const void *data, + size_t data_len, const void *sig_data, + size_t sig_len); bool verity_verify_is_sig_opt_arg(const char *arg_name); int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v, @@ -34,8 +35,9 @@ void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts); #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 0 -static inline int verity_verify_root_hash(const void *data, size_t data_len, - const void *sig_data, size_t sig_len) +int verity_verify_root_hash(struct block_device *bdev, const void *data, + size_t data_len, const void *sig_data, + size_t sig_len) { return 0; } diff --git a/include/linux/dm-verity.h b/include/linux/dm-verity.h new file mode 100644 index 000000000000..bb0413d55d72 --- /dev/null +++ b/include/linux/dm-verity.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef _LINUX_DM_VERITY_H +#define _LINUX_DM_VERITY_H + +#include <linux/types.h> +#include <crypto/hash_info.h> +#include <linux/device-mapper.h> + +struct dm_verity_digest { + const char *algo; + const u8 *digest; + size_t digest_len; +}; + +#define DM_VERITY_SIGNATURE_SEC_NAME DM_NAME ".verity-signature" +#define DM_VERITY_ROOTHASH_SEC_NAME DM_NAME ".verity-roothash" + +#endif /* _LINUX_DM_VERITY_H */