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

2021-02-21 Thread Matthew Garrett
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.


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

2021-02-19 Thread Randy Dunlap
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



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

2021-02-19 Thread Matthew Garrett
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 
---
 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 
 
 #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 
 #include 
 #include 
-#include 
 
 #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 */
-   charorig_sig[10];
-   charsig[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);
+