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 [email protected] 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
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 33 ++++++++++++++++++++++++---------
1 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 6996ff7..c26ae77 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -121,12 +121,30 @@ static int RSAVP1(const struct public_key *key, MPI s, MPI *_m)
/*
* Integer to Octet String conversion [RFC3447 sec 4.1]
*/
-static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
+static int _RSA_I2OSP(MPI x, unsigned *X_size, u8 **_X)
{
- unsigned X_size, x_size;
int X_sign;
u8 *X;
+ X = mpi_get_buffer(x, X_size, &X_sign);
+ if (!X)
+ return -ENOMEM;
+ if (X_sign < 0) {
+ kfree(X);
+ return -EBADMSG;
+ }
+
+ *_X = X;
+ return 0;
+}
+
+static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
+{
+ unsigned x_size;
+ unsigned X_size;
+ u8 *X = NULL;
+ int ret;
+
/* Make sure the string is the right length. The number should begin
* with { 0x00, 0x01, ... } so we have to account for 15 leading zero
* bits not being reported by MPI.
@@ -136,13 +154,10 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
if (x_size != xLen * 8 - 15)
return -ERANGE;
- X = mpi_get_buffer(x, &X_size, &X_sign);
- if (!X)
- return -ENOMEM;
- if (X_sign < 0) {
- kfree(X);
- return -EBADMSG;
- }
+ ret = _RSA_I2OSP(x, &X_size, &X);
+ if (ret < 0)
+ return ret;
+
if (X_size != xLen - 1) {
kfree(X);
return -EBADMSG;
--
1.6.4.2
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 47 +++++++++++++++++++++++++++++++++++++++--
1 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 0862018..e60defe 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -86,6 +86,39 @@ static const struct {
};
/*
+ * RSASP1() function [RFC3447 sec 5.2.1]
+ */
+static int RSASP1(const struct private_key *key, MPI m, MPI *_s)
+{
+ MPI s;
+ int ret;
+
+ /* (1) Validate 0 <= m < n */
+ if (mpi_cmp_ui(m, 0) < 0) {
+ kleave(" = -EBADMSG [m < 0]");
+ return -EBADMSG;
+ }
+ if (mpi_cmp(m, key->rsa.n) >= 0) {
+ kleave(" = -EBADMSG [m >= n]");
+ return -EBADMSG;
+ }
+
+ s = mpi_alloc(0);
+ if (!s)
+ return -ENOMEM;
+
+ /* (2) s = m^d mod n */
+ ret = mpi_powm(s, m, key->rsa.d, key->rsa.n);
+ if (ret < 0) {
+ mpi_free(s);
+ return ret;
+ }
+
+ *_s = s;
+ return 0;
+}
+
+/*
* RSAVP1() function [RFC3447 sec 5.2.2]
*/
static int RSAVP1(const struct public_key *key, MPI s, MPI *_m)
@@ -173,9 +206,12 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
static int RSA_OS2IP(u8 *X, size_t XLen, MPI *_x)
{
MPI x;
+ int ret;
x = mpi_alloc((XLen + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB);
- mpi_set_buffer(x, X, XLen, 0);
+ ret = mpi_set_buffer(x, X, XLen, 0);
+ if (ret < 0)
+ return ret;
*_x = x;
return 0;
@@ -453,8 +489,13 @@ static struct public_key_signature *RSA_generate_signature(
if (ret < 0)
goto error_v1_5_encode;
- /* TODO 3): s = RSASP1 (K, m) */
- s = m;
+ /* 3): s = RSASP1 (K, m) */
+ RSASP1(key, m, &s);
+
+ pks->rsa.s = s;
+ pks->nr_mpi = 1;
+ pks->k = mpi_get_nbits(s);
+ pks->k = (pks->k + 7) / 8;
/* 4): S = I2OSP (s, k) */
_RSA_I2OSP(s, &X_size, &pks->S);
--
1.6.4.2
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
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/public_key.c | 1 +
include/crypto/public_key.h | 1 +
9 files changed, 400 insertions(+), 0 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
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 6d2c2ea..c0ebd57 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -35,4 +35,15 @@ config X509_CERTIFICATE_PARSER
data and provides the ability to instantiate a crypto key from a
public key packet found inside the certificate.
+config PKCS8_PRIVATE_KEY_INFO_PARSER
+ tristate "PKCS #8 private key info parser"
+ depends on ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+ select ASN1
+ select OID_REGISTRY
+ select CRYPTO_SHA256
+ help
+ This option provides support for parsing PKCS #8 RSA private key info
+ format blobs for key data and provides the ability to instantiate a
+ crypto key from a private key packet.
+
endif # ASYMMETRIC_KEY_TYPE
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index 0727204..65fbc45 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -23,5 +23,21 @@ $(obj)/x509_cert_parser.o: $(obj)/x509-asn1.h $(obj)/x509_rsakey-asn1.h
$(obj)/x509-asn1.o: $(obj)/x509-asn1.c $(obj)/x509-asn1.h
$(obj)/x509_rsakey-asn1.o: $(obj)/x509_rsakey-asn1.c $(obj)/x509_rsakey-asn1.h
+#
+# PKCS8 Private Key handling
+#
+obj-$(CONFIG_PKCS8_PRIVATE_KEY_INFO_PARSER) += pkcs8_key_parser.o
+pkcs8_key_parser-y := \
+ pkcs8-asn1.o \
+ pkcs8_rsakey-asn1.o \
+ pkcs8_info_parser.o \
+ pkcs8_private_key.o
+
+$(obj)/pkcs8_info_parser.o: $(obj)/pkcs8-asn1.c $(obj)/pkcs8_rsakey-asn1.h
+$(obj)/pkcs8-asn1.o: $(obj)/pkcs8-asn1.c $(obj)/pkcs8-asn1.h
+$(obj)/pkcs8_rsakey-asn1.o: $(obj)/pkcs8_rsakey-asn1.c $(obj)/pkcs8_rsakey-asn1.h
+
clean-files += x509-asn1.c x509-asn1.h
clean-files += x509_rsakey-asn1.c x509_rsakey-asn1.h
+clean-files += pkcs8-asn1.c pkcs8-asn1.h
+clean-files += pkcs8_rsakey-asn1.c pkcs8_rsakey-asn1.h
diff --git a/crypto/asymmetric_keys/pkcs8.asn1 b/crypto/asymmetric_keys/pkcs8.asn1
new file mode 100644
index 0000000..89e845d
--- /dev/null
+++ b/crypto/asymmetric_keys/pkcs8.asn1
@@ -0,0 +1,19 @@
+--
+-- Representation of RSA PKCS#8 private key information.
+--
+
+PrivateKeyInfo ::= SEQUENCE {
+ version Version,
+ privateKeyAlgorithm AlgorithmIdentifier,
+ privateKey OCTET STRING ({ pkcs8_extract_key_data })
+ -- Does not support attributes
+ -- attributes [ 0 ] Attributes OPTIONAL
+ }
+
+-- Version ::= INTEGER { two-prime(0), multi(1) }
+Version ::= INTEGER
+
+AlgorithmIdentifier ::= SEQUENCE {
+ algorithm OBJECT IDENTIFIER ({ pkcs8_note_OID }),
+ parameters ANY OPTIONAL
+ }
diff --git a/crypto/asymmetric_keys/pkcs8_info_parser.c b/crypto/asymmetric_keys/pkcs8_info_parser.c
new file mode 100644
index 0000000..2da19b9
--- /dev/null
+++ b/crypto/asymmetric_keys/pkcs8_info_parser.c
@@ -0,0 +1,152 @@
+/* X.509 certificate parser
+ *
+ * Copyright (C) 2013 SUSE Linux Products GmbH. All rights reserved.
+ * Written by Lee, Chun-Yi ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "PKCS8: "fmt
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/oid_registry.h>
+#include "public_key.h"
+#include "pkcs8_parser.h"
+#include "pkcs8-asn1.h"
+#include "pkcs8_rsakey-asn1.h"
+
+struct pkcs8_parse_context {
+ struct pkcs8_info *info; /* Certificate being constructed */
+ unsigned long data; /* Start of data */
+ const void *key; /* Key data */
+ size_t key_size; /* Size of key data */
+ enum OID algo_oid; /* Algorithm OID */
+ unsigned char nr_mpi; /* Number of MPIs stored */
+};
+
+/*
+ * Free an PKCS #8 private key info
+ */
+void pkcs8_free_info(struct pkcs8_info *info)
+{
+ if (info) {
+ public_key_destroy(info->priv);
+ kfree(info);
+ }
+}
+
+/*
+ * Parse an PKCS #8 Private Key Info
+ */
+struct pkcs8_info *pkcs8_info_parse(const void *data, size_t datalen)
+{
+ struct pkcs8_info *info;
+ struct pkcs8_parse_context *ctx;
+ long ret;
+
+ ret = -ENOMEM;
+ info = kzalloc(sizeof(struct pkcs8_info), GFP_KERNEL);
+ if (!info)
+ goto error_no_info;
+ info->priv = kzalloc(sizeof(struct private_key), GFP_KERNEL);
+ if (!info->priv)
+ goto error_no_ctx;
+ ctx = kzalloc(sizeof(struct pkcs8_parse_context), GFP_KERNEL);
+ if (!ctx)
+ goto error_no_ctx;
+
+ ctx->info = info;
+ ctx->data = (unsigned long)data;
+
+ /* Attempt to decode the private key info */
+ ret = asn1_ber_decoder(&pkcs8_decoder, ctx, data, datalen);
+ if (ret < 0)
+ goto error_decode;
+
+ /* Decode the private key */
+ ret = asn1_ber_decoder(&pkcs8_rsakey_decoder, ctx,
+ ctx->key, ctx->key_size);
+ if (ret < 0)
+ goto error_decode;
+
+ kfree(ctx);
+ return info;
+
+error_decode:
+ kfree(ctx);
+error_no_ctx:
+ pkcs8_free_info(info);
+error_no_info:
+ return ERR_PTR(ret);
+}
+
+/*
+ * Note an OID when we find one for later processing when we know how
+ * to interpret it.
+ */
+int pkcs8_note_OID(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct pkcs8_parse_context *ctx = context;
+
+ ctx->algo_oid = look_up_OID(value, vlen);
+ if (ctx->algo_oid == OID__NR) {
+ char buffer[50];
+ sprint_oid(value, vlen, buffer, sizeof(buffer));
+ pr_debug("Unknown OID: [%lu] %s\n",
+ (unsigned long)value - ctx->data, buffer);
+ }
+ return 0;
+}
+
+/*
+ * Extract the data for the private key algorithm
+ */
+int pkcs8_extract_key_data(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct pkcs8_parse_context *ctx = context;
+
+ if (ctx->algo_oid != OID_rsaEncryption)
+ return -ENOPKG;
+
+ ctx->info->privkey_algo = PKEY_ALGO_RSA;
+ ctx->key = value;
+ ctx->key_size = vlen;
+ return 0;
+}
+
+/*
+ * Extract a RSA private key value
+ */
+int rsa_priv_extract_mpi(void *context, size_t hdrlen,
+ unsigned char tag,
+ const void *value, size_t vlen)
+{
+ struct pkcs8_parse_context *ctx = context;
+ MPI mpi;
+
+ if (ctx->nr_mpi >= ARRAY_SIZE(ctx->info->priv->mpi)) {
+ /* does not grab exponent1, exponent2 and coefficient */
+ if (ctx->nr_mpi > 8) {
+ pr_err("Too many public key MPIs in pkcs1 private key\n");
+ return -EBADMSG;
+ } else {
+ ctx->nr_mpi++;
+ return 0;
+ }
+ }
+
+ mpi = mpi_read_raw_data(value, vlen);
+ if (!mpi)
+ return -ENOMEM;
+
+ ctx->info->priv->mpi[ctx->nr_mpi++] = mpi;
+ return 0;
+}
diff --git a/crypto/asymmetric_keys/pkcs8_parser.h b/crypto/asymmetric_keys/pkcs8_parser.h
new file mode 100644
index 0000000..7f5d801
--- /dev/null
+++ b/crypto/asymmetric_keys/pkcs8_parser.h
@@ -0,0 +1,23 @@
+/* PKCS #8 parser internal definitions
+ *
+ * Copyright (C) 2013 SUSE Linux Products GmbH. All rights reserved.
+ * Written by Lee, Chun-Yi ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <crypto/public_key.h>
+
+struct pkcs8_info {
+ enum pkey_algo privkey_algo:8; /* Private key algorithm */
+ struct private_key *priv; /* Private key */
+};
+
+/*
+ * pkcs8_parser.c
+ */
+extern void pkcs8_free_info(struct pkcs8_info *info);
+extern struct pkcs8_info *pkcs8_info_parse(const void *data, size_t datalen);
diff --git a/crypto/asymmetric_keys/pkcs8_private_key.c b/crypto/asymmetric_keys/pkcs8_private_key.c
new file mode 100644
index 0000000..cf2545b
--- /dev/null
+++ b/crypto/asymmetric_keys/pkcs8_private_key.c
@@ -0,0 +1,148 @@
+/* Instantiate a private key crypto key
+ *
+ * Copyright (C) 2013 SUSE Linux Products GmbH. All rights reserved.
+ * Written by Chun-Yi Lee ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "PKCS8: "fmt
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/asymmetric-parser.h>
+#include <crypto/hash.h>
+#include "private_key.h"
+#include "pkcs8-asn1.h"
+#include "pkcs8_parser.h"
+
+#define KEY_PREFIX "Private Key: "
+#define FINGERPRINT_HASH "sha256"
+
+static const
+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
+};
+
+/*
+ * Attempt to parse a data blob for a private key.
+ */
+static int pkcs8_key_preparse(struct key_preparsed_payload *prep)
+{
+ struct pkcs8_info *info;
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ u8 *digest;
+ size_t digest_size, desc_size;
+ char *fingerprint, *description;
+ int i, ret;
+
+ pr_info("pkcs8_key_preparse start\n");
+
+ info = pkcs8_info_parse(prep->data, prep->datalen);
+ if (IS_ERR(info))
+ return PTR_ERR(info);
+
+ info->priv->algo = pkcs8_private_key_algorithms[info->privkey_algo];
+ info->priv->id_type = PKEY_ID_PKCS8;
+
+ /* Hash the pkcs #8 blob to generate fingerprint */
+ tfm = crypto_alloc_shash(FINGERPRINT_HASH, 0, 0);
+ if (IS_ERR(tfm)) {
+ ret = PTR_ERR(tfm);
+ goto error_shash;
+ }
+ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+ digest_size = crypto_shash_digestsize(tfm);
+
+ ret = -ENOMEM;
+
+ digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
+ if (!digest)
+ goto error_digest;
+ desc = (void *) digest + digest_size;
+ desc->tfm = tfm;
+ desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+
+ ret = crypto_shash_init(desc);
+ if (ret < 0)
+ goto error_shash_init;
+ ret = crypto_shash_finup(desc, prep->data, prep->datalen, digest);
+ if (ret < 0)
+ goto error_shash_finup;
+
+ fingerprint = kzalloc(digest_size * 2 + 1, GFP_KERNEL);
+ if (!fingerprint)
+ goto error_fingerprint;
+ for (i = 0; i < digest_size; i++)
+ sprintf(fingerprint + i * 2, "%02x", digest[i]);
+
+ /* Propose a description */
+ description = kzalloc(strlen(KEY_PREFIX) + strlen(fingerprint) + 1, GFP_KERNEL);
+ if (!description)
+ goto error_description;
+ sprintf(description, "%s", KEY_PREFIX);
+ memcpy(description + strlen(KEY_PREFIX), fingerprint, strlen(fingerprint));
+
+ /* We're pinning the module by being linked against it */
+ __module_get(private_key_subtype.owner);
+ prep->type_data[0] = &private_key_subtype;
+ prep->type_data[1] = fingerprint;
+ prep->payload = info->priv;
+ prep->description = description;
+
+ /* size of 4096 bits private key file is 2.3K */
+ prep->quotalen = 700;
+
+ pr_info("pkcs8_key_preparse done\n");
+
+ /* We've finished with the information */
+ kfree(digest);
+ crypto_free_shash(tfm);
+ info->priv = NULL;
+ pkcs8_free_info(info);
+
+ return 0;
+
+error_description:
+ kfree(fingerprint);
+error_fingerprint:
+error_shash_finup:
+error_shash_init:
+ kfree(digest);
+error_digest:
+ crypto_free_shash(tfm);
+error_shash:
+ info->priv = NULL;
+ pkcs8_free_info(info);
+ return ret;
+}
+
+static struct asymmetric_key_parser pkcs8_private_key_parser = {
+ .owner = THIS_MODULE,
+ .name = "pkcs8",
+ .parse = pkcs8_key_preparse,
+};
+
+/*
+ * Module stuff
+ */
+static int __init pkcs8_private_key_init(void)
+{
+ return register_asymmetric_key_parser(&pkcs8_private_key_parser);
+}
+
+static void __exit pkcs8_private_key_exit(void)
+{
+ unregister_asymmetric_key_parser(&pkcs8_private_key_parser);
+}
+
+module_init(pkcs8_private_key_init);
+module_exit(pkcs8_private_key_exit);
diff --git a/crypto/asymmetric_keys/pkcs8_rsakey.asn1 b/crypto/asymmetric_keys/pkcs8_rsakey.asn1
new file mode 100644
index 0000000..d997c5e
--- /dev/null
+++ b/crypto/asymmetric_keys/pkcs8_rsakey.asn1
@@ -0,0 +1,29 @@
+--
+-- Representation of RSA private key with information.
+--
+
+RSAPrivateKey ::= SEQUENCE {
+ version Version,
+ modulus INTEGER ({ rsa_priv_extract_mpi }), -- n
+ publicExponent INTEGER ({ rsa_priv_extract_mpi }), -- e
+ privateExponent INTEGER ({ rsa_priv_extract_mpi }), -- d
+ prime1 INTEGER ({ rsa_priv_extract_mpi }), -- p
+ prime2 INTEGER ({ rsa_priv_extract_mpi }), -- q
+ exponent1 INTEGER ({ rsa_priv_extract_mpi }), -- d mod (p-1)
+ exponent2 INTEGER ({ rsa_priv_extract_mpi }), -- d mod (q-1)
+ coefficient INTEGER ({ rsa_priv_extract_mpi }) -- (inverse of q) mod p
+ -- Doesn't support multi-prime
+ -- otherPrimeInfos [ 0 ] OtherPrimeInfos OPTIONAL
+ }
+
+-- Version ::= INTEGER { two-prime(0), multi(1) }
+Version ::= INTEGER
+
+-- OtherPrimeInfos ::= SEQUENCE SIZE(1..MAX) OF OtherPrimeInfo
+OtherPrimeInfos ::= SEQUENCE OF OtherPrimeInfo
+
+OtherPrimeInfo ::= SEQUENCE {
+ prime INTEGER, -- ri
+ exponent INTEGER, -- di
+ coefficient INTEGER -- ti
+}
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 97ff932..1636c4c 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -44,6 +44,7 @@ EXPORT_SYMBOL_GPL(pkey_hash_algo);
const char *const pkey_id_type[PKEY_ID_TYPE__LAST] = {
[PKEY_ID_PGP] = "PGP",
[PKEY_ID_X509] = "X509",
+ [PKEY_ID_PKCS8] = "PKCS8",
};
EXPORT_SYMBOL_GPL(pkey_id_type);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 1cdf457..e51f294 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -41,6 +41,7 @@ extern const char *const pkey_hash_algo[PKEY_HASH__LAST];
enum pkey_id_type {
PKEY_ID_PGP, /* OpenPGP generated key ID */
PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
+ PKEY_ID_PKCS8, /* PKCS #8 Private Key */
PKEY_ID_TYPE__LAST
};
--
1.6.4.2
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index e60defe..1fadc7f 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -401,6 +401,7 @@ static int RSA_verify_signature(const struct public_key *key,
/* Variables as per RFC3447 sec 8.2.2 */
const u8 *H = sig->digest;
u8 *EM = NULL;
+ u8 *_EM = NULL;
MPI m = NULL;
size_t k;
@@ -435,14 +436,19 @@ static int RSA_verify_signature(const struct public_key *key,
/* (2c) Convert the message representative (m) to an encoded message
* (EM) of length k octets.
*
- * NOTE! The leading zero byte is suppressed by MPI, so we pass a
- * pointer to the _preceding_ byte to RSA_verify()!
+ * NOTE! The leading zero byte is suppressed by MPI, so we add it
+ * back to EM before input to RSA_verify()!
*/
- ret = RSA_I2OSP(m, k, &EM);
+ ret = RSA_I2OSP(m, k, &_EM);
if (ret < 0)
goto error;
- 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);
+
+ ret = RSA_verify(H, EM, k, sig->digest_size,
RSA_ASN1_templates[sig->pkey_hash_algo].data,
RSA_ASN1_templates[sig->pkey_hash_algo].size);
--
1.6.4.2
From: Matthew Garrett <[email protected]>
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.
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Lee, Chun-Yi <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
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)
--
1.6.4.2
From: Josh Boyer <[email protected]>
This forcibly drops CAP_COMPROMISE_KERNEL from both cap_permitted and cap_bset
in the init_cred struct, which everything else inherits from. This works on
any machine and can be used to develop even if the box doesn't have UEFI.
Signed-off-by: Josh Boyer <[email protected]>
Acked-by: Lee, Chun-Yi <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
Documentation/kernel-parameters.txt | 7 +++++++
kernel/cred.c | 17 +++++++++++++++++
2 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 15356ac..6ad8292 100644
--- 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);
+}
+
+/* Dummy Secure Boot enable option to fake out UEFI SB=1 */
+static int __init secureboot_enable_opt(char *str)
+{
+ int sb_enable = !!simple_strtol(str, NULL, 0);
+ if (sb_enable)
+ secureboot_enable();
+ return 1;
+}
+__setup("secureboot_enable=", secureboot_enable_opt);
+
/**
* prepare_kernel_cred - Prepare a set of credentials for a kernel service
* @daemon: A userspace daemon to be used as a reference
--
1.6.4.2
This patch applied SNAPSHOT_VERIFICATION kernel config for switching
signature check of hibernate snapshot image.
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/snapshot.c | 19 +++++++++++++++++++
kernel/power/swap.c | 30 +++++++++++++++++++-----------
kernel/power/user.c | 2 ++
3 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 9e4c94b..cf3d69c 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1052,6 +1052,8 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
{
struct zone *zone;
unsigned long pfn, dst_pfn;
+
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
struct page *d_page;
void *hash_buffer = NULL;
struct crypto_shash *tfm;
@@ -1083,6 +1085,7 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
ret = crypto_shash_init(desc);
if (ret < 0)
goto error_shash;
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
for_each_populated_zone(zone) {
unsigned long max_zone_pfn;
@@ -1102,6 +1105,7 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
dst_pfn = memory_bm_next_pfn(copy_bm);
copy_data_page(dst_pfn, pfn);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
/* Generate digest */
d_page = pfn_to_page(dst_pfn);
if (PageHighMem(d_page)) {
@@ -1116,8 +1120,10 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
ret = crypto_shash_update(desc, hash_buffer, PAGE_SIZE);
if (ret)
goto error_shash;
+#endif
}
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
crypto_shash_final(desc, digest);
if (ret)
goto error_shash;
@@ -1147,9 +1153,11 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
kfree(pks);
kfree(digest);
crypto_free_shash(tfm);
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
return 0;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
error_sign:
destroy_sign_key(s4_sign_key);
error_key:
@@ -1158,6 +1166,7 @@ error_shash:
error_digest:
crypto_free_shash(tfm);
return ret;
+#endif
}
/* Total number of image pages */
@@ -2321,8 +2330,10 @@ static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca,
return pbe->address;
}
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
void **h_buf;
void *skey_snapshot_buf;
+#endif
/**
* snapshot_write_next - used for writing the system memory snapshot.
@@ -2367,12 +2378,14 @@ int snapshot_write_next(struct snapshot_handle *handle)
if (error)
return error;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
/* Allocate void * array to keep buffer point for generate hash,
* h_buf will freed in snapshot_image_verify().
*/
h_buf = kmalloc(sizeof(void *) * nr_copy_pages, GFP_KERNEL);
if (!h_buf)
pr_err("Allocate hash buffer fail!");
+#endif
error = memory_bm_create(©_bm, GFP_ATOMIC, PG_ANY);
if (error)
@@ -2400,8 +2413,10 @@ int snapshot_write_next(struct snapshot_handle *handle)
handle->sync_read = 0;
if (IS_ERR(handle->buffer))
return PTR_ERR(handle->buffer);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
if (h_buf)
*h_buf = handle->buffer;
+#endif
}
} else {
copy_last_highmem_page();
@@ -2412,11 +2427,13 @@ int snapshot_write_next(struct snapshot_handle *handle)
return PTR_ERR(handle->buffer);
if (handle->buffer != buffer)
handle->sync_read = 0;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
if (h_buf)
*(h_buf + (handle->cur - nr_meta_pages - 1)) = handle->buffer;
/* Keep the buffer of sign key in snapshot */
if (pfn == skey_data_buf_pfn)
skey_snapshot_buf = handle->buffer;
+#endif
}
handle->cur++;
return PAGE_SIZE;
@@ -2449,6 +2466,7 @@ int snapshot_image_loaded(struct snapshot_handle *handle)
handle->cur <= nr_meta_pages + nr_copy_pages);
}
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
int snapshot_verify_signature(u8 *digest, size_t digest_size)
{
struct key *s4_wake_key;
@@ -2575,6 +2593,7 @@ void snapshot_fill_s4_skey(void)
erase_skey_data();
pr_info("PM: Fill new s4 key to snapshot");
}
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
#ifdef CONFIG_HIGHMEM
/* Assumes that @buf is ready and points to a "safe" page */
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index f6eaf6e..b5f8ce1 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1004,13 +1004,17 @@ static int load_image(struct swap_map_handle *handle,
snapshot_write_finalize(snapshot);
if (!snapshot_image_loaded(snapshot))
ret = -ENODATA;
- ret = snapshot_image_verify();
- if (ret)
- pr_info("PM: snapshot signature check FAIL: %d\n", ret);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
else {
- pr_info("PM: snapshot signature check SUCCESS!\n");
- snapshot_fill_s4_skey();
+ ret = snapshot_image_verify();
+ if (ret)
+ pr_info("PM: snapshot signature check FAIL: %d\n", ret);
+ else {
+ pr_info("PM: snapshot signature check SUCCESS!\n");
+ snapshot_fill_s4_skey();
+ }
}
+#endif
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
return ret;
@@ -1365,13 +1369,17 @@ out_finish:
}
}
}
- ret = snapshot_image_verify();
- if (ret)
- pr_info("PM: snapshot signature check FAIL: %d\n", ret);
- else {
- pr_info("PM: snapshot signature check SUCCESS!\n");
- snapshot_fill_s4_skey();
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+ if (!ret) {
+ ret = snapshot_image_verify();
+ if (ret)
+ pr_info("PM: snapshot signature check FAIL: %d\n", ret);
+ else {
+ pr_info("PM: snapshot signature check SUCCESS!\n");
+ snapshot_fill_s4_skey();
+ }
}
+#endif
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
out_clean:
diff --git a/kernel/power/user.c b/kernel/power/user.c
index c092e81..27b21ee 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -254,6 +254,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
error = -EPERM;
break;
}
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
if (!snapshot_image_verify()) {
pr_info("PM: snapshot signature check SUCCESS!\n");
snapshot_fill_s4_skey();
@@ -262,6 +263,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
error = -EPERM;
break;
}
+#endif
error = hibernation_restore(data->platform_support);
break;
--
1.6.4.2
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
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.
+
+config SNAPSHOT_SIG_SHA1
+ bool "Sign modules with SHA-1"
+ select CRYPTO_SHA1
+ select CRYPTO_SHA1_SSSE3 if X86_64
+
+config SNAPSHOT_SIG_SHA224
+ bool "Sign modules with SHA-224"
+ select CRYPTO_SHA256
+ select CRYPTO_SHA256_SSSE3 if X86_64
+
+config SNAPSHOT_SIG_SHA256
+ bool "Sign modules with SHA-256"
+ select CRYPTO_SHA256
+ select CRYPTO_SHA256_SSSE3 if X86_64
+
+config SNAPSHOT_SIG_SHA384
+ bool "Sign modules with SHA-384"
+ select CRYPTO_SHA512
+ select CRYPTO_SHA512_SSSE3 if X86_64
+
+config SNAPSHOT_SIG_SHA512
+ bool "Sign modules with SHA-512"
+ select CRYPTO_SHA512
+ select CRYPTO_SHA512_SSSE3 if X86_64
+
+endchoice
+
+config SNAPSHOT_SIG_HASH
+ string
+ depends on SNAPSHOT_VERIFICATION
+ default "sha1" if SNAPSHOT_SIG_SHA1
+ default "sha224" if SNAPSHOT_SIG_SHA224
+ default "sha256" if SNAPSHOT_SIG_SHA256
+ default "sha384" if SNAPSHOT_SIG_SHA384
+ default "sha512" if SNAPSHOT_SIG_SHA512
+
config PM_STD_PARTITION
string "Default resume partition"
depends on HIBERNATION
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index b9c6a8a..f02e351 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1042,12 +1042,29 @@ static inline void copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
}
#endif /* CONFIG_HIGHMEM */
-#define SNAPSHOT_HASH "sha256"
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+static const char *snapshot_hash = CONFIG_SNAPSHOT_SIG_HASH;
+
+static int pkey_hash(void)
+{
+ int i, ret;
+
+ ret = -1;
+ for (i = 0; i < PKEY_HASH__LAST; i++) {
+ if (!strcmp(pkey_hash_algo[i], snapshot_hash)) {
+ ret = i;
+ break;
+ }
+ }
+
+ return ret;
+}
/*
* Signature of snapshot for check.
*/
static u8 signature[SIG_LENG];
+#endif
static int
copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
@@ -1068,7 +1085,7 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
ret = -ENOMEM;
if (!capable(CAP_COMPROMISE_KERNEL)) {
- tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0);
+ 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);
@@ -1145,7 +1162,7 @@ copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
goto error_key;
}
- pks = generate_signature(s4_sign_key, digest, PKEY_HASH_SHA256, false);
+ pks = generate_signature(s4_sign_key, digest, pkey_hash(), false);
if (IS_ERR(pks)) {
pr_err("Generate signature fail: %lx", PTR_ERR(pks));
ret = PTR_ERR(pks);
@@ -2499,7 +2516,7 @@ int snapshot_verify_signature(u8 *digest, size_t digest_size)
pr_err("PM: Allocate public key signature fail!");
return -ENOMEM;
}
- pks->pkey_hash_algo = PKEY_HASH_SHA256;
+ pks->pkey_hash_algo = pkey_hash();
pks->digest = digest;
pks->digest_size = digest_size;
@@ -2547,7 +2564,7 @@ int snapshot_image_verify(void)
if (!h_buf)
return 0;
- tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0);
+ 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);
--
1.6.4.2
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 <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/Kconfig | 15 +++++++++++++++
kernel/power/hibernate_keys.c | 39 +++++++++++++++++++++++++++++++++++++++
kernel/power/power.h | 2 ++
kernel/power/snapshot.c | 3 +++
4 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index 79b34fa..63bda98 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -78,6 +78,21 @@ config SNAPSHOT_VERIFICATION
dependent on UEFI environment. EFI bootloader should generate the
key-pair.
+config SNAPSHOT_REGEN_KEYS
+ bool "Regenerate key-pair for each snapshot verification"
+ depends on SNAPSHOT_VERIFICATION
+ help
+ Enabled this option let kernel notify booloader (e.g. shim) to
+ regenerate key-pair of snapshot verification for each hibernate.
+ Linux 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 then 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.
+
choice
prompt "Which hash algorithm should snapshot be signed with?"
depends on SNAPSHOT_VERIFICATION
diff --git a/kernel/power/hibernate_keys.c b/kernel/power/hibernate_keys.c
index 1bc5976..60d3e73 100644
--- a/kernel/power/hibernate_keys.c
+++ b/kernel/power/hibernate_keys.c
@@ -7,6 +7,8 @@
#include "power.h"
+static efi_char16_t efi_gens4key_name[9] = { 'G', 'e', 'n', 'S', '4', 'K', 'e', 'y', 0 };
+
static void *skey_data;
static void *skey_data_buf;
static unsigned long skey_dsize;
@@ -273,6 +275,42 @@ size_t get_key_length(const struct key *key)
return len;
}
+void set_key_regen_flag(void)
+{
+#ifdef CONFIG_SNAPSHOT_REGEN_KEYS
+ unsigned long datasize;
+ u8 gens4key;
+ efi_status_t status;
+
+ /* existing flag may set by userland, respect but not overwrite it */
+ datasize = 0;
+ status = efi.get_variable(efi_gens4key_name, &EFI_HIBERNATE_GUID,
+ NULL, &datasize, NULL);
+ if (status == EFI_BUFFER_TOO_SMALL)
+ return;
+
+ /* set flag of key-pair regeneration */
+ gens4key = 1;
+ status = efi.set_variable(efi_gens4key_name, &EFI_HIBERNATE_GUID,
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ 1, (void *)&gens4key);
+ if (status)
+ pr_err("PM: Set GenS4Key flag fail: 0x%lx\n", status);
+#endif
+}
+
+static void clean_key_regen_flag(void)
+{
+ /* clean flag of key-pair regeneration */
+ efi.set_variable(efi_gens4key_name, &EFI_HIBERNATE_GUID,
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ 0, NULL);
+}
+
static int __init init_sign_key_data(void)
{
skey_data = (void *)get_zeroed_page(GFP_KERNEL);
@@ -283,6 +321,7 @@ static int __init init_sign_key_data(void)
efi_erase_s4_skey_data();
pr_info("PM: Load s4 sign key from EFI\n");
}
+ clean_key_regen_flag();
return 0;
}
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 84e0b06..3fcde36 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -181,6 +181,7 @@ extern void restore_sign_key_data(void);
extern bool swsusp_page_is_sign_key(struct page *page);
extern unsigned long get_skey_data_buf_pfn(void);
extern void clone_skey_data(void *page_addr);
+extern void set_key_regen_flag(void);
#else /* !CONFIG_SUSPEND */
static inline void restore_sign_key_data(void) {}
static inline bool swsusp_page_is_sign_key(struct page *page)
@@ -191,6 +192,7 @@ static inline unsigned long get_skey_data_buf_pfn(void)
{
return 0;
}
+static inline void set_key_regen_flag(void) {}
#endif /* !CONFIG_SNAPSHOT_VERIFICATION */
/* kernel/power/block_io.c */
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index f02e351..12174ff 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1769,6 +1769,9 @@ asmlinkage int swsusp_save(void)
printk(KERN_INFO "PM: Hibernation image created (%d pages copied)\n",
nr_pages);
+ /* set regenerate key flag */
+ set_key_regen_flag();
+
return 0;
}
--
1.6.4.2
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 <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> +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
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 <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> +#include <crypto/public_key.h>
> +
> +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
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 <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> - 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
On Thu 2013-08-22 19:01:47, Lee, Chun-Yi wrote:
> From: Matthew Garrett <[email protected]>
>
> 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 <[email protected]>
> Acked-by: Lee, Chun-Yi <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> ---
> 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
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
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 <[email protected]>
> Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
> ---
> 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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
>
> > +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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
>
> > +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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
>
> > +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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
>
> > +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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
>
> > +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
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 <[email protected]>
> > > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
> >
> > > +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
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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
> > >
> > > > +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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > 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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
> > ---
> > 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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > 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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > 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
於 日，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 <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > 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
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 <[email protected]>
> > > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > > ---
> > > 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
於 二，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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/[email protected]>
> > > > ---
> > > > 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
於 二，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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > > > ---
> > > > 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
於 二，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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > > > ---
> > > > 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
於 二，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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > > > ---
> > > > 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
於 二，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 <[email protected]>
> > > > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > > > ---
> > > > 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
* 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?
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
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
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
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
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
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
於 四，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
於 四，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
於 四，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
於 四，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
於 四，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
* 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.
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 | [email protected]
* 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.
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 | [email protected]
於 日，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
於 日，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
於 日，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
於 日，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
於 日，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