Message ID | 20190103143227.9138-4-jlee@suse.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | Encryption and authentication for hibernate snapshot image | expand |
Am Donnerstag, 3. Januar 2019, 15:32:25 CET schrieb Lee, Chun-Yi: Hi Chun, > To protect the secret in memory snapshot image, this patch adds the > logic to encrypt snapshot pages by AES-CTR. Using AES-CTR is because > it's simple, fast and parallelizable. But this patch didn't implement > parallel encryption. > > The encrypt key is derived from the snapshot key. And the initialization > vector will be kept in snapshot header for resuming. > > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Chen Yu <yu.c.chen@intel.com> > Cc: Oliver Neukum <oneukum@suse.com> > Cc: Ryan Chen <yu.chen.surf@gmail.com> > Cc: David Howells <dhowells@redhat.com> > Cc: Giovanni Gherdovich <ggherdovich@suse.cz> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Jann Horn <jannh@google.com> > Cc: Andy Lutomirski <luto@kernel.org> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > --- > kernel/power/hibernate.c | 8 ++- > kernel/power/power.h | 6 ++ > kernel/power/snapshot.c | 154 > ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 164 > insertions(+), 4 deletions(-) > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index 0dda6a9f0af1..5ac2ab6f4a0e 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -275,10 +275,14 @@ static int create_image(int platform_mode) > if (error) > return error; > > + error = snapshot_prepare_crypto(false, true); > + if (error) > + goto finish_hash; > + > error = dpm_suspend_end(PMSG_FREEZE); > if (error) { > pr_err("Some devices failed to power down, aborting hibernation\n"); > - goto finish_hash; > + goto finish_crypto; > } > > error = platform_pre_snapshot(platform_mode); > @@ -335,6 +339,8 @@ static int create_image(int platform_mode) > dpm_resume_start(in_suspend ? > (error ? PMSG_RECOVER : PMSG_THAW) : PMSG_RESTORE); > > + finish_crypto: > + snapshot_finish_crypto(); > finish_hash: > snapshot_finish_hash(); > > diff --git a/kernel/power/power.h b/kernel/power/power.h > index c614b0a294e3..41263fdd3a54 100644 > --- a/kernel/power/power.h > +++ b/kernel/power/power.h > @@ -5,6 +5,7 @@ > #include <linux/freezer.h> > #include <linux/compiler.h> > #include <crypto/sha.h> > +#include <crypto/aes.h> > > /* The max size of encrypted key blob */ > #define KEY_BLOB_BUFF_LEN 512 > @@ -24,6 +25,7 @@ struct swsusp_info { > unsigned long pages; > unsigned long size; > unsigned long trampoline_pfn; > + u8 iv[AES_BLOCK_SIZE]; > u8 signature[SNAPSHOT_DIGEST_SIZE]; > } __aligned(PAGE_SIZE); > > @@ -44,6 +46,8 @@ extern void __init hibernate_image_size_init(void); > #ifdef CONFIG_HIBERNATION_ENC_AUTH > /* kernel/power/snapshot.c */ > extern int snapshot_image_verify_decrypt(void); > +extern int snapshot_prepare_crypto(bool may_sleep, bool create_iv); > +extern void snapshot_finish_crypto(void); > extern int snapshot_prepare_hash(bool may_sleep); > extern void snapshot_finish_hash(void); > /* kernel/power/snapshot_key.c */ > @@ -53,6 +57,8 @@ extern int snapshot_get_auth_key(u8 *auth_key, bool > may_sleep); extern int snapshot_get_enc_key(u8 *enc_key, bool may_sleep); > #else > static inline int snapshot_image_verify_decrypt(void) { return 0; } > +static inline int snapshot_prepare_crypto(bool may_sleep, bool create_iv) { > return 0; } +static inline void snapshot_finish_crypto(void) {} > static inline int snapshot_prepare_hash(bool may_sleep) { return 0; } > static inline void snapshot_finish_hash(void) {} > static inline int snapshot_key_init(void) { return 0; } > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c > index e817c035f378..cd10ab5e4850 100644 > --- a/kernel/power/snapshot.c > +++ b/kernel/power/snapshot.c > @@ -41,7 +41,11 @@ > #include <asm/tlbflush.h> > #include <asm/io.h> > #ifdef CONFIG_HIBERNATION_ENC_AUTH > +#include <linux/random.h> > +#include <linux/scatterlist.h> > +#include <crypto/aes.h> > #include <crypto/hash.h> > +#include <crypto/skcipher.h> > #endif > > #include "power.h" > @@ -1413,6 +1417,127 @@ static unsigned int nr_copy_pages; > static void **h_buf; > > #ifdef CONFIG_HIBERNATION_ENC_AUTH > +static struct skcipher_request *sk_req; > +static u8 iv[AES_BLOCK_SIZE]; May I ask for a different name here? The variable iv is used throughout the kernel crypto API and it is always a challenge when doing code reviews to trace the right variable when using common names :-) > +static void *c_buffer; > + > +static void init_iv(struct swsusp_info *info) > +{ > + memcpy(info->iv, iv, AES_BLOCK_SIZE); > +} > + > +static void load_iv(struct swsusp_info *info) > +{ > + memcpy(iv, info->iv, AES_BLOCK_SIZE); > +} > + > +int snapshot_prepare_crypto(bool may_sleep, bool create_iv) > +{ > + char enc_key[DERIVED_KEY_SIZE]; > + struct crypto_skcipher *tfm; > + int ret = 0; > + > + ret = snapshot_get_enc_key(enc_key, may_sleep); > + if (ret) { > + pr_warn_once("enc key is invalid\n"); > + return -EINVAL; > + } > + > + c_buffer = (void *)get_zeroed_page(GFP_KERNEL); > + if (!c_buffer) { > + pr_err("Allocate crypto buffer page failed\n"); > + return -ENOMEM; > + } > + > + tfm = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); What is the reason for choosing CTR-AES to store data on disk? > + if (IS_ERR(tfm)) { > + ret = PTR_ERR(tfm); > + pr_err("failed to allocate skcipher (%d)\n", ret); > + goto alloc_fail; > + } > + > + ret = crypto_skcipher_setkey(tfm, enc_key, AES_MAX_KEY_SIZE); > + if (ret) { > + pr_err("failed to setkey (%d)\n", ret); > + goto set_fail; > + } > + > + sk_req = skcipher_request_alloc(tfm, GFP_KERNEL); > + if (!sk_req) { > + pr_err("failed to allocate request\n"); > + ret = -ENOMEM; > + goto set_fail; > + } > + if (may_sleep) > + skcipher_request_set_callback(sk_req, CRYPTO_TFM_REQ_MAY_SLEEP, > + NULL, NULL); > + if (create_iv) > + get_random_bytes(iv, AES_BLOCK_SIZE); > + > + return 0; > + > +set_fail: > + crypto_free_skcipher(tfm); > +alloc_fail: > + __free_page(c_buffer); May I recommend to memzero_explicit(enc_key)? > + > + return ret; > +} > + > +void snapshot_finish_crypto(void) > +{ > + struct crypto_skcipher *tfm; > + > + if (!sk_req) > + return; > + > + tfm = crypto_skcipher_reqtfm(sk_req); > + skcipher_request_zero(sk_req); > + skcipher_request_free(sk_req); > + crypto_free_skcipher(tfm); > + __free_page(c_buffer); > + sk_req = NULL; > +} > + > +static int encrypt_data_page(void *hash_buffer) > +{ > + struct scatterlist src[1], dst[1]; > + u8 iv_tmp[AES_BLOCK_SIZE]; > + int ret = 0; > + > + if (!sk_req) > + return 0; > + > + memcpy(iv_tmp, iv, sizeof(iv)); Why do you copy the IV? If I see that right, we would have a key/counter collision as follows: 1. you copy the IV into a tmp variable 2. CTR AES is invoked which updates iv_tmp 3. iv_tmp is discarded 4. a repeated invocation of this function would again use the initially set IV to copy it into iv_tmp which means that the subsequent cipher operation uses yet again the same IV. If my hunch is correct, the cryptographic strength of the cipher is defeated. > + sg_init_one(src, hash_buffer, PAGE_SIZE); > + sg_init_one(dst, c_buffer, PAGE_SIZE); > + skcipher_request_set_crypt(sk_req, src, dst, PAGE_SIZE, iv_tmp); > + ret = crypto_skcipher_encrypt(sk_req); > + > + copy_page(hash_buffer, c_buffer); > + memset(c_buffer, 0, PAGE_SIZE); > + > + return ret; > +} > + > +static int decrypt_data_page(void *encrypted_page) This function looks almost identical to encrypt_data_page - may I suggest to collapse it into one function? > +{ > + struct scatterlist src[1], dst[1]; > + u8 iv_tmp[AES_BLOCK_SIZE]; > + int ret = 0; > + > + memcpy(iv_tmp, iv, sizeof(iv)); > + sg_init_one(src, encrypted_page, PAGE_SIZE); > + sg_init_one(dst, c_buffer, PAGE_SIZE); > + skcipher_request_set_crypt(sk_req, src, dst, PAGE_SIZE, iv_tmp); > + ret = crypto_skcipher_decrypt(sk_req); > + > + copy_page(encrypted_page, c_buffer); > + memset(c_buffer, 0, PAGE_SIZE); > + > + return ret; > +} > + > /* > * Signature of snapshot image > */ > @@ -1508,22 +1633,30 @@ int snapshot_image_verify_decrypt(void) > if (ret || !s4_verify_desc) > goto error_prep; > > + ret = snapshot_prepare_crypto(true, false); > + if (ret) > + goto error_prep; > + > for (i = 0; i < nr_copy_pages; i++) { > ret = crypto_shash_update(s4_verify_desc, *(h_buf + i), PAGE_SIZE); > if (ret) > - goto error_shash; > + goto error_shash_crypto; > + ret = decrypt_data_page(*(h_buf + i)); > + if (ret) > + goto error_shash_crypto; > } > > ret = crypto_shash_final(s4_verify_desc, s4_verify_digest); > if (ret) > - goto error_shash; > + goto error_shash_crypto; > > pr_debug("Signature %*phN\n", SNAPSHOT_DIGEST_SIZE, signature); > pr_debug("Digest %*phN\n", SNAPSHOT_DIGEST_SIZE, s4_verify_digest); > if (memcmp(signature, s4_verify_digest, SNAPSHOT_DIGEST_SIZE)) > ret = -EKEYREJECTED; > > - error_shash: > + error_shash_crypto: > + snapshot_finish_crypto(); > snapshot_finish_hash(); > > error_prep: > @@ -1564,6 +1697,17 @@ __copy_data_pages(struct memory_bitmap *copy_bm, > struct memory_bitmap *orig_bm) crypto_buffer = page_address(d_page); > } > > + /* Encrypt hashed page */ > + encrypt_data_page(crypto_buffer); > + > + /* Copy encrypted buffer to destination page in high memory */ > + if (PageHighMem(d_page)) { > + void *kaddr = kmap_atomic(d_page); > + > + copy_page(kaddr, crypto_buffer); > + kunmap_atomic(kaddr); > + } > + > /* Generate digest */ > if (!s4_verify_desc) > continue; > @@ -1638,6 +1782,8 @@ __copy_data_pages(struct memory_bitmap *copy_bm, > struct memory_bitmap *orig_bm) } > > static inline void alloc_h_buf(void) {} > +static inline void init_iv(struct swsusp_info *info) {} > +static inline void load_iv(struct swsusp_info *info) {} > static inline void init_signature(struct swsusp_info *info) {} > static inline void load_signature(struct swsusp_info *info) {} > static inline void init_sig_verify(struct trampoline *t) {} > @@ -2286,6 +2432,7 @@ static int init_header(struct swsusp_info *info) > info->size = info->pages; > info->size <<= PAGE_SHIFT; > info->trampoline_pfn = page_to_pfn(virt_to_page(trampoline_virt)); > + init_iv(info); > init_signature(info); > return init_header_complete(info); > } > @@ -2524,6 +2671,7 @@ static int load_header(struct swsusp_info *info) > nr_copy_pages = info->image_pages; > nr_meta_pages = info->pages - info->image_pages - 1; > trampoline_pfn = info->trampoline_pfn; > + load_iv(info); > load_signature(info); > } > return error; Ciao Stephan
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index 0dda6a9f0af1..5ac2ab6f4a0e 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -275,10 +275,14 @@ static int create_image(int platform_mode) if (error) return error; + error = snapshot_prepare_crypto(false, true); + if (error) + goto finish_hash; + error = dpm_suspend_end(PMSG_FREEZE); if (error) { pr_err("Some devices failed to power down, aborting hibernation\n"); - goto finish_hash; + goto finish_crypto; } error = platform_pre_snapshot(platform_mode); @@ -335,6 +339,8 @@ static int create_image(int platform_mode) dpm_resume_start(in_suspend ? (error ? PMSG_RECOVER : PMSG_THAW) : PMSG_RESTORE); + finish_crypto: + snapshot_finish_crypto(); finish_hash: snapshot_finish_hash(); diff --git a/kernel/power/power.h b/kernel/power/power.h index c614b0a294e3..41263fdd3a54 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -5,6 +5,7 @@ #include <linux/freezer.h> #include <linux/compiler.h> #include <crypto/sha.h> +#include <crypto/aes.h> /* The max size of encrypted key blob */ #define KEY_BLOB_BUFF_LEN 512 @@ -24,6 +25,7 @@ struct swsusp_info { unsigned long pages; unsigned long size; unsigned long trampoline_pfn; + u8 iv[AES_BLOCK_SIZE]; u8 signature[SNAPSHOT_DIGEST_SIZE]; } __aligned(PAGE_SIZE); @@ -44,6 +46,8 @@ extern void __init hibernate_image_size_init(void); #ifdef CONFIG_HIBERNATION_ENC_AUTH /* kernel/power/snapshot.c */ extern int snapshot_image_verify_decrypt(void); +extern int snapshot_prepare_crypto(bool may_sleep, bool create_iv); +extern void snapshot_finish_crypto(void); extern int snapshot_prepare_hash(bool may_sleep); extern void snapshot_finish_hash(void); /* kernel/power/snapshot_key.c */ @@ -53,6 +57,8 @@ extern int snapshot_get_auth_key(u8 *auth_key, bool may_sleep); extern int snapshot_get_enc_key(u8 *enc_key, bool may_sleep); #else static inline int snapshot_image_verify_decrypt(void) { return 0; } +static inline int snapshot_prepare_crypto(bool may_sleep, bool create_iv) { return 0; } +static inline void snapshot_finish_crypto(void) {} static inline int snapshot_prepare_hash(bool may_sleep) { return 0; } static inline void snapshot_finish_hash(void) {} static inline int snapshot_key_init(void) { return 0; } diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index e817c035f378..cd10ab5e4850 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -41,7 +41,11 @@ #include <asm/tlbflush.h> #include <asm/io.h> #ifdef CONFIG_HIBERNATION_ENC_AUTH +#include <linux/random.h> +#include <linux/scatterlist.h> +#include <crypto/aes.h> #include <crypto/hash.h> +#include <crypto/skcipher.h> #endif #include "power.h" @@ -1413,6 +1417,127 @@ static unsigned int nr_copy_pages; static void **h_buf; #ifdef CONFIG_HIBERNATION_ENC_AUTH +static struct skcipher_request *sk_req; +static u8 iv[AES_BLOCK_SIZE]; +static void *c_buffer; + +static void init_iv(struct swsusp_info *info) +{ + memcpy(info->iv, iv, AES_BLOCK_SIZE); +} + +static void load_iv(struct swsusp_info *info) +{ + memcpy(iv, info->iv, AES_BLOCK_SIZE); +} + +int snapshot_prepare_crypto(bool may_sleep, bool create_iv) +{ + char enc_key[DERIVED_KEY_SIZE]; + struct crypto_skcipher *tfm; + int ret = 0; + + ret = snapshot_get_enc_key(enc_key, may_sleep); + if (ret) { + pr_warn_once("enc key is invalid\n"); + return -EINVAL; + } + + c_buffer = (void *)get_zeroed_page(GFP_KERNEL); + if (!c_buffer) { + pr_err("Allocate crypto buffer page failed\n"); + return -ENOMEM; + } + + tfm = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC); + if (IS_ERR(tfm)) { + ret = PTR_ERR(tfm); + pr_err("failed to allocate skcipher (%d)\n", ret); + goto alloc_fail; + } + + ret = crypto_skcipher_setkey(tfm, enc_key, AES_MAX_KEY_SIZE); + if (ret) { + pr_err("failed to setkey (%d)\n", ret); + goto set_fail; + } + + sk_req = skcipher_request_alloc(tfm, GFP_KERNEL); + if (!sk_req) { + pr_err("failed to allocate request\n"); + ret = -ENOMEM; + goto set_fail; + } + if (may_sleep) + skcipher_request_set_callback(sk_req, CRYPTO_TFM_REQ_MAY_SLEEP, + NULL, NULL); + if (create_iv) + get_random_bytes(iv, AES_BLOCK_SIZE); + + return 0; + +set_fail: + crypto_free_skcipher(tfm); +alloc_fail: + __free_page(c_buffer); + + return ret; +} + +void snapshot_finish_crypto(void) +{ + struct crypto_skcipher *tfm; + + if (!sk_req) + return; + + tfm = crypto_skcipher_reqtfm(sk_req); + skcipher_request_zero(sk_req); + skcipher_request_free(sk_req); + crypto_free_skcipher(tfm); + __free_page(c_buffer); + sk_req = NULL; +} + +static int encrypt_data_page(void *hash_buffer) +{ + struct scatterlist src[1], dst[1]; + u8 iv_tmp[AES_BLOCK_SIZE]; + int ret = 0; + + if (!sk_req) + return 0; + + memcpy(iv_tmp, iv, sizeof(iv)); + sg_init_one(src, hash_buffer, PAGE_SIZE); + sg_init_one(dst, c_buffer, PAGE_SIZE); + skcipher_request_set_crypt(sk_req, src, dst, PAGE_SIZE, iv_tmp); + ret = crypto_skcipher_encrypt(sk_req); + + copy_page(hash_buffer, c_buffer); + memset(c_buffer, 0, PAGE_SIZE); + + return ret; +} + +static int decrypt_data_page(void *encrypted_page) +{ + struct scatterlist src[1], dst[1]; + u8 iv_tmp[AES_BLOCK_SIZE]; + int ret = 0; + + memcpy(iv_tmp, iv, sizeof(iv)); + sg_init_one(src, encrypted_page, PAGE_SIZE); + sg_init_one(dst, c_buffer, PAGE_SIZE); + skcipher_request_set_crypt(sk_req, src, dst, PAGE_SIZE, iv_tmp); + ret = crypto_skcipher_decrypt(sk_req); + + copy_page(encrypted_page, c_buffer); + memset(c_buffer, 0, PAGE_SIZE); + + return ret; +} + /* * Signature of snapshot image */ @@ -1508,22 +1633,30 @@ int snapshot_image_verify_decrypt(void) if (ret || !s4_verify_desc) goto error_prep; + ret = snapshot_prepare_crypto(true, false); + if (ret) + goto error_prep; + for (i = 0; i < nr_copy_pages; i++) { ret = crypto_shash_update(s4_verify_desc, *(h_buf + i), PAGE_SIZE); if (ret) - goto error_shash; + goto error_shash_crypto; + ret = decrypt_data_page(*(h_buf + i)); + if (ret) + goto error_shash_crypto; } ret = crypto_shash_final(s4_verify_desc, s4_verify_digest); if (ret) - goto error_shash; + goto error_shash_crypto; pr_debug("Signature %*phN\n", SNAPSHOT_DIGEST_SIZE, signature); pr_debug("Digest %*phN\n", SNAPSHOT_DIGEST_SIZE, s4_verify_digest); if (memcmp(signature, s4_verify_digest, SNAPSHOT_DIGEST_SIZE)) ret = -EKEYREJECTED; - error_shash: + error_shash_crypto: + snapshot_finish_crypto(); snapshot_finish_hash(); error_prep: @@ -1564,6 +1697,17 @@ __copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm) crypto_buffer = page_address(d_page); } + /* Encrypt hashed page */ + encrypt_data_page(crypto_buffer); + + /* Copy encrypted buffer to destination page in high memory */ + if (PageHighMem(d_page)) { + void *kaddr = kmap_atomic(d_page); + + copy_page(kaddr, crypto_buffer); + kunmap_atomic(kaddr); + } + /* Generate digest */ if (!s4_verify_desc) continue; @@ -1638,6 +1782,8 @@ __copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm) } static inline void alloc_h_buf(void) {} +static inline void init_iv(struct swsusp_info *info) {} +static inline void load_iv(struct swsusp_info *info) {} static inline void init_signature(struct swsusp_info *info) {} static inline void load_signature(struct swsusp_info *info) {} static inline void init_sig_verify(struct trampoline *t) {} @@ -2286,6 +2432,7 @@ static int init_header(struct swsusp_info *info) info->size = info->pages; info->size <<= PAGE_SHIFT; info->trampoline_pfn = page_to_pfn(virt_to_page(trampoline_virt)); + init_iv(info); init_signature(info); return init_header_complete(info); } @@ -2524,6 +2671,7 @@ static int load_header(struct swsusp_info *info) nr_copy_pages = info->image_pages; nr_meta_pages = info->pages - info->image_pages - 1; trampoline_pfn = info->trampoline_pfn; + load_iv(info); load_signature(info); } return error;
To protect the secret in memory snapshot image, this patch adds the logic to encrypt snapshot pages by AES-CTR. Using AES-CTR is because it's simple, fast and parallelizable. But this patch didn't implement parallel encryption. The encrypt key is derived from the snapshot key. And the initialization vector will be kept in snapshot header for resuming. Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: Chen Yu <yu.c.chen@intel.com> Cc: Oliver Neukum <oneukum@suse.com> Cc: Ryan Chen <yu.chen.surf@gmail.com> Cc: David Howells <dhowells@redhat.com> Cc: Giovanni Gherdovich <ggherdovich@suse.cz> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Jann Horn <jannh@google.com> Cc: Andy Lutomirski <luto@kernel.org> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- kernel/power/hibernate.c | 8 ++- kernel/power/power.h | 6 ++ kernel/power/snapshot.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 164 insertions(+), 4 deletions(-)