2021-02-20 01:37:11

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 0/9] Enable hibernation when Lockdown is enabled

Lockdown in integrity mode aims to ensure that arbitrary code cannot end
up in ring 0 without owner approval. The combination of module signing
and secure boot gets most of the way there, but various other features
are disabled by lockdown in order to avoid more convoluted approaches
that would enable unwanted code to end up in kernel space. One of these
is hibernation, since at present the kernel performs no verification of
the code it's resuming. If hibernation were permitted, an attacker with
root (but not kernel) privileges could disable swap, write a valid
hibernation image containing code of their own choosing to the swap
partition, and then reboot. On reboot, the kernel would happily resume
the provided code.

This patchset aims to provide a secure implementation of hibernation. It
is based on the presumption that simply storing a key in-kernel is
insufficient, since if Lockdown is merely in integrity (rather than
confidentiality) mode we assume that root is able to read kernel memory
and so would be able to obtain these secrets. It also aims to be
unspoofable - an attacker should not be able to write out a hibernation
image using cryptographic material under their control.

TPMs can be used to generate key material that is encrypted with a key
that does not leave the TPM. This means that we can generate an AES key,
encrypt the image hash with it, encrypt it with a TPM-backed key, and store
the encrypted key in the hibernation image. On resume, we pass the key
back to the TPM, receive the unencrypted AES key, and use that to
validate the image.

However, this is insufficient. Nothing prevents anyone else with access
to the TPM asking it to decrypt the key. We need to be able to
distinguish between accesses that come from the kernel directly and
accesses that come from userland.

TPMs have several Platform Configuration Registers (PCRs) which are used
for different purposes. PCRs are initialised to a known value, and
cannot be modified directly by the platform. Instead, the platform can
provide a hash of some data to the TPM. The TPM combines the existing
PCR value with the new hash, and stores the hash of this combination in
the PCR. Most PCRs can only be extended, which means that the only way
to achieve a specific value for a TPM is to perform the same series of
writes.

When secrets are encrypted by the TPM, they can be accompanied by a
policy that describes the state the TPM must be in in order for it to
decrypt them. If the TPM is not in this state, it will refuse to decrypt
the material even if it is otherwise capable of doing so. This allows
keys to be tied to specific system state - if the system is not in that
state, the TPM will not grant access.

PCR 23 is special in that it can be reset on demand. This patchset
re-purposes PCR 23 and blocks userland's ability to extend or reset it.
The kernel is then free to impose policy by resetting PCR 23 to a known
starting point, extending it with a known value, encrypting key material
with a policy that ties it to PCR 23, and then resetting it. Even if
userland has access to the encrypted blob, it cannot decrypt it since it
has no way to force PCR 23 to be in the same state.

So. During hibernation we freeze userland. We then reset PCR 23 and
extend it to a known value. We generate a key, use it and then encrypt
it with the TPM. When we encrypt it, we define a policy which states
that the TPM must have the same PCR 23 value as it presently does. We
also store the current PCR 23 value in the key metadata. On resume, we
again freeze userland, reset PCR 23 and extend it to the same value. We
decrypt the key, and verify from the metadata that it was created when
PCR 23 had the expected value. If so, we use it to decrypt the hash used
to verify the hibernation image and ensure that the image matches it. If
everything looks good, we resume. If not, we return to userland. Either
way, we reset PCR 23 before any userland code runs again.

This all works on my machine, but it's imperfect - there's a meaningful
performance hit on resume forced by reading all the blocks in-order, and
it probably makes more sense to do that after reads are complete instead
but I wanted to get the other components of this out for review first.
It's also not ideal from a security perspective until there's more
ecosystem integration - we need a kernel to be able to assert to a
signed bootloader that it implements this, since otherwise an attacker
can simply downgrade to a kernel that doesn't implement this policy and
gain access to PCR 23 themselves. There's ongoing work in the UEFI
bootloader space that would make this possible.



2021-02-20 01:37:12

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

Performing any sort of state validation of a sealed TPM blob requires
being able to access the individual members in the response. Parse the
blob sufficiently to be able to stash pointers to each member, along
with the length.

Signed-off-by: Matthew Garrett <[email protected]>
---
include/keys/trusted-type.h | 8 +++
security/keys/trusted-keys/trusted_tpm2.c | 67 ++++++++++++++++++++++-
2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index a94c03a61d8f..020e01a99ea4 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -16,14 +16,22 @@
#define MAX_BLOB_SIZE 512
#define MAX_PCRINFO_SIZE 64
#define MAX_DIGEST_SIZE 64
+#define MAX_CREATION_DATA 412
+#define MAX_TK 76

struct trusted_key_payload {
struct rcu_head rcu;
unsigned int key_len;
unsigned int blob_len;
+ unsigned int creation_len;
+ unsigned int creation_hash_len;
+ unsigned int tk_len;
unsigned char migratable;
unsigned char key[MAX_KEY_SIZE + 1];
unsigned char blob[MAX_BLOB_SIZE];
+ unsigned char *creation;
+ unsigned char *creation_hash;
+ unsigned char *tk;
};

struct trusted_key_options {
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index 08ec7f48f01d..6357a51a24e9 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -50,6 +50,63 @@ static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle,
tpm_buf_append(buf, hmac, hmac_len);
}

+static int tpm2_unpack_blob(struct trusted_key_payload *payload)
+{
+ int tmp, offset;
+
+ /* Find the length of the private data */
+ tmp = be16_to_cpup((__be16 *) &payload->blob[0]);
+ offset = tmp + 2;
+ if (offset > payload->blob_len)
+ return -EFAULT;
+
+ /* Find the length of the public data */
+ tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+ offset += tmp + 2;
+ if (offset > payload->blob_len)
+ return -EFAULT;
+
+ /* Find the length of the creation data and store it */
+ tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+ if (tmp > MAX_CREATION_DATA)
+ return -E2BIG;
+
+ if ((offset + tmp + 2) > payload->blob_len)
+ return -EFAULT;
+
+ payload->creation = &payload->blob[offset + 2];
+ payload->creation_len = tmp;
+ offset += tmp + 2;
+
+ /* Find the length of the creation hash and store it */
+ tmp = be16_to_cpup((__be16 *) &payload->blob[offset]);
+ if (tmp > MAX_DIGEST_SIZE)
+ return -E2BIG;
+
+ if ((offset + tmp + 2) > payload->blob_len)
+ return -EFAULT;
+
+ payload->creation_hash = &payload->blob[offset + 2];
+ payload->creation_hash_len = tmp;
+ offset += tmp + 2;
+
+ /*
+ * Store the creation ticket. TPMT_TK_CREATION is two bytes of tag,
+ * four bytes of handle, and then the digest length and digest data
+ */
+ tmp = be16_to_cpup((__be16 *) &payload->blob[offset + 6]);
+ if (tmp > MAX_TK)
+ return -E2BIG;
+
+ if ((offset + tmp + 8) > payload->blob_len)
+ return -EFAULT;
+
+ payload->tk = &payload->blob[offset];
+ payload->tk_len = tmp + 8;
+
+ return 0;
+}
+
/**
* tpm2_seal_trusted() - seal the payload of a trusted key
*
@@ -64,6 +121,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
struct trusted_key_options *options)
{
unsigned int blob_len;
+ unsigned int offset;
struct tpm_buf buf;
u32 hash;
int i;
@@ -139,14 +197,16 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
rc = -E2BIG;
goto out;
}
- if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 4 + blob_len) {
+ offset = TPM_HEADER_SIZE + 4;
+ if (tpm_buf_length(&buf) < offset + blob_len) {
rc = -EFAULT;
goto out;
}

- memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+ memcpy(payload->blob, &buf.data[offset], blob_len);
payload->blob_len = blob_len;

+ rc = tpm2_unpack_blob(payload);
out:
tpm_buf_destroy(&buf);

@@ -215,7 +275,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
if (!rc)
*blob_handle = be32_to_cpup(
(__be32 *) &buf.data[TPM_HEADER_SIZE]);
+ else
+ goto out;

+ rc = tpm2_unpack_blob(payload);
out:
tpm_buf_destroy(&buf);

--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:37:12

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 1/9] tpm: Add support for in-kernel resetting of PCRs

Add an internal command for resetting a PCR.

Signed-off-by: Matthew Garrett <[email protected]>
---
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 1621ce818705..17b8643ee109 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 947d1db0a5cc..746f7696bdc0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -176,6 +176,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);
@@ -220,6 +221,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 ca7158fa6e6c..36990e9d2dc1 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 eff1f12d981a..9609ae8086c6 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 8f4ff39f51e7..e2075e2242a0 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -211,6 +211,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,
@@ -399,6 +400,7 @@ static inline u32 tpm2_rc_value(u32 rc)
extern int tpm_is_tpm2(struct tpm_chip *chip);
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);
@@ -417,6 +419,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)
{
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:37:31

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 6/9] pm: hibernate: Optionally store and verify a hash of the image

Calculate and store a cryptographically secure hash of the hibernation
image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
detection of a corrupt image, but has the disadvantage that it requires
the blocks be read in in linear order.

Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/power/power.h | 1 +
kernel/power/swap.c | 131 +++++++++++++++++++++++++++++++++++--------
2 files changed, 110 insertions(+), 22 deletions(-)

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 778bf431ec02..b8e00b9dcee8 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
#define SF_PLATFORM_MODE 1
#define SF_NOCOMPRESS_MODE 2
#define SF_CRC32_MODE 4
+#define SF_VERIFY_IMAGE 8

/* kernel/power/hibernate.c */
extern int swsusp_check(void);
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 72e33054a2e1..a13241a20567 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -31,6 +31,8 @@
#include <linux/kthread.h>
#include <linux/crc32.h>
#include <linux/ktime.h>
+#include <crypto/hash.h>
+#include <crypto/sha2.h>

