| Message ID | 20220106203436.281629-3-public@thson.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dm ima: allow targets to remeasure their state | expand |
Hi Thore, On 1/6/2022 12:34 PM, Thore Sommer wrote: > On first corruption the verity targets triggers a dm_target_update event. > This allows other systems to check using IMA if the state of the device is > still trustworthy via remote attestation. In the patch description please state the existing problem (or, limitation in the existing implementation) and then describe how the proposed change addresses the issue. > > Signed-off-by: Thore Sommer <public@thson.de> > --- > drivers/md/dm-verity-target.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c > index 80133aae0db3..09696e25bf1c 100644 > --- a/drivers/md/dm-verity-target.c > +++ b/drivers/md/dm-verity-target.c > @@ -16,6 +16,7 @@ > #include "dm-verity.h" > #include "dm-verity-fec.h" > #include "dm-verity-verify-sig.h" > +#include "dm-ima.h" > #include <linux/module.h> > #include <linux/reboot.h> > #include <linux/scatterlist.h> > @@ -218,10 +219,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type, > char *envp[] = { verity_env, NULL }; > const char *type_str = ""; > struct mapped_device *md = dm_table_get_md(v->ti->table); > + int old_hash_failed = v->hash_failed; > > /* Corruption should be visible in device status in all modes */ > v->hash_failed = 1; > > + /* Only remeasure on first failure */ > + if (!old_hash_failed) > + dm_ima_measure_on_target_update(v->ti); It is not obvious to me why the call to measure on target update is not done here immediately. Updating the comment to explain the logic would help. thanks, -lakshmi > + > if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS) > goto out; >
Hi Lakshmi, On 06.05.22 22:35, Lakshmi Ramasubramanian wrote: > Hi Thore, > > On 1/6/2022 12:34 PM, Thore Sommer wrote: >> On first corruption the verity targets triggers a dm_target_update event. >> This allows other systems to check using IMA if the state of the >> device is >> still trustworthy via remote attestation. > In the patch description please state the existing problem (or, > limitation in the existing implementation) and then describe how the > proposed change addresses the issue. The problem is that we never see a IMA entry when a target gets marked as corrupted. The proposed change uses the new dm_target_update event to remeasure the table when the target table entry changes from valid to corrupted. I will add a more complete description to v2. > >> >> Signed-off-by: Thore Sommer <public@thson.de> >> --- >> drivers/md/dm-verity-target.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/md/dm-verity-target.c >> b/drivers/md/dm-verity-target.c >> index 80133aae0db3..09696e25bf1c 100644 >> --- a/drivers/md/dm-verity-target.c >> +++ b/drivers/md/dm-verity-target.c >> @@ -16,6 +16,7 @@ >> #include "dm-verity.h" >> #include "dm-verity-fec.h" >> #include "dm-verity-verify-sig.h" >> +#include "dm-ima.h" >> #include <linux/module.h> >> #include <linux/reboot.h> >> #include <linux/scatterlist.h> >> @@ -218,10 +219,15 @@ static int verity_handle_err(struct dm_verity >> *v, enum verity_block_type type, >> char *envp[] = { verity_env, NULL }; >> const char *type_str = ""; >> struct mapped_device *md = dm_table_get_md(v->ti->table); >> + int old_hash_failed = v->hash_failed; >> /* Corruption should be visible in device status in all modes */ >> v->hash_failed = 1; >> + /* Only remeasure on first failure */ >> + if (!old_hash_failed) >> + dm_ima_measure_on_target_update(v->ti); > It is not obvious to me why the call to measure on target update is not > done here immediately. Updating the comment to explain the logic would > help. The change should be only measured once, because otherwise we would create many IMA entries each for every corrupted block read if I understand the verity code correctly. So we need to check if the state before was valid, but we need to measure after the table was changed to corrupted (v->hash_failed = 1). Something like this might be a bit clearer and does not use a extra variable: + if (!v->hash_failed) + v->hash_failed = 1; + dm_ima_measure_on_target_update(v->ti); Regards, Thore > > thanks, > -lakshmi > >> + >> if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS) >> goto out;
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index 80133aae0db3..09696e25bf1c 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -16,6 +16,7 @@ #include "dm-verity.h" #include "dm-verity-fec.h" #include "dm-verity-verify-sig.h" +#include "dm-ima.h" #include <linux/module.h> #include <linux/reboot.h> #include <linux/scatterlist.h> @@ -218,10 +219,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type, char *envp[] = { verity_env, NULL }; const char *type_str = ""; struct mapped_device *md = dm_table_get_md(v->ti->table); + int old_hash_failed = v->hash_failed; /* Corruption should be visible in device status in all modes */ v->hash_failed = 1; + /* Only remeasure on first failure */ + if (!old_hash_failed) + dm_ima_measure_on_target_update(v->ti); + if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS) goto out;
On first corruption the verity targets triggers a dm_target_update event. This allows other systems to check using IMA if the state of the device is still trustworthy via remote attestation. Signed-off-by: Thore Sommer <public@thson.de> --- drivers/md/dm-verity-target.c | 6 ++++++ 1 file changed, 6 insertions(+)