Re: [PATCH 13/18] Hibernate: Avoid S4 sign key data included in snapshot image
於 日，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 jkos...@suse.cz Signed-off-by: Lee, Chun-Yi j...@suse.com --- 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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/18] asymmetric keys: separate the length checking of octet string from RSA_I2OSP
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 jkos...@suse.cz
Signed-off-by: Lee, Chun-Yi j...@suse.com
+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 from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kernel/padata.c: Register hotcpu notifier after initialization
On Fri, Aug 23, 2013 at 01:12:33PM +0200, Richard Weinberger wrote: padata_cpu_callback() takes pinst-lock, to avoid taking an uninitialized lock, register the notifier after it's initialization. Signed-off-by: Richard Weinberger rich...@nod.at Looks ok, Acked-by: Steffen Klassert steffen.klass...@secunet.com -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/18] Hibernate: adapt to UEFI secure boot with signature check
於 日，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 jkos...@suse.cz
Signed-off-by: Lee, Chun-Yi j...@suse.com
---
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 linux/ctype.h
#include linux/genhd.h
#include linux/key.h
+#include linux/efi.h
#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 from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/18] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm
於 日，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 jkos...@suse.cz Signed-off-by: Lee, Chun-Yi j...@suse.com --- 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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
@@ -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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/18] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm
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 jkos...@suse.cz Signed-off-by: Lee, Chun-Yi j...@suse.com --- 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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 17/18] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm
於 二，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 jkos...@suse.cz Signed-off-by: Lee, Chun-Yi j...@suse.com --- 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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
於 二，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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
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 from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