#include "power.h"

@@ -95,17 +97,20 @@ struct swap_map_page_list {
struct swap_map_handle {
struct swap_map_page *cur;
struct swap_map_page_list *maps;
+ struct shash_desc *desc;
sector_t cur_swap;
sector_t first_sector;
unsigned int k;
unsigned long reqd_free_pages;
u32 crc32;
+ u8 digest[SHA256_DIGEST_SIZE];
};

struct swsusp_header {
char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
- sizeof(u32)];
+ sizeof(u32) - SHA256_DIGEST_SIZE];
u32 crc32;
+ u8 digest[SHA256_DIGEST_SIZE];
sector_t image;
unsigned int flags; /* Flags to pass to the "boot" kernel */
char orig_sig[10];
@@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
* We are relying on the behavior of blk_plug that a thread with
* a plug will flush the plug list before sleeping.
*/
+ if (!hb)
+ return 0;
+
wait_event(hb->wait, atomic_read(&hb->count) == 0);
return blk_status_to_errno(hb->error);
}
@@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
swsusp_header->flags = flags;
if (flags & SF_CRC32_MODE)
swsusp_header->crc32 = handle->crc32;
+ memcpy(swsusp_header->digest, handle->digest,
+ SHA256_DIGEST_SIZE);
error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
swsusp_resume_block, swsusp_header, NULL);
} else {
@@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
static int get_swap_writer(struct swap_map_handle *handle)
{
int ret;
+ struct crypto_shash *tfm;

ret = swsusp_swap_check();
if (ret) {
@@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
handle->k = 0;
handle->reqd_free_pages = reqd_free_pages();
handle->first_sector = handle->cur_swap;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm)) {
+ ret = -EINVAL;
+ goto err_rel;
+ }
+ handle->desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!handle->desc) {
+ ret = -ENOMEM;
+ goto err_rel;
+ }
+
+ handle->desc->tfm = tfm;
+
+ ret = crypto_shash_init(handle->desc);
+ if (ret != 0)
+ goto err_free;
+
return 0;
+err_free:
+ kfree(handle->desc);
err_rel:
release_swap_writer(handle);
err_close:
@@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
}

static int swap_write_page(struct swap_map_handle *handle, void *buf,
- struct hib_bio_batch *hb)
+ struct hib_bio_batch *hb, bool hash)
{
int error = 0;
sector_t offset;
@@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
if (!handle->cur)
return -EINVAL;
offset = alloc_swapdev_block(root_swap);
+ crypto_shash_update(handle->desc, buf, PAGE_SIZE);
error = write_page(buf, offset, hb);
if (error)
return error;
@@ -496,6 +529,7 @@ static int flush_swap_writer(struct swap_map_handle *handle)
static int swap_writer_finish(struct swap_map_handle *handle,
unsigned int flags, int error)
{
+ crypto_shash_final(handle->desc, handle->digest);
if (!error) {
pr_info("S");
error = mark_swapfiles(handle, flags);
@@ -560,7 +594,7 @@ static int save_image(struct swap_map_handle *handle,
ret = snapshot_read_next(snapshot);
if (ret <= 0)
break;
- ret = swap_write_page(handle, data_of(*snapshot), &hb);
+ ret = swap_write_page(handle, data_of(*snapshot), &hb, true);
if (ret)
break;
if (!(nr_pages % m))
@@ -844,7 +878,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
off += PAGE_SIZE) {
memcpy(page, data[thr].cmp + off, PAGE_SIZE);

- ret = swap_write_page(handle, page, &hb);
+ ret = swap_write_page(handle, page, &hb, true);
if (ret)
goto out_finish;
}
@@ -938,7 +972,7 @@ int swsusp_write(unsigned int flags)
goto out_finish;
}
header = (struct swsusp_info *)data_of(snapshot);
- error = swap_write_page(&handle, header, NULL);
+ error = swap_write_page(&handle, header, NULL, false);
if (!error) {
error = (flags & SF_NOCOMPRESS_MODE) ?
save_image(&handle, &snapshot, pages - 1) :
@@ -974,6 +1008,7 @@ static int get_swap_reader(struct swap_map_handle *handle,
int error;
struct swap_map_page_list *tmp, *last;
sector_t offset;
+ struct crypto_shash *tfm;

*flags_p = swsusp_header->flags;

@@ -1011,11 +1046,34 @@ static int get_swap_reader(struct swap_map_handle *handle,
}
handle->k = 0;
handle->cur = handle->maps->map;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm)) {
+ error = -EINVAL;
+ goto err_rel;
+ }
+ handle->desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!handle->desc) {
+ error = -ENOMEM;
+ goto err_rel;
+ }
+
+ handle->desc->tfm = tfm;
+
+ error = crypto_shash_init(handle->desc);
+ if (error != 0)
+ goto err_free;
return 0;
+err_free:
+ kfree(handle->desc);
+err_rel:
+ release_swap_reader(handle);
+ return error;
}

static int swap_read_page(struct swap_map_handle *handle, void *buf,
- struct hib_bio_batch *hb)
+ struct hib_bio_batch *hb, bool hash)
{
sector_t offset;
int error;
@@ -1029,6 +1087,7 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
error = hib_submit_io(REQ_OP_READ, 0, offset, buf, hb);
if (error)
return error;
+ crypto_shash_update(handle->desc, buf, PAGE_SIZE);
if (++handle->k >= MAP_PAGE_ENTRIES) {
handle->k = 0;
free_page((unsigned long)handle->maps->map);
@@ -1043,11 +1102,21 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
return error;
}

-static int swap_reader_finish(struct swap_map_handle *handle)
+static int swap_reader_finish(struct swap_map_handle *handle,
+ struct swsusp_info *header)
{
+ int ret = 0;
+
+ crypto_shash_final(handle->desc, handle->digest);
+ if (memcmp(handle->digest, swsusp_header->digest,
+ SHA256_DIGEST_SIZE) != 0) {
+ pr_err("Image digest doesn't match header digest\n");
+ ret = -ENODATA;
+ }
+
release_swap_reader(handle);

- return 0;
+ return ret;
}

/**
@@ -1064,11 +1133,20 @@ static int load_image(struct swap_map_handle *handle,
int ret = 0;
ktime_t start;
ktime_t stop;
- struct hib_bio_batch hb;
+ struct hib_bio_batch *hb, real_hb;
int err2;
unsigned nr_pages;

- hib_init_batch(&hb);
+ /*
+ * If we're calculating the SHA256 of the image, we need the blocks
+ * to be read in in order
+ */
+ if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+ hb = NULL;
+ } else {
+ hib_init_batch(&real_hb);
+ hb = &real_hb;
+ }

clean_pages_on_read = true;
pr_info("Loading image data pages (%u pages)...\n", nr_to_read);
@@ -1081,11 +1159,11 @@ static int load_image(struct swap_map_handle *handle,
ret = snapshot_write_next(snapshot);
if (ret <= 0)
break;
- ret = swap_read_page(handle, data_of(*snapshot), &hb);
+ ret = swap_read_page(handle, data_of(*snapshot), hb, true);
if (ret)
break;
if (snapshot->sync_read)
- ret = hib_wait_io(&hb);
+ ret = hib_wait_io(hb);
if (ret)
break;
if (!(nr_pages % m))
@@ -1093,8 +1171,8 @@ static int load_image(struct swap_map_handle *handle,
nr_pages / m * 10);
nr_pages++;
}
- err2 = hib_wait_io(&hb);
- hib_finish_batch(&hb);
+ err2 = hib_wait_io(hb);
+ hib_finish_batch(hb);
stop = ktime_get();
if (!ret)
ret = err2;
@@ -1169,7 +1247,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
unsigned int m;
int ret = 0;
int eof = 0;
- struct hib_bio_batch hb;
+ struct hib_bio_batch *hb, real_hb;
ktime_t start;
ktime_t stop;
unsigned nr_pages;
@@ -1182,7 +1260,16 @@ static int load_image_lzo(struct swap_map_handle *handle,
struct dec_data *data = NULL;
struct crc_data *crc = NULL;

- hib_init_batch(&hb);
+ /*
+ * If we're calculating the SHA256 of the image, we need the blocks
+ * to be read in in order
+ */
+ if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+ hb = NULL;
+ } else {
+ hib_init_batch(&real_hb);
+ hb = &real_hb;
+ }

