[opensuse-kernel] [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
Hi experts, This patchset is the implementation for signature verification of hibernate snapshot image. The origin idea is from Jiri Kosina: Let EFI bootloader generate key-pair in UEFI secure boot environment, then pass it to kernel for sign/verify S4 image. Due to there have potential threat from the S4 image hacked, it may causes kernel lost the trust in UEFI secure boot. Hacker attack the S4 snapshot image in swap partition through whatever exploit from another trusted OS, and the exploit may don't need physical access machine. So, this patchset give the ability to kernel for parsing the RSA private key from EFI bootloader, then using the private key to generate the signature of S4 snapshot image. Kernel put the signature to snapshot header, and verify the signature when kernel try to recover snapshot image to memory. ============== How To Enable ============== Set enable the CONFIG_SNAPSHOT_VERIFICATION kernel config. And you can also choice which hash algorithm should snapshot be signed with. Then rebuild kernel. Please note this function need UEFI bootloader's support to generate key-pair in UEFI secure boot environment, e.g. shim. Current shim implementation by Gary Lin: Git: https://github.com/lcp/shim/tree/s4-key-upstream RPM: https://build.opensuse.org/package/show/home:gary_lin:UEFI/shim Please use the shim from above URL if you want to try. Please remember add the hash of shim to db in UEFI BIOS because it didn't sign by Microsoft or any OSV key. ========= Behavior ========= The RSA key-pair are generated by EFI bootloader(e.g. shim) in UEFI secure boot environment, so this function binding with EFI secure boot enabled. The kernel behavior is: + UEFI Secure Boot ON. Kernel found private key from shim: Kernel will run the signature check when S4. + UEFI Secure Boot ON. Kernel didn't find key from shim: Kernel will lock down S4 function. + UEFI Secure Boot OFF Kernel will disable S4 signature check, and ignore any keys from EFI bootloader. Unconditional allow hibernate launch. On EFI bootloader side, the behavior as following: + First, kernel will check the following 2 EFI variable: S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21 [BootService] S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21 [Runtime][Volatile] S4SignKey and S4WakeKey is a RSA key-pair: - S4SignKey is a private key that's used to generate signature of S4 snapshot. The blob format of S4SignKey is PKCS#8 uncompressed format, it should packaged a RSA private key that's followed PKCS#1. - S4WakeKey is a public key that's used to verify signature of S4 snapshot. The blob format of S4WakeKey is X.509 format, it should packaged a RSA public key that's followed PKCS#1. + EFI bootloader must generate RSA key-pair when system boot: - Bootloader store the public key to EFI boottime variable by itself - Bootloader put The private key to S4SignKey EFI variable for forward to kernel. + EFI stub kernel will load the S4SignKey blob to RAM before ExitBootServices, then copy to a memory page to maintain by hibernate_key.c. This private key will used to sign snapshot when S4. + When machine resume from hibernate: - EFI bootloader should copy the public key from boottime variable to S4WakeKey EFI variable. - Bootloader need generates a new key-pair for next round S4 usage. It should put new private key to S4SignKey variable. + EFI bootlaoder need check the following EFI runtime variable for regenerate new key-pair: GenS4Key-fe141863-c070-478e-b8a3-878a5dc9ef21 The size of GenS4Key is 1 byte, OS(kernel or userland tool) will set it to "1" for notify efi bootloader regenerate key-pair. ============== Implementation ============== Whole implementation including 3 parts: shim, asymmetric keys and hibernate: + shim: Current solution implemented by Gary Lin: https://github.com/lcp/shim/tree/s4-key-upstream Please use shim from the above URL if you want to try. Please remember add this shim to db because it didn't sign by Microsoft or any OSV key. + Asymmetric keys: This patchset implemented uncompressed PKCS#8 and RSA private key parser, it also implement the signature generation operation of RSASSA-PKCS1-v_5 in PKCS#1 spec. [RFC3447 sec 8.2.2] Set CONFIG_PKCS8_PRIVATE_KEY_INFO_PARSER=y will give kernel the abilities to parsing private key in uncompressed PKCS#8 blob and generate signature. + Hibernate: Set CONFIG_SNAPSHOT_VERIFICATION=y will enable the function of snapshot signature generation and verification. I reserved 512 byes size in snapshot header for store the signature that's generated from the digest with SHA algorithms. For adapt S4 signature check to secure boot, I have porting 3 patches from Fedora kernel, authors are Josh Boyer and Matthew Garrett. I also add Cc. to them. Please help review this RFC patchset! Appreciate for any comments! v3: - Load S4 sign key before ExitBootServices in efi stub. - In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign key before check hibernate image. It makes sure the new sign key will be transfer to resume target kernel. - Set "depends on EFI_STUB" in Kconfig. v2: - Moved SNAPSHOT_VERIFICATION kernel config to earlier patch. - Add dummy functions to simplify the ifdef check. - Sent to opensuse-kernel@opensuse.org for review: http://lists.opensuse.org/opensuse-kernel/2013-08/msg00025.html v1: - Internal review - github: https://github.com/joeyli/linux-s4sign/commits/devel-s4sign Josh Boyer (1): Secure boot: Add a dummy kernel parameter that will switch on Secure Boot mode Lee, Chun-Yi (15): asymmetric keys: add interface and skeleton for implement signature generation asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa asymmetric keys: separate the length checking of octet string from RSA_I2OSP asymmetric keys: implement OS2IP in rsa asymmetric keys: implement RSASP1 asymmetric keys: support parsing PKCS #8 private key information asymmetric keys: explicitly add the leading zero byte to encoded message Hibernate: introduced RSA key-pair to verify signature of snapshot Hibernate: generate and verify signature of snapshot Hibernate: Avoid S4 sign key data included in snapshot image Hibernate: applied SNAPSHOT_VERIFICATION config to switch signature check Hibernate: adapt to UEFI secure boot with signature check Hibernate: show the verification time for monitor performance Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm Hibernate: notify bootloader regenerate key-pair for snapshot verification Matthew Garrett (2): Secure boot: Add new capability efi: Enable secure boot lockdown automatically when enabled in firmware Documentation/kernel-parameters.txt | 7 + Documentation/x86/zero-page.txt | 2 + arch/x86/boot/compressed/eboot.c | 121 ++++++++++ arch/x86/include/asm/bootparam_utils.h | 8 +- arch/x86/include/asm/efi.h | 9 + arch/x86/include/uapi/asm/bootparam.h | 4 +- arch/x86/kernel/setup.c | 7 + arch/x86/platform/efi/efi.c | 68 ++++++ crypto/asymmetric_keys/Kconfig | 11 + crypto/asymmetric_keys/Makefile | 16 ++ crypto/asymmetric_keys/pkcs8.asn1 | 19 ++ crypto/asymmetric_keys/pkcs8_info_parser.c | 152 ++++++++++++ crypto/asymmetric_keys/pkcs8_parser.h | 23 ++ crypto/asymmetric_keys/pkcs8_private_key.c | 148 ++++++++++++ crypto/asymmetric_keys/pkcs8_rsakey.asn1 | 29 +++ crypto/asymmetric_keys/private_key.h | 29 +++ crypto/asymmetric_keys/public_key.c | 32 +++ crypto/asymmetric_keys/rsa.c | 283 ++++++++++++++++++++++- crypto/asymmetric_keys/signature.c | 28 +++ include/crypto/public_key.h | 28 +++ include/keys/asymmetric-subtype.h | 6 + include/linux/cred.h | 2 + include/linux/efi.h | 18 ++ include/uapi/linux/capability.h | 6 +- kernel/cred.c | 17 ++ kernel/power/Kconfig | 77 ++++++- kernel/power/Makefile | 1 + kernel/power/hibernate.c | 37 +++ kernel/power/hibernate_keys.c | 329 ++++++++++++++++++++++++++ kernel/power/main.c | 11 +- kernel/power/power.h | 35 +++ kernel/power/snapshot.c | 345 +++++++++++++++++++++++++++- kernel/power/swap.c | 22 ++ kernel/power/user.c | 22 ++ 34 files changed, 1925 insertions(+), 27 deletions(-) create mode 100644 crypto/asymmetric_keys/pkcs8.asn1 create mode 100644 crypto/asymmetric_keys/pkcs8_info_parser.c create mode 100644 crypto/asymmetric_keys/pkcs8_parser.h create mode 100644 crypto/asymmetric_keys/pkcs8_private_key.c create mode 100644 crypto/asymmetric_keys/pkcs8_rsakey.asn1 create mode 100644 crypto/asymmetric_keys/private_key.h create mode 100644 kernel/power/hibernate_keys.c -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Add generate_signature interface on signature.c, asymmetric-subtype and
rsa.c for prepare to implement signature generation.
Reviewed-by: Jiri Kosina
Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
This patch is temporary set emLen to pks->k, and temporary set EM to
pks->S for debugging. We will replace the above values to real signature
after implement RSASP1.
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:41, Lee, Chun-Yi wrote:
Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
Is this your own code, or did you copy it from somewhere?
+ if (!T) + goto error_T; + + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size); + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size); + + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */ + if (emLen < tLen + 11) { + ret = EINVAL; + goto error_emLen; + }
Normal kernel calling convention is 0 / -EINVAL.
+ memcpy(EM + 2, PS, emLen - tLen - 3); + EM[2 + emLen - tLen - 3] = 0x00; + memcpy(EM + 2 + emLen - tLen - 3 + 1, T, tLen);
ThisDoesNotLookLikeKernelCode, NoCamelCase, please.
+ *_EM = EM;
And we don't usually use _ prefix like this.
--- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload); struct public_key_signature { u8 *digest; u8 digest_size; /* Number of bytes in digest */ + u8 *S; /* signature S of length k octets */
u8 *signature?
+ size_t k; /* length k of signature S */
u8 *signature_length. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Pavel, First, thanks for your review! 於 日,2013-08-25 於 17:53 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:41, Lee, Chun-Yi wrote:
Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
Is this your own code, or did you copy it from somewhere?
It's my own code, development base on RSA PKCS#1 spec. So the naming of variables are match with PKCS#1 spec.
+ if (!T) + goto error_T; + + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size); + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size); + + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */ + if (emLen < tLen + 11) { + ret = EINVAL; + goto error_emLen; + }
Normal kernel calling convention is 0 / -EINVAL.
Yes, here is my mistake, I will modify it.
+ memcpy(EM + 2, PS, emLen - tLen - 3); + EM[2 + emLen - tLen - 3] = 0x00; + memcpy(EM + 2 + emLen - tLen - 3 + 1, T, tLen);
ThisDoesNotLookLikeKernelCode, NoCamelCase, please.
Thanks for you point out, I will change it.
+ *_EM = EM;
And we don't usually use _ prefix like this.
Thanks! I will change it.
--- a/include/crypto/public_key.h +++ b/include/crypto/public_key.h @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload); struct public_key_signature { u8 *digest; u8 digest_size; /* Number of bytes in digest */ + u8 *S; /* signature S of length k octets */
u8 *signature?
Yes, this 'S' is signature. I put the naming full match with spec for development, I will change it to match kernel rule. e.g. signature_S
+ size_t k; /* length k of signature S */
u8 *signature_length.
I will use signature_leng_k to also match with PKCS#1 spec, I think it's better for review source code with the spec for debugging. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Due to RSA_I2OSP is not only used by signature verification path but also used
in signature generation path. So, separate the length checking of octet string
because it's not for generate 0x00 0x01 leading string when used in signature
generation.
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:42, Lee, Chun-Yi wrote:
Due to RSA_I2OSP is not only used by signature verification path but also used in signature generation path. So, separate the length checking of octet string because it's not for generate 0x00 0x01 leading string when used in signature generation.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi
+static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X) +{ + unsigned x_size; + unsigned X_size; + u8 *X = NULL;
Is this kernel code or entry into obfuscated C code contest? This is not funny. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 日,2013-08-25 於 18:01 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:42, Lee, Chun-Yi wrote:
Due to RSA_I2OSP is not only used by signature verification path but also used in signature generation path. So, separate the length checking of octet string because it's not for generate 0x00 0x01 leading string when used in signature generation.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X) +{ + unsigned x_size; + unsigned X_size; + u8 *X = NULL;
Is this kernel code or entry into obfuscated C code contest? This is not funny.
Pavel
The small "x" is the input integer that will transfer to big "X" that is a octet sting. Sorry for I direct give variable name to match with spec, maybe I need use big_X or.... Do you have good suggest for the naming? Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi!
Due to RSA_I2OSP is not only used by signature verification path but also used in signature generation path. So, separate the length checking of octet string because it's not for generate 0x00 0x01 leading string when used in signature generation.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X) +{ + unsigned x_size; + unsigned X_size; + u8 *X = NULL;
Is this kernel code or entry into obfuscated C code contest? This is not funny.
The small "x" is the input integer that will transfer to big "X" that is a octet sting.
Sorry for I direct give variable name to match with spec, maybe I need use big_X or....
Having variables that differ only in case is confusing. Actually RSA_I2OSP is not a good name for function, either. If it converts x into X, perhaps you can name one input and second output?
Do you have good suggest for the naming?
Not really, sorry. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Mon, 26 Aug 2013, Pavel Machek wrote:
Due to RSA_I2OSP is not only used by signature verification path but also used in signature generation path. So, separate the length checking of octet string because it's not for generate 0x00 0x01 leading string when used in signature generation.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi +static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X) +{ + unsigned x_size; + unsigned X_size; + u8 *X = NULL;
Is this kernel code or entry into obfuscated C code contest? This is not funny.
The small "x" is the input integer that will transfer to big "X" that is a octet sting.
Sorry for I direct give variable name to match with spec, maybe I need use big_X or....
Having variables that differ only in case is confusing. Actually RSA_I2OSP is not a good name for function, either.
If it converts x into X, perhaps you can name one input and second output?
The variable naming is according to spec, and I believe it makes sense to keep it so, no matter how stupid the naming in the spec might be. Otherwise you have to do mental remapping when looking at the code and the spec at the same time, which is very inconvenient. Would a comment next to the variable declaration, that would point this fact out, be satisfactory for you? -- Jiri Kosina SUSE Labs -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Implement Octet String to Integer conversion [RFC3447 sec 4.2] in rsa.c. It's
the second step of signature generation operation.
This patch is temporary set non-RSASP1 message to pks->S for debugging.
Reviewed-by: Jiri Kosina
Implement RSASP1 and fill-in the following data to public key signature
structure: signature length (pkcs->k), signature octet
strings (pks->S) and MPI of signature (pks->rsa.s).
Reviewed-by: Jiri Kosina
Add ASN.1 files and parser to support parsing PKCS #8 noncompressed private
key information. It's better than direct parsing pure private key because
PKCS #8 has a privateKeyAlgorithm to indicate the algorithm of private
key, e.g. RSA from PKCS #1
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:45, Lee, Chun-Yi wrote:
Add ASN.1 files and parser to support parsing PKCS #8 noncompressed private key information. It's better than direct parsing pure private key because PKCS #8 has a privateKeyAlgorithm to indicate the algorithm of private key, e.g. RSA from PKCS #1
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi
+#include
+ +struct pkcs8_info { + enum pkey_algo privkey_algo:8; /* Private key algorithm */
Are you sure this is well-defined?
+struct private_key_algorithm *pkcs8_private_key_algorithms[PKEY_ALGO__LAST] = { + [PKEY_ALGO_DSA] = NULL, +#if defined(CONFIG_PUBLIC_KEY_ALGO_RSA) || \ + defined(CONFIG_PUBLIC_KEY_ALGO_RSA_MODULE) + [PKEY_ALGO_RSA] = &RSA_private_key_algorithm, +#endif +};
pkey_algo privkey_algo private_key_algorithm ...you use all variants. Having symbols with "__" inside them is "interesting". I'd not do it. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Per PKCS1 spec, the EMSA-PKCS1-v1_5 encoded message is leading by 0x00 0x01 in
its first 2 bytes. The leading zero byte is suppressed by MPI so we pass a
pointer to the _preceding_ byte to RSA_verify() in original code, but it has
risk for the byte is not zero because it's not in EM buffer's scope, neither
RSA_verify() nor mpi_get_buffer() didn't take care the leading byte.
To avoid the risk, that's better we explicitly add the leading zero byte to EM
for pass to RSA_verify(). This patch allocate a _EM buffer to capture the
result from RSA_I2OSP(), then set the first byte to zero in EM and copy the
remaining bytes from _EM.
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:46, Lee, Chun-Yi wrote:
Per PKCS1 spec, the EMSA-PKCS1-v1_5 encoded message is leading by 0x00 0x01 in its first 2 bytes. The leading zero byte is suppressed by MPI so we pass a pointer to the _preceding_ byte to RSA_verify() in original code, but it has risk for the byte is not zero because it's not in EM buffer's scope, neither RSA_verify() nor mpi_get_buffer() didn't take care the leading byte.
To avoid the risk, that's better we explicitly add the leading zero byte to EM for pass to RSA_verify(). This patch allocate a _EM buffer to capture the result from RSA_I2OSP(), then set the first byte to zero in EM and copy the remaining bytes from _EM.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi
- ret = RSA_verify(H, EM - 1, k, sig->digest_size, + EM = kmalloc(k, GFP_KERNEL); + memset(EM, 0, 1); + memcpy(EM + 1, _EM, k-1); + kfree(_EM);
Spot a crash waiting to happen. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: Matthew Garrett
On Thu 2013-08-22 19:01:47, Lee, Chun-Yi wrote:
From: Matthew Garrett
Secure boot adds certain policy requirements, including that root must not be able to do anything that could cause the kernel to execute arbitrary code. The simplest way to handle this would seem to be to add a new capability and gate various functionality on that. We'll then strip it from the initial capability set if required.
There was some discussion about this before, right? And I don't think conclusion was it was acceptable...?
Signed-off-by: Matthew Garrett
Acked-by: Lee, Chun-Yi Signed-off-by: Lee, Chun-Yi --- include/uapi/linux/capability.h | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/include/uapi/linux/capability.h b/include/uapi/linux/capability.h index ba478fa..7109e65 100644 --- a/include/uapi/linux/capability.h +++ b/include/uapi/linux/capability.h @@ -343,7 +343,11 @@ struct vfs_cap_data {
#define CAP_BLOCK_SUSPEND 36
-#define CAP_LAST_CAP CAP_BLOCK_SUSPEND +/* Allow things that trivially permit root to modify the running kernel */ + +#define CAP_COMPROMISE_KERNEL 37 + +#define CAP_LAST_CAP CAP_COMPROMISE_KERNEL
#define cap_valid(x) ((x) >= 0 && (x) <= CAP_LAST_CAP)
-- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: Josh Boyer
You may want to check subject. If it does something, it is not dummy.
--- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -2784,6 +2784,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted. Note: increases power consumption, thus should only be enabled if running jitter sensitive (HPC/RT) workloads.
+ secureboot_enable= + [KNL] Enables an emulated UEFI Secure Boot mode. This + locks down various aspects of the kernel guarded by the + CAP_COMPROMISE_KERNEL capability. This includes things + like /dev/mem, IO port access, and other areas. It can + be used on non-UEFI machines for testing purposes. + security= [SECURITY] Choose a security module to enable at boot. If this boot parameter is not specified, only the first security module asking for security registration will be diff --git a/kernel/cred.c b/kernel/cred.c index e0573a4..c3f4e3e 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -565,6 +565,23 @@ void __init cred_init(void) 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL); }
+void __init secureboot_enable() +{ + pr_info("Secure boot enabled\n"); + cap_lower((&init_cred)->cap_bset, CAP_COMPROMISE_KERNEL); + cap_lower((&init_cred)->cap_permitted, CAP_COMPROMISE_KERNEL); +}
OTOH you don't implement CAP_COMPROMISE_KERNEL, so it is dummy after all. But CAP_COMPROMISE_KERNEL is infeasible to implement, right? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: Matthew Garrett
On Thu 2013-08-22 19:01:49, Lee, Chun-Yi wrote:
From: Matthew Garrett
The firmware has a set of flags that indicate whether secure boot is enabled and enforcing. Use them to indicate whether the kernel should lock itself down. We also indicate the machine is in secure boot mode by adding the EFI_SECURE_BOOT bit for use with efi_enabled.
+ status = efi_call_phys5(sys_table->runtime->get_variable, + L"SecureBoot", &var_guid, NULL, &datasize, &sb);
What is this L"..." thing? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Sun, Aug 25, 2013 at 06:22:43PM +0200, Pavel Machek wrote:
On Thu 2013-08-22 19:01:49, Lee, Chun-Yi wrote:
From: Matthew Garrett
The firmware has a set of flags that indicate whether secure boot is enabled and enforcing. Use them to indicate whether the kernel should lock itself down. We also indicate the machine is in secure boot mode by adding the EFI_SECURE_BOOT bit for use with efi_enabled.
+ status = efi_call_phys5(sys_table->runtime->get_variable, + L"SecureBoot", &var_guid, NULL, &datasize, &sb);
What is this L"..." thing?
http://en.wikipedia.org/wiki/C_syntax#Wide_character_strings -- Matthew Garrett | mjg59@srcf.ucam.org -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Thu, 22 Aug, at 07:01:49PM, Lee, Chun-Yi wrote:
From: Matthew Garrett
The firmware has a set of flags that indicate whether secure boot is enabled and enforcing. Use them to indicate whether the kernel should lock itself down. We also indicate the machine is in secure boot mode by adding the EFI_SECURE_BOOT bit for use with efi_enabled.
Signed-off-by: Matthew Garrett
Signed-off-by: Josh Boyer Acked-by: Lee, Chun-Yi Signed-off-by: Lee, Chun-Yi --- Documentation/x86/zero-page.txt | 2 ++ arch/x86/boot/compressed/eboot.c | 32 ++++++++++++++++++++++++++++++++ arch/x86/include/asm/bootparam_utils.h | 8 ++++++-- arch/x86/include/uapi/asm/bootparam.h | 3 ++- arch/x86/kernel/setup.c | 7 +++++++ include/linux/cred.h | 2 ++ include/linux/efi.h | 1 + 7 files changed, 52 insertions(+), 3 deletions(-)
[...]
+static int get_secure_boot(efi_system_table_t *_table) +{ + u8 sb, setup; + unsigned long datasize = sizeof(sb); + efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID; + efi_status_t status; + + status = efi_call_phys5(sys_table->runtime->get_variable, + L"SecureBoot", &var_guid, NULL, &datasize, &sb); +
The _table argument isn't needed because it's never used. [...]
io_delay_init();
+ if (boot_params.secure_boot) { +#ifdef CONFIG_EFI + set_bit(EFI_SECURE_BOOT, &x86_efi_facility); +#endif + secureboot_enable(); + } +
efi_enabled(EFI_BOOT) should be checked also, instead of assuming that secure_boot contains a sensible value. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Introduced a hibernate_key.c file to query the key pair from EFI variables
and maintain key pair for check signature of S4 snapshot image. We
loaded the private key when snapshot image stored success.
This patch introduced 2 EFI variables for store the key to sign S4 image and
verify signature when S4 wake up. The names and GUID are:
S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21
S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21
S4SignKey is used by EFI bootloader to pass the RSA private key that packaged
by PKCS#8 format, kernel will read and parser it when system boot and reload
it when S4 resume. EFI bootloader need gnerate a new private key when every
time system boot.
S4WakeKey is used to pass the RSA public key that packaged by X.509
certificate, kernel will read and parser it for check the signature of
S4 snapshot image when S4 resume.
The follow-up patch will remove S4SignKey and S4WakeKey after load them
to kernel for avoid anyone can access it through efivarfs.
v3:
- Load S4 sign key before ExitBootServices.
Load private key before ExitBootServices() then bootloader doesn't need
generate key-pair for each booting:
+ Add setup_s4_keys() to eboot.c to load S4 sign key before ExitBootServices.
+ Reserve the memory block of sign key data blob in efi.c
- In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign
key before check hibernate image. It makes sure the new sign key will be
transfer to resume target kernel.
- Set "depends on EFI_STUB" in Kconfig
v2:
Add CONFIG_SNAPSHOT_VERIFICATION for build of hibernate_keys.c depend on
Kconfig.
Cc: Matthew Garrett
On Thu 2013-08-22 19:01:50, Lee, Chun-Yi wrote:
Introduced a hibernate_key.c file to query the key pair from EFI variables and maintain key pair for check signature of S4 snapshot image. We loaded the private key when snapshot image stored success.
This patch introduced 2 EFI variables for store the key to sign S4 image and verify signature when S4 wake up. The names and GUID are: S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21 S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21
S4SignKey is used by EFI bootloader to pass the RSA private key that packaged by PKCS#8 format, kernel will read and parser it when system boot and reload it when S4 resume. EFI bootloader need gnerate a new private key when every time system boot.
S4WakeKey is used to pass the RSA public key that packaged by X.509 certificate, kernel will read and parser it for check the signature of S4 snapshot image when S4 resume.
The follow-up patch will remove S4SignKey and S4WakeKey after load them to kernel for avoid anyone can access it through efivarfs.
v3: - Load S4 sign key before ExitBootServices. Load private key before ExitBootServices() then bootloader doesn't need generate key-pair for each booting: + Add setup_s4_keys() to eboot.c to load S4 sign key before ExitBootServices. + Reserve the memory block of sign key data blob in efi.c - In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign key before check hibernate image. It makes sure the new sign key will be transfer to resume target kernel. - Set "depends on EFI_STUB" in Kconfig
v2: Add CONFIG_SNAPSHOT_VERIFICATION for build of hibernate_keys.c depend on Kconfig.
Cc: Matthew Garrett
Cc: Takashi Iwai Reviewed-by: Jiri Kosina Signed-off-by: Lee, Chun-Yi
--- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -368,6 +368,91 @@ free_handle: return status; }
+#ifdef CONFIG_SNAPSHOT_VERIFICATION +static efi_status_t setup_s4_keys(struct boot_params *params) +{ + struct setup_data *data; + unsigned long datasize; + u32 attr; + struct efi_s4_key *s4key; + efi_status_t status; + + data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
A bit too many casts.
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
@@ -729,6 +792,11 @@ void __init efi_init(void)
set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + /* keep s4 key from setup_data */ + efi_reserve_s4_skey_data(); +#endif +
Here too. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Pavel, 於 日,2013-08-25 於 18:25 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:50, Lee, Chun-Yi wrote:
Introduced a hibernate_key.c file to query the key pair from EFI variables and maintain key pair for check signature of S4 snapshot image. We loaded the private key when snapshot image stored success.
This patch introduced 2 EFI variables for store the key to sign S4 image and verify signature when S4 wake up. The names and GUID are: S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21 S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21
S4SignKey is used by EFI bootloader to pass the RSA private key that packaged by PKCS#8 format, kernel will read and parser it when system boot and reload it when S4 resume. EFI bootloader need gnerate a new private key when every time system boot.
S4WakeKey is used to pass the RSA public key that packaged by X.509 certificate, kernel will read and parser it for check the signature of S4 snapshot image when S4 resume.
The follow-up patch will remove S4SignKey and S4WakeKey after load them to kernel for avoid anyone can access it through efivarfs.
v3: - Load S4 sign key before ExitBootServices. Load private key before ExitBootServices() then bootloader doesn't need generate key-pair for each booting: + Add setup_s4_keys() to eboot.c to load S4 sign key before ExitBootServices. + Reserve the memory block of sign key data blob in efi.c - In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign key before check hibernate image. It makes sure the new sign key will be transfer to resume target kernel. - Set "depends on EFI_STUB" in Kconfig
v2: Add CONFIG_SNAPSHOT_VERIFICATION for build of hibernate_keys.c depend on Kconfig.
Cc: Matthew Garrett
Cc: Takashi Iwai Reviewed-by: Jiri Kosina Signed-off-by: Lee, Chun-Yi --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -368,6 +368,91 @@ free_handle: return status; }
+#ifdef CONFIG_SNAPSHOT_VERIFICATION +static efi_status_t setup_s4_keys(struct boot_params *params) +{ + struct setup_data *data; + unsigned long datasize; + u32 attr; + struct efi_s4_key *s4key; + efi_status_t status; + + data = (struct setup_data *)(unsigned long)params->hdr.setup_data;
A bit too many casts.
Thanks. Yes, here is my mistake, I will remove "unsigned long" cast.
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
OK, I will define a dummy function for non-verification situation.
@@ -729,6 +792,11 @@ void __init efi_init(void)
set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + /* keep s4 key from setup_data */ + efi_reserve_s4_skey_data(); +#endif +
Here too.
I will also use dummy function here. Thanks Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
OK, I will define a dummy function for non-verification situation.
IIRC you can just put the #ifdef inside the function body. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Tue, 27 Aug 2013, 13:29:43 +0200, Pavel Machek wrote:
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
OK, I will define a dummy function for non-verification situation.
IIRC you can just put the #ifdef inside the function body.
This is certainly not to be invoked on a frequent basis (and therefore not on a hot path), but from a more general angle, wouldn't this leave a(nother) plain "jsr... rts" sequence without any effect other than burning a few cycles? If the whole function call can be disabled (ignored) in a certain configuration, it shouldn't call at all, should it? Cheers. l8er manfred -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Tue 2013-08-27 14:01:42, Manfred Hollstein wrote:
On Tue, 27 Aug 2013, 13:29:43 +0200, Pavel Machek wrote:
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
OK, I will define a dummy function for non-verification situation.
IIRC you can just put the #ifdef inside the function body.
This is certainly not to be invoked on a frequent basis (and therefore not on a hot path), but from a more general angle, wouldn't this leave a(nother) plain "jsr... rts" sequence without any effect other than burning a few cycles? If the whole function call can be disabled (ignored) in a certain configuration, it shouldn't call at all, should it?
gcc should be able to deal with optimizing that out. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 二,2013-08-27 於 13:29 +0200,Pavel Machek 提到:
@@ -1205,6 +1290,10 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,
setup_efi_pci(boot_params);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + setup_s4_keys(boot_params); +#endif +
Move ifdef inside the function?
OK, I will define a dummy function for non-verification situation.
IIRC you can just put the #ifdef inside the function body. Pavel
I want use inline dummy function like saveable_highmem_page() in snapshot.c, maybe it's better then additional function call? Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
+static int efi_status_to_err(efi_status_t status) +{ + int err; + + switch (status) { + case EFI_INVALID_PARAMETER: + err = -EINVAL; + break; + case EFI_OUT_OF_RESOURCES: + err = -ENOSPC; + break; + case EFI_DEVICE_ERROR: + err = -EIO; + break; + case EFI_WRITE_PROTECTED: + err = -EROFS; + break; + case EFI_SECURITY_VIOLATION: + err = -EACCES; + break; + case EFI_NOT_FOUND: + err = -ENODATA; + break; + default: + err = -EINVAL; + } + + return err; +}
Please don't reimplement this. Instead make the existing function global. [...]
+static void *load_wake_key_data(unsigned long *datasize) +{ + u32 attr; + void *wkey_data; + efi_status_t status; + + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return ERR_PTR(-EPERM); + + /* obtain the size */ + *datasize = 0; + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID, + NULL, datasize, NULL); + if (status != EFI_BUFFER_TOO_SMALL) { + wkey_data = ERR_PTR(efi_status_to_err(status)); + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status); + goto error; + }
Is it safe to completely bypass the efivars interface and access efi.get_variable() directly? I wouldn't have thought so, unless you can guarantee that the kernel isn't going to access any of the EFI runtime services while you execute this function. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Matt, First, thanks for your review! 於 四,2013-09-05 於 09:53 +0100,Matt Fleming 提到:
On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
+static int efi_status_to_err(efi_status_t status) +{ + int err; + + switch (status) { + case EFI_INVALID_PARAMETER: + err = -EINVAL; + break; + case EFI_OUT_OF_RESOURCES: + err = -ENOSPC; + break; + case EFI_DEVICE_ERROR: + err = -EIO; + break; + case EFI_WRITE_PROTECTED: + err = -EROFS; + break; + case EFI_SECURITY_VIOLATION: + err = -EACCES; + break; + case EFI_NOT_FOUND: + err = -ENODATA; + break; + default: + err = -EINVAL; + } + + return err; +}
Please don't reimplement this. Instead make the existing function global.
OK, I will make the function to global.
[...]
+static void *load_wake_key_data(unsigned long *datasize) +{ + u32 attr; + void *wkey_data; + efi_status_t status; + + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return ERR_PTR(-EPERM); + + /* obtain the size */ + *datasize = 0; + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID, + NULL, datasize, NULL); + if (status != EFI_BUFFER_TOO_SMALL) { + wkey_data = ERR_PTR(efi_status_to_err(status)); + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status); + goto error; + }
Is it safe to completely bypass the efivars interface and access efi.get_variable() directly? I wouldn't have thought so, unless you can guarantee that the kernel isn't going to access any of the EFI runtime services while you execute this function.
This S4WakeKey is a VOLATILE variable that could not modify by SetVariable() at runtime. So, it's read only even through efivars. Does it what your concern? Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Thu, 05 Sep, at 06:13:36PM, joeyli wrote:
This S4WakeKey is a VOLATILE variable that could not modify by SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
No, the UEFI spec probibits certain runtime functions from being executed concurrently on separate cpus and the spinlock used in the efivars code ensures that we adhere to that restriction. See table 31 in section 7.1 of the UEFI 2.4 spec for the list of services that are non-reentrant. The problem isn't that we want to avoid simultaneous access to S4WakeKey, it's that we can't invoke any of the variable runtime service functions concurrently. -- Matt Fleming, Intel Open Source Technology Center -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 四,2013-09-05 於 11:31 +0100,Matt Fleming 提到:
On Thu, 05 Sep, at 06:13:36PM, joeyli wrote:
This S4WakeKey is a VOLATILE variable that could not modify by SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
No, the UEFI spec probibits certain runtime functions from being executed concurrently on separate cpus and the spinlock used in the efivars code ensures that we adhere to that restriction. See table 31 in section 7.1 of the UEFI 2.4 spec for the list of services that are non-reentrant.
The problem isn't that we want to avoid simultaneous access to S4WakeKey, it's that we can't invoke any of the variable runtime service functions concurrently.
I see! I will use efivar api to access EFI variable. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
This patch add the code for generate/verify signature of snapshot, it
put the signature to snapshot header. This approach can support both
on userspace hibernate and in-kernel hibernate.
v2:
- Due to loaded S4 sign key before ExitBootServices, we need forward key from
boot kernel to resume target kernel. So this patch add a empty page in
snapshot image, then we keep the pfn of this empty page in snapshot header.
When system resume from hibernate, we fill new sign key to this empty page
space after snapshot image checked pass. This mechanism let boot kernel can
forward new sign key to resume target kernel but don't need write new private
key to any other storage, e.g. swap.
Cc: Matthew Garrett
On Thu 2013-08-22 19:01:51, Lee, Chun-Yi wrote:
This patch add the code for generate/verify signature of snapshot, it put the signature to snapshot header. This approach can support both on userspace hibernate and in-kernel hibernate.
v2: - Due to loaded S4 sign key before ExitBootServices, we need forward key from boot kernel to resume target kernel. So this patch add a empty page in snapshot image, then we keep the pfn of this empty page in snapshot header. When system resume from hibernate, we fill new sign key to this empty page space after snapshot image checked pass. This mechanism let boot kernel can forward new sign key to resume target kernel but don't need write new private key to any other storage, e.g. swap.
Cc: Matthew Garrett
Reviewed-by: Jiri Kosina Signed-off-by: Lee, Chun-Yi --- kernel/power/power.h | 6 + kernel/power/snapshot.c | 280 +++++++++++++++++++++++++++++++++++++++++++++- kernel/power/swap.c | 14 +++ kernel/power/user.c | 9 ++ 4 files changed, 302 insertions(+), 7 deletions(-) diff --git a/kernel/power/power.h b/kernel/power/power.h index 69a81d8..84e0b06 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -3,6 +3,9 @@ #include
#include +/* The maximum length of snapshot signature */ +#define SIG_LENG 512 + struct swsusp_info { struct new_utsname uts; u32 version_code; @@ -11,6 +14,8 @@ struct swsusp_info { unsigned long image_pages; unsigned long pages; unsigned long size; + unsigned long skey_data_buf_pfn; + u8 signature[SIG_LENG]; } __attribute__((aligned(PAGE_SIZE)));
SIG_LEN or SIG_LENGTH. Select one.
+static int copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm) { struct zone *zone; - unsigned long pfn; + unsigned long pfn, dst_pfn; ... + tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0); + if (IS_ERR(tfm)) { + pr_err("IS_ERR(tfm): %ld", PTR_ERR(tfm)); + return PTR_ERR(tfm); + } + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + digest_size = crypto_shash_digestsize(tfm); + digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
Are you sure GFP_KERNEL allocation is okay at this phase of hibernation? Could the hashing be done at later phase, when writing the image to disk?
+void **h_buf;
helpfully named.
+ ret = verify_signature(s4_wake_key, pks); + if (ret) { + pr_err("snapshot S4 signature verification fail: %d\n", ret); + goto error_verify; + } else + pr_info("snapshot S4 signature verification pass!\n"); + + if (pks->rsa.s) + mpi_free(pks->rsa.s); + kfree(pks);
ret = 0 and fall through?
+ return 0; + +error_verify: + if (pks->rsa.s) + mpi_free(pks->rsa.s); +error_mpi: + kfree(pks); + return ret; +}
+ ret = crypto_shash_final(desc, digest); + if (ret) + goto error_shash; + + ret = snapshot_verify_signature(digest, digest_size); + if (ret) + goto error_verify; + + kfree(h_buf); + kfree(digest); + crypto_free_shash(tfm); + return 0;
These four lines can be deleted.
+ +error_verify: +error_shash: + kfree(h_buf); + kfree(digest); +error_digest: + crypto_free_shash(tfm); + return ret; +} + Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Pavel, Thanks for your time to review my patches. 於 日,2013-08-25 於 18:36 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:51, Lee, Chun-Yi wrote:
This patch add the code for generate/verify signature of snapshot, it put the signature to snapshot header. This approach can support both on userspace hibernate and in-kernel hibernate.
v2: - Due to loaded S4 sign key before ExitBootServices, we need forward key from boot kernel to resume target kernel. So this patch add a empty page in snapshot image, then we keep the pfn of this empty page in snapshot header. When system resume from hibernate, we fill new sign key to this empty page space after snapshot image checked pass. This mechanism let boot kernel can forward new sign key to resume target kernel but don't need write new private key to any other storage, e.g. swap.
Cc: Matthew Garrett
Reviewed-by: Jiri Kosina Signed-off-by: Lee, Chun-Yi --- kernel/power/power.h | 6 + kernel/power/snapshot.c | 280 +++++++++++++++++++++++++++++++++++++++++++++- kernel/power/swap.c | 14 +++ kernel/power/user.c | 9 ++ 4 files changed, 302 insertions(+), 7 deletions(-) diff --git a/kernel/power/power.h b/kernel/power/power.h index 69a81d8..84e0b06 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -3,6 +3,9 @@ #include
#include +/* The maximum length of snapshot signature */ +#define SIG_LENG 512 + struct swsusp_info { struct new_utsname uts; u32 version_code; @@ -11,6 +14,8 @@ struct swsusp_info { unsigned long image_pages; unsigned long pages; unsigned long size; + unsigned long skey_data_buf_pfn; + u8 signature[SIG_LENG]; } __attribute__((aligned(PAGE_SIZE)));
SIG_LEN or SIG_LENGTH. Select one.
I will use SIG_LEN at next version, thanks!
+static int copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm) { struct zone *zone; - unsigned long pfn; + unsigned long pfn, dst_pfn; ... + tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0); + if (IS_ERR(tfm)) { + pr_err("IS_ERR(tfm): %ld", PTR_ERR(tfm)); + return PTR_ERR(tfm); + } + + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc); + digest_size = crypto_shash_digestsize(tfm); + digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
Are you sure GFP_KERNEL allocation is okay at this phase of hibernation?
Could the hashing be done at later phase, when writing the image to disk?
Thanks for you point out! Yes, call memory allocate here is not a good design due to it causes garbage in snapshot that will not released by resumed kernel. I just finished another implementation, the respin patch extracts the signature generation code to another function then call the function in swsusp_save() after copy_data_pages() finished. We can write to memory at that stage.
+void **h_buf;
helpfully named.
I will change the name to handle_buffers;
+ ret = verify_signature(s4_wake_key, pks); + if (ret) { + pr_err("snapshot S4 signature verification fail: %d\n", ret); + goto error_verify; + } else + pr_info("snapshot S4 signature verification pass!\n"); + + if (pks->rsa.s) + mpi_free(pks->rsa.s); + kfree(pks);
ret = 0 and fall through?
When verification success, verify_signature() will return 0. Yes, here have duplicate code, I will clear up it.
+ return 0; + +error_verify: + if (pks->rsa.s) + mpi_free(pks->rsa.s); +error_mpi: + kfree(pks); + return ret; +}
+ ret = crypto_shash_final(desc, digest); + if (ret) + goto error_shash; + + ret = snapshot_verify_signature(digest, digest_size); + if (ret) + goto error_verify; + + kfree(h_buf); + kfree(digest); + crypto_free_shash(tfm); + return 0;
These four lines can be deleted.
Yes, here also duplicate, I will remove.
+ +error_verify: +error_shash: + kfree(h_buf); + kfree(digest); +error_digest: + crypto_free_shash(tfm); + return ret; +} + Pavel
Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
This patch add swsusp_page_is_sign_key() method to hibernate_key.c and
check the page is S4 sign key data when collect saveable page in
snapshot.c to avoid sign key data included in snapshot image.
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:52, Lee, Chun-Yi wrote:
This patch add swsusp_page_is_sign_key() method to hibernate_key.c and check the page is S4 sign key data when collect saveable page in snapshot.c to avoid sign key data included in snapshot image.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/snapshot.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 72e2911..9e4c94b 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -860,6 +860,9 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
BUG_ON(!PageHighMem(page));
+ if (swsusp_page_is_sign_key(page)) + return NULL; + if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) || PageReserved(page)) return NULL;
Just do set_page_forbidden() on your page? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 日,2013-08-25 於 18:39 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:52, Lee, Chun-Yi wrote:
This patch add swsusp_page_is_sign_key() method to hibernate_key.c and check the page is S4 sign key data when collect saveable page in snapshot.c to avoid sign key data included in snapshot image.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/snapshot.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c index 72e2911..9e4c94b 100644 --- a/kernel/power/snapshot.c +++ b/kernel/power/snapshot.c @@ -860,6 +860,9 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
BUG_ON(!PageHighMem(page));
+ if (swsusp_page_is_sign_key(page)) + return NULL; + if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) || PageReserved(page)) return NULL;
Just do set_page_forbidden() on your page?
Before call swsusp_unset_page_forbidden(), we need make sure the create_basic_memory_bitmaps() function already called to initial forbidden_pages_map. That means need careful the timing, otherwise the page of private key will not register to forbidden pages map. So, I choice to refuse the page of private key when snapshot finding which page is saveable. It has clearer code and we don't need worry the future change of hibernate.c or user.c for when they call create_basic_memory_bitmaps() and when the code clear forbidden pages map. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
This patch applied SNAPSHOT_VERIFICATION kernel config for switching
signature check of hibernate snapshot image.
Reviewed-by: Jiri Kosina
In current solution, the snapshot signature check used the RSA key-pair
that are generated by bootloader(e.g. shim) and pass the key-pair to
kernel through EFI variables. I choice to binding the snapshot
signature check mechanism with UEFI secure boot for provide stronger
protection of hibernate. Current behavior is following:
+ UEFI Secure Boot ON, Kernel found key-pair from shim:
Will do the S4 signature check.
+ UEFI Secure Boot ON, Kernel didn't find key-pair from shim:
Will lock down S4 function.
+ UEFI Secure Boot OFF
Will NOT do the S4 signature check.
Ignore any keys from bootloader.
v2:
Replace sign_key_data_loaded() by skey_data_available() to check sign key data
is available for hibernate.
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:54, Lee, Chun-Yi wrote:
In current solution, the snapshot signature check used the RSA key-pair that are generated by bootloader(e.g. shim) and pass the key-pair to kernel through EFI variables. I choice to binding the snapshot signature check mechanism with UEFI secure boot for provide stronger protection of hibernate. Current behavior is following:
+ UEFI Secure Boot ON, Kernel found key-pair from shim: Will do the S4 signature check.
+ UEFI Secure Boot ON, Kernel didn't find key-pair from shim: Will lock down S4 function.
+ UEFI Secure Boot OFF Will NOT do the S4 signature check. Ignore any keys from bootloader.
v2: Replace sign_key_data_loaded() by skey_data_available() to check sign key data is available for hibernate.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/hibernate.c | 36 +++++++++++++++++- kernel/power/main.c | 11 +++++- kernel/power/snapshot.c | 95 ++++++++++++++++++++++++++-------------------- kernel/power/swap.c | 4 +- kernel/power/user.c | 11 +++++ 5 files changed, 112 insertions(+), 45 deletions(-) diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index c545b15..0f19f3d 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -29,6 +29,7 @@ #include
#include #include +#include #include "power.h"
@@ -632,7 +633,14 @@ static void power_down(void) int hibernate(void) { int error; - int skey_error; + +#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !skey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + return -EPERM; + }
lock_system_sleep(); /* The snapshot device should not be opened while we're running */ @@ -799,6 +807,15 @@ static int software_resume(void) if (error) goto Unlock;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !wkey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + mutex_unlock(&pm_mutex); + return -EPERM; + } + /* The snapshot device should not be opened while we're running */ if (!atomic_add_unless(&snapshot_device_available, -1, 0)) { error = -EBUSY; @@ -892,6 +909,15 @@ static ssize_t disk_show(struct kobject *kobj, struct kobj_attribute *attr, int i; char *start = buf;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (efi_enabled(EFI_SECURE_BOOT) && !skey_data_available()) { +#else + if (efi_enabled(EFI_SECURE_BOOT)) { +#endif + buf += sprintf(buf, "[%s]\n", "disabled"); + return buf-start; + } + for (i = HIBERNATION_FIRST; i <= HIBERNATION_MAX; i++) { if (!hibernation_modes[i]) continue; @@ -926,6 +952,14 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr, char *p; int mode = HIBERNATION_INVALID;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !skey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + return -EPERM; + } + p = memchr(buf, '\n', n); len = p ? p - buf : n;
You clearly need some helper function. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 日,2013-08-25 於 18:42 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:54, Lee, Chun-Yi wrote:
In current solution, the snapshot signature check used the RSA key-pair that are generated by bootloader(e.g. shim) and pass the key-pair to kernel through EFI variables. I choice to binding the snapshot signature check mechanism with UEFI secure boot for provide stronger protection of hibernate. Current behavior is following:
+ UEFI Secure Boot ON, Kernel found key-pair from shim: Will do the S4 signature check.
+ UEFI Secure Boot ON, Kernel didn't find key-pair from shim: Will lock down S4 function.
+ UEFI Secure Boot OFF Will NOT do the S4 signature check. Ignore any keys from bootloader.
v2: Replace sign_key_data_loaded() by skey_data_available() to check sign key data is available for hibernate.
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/hibernate.c | 36 +++++++++++++++++- kernel/power/main.c | 11 +++++- kernel/power/snapshot.c | 95 ++++++++++++++++++++++++++-------------------- kernel/power/swap.c | 4 +- kernel/power/user.c | 11 +++++ 5 files changed, 112 insertions(+), 45 deletions(-) diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index c545b15..0f19f3d 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -29,6 +29,7 @@ #include
#include #include +#include #include "power.h"
@@ -632,7 +633,14 @@ static void power_down(void) int hibernate(void) { int error; - int skey_error; + +#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !skey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + return -EPERM; + }
lock_system_sleep(); /* The snapshot device should not be opened while we're running */ @@ -799,6 +807,15 @@ static int software_resume(void) if (error) goto Unlock;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !wkey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + mutex_unlock(&pm_mutex); + return -EPERM; + } + /* The snapshot device should not be opened while we're running */ if (!atomic_add_unless(&snapshot_device_available, -1, 0)) { error = -EBUSY; @@ -892,6 +909,15 @@ static ssize_t disk_show(struct kobject *kobj, struct kobj_attribute *attr, int i; char *start = buf;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (efi_enabled(EFI_SECURE_BOOT) && !skey_data_available()) { +#else + if (efi_enabled(EFI_SECURE_BOOT)) { +#endif + buf += sprintf(buf, "[%s]\n", "disabled"); + return buf-start; + } + for (i = HIBERNATION_FIRST; i <= HIBERNATION_MAX; i++) { if (!hibernation_modes[i]) continue; @@ -926,6 +952,14 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr, char *p; int mode = HIBERNATION_INVALID;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION + if (!capable(CAP_COMPROMISE_KERNEL) && !skey_data_available()) { +#else + if (!capable(CAP_COMPROMISE_KERNEL)) { +#endif + return -EPERM; + } + p = memchr(buf, '\n', n); len = p ? p - buf : n;
You clearly need some helper function. Pavel
I will use a help function to replace those ifdef block. Thanks for your suggestion! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Show the verification time for monitor the performance of SHA256 and RSA
verification.
Reviewed-by: Jiri Kosina
This patch introduced SNAPSHOT_SIG_HASH config for user to select which
hash algorithm will be used during signature generation of snapshot.
v2:
Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before
declare pkey_hash().
Reviewed-by: Jiri Kosina
On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
This patch introduced SNAPSHOT_SIG_HASH config for user to select which hash algorithm will be used during signature generation of snapshot.
v2: Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before declare pkey_hash().
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/Kconfig | 46 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/power/snapshot.c | 27 ++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index b592d88..79b34fa 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION dependent on UEFI environment. EFI bootloader should generate the key-pair.
+choice + prompt "Which hash algorithm should snapshot be signed with?" + depends on SNAPSHOT_VERIFICATION + help + This determines which sort of hashing algorithm will be used during + signature generation of snapshot. This algorithm _must_ be built into + the kernel directly so that signature verification can take place. + It is not possible to load a signed snapshot containing the algorithm + to check the signature on that module.
Like if 1000 ifdefs you already added to the code are not enough, you make some new ones? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 日,2013-08-25 於 18:43 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
This patch introduced SNAPSHOT_SIG_HASH config for user to select which hash algorithm will be used during signature generation of snapshot.
v2: Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before declare pkey_hash().
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/Kconfig | 46 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/power/snapshot.c | 27 ++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index b592d88..79b34fa 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION dependent on UEFI environment. EFI bootloader should generate the key-pair.
+choice + prompt "Which hash algorithm should snapshot be signed with?" + depends on SNAPSHOT_VERIFICATION + help + This determines which sort of hashing algorithm will be used during + signature generation of snapshot. This algorithm _must_ be built into + the kernel directly so that signature verification can take place. + It is not possible to load a signed snapshot containing the algorithm + to check the signature on that module.
Like if 1000 ifdefs you already added to the code are not enough, you make some new ones? Pavel
This SNAPSHOT_SIG_HASH kernel config is to select which SHA algorithms used for generate digest of snapshot. The configuration will captured by a const char* in code: +static const char *snapshot_hash = CONFIG_SNAPSHOT_SIG_HASH; + +static int pkey_hash(void) So, there doesn't have any ifdef block derived from this new config. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Tue 2013-08-27 18:22:17, joeyli wrote:
於 日,2013-08-25 於 18:43 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
This patch introduced SNAPSHOT_SIG_HASH config for user to select which hash algorithm will be used during signature generation of snapshot.
v2: Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before declare pkey_hash().
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/Kconfig | 46 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/power/snapshot.c | 27 ++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index b592d88..79b34fa 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION dependent on UEFI environment. EFI bootloader should generate the key-pair.
+choice + prompt "Which hash algorithm should snapshot be signed with?" + depends on SNAPSHOT_VERIFICATION + help + This determines which sort of hashing algorithm will be used during + signature generation of snapshot. This algorithm _must_ be built into + the kernel directly so that signature verification can take place. + It is not possible to load a signed snapshot containing the algorithm + to check the signature on that module.
Like if 1000 ifdefs you already added to the code are not enough, you make some new ones? Pavel
This SNAPSHOT_SIG_HASH kernel config is to select which SHA algorithms used for generate digest of snapshot. The configuration will captured by a const char* in code:
+static const char *snapshot_hash = CONFIG_SNAPSHOT_SIG_HASH; + +static int pkey_hash(void)
So, there doesn't have any ifdef block derived from this new config.
I'd say select one hash function, and use it. There's no need to make it configurable. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 二,2013-08-27 於 13:30 +0200,Pavel Machek 提到:
On Tue 2013-08-27 18:22:17, joeyli wrote:
於 日,2013-08-25 於 18:43 +0200,Pavel Machek 提到:
On Thu 2013-08-22 19:01:56, Lee, Chun-Yi wrote:
This patch introduced SNAPSHOT_SIG_HASH config for user to select which hash algorithm will be used during signature generation of snapshot.
v2: Add define check of oCONFIG_SNAPSHOT_VERIFICATION in snapshot.c before declare pkey_hash().
Reviewed-by: Jiri Kosina
Signed-off-by: Lee, Chun-Yi --- kernel/power/Kconfig | 46 ++++++++++++++++++++++++++++++++++++++++++++++ kernel/power/snapshot.c | 27 ++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index b592d88..79b34fa 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -78,6 +78,52 @@ config SNAPSHOT_VERIFICATION dependent on UEFI environment. EFI bootloader should generate the key-pair.
+choice + prompt "Which hash algorithm should snapshot be signed with?" + depends on SNAPSHOT_VERIFICATION + help + This determines which sort of hashing algorithm will be used during + signature generation of snapshot. This algorithm _must_ be built into + the kernel directly so that signature verification can take place. + It is not possible to load a signed snapshot containing the algorithm + to check the signature on that module.
Like if 1000 ifdefs you already added to the code are not enough, you make some new ones? Pavel
This SNAPSHOT_SIG_HASH kernel config is to select which SHA algorithms used for generate digest of snapshot. The configuration will captured by a const char* in code:
+static const char *snapshot_hash = CONFIG_SNAPSHOT_SIG_HASH; + +static int pkey_hash(void)
So, there doesn't have any ifdef block derived from this new config.
I'd say select one hash function, and use it. There's no need to make it configurable. Pavel
There have better performance when SHA algorithm output shorter hash result. On the other hand, longer hash result provide better security. And, on 64-bits system, the SHA512 has better performance then SHA256. Due to user have different use case and different hardware, why not give them this option to make decision? Thanks a lot! Joey LEe -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
This patch introduced SNAPSHOT_REGEN_KEYS kernel config, enable this
option let kernel notify booloader (e.g. shim) to regenerate key-pair of
snapshot verification for each hibernate.
Kernel loaded S4 sign key in efi stub, so the private key forward from
efi bootloader to kernel in UEFI secure environment. Regenerate key-pair
for each hibernate will gain more security but it hurt the lifetime of
EFI flash memory.
Kernel write an non-volatile runtime efi variable, the name is
GenS4Key-fe141863-c070-478e-b8a3-878a5dc9ef21, to notify efi bootloader
regenerate key-pair for next hibernate cycle.
Userland hibernate tool can write GenS4Key at runtime, kernel will
respect the value but not overwrite it when S4. This mechanism let
userland tool can also notify bootloader to regenerate key-pair through
GenS4Key flag.
Cc: Matthew Garrett
* Chun-Yi Lee:
+ EFI bootloader must generate RSA key-pair when system boot: - Bootloader store the public key to EFI boottime variable by itself - Bootloader put The private key to S4SignKey EFI variable for forward to kernel.
Is the UEFI NVRAM really suited for such regular updates? -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi Florian, Thanks for your response. 於 三,2013-08-28 於 23:01 +0200,Florian Weimer 提到:
* Chun-Yi Lee:
+ EFI bootloader must generate RSA key-pair when system boot:
I should add more information on this sentence for mention need GenS4Key runtime variable then re-generate key-pair. Thanks!
- Bootloader store the public key to EFI boottime variable by itself - Bootloader put The private key to S4SignKey EFI variable for forward to kernel.
Is the UEFI NVRAM really suited for such regular updates?
Yes, Matthew raised this concern at before. I modified patch to load private key in efi stub kernel, before ExitBootServices(), that means we don't need generate key-pair at every system boot. So, the above procedure of efi bootloader will only run one time. User can enable SNAPSHOT_REGEN_KEYS kernel config to notify efi booloader regenerate key-pair for every S4 to improve security if he want. So, the key-pair re-generate procedure will only launched when S4 resume, not every system boot. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
Hi!
- Bootloader store the public key to EFI boottime variable by itself - Bootloader put The private key to S4SignKey EFI variable for forward to kernel.
Is the UEFI NVRAM really suited for such regular updates?
Yes, Matthew raised this concern at before. I modified patch to load private key in efi stub kernel, before ExitBootServices(), that means we don't need generate key-pair at every system boot. So, the above procedure of efi bootloader will only run one time.
User can enable SNAPSHOT_REGEN_KEYS kernel config to notify efi booloader regenerate key-pair for every S4 to improve security if he want. So, the key-pair re-generate procedure will only launched when S4 resume, not every system boot.
How many writes can UEFI NVRAM survive? (Is it NOR?) "every S4 resume" may be approximately "every boot" for some users... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 四,2013-08-29 於 23:32 +0200,Pavel Machek 提到:
Hi!
- Bootloader store the public key to EFI boottime variable by itself - Bootloader put The private key to S4SignKey EFI variable for forward to kernel.
Is the UEFI NVRAM really suited for such regular updates?
Yes, Matthew raised this concern at before. I modified patch to load private key in efi stub kernel, before ExitBootServices(), that means we don't need generate key-pair at every system boot. So, the above procedure of efi bootloader will only run one time.
User can enable SNAPSHOT_REGEN_KEYS kernel config to notify efi booloader regenerate key-pair for every S4 to improve security if he want. So, the key-pair re-generate procedure will only launched when S4 resume, not every system boot.
How many writes can UEFI NVRAM survive? (Is it NOR?)
Currently doesn't have enough information for normal. Yes, I don't know.
"every S4 resume" may be approximately "every boot" for some users... Pavel
Yes, it's possible. So, this option will be disabled by default. Default will only generate one key-pair for every hibernate. If "re-generate key-pair for every S4" is still hurt lift of UEFI NVRAM too much, then another thinking for re-generate key-pair are: + Re-generate key-pair after a number of hibernates. e.g. after 5, 10, 20... times or + Random re-generate key-pair? On the other hand... In current design, GenS4Key EFI variable could be write by userland hibernate tool, kernel will respect GenS4Key value from userland when hibernate launch. So, userland can tell bootloader to lunch the key-pair regeneration procedure. Thanks a lot! Joey LEe -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
* joeyli:
Yes, Matthew raised this concern at before. I modified patch to load private key in efi stub kernel, before ExitBootServices(), that means we don't need generate key-pair at every system boot. So, the above procedure of efi bootloader will only run one time.
But if you don't generate fresh keys on every boot, the persistent keys are mor exposed to other UEFI applications. Correct me if I'm wrong, but I don't think UEFI variables are segregated between different UEFI applications, so if anyone gets a generic UEFI variable dumper (or setter) signed by the trusted key, this cryptographic validation of hibernate snapshots is bypassable. -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
But if you don't generate fresh keys on every boot, the persistent keys are mor exposed to other UEFI applications. Correct me if I'm wrong, but I don't think UEFI variables are segregated between different UEFI applications, so if anyone gets a generic UEFI variable dumper (or setter) signed by the trusted key, this cryptographic validation of hibernate snapshots is bypassable.
If anyone can execute arbitrary code in your UEFI environment then you've already lost. -- Matthew Garrett | mjg59@srcf.ucam.org -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
* Matthew Garrett:
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
But if you don't generate fresh keys on every boot, the persistent keys are mor exposed to other UEFI applications. Correct me if I'm wrong, but I don't think UEFI variables are segregated between different UEFI applications, so if anyone gets a generic UEFI variable dumper (or setter) signed by the trusted key, this cryptographic validation of hibernate snapshots is bypassable.
If anyone can execute arbitrary code in your UEFI environment then you've already lost.
This is not about arbitrary code execution. The problematic applications which conflict with this proposed functionality are not necessarily malicious by themselves and even potentially useful. For example, if you want to provision a bunch of machines and you have to set certain UEFI variables, it might be helpful to do so in an unattended fashion, just by booting from a USB stick with a suitable UEFI application. Is this evil? I don't think so. -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Sun, Sep 01, 2013 at 06:40:41PM +0200, Florian Weimer wrote:
* Matthew Garrett:
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
But if you don't generate fresh keys on every boot, the persistent keys are mor exposed to other UEFI applications. Correct me if I'm wrong, but I don't think UEFI variables are segregated between different UEFI applications, so if anyone gets a generic UEFI variable dumper (or setter) signed by the trusted key, this cryptographic validation of hibernate snapshots is bypassable.
If anyone can execute arbitrary code in your UEFI environment then you've already lost.
This is not about arbitrary code execution. The problematic applications which conflict with this proposed functionality are not necessarily malicious by themselves and even potentially useful.
A signed application that permits the modification of arbitrary boot services variables *is* malicious. No implementation is designed to be safe in that scenario. Why bother with modifying encryption keys when you can just modify MOK instead? -- Matthew Garrett | mjg59@srcf.ucam.org -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
於 日,2013-09-01 於 18:40 +0200,Florian Weimer 提到:
* Matthew Garrett:
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
But if you don't generate fresh keys on every boot, the persistent keys are mor exposed to other UEFI applications. Correct me if I'm wrong, but I don't think UEFI variables are segregated between different UEFI applications, so if anyone gets a generic UEFI variable dumper (or setter) signed by the trusted key, this cryptographic validation of hibernate snapshots is bypassable.
If anyone can execute arbitrary code in your UEFI environment then you've already lost.
This is not about arbitrary code execution. The problematic applications which conflict with this proposed functionality are not necessarily malicious by themselves and even potentially useful.
For example, if you want to provision a bunch of machines and you have to set certain UEFI variables, it might be helpful to do so in an unattended fashion, just by booting from a USB stick with a suitable UEFI application. Is this evil? I don't think so. --
Yes, if there have the kind of UEFI tools like you said, and it leak to attacker, then we lost. Even we re-generate key-pair for every S4, the tool, if it can set variable, means it can replace the public key that was stored by efi bootloader in bootservices variable. Then we still lost. When the tool can only dump variable but not set, then re-generate key-pair to every S4 can prevent it. If the tool can also set variable, then I don't think there have any way to protect key-pair in UEFI variables. Using TPM is a way to protect key-pair, but user need key-in password when generate and use key to sign stuff. It noises to user, but the best way to keep the password is in brain. Thanks a lot! Joey Lee -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
participants (9)
-
Florian Weimer
-
Jiri Kosina
-
joeyli
-
Lee, Chun-Yi
-
Manfred Hollstein
-
Matt Fleming
-
Matthew Garrett
-
Pavel Machek
-
Pavel Machek