| Message ID | 20220823152108.v2.1.I776854f47e3340cc2913ed4d8ecdd328048b73c3@changeid (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Encrypted Hibernation | expand |
On Tue, Aug 23, 2022 at 03:25:17PM -0700, Evan Green wrote: > From: Matthew Garrett <matthewgarrett@google.com> > > Add an internal command for resetting a PCR. This will be used by the > encrypted hibernation code to set PCR23 to a known value. The > hibernation code will seal the hibernation key with a policy specifying > PCR23 be set to this known value as a mechanism to ensure that the > hibernation key is genuine. But to do this repeatedly, resetting the PCR > is necessary as well. > > From: Matthew Garrett <mjg59@google.com> This is probably here by mistake. > Signed-off-by: Matthew Garrett <mjg59@google.com> > No empty line here. > Signed-off-by: Evan Green <evgreen@chromium.org> > --- > Matthew's original version of this patch was at: > https://patchwork.kernel.org/patch/12096487/ > > (no changes since v1) > > drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++++++++++ > drivers/char/tpm/tpm.h | 2 ++ > drivers/char/tpm/tpm1-cmd.c | 34 ++++++++++++++++++++++++++++++ > drivers/char/tpm/tpm2-cmd.c | 36 ++++++++++++++++++++++++++++++++ > include/linux/tpm.h | 7 +++++++ > 5 files changed, 107 insertions(+) > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > index 1621ce8187052c..17b8643ee109c2 100644 > --- a/drivers/char/tpm/tpm-interface.c > +++ b/drivers/char/tpm/tpm-interface.c > @@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > } > EXPORT_SYMBOL_GPL(tpm_pcr_extend); > > +/** > + * tpm_pcr_reset - reset the specified PCR > + * @chip: a &struct tpm_chip instance, %NULL for the default chip > + * @pcr_idx: the PCR to be reset > + * > + * Return: same as with tpm_transmit_cmd() > + */ > +int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx) > +{ > + int rc; > + > + chip = tpm_find_get_ops(chip); > + if (!chip) > + return -ENODEV; > + > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > + rc = tpm2_pcr_reset(chip, pcr_idx); > + goto out; > + } > + > + rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); > + > +out: > + tpm_put_ops(chip); if (chip->flags & TPM_CHIP_FLAG_TPM2) rc = tpm2_pcr_reset(chip, pcr_idx); else rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); Where does this asymmetry come with the parameters? BR, Jarkko
On Thu, Aug 25, 2022 at 8:00 PM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > On Tue, Aug 23, 2022 at 03:25:17PM -0700, Evan Green wrote: > > From: Matthew Garrett <matthewgarrett@google.com> > > > > Add an internal command for resetting a PCR. This will be used by the > > encrypted hibernation code to set PCR23 to a known value. The > > hibernation code will seal the hibernation key with a policy specifying > > PCR23 be set to this known value as a mechanism to ensure that the > > hibernation key is genuine. But to do this repeatedly, resetting the PCR > > is necessary as well. > > > > From: Matthew Garrett <mjg59@google.com> > > This is probably here by mistake. > > > Signed-off-by: Matthew Garrett <mjg59@google.com> > > > > No empty line here. > > > Signed-off-by: Evan Green <evgreen@chromium.org> > > --- > > Matthew's original version of this patch was at: > > https://patchwork.kernel.org/patch/12096487/ > > > > (no changes since v1) > > > > drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++++++++++ > > drivers/char/tpm/tpm.h | 2 ++ > > drivers/char/tpm/tpm1-cmd.c | 34 ++++++++++++++++++++++++++++++ > > drivers/char/tpm/tpm2-cmd.c | 36 ++++++++++++++++++++++++++++++++ > > include/linux/tpm.h | 7 +++++++ > > 5 files changed, 107 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > > index 1621ce8187052c..17b8643ee109c2 100644 > > --- a/drivers/char/tpm/tpm-interface.c > > +++ b/drivers/char/tpm/tpm-interface.c > > @@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > > } > > EXPORT_SYMBOL_GPL(tpm_pcr_extend); > > > > +/** > > + * tpm_pcr_reset - reset the specified PCR > > + * @chip: a &struct tpm_chip instance, %NULL for the default chip > > + * @pcr_idx: the PCR to be reset > > + * > > + * Return: same as with tpm_transmit_cmd() > > + */ > > +int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx) > > +{ > > + int rc; > > + > > + chip = tpm_find_get_ops(chip); > > + if (!chip) > > + return -ENODEV; > > + > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > > + rc = tpm2_pcr_reset(chip, pcr_idx); > > + goto out; > > + } > > + > > + rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); > > + > > +out: > > + tpm_put_ops(chip); > > if (chip->flags & TPM_CHIP_FLAG_TPM2) > rc = tpm2_pcr_reset(chip, pcr_idx); > else > rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); > > Where does this asymmetry come with the parameters? Sorry for the delay, I was out last week. I think it's modeled to match the tpm1/2_pcr_extend functions, which have this same odd asymmetry. Should I change it to have both use the tpm2_pcr_reset() prototype? -Evan
On Wed, Sep 07, 2022 at 10:02:14AM -0700, Evan Green wrote: > On Thu, Aug 25, 2022 at 8:00 PM Jarkko Sakkinen <jarkko@kernel.org> wrote: > > > > On Tue, Aug 23, 2022 at 03:25:17PM -0700, Evan Green wrote: > > > From: Matthew Garrett <matthewgarrett@google.com> > > > > > > Add an internal command for resetting a PCR. This will be used by the > > > encrypted hibernation code to set PCR23 to a known value. The > > > hibernation code will seal the hibernation key with a policy specifying > > > PCR23 be set to this known value as a mechanism to ensure that the > > > hibernation key is genuine. But to do this repeatedly, resetting the PCR > > > is necessary as well. > > > > > > From: Matthew Garrett <mjg59@google.com> > > > > This is probably here by mistake. > > > > > Signed-off-by: Matthew Garrett <mjg59@google.com> > > > > > > > No empty line here. > > > > > Signed-off-by: Evan Green <evgreen@chromium.org> > > > --- > > > Matthew's original version of this patch was at: > > > https://patchwork.kernel.org/patch/12096487/ > > > > > > (no changes since v1) > > > > > > drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++++++++++ > > > drivers/char/tpm/tpm.h | 2 ++ > > > drivers/char/tpm/tpm1-cmd.c | 34 ++++++++++++++++++++++++++++++ > > > drivers/char/tpm/tpm2-cmd.c | 36 ++++++++++++++++++++++++++++++++ > > > include/linux/tpm.h | 7 +++++++ > > > 5 files changed, 107 insertions(+) > > > > > > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c > > > index 1621ce8187052c..17b8643ee109c2 100644 > > > --- a/drivers/char/tpm/tpm-interface.c > > > +++ b/drivers/char/tpm/tpm-interface.c > > > @@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > > > } > > > EXPORT_SYMBOL_GPL(tpm_pcr_extend); > > > > > > +/** > > > + * tpm_pcr_reset - reset the specified PCR > > > + * @chip: a &struct tpm_chip instance, %NULL for the default chip > > > + * @pcr_idx: the PCR to be reset > > > + * > > > + * Return: same as with tpm_transmit_cmd() > > > + */ > > > +int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx) > > > +{ > > > + int rc; > > > + > > > + chip = tpm_find_get_ops(chip); > > > + if (!chip) > > > + return -ENODEV; > > > + > > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) { > > > + rc = tpm2_pcr_reset(chip, pcr_idx); > > > + goto out; > > > + } > > > + > > > + rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); > > > + > > > +out: > > > + tpm_put_ops(chip); > > > > if (chip->flags & TPM_CHIP_FLAG_TPM2) > > rc = tpm2_pcr_reset(chip, pcr_idx); > > else > > rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); > > > > Where does this asymmetry come with the parameters? > > Sorry for the delay, I was out last week. I think it's modeled to > match the tpm1/2_pcr_extend functions, which have this same odd > asymmetry. Should I change it to have both use the tpm2_pcr_reset() > prototype? > -Evan Yeah, I think it'd be a good idea. BR, Jarkko
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 1621ce8187052c..17b8643ee109c2 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -342,6 +342,34 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, } EXPORT_SYMBOL_GPL(tpm_pcr_extend); +/** + * tpm_pcr_reset - reset the specified PCR + * @chip: a &struct tpm_chip instance, %NULL for the default chip + * @pcr_idx: the PCR to be reset + * + * Return: same as with tpm_transmit_cmd() + */ +int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx) +{ + int rc; + + chip = tpm_find_get_ops(chip); + if (!chip) + return -ENODEV; + + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + rc = tpm2_pcr_reset(chip, pcr_idx); + goto out; + } + + rc = tpm1_pcr_reset(chip, pcr_idx, "attempting to reset a PCR"); + +out: + tpm_put_ops(chip); + return rc; +} +EXPORT_SYMBOL_GPL(tpm_pcr_reset); + /** * tpm_send - send a TPM command * @chip: a &struct tpm_chip instance, %NULL for the default chip diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 24ee4e1cc452a0..a80b341d38eb8c 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -175,6 +175,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip); unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal); int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash, const char *log_msg); +int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg); int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf); ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, const char *desc, size_t min_cap_length); @@ -217,6 +218,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digest, u16 *digest_size_ptr); int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digests); +int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx); int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max); ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value, const char *desc); diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index cf64c738510529..8ec743dec26544 100644 --- a/drivers/char/tpm/tpm1-cmd.c +++ b/drivers/char/tpm/tpm1-cmd.c @@ -478,6 +478,40 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash, return rc; } +struct tpm_pcr_selection { + u16 size_of_select; + u8 pcr_select[3]; +} __packed; + +#define TPM_ORD_PCR_RESET 200 +int tpm1_pcr_reset(struct tpm_chip *chip, u32 pcr_idx, const char *log_msg) +{ + struct tpm_pcr_selection selection; + struct tpm_buf buf; + int i, rc; + char tmp; + + rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_RESET); + if (rc) + return rc; + + selection.size_of_select = 3; + + for (i = 0; i < selection.size_of_select; i++) { + tmp = 0; + if (pcr_idx / 3 == i) { + pcr_idx -= i * 8; + tmp |= 1 << pcr_idx; + } + selection.pcr_select[i] = tmp; + } + tpm_buf_append(&buf, (u8 *)&selection, sizeof(selection)); + + rc = tpm_transmit_cmd(chip, &buf, sizeof(selection), log_msg); + tpm_buf_destroy(&buf); + return rc; +} + #define TPM_ORD_GET_CAP 101 ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, const char *desc, size_t min_cap_length) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 65d03867e114c5..69126a6770386e 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -269,6 +269,42 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, return rc; } +/** + * tpm2_pcr_reset() - reset a PCR + * + * @chip: TPM chip to use. + * @pcr_idx: index of the PCR. + * + * Return: Same as with tpm_transmit_cmd. + */ +int tpm2_pcr_reset(struct tpm_chip *chip, u32 pcr_idx) +{ + struct tpm_buf buf; + struct tpm2_null_auth_area auth_area; + int rc; + + rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_RESET); + if (rc) + return rc; + + tpm_buf_append_u32(&buf, pcr_idx); + + auth_area.handle = cpu_to_be32(TPM2_RS_PW); + auth_area.nonce_size = 0; + auth_area.attributes = 0; + auth_area.auth_size = 0; + + tpm_buf_append_u32(&buf, sizeof(struct tpm2_null_auth_area)); + tpm_buf_append(&buf, (const unsigned char *)&auth_area, + sizeof(auth_area)); + + rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to reset a PCR"); + + tpm_buf_destroy(&buf); + + return rc; +} + struct tpm2_get_random_out { __be16 size; u8 buffer[TPM_MAX_RNG_DATA]; diff --git a/include/linux/tpm.h b/include/linux/tpm.h index dfeb25a0362dee..8320cbac6f4009 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -219,6 +219,7 @@ enum tpm2_command_codes { TPM2_CC_HIERARCHY_CONTROL = 0x0121, TPM2_CC_HIERARCHY_CHANGE_AUTH = 0x0129, TPM2_CC_CREATE_PRIMARY = 0x0131, + TPM2_CC_PCR_RESET = 0x013D, TPM2_CC_SEQUENCE_COMPLETE = 0x013E, TPM2_CC_SELF_TEST = 0x0143, TPM2_CC_STARTUP = 0x0144, @@ -423,6 +424,7 @@ extern ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf, size_t min_rsp_body_length, const char *desc); extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digest); +extern int tpm_pcr_reset(struct tpm_chip *chip, u32 pcr_idx); extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digests); extern int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen); @@ -440,6 +442,11 @@ static inline int tpm_pcr_read(struct tpm_chip *chip, int pcr_idx, return -ENODEV; } +static inline int tpm_pcr_reset(struct tpm_chip *chip, int pcr_idx) +{ + return -ENODEV; +} + static inline int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digests) {