/*
* We'll limit the number of threads for decompression to limit memory
@@ -1301,7 +1388,7 @@ static int load_image_lzo(struct swap_map_handle *handle,

for(;;) {
for (i = 0; !eof && i < want; i++) {
- ret = swap_read_page(handle, page[ring], &hb);
+ ret = swap_read_page(handle, page[ring], hb, true);
if (ret) {
/*
* On real read error, finish. On end of data,
@@ -1328,7 +1415,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
if (!asked)
break;

- ret = hib_wait_io(&hb);
+ ret = hib_wait_io(hb);
if (ret)
goto out_finish;
have += asked;
@@ -1382,7 +1469,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
* Wait for more data while we are decompressing.
*/
if (have < LZO_CMP_PAGES && asked) {
- ret = hib_wait_io(&hb);
+ ret = hib_wait_io(hb);
if (ret)
goto out_finish;
have += asked;
@@ -1458,7 +1545,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
}
swsusp_show_speed(start, stop, nr_to_read, "Read");
out_clean:
- hib_finish_batch(&hb);
+ hib_finish_batch(hb);
for (i = 0; i < ring_size; i++)
free_page((unsigned long)page[i]);
if (crc) {
@@ -1499,13 +1586,13 @@ int swsusp_read(unsigned int *flags_p)
if (error)
goto end;
if (!error)
- error = swap_read_page(&handle, header, NULL);
+ error = swap_read_page(&handle, header, NULL, false);
if (!error) {
error = (*flags_p & SF_NOCOMPRESS_MODE) ?
load_image(&handle, &snapshot, header->pages - 1) :
load_image_lzo(&handle, &snapshot, header->pages - 1);
}
- swap_reader_finish(&handle);
+ error = swap_reader_finish(&handle, header);
end:
if (!error)
pr_debug("Image successfully loaded\n");
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:38:03

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

When TPMs generate keys, they can also generate some information
describing the state of the PCRs at creation time. This data can then
later be certified by the TPM, allowing verification of the PCR values.
This allows us to determine the state of the system at the time a key
was generated. Add an additional argument to the trusted key creation
options, allowing the user to provide the set of PCRs that should have
their values incorporated into the creation data.

Signed-off-by: Matthew Garrett <[email protected]>
---
.../security/keys/trusted-encrypted.rst | 4 +++
include/keys/trusted-type.h | 1 +
security/keys/trusted-keys/trusted_tpm1.c | 9 +++++++
security/keys/trusted-keys/trusted_tpm2.c | 25 +++++++++++++++++--
4 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index 1da879a68640..27bc43463ec8 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -72,6 +72,10 @@ Usage::
policyhandle= handle to an authorization policy session that defines the
same policy and with the same hash algorithm as was used to
seal the key.
+ creationpcrs= hex integer representing the set of PCR values to be
+ included in the PCR creation data. The bit corresponding
+ to each PCR should be 1 to be included, 0 to be ignored.
+ TPM2 only.

"keyctl print" returns an ascii hex copy of the sealed key, which is in standard
TPM_STORED_DATA format. The key length for new keys are always in bytes.
diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 154d8a1769c3..875e05f33b84 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -47,6 +47,7 @@ struct trusted_key_options {
uint32_t policydigest_len;
unsigned char policydigest[MAX_DIGEST_SIZE];
uint32_t policyhandle;
+ uint32_t creation_pcrs;
};

extern struct key_type key_type_trusted;
diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
index 74d82093cbaa..3d371ab3441f 100644
--- a/security/keys/trusted-keys/trusted_tpm1.c
+++ b/security/keys/trusted-keys/trusted_tpm1.c
@@ -709,6 +709,7 @@ enum {
Opt_hash,
Opt_policydigest,
Opt_policyhandle,
+ Opt_creationpcrs,
};

static const match_table_t key_tokens = {
@@ -724,6 +725,7 @@ static const match_table_t key_tokens = {
{Opt_hash, "hash=%s"},
{Opt_policydigest, "policydigest=%s"},
{Opt_policyhandle, "policyhandle=%s"},
+ {Opt_creationpcrs, "creationpcrs=%s"},
{Opt_err, NULL}
};

@@ -834,6 +836,13 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
return -EINVAL;
opt->policyhandle = handle;
break;
+ case Opt_creationpcrs:
+ if (!tpm2)
+ return -EINVAL;
+ res = kstrtoint(args[0].from, 16, &opt->creation_pcrs);
+ if (res < 0)
+ return -EINVAL;
+ break;
default:
return -EINVAL;
}
diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
index a3673fffd834..282f956ad610 100644
--- a/security/keys/trusted-keys/trusted_tpm2.c
+++ b/security/keys/trusted-keys/trusted_tpm2.c
@@ -124,7 +124,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
unsigned int offset;
struct tpm_buf buf;
u32 hash;
- int i;
+ int i, j;
int rc;

for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) {
@@ -181,7 +181,28 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
tpm_buf_append_u16(&buf, 0);

/* creation PCR */
- tpm_buf_append_u32(&buf, 0);
+ if (options->creation_pcrs) {
+ /* One bank */
+ tpm_buf_append_u32(&buf, 1);
+ /* Which bank to use */
+ tpm_buf_append_u16(&buf, hash);
+ /* Length of the PCR bitmask */
+ tpm_buf_append_u8(&buf, 3);
+ /* PCR bitmask */
+ for (i = 0; i < 3; i++) {
+ char tmp = 0;
+
+ for (j = 0; j < 8; j++) {
+ char bit = (i * 8) + j;
+
+ if (options->creation_pcrs & (1 << bit))
+ tmp |= (1 << j);
+ }
+ tpm_buf_append_u8(&buf, tmp);
+ }
+ } else {
+ tpm_buf_append_u32(&buf, 0);
+ }

if (buf.flags & TPM_BUF_OVERFLOW) {
rc = -E2BIG;
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:40:21

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

A plain hash protects the hibernation image against accidental
modification, but in the face of an active attack the hash can simply be
updated to match the new image. Generate a random AES key and seal this
with the TPM, and use it to encrypt the hash. On resume, the key can be
unsealed and used to decrypt the hash. By setting PCR 23 to a specific
value we can verify that the key used was generated by the kernel during
hibernation and prevent an attacker providing their own key.

Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/power/Kconfig | 15 ++
kernel/power/Makefile | 1 +
kernel/power/hibernate.c | 11 +-
kernel/power/swap.c | 99 +++----------
kernel/power/swap.h | 38 +++++
kernel/power/tpm.c | 294 +++++++++++++++++++++++++++++++++++++++
kernel/power/tpm.h | 37 +++++
7 files changed, 417 insertions(+), 78 deletions(-)
create mode 100644 kernel/power/swap.h
create mode 100644 kernel/power/tpm.c
create mode 100644 kernel/power/tpm.h

diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index a7320f07689d..0279cc10f319 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -92,6 +92,21 @@ config HIBERNATION_SNAPSHOT_DEV

If in doubt, say Y.

+config SECURE_HIBERNATION
+ bool "Implement secure hibernation support"
+ depends on HIBERNATION && TCG_TPM
+ select KEYS
+ select TRUSTED_KEYS
+ select CRYPTO
+ select CRYPTO_SHA256
+ select CRYPTO_AES
+ select TCG_TPM_RESTRICT_PCR
+ help
+ Use a TPM-backed key to securely determine whether a hibernation
+ image was written out by the kernel and has not been tampered with.
+ This requires a TCG-compliant TPM2 device, which is present on most
+ modern hardware.
+
config PM_STD_PARTITION
string "Default resume partition"
depends on HIBERNATION
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 5899260a8bef..2edfef897607 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o
obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o
obj-$(CONFIG_HIBERNATION_SNAPSHOT_DEV) += user.o
+obj-$(CONFIG_SECURE_HIBERNATION) += tpm.o
obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o
obj-$(CONFIG_PM_WAKELOCKS) += wakelock.o

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index da0b41914177..608bfbee38f5 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -34,6 +34,7 @@
#include <trace/events/power.h>

#include "power.h"
+#include "tpm.h"


static int nocompress;
@@ -81,7 +82,11 @@ void hibernate_release(void)

bool hibernation_available(void)
{
- return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
+ if (security_locked_down(LOCKDOWN_HIBERNATION) &&
+ !secure_hibernation_available())
+ return false;
+
+ return nohibernate == 0;
}

/**
@@ -752,7 +757,9 @@ int hibernate(void)
flags |= SF_NOCOMPRESS_MODE;
else
flags |= SF_CRC32_MODE;
-
+#ifdef CONFIG_SECURE_HIBERNATION
+ flags |= SF_VERIFY_IMAGE;
+#endif
pm_pr_dbg("Writing hibernation image.\n");
error = swsusp_write(flags);
swsusp_free();
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index a13241a20567..eaa585731314 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -32,9 +32,10 @@
#include <linux/crc32.h>
#include <linux/ktime.h>
#include <crypto/hash.h>
-#include <crypto/sha2.h>

#include "power.h"
+#include "swap.h"
+#include "tpm.h"

#define HIBERNATE_SIG "S1SUSPEND"

@@ -89,34 +90,6 @@ struct swap_map_page_list {
struct swap_map_page_list *next;
};

-/**
- * The swap_map_handle structure is used for handling swap in
- * a file-alike way
- */
-
-struct swap_map_handle {
- struct swap_map_page *cur;
- struct swap_map_page_list *maps;
- struct shash_desc *desc;
- sector_t cur_swap;
- sector_t first_sector;
- unsigned int k;
- unsigned long reqd_free_pages;
- u32 crc32;
- u8 digest[SHA256_DIGEST_SIZE];
-};
-
-struct swsusp_header {
- char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
- sizeof(u32) - SHA256_DIGEST_SIZE];
- u32 crc32;
- u8 digest[SHA256_DIGEST_SIZE];
- sector_t image;
- unsigned int flags; /* Flags to pass to the "boot" kernel */
- char orig_sig[10];
- char sig[10];
-} __packed;
-
static struct swsusp_header *swsusp_header;

/**
@@ -337,6 +310,9 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
swsusp_header->crc32 = handle->crc32;
memcpy(swsusp_header->digest, handle->digest,
SHA256_DIGEST_SIZE);
+ error = swsusp_encrypt_digest(swsusp_header);
+ if (error)
+ return error;
error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
swsusp_resume_block, swsusp_header, NULL);
} else {
@@ -427,7 +403,6 @@ static void release_swap_writer(struct swap_map_handle *handle)
static int get_swap_writer(struct swap_map_handle *handle)
{
int ret;
- struct crypto_shash *tfm;

ret = swsusp_swap_check();
if (ret) {
@@ -449,27 +424,11 @@ static int get_swap_writer(struct swap_map_handle *handle)
handle->reqd_free_pages = reqd_free_pages();
handle->first_sector = handle->cur_swap;

- tfm = crypto_alloc_shash("sha256", 0, 0);
- if (IS_ERR(tfm)) {
- ret = -EINVAL;
- goto err_rel;
- }
- handle->desc = kmalloc(sizeof(struct shash_desc) +
- crypto_shash_descsize(tfm), GFP_KERNEL);
- if (!handle->desc) {
- ret = -ENOMEM;
+ ret = swsusp_digest_setup(handle);
+ if (ret)
goto err_rel;
- }
-
- handle->desc->tfm = tfm;
-
- ret = crypto_shash_init(handle->desc);
- if (ret != 0)
- goto err_free;

return 0;
-err_free:
- kfree(handle->desc);
err_rel:
release_swap_writer(handle);
err_close:
@@ -486,7 +445,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
if (!handle->cur)
return -EINVAL;
offset = alloc_swapdev_block(root_swap);
- crypto_shash_update(handle->desc, buf, PAGE_SIZE);
+ swsusp_digest_update(handle, buf, PAGE_SIZE);
error = write_page(buf, offset, hb);
if (error)
return error;
@@ -529,7 +488,7 @@ static int flush_swap_writer(struct swap_map_handle *handle)
static int swap_writer_finish(struct swap_map_handle *handle,
unsigned int flags, int error)
{
- crypto_shash_final(handle->desc, handle->digest);
+ swsusp_digest_final(handle);
if (!error) {
pr_info("S");
error = mark_swapfiles(handle, flags);
@@ -1008,7 +967,6 @@ static int get_swap_reader(struct swap_map_handle *handle,
int error;
struct swap_map_page_list *tmp, *last;
sector_t offset;
- struct crypto_shash *tfm;

*flags_p = swsusp_header->flags;

@@ -1047,27 +1005,12 @@ static int get_swap_reader(struct swap_map_handle *handle,
handle->k = 0;
handle->cur = handle->maps->map;

- tfm = crypto_alloc_shash("sha256", 0, 0);
- if (IS_ERR(tfm)) {
- error = -EINVAL;
- goto err_rel;
- }
- handle->desc = kmalloc(sizeof(struct shash_desc) +
- crypto_shash_descsize(tfm), GFP_KERNEL);
- if (!handle->desc) {
- error = -ENOMEM;
- goto err_rel;
- }
-
- handle->desc->tfm = tfm;
+ error = swsusp_digest_setup(handle);
+ if (error)
+ goto err;

- error = crypto_shash_init(handle->desc);
- if (error != 0)
- goto err_free;
return 0;
-err_free:
- kfree(handle->desc);
-err_rel:
+err:
release_swap_reader(handle);
return error;
}
@@ -1087,7 +1030,7 @@ static int swap_read_page(struct swap_map_handle *handle, void *buf,
error = hib_submit_io(REQ_OP_READ, 0, offset, buf, hb);
if (error)
return error;
- crypto_shash_update(handle->desc, buf, PAGE_SIZE);
+ swsusp_digest_update(handle, buf, PAGE_SIZE);
if (++handle->k >= MAP_PAGE_ENTRIES) {
handle->k = 0;
free_page((unsigned long)handle->maps->map);
@@ -1107,11 +1050,13 @@ static int swap_reader_finish(struct swap_map_handle *handle,
{
int ret = 0;

- crypto_shash_final(handle->desc, handle->digest);
- if (memcmp(handle->digest, swsusp_header->digest,
- SHA256_DIGEST_SIZE) != 0) {
- pr_err("Image digest doesn't match header digest\n");
- ret = -ENODATA;
+ swsusp_digest_final(handle);
+ if (swsusp_header->flags & SF_VERIFY_IMAGE) {
+ if (memcmp(handle->digest, swsusp_header->digest,
+ SHA256_DIGEST_SIZE) != 0) {
+ pr_err("Image digest doesn't match header digest\n");
+ ret = -ENODATA;
+ }
}

release_swap_reader(handle);
@@ -1630,6 +1575,8 @@ int swsusp_check(void)
error = -EINVAL;
}

+ if (!error)
+ error = swsusp_decrypt_digest(swsusp_header);
put:
if (error)
blkdev_put(hib_resume_bdev, FMODE_READ);
diff --git a/kernel/power/swap.h b/kernel/power/swap.h
new file mode 100644
index 000000000000..342189344f5f
--- /dev/null
+++ b/kernel/power/swap.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <keys/trusted-type.h>
+#include <crypto/sha2.h>
+
+#ifndef _POWER_SWAP_H
+#define _POWER_SWAP_H 1
+/**
+ * The swap_map_handle structure is used for handling swap in
+ * a file-alike way
+ */
+
+struct swap_map_handle {
+ struct swap_map_page *cur;
+ struct swap_map_page_list *maps;
+ struct shash_desc *desc;
+ sector_t cur_swap;
+ sector_t first_sector;
+ unsigned int k;
+ unsigned long reqd_free_pages;
+ u32 crc32;
+ u8 digest[SHA256_DIGEST_SIZE];
+};
+
+struct swsusp_header {
+ char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
+ sizeof(u32) - SHA256_DIGEST_SIZE - MAX_BLOB_SIZE -
+ sizeof(u32)];
+ u32 blob_len;
+ u8 blob[MAX_BLOB_SIZE];
+ u8 digest[SHA256_DIGEST_SIZE];
+ u32 crc32;
+ sector_t image;
+ unsigned int flags; /* Flags to pass to the "boot" kernel */
+ char orig_sig[10];
+ char sig[10];
+} __packed;
+
+#endif /* _POWER_SWAP_H */
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
new file mode 100644
index 000000000000..953dcbdc56d8
--- /dev/null
+++ b/kernel/power/tpm.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <crypto/hash.h>
+#include <crypto/skcipher.h>
+#include <linux/key-type.h>
+#include <linux/scatterlist.h>
+
+#include "swap.h"
+#include "tpm.h"
+
+/* sha256("To sleep, perchance to dream") */
+static struct tpm_digest digest = { .alg_id = TPM_ALG_SHA256,
+ .digest = {0x92, 0x78, 0x3d, 0x79, 0x2d, 0x00, 0x31, 0xb0, 0x55, 0xf9,
+ 0x1e, 0x0d, 0xce, 0x83, 0xde, 0x1d, 0xc4, 0xc5, 0x8e, 0x8c,
+ 0xf1, 0x22, 0x38, 0x6c, 0x33, 0xb1, 0x14, 0xb7, 0xec, 0x05,
+ 0x5f, 0x49}};
+
+struct skcipher_def {
+ struct scatterlist sg;
+ struct crypto_skcipher *tfm;
+ struct skcipher_request *req;
+ struct crypto_wait wait;
+};
+
+static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
+ int enc)
+{
+ struct skcipher_def sk;
+ struct crypto_skcipher *skcipher = NULL;
+ struct skcipher_request *req = NULL;
+ char *ivdata = NULL;
+ int ret;
+
+ skcipher = crypto_alloc_skcipher("cbc-aes-aesni", 0, 0);
+ if (IS_ERR(skcipher))
+ return PTR_ERR(skcipher);
+
+ req = skcipher_request_alloc(skcipher, GFP_KERNEL);
+ if (!req) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
+ crypto_req_done,
+ &sk.wait);
+
+ /* AES 256 */
+ if (crypto_skcipher_setkey(skcipher, payload->key, 32)) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
+ /* Key will never be re-used, just fix the IV to 0 */
+ ivdata = kzalloc(16, GFP_KERNEL);
+ if (!ivdata) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ sk.tfm = skcipher;
+ sk.req = req;
+
+ sg_init_one(&sk.sg, buf, 32);
+ skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
+ crypto_init_wait(&sk.wait);
+
+ /* perform the operation */
+ if (enc)
+ ret = crypto_wait_req(crypto_skcipher_encrypt(sk.req),
+ &sk.wait);
+ else
+ ret = crypto_wait_req(crypto_skcipher_decrypt(sk.req),
+ &sk.wait);
+
+ if (ret)
+ pr_info("skcipher encrypt returned with result %d\n", ret);
+
+ goto out;
+
+out:
+ if (skcipher)
+ crypto_free_skcipher(skcipher);
+ if (req)
+ skcipher_request_free(req);
+ kfree(ivdata);
+ return ret;
+}
+
+int swsusp_encrypt_digest(struct swsusp_header *header)
+{
+ const struct cred *cred = current_cred();
+ struct trusted_key_payload *payload;
+ struct tpm_digest *digests = NULL;
+ struct tpm_chip *chip;
+ struct key *key;
+ int ret, i;
+
+ char *keyinfo = "new\t32\tkeyhandle=0x81000001";
+
+ chip = tpm_default_chip();
+
+ if (!chip)
+ return -ENODEV;
+
+ if (!(tpm_is_tpm2(chip)))
+ return -ENODEV;
+
+ ret = tpm_pcr_reset(chip, 23);
+ if (ret != 0)
+ return ret;
+
+ digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
+ GFP_KERNEL);
+ if (!digests) {
+ ret = -ENOMEM;
+ goto reset;
+ }
+
+ for (i = 0; i <= chip->nr_allocated_banks; i++) {
+ digests[i].alg_id = chip->allocated_banks[i].alg_id;
+ if (digests[i].alg_id == digest.alg_id)
+ memcpy(&digests[i], &digest, sizeof(digest));
+ }
+
+ ret = tpm_pcr_extend(chip, 23, digests);
+ if (ret != 0)
+ goto reset;
+
+ key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
+ GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
+ NULL);
+
+ if (IS_ERR(key)) {
+ ret = PTR_ERR(key);
+ goto reset;
+ }
+
+ ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
+ NULL);
+ if (ret < 0)
+ goto error;
+
+ payload = key->payload.data[0];
+
+ ret = swsusp_enc_dec(payload, header->digest, 1);
+ if (ret)
+ goto error;
+
+ memcpy(header->blob, payload->blob, payload->blob_len);
+ header->blob_len = payload->blob_len;
+
+error:
+ key_revoke(key);
+ key_put(key);
+reset:
+ kfree(digests);
+ tpm_pcr_reset(chip, 23);
+ return ret;
+}
+
+int swsusp_decrypt_digest(struct swsusp_header *header)
+{
+ const struct cred *cred = current_cred();
+ char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
+ struct trusted_key_payload *payload;
+ struct tpm_digest *digests = NULL;
+ char *blobstring = NULL;
+ char *keyinfo = NULL;
+ struct tpm_chip *chip;
+ struct key *key;
+ int i, ret;
+
+ chip = tpm_default_chip();
+
+ if (!chip)
+ return -ENODEV;
+
+ if (!(tpm_is_tpm2(chip)))
+ return -ENODEV;
+
+ ret = tpm_pcr_reset(chip, 23);
+ if (ret != 0)
+ return ret;
+
+ digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
+ GFP_KERNEL);
+ if (!digests)
+ goto reset;
+
+ for (i = 0; i <= chip->nr_allocated_banks; i++) {
+ digests[i].alg_id = chip->allocated_banks[i].alg_id;
+ if (digests[i].alg_id == digest.alg_id)
+ memcpy(&digests[i], &digest, sizeof(digest));
+ }
+
+ ret = tpm_pcr_extend(chip, 23, digests);
+ if (ret != 0)
+ goto reset;
+
+ blobstring = kmalloc(header->blob_len * 2, GFP_KERNEL);
+ if (!blobstring) {
+ ret = -ENOMEM;
+ goto reset;
+ }
+
+ bin2hex(blobstring, header->blob, header->blob_len);
+
+ keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring);
+ if (!keyinfo) {
+ ret = -ENOMEM;
+ goto reset;
+ }
+
+ key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
+ GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
+ NULL);
+
+ if (IS_ERR(key)) {
+ ret = PTR_ERR(key);
+ goto out;
+ }
+
+ ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
+ NULL);
+ if (ret < 0)
+ goto out;
+
+ payload = key->payload.data[0];
+
+ ret = swsusp_enc_dec(payload, header->digest, 0);
+
+out:
+ key_revoke(key);
+ key_put(key);
+reset:
+ kfree(keyinfo);
+ kfree(blobstring);
+ kfree(digests);
+ tpm_pcr_reset(chip, 23);
+ return ret;
+}
+
+int swsusp_digest_setup(struct swap_map_handle *handle)
+{
+ struct crypto_shash *tfm;
+ int ret;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ handle->desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!handle->desc) {
+ crypto_free_shash(tfm);
+ return -ENOMEM;
+ }
+
+ handle->desc->tfm = tfm;
+ ret = crypto_shash_init(handle->desc);
+ if (ret != 0) {
+ crypto_free_shash(tfm);
+ kfree(handle->desc);
+ return ret;
+ }
+
+ return 0;
+}
+
+void swsusp_digest_update(struct swap_map_handle *handle, char *buf,
+ size_t size)
+{
+ crypto_shash_update(handle->desc, buf, size);
+}
+
+void swsusp_digest_final(struct swap_map_handle *handle)
+{
+ crypto_shash_final(handle->desc, handle->digest);
+ crypto_free_shash(handle->desc->tfm);
+ kfree(handle->desc);
+}
+
+int secure_hibernation_available(void)
+{
+ struct tpm_chip *chip = tpm_default_chip();
+
+ if (!chip)
+ return -ENODEV;
+
+ if (!(tpm_is_tpm2(chip)))
+ return -ENODEV;
+
+ return 0;
+}
diff --git a/kernel/power/tpm.h b/kernel/power/tpm.h
new file mode 100644
index 000000000000..75b9140e5dc2
--- /dev/null
+++ b/kernel/power/tpm.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include "swap.h"
+
+#ifndef _POWER_TPM_H
+#define _POWER_TPM_H
+
+#ifdef CONFIG_SECURE_HIBERNATION
+int secure_hibernation_available(void);
+int swsusp_encrypt_digest(struct swsusp_header *header);
+int swsusp_decrypt_digest(struct swsusp_header *header);
+int swsusp_digest_setup(struct swap_map_handle *handle);
+void swsusp_digest_update(struct swap_map_handle *handle, char *buf,
+ size_t size);
+void swsusp_digest_final(struct swap_map_handle *handle);
+#else
+static inline int secure_hibernation_available(void)
+{
+ return -ENODEV;
+};
+static inline int swsusp_encrypt_digest(struct swsusp_header *header)
+{
+ return 0;
+}
+static inline int swsusp_decrypt_digest(struct swsusp_header *header)
+{
+ return 0;
+}
+static inline int swsusp_digest_setup(struct swap_map_handle *handle)
+{
+ return 0;
+}
+static inline void swsusp_digest_update(struct swap_map_handle *handle,
+ char *buf, size_t size) {};
+static inline void swsusp_digest_final(struct swap_map_handle *handle) {};
+#endif
+
+#endif /* _POWER_TPM_H */
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:40:27

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 8/9] pm: hibernate: Verify the digest encryption key

We want to ensure that the key used to encrypt the digest was created by
the kernel during hibernation. To do this we request that the TPM include
information about the value of PCR 23 at the time of key creation in the
sealed blob. On resume, we can ask the TPM to certify that the creation
data is accurate and then make sure that the PCR information in the blob
corresponds to the expected value. Since only the kernel can touch PCR
23, if an attacker generates a key themselves the value of PCR 23 will
have been different, allowing us to reject the key and boot normally
instead of resuming.

Signed-off-by: Matthew Garrett <[email protected]>
---
include/linux/tpm.h | 1 +
kernel/power/tpm.c | 150 +++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 148 insertions(+), 3 deletions(-)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index e2075e2242a0..f6970986b097 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -216,6 +216,7 @@ enum tpm2_command_codes {
TPM2_CC_SELF_TEST = 0x0143,
TPM2_CC_STARTUP = 0x0144,
TPM2_CC_SHUTDOWN = 0x0145,
+ TPM2_CC_CERTIFYCREATION = 0x014A,
TPM2_CC_NV_READ = 0x014E,
TPM2_CC_CREATE = 0x0153,
TPM2_CC_LOAD = 0x0157,
diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 953dcbdc56d8..34e6cfb98ce4 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -14,6 +14,12 @@ static struct tpm_digest digest = { .alg_id = TPM_ALG_SHA256,
0xf1, 0x22, 0x38, 0x6c, 0x33, 0xb1, 0x14, 0xb7, 0xec, 0x05,
0x5f, 0x49}};

+/* sha256(sha256(empty_pcr | digest)) */
+static char expected_digest[] = {0x2f, 0x96, 0xf2, 0x1b, 0x70, 0xa9, 0xe8,
+ 0x42, 0x25, 0x8e, 0x66, 0x07, 0xbe, 0xbc, 0xe3, 0x1f, 0x2c, 0x84, 0x4a,
+ 0x3f, 0x85, 0x17, 0x31, 0x47, 0x9a, 0xa5, 0x53, 0xbb, 0x23, 0x0c, 0x32,
+ 0xf3};
+
struct skcipher_def {
struct scatterlist sg;
struct crypto_skcipher *tfm;
@@ -21,6 +27,39 @@ struct skcipher_def {
struct crypto_wait wait;
};

+static int sha256_data(char *buf, int size, char *output)
+{
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ int ret;
+
+ tfm = crypto_alloc_shash("sha256", 0, 0);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ desc = kmalloc(sizeof(struct shash_desc) +
+ crypto_shash_descsize(tfm), GFP_KERNEL);
+ if (!desc) {
+ crypto_free_shash(tfm);
+ return -ENOMEM;
+ }
+
+ desc->tfm = tfm;
+ ret = crypto_shash_init(desc);
+ if (ret != 0) {
+ crypto_free_shash(tfm);
+ kfree(desc);
+ return ret;
+ }
+
+ crypto_shash_update(desc, buf, size);
+ crypto_shash_final(desc, output);
+ crypto_free_shash(desc->tfm);
+ kfree(desc);
+
+ return 0;
+}
+
static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
int enc)
{
@@ -86,6 +125,58 @@ static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
return ret;
}

+static int tpm_certify_creationdata(struct tpm_chip *chip,
+ struct trusted_key_payload *payload)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ int rc;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CERTIFYCREATION);
+ if (rc)
+ return rc;
+
+ /* Use TPM_RH_NULL for signHandle */
+ tpm_buf_append_u32(&buf, 0x40000007);
+
+ /* Object handle */
+ tpm_buf_append_u32(&buf, payload->blob_handle);
+
+ /* Auth */
+ tpm_buf_append_u32(&buf, 9);
+ tpm_buf_append_u32(&buf, TPM2_RS_PW);
+ tpm_buf_append_u16(&buf, 0);
+ tpm_buf_append_u8(&buf, 0);
+ tpm_buf_append_u16(&buf, 0);
+
+ /* Qualifying data */
+ tpm_buf_append_u16(&buf, 0);
+
+ /* Creation data hash */
+ tpm_buf_append_u16(&buf, payload->creation_hash_len);
+ tpm_buf_append(&buf, payload->creation_hash,
+ payload->creation_hash_len);
+
+ /* signature scheme */
+ tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+
+ /* creation ticket */
+ tpm_buf_append(&buf, payload->tk, payload->tk_len);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+
+ if (head->return_code != 0)
+ rc = -EINVAL;
+out:
+ tpm_buf_destroy(&buf);
+
+ return rc;
+}
+
int swsusp_encrypt_digest(struct swsusp_header *header)
{
const struct cred *cred = current_cred();
@@ -95,7 +186,7 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
struct key *key;
int ret, i;

- char *keyinfo = "new\t32\tkeyhandle=0x81000001";
+ char *keyinfo = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000";

chip = tpm_default_chip();

@@ -164,6 +255,7 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
struct trusted_key_payload *payload;
struct tpm_digest *digests = NULL;
+ char certhash[SHA256_DIGEST_SIZE];
char *blobstring = NULL;
char *keyinfo = NULL;
struct tpm_chip *chip;
@@ -184,8 +276,10 @@ int swsusp_decrypt_digest(struct swsusp_header *header)

digests = kcalloc(chip->nr_allocated_banks, sizeof(struct tpm_digest),
GFP_KERNEL);
- if (!digests)
+ if (!digests) {
+ ret = -ENOMEM;
goto reset;
+ }

for (i = 0; i <= chip->nr_allocated_banks; i++) {
digests[i].alg_id = chip->allocated_banks[i].alg_id;
@@ -227,8 +321,58 @@ int swsusp_decrypt_digest(struct swsusp_header *header)

payload = key->payload.data[0];

- ret = swsusp_enc_dec(payload, header->digest, 0);
+ ret = sha256_data(payload->creation, payload->creation_len, certhash);
+ if (ret < 0)
+ goto out;
+
+ if (memcmp(payload->creation_hash, certhash, SHA256_DIGEST_SIZE) != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = tpm_certify_creationdata(chip, payload);
+ if (ret != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* We now know that the creation data is authentic - parse it */
+
+ /* TPML_PCR_SELECTION.count */
+ if (be32_to_cpu(*(int *)payload->creation) != 1) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (be16_to_cpu(*(u16 *)&payload->creation[4]) != TPM_ALG_SHA256) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (*(char *)&payload->creation[6] != 3) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* PCR 23 selected */
+ if (be32_to_cpu(*(int *)&payload->creation[6]) != 0x03000080) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (be16_to_cpu(*(u16 *)&payload->creation[10]) !=
+ SHA256_DIGEST_SIZE) {
+ ret = -EINVAL;
+ goto out;
+ }

+ if (memcmp(&payload->creation[12], expected_digest,
+ SHA256_DIGEST_SIZE) != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = swsusp_enc_dec(payload, header->digest, 0);
out:
key_revoke(key);
key_put(key);
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 01:40:38

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 9/9] pm: hibernate: seal the encryption key with a PCR policy

The key blob is not secret, and by default the TPM will happily unseal
it regardless of system state. We can protect against that by sealing
the secret with a PCR policy - if the current PCR state doesn't match,
the TPM will refuse to release the secret. For now let's just seal it to
PCR 23. In the long term we may want a more flexible policy around this,
such as including PCR 7.

Signed-off-by: Matthew Garrett <[email protected]>
---
include/linux/tpm.h | 4 ++
kernel/power/tpm.c | 161 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 160 insertions(+), 5 deletions(-)

diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index f6970986b097..2e0141978c87 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -225,18 +225,22 @@ enum tpm2_command_codes {
TPM2_CC_CONTEXT_LOAD = 0x0161,
TPM2_CC_CONTEXT_SAVE = 0x0162,
TPM2_CC_FLUSH_CONTEXT = 0x0165,
+ TPM2_CC_START_AUTH_SESSION = 0x0176,
TPM2_CC_VERIFY_SIGNATURE = 0x0177,
TPM2_CC_GET_CAPABILITY = 0x017A,
TPM2_CC_GET_RANDOM = 0x017B,
TPM2_CC_PCR_READ = 0x017E,
+ TPM2_CC_POLICY_PCR = 0x017F,
TPM2_CC_PCR_EXTEND = 0x0182,
TPM2_CC_EVENT_SEQUENCE_COMPLETE = 0x0185,
TPM2_CC_HASH_SEQUENCE_START = 0x0186,
+ TPM2_CC_POLICY_GET_DIGEST = 0x0189,
TPM2_CC_CREATE_LOADED = 0x0191,
TPM2_CC_LAST = 0x0193, /* Spec 1.36 */
};

enum tpm2_permanent_handles {
+ TPM2_RH_NULL = 0x40000007,
TPM2_RS_PW = 0x40000009,
};

diff --git a/kernel/power/tpm.c b/kernel/power/tpm.c
index 34e6cfb98ce4..5de27c2f08be 100644
--- a/kernel/power/tpm.c
+++ b/kernel/power/tpm.c
@@ -125,6 +125,118 @@ static int swsusp_enc_dec(struct trusted_key_payload *payload, char *buf,
return ret;
}

+static int tpm_setup_policy(struct tpm_chip *chip, int *session_handle)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ char nonce[32] = {0x00};
+ int rc;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+ TPM2_CC_START_AUTH_SESSION);
+ if (rc)
+ return rc;
+
+ /* Decrypt key */
+ tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+
+ /* Auth entity */
+ tpm_buf_append_u32(&buf, TPM2_RH_NULL);
+
+ /* Nonce - blank is fine here */
+ tpm_buf_append_u16(&buf, sizeof(nonce));
+ tpm_buf_append(&buf, nonce, sizeof(nonce));
+
+ /* Encrypted secret - empty */
+ tpm_buf_append_u16(&buf, 0);
+
+ /* Policy type - session */
+ tpm_buf_append_u8(&buf, 0x01);
+
+ /* Encryption type - NULL */
+ tpm_buf_append_u16(&buf, TPM_ALG_NULL);
+
+ /* Hash type - SHA256 */
+ tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+
+ if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+ sizeof(int) + sizeof(u16) + sizeof(nonce)) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ *session_handle = be32_to_cpu(*(int *)&buf.data[10]);
+ memcpy(nonce, &buf.data[16], sizeof(nonce));
+
+ tpm_buf_destroy(&buf);
+
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_PCR);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, *session_handle);
+
+ /* PCR digest - read from the PCR, we'll verify creation data later */
+ tpm_buf_append_u16(&buf, 0);
+
+ /* One PCR */
+ tpm_buf_append_u32(&buf, 1);
+
+ /* SHA256 banks */
+ tpm_buf_append_u16(&buf, TPM_ALG_SHA256);
+
+ /* Select PCR 23 */
+ tpm_buf_append_u32(&buf, 0x03000080);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+ if (rc)
+ goto out;
+
+out:
+ tpm_buf_destroy(&buf);
+ return rc;
+}
+
+static int tpm_policy_get_digest(struct tpm_chip *chip, int handle,
+ char *digest)
+{
+ struct tpm_header *head;
+ struct tpm_buf buf;
+ int rc;
+
+ rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_POLICY_GET_DIGEST);
+ if (rc)
+ return rc;
+
+ tpm_buf_append_u32(&buf, handle);
+
+ rc = tpm_send(chip, buf.data, tpm_buf_length(&buf));
+
+ if (rc)
+ goto out;
+
+ head = (struct tpm_header *)buf.data;
+ if (be32_to_cpu(head->length) != sizeof(struct tpm_header) +
+ sizeof(u16) + SHA256_DIGEST_SIZE) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ memcpy(digest, &buf.data[12], SHA256_DIGEST_SIZE);
+out:
+ tpm_buf_destroy(&buf);
+
+ return rc;
+}
+
static int tpm_certify_creationdata(struct tpm_chip *chip,
struct trusted_key_payload *payload)
{
@@ -182,11 +294,14 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
const struct cred *cred = current_cred();
struct trusted_key_payload *payload;
struct tpm_digest *digests = NULL;
+ char policy[SHA256_DIGEST_SIZE];
+ char *policydigest = NULL;
struct tpm_chip *chip;
struct key *key;
+ int session_handle;
int ret, i;
-
- char *keyinfo = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000";
+ char *keyinfo = NULL;
+ char *keytemplate = "new\t32\tkeyhandle=0x81000001\tcreationpcrs=0x00800000\tpolicydigest=%s";

chip = tpm_default_chip();

@@ -213,10 +328,35 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
memcpy(&digests[i], &digest, sizeof(digest));
}

+ policydigest = kmalloc(SHA256_DIGEST_SIZE * 2 + 1, GFP_KERNEL);
+ if (!policydigest) {
+ ret = -ENOMEM;
+ goto reset;
+ }
+
ret = tpm_pcr_extend(chip, 23, digests);
if (ret != 0)
goto reset;

+ ret = tpm_setup_policy(chip, &session_handle);
+
+ if (ret != 0)
+ goto reset;
+
+ ret = tpm_policy_get_digest(chip, session_handle, policy);
+
+ if (ret != 0)
+ goto reset;
+
+ bin2hex(policydigest, policy, SHA256_DIGEST_SIZE);
+ policydigest[64] = '\0';
+
+ keyinfo = kasprintf(GFP_KERNEL, keytemplate, policydigest);
+ if (!keyinfo) {
+ ret = -ENOMEM;
+ goto reset;
+ }
+
key = key_alloc(&key_type_trusted, "swsusp", GLOBAL_ROOT_UID,
GLOBAL_ROOT_GID, cred, 0, KEY_ALLOC_NOT_IN_QUOTA,
NULL);
@@ -228,6 +368,7 @@ int swsusp_encrypt_digest(struct swsusp_header *header)

ret = key_instantiate_and_link(key, keyinfo, strlen(keyinfo) + 1, NULL,
NULL);
+
if (ret < 0)
goto error;

@@ -244,6 +385,8 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
key_revoke(key);
key_put(key);
reset:
+ kfree(keyinfo);
+ kfree(policydigest);
kfree(digests);
tpm_pcr_reset(chip, 23);
return ret;
@@ -252,13 +395,14 @@ int swsusp_encrypt_digest(struct swsusp_header *header)
int swsusp_decrypt_digest(struct swsusp_header *header)
{
const struct cred *cred = current_cred();
- char *keytemplate = "load\t%s\tkeyhandle=0x81000001";
+ char *keytemplate = "load\t%s\tkeyhandle=0x81000001\tpolicyhandle=0x%x";
struct trusted_key_payload *payload;
struct tpm_digest *digests = NULL;
char certhash[SHA256_DIGEST_SIZE];
char *blobstring = NULL;
char *keyinfo = NULL;
struct tpm_chip *chip;
+ int session_handle;
struct key *key;
int i, ret;

@@ -291,15 +435,22 @@ int swsusp_decrypt_digest(struct swsusp_header *header)
if (ret != 0)
goto reset;

- blobstring = kmalloc(header->blob_len * 2, GFP_KERNEL);
+ ret = tpm_setup_policy(chip, &session_handle);
+
+ if (ret != 0)
+ goto reset;
+
+ blobstring = kmalloc(header->blob_len * 2 + 1, GFP_KERNEL);
if (!blobstring) {
ret = -ENOMEM;
goto reset;
}

bin2hex(blobstring, header->blob, header->blob_len);
+ blobstring[header->blob_len * 2] = '\0';

- keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring);
+ keyinfo = kasprintf(GFP_KERNEL, keytemplate, blobstring,
+ session_handle);
if (!keyinfo) {
ret = -ENOMEM;
goto reset;
--
2.30.0.617.g56c4b15f3c-goog

2021-02-20 02:22:04

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

Hi--

On 2/19/21 5:32 PM, Matthew Garrett wrote:
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index a7320f07689d..0279cc10f319 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -92,6 +92,21 @@ config HIBERNATION_SNAPSHOT_DEV
>
> If in doubt, say Y.
>
> +config SECURE_HIBERNATION
> + bool "Implement secure hibernation support"
> + depends on HIBERNATION && TCG_TPM
> + select KEYS
> + select TRUSTED_KEYS
> + select CRYPTO
> + select CRYPTO_SHA256
> + select CRYPTO_AES
> + select TCG_TPM_RESTRICT_PCR
> + help
> + Use a TPM-backed key to securely determine whether a hibernation
> + image was written out by the kernel and has not been tampered with.
> + This requires a TCG-compliant TPM2 device, which is present on most
> + modern hardware.

Please follow coding-style for Kconfig files:

from Documentation/process/coding-style.rst, section 10):

For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different. Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces.


Also, one feature should not be responsible for enabling other "subsystems,"
such as KEYS and CRYPTO. They should instead be listed as dependencies.


--
~Randy

2021-02-20 02:54:05

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/9] tpm: Add support for in-kernel resetting of PCRs

On Sat, Feb 20, 2021 at 01:32:47AM +0000, Matthew Garrett wrote:

Perhaps, prepend with a paragraph elaborating the background just a bit:

"When entering to hibernation, PCR 23 will be used to store a known value.
This value defines a policy for the decryption of the key used to encrypt
the hibernate image."

At minimum that. You can of course edit this however you see fit.

> Add an internal command for resetting a PCR.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> 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 1621ce818705..17b8643ee109 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 947d1db0a5cc..746f7696bdc0 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -176,6 +176,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);
> @@ -220,6 +221,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 ca7158fa6e6c..36990e9d2dc1 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 eff1f12d981a..9609ae8086c6 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 8f4ff39f51e7..e2075e2242a0 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -211,6 +211,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,
> @@ -399,6 +400,7 @@ static inline u32 tpm2_rc_value(u32 rc)
> extern int tpm_is_tpm2(struct tpm_chip *chip);
> 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);
> @@ -417,6 +419,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)
> {
> --
> 2.30.0.617.g56c4b15f3c-goog
>
>

/Jarkko

2021-02-20 03:09:25

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> Performing any sort of state validation of a sealed TPM blob requires
> being able to access the individual members in the response. Parse the
> blob sufficiently to be able to stash pointers to each member, along
> with the length.
>
> Signed-off-by: Matthew Garrett <[email protected]>

I'll just say LGTM for now. Did not see anything obviously wrong in
the code change (and does make sense to nitpick minor things just
yet).

Need to understand the whole use case just a little bit better.

/Jarkko

2021-02-20 03:10:56

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

On Sat, Feb 20, 2021 at 01:32:51AM +0000, Matthew Garrett wrote:
> When TPMs generate keys, they can also generate some information
> describing the state of the PCRs at creation time. This data can then
> later be certified by the TPM, allowing verification of the PCR values.
> This allows us to determine the state of the system at the time a key
> was generated. Add an additional argument to the trusted key creation
> options, allowing the user to provide the set of PCRs that should have
> their values incorporated into the creation data.
>
> Signed-off-by: Matthew Garrett <[email protected]>

LGTM too.

Something popped into mind: could we make PCR 23 reservation dynamic
instead of a config option.

E.g. if the user space uses it, then it's dirty and hibernate will
fail. I really dislike the static compilation time firewall on it.

/Jarkko

2021-02-21 19:49:53

by Ben Boeckel

[permalink] [raw]
Subject: Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

On Sat, Feb 20, 2021 at 05:09:07 +0200, Jarkko Sakkinen wrote:
> Something popped into mind: could we make PCR 23 reservation dynamic
> instead of a config option.
>
> E.g. if the user space uses it, then it's dirty and hibernate will
> fail. I really dislike the static compilation time firewall on it.

I don't know the threat model here, but couldn't hibernation then be
blocked by userspace using PCR 23 in some way (thus becoming a Denial of
Service)? Are elevated permissions required to use PCR values?

--Ben

2021-02-22 07:40:49

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

On Sat, Feb 20, 2021 at 05:05:36AM +0200, Jarkko Sakkinen wrote:
> On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> > Performing any sort of state validation of a sealed TPM blob requires
> > being able to access the individual members in the response. Parse the
> > blob sufficiently to be able to stash pointers to each member, along
> > with the length.
> >
> > Signed-off-by: Matthew Garrett <[email protected]>
>
> I'll just say LGTM for now. Did not see anything obviously wrong in
> the code change (and does make sense to nitpick minor things just
> yet).
>
> Need to understand the whole use case just a little bit better.

I wrote this up with some more detail at
https://mjg59.dreamwidth.org/55845.html - it seemed longer than
appropriate for a commit message, but if you'd like more detail
somewhere I can certainly add it.

2021-02-22 07:46:00

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 7/9] pm: hibernate: Optionally use TPM-backed keys to protect image integrity

On Fri, Feb 19, 2021 at 06:20:13PM -0800, Randy Dunlap wrote:
> For all of the Kconfig* configuration files throughout the source tree,
> the indentation is somewhat different. Lines under a ``config`` definition
> are indented with one tab, while help text is indented an additional two
> spaces.

Whoops, I've no idea how I screwed that up. I'll fix for V2, thanks!

> Also, one feature should not be responsible for enabling other "subsystems,"
> such as KEYS and CRYPTO. They should instead be listed as dependencies.

ACK.

2021-02-22 07:46:00

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 5/9] security: keys: trusted: Allow storage of PCR values in creation data

On Sat, Feb 20, 2021 at 05:09:07AM +0200, Jarkko Sakkinen wrote:

> Something popped into mind: could we make PCR 23 reservation dynamic
> instead of a config option.
>
> E.g. if the user space uses it, then it's dirty and hibernate will
> fail. I really dislike the static compilation time firewall on it.

We can fail hibernation if userland hasn't flagged things, but the
concern is that if you hibernate with PCR 23 blocking enabled and then
reboot with the blocking disabled, userland can obtain the blob from the
hibernation image, extend PCR 23, modify the image and use the key
they've recovered to make it look legitimate, enable PCR 23 blocking
again and then resume into their own code.

2021-02-24 17:27:14

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 3/9] security: keys: trusted: Parse out individual components of the key blob

On Mon, Feb 22, 2021 at 07:36:27AM +0000, Matthew Garrett wrote:
> On Sat, Feb 20, 2021 at 05:05:36AM +0200, Jarkko Sakkinen wrote:
> > On Sat, Feb 20, 2021 at 01:32:49AM +0000, Matthew Garrett wrote:
> > > Performing any sort of state validation of a sealed TPM blob requires
> > > being able to access the individual members in the response. Parse the
> > > blob sufficiently to be able to stash pointers to each member, along
> > > with the length.
> > >
> > > Signed-off-by: Matthew Garrett <[email protected]>
> >
> > I'll just say LGTM for now. Did not see anything obviously wrong in
> > the code change (and does make sense to nitpick minor things just
> > yet).
> >
> > Need to understand the whole use case just a little bit better.
>
> I wrote this up with some more detail at
> https://mjg59.dreamwidth.org/55845.html - it seemed longer than
> appropriate for a commit message, but if you'd like more detail
> somewhere I can certainly add it.

Thanks (bookmarked). I'll read it before reviewing +1 version.

Requiring a config flag is something that slows down adoption in the
stock kernels.

Since we are talking about hibernate the decision whether to have this
feature set, does not have to be something that needs to be changed
dynamically to a running system.

So: maybe the best compromise would be to have it kernel command line
option? That way it's easier feature to adapt (e.g. with GRUB
configuration) and to enable in the kernel.

/Jarkko

2021-05-04 22:24:14

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

Does anyone know if this series is abandoned, or is Matthew planning
to do another spin? Email to [email protected] bounces.

-Evan

2021-05-05 03:28:18

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> Does anyone know if this series is abandoned, or is Matthew planning
> to do another spin? Email to [email protected] bounces.
>
> -Evan

Good question.

It could be that because James' patches did not end up to 5.12, but 5.13
instead, Matthew has just put this into hold for a while.

I mean the review comments I gave, were relatively cosmetic.

If I was the author, that's at least I might have done...

/Jarkko

2021-05-05 03:34:38

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

On Wed, May 05, 2021 at 06:18:15AM +0300, Jarkko Sakkinen wrote:
> On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> > Does anyone know if this series is abandoned, or is Matthew planning
> > to do another spin? Email to [email protected] bounces.
> >
> > -Evan
>
> Good question.
>
> It could be that because James' patches did not end up to 5.12, but 5.13
> instead, Matthew has just put this into hold for a while.
>
> I mean the review comments I gave, were relatively cosmetic.
>
> If I was the author, that's at least I might have done...

Few months can pass surprisingly fast if you have all sorts of stuff going
on :-)

/Jarkko

2021-05-05 18:08:28

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH 0/9] Enable hibernation when Lockdown is enabled

On Tue, May 4, 2021 at 8:19 PM Jarkko Sakkinen <[email protected]> wrote:
>
> On Wed, May 05, 2021 at 06:18:15AM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 04, 2021 at 02:56:35PM -0700, Evan Green wrote:
> > > Does anyone know if this series is abandoned, or is Matthew planning
> > > to do another spin? Email to [email protected] bounces.
> > >
> > > -Evan
> >
> > Good question.
> >
> > It could be that because James' patches did not end up to 5.12, but 5.13
> > instead, Matthew has just put this into hold for a while.

Ah, thanks for the context. Which patches are those?

> >
> > I mean the review comments I gave, were relatively cosmetic.
> >
> > If I was the author, that's at least I might have done...
>
> Few months can pass surprisingly fast if you have all sorts of stuff going
> on :-)

Hehe or an entire year can blur by if you're not careful.

>
> /Jarkko

2021-05-05 18:20:11

by Evan Green

[permalink] [raw]
Subject: Re: [PATCH 6/9] pm: hibernate: Optionally store and verify a hash of the image

On Fri, Feb 19, 2021 at 5:36 PM Matthew Garrett
<[email protected]> wrote:
>
> Calculate and store a cryptographically secure hash of the hibernation
> image if SF_VERIFY_IMAGE is set in the hibernation flags. This allows
> detection of a corrupt image, but has the disadvantage that it requires
> the blocks be read in in linear order.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> kernel/power/power.h | 1 +
> kernel/power/swap.c | 131 +++++++++++++++++++++++++++++++++++--------
> 2 files changed, 110 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index 778bf431ec02..b8e00b9dcee8 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -168,6 +168,7 @@ extern int swsusp_swap_in_use(void);
> #define SF_PLATFORM_MODE 1
> #define SF_NOCOMPRESS_MODE 2
> #define SF_CRC32_MODE 4
> +#define SF_VERIFY_IMAGE 8
>
> /* kernel/power/hibernate.c */
> extern int swsusp_check(void);
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 72e33054a2e1..a13241a20567 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -31,6 +31,8 @@
> #include <linux/kthread.h>
> #include <linux/crc32.h>
> #include <linux/ktime.h>
> +#include <crypto/hash.h>
> +#include <crypto/sha2.h>
>
> #include "power.h"
>
> @@ -95,17 +97,20 @@ struct swap_map_page_list {
> struct swap_map_handle {
> struct swap_map_page *cur;
> struct swap_map_page_list *maps;
> + struct shash_desc *desc;
> sector_t cur_swap;
> sector_t first_sector;
> unsigned int k;
> unsigned long reqd_free_pages;
> u32 crc32;
> + u8 digest[SHA256_DIGEST_SIZE];
> };
>
> struct swsusp_header {
> char reserved[PAGE_SIZE - 20 - sizeof(sector_t) - sizeof(int) -
> - sizeof(u32)];
> + sizeof(u32) - SHA256_DIGEST_SIZE];
> u32 crc32;
> + u8 digest[SHA256_DIGEST_SIZE];
> sector_t image;
> unsigned int flags; /* Flags to pass to the "boot" kernel */
> char orig_sig[10];
> @@ -305,6 +310,9 @@ static blk_status_t hib_wait_io(struct hib_bio_batch *hb)
> * We are relying on the behavior of blk_plug that a thread with
> * a plug will flush the plug list before sleeping.
> */
> + if (!hb)
> + return 0;
> +
> wait_event(hb->wait, atomic_read(&hb->count) == 0);
> return blk_status_to_errno(hb->error);
> }
> @@ -327,6 +335,8 @@ static int mark_swapfiles(struct swap_map_handle *handle, unsigned int flags)
> swsusp_header->flags = flags;
> if (flags & SF_CRC32_MODE)
> swsusp_header->crc32 = handle->crc32;
> + memcpy(swsusp_header->digest, handle->digest,
> + SHA256_DIGEST_SIZE);
> error = hib_submit_io(REQ_OP_WRITE, REQ_SYNC,
> swsusp_resume_block, swsusp_header, NULL);
> } else {
> @@ -417,6 +427,7 @@ static void release_swap_writer(struct swap_map_handle *handle)
> static int get_swap_writer(struct swap_map_handle *handle)
> {
> int ret;
> + struct crypto_shash *tfm;
>
> ret = swsusp_swap_check();
> if (ret) {
> @@ -437,7 +448,28 @@ static int get_swap_writer(struct swap_map_handle *handle)
> handle->k = 0;
> handle->reqd_free_pages = reqd_free_pages();
> handle->first_sector = handle->cur_swap;
> +
> + tfm = crypto_alloc_shash("sha256", 0, 0);
> + if (IS_ERR(tfm)) {
> + ret = -EINVAL;
> + goto err_rel;
> + }
> + handle->desc = kmalloc(sizeof(struct shash_desc) +
> + crypto_shash_descsize(tfm), GFP_KERNEL);
> + if (!handle->desc) {
> + ret = -ENOMEM;
> + goto err_rel;
> + }
> +
> + handle->desc->tfm = tfm;
> +
> + ret = crypto_shash_init(handle->desc);
> + if (ret != 0)
> + goto err_free;
> +
> return 0;
> +err_free:
> + kfree(handle->desc);
> err_rel:
> release_swap_writer(handle);
> err_close:
> @@ -446,7 +478,7 @@ static int get_swap_writer(struct swap_map_handle *handle)
> }
>
> static int swap_write_page(struct swap_map_handle *handle, void *buf,
> - struct hib_bio_batch *hb)
> + struct hib_bio_batch *hb, bool hash)
> {
> int error = 0;
> sector_t offset;
> @@ -454,6 +486,7 @@ static int swap_write_page(struct swap_map_handle *handle, void *buf,
> if (!handle->cur)
> return -EINVAL;
> offset = alloc_swapdev_block(root_swap);
> + crypto_shash_update(handle->desc, buf, PAGE_SIZE);

Was this supposed to be conditionalized behind the new parameter, ie:
if (hash) crypto_shash_update()? If so, the same comment would apply
to the read as well.