2013-09-15 00:56:38

by Lee, Chun-Yi

[permalink] [raw]
Subject: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

Hi experts,

This patchset is the implementation for signature verification of hibernate
snapshot image. The origin idea is from Jiri Kosina: Let EFI bootloader
generate key-pair in UEFI secure boot environment, then pass it to kernel
for sign/verify S4 image.

Due to there have potential threat from the S4 image hacked, it may causes
kernel lost the trust in UEFI secure boot. Hacker attack the S4 snapshot
image in swap partition through whatever exploit from another trusted OS,
and the exploit may don't need physical access machine.

So, this patchset give the ability to kernel for parsing 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 CONFIG_SNAPSHOT_VERIFICATION kernel config to enable. And you can also
choice which hash algorithm should snapshot be signed with. Then rebuild
kernel. This function depends on EFI_STUB.

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.

The default behavior is taint kernel when signature check fail. If you want
direct fail whole hibernate snapshot restore procedure when signature check
does not pass, please use snapshot_sig_enforce kernel parameter or
CONFIG_EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE config.

If you want binding UEF secure boot with sig_enforce flag, then you can use
CONFIG_EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE config. The sig_enforce flag will
auto enabled when UEFI secure boot enabled.

Behavior
=========

The RSA key-pair are generated by EFI bootloader(e.g. shim) in UEFI secure
boot environment, then put private key to S4SignKey, public key to S4WakeKey
EFI variable. Kernel's behavior as following:

+ First, EFI stub 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 stub kernel will load the S4SignKey blob to RAM before ExitBootServices,
then copy to a memory page that's maintained by hibernate_key.c. This
private key will be used to sign snapshot when hibernate launched.

+ When sig_enforce flag set to TRUE, means force check verification pass:
- If kernel didn't find S4 key-pair, then kernel will block hibernate
functions including kernel space and userspace hibernate.
- If snapshot signature check fail when hibernate resume, the snapshot
restore procedure will fail and running normal boot process.

+ When sig_enforce flag set to FALSE, means not force the verification pass:
- If kernel didn't find S4 key-pair, then the hibernate function still
available. But kernel will be tainted after hibernate resume.
- If snapshot signature check fail when hibernate resume, the snapshot
restore procedure will allow continue. But kernel will be tainted.

On EFI bootloader side, the behavior as following:

+ EFI bootloader must generate RSA key-pair when:
- First time boot, bootloader generate key-pair when didn't find it.

- Bootloader need re-generate after found GenS4Key efi variable from OS:
GenS4Key-fe141863-c070-478e-b8a3-878a5dc9ef21 [Runtime][Non-Volatile]
The size of GenS4Key is 1 byte, OS(kernel or userland tool) will set it to
"1" for notify efi bootloader regenerate key-pair.

- If re-generate key-pair, bootloader need store the new public key to
EFI bootservices variable by itself, e.g. store to NextWakeKey variable
When system resumed from hibernate, bootloader need copy public key from
NextWakeKey to S4WakeKey, then kernel will use it to verify snapshot
image.

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, It Base on Matthew Garrett's 2
patches in "[PATCH] Add additional security checks when module loading is
restricted" series: https://lkml.org/lkml/2013/8/19/331
[PATCH 01/10] Add secure_modules() call
[PATCH V3 11/11] Add option to automatically enforce module signatures when in Secure Boot mode

Please help review this RFC patchset! Appreciate for any comments!


V4:
- Cleaned source code, use helper functions to reduce #ifdef chunk.
- Introduced sig_enforce flag, taint kernel when signature check fail.
- Use forward_info structure to maintain the empty of forward information and
new sign key from boot kernel to resume target kernel.
- Use efivar API to access S4WakeKey variable.
- Call set_key_regen_flag() to set key-pair regeneration flag in hibernate.c
and user.c

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


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: taint kernel when signature check fail
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
Hibernate: adapt to UEFI secure boot with signature check

Documentation/kernel-parameters.txt | 7 +
arch/x86/Kconfig | 11 +
arch/x86/boot/compressed/eboot.c | 92 +++++++
arch/x86/include/asm/efi.h | 9 +
arch/x86/include/uapi/asm/bootparam.h | 1 +
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 | 296 +++++++++++++++++++-
crypto/asymmetric_keys/signature.c | 28 ++
include/crypto/public_key.h | 28 ++
include/keys/asymmetric-subtype.h | 6 +
include/linux/efi.h | 25 ++
include/linux/kernel.h | 1 +
include/linux/suspend.h | 7 +
kernel/panic.c | 2 +
kernel/power/Kconfig | 77 +++++-
kernel/power/Makefile | 1 +
kernel/power/hibernate.c | 23 ++-
kernel/power/hibernate_keys.c | 410 ++++++++++++++++++++++++++++
kernel/power/main.c | 7 +-
kernel/power/power.h | 62 +++++
kernel/power/snapshot.c | 323 ++++++++++++++++++++++-
kernel/power/swap.c | 4 +
kernel/power/user.c | 14 +-
33 files changed, 1946 insertions(+), 22 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


2013-09-15 00:57:09

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).

This patch is temporary set emLen to pks->k, and temporary set EM to
pks->S for debugging. We will replace the above values to real signature
after implement RSASP1.

The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
accord PKCS#1 spec but not follow kernel naming convention, it useful when look
at them with spec.

Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf

V2:
- Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
- Add comment to EMSA_PKCS1-v1_5-ENCODE function.

Cc: Pavel Machek <[email protected]>
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
include/crypto/public_key.h | 2 +
2 files changed, 164 insertions(+), 1 deletions(-)

diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 47f3be4..352ba45 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/slab.h>
+#include <crypto/hash.h>
#include "public_key.h"
#include "private_key.h"

@@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
}

/*
+ * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
+ * @M: message to be signed, an octet string
+ * @emLen: intended length in octets of the encoded message
+ * @hash_algo: hash function (option)
+ * @hash: true means hash M, otherwise M is already a digest
+ * @EM: encoded message, an octet string of length emLen
+ *
+ * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
+ * in RSA PKCS#1 spec. It used by the signautre generation operation of
+ * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
+ *
+ * The variables used in this function accord PKCS#1 spec but not follow kernel
+ * naming convention, it useful when look at them with spec.
+ */
+static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
+ enum pkey_hash_algo hash_algo, const bool hash,
+ u8 **EM, struct public_key_signature *pks)
+{
+ u8 *digest;
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ size_t digest_size, desc_size;
+ size_t tLen;
+ u8 *T, *PS, *EM_tmp;
+ int i, ret;
+
+ pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
+
+ if (!RSA_ASN1_templates[hash_algo].data)
+ ret = -ENOTSUPP;
+ else
+ pks->pkey_hash_algo = hash_algo;
+
+ /* 1) Apply the hash function to the message M to produce a hash value H */
+ tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
+ if (IS_ERR(tfm))
+ return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
+
+ 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;
+ pks->digest = digest;
+ pks->digest_size = digest_size;
+
+ if (hash) {
+ 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;
+ ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
+ if (ret < 0)
+ goto error_shash;
+ } else {
+ memcpy(pks->digest, M, pks->digest_size);
+ pks->digest_size = digest_size;
+ }
+ crypto_free_shash(tfm);
+
+ /* 2) Encode the algorithm ID for the hash function and the hash value into
+ * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
+ * the DigestInfo value and let tLen be the length in octets of T.
+ */
+ tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
+ T = kmalloc(tLen, GFP_KERNEL);
+ if (!T)
+ goto error_T;
+
+ memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
+ memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
+
+ /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
+ if (emLen < tLen + 11) {
+ ret = -EINVAL;
+ goto error_emLen;
+ }
+
+ /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
+ PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
+ if (!PS)
+ goto error_P;
+
+ for (i = 0; i < (emLen - tLen - 3); i++)
+ PS[i] = 0xff;
+
+ /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
+ * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
+ */
+ EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
+ if (!EM_tmp)
+ goto error_EM;
+
+ EM_tmp[0] = 0x00;
+ EM_tmp[1] = 0x01;
+ memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
+ EM_tmp[2 + emLen - tLen - 3] = 0x00;
+ memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
+
+ *EM = EM_tmp;
+
+ kfree(PS);
+ kfree(T);
+
+ return 0;
+
+error_EM:
+ kfree(PS);
+error_P:
+error_emLen:
+ kfree(T);
+error_T:
+error_shash:
+ kfree(digest);
+error_digest:
+ crypto_free_shash(tfm);
+ return ret;
+}
+
+/*
* Perform the RSA signature verification.
* @H: Value of hash of data and metadata
* @EM: The computed signature value
@@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
const struct private_key *key, u8 *M,
enum pkey_hash_algo hash_algo, const bool hash)
{
+ struct public_key_signature *pks;
+ u8 *EM = NULL;
+ size_t emLen;
+ int ret;
+
pr_info("RSA_generate_signature start\n");

- return 0;
+ ret = -ENOMEM;
+ pks = kzalloc(sizeof(*pks), GFP_KERNEL);
+ if (!pks)
+ goto error_no_pks;
+
+ /* 1): EMSA-PKCS1-v1_5 encoding: */
+ /* Use the private key modulus size to be EM length */
+ emLen = mpi_get_nbits(key->rsa.n);
+ emLen = (emLen + 7) / 8;
+
+ ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
+ if (ret < 0)
+ goto error_v1_5_encode;
+
+ /* TODO 2): m = OS2IP (EM) */
+
+ /* TODO 3): s = RSASP1 (K, m) */
+
+ /* TODO 4): S = I2OSP (s, k) */
+
+ /* TODO: signature S to a u8* S or set to sig->rsa.s? */
+ pks->S = EM; /* TODO: temporary set S to EM */
+
+ return pks;
+
+error_v1_5_encode:
+ kfree(pks);
+error_no_pks:
+ pr_info("<==%s() = %d\n", __func__, ret);
+ return ERR_PTR(ret);
}

const struct public_key_algorithm RSA_public_key_algorithm = {
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index d44b29f..1cdf457 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
struct public_key_signature {
u8 *digest;
u8 digest_size; /* Number of bytes in digest */
+ u8 *S; /* signature S of length k octets */
+ size_t k; /* length k of signature S */
u8 nr_mpi; /* Occupancy of mpi[] */
enum pkey_hash_algo pkey_hash_algo : 8;
union {
--
1.6.0.2

2013-09-15 00:57:17

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 04/15] asymmetric keys: implement OS2IP in rsa

Implement Octet String to Integer conversion [RFC3447 sec 4.2] in rsa.c. It's
the second step of signature generation operation.

This patch is temporary set non-RSASP1 message to pks->S for debugging.

The naming of RSA_OS2IP and the variables used in this function accord PKCS#1
spec but not follow kernel naming convention, it useful when look at them with
spec.

Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf

Cc: Pavel Machek <[email protected]>
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 29 ++++++++++++++++++++++++-----
1 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index aac8b77..a092aac 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -168,6 +168,20 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
}

/*
+ * Octet String to Integer conversion [RFC3447 sec 4.2]
+ */
+static int RSA_OS2IP(u8 *X, size_t XLen, MPI *_x)
+{
+ MPI x;
+
+ x = mpi_alloc((XLen + BYTES_PER_MPI_LIMB - 1) / BYTES_PER_MPI_LIMB);
+ mpi_set_buffer(x, X, XLen, 0);
+
+ *_x = x;
+ return 0;
+}
+
+/*
* EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
* @M: message to be signed, an octet string
* @emLen: intended length in octets of the encoded message
@@ -419,6 +433,9 @@ static struct public_key_signature *RSA_generate_signature(
{
struct public_key_signature *pks;
u8 *EM = NULL;
+ MPI m = NULL;
+ MPI s = NULL;
+ unsigned X_size;
size_t emLen;
int ret;

@@ -438,14 +455,16 @@ static struct public_key_signature *RSA_generate_signature(
if (ret < 0)
goto error_v1_5_encode;

- /* TODO 2): m = OS2IP (EM) */
+ /* 2): m = OS2IP (EM) */
+ ret = RSA_OS2IP(EM, emLen, &m);
+ if (ret < 0)
+ goto error_v1_5_encode;

/* TODO 3): s = RSASP1 (K, m) */
+ s = m;

- /* TODO 4): S = I2OSP (s, k) */
-
- /* TODO: signature S to a u8* S or set to sig->rsa.s? */
- pks->S = EM; /* TODO: temporary set S to EM */
+ /* 4): S = I2OSP (s, k) */
+ _RSA_I2OSP(s, &X_size, &pks->S);

return pks;

--
1.6.0.2

2013-09-15 00:57:28

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 05/15] asymmetric keys: implement RSASP1

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).

The naming of RSASP1 and the variables used in this function accord PKCS#1
spec but not follow kernel naming convention, it useful when look at them with
spec.

Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf

Cc: Pavel Machek <[email protected]>
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 a092aac..0ede317 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;
@@ -460,8 +496,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.0.2

2013-09-15 00:57:40

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 06/15] asymmetric keys: support parsing PKCS #8 private key information

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

v2:
- Removed bitfield declare of privkey_algo in struct pkcs8_info because it does
not help on reduce memory space.
- Replace privkey_algo by pkey_algo in struct pkcs8_info to simplify naming.

Cc: Pavel Machek <[email protected]>
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..7475732
--- /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->pkey_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..9abaf01
--- /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 pkey_algo; /* 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..b52d7aa
--- /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->pkey_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 80c19cd..829788d 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.0.2

2013-09-15 00:57:47

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 07/15] asymmetric keys: explicitly add the leading zero byte to encoded message

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.

V2:
- Check the memory allocate result of EM to avoid use it when allocate fail.

Cc: Pavel Machek <[email protected]>
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/rsa.c | 20 +++++++++++++++-----
1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 0ede317..9763df7 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -408,6 +408,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_tmp = NULL;
MPI m = NULL;
size_t k;

@@ -442,19 +443,28 @@ 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_tmp);
if (ret < 0)
goto error;

- ret = RSA_verify(H, EM - 1, k, sig->digest_size,
+ EM = kmalloc(k, GFP_KERNEL);
+ if (!EM) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ memset(EM, 0, 1);
+ memcpy(EM + 1, EM_tmp, k-1);
+
+ 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);

-error:
kfree(EM);
+error:
+ kfree(EM_tmp);
mpi_free(m);
kleave(" = %d", ret);
return ret;
--
1.6.0.2

2013-09-15 00:57:59

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 08/15] Hibernate: introduced RSA key-pair to verify signature of snapshot

Introduced a hibernate_key.c file to query the key pair from EFI variables
and maintain key pair for check signature of S4 snapshot image. We
loaded the private key when snapshot image stored success.

This patch introduced 2 EFI variables for store the key to sign S4 image and
verify signature when S4 wake up. The names and GUID are:
S4SignKey-fe141863-c070-478e-b8a3-878a5dc9ef21 [BOOT SERVICES][NV]
S4WakeKey-fe141863-c070-478e-b8a3-878a5dc9ef21 [RUNTIME SERVICES][V]

S4SignKey is used by EFI bootloader to pass the RSA private key that packaged
by PKCS#8 format, kernel will read and parser it when system boot and reload
it when S4 resume. EFI bootloader need gnerate a new private key when every
time system boot.

S4WakeKey is used to parse the RSA public key that packaged by X.509
certificate, kernel will read and parse it for check the signature of
S4 snapshot image when S4 resume.

The follow-up patch will remove S4SignKey and S4WakeKey after load them
to kernel for avoid anyone can access it through efivarfs.

V4:
- Removed duplicate cast of params->hdr.setup_data
- Declare dummy function of setup_s4_keys() and efi_reserve_s4_skey_data() to
avoid ifdef.
- Use forward_info structure to maintain the empty of forward information and
new sign key from boot kernel to resume target kernel.
- Use efivar API to access S4WakeKey variable.

V3:
- Load S4 sign key before ExitBootServices.
Load private key before ExitBootServices() then bootloader doesn't need
generate key-pair for each booting:
+ Add setup_s4_keys() to eboot.c to load S4 sign key before ExitBootServices.
+ Reserve the memory block of sign key data blob in efi.c
- In Makefile, moved hibernate_keys.o before hibernate.o for load S4 sign
key before check hibernate image. It makes sure the new sign key will be
transfer to resume target kernel.
- Set "depends on EFI_STUB" in Kconfig

V2:
Add CONFIG_SNAPSHOT_VERIFICATION for build of hibernate_keys.c depend on
Kconfig.

Cc: Matthew Garrett <[email protected]>
Cc: Takashi Iwai <[email protected]>
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
arch/x86/boot/compressed/eboot.c | 92 +++++++++++
arch/x86/include/asm/efi.h | 9 +
arch/x86/include/uapi/asm/bootparam.h | 1 +
arch/x86/platform/efi/efi.c | 68 ++++++++
include/linux/efi.h | 25 +++
kernel/power/Kconfig | 16 ++-
kernel/power/Makefile | 1 +
kernel/power/hibernate.c | 2 +
kernel/power/hibernate_keys.c | 292 +++++++++++++++++++++++++++++++++
kernel/power/power.h | 30 ++++
10 files changed, 534 insertions(+), 2 deletions(-)
create mode 100644 kernel/power/hibernate_keys.c

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 53bfe4f..983dc7d 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -369,6 +369,96 @@ free_handle:
return status;
}

+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+static efi_status_t setup_s4_keys(struct boot_params *params)
+{
+ struct setup_data *data;
+ unsigned long datasize;
+ u32 attr;
+ struct efi_s4_key *s4key;
+ efi_status_t status;
+
+ data = (struct setup_data *)params->hdr.setup_data;
+
+ while (data && data->next)
+ data = (struct setup_data *)(unsigned long)data->next;
+
+ status = efi_call_phys3(sys_table->boottime->allocate_pool,
+ EFI_LOADER_DATA, sizeof(*s4key), &s4key);
+ if (status != EFI_SUCCESS) {
+ efi_printk("Failed to alloc memory for efi_s4_key\n");
+ goto error_setup;
+ }
+
+ s4key->data.type = SETUP_S4_KEY;
+ s4key->data.len = sizeof(struct efi_s4_key) -
+ sizeof(struct setup_data);
+ s4key->data.next = 0;
+ s4key->skey_dsize = 0;
+ s4key->err_status = 0;
+
+ if (data)
+ data->next = (unsigned long)s4key;
+ else
+ params->hdr.setup_data = (unsigned long)s4key;
+
+ /* obtain the size of key data */
+ datasize = 0;
+ status = efi_call_phys5(sys_table->runtime->get_variable,
+ EFI_S4_SIGN_KEY_NAME, &EFI_HIBERNATE_GUID,
+ NULL, &datasize, NULL);
+ if (status != EFI_BUFFER_TOO_SMALL) {
+ efi_printk("Couldn't get S4 key data size\n");
+ goto error_size;
+ }
+ if (datasize > SKEY_DBUF_MAX_SIZE) {
+ efi_printk("The size of S4 sign key is too large\n");
+ status = EFI_UNSUPPORTED;
+ goto error_size;
+ }
+
+ s4key->skey_dsize = datasize;
+ status = efi_call_phys3(sys_table->boottime->allocate_pool,
+ EFI_LOADER_DATA, s4key->skey_dsize,
+ &s4key->skey_data_addr);
+ if (status != EFI_SUCCESS) {
+ efi_printk("Failed to alloc page for S4 key data\n");
+ goto error_s4key;
+ }
+
+ attr = 0;
+ memset((void *)s4key->skey_data_addr, 0, s4key->skey_dsize);
+ status = efi_call_phys5(sys_table->runtime->get_variable,
+ EFI_S4_SIGN_KEY_NAME, &EFI_HIBERNATE_GUID, &attr,
+ &(s4key->skey_dsize), s4key->skey_data_addr);
+ if (status) {
+ efi_printk("Couldn't get S4 key data\n");
+ goto error_gets4key;
+ }
+ if (attr & EFI_VARIABLE_RUNTIME_ACCESS) {
+ efi_printk("S4 sign key can not be a runtime variable\n");
+ memset((void *)s4key->skey_data_addr, 0, s4key->skey_dsize);
+ status = EFI_UNSUPPORTED;
+ goto error_gets4key;
+ }
+
+ return 0;
+
+error_gets4key:
+ efi_call_phys1(sys_table->boottime->free_pool, s4key->skey_data_addr);
+error_s4key:
+error_size:
+ s4key->err_status = status;
+error_setup:
+ return status;
+}
+#else
+static inline efi_status_t setup_s4_keys(struct boot_params *params)
+{
+ return 0;
+}
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+
/*
* See if we have Graphics Output Protocol
*/
@@ -1209,6 +1299,8 @@ struct boot_params *efi_main(void *handle, efi_system_table_t *_table,

setup_efi_pci(boot_params);

+ setup_s4_keys(boot_params);
+
status = efi_call_phys3(sys_table->boottime->allocate_pool,
EFI_LOADER_DATA, sizeof(*gdt),
(void **)&gdt);
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 0062a01..56ececa 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -102,6 +102,15 @@ extern void efi_call_phys_epilog(void);
extern void efi_unmap_memmap(void);
extern void efi_memory_uc(u64 addr, unsigned long size);

+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+struct efi_s4_key {
+ struct setup_data data;
+ unsigned long err_status;
+ unsigned long skey_dsize;
+ void *skey_data_addr;
+};
+#endif
+
#ifdef CONFIG_EFI

static inline bool efi_is_native(void)
diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
index 85d7685..79398ff 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -6,6 +6,7 @@
#define SETUP_E820_EXT 1
#define SETUP_DTB 2
#define SETUP_PCI 3
+#define SETUP_S4_KEY 4

/* ram_size flags */
#define RAMDISK_IMAGE_START_MASK 0x07FF
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 90f6ed1..dd807e3 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -704,6 +704,71 @@ static int __init efi_memmap_init(void)
return 0;
}

+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+static unsigned long skey_dsize;
+static u64 skey_data_addr;
+static unsigned long skey_err_status;
+
+bool efi_s4_key_available(void)
+{
+ return skey_dsize && skey_data_addr && !skey_err_status;
+}
+
+unsigned long __init efi_copy_skey_data(void *page_addr)
+{
+ void *key_addr;
+
+ if (efi_s4_key_available()) {
+ key_addr = early_ioremap(skey_data_addr, skey_dsize);
+ memcpy(page_addr, key_addr, skey_dsize);
+ early_iounmap(key_addr, skey_dsize);
+ }
+
+ return skey_dsize;
+}
+
+void __init efi_erase_s4_skey_data(void)
+{
+ void *key_addr;
+
+ key_addr = early_ioremap(skey_data_addr, skey_dsize);
+ memset(key_addr, 0, skey_dsize);
+ early_iounmap(key_addr, skey_dsize);
+ memblock_free(skey_data_addr, skey_dsize);
+ skey_data_addr = 0;
+ skey_dsize = 0;
+}
+
+static void __init efi_reserve_s4_skey_data(void)
+{
+ u64 pa_data;
+ struct setup_data *data;
+ struct efi_s4_key *s4key;
+
+ skey_err_status = 0;
+ pa_data = boot_params.hdr.setup_data;
+ while (pa_data) {
+ data = early_ioremap(pa_data, sizeof(*s4key));
+ if (data->type == SETUP_S4_KEY) {
+ s4key = (struct efi_s4_key *)data;
+ if (!s4key->err_status) {
+ skey_dsize = s4key->skey_dsize;
+ skey_data_addr = (u64) s4key->skey_data_addr;
+ memblock_reserve(skey_data_addr, skey_dsize);
+ } else {
+ skey_err_status = s4key->err_status;
+ pr_err("Get S4 sign key from EFI fail: 0x%lx\n",
+ skey_err_status);
+ }
+ }
+ pa_data = data->next;
+ early_iounmap(data, sizeof(*s4key));
+ }
+}
+#else
+static inline void efi_reserve_s4_skey_data(void) {}
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+
void __init efi_init(void)
{
efi_char16_t *c16;
@@ -729,6 +794,9 @@ void __init efi_init(void)

set_bit(EFI_SYSTEM_TABLES, &x86_efi_facility);

+ /* keep s4 key from setup_data */
+ efi_reserve_s4_skey_data();
+
/*
* Show what we know for posterity
*/
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 5f8f176..57e78fc 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -389,6 +389,18 @@ typedef efi_status_t efi_query_variable_store_t(u32 attributes, unsigned long si
#define EFI_FILE_SYSTEM_GUID \
EFI_GUID( 0x964e5b22, 0x6459, 0x11d2, 0x8e, 0x39, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b )

+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+#define EFI_HIBERNATE_GUID \
+ EFI_GUID(0xfe141863, 0xc070, 0x478e, 0xb8, 0xa3, 0x87, 0x8a, 0x5d, 0xc9, 0xef, 0x21)
+/*
+ * The UEFI variable names of the key-pair to verify S4 snapshot image:
+ * S4SignKey-EFI_HIBERNATE_GUID: The private key is used to sign snapshot
+ * S4WakeKey-EFI_HIBERNATE_GUID: The public key is used to verify snapshot
+ */
+#define EFI_S4_SIGN_KEY_NAME ((efi_char16_t [10]) { 'S', '4', 'S', 'i', 'g', 'n', 'K', 'e', 'y', 0 })
+#define EFI_S4_WAKE_KEY_NAME ((efi_char16_t [10]) { 'S', '4', 'W', 'a', 'k', 'e', 'K', 'e', 'y', 0 })
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+
typedef struct {
efi_guid_t guid;
u64 table;
@@ -577,6 +589,19 @@ extern void efi_enter_virtual_mode (void); /* switch EFI to virtual mode, if pos
extern void efi_late_init(void);
extern void efi_free_boot_services(void);
extern efi_status_t efi_query_variable_store(u32 attributes, unsigned long size);
+
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+struct forward_info_head {
+ bool sig_enforce;
+ int sig_check_ret;
+ unsigned long skey_dsize;
+} __attribute__((packed));
+#define SKEY_DBUF_MAX_SIZE (PAGE_SIZE - sizeof(struct forward_info_head))
+extern bool efi_s4_key_available(void);
+extern unsigned long efi_copy_skey_data(void *page_addr);
+extern void efi_erase_s4_skey_data(void);
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+
#else
static inline void efi_late_init(void) {}
static inline void efi_free_boot_services(void) {}
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
index d444c4e..b592d88 100644
--- a/kernel/power/Kconfig
+++ b/kernel/power/Kconfig
@@ -66,8 +66,17 @@ config HIBERNATION

For more information take a look at <file:Documentation/power/swsusp.txt>.

-config ARCH_SAVE_PAGE_KEYS
- bool
+config SNAPSHOT_VERIFICATION
+ bool "Hibernate snapshot verification"
+ depends on HIBERNATION
+ depends on EFI_STUB
+ depends on X86
+ select PKCS8_PRIVATE_KEY_INFO_PARSER
+ help
+ This option provides support for generate anad verify the signautre by
+ RSA key-pair against hibernate snapshot image. Current mechanism
+ dependent on UEFI environment. EFI bootloader should generate the
+ key-pair.

config PM_STD_PARTITION
string "Default resume partition"
@@ -91,6 +100,9 @@ config PM_STD_PARTITION
suspended image to. It will simply pick the first available swap
device.

+config ARCH_SAVE_PAGE_KEYS
+ bool
+
config PM_SLEEP
def_bool y
depends on SUSPEND || HIBERNATE_CALLBACKS
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 29472bf..46b6422 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_VT_CONSOLE_SLEEP) += console.o
obj-$(CONFIG_FREEZER) += process.o
obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o
+obj-$(CONFIG_SNAPSHOT_VERIFICATION) += hibernate_keys.o
obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o \
block_io.o
obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b26f5f1..90a25c7 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -28,6 +28,7 @@
#include <linux/syscore_ops.h>
#include <linux/ctype.h>
#include <linux/genhd.h>
+#include <linux/key.h>

#include "power.h"

@@ -680,6 +681,7 @@ int hibernate(void)
pm_restore_gfp_mask();
} else {
pr_debug("PM: Image restored successfully.\n");
+ restore_sig_forward_info();
}

Thaw:
diff --git a/kernel/power/hibernate_keys.c b/kernel/power/hibernate_keys.c
new file mode 100644
index 0000000..0bce9ab
--- /dev/null
+++ b/kernel/power/hibernate_keys.c
@@ -0,0 +1,292 @@
+#include <linux/sched.h>
+#include <linux/efi.h>
+#include <linux/mpi.h>
+#include <linux/asn1.h>
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>
+
+#include "power.h"
+
+static unsigned char const_seq = (ASN1_SEQ | (ASN1_CONS << 5));
+
+struct forward_info {
+ struct forward_info_head head;
+ unsigned char skey_data_buf[SKEY_DBUF_MAX_SIZE];
+};
+
+static void *skey_data;
+static void *forward_info_buf;
+static unsigned long skey_dsize;
+
+bool swsusp_page_is_sign_key(struct page *page)
+{
+ unsigned long skey_data_pfn;
+ bool ret;
+
+ if (!skey_data || IS_ERR(skey_data))
+ return false;
+
+ skey_data_pfn = page_to_pfn(virt_to_page(skey_data));
+ ret = (page_to_pfn(page) == skey_data_pfn) ? true : false;
+ if (ret)
+ pr_info("PM: Avoid snapshot the page of S4 sign key.\n");
+
+ return ret;
+}
+
+unsigned long get_sig_forward_info_pfn(void)
+{
+ if (!forward_info_buf)
+ return 0;
+
+ return page_to_pfn(virt_to_page(forward_info_buf));
+}
+
+void fill_sig_forward_info(void *page, int sig_check_ret_in)
+{
+ struct forward_info *info;
+
+ if (!page)
+ return;
+
+ memset(page, 0, PAGE_SIZE);
+ info = (struct forward_info *)page;
+
+ info->head.sig_check_ret = sig_check_ret_in;
+ if (skey_data && !IS_ERR(skey_data) &&
+ skey_dsize <= SKEY_DBUF_MAX_SIZE) {
+ info->head.skey_dsize = skey_dsize;
+ memcpy(info->skey_data_buf, skey_data, skey_dsize);
+ } else
+ pr_info("PM: Fill S4 sign key fail, size: %ld\n", skey_dsize);
+
+ pr_info("PM: Filled sign information to forward buffer\n");
+}
+
+void restore_sig_forward_info(void)
+{
+ struct forward_info *info;
+ int sig_check_ret;
+
+ if (!forward_info_buf) {
+ pr_err("PM: Restore S4 sign key fail\n");
+ return;
+ }
+ info = (struct forward_info *)forward_info_buf;
+
+ sig_check_ret = info->head.sig_check_ret;
+ if (sig_check_ret)
+ pr_info("PM: Signature check fail: %d\n", sig_check_ret);
+
+ if (info->head.skey_dsize <= SKEY_DBUF_MAX_SIZE &&
+ info->skey_data_buf[0] == const_seq) {
+
+ /* restore sign key size and data from buffer */
+ skey_dsize = info->head.skey_dsize;
+ memset(skey_data, 0, PAGE_SIZE);
+ memcpy(skey_data, info->skey_data_buf, skey_dsize);
+ }
+
+ /* reset skey page buffer */
+ memset(forward_info_buf, 0, PAGE_SIZE);
+}
+
+bool skey_data_available(void)
+{
+ bool ret = false;
+
+ /* Sign key is PKCS#8 format that must be a Constructed SEQUENCE */
+ ret = skey_data && !IS_ERR(skey_data) &&
+ (skey_dsize != 0) &&
+ ((unsigned char *)skey_data)[0] == const_seq;
+
+ return ret;
+}
+
+struct key *get_sign_key(void)
+{
+ const struct cred *cred = current_cred();
+ struct key *skey;
+ int err;
+
+ if (!skey_data || IS_ERR(skey_data))
+ return ERR_PTR(-EBADMSG);
+
+ skey = key_alloc(&key_type_asymmetric, "s4_sign_key",
+ GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ cred, 0, KEY_ALLOC_NOT_IN_QUOTA);
+ if (IS_ERR(skey)) {
+ pr_err("PM: Allocate s4 sign key error: %ld\n", PTR_ERR(skey));
+ goto error_keyalloc;
+ }
+
+ err = key_instantiate_and_link(skey, skey_data, skey_dsize, NULL, NULL);
+ if (err < 0) {
+ pr_err("PM: S4 sign key instantiate error: %d\n", err);
+ if (skey)
+ key_put(skey);
+ skey = ERR_PTR(err);
+ goto error_keyinit;
+ }
+
+ return skey;
+
+error_keyinit:
+error_keyalloc:
+ return skey;
+}
+
+void erase_skey_data(void)
+{
+ if (!skey_data || IS_ERR(skey_data))
+ return;
+
+ memset(skey_data, 0, PAGE_SIZE);
+}
+
+void destroy_sign_key(struct key *skey)
+{
+ erase_skey_data();
+ if (skey)
+ key_put(skey);
+}
+
+static void *load_wake_key_data(unsigned long *datasize)
+{
+ struct efivar_entry *entry;
+ u32 attr;
+ void *wkey_data;
+ int ret;
+
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return ERR_PTR(-ENOMEM);
+
+ memcpy(entry->var.VariableName, EFI_S4_WAKE_KEY_NAME, sizeof(EFI_S4_WAKE_KEY_NAME));
+ memcpy(&(entry->var.VendorGuid), &EFI_HIBERNATE_GUID, sizeof(efi_guid_t));
+
+ /* obtain the size */
+ *datasize = 0;
+ ret = efivar_entry_size(entry, datasize);
+ if (ret)
+ goto error_size;
+
+ wkey_data = kzalloc(*datasize, GFP_KERNEL);
+ if (!wkey_data) {
+ ret = -ENOMEM;
+ goto error_size;
+ }
+
+ ret = efivar_entry_get(entry, &attr, datasize, wkey_data);
+ if (ret) {
+ pr_err("PM: Get wake key data error: %d\n", ret);
+ goto error_get;
+ }
+ /* check attributes */
+ if (attr & EFI_VARIABLE_NON_VOLATILE) {
+ pr_err("PM: Wake key has wrong attributes: 0x%x\n", attr);
+ goto error_get;
+ }
+
+ kfree(entry);
+
+ return wkey_data;
+
+error_get:
+ memset(wkey_data, 0, *datasize);
+ kfree(wkey_data);
+ *datasize = 0;
+error_size:
+ kfree(entry);
+
+ return ERR_PTR(ret);
+}
+
+int wkey_data_available(void)
+{
+ static int ret = 1;
+ unsigned long datasize;
+ void *wkey_data;
+
+ if (ret > 0) {
+ wkey_data = load_wake_key_data(&datasize);
+ if (wkey_data && IS_ERR(wkey_data)) {
+ ret = PTR_ERR(wkey_data);
+ goto error;
+ } else {
+ if (wkey_data) {
+ memset(wkey_data, 0, datasize);
+ kfree(wkey_data);
+ }
+ ret = 0;
+ }
+ }
+
+error:
+ return ret;
+}
+
+struct key *get_wake_key(void)
+{
+ const struct cred *cred = current_cred();
+ void *wkey_data;
+ unsigned long datasize = 0;
+ struct key *wkey;
+ int err;
+
+ wkey_data = load_wake_key_data(&datasize);
+ if (IS_ERR(wkey_data)) {
+ wkey = (struct key *)wkey_data;
+ goto error_data;
+ }
+
+ wkey = key_alloc(&key_type_asymmetric, "s4_wake_key",
+ GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ cred, 0, KEY_ALLOC_NOT_IN_QUOTA);
+ if (IS_ERR(wkey)) {
+ pr_err("PM: Allocate s4 wake key error: %ld\n", PTR_ERR(wkey));
+ goto error_keyalloc;
+ }
+ err = key_instantiate_and_link(wkey, wkey_data, datasize, NULL, NULL);
+ if (err < 0) {
+ pr_err("PM: S4 wake key instantiate error: %d\n", err);
+ if (wkey)
+ key_put(wkey);
+ wkey = ERR_PTR(err);
+ }
+
+error_keyalloc:
+ if (wkey_data && !IS_ERR(wkey_data))
+ kfree(wkey_data);
+error_data:
+ return wkey;
+}
+
+size_t get_key_length(const struct key *key)
+{
+ const struct public_key *pk = key->payload.data;
+ size_t len;
+
+ /* TODO: better check the RSA type */
+
+ len = mpi_get_nbits(pk->rsa.n);
+ len = (len + 7) / 8;
+
+ return len;
+}
+
+static int __init init_sign_key_data(void)
+{
+ skey_data = (void *)get_zeroed_page(GFP_KERNEL);
+ forward_info_buf = (void *)get_zeroed_page(GFP_KERNEL);
+
+ if (skey_data && efi_s4_key_available()) {
+ skey_dsize = efi_copy_skey_data(skey_data);
+ efi_erase_s4_skey_data();
+ pr_info("PM: Load s4 sign key from EFI\n");
+ }
+
+ return 0;
+}
+
+late_initcall(init_sign_key_data);
diff --git a/kernel/power/power.h b/kernel/power/power.h
index 7d4b7ff..661f124 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -160,6 +160,36 @@ extern void swsusp_close(fmode_t);
extern int swsusp_unmark(void);
#endif

+/* kernel/power/hibernate_key.c */
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+extern bool skey_data_available(void);
+extern struct key *get_sign_key(void);
+extern void erase_skey_data(void);
+extern void destroy_sign_key(struct key *key);
+extern int wkey_data_available(void);
+extern struct key *get_wake_key(void);
+extern size_t get_key_length(const struct key *key);
+
+extern void restore_sig_forward_info(void);
+extern bool swsusp_page_is_sign_key(struct page *page);
+extern unsigned long get_sig_forward_info_pfn(void);
+extern void fill_sig_forward_info(void *page_addr, int sig_check_ret);
+#else
+static inline bool skey_data_available(void)
+{
+ return false;
+}
+static inline void restore_sig_forward_info(void) {}
+static inline bool swsusp_page_is_sign_key(struct page *page)
+{
+ return false;
+}
+static inline unsigned long get_sig_forward_info_pfn(void)
+{
+ return 0;
+}
+#endif /* !CONFIG_SNAPSHOT_VERIFICATION */
+
/* kernel/power/block_io.c */
extern struct block_device *hib_resume_bdev;

--
1.6.0.2

2013-09-15 00:58:10

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 09/15] Hibernate: generate and verify signature of snapshot

This patch add the code for generate/verify signature of snapshot, it
put the signature to snapshot header. This approach can support both
on userspace hibernate and in-kernel hibernate.

v3:
- Change the naming of SIG_LENG to SIG_LEN
- Extracts the code of signature generation code from copy_data_pages() to
swsusp_generate_signature(), call it in swsusp_save() after copy_data_pages()
finished.
- Change the naming of h_buf to handle_buffers.
- Removed duplicate code in snapshot_verify_signature() and
snapshot_image_verify().
- Merged [PATCH 14/18]
Hibernate: applied SNAPSHOT_VERIFICATION config to switch signature check

v2:
- Due to loaded S4 sign key before ExitBootServices, we need forward key from
boot kernel to resume target kernel. So this patch add a empty page in
snapshot image, then we keep the pfn of this empty page in snapshot header.
When system resume from hibernate, we fill new sign key to this empty page
space after snapshot image checked pass. This mechanism let boot kernel can
forward new sign key to resume target kernel but don't need write new private
key to any other storage, e.g. swap.

Cc: Matthew Garrett <[email protected]>
Cc: Pavel Machek <[email protected]>
Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/power.h | 13 ++
kernel/power/snapshot.c | 288 ++++++++++++++++++++++++++++++++++++++++++++++-
kernel/power/swap.c | 4 +
kernel/power/user.c | 5 +
4 files changed, 307 insertions(+), 3 deletions(-)

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 661f124..d2da75b 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -3,6 +3,9 @@
#include <linux/utsname.h>
#include <linux/freezer.h>

+/* The maximum length of snapshot signature */
+#define SIG_LEN 512
+
struct swsusp_info {
struct new_utsname uts;
u32 version_code;
@@ -11,6 +14,8 @@ struct swsusp_info {
unsigned long image_pages;
unsigned long pages;
unsigned long size;
+ unsigned long sig_forward_info_pfn;
+ u8 signature[SIG_LEN];
} __attribute__((aligned(PAGE_SIZE)));

#ifdef CONFIG_HIBERNATION
@@ -134,6 +139,14 @@ extern int snapshot_read_next(struct snapshot_handle *handle);
extern int snapshot_write_next(struct snapshot_handle *handle);
extern void snapshot_write_finalize(struct snapshot_handle *handle);
extern int snapshot_image_loaded(struct snapshot_handle *handle);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+extern int snapshot_image_verify(void);
+#else
+static inline int snapshot_image_verify(void)
+{
+ return 0;
+}
+#endif

/* If unset, the snapshot device cannot be open. */
extern atomic_t snapshot_device_available;
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 349587b..edab31f 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -27,6 +27,9 @@
#include <linux/highmem.h>
#include <linux/list.h>
#include <linux/slab.h>
+#include <crypto/hash.h>
+#include <crypto/public_key.h>
+#include <keys/asymmetric-type.h>

#include <asm/uaccess.h>
#include <asm/mmu_context.h>
@@ -1031,6 +1034,126 @@ static inline void copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
}
#endif /* CONFIG_HIGHMEM */

+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+#define SNAPSHOT_HASH "sha256"
+#endif
+
+/*
+ * Signature of snapshot for check.
+ */
+static u8 signature[SIG_LEN];
+
+/*
+ * Keep the pfn of forward information buffer from resume target. We write
+ * the next time sign key to this page in snapshot image before restore.
+ */
+unsigned long sig_forward_info_pfn;
+
+void **handle_buffers;
+void *sig_forward_info_buf;
+
+static int
+swsusp_generate_signature(struct memory_bitmap *copy_bm, unsigned int nr_pages)
+{
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+ unsigned long pfn;
+ struct page *d_page;
+ void *hash_buffer = NULL;
+ struct crypto_shash *tfm;
+ struct shash_desc *desc;
+ u8 *digest;
+ size_t digest_size, desc_size;
+ struct key *s4_sign_key;
+ struct public_key_signature *pks;
+ int ret, i;
+
+ ret = -ENOMEM;
+ tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0);
+ if (IS_ERR(tfm)) {
+ pr_err("IS_ERR(tfm): %ld", PTR_ERR(tfm));
+ return PTR_ERR(tfm);
+ }
+
+ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+ digest_size = crypto_shash_digestsize(tfm);
+ digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
+ if (!digest) {
+ pr_err("digest allocate fail");
+ ret = -ENOMEM;
+ 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;
+
+ memory_bm_position_reset(copy_bm);
+ for (i = 0; i < nr_pages; i++) {
+ pfn = memory_bm_next_pfn(copy_bm);
+
+ /* Generate digest */
+ d_page = pfn_to_page(pfn);
+ if (PageHighMem(d_page)) {
+ void *kaddr;
+ kaddr = kmap_atomic(d_page);
+ copy_page(buffer, kaddr);
+ kunmap_atomic(kaddr);
+ hash_buffer = buffer;
+ } else {
+ hash_buffer = page_address(d_page);
+ }
+ ret = crypto_shash_update(desc, hash_buffer, PAGE_SIZE);
+ if (ret)
+ goto error_shash;
+ }
+
+ crypto_shash_final(desc, digest);
+ if (ret)
+ goto error_shash;
+
+ /* Generate signature by private key */
+ s4_sign_key = get_sign_key();
+ if (!s4_sign_key || IS_ERR(s4_sign_key)) {
+ pr_err("Get S4 sign key fail: %ld\n", PTR_ERR(s4_sign_key));
+ ret = PTR_ERR(s4_sign_key);
+ goto error_key;
+ }
+
+ pks = generate_signature(s4_sign_key, digest, PKEY_HASH_SHA256, false);
+ if (IS_ERR(pks)) {
+ pr_err("Generate signature fail: %lx", PTR_ERR(pks));
+ ret = PTR_ERR(pks);
+ goto error_sign;
+ } else
+ memcpy(signature, pks->S, pks->k);
+
+ destroy_sign_key(s4_sign_key);
+
+ if (pks && pks->digest)
+ kfree(pks->digest);
+ if (pks && pks->rsa.s)
+ mpi_free(pks->rsa.s);
+ kfree(pks);
+ kfree(digest);
+ crypto_free_shash(tfm);
+
+ return 0;
+
+error_sign:
+ destroy_sign_key(s4_sign_key);
+error_key:
+error_shash:
+ kfree(digest);
+error_digest:
+ crypto_free_shash(tfm);
+ return ret;
+#else
+ return 0;
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+}
+
static void
copy_data_pages(struct memory_bitmap *copy_bm, struct memory_bitmap *orig_bm)
{
@@ -1580,6 +1703,7 @@ swsusp_alloc(struct memory_bitmap *orig_bm, struct memory_bitmap *copy_bm,
asmlinkage int swsusp_save(void)
{
unsigned int nr_pages, nr_highmem;
+ int ret;

printk(KERN_INFO "PM: Creating hibernation image:\n");

@@ -1614,6 +1738,14 @@ asmlinkage int swsusp_save(void)
nr_copy_pages = nr_pages;
nr_meta_pages = DIV_ROUND_UP(nr_pages * sizeof(long), PAGE_SIZE);

+ if (skey_data_available()) {
+ ret = swsusp_generate_signature(&copy_bm, nr_pages);
+ if (ret)
+ return ret;
+ } else
+ /* set zero signature if skey doesn't exist */
+ memset(signature, 0, SIG_LEN);
+
printk(KERN_INFO "PM: Hibernation image created (%d pages copied)\n",
nr_pages);

@@ -1657,6 +1789,8 @@ static int init_header(struct swsusp_info *info)
info->pages = snapshot_get_image_size();
info->size = info->pages;
info->size <<= PAGE_SHIFT;
+ info->sig_forward_info_pfn = get_sig_forward_info_pfn();
+ memcpy(info->signature, signature, SIG_LEN);
return init_header_complete(info);
}

@@ -1819,6 +1953,8 @@ load_header(struct swsusp_info *info)
if (!error) {
nr_copy_pages = info->image_pages;
nr_meta_pages = info->pages - info->image_pages - 1;
+ sig_forward_info_pfn = info->sig_forward_info_pfn;
+ memcpy(signature, info->signature, SIG_LEN);
}
return error;
}
@@ -2159,7 +2295,8 @@ prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm)
* set for its caller to write to.
*/

-static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca)
+static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca,
+ unsigned long *_pfn)
{
struct pbe *pbe;
struct page *page;
@@ -2168,6 +2305,9 @@ static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca)
if (pfn == BM_END_OF_MAP)
return ERR_PTR(-EFAULT);

+ if (_pfn)
+ *_pfn = pfn;
+
page = pfn_to_page(pfn);
if (PageHighMem(page))
return get_highmem_page_buffer(page, ca);
@@ -2214,6 +2354,7 @@ static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca)
int snapshot_write_next(struct snapshot_handle *handle)
{
static struct chain_allocator ca;
+ unsigned long pfn;
int error = 0;

/* Check if we have already loaded the entire image */
@@ -2236,6 +2377,15 @@ 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,
+ * handle_buffers will freed in snapshot_image_verify().
+ */
+ handle_buffers = kmalloc(sizeof(void *) * nr_copy_pages, GFP_KERNEL);
+ if (!handle_buffers)
+ pr_err("Allocate hash buffer fail!\n");
+#endif
+
error = memory_bm_create(&copy_bm, GFP_ATOMIC, PG_ANY);
if (error)
return error;
@@ -2258,20 +2408,31 @@ int snapshot_write_next(struct snapshot_handle *handle)
chain_init(&ca, GFP_ATOMIC, PG_SAFE);
memory_bm_position_reset(&orig_bm);
restore_pblist = NULL;
- handle->buffer = get_buffer(&orig_bm, &ca);
+ handle->buffer = get_buffer(&orig_bm, &ca, &pfn);
handle->sync_read = 0;
if (IS_ERR(handle->buffer))
return PTR_ERR(handle->buffer);
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+ if (handle_buffers)
+ *handle_buffers = handle->buffer;
+#endif
}
} else {
copy_last_highmem_page();
/* Restore page key for data page (s390 only). */
page_key_write(handle->buffer);
- handle->buffer = get_buffer(&orig_bm, &ca);
+ handle->buffer = get_buffer(&orig_bm, &ca, &pfn);
if (IS_ERR(handle->buffer))
return PTR_ERR(handle->buffer);
if (handle->buffer != buffer)
handle->sync_read = 0;
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+ if (handle_buffers)
+ *(handle_buffers + (handle->cur - nr_meta_pages - 1)) = handle->buffer;
+ /* Keep the buffer of sign key in snapshot */
+ if (pfn == sig_forward_info_pfn)
+ sig_forward_info_buf = handle->buffer;
+#endif
}
handle->cur++;
return PAGE_SIZE;
@@ -2304,6 +2465,127 @@ 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;
+ struct public_key_signature *pks;
+ int ret;
+ MPI mpi;
+
+ /* load public key */
+ s4_wake_key = get_wake_key();
+ if (!s4_wake_key || IS_ERR(s4_wake_key)) {
+ pr_err("PM: Get S4 wake key fail: %ld\n", PTR_ERR(s4_wake_key));
+ return PTR_ERR(s4_wake_key);
+ }
+
+ pks = kzalloc(digest_size + sizeof(*pks), GFP_KERNEL);
+ if (!pks) {
+ pr_err("PM: Allocate public key signature fail!");
+ return -ENOMEM;
+ }
+ pks->pkey_hash_algo = PKEY_HASH_SHA256;
+ pks->digest = digest;
+ pks->digest_size = digest_size;
+
+ mpi = mpi_read_raw_data(signature, get_key_length(s4_wake_key));
+ if (!mpi) {
+ pr_err("PM: mpi_read_raw_data fail!\n");
+ ret = -ENOMEM;
+ goto error_mpi;
+ }
+ pks->mpi[0] = mpi;
+ pks->nr_mpi = 1;
+
+ /* RSA signature check */
+ ret = verify_signature(s4_wake_key, pks);
+ if (ret)
+ pr_err("snapshot S4 signature verification fail: %d\n", ret);
+
+ if (pks->rsa.s)
+ mpi_free(pks->rsa.s);
+error_mpi:
+ kfree(pks);
+ return ret;
+}
+
+static void snapshot_fill_sig_forward_info(int sig_check_ret)
+{
+ if (!sig_forward_info_buf)
+ return;
+
+ /* Fill new s4 sign key to snapshot in memory */
+ fill_sig_forward_info(sig_forward_info_buf, sig_check_ret);
+ /* clean skey page data */
+ erase_skey_data();
+}
+
+int snapshot_image_verify(void)
+{
+ struct crypto_shash *tfm = NULL
+ struct shash_desc *desc;
+ u8 *digest = NULL;
+ size_t digest_size, desc_size;
+ int ret, i;
+
+ if (!handle_buffers)
+ return 0;
+
+ ret = wkey_data_available();
+ if (ret)
+ goto forward_ret;
+
+ tfm = crypto_alloc_shash(SNAPSHOT_HASH, 0, 0);
+ if (IS_ERR(tfm)) {
+ pr_err("IS_ERR(tfm): %ld", PTR_ERR(tfm));
+ return PTR_ERR(tfm);
+ }
+
+ desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+ digest_size = crypto_shash_digestsize(tfm);
+ digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
+ if (!digest) {
+ pr_err("digest allocate fail");
+ ret = -ENOMEM;
+ 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;
+
+ for (i = 0; i < nr_copy_pages; i++) {
+ ret = crypto_shash_update(desc, *(handle_buffers + i), PAGE_SIZE);
+ if (ret)
+ goto error_shash;
+ }
+
+ ret = crypto_shash_final(desc, digest);
+ if (ret)
+ goto error_shash;
+
+ ret = snapshot_verify_signature(digest, digest_size);
+ if (ret)
+ pr_info("PM: snapshot signature check FAIL: %d\n", ret);
+ else
+ pr_info("PM: snapshot signature check SUCCESS!\n");
+
+forward_ret:
+ snapshot_fill_sig_forward_info(ret);
+error_shash:
+ kfree(handle_buffers);
+ kfree(digest);
+error_digest:
+ if (tfm)
+ crypto_free_shash(tfm);
+ return ret;
+}
+#endif /* CONFIG_SNAPSHOT_VERIFICATION */
+
#ifdef CONFIG_HIGHMEM
/* Assumes that @buf is ready and points to a "safe" page */
static inline void
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 7c33ed2..5aef236 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1004,6 +1004,8 @@ static int load_image(struct swap_map_handle *handle,
snapshot_write_finalize(snapshot);
if (!snapshot_image_loaded(snapshot))
ret = -ENODATA;
+ else
+ ret = snapshot_image_verify();
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
return ret;
@@ -1358,6 +1360,8 @@ out_finish:
}
}
}
+ if (!ret)
+ ret = snapshot_image_verify();
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
out_clean:
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 4ed81e7..e2088af 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -228,6 +228,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
if (!data->frozen || data->ready)
break;
pm_restore_gfp_mask();
+ restore_sig_forward_info();
thaw_processes();
data->frozen = 0;
break;
@@ -253,6 +254,10 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
error = -EPERM;
break;
}
+ if (snapshot_image_verify()) {
+ error = -EPERM;
+ break;
+ }
error = hibernation_restore(data->platform_support);
break;

--
1.6.0.2

2013-09-15 00:58:18

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH 10/15] Hibernate: Avoid S4 sign key data included in snapshot image

This patch add swsusp_page_is_sign_key() method to hibernate_key.c and
check the page is S4 sign key data when collect saveable page in
snapshot.c to avoid sign key data included in snapshot image.

Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/snapshot.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index edab31f..d3e14aa 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -860,6 +860,9 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)

BUG_ON(!PageHighMem(page));

+ if (swsusp_page_is_sign_key(page))
+ return NULL;
+
if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
PageReserved(page))
return NULL;
@@ -922,6 +925,9 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)

BUG_ON(PageHighMem(page));

+ if (swsusp_page_is_sign_key(page))
+ return NULL;
+
if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
return NULL;

--
1.6.0.2

2013-09-15 00:58:28

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 11/15] Hibernate: taint kernel when signature check fail

We will not direct fail the hibernate snapshot restore when the
signature check fail, instead kernel will complain by warning
message and taint kernel.

This patch also introduced a sig_enforce flag to indicate if we want
direct fail the snapshot restore when signature check fail. User can
enable it through snapshot_sig_enforce parameter or
EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE.

Signed-off-by: Lee, Chun-Yi <[email protected]>
---
Documentation/kernel-parameters.txt | 7 +++++++
arch/x86/Kconfig | 11 +++++++++++
include/linux/kernel.h | 1 +
include/linux/suspend.h | 7 +++++++
kernel/panic.c | 2 ++
kernel/power/hibernate_keys.c | 35 +++++++++++++++++++++++++++++++++++
kernel/power/power.h | 1 +
kernel/power/snapshot.c | 6 +++++-
8 files changed, 69 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7f9d4f5..4c686c0 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2730,6 +2730,13 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
Useful for devices that are detected asynchronously
(e.g. USB and MMC devices).

+ snapshot_sig_enforce
+ [HIBERNATE] When CONFIG_SNAPSHOT_VERIFICATION is set,
+ this means the snapshot image without (valid) signatures
+ will fail to recover. This parameter provides user to
+ force launch the snapshot signature check even the UEFI
+ secure boot didn't enable.
+
hibernate= [HIBERNATION]
noresume Don't check if there's a hibernation image
present during boot.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ea73d2f..b43217a 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1591,6 +1591,17 @@ config EFI_SECURE_BOOT_SIG_ENFORCE
Say Y here to automatically enable module signature enforcement
when a system boots with UEFI Secure Boot enabled.

+config EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE
+ def_bool n
+ prompt "Force snapshot signing when UEFI Secure Boot is enabled"
+ ---help---
+ UEFI Secure Boot provides a mechanism for ensuring that the
+ firmware will only load signed bootloaders and kernels. Certain
+ use cases may also require that the snapshot image of hibernate
+ also be signed.
+ Say Y here to automatically enable snapshot iage signature
+ enforcement when a system boots with UEFI Secure Boot enabled.
+
config SECCOMP
def_bool y
prompt "Enable seccomp to safely compute untrusted bytecode"
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 482ad2d..95df772 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -427,6 +427,7 @@ extern enum system_states {
#define TAINT_CRAP 10
#define TAINT_FIRMWARE_WORKAROUND 11
#define TAINT_OOT_MODULE 12
+#define TAINT_UNSAFE_HIBERNATE 13

extern const char hex_asc[];
#define hex_asc_lo(x) hex_asc[((x) & 0x0f)]
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index f73cabf..6b46726 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -320,6 +320,13 @@ extern unsigned long get_safe_page(gfp_t gfp_mask);
extern void hibernation_set_ops(const struct platform_hibernation_ops *ops);
extern int hibernate(void);
extern bool system_entering_hibernation(void);
+
+#ifdef CONFIG_SNAPSHOT_VERIFICATION
+extern void enforce_signed_snapshot(void);
+#else
+static inline void enforce_signed_snapshot(void) {};
+#endif
+
#else /* CONFIG_HIBERNATION */
static inline void register_nosave_region(unsigned long b, unsigned long e) {}
static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
diff --git a/kernel/panic.c b/kernel/panic.c
index 8018646..c59b1f6 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -206,6 +206,7 @@ static const struct tnt tnts[] = {
{ TAINT_CRAP, 'C', ' ' },
{ TAINT_FIRMWARE_WORKAROUND, 'I', ' ' },
{ TAINT_OOT_MODULE, 'O', ' ' },
+ { TAINT_UNSAFE_HIBERNATE, 'H', ' ' },
};

/**
@@ -224,6 +225,7 @@ static const struct tnt tnts[] = {
* 'C' - modules from drivers/staging are loaded.
* 'I' - Working around severe firmware bug.
* 'O' - Out-of-tree module has been loaded.
+ * 'H' - System restored from unsafe hibernate snapshot image.
*
* The string is overwritten by the next call to print_tainted().
*/
diff --git a/kernel/power/hibernate_keys.c b/kernel/power/hibernate_keys.c
index 0bce9ab..daf08e0 100644
--- a/kernel/power/hibernate_keys.c
+++ b/kernel/power/hibernate_keys.c
@@ -17,6 +17,7 @@ struct forward_info {
static void *skey_data;
static void *forward_info_buf;
static unsigned long skey_dsize;
+static bool sig_enforce = false;

bool swsusp_page_is_sign_key(struct page *page)
{
@@ -52,6 +53,7 @@ void fill_sig_forward_info(void *page, int sig_check_ret_in)
memset(page, 0, PAGE_SIZE);
info = (struct forward_info *)page;

+ info->head.sig_enforce = sig_enforce;
info->head.sig_check_ret = sig_check_ret_in;
if (skey_data && !IS_ERR(skey_data) &&
skey_dsize <= SKEY_DBUF_MAX_SIZE) {
@@ -74,6 +76,11 @@ void restore_sig_forward_info(void)
}
info = (struct forward_info *)forward_info_buf;

+ /* eanble sig_enforce either boot kernel or resume target kernel set it */
+ sig_enforce = sig_enforce || info->head.sig_enforce;
+ if (sig_enforce)
+ pr_info("PM: Enforce S4 snapshot signature check\n");
+
sig_check_ret = info->head.sig_check_ret;
if (sig_check_ret)
pr_info("PM: Signature check fail: %d\n", sig_check_ret);
@@ -89,6 +96,14 @@ void restore_sig_forward_info(void)

/* reset skey page buffer */
memset(forward_info_buf, 0, PAGE_SIZE);
+
+ /* taint kernel */
+ if (!sig_enforce && sig_check_ret) {
+ pr_warning("PM: Hibernate signature check fail, system "
+ "restored from unsafe snapshot: tainting kernel\n");
+ add_taint(TAINT_UNSAFE_HIBERNATE, LOCKDEP_STILL_OK);
+ pr_info("%s\n", print_tainted());
+ }
}

bool skey_data_available(void)
@@ -275,6 +290,17 @@ size_t get_key_length(const struct key *key)
return len;
}

+void enforce_signed_snapshot(void)
+{
+ sig_enforce = true;
+ pr_info("PM: Enforce signature verification of hibernate snapshot\n");
+}
+
+bool sig_enforced(void)
+{
+ return sig_enforce;
+}
+
static int __init init_sign_key_data(void)
{
skey_data = (void *)get_zeroed_page(GFP_KERNEL);
@@ -290,3 +316,12 @@ static int __init init_sign_key_data(void)
}

late_initcall(init_sign_key_data);
+
+static int __init sig_enforce_setup(char *str)
+{
+ sig_enforce = true;
+ pr_info("PM: Enforce signature verification of hibernate snapshot\n");
+ return 1;
+}
+
+__setup("snapshot_sig_enforce", sig_enforce_setup);
diff --git a/kernel/power/power.h b/kernel/power/power.h
index d2da75b..4f411ac 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -187,6 +187,7 @@ extern void restore_sig_forward_info(void);
extern bool swsusp_page_is_sign_key(struct page *page);
extern unsigned long get_sig_forward_info_pfn(void);
extern void fill_sig_forward_info(void *page_addr, int sig_check_ret);
+extern bool sig_enforced(void);
#else
static inline bool skey_data_available(void)
{
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index d3e14aa..8a166e1 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -2581,7 +2581,11 @@ int snapshot_image_verify(void)
pr_info("PM: snapshot signature check SUCCESS!\n");

forward_ret:
- snapshot_fill_sig_forward_info(ret);
+ /* forward check result when pass or not enforce verify success */
+ if (!ret || !sig_enforced()) {
+ snapshot_fill_sig_forward_info(ret);
+ ret = 0;
+ }
error_shash:
kfree(handle_buffers);
kfree(digest);
--
1.6.0.2

2013-09-15 00:58:38

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 12/15] Hibernate: show the verification time for monitor performance

Show the verification time for monitor the performance of SHA256 and RSA
verification.

Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/snapshot.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 8a166e1..804feb6 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -2529,6 +2529,8 @@ static void snapshot_fill_sig_forward_info(int sig_check_ret)

int snapshot_image_verify(void)
{
+ struct timeval start;
+ struct timeval stop;
struct crypto_shash *tfm = NULL
struct shash_desc *desc;
u8 *digest = NULL;
@@ -2560,6 +2562,8 @@ int snapshot_image_verify(void)
desc->tfm = tfm;
desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;

+ do_gettimeofday(&start);
+
ret = crypto_shash_init(desc);
if (ret < 0)
goto error_shash;
@@ -2580,6 +2584,9 @@ int snapshot_image_verify(void)
else
pr_info("PM: snapshot signature check SUCCESS!\n");

+ do_gettimeofday(&stop);
+ swsusp_show_speed(&start, &stop, nr_copy_pages, "Verified");
+
forward_ret:
/* forward check result when pass or not enforce verify success */
if (!ret || !sig_enforced()) {
--
1.6.0.2

2013-09-15 00:58:48

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

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 | 25 ++++++++++++++++++++-----
2 files changed, 66 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 804feb6..896f11d 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1041,7 +1041,22 @@ static inline void copy_data_page(unsigned long dst_pfn, unsigned long src_pfn)
#endif /* CONFIG_HIGHMEM */

#ifdef CONFIG_SNAPSHOT_VERIFICATION
-#define SNAPSHOT_HASH "sha256"
+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;
+}
#endif

/*
@@ -1074,7 +1089,7 @@ swsusp_generate_signature(struct memory_bitmap *copy_bm, unsigned int nr_pages)
int ret, i;

ret = -ENOMEM;
- 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);
@@ -1127,7 +1142,7 @@ swsusp_generate_signature(struct memory_bitmap *copy_bm, unsigned int nr_pages)
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);
@@ -2491,7 +2506,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;

@@ -2544,7 +2559,7 @@ int snapshot_image_verify(void)
if (ret)
goto forward_ret;

- 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.0.2

2013-09-15 00:58:56

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 14/15] Hibernate: notify bootloader regenerate key-pair for snapshot verification

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.

V4:
- Use efivar API to access GenS4Key variable.
- Call set_key_regen_flag() in hibernate.c and user.c

Cc: Matthew Garrett <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
kernel/power/Kconfig | 15 +++++++++
kernel/power/hibernate.c | 4 ++-
kernel/power/hibernate_keys.c | 67 +++++++++++++++++++++++++++++++++++++++++
kernel/power/power.h | 5 +++
kernel/power/user.c | 6 +++-
5 files changed, 95 insertions(+), 2 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.c b/kernel/power/hibernate.c
index 90a25c7..6336499 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -675,8 +675,10 @@ int hibernate(void)
pr_debug("PM: writing image.\n");
error = swsusp_write(flags);
swsusp_free();
- if (!error)
+ if (!error) {
+ set_key_regen_flag();
power_down();
+ }
in_suspend = 0;
pm_restore_gfp_mask();
} else {
diff --git a/kernel/power/hibernate_keys.c b/kernel/power/hibernate_keys.c
index daf08e0..72d5c7a 100644
--- a/kernel/power/hibernate_keys.c
+++ b/kernel/power/hibernate_keys.c
@@ -14,6 +14,8 @@ struct forward_info {
unsigned char skey_data_buf[SKEY_DBUF_MAX_SIZE];
};

+static efi_char16_t efi_gens4key_name[9] = { 'G', 'e', 'n', 'S', '4', 'K', 'e', 'y', 0 };
+
static void *skey_data;
static void *forward_info_buf;
static unsigned long skey_dsize;
@@ -301,6 +303,70 @@ bool sig_enforced(void)
return sig_enforce;
}

+int set_key_regen_flag(void)
+{
+#ifdef CONFIG_SNAPSHOT_REGEN_KEYS
+ struct efivar_entry *entry;
+ unsigned long datasize;
+ u8 gens4key;
+ int ret;
+
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ memcpy(entry->var.VariableName, efi_gens4key_name, sizeof(efi_gens4key_name));
+ memcpy(&(entry->var.VendorGuid), &EFI_HIBERNATE_GUID, sizeof(efi_guid_t));
+
+ /* existing flag may set by userland, respect it do not overwrite */
+ datasize = 0;
+ ret = efivar_entry_size(entry, &datasize);
+ if (!ret && datasize > 0) {
+ kfree(entry);
+ return 0;
+ }
+
+ /* set flag of key-pair regeneration */
+ gens4key = 1;
+ ret = efivar_entry_set(entry,
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ 1, (void *)&gens4key, false);
+ if (ret)
+ pr_err("PM: Set GenS4Key flag fail: %d\n", ret);
+
+ kfree(entry);
+
+ return ret;
+#else
+ return 0;
+#endif
+}
+
+static int clean_key_regen_flag(void)
+{
+ struct efivar_entry *entry;
+ int ret;
+
+ entry = kmalloc(sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ memcpy(entry->var.VariableName, efi_gens4key_name, sizeof(efi_gens4key_name));
+ memcpy(&(entry->var.VendorGuid), &EFI_HIBERNATE_GUID, sizeof(efi_guid_t));
+
+ /* clean flag of key-pair regeneration */
+ ret = efivar_entry_set(entry,
+ EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS,
+ 0, NULL, false);
+ kfree(entry);
+
+ return ret;
+}
+
static int __init init_sign_key_data(void)
{
skey_data = (void *)get_zeroed_page(GFP_KERNEL);
@@ -311,6 +377,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 4f411ac..da5733f 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -188,6 +188,7 @@ extern bool swsusp_page_is_sign_key(struct page *page);
extern unsigned long get_sig_forward_info_pfn(void);
extern void fill_sig_forward_info(void *page_addr, int sig_check_ret);
extern bool sig_enforced(void);
+extern int set_key_regen_flag(void);
#else
static inline bool skey_data_available(void)
{
@@ -202,6 +203,10 @@ static inline unsigned long get_sig_forward_info_pfn(void)
{
return 0;
}
+static inline int set_key_regen_flag(void)
+{
+ return 0;
+}
#endif /* !CONFIG_SNAPSHOT_VERIFICATION */

/* kernel/power/block_io.c */
diff --git a/kernel/power/user.c b/kernel/power/user.c
index e2088af..3d3632b 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -323,6 +323,8 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
error = -EPERM;
break;
}
+ /* set regenerate S4 key flag */
+ set_key_regen_flag();
/*
* Tasks are frozen and the notifiers have been called with
* PM_HIBERNATION_PREPARE
@@ -336,8 +338,10 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
break;

case SNAPSHOT_POWER_OFF:
- if (data->platform_support)
+ if (data->platform_support) {
+ set_key_regen_flag();
error = hibernation_platform_enter();
+ }
break;

case SNAPSHOT_SET_SWAP_AREA:
--
1.6.0.2

2013-09-15 00:59:07

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 15/15] Hibernate: adapt to UEFI secure boot with signature check

Base on Matthew Garrett's 2 patches in
"[PATCH] Add additional security checks when module loading is restricted" series
[PATCH 01/10] Add secure_modules() call
[PATCH V3 11/11] Add option to automatically enforce module signatures when in Secure Boot mode

This patch introduced EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE kernel config, it's
provide user to binding UEFI secure boot with SIG_ENFORCE flag of hibernate.

In current solution, the snapshot signature check used the RSA key-pair
that are generated by bootloader(e.g. shim) then pass the key-pair to
kernel through EFI variables. Simply say: The root of trust is base on
UEFI secure boot enabled. So, that makes sense to binding the snapshot
signature check mechanism with UEFI secure boot for provide stronger
protection of hibernate. The behavior when enabled
EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE as following:

+ UEFI Secure Boot ON (means SIG_ENFORCE on),
and Kernel found key-pair from bootloader:
Will do the S4 signature check.

+ UEFI Secure Boot ON, (SIG_ENFORCE on)
but Kernel didn't find key-pair from shim:
Will lock down S4 function.

+ UEFI Secure Boot OFF (means SIG_ENFORCE off)
taint kernel when signature check fail or didn't find key-pair.

V3:
Use helper function secure_hibernate() to reduce ifdef block.

V2:
Replace sign_key_data_loaded() by skey_data_available() to check sign key data
is available for hibernate.

Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
arch/x86/kernel/setup.c | 7 ++++++
kernel/power/hibernate.c | 17 ++++++++++++++
kernel/power/hibernate_keys.c | 16 +++++++++++++
kernel/power/main.c | 7 +++++-
kernel/power/power.h | 13 +++++++++++
kernel/power/snapshot.c | 49 +++++++++++++++++++++-------------------
kernel/power/swap.c | 4 +-
kernel/power/user.c | 5 +++-
8 files changed, 91 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index deeb7bc..b3878b4 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -50,6 +50,7 @@
#include <linux/init_ohci1394_dma.h>
#include <linux/kvm_para.h>
#include <linux/dma-contiguous.h>
+#include <linux/suspend.h>

#include <linux/errno.h>
#include <linux/kernel.h>
@@ -1135,6 +1136,12 @@ void __init setup_arch(char **cmdline_p)
}
#endif

+#ifdef CONFIG_EFI_SECURE_BOOT_SNAPSHOT_SIG_ENFORCE
+ if (boot_params.secure_boot) {
+ enforce_signed_snapshot();
+ }
+#endif
+
/*
* Parse the ACPI tables for possible boot-time SMP configuration.
*/
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 6336499..abbc5b0 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -29,6 +29,7 @@
#include <linux/ctype.h>
#include <linux/genhd.h>
#include <linux/key.h>
+#include <linux/efi.h>

#include "power.h"

@@ -633,6 +634,9 @@ int hibernate(void)
{
int error;

+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_SKEY))
+ return -EPERM;
+
lock_system_sleep();
/* The snapshot device should not be opened while we're running */
if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
@@ -800,6 +804,11 @@ static int software_resume(void)
if (error)
goto Unlock;

+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_WKEY)) {
+ mutex_unlock(&pm_mutex);
+ return -EPERM;
+ }
+
/* The snapshot device should not be opened while we're running */
if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
error = -EBUSY;
@@ -893,6 +902,11 @@ static ssize_t disk_show(struct kobject *kobj, struct kobj_attribute *attr,
int i;
char *start = buf;

+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_SKEY)) {
+ buf += sprintf(buf, "[%s]\n", "disabled");
+ return buf-start;
+ }
+
for (i = HIBERNATION_FIRST; i <= HIBERNATION_MAX; i++) {
if (!hibernation_modes[i])
continue;
@@ -927,6 +941,9 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr,
char *p;
int mode = HIBERNATION_INVALID;

+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_SKEY))
+ return -EPERM;
+
p = memchr(buf, '\n', n);
len = p ? p - buf : n;

diff --git a/kernel/power/hibernate_keys.c b/kernel/power/hibernate_keys.c
index 72d5c7a..b70f397 100644
--- a/kernel/power/hibernate_keys.c
+++ b/kernel/power/hibernate_keys.c
@@ -367,6 +367,22 @@ static int clean_key_regen_flag(void)
return ret;
}

+bool secure_hibernate(u8 check_items)
+{
+ bool ret = true;
+
+ if (check_items & SIG_ENFORCE)
+ ret = sig_enforce;
+
+ /* check S4 key to lock hibernate when not available */
+ if (ret && (check_items & SIG_CHECK_SKEY))
+ ret = ret && !skey_data_available();
+ if (ret && (check_items & SIG_CHECK_WKEY))
+ ret = ret && (wkey_data_available() != 0);
+
+ return ret;
+}
+
static int __init init_sign_key_data(void)
{
skey_data = (void *)get_zeroed_page(GFP_KERNEL);
diff --git a/kernel/power/main.c b/kernel/power/main.c
index 1d1bf63..10ff23f 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -15,6 +15,7 @@
#include <linux/workqueue.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/efi.h>

#include "power.h"

@@ -301,7 +302,11 @@ static ssize_t state_show(struct kobject *kobj, struct kobj_attribute *attr,
}
#endif
#ifdef CONFIG_HIBERNATION
- s += sprintf(s, "%s\n", "disk");
+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_SKEY)) {
+ s += sprintf(s, "\n");
+ } else {
+ s += sprintf(s, "%s\n", "disk");
+ }
#else
if (s != buf)
/* convert the last space to a newline */
diff --git a/kernel/power/power.h b/kernel/power/power.h
index da5733f..8fc94cc 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -2,6 +2,7 @@
#include <linux/suspend_ioctls.h>
#include <linux/utsname.h>
#include <linux/freezer.h>
+#include <linux/module.h>

/* The maximum length of snapshot signature */
#define SIG_LEN 512
@@ -174,6 +175,11 @@ extern int swsusp_unmark(void);
#endif

/* kernel/power/hibernate_key.c */
+#define SIG_ENFORCE (1<<0) /* Check sig_enforce flag */
+#define SIG_CHECK_SKEY (1<<1) /* Check S4SignKey exist */
+#define SIG_CHECK_WKEY (1<<2) /* Check S4WakeKey exist */
+#define SIG_SECURE_LOCKDOWN (1<<3) /* TODO: binding to new secure level */
+
#ifdef CONFIG_SNAPSHOT_VERIFICATION
extern bool skey_data_available(void);
extern struct key *get_sign_key(void);
@@ -189,6 +195,7 @@ extern unsigned long get_sig_forward_info_pfn(void);
extern void fill_sig_forward_info(void *page_addr, int sig_check_ret);
extern bool sig_enforced(void);
extern int set_key_regen_flag(void);
+extern bool secure_hibernate(u8 check_items);
#else
static inline bool skey_data_available(void)
{
@@ -207,6 +214,12 @@ static inline int set_key_regen_flag(void)
{
return 0;
}
+static inline bool secure_hibernate(u8 check_items)
+{
+ /* TODO: adapt to kernel lockdown */
+
+ return false;
+}
#endif /* !CONFIG_SNAPSHOT_VERIFICATION */

/* kernel/power/block_io.c */
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 896f11d..29f1ec1 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1759,13 +1759,15 @@ asmlinkage int swsusp_save(void)
nr_copy_pages = nr_pages;
nr_meta_pages = DIV_ROUND_UP(nr_pages * sizeof(long), PAGE_SIZE);

- if (skey_data_available()) {
- ret = swsusp_generate_signature(&copy_bm, nr_pages);
- if (ret)
- return ret;
- } else
- /* set zero signature if skey doesn't exist */
- memset(signature, 0, SIG_LEN);
+ if (secure_hibernate(0)) {
+ if (skey_data_available()) {
+ ret = swsusp_generate_signature(&copy_bm, nr_pages);
+ if (ret)
+ return ret;
+ } else
+ /* set zero signature if skey doesn't exist */
+ memset(signature, 0, SIG_LEN);
+ }

printk(KERN_INFO "PM: Hibernation image created (%d pages copied)\n",
nr_pages);
@@ -2398,14 +2400,15 @@ 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,
* handle_buffers will freed in snapshot_image_verify().
*/
- handle_buffers = kmalloc(sizeof(void *) * nr_copy_pages, GFP_KERNEL);
- if (!handle_buffers)
- pr_err("Allocate hash buffer fail!\n");
-#endif
+ if (secure_hibernate(0)) {
+ handle_buffers =
+ kmalloc(sizeof(void *) * nr_copy_pages, GFP_KERNEL);
+ if (!handle_buffers)
+ pr_err("Allocate hash buffer fail!\n");
+ }

error = memory_bm_create(&copy_bm, GFP_ATOMIC, PG_ANY);
if (error)
@@ -2433,10 +2436,8 @@ 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 (handle_buffers)
+ if (secure_hibernate(0) && handle_buffers)
*handle_buffers = handle->buffer;
-#endif
}
} else {
copy_last_highmem_page();
@@ -2447,13 +2448,15 @@ 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 (handle_buffers)
- *(handle_buffers + (handle->cur - nr_meta_pages - 1)) = handle->buffer;
- /* Keep the buffer of sign key in snapshot */
- if (pfn == sig_forward_info_pfn)
- sig_forward_info_buf = handle->buffer;
-#endif
+ if (secure_hibernate(0)) {
+ if (handle_buffers) {
+ unsigned int offset = handle->cur - nr_meta_pages - 1;
+ *(handle_buffers + offset) = handle->buffer;
+ }
+ /* Keep the buffer of sign key in snapshot */
+ if (pfn == sig_forward_info_pfn)
+ sig_forward_info_buf = handle->buffer;
+ }
}
handle->cur++;
return PAGE_SIZE;
@@ -2546,7 +2549,7 @@ int snapshot_image_verify(void)
{
struct timeval start;
struct timeval stop;
- struct crypto_shash *tfm = NULL
+ struct crypto_shash *tfm = NULL;
struct shash_desc *desc;
u8 *digest = NULL;
size_t digest_size, desc_size;
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index 5aef236..19cb393 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1004,7 +1004,7 @@ static int load_image(struct swap_map_handle *handle,
snapshot_write_finalize(snapshot);
if (!snapshot_image_loaded(snapshot))
ret = -ENODATA;
- else
+ else if (secure_hibernate(0))
ret = snapshot_image_verify();
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
@@ -1360,7 +1360,7 @@ out_finish:
}
}
}
- if (!ret)
+ if (!ret && secure_hibernate(0))
ret = snapshot_image_verify();
}
swsusp_show_speed(&start, &stop, nr_to_read, "Read");
diff --git a/kernel/power/user.c b/kernel/power/user.c
index 3d3632b..6c75427 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -48,6 +48,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
struct snapshot_data *data;
int error;

+ if (secure_hibernate(SIG_ENFORCE | SIG_CHECK_SKEY | SIG_SECURE_LOCKDOWN))
+ return -EPERM;
+
lock_system_sleep();

if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
@@ -254,7 +257,7 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
error = -EPERM;
break;
}
- if (snapshot_image_verify()) {
+ if (secure_hibernate(0) && snapshot_image_verify()) {
error = -EPERM;
break;
}
--
1.6.0.2

2013-09-15 00:57:06

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 01/15] asymmetric keys: add interface and skeleton for implement signature generation

Add generate_signature interface on signature.c, asymmetric-subtype and
rsa.c for prepare to implement signature generation.

Reviewed-by: Jiri Kosina <[email protected]>
Signed-off-by: Lee, Chun-Yi <[email protected]>
---
crypto/asymmetric_keys/private_key.h | 29 +++++++++++++++++++++++++++++
crypto/asymmetric_keys/public_key.c | 31 +++++++++++++++++++++++++++++++
crypto/asymmetric_keys/rsa.c | 22 ++++++++++++++++++++++
crypto/asymmetric_keys/signature.c | 28 ++++++++++++++++++++++++++++
include/crypto/public_key.h | 25 +++++++++++++++++++++++++
include/keys/asymmetric-subtype.h | 6 ++++++
6 files changed, 141 insertions(+), 0 deletions(-)
create mode 100644 crypto/asymmetric_keys/private_key.h

diff --git a/crypto/asymmetric_keys/private_key.h b/crypto/asymmetric_keys/private_key.h
new file mode 100644
index 0000000..c022eee
--- /dev/null
+++ b/crypto/asymmetric_keys/private_key.h
@@ -0,0 +1,29 @@
+/* Private key algorithm internals
+ *
+ * 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.
+ */
+
+#include <crypto/public_key.h>
+
+extern struct asymmetric_key_subtype private_key_subtype;
+
+/*
+ * Private key algorithm definition.
+ */
+struct private_key_algorithm {
+ const char *name;
+ u8 n_pub_mpi; /* Number of MPIs in public key */
+ u8 n_sec_mpi; /* Number of MPIs in secret key */
+ u8 n_sig_mpi; /* Number of MPIs in a signature */
+ struct public_key_signature* (*generate_signature)(
+ const struct private_key *key, u8 *M,
+ enum pkey_hash_algo hash_algo, const bool hash);
+};
+
+extern const struct private_key_algorithm RSA_private_key_algorithm;
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index cb2e291..80c19cd 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -19,6 +19,7 @@
#include <linux/seq_file.h>
#include <keys/asymmetric-subtype.h>
#include "public_key.h"
+#include "private_key.h"

MODULE_LICENSE("GPL");

@@ -96,6 +97,24 @@ static int public_key_verify_signature(const struct key *key,
}

/*
+ * Generate a signature using a private key.
+ */
+static struct public_key_signature *private_key_generate_signature(
+ const struct key *key, u8 *M, enum pkey_hash_algo hash_algo,
+ const bool hash)
+{
+ const struct private_key *pk = key->payload.data;
+
+ pr_info("private_key_generate_signature start\n");
+
+ if (!pk->algo->generate_signature)
+ return ERR_PTR(-ENOTSUPP);
+
+ return pk->algo->generate_signature(pk, M, hash_algo, hash);
+
+}
+
+/*
* Public key algorithm asymmetric key subtype
*/
struct asymmetric_key_subtype public_key_subtype = {
@@ -106,3 +125,15 @@ struct asymmetric_key_subtype public_key_subtype = {
.verify_signature = public_key_verify_signature,
};
EXPORT_SYMBOL_GPL(public_key_subtype);
+
+/*
+ * Private key algorithm asymmetric key subtype
+ */
+struct asymmetric_key_subtype private_key_subtype = {
+ .owner = THIS_MODULE,
+ .name = "private_key",
+ .describe = public_key_describe,
+ .destroy = public_key_destroy,
+ .generate_signature = private_key_generate_signature,
+};
+EXPORT_SYMBOL_GPL(private_key_subtype);
diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
index 4a6a069..47f3be4 100644
--- a/crypto/asymmetric_keys/rsa.c
+++ b/crypto/asymmetric_keys/rsa.c
@@ -14,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/slab.h>
#include "public_key.h"
+#include "private_key.h"

MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("RSA Public Key Algorithm");
@@ -267,6 +268,18 @@ error:
return ret;
}

+/*
+ * Perform the generation step [RFC3447 sec 8.2.1].
+ */
+static struct public_key_signature *RSA_generate_signature(
+ const struct private_key *key, u8 *M,
+ enum pkey_hash_algo hash_algo, const bool hash)
+{
+ pr_info("RSA_generate_signature start\n");
+
+ return 0;
+}
+
const struct public_key_algorithm RSA_public_key_algorithm = {
.name = "RSA",
.n_pub_mpi = 2,
@@ -275,3 +288,12 @@ const struct public_key_algorithm RSA_public_key_algorithm = {
.verify_signature = RSA_verify_signature,
};
EXPORT_SYMBOL_GPL(RSA_public_key_algorithm);
+
+const struct private_key_algorithm RSA_private_key_algorithm = {
+ .name = "RSA",
+ .n_pub_mpi = 2,
+ .n_sec_mpi = 3,
+ .n_sig_mpi = 1,
+ .generate_signature = RSA_generate_signature,
+};
+EXPORT_SYMBOL_GPL(RSA_private_key_algorithm);
diff --git a/crypto/asymmetric_keys/signature.c b/crypto/asymmetric_keys/signature.c
index 50b3f88..a1bf6be 100644
--- a/crypto/asymmetric_keys/signature.c
+++ b/crypto/asymmetric_keys/signature.c
@@ -47,3 +47,31 @@ int verify_signature(const struct key *key,
return ret;
}
EXPORT_SYMBOL_GPL(verify_signature);
+
+/**
+ * generate_signature - Initiate the use of an asymmetric key to generate a signature
+ * @key: The asymmetric key to generate against
+ * @M: The message to be signed, or a hash result. Dependent on the hash parameter
+ * @hash_algo: The hash algorithm to generate digest
+ * @hash: true means M is a original mesagse, false means M is a hash result
+ *
+ * Returns public_key-signature if successful or else an error.
+ */
+struct public_key_signature *generate_signature(const struct key *key, u8 *M,
+ enum pkey_hash_algo hash_algo, const bool hash)
+{
+ const struct asymmetric_key_subtype *subtype;
+
+ pr_info("==>%s()\n", __func__);
+
+ if (key->type != &key_type_asymmetric)
+ return ERR_PTR(-EINVAL);
+ subtype = asymmetric_key_subtype(key);
+ if (!subtype || !key->payload.data)
+ return ERR_PTR(-EINVAL);
+ if (!subtype->generate_signature)
+ return ERR_PTR(-ENOTSUPP);
+
+ return subtype->generate_signature(key, M, hash_algo, hash);
+}
+EXPORT_SYMBOL_GPL(generate_signature);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index f5b0224..d44b29f 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -79,6 +79,29 @@ struct public_key {
};
};

+struct private_key {
+ const struct private_key_algorithm *algo;
+ u8 capabilities;
+ enum pkey_id_type id_type:8;
+ union {
+ MPI mpi[5];
+ struct {
+ MPI p; /* DSA prime */
+ MPI q; /* DSA group order */
+ MPI g; /* DSA group generator */
+ MPI y; /* DSA public-key value = g^x mod p */
+ MPI x; /* DSA secret exponent (if present) */
+ } dsa;
+ struct {
+ MPI n; /* RSA public modulus */
+ MPI e; /* RSA public encryption exponent */
+ MPI d; /* RSA secret encryption exponent (if present) */
+ MPI p; /* RSA secret prime (if present) */
+ MPI q; /* RSA secret prime (if present) */
+ } rsa;
+ };
+};
+
extern void public_key_destroy(void *payload);

/*
@@ -104,5 +127,7 @@ struct public_key_signature {
struct key;
extern int verify_signature(const struct key *key,
const struct public_key_signature *sig);
+extern struct public_key_signature *generate_signature(const struct key *key,
+ u8 *M, enum pkey_hash_algo hash_algo, const bool hash);

#endif /* _LINUX_PUBLIC_KEY_H */
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index 4b840e8..af79939 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -18,6 +18,7 @@
#include <keys/asymmetric-type.h>

struct public_key_signature;
+enum pkey_hash_algo;

/*
* Keys of this type declare a subtype that indicates the handlers and
@@ -37,6 +38,11 @@ struct asymmetric_key_subtype {
/* Verify the signature on a key of this subtype (optional) */
int (*verify_signature)(const struct key *key,
const struct public_key_signature *sig);
+
+ /* Generate the signature by key of this subtype (optional) */
+ struct public_key_signature* (*generate_signature)
+ (const struct key *key, u8 *M, enum pkey_hash_algo hash_algo,
+ const bool hash);
};

/**
--
1.6.0.2

2013-09-15 01:01:25

by Lee, Chun-Yi

[permalink] [raw]
Subject: [PATCH V4 03/15] asymmetric keys: separate the length checking of octet string from RSA_I2OSP

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.

The naming of _RSA_I2OSP and the variables used in this function accord PKCS#1
spec but not follow kernel naming convention, it useful when look at them with
spec.

Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf

Cc: Pavel Machek <[email protected]>
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 352ba45..aac8b77 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.0.2

2013-09-17 21:51:20

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

Hello,


On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <[email protected]> wrote:
> Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
>
> This patch is temporary set emLen to pks->k, and temporary set EM to
> pks->S for debugging. We will replace the above values to real signature
> after implement RSASP1.
>
> The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> at them with spec.
>
> Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
>
> V2:
> - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
>
> Cc: Pavel Machek <[email protected]>
> Reviewed-by: Jiri Kosina <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> ---
> crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
> include/crypto/public_key.h | 2 +
> 2 files changed, 164 insertions(+), 1 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> index 47f3be4..352ba45 100644
> --- a/crypto/asymmetric_keys/rsa.c
> +++ b/crypto/asymmetric_keys/rsa.c
> @@ -13,6 +13,7 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <crypto/hash.h>
> #include "public_key.h"
> #include "private_key.h"
>
> @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> }
>
> /*
> + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> + * @M: message to be signed, an octet string
> + * @emLen: intended length in octets of the encoded message
> + * @hash_algo: hash function (option)
> + * @hash: true means hash M, otherwise M is already a digest
> + * @EM: encoded message, an octet string of length emLen
> + *
> + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> + *
> + * The variables used in this function accord PKCS#1 spec but not follow kernel
> + * naming convention, it useful when look at them with spec.
> + */
> +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> + enum pkey_hash_algo hash_algo, const bool hash,
> + u8 **EM, struct public_key_signature *pks)
> +{
> + u8 *digest;
> + struct crypto_shash *tfm;
> + struct shash_desc *desc;
> + size_t digest_size, desc_size;
> + size_t tLen;
> + u8 *T, *PS, *EM_tmp;
> + int i, ret;
> +
> + pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> +
> + if (!RSA_ASN1_templates[hash_algo].data)

What about checking hash_algo against PKEY_HASH__LAST, or it relies on
the caller?


> + ret = -ENOTSUPP;
> + else
> + pks->pkey_hash_algo = hash_algo;
> +
> + /* 1) Apply the hash function to the message M to produce a hash value H */
> + tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> + if (IS_ERR(tfm))
> + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> +
> + 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;
> + pks->digest = digest;
> + pks->digest_size = digest_size;
> +

Ok. You allocated tfm to get hash size, right?

But why do you allocate descriptor even it might not be needed?

> + if (hash) {
> + 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;
> + ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);

This is I completely fail to understand... You expect sizeof(M) to be
the message length?????
Have you ever tested it?

> + if (ret < 0)
> + goto error_shash;
> + } else {
> + memcpy(pks->digest, M, pks->digest_size);
> + pks->digest_size = digest_size;
> + }

Does caller use pks->digest and pks->digest_size after return?
I think it needs encoded value, not the hash...
So why do you pass pks?



> + crypto_free_shash(tfm);
> +
> + /* 2) Encode the algorithm ID for the hash function and the hash value into
> + * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> + * the DigestInfo value and let tLen be the length in octets of T.
> + */
> + tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> + T = kmalloc(tLen, GFP_KERNEL);
> + if (!T)
> + goto error_T;
> +

Why do you need T and PS memory allocations at all?
You need only EM_tmp allocation and copy directly to the destination...


> + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> +
> + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> + if (emLen < tLen + 11) {
> + ret = -EINVAL;
> + goto error_emLen;
> + }
> +
> + /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> + PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> + if (!PS)
> + goto error_P;
> +

ditto

> + for (i = 0; i < (emLen - tLen - 3); i++)
> + PS[i] = 0xff;
> +
> + /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> + * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> + */
> + EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> + if (!EM_tmp)
> + goto error_EM;
> +
> + EM_tmp[0] = 0x00;
> + EM_tmp[1] = 0x01;
> + memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> + EM_tmp[2 + emLen - tLen - 3] = 0x00;
> + memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> +
> + *EM = EM_tmp;
> +
> + kfree(PS);
> + kfree(T);

get rid of it...


- Dmitry

> +
> + return 0;
> +
> +error_EM:
> + kfree(PS);
> +error_P:
> +error_emLen:
> + kfree(T);
> +error_T:
> +error_shash:
> + kfree(digest);
> +error_digest:
> + crypto_free_shash(tfm);
> + return ret;
> +}
> +
> +/*
> * Perform the RSA signature verification.
> * @H: Value of hash of data and metadata
> * @EM: The computed signature value
> @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> const struct private_key *key, u8 *M,
> enum pkey_hash_algo hash_algo, const bool hash)
> {
> + struct public_key_signature *pks;
> + u8 *EM = NULL;
> + size_t emLen;
> + int ret;
> +
> pr_info("RSA_generate_signature start\n");
>
> - return 0;
> + ret = -ENOMEM;
> + pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> + if (!pks)
> + goto error_no_pks;
> +
> + /* 1): EMSA-PKCS1-v1_5 encoding: */
> + /* Use the private key modulus size to be EM length */
> + emLen = mpi_get_nbits(key->rsa.n);
> + emLen = (emLen + 7) / 8;
> +
> + ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> + if (ret < 0)
> + goto error_v1_5_encode;
> +
> + /* TODO 2): m = OS2IP (EM) */
> +
> + /* TODO 3): s = RSASP1 (K, m) */
> +
> + /* TODO 4): S = I2OSP (s, k) */
> +
> + /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> + pks->S = EM; /* TODO: temporary set S to EM */
> +
> + return pks;
> +
> +error_v1_5_encode:
> + kfree(pks);
> +error_no_pks:
> + pr_info("<==%s() = %d\n", __func__, ret);
> + return ERR_PTR(ret);
> }
>
> const struct public_key_algorithm RSA_public_key_algorithm = {
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index d44b29f..1cdf457 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> struct public_key_signature {
> u8 *digest;
> u8 digest_size; /* Number of bytes in digest */
> + u8 *S; /* signature S of length k octets */
> + size_t k; /* length k of signature S */
> u8 nr_mpi; /* Occupancy of mpi[] */
> enum pkey_hash_algo pkey_hash_algo : 8;
> union {
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Thanks,
Dmitry

2013-09-17 22:30:00

by Dmitry Kasatkin

[permalink] [raw]
Subject: Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <[email protected]> wrote:
> Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
>
> This patch is temporary set emLen to pks->k, and temporary set EM to
> pks->S for debugging. We will replace the above values to real signature
> after implement RSASP1.
>
> The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> at them with spec.
>
> Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
>
> V2:
> - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
>
> Cc: Pavel Machek <[email protected]>
> Reviewed-by: Jiri Kosina <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> ---
> crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
> include/crypto/public_key.h | 2 +
> 2 files changed, 164 insertions(+), 1 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> index 47f3be4..352ba45 100644
> --- a/crypto/asymmetric_keys/rsa.c
> +++ b/crypto/asymmetric_keys/rsa.c
> @@ -13,6 +13,7 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <crypto/hash.h>
> #include "public_key.h"
> #include "private_key.h"
>
> @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> }
>
> /*
> + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> + * @M: message to be signed, an octet string
> + * @emLen: intended length in octets of the encoded message
> + * @hash_algo: hash function (option)
> + * @hash: true means hash M, otherwise M is already a digest
> + * @EM: encoded message, an octet string of length emLen
> + *
> + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> + *
> + * The variables used in this function accord PKCS#1 spec but not follow kernel
> + * naming convention, it useful when look at them with spec.
> + */
> +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> + enum pkey_hash_algo hash_algo, const bool hash,
> + u8 **EM, struct public_key_signature *pks)
> +{
> + u8 *digest;
> + struct crypto_shash *tfm;
> + struct shash_desc *desc;
> + size_t digest_size, desc_size;
> + size_t tLen;
> + u8 *T, *PS, *EM_tmp;
> + int i, ret;
> +
> + pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> +
> + if (!RSA_ASN1_templates[hash_algo].data)
> + ret = -ENOTSUPP;
> + else
> + pks->pkey_hash_algo = hash_algo;
> +
> + /* 1) Apply the hash function to the message M to produce a hash value H */
> + tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> + if (IS_ERR(tfm))
> + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> +
> + 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;
> + pks->digest = digest;
> + pks->digest_size = digest_size;
> +
> + if (hash) {
> + 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;
> + ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> + if (ret < 0)
> + goto error_shash;
> + } else {
> + memcpy(pks->digest, M, pks->digest_size);
> + pks->digest_size = digest_size;
> + }
> + crypto_free_shash(tfm);
> +
> + /* 2) Encode the algorithm ID for the hash function and the hash value into
> + * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> + * the DigestInfo value and let tLen be the length in octets of T.
> + */
> + tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> + T = kmalloc(tLen, GFP_KERNEL);
> + if (!T)
> + goto error_T;
> +

as I said, remove it... see bellow....

> + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> +
> + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> + if (emLen < tLen + 11) {
> + ret = -EINVAL;
> + goto error_emLen;
> + }
> +
> + /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> + PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> + if (!PS)
> + goto error_P;
> +
as I said remove it...
see bellow..

> + for (i = 0; i < (emLen - tLen - 3); i++)
> + PS[i] = 0xff;
> +

memset() does not work here?

> + /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> + * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> + */
> + EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> + if (!EM_tmp)
> + goto error_EM;
> +
> + EM_tmp[0] = 0x00;
> + EM_tmp[1] = 0x01;

> + memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> + EM_tmp[2 + emLen - tLen - 3] = 0x00;

above 2 lines can be replaced by:

PS = &EM_tmp[2];
PS_len = emLen - tLen - 3
memset(PS, 0xff, PS_len);
EM_tmp[2 + PS_len] = 0x00;


> + memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> +
This can be replaced by:

T = &EM_tmp[2+ PS_Len + 1];
memcpy(T, RSA_ASN1_templates[hash_algo].data,
RSA_ASN1_templates[hash_algo].size);
memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest,
pks->digest_size);

> + *EM = EM_tmp;
> +
> + kfree(PS);
> + kfree(T);
> +

Right? So please remove unneeded allocations...

Dmitry

> + return 0;
> +
> +error_EM:
> + kfree(PS);
> +error_P:
> +error_emLen:
> + kfree(T);
> +error_T:
> +error_shash:
> + kfree(digest);
> +error_digest:
> + crypto_free_shash(tfm);
> + return ret;
> +}
> +
> +/*
> * Perform the RSA signature verification.
> * @H: Value of hash of data and metadata
> * @EM: The computed signature value
> @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> const struct private_key *key, u8 *M,
> enum pkey_hash_algo hash_algo, const bool hash)
> {
> + struct public_key_signature *pks;
> + u8 *EM = NULL;
> + size_t emLen;
> + int ret;
> +
> pr_info("RSA_generate_signature start\n");
>
> - return 0;
> + ret = -ENOMEM;
> + pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> + if (!pks)
> + goto error_no_pks;
> +
> + /* 1): EMSA-PKCS1-v1_5 encoding: */
> + /* Use the private key modulus size to be EM length */
> + emLen = mpi_get_nbits(key->rsa.n);
> + emLen = (emLen + 7) / 8;
> +
> + ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> + if (ret < 0)
> + goto error_v1_5_encode;
> +
> + /* TODO 2): m = OS2IP (EM) */
> +
> + /* TODO 3): s = RSASP1 (K, m) */
> +
> + /* TODO 4): S = I2OSP (s, k) */
> +
> + /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> + pks->S = EM; /* TODO: temporary set S to EM */
> +
> + return pks;
> +
> +error_v1_5_encode:
> + kfree(pks);
> +error_no_pks:
> + pr_info("<==%s() = %d\n", __func__, ret);
> + return ERR_PTR(ret);
> }
>
> const struct public_key_algorithm RSA_public_key_algorithm = {
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index d44b29f..1cdf457 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> struct public_key_signature {
> u8 *digest;
> u8 digest_size; /* Number of bytes in digest */
> + u8 *S; /* signature S of length k octets */
> + size_t k; /* length k of signature S */
> u8 nr_mpi; /* Occupancy of mpi[] */
> enum pkey_hash_algo pkey_hash_algo : 8;
> union {
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Thanks,
Dmitry

2013-09-18 09:07:38

by joeyli

[permalink] [raw]
Subject: Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

Hi Dmitry,

First, thanks for your time to review my patches!

於 二,2013-09-17 於 16:51 -0500,Dmitry Kasatkin 提到:
> Hello,
>
>
> On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <[email protected]> wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <[email protected]>
> > Reviewed-by: Jiri Kosina <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
> > include/crypto/public_key.h | 2 +
> > 2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> > #include <linux/module.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > +#include <crypto/hash.h>
> > #include "public_key.h"
> > #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> > }
> >
> > /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > + enum pkey_hash_algo hash_algo, const bool hash,
> > + u8 **EM, struct public_key_signature *pks)
> > +{
> > + u8 *digest;
> > + struct crypto_shash *tfm;
> > + struct shash_desc *desc;
> > + size_t digest_size, desc_size;
> > + size_t tLen;
> > + u8 *T, *PS, *EM_tmp;
> > + int i, ret;
> > +
> > + pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > + if (!RSA_ASN1_templates[hash_algo].data)
>
> What about checking hash_algo against PKEY_HASH__LAST, or it relies on
> the caller?
>

Yes, check PKEY_HASH__LAST is more easy and clear, I will change it.
Thanks!

>
> > + ret = -ENOTSUPP;
> > + else
> > + pks->pkey_hash_algo = hash_algo;
> > +
> > + /* 1) Apply the hash function to the message M to produce a hash value H */
> > + tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > + if (IS_ERR(tfm))
> > + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > + 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;
> > + pks->digest = digest;
> > + pks->digest_size = digest_size;
> > +
>
> Ok. You allocated tfm to get hash size, right?
>
> But why do you allocate descriptor even it might not be needed?
>

You are right, I should skip the code of allocate descriptor when the
hash is supported. I will modified it.
Thanks!

> > + if (hash) {
> > + 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;
> > + ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
>
> This is I completely fail to understand... You expect sizeof(M) to be
> the message length?????
> Have you ever tested it?
>

Sigh!
I just checked my test program for this code path, this stupid problem
causes by my test program doesn't feed the right hash result that should
used to verify signature. So, I didn't capture this bug.

And, the hibernate signature check mechanism doesn't run into this code
path because the hash generation is done by hibernate code but not in
here. So, I also didn't find this problem when running hibernate check.

Appreciate for your point out! I just fix it in next patch version.

> > + if (ret < 0)
> > + goto error_shash;
> > + } else {
> > + memcpy(pks->digest, M, pks->digest_size);
> > + pks->digest_size = digest_size;
> > + }
>
> Does caller use pks->digest and pks->digest_size after return?
> I think it needs encoded value, not the hash...
> So why do you pass pks?
>

I put the signature MPI to pks->rsa.s, and put the encoded signature to
pks->S. So caller can grab encoded signature.

I also pass the pks->digest and pks->digest_size to caller for
reference. Then caller can simply feed this pks to
RSA_verify_signature() for verify the signature result, don't need
generate hash again.

>
>
> > + crypto_free_shash(tfm);
> > +
> > + /* 2) Encode the algorithm ID for the hash function and the hash value into
> > + * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> > + * the DigestInfo value and let tLen be the length in octets of T.
> > + */
> > + tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> > + T = kmalloc(tLen, GFP_KERNEL);
> > + if (!T)
> > + goto error_T;
> > +
>
> Why do you need T and PS memory allocations at all?
> You need only EM_tmp allocation and copy directly to the destination...
>

OK, I will change the code to allocate T and PS size in EM_tmp.

>
> > + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> > + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> > +
> > + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> > + if (emLen < tLen + 11) {
> > + ret = -EINVAL;
> > + goto error_emLen;
> > + }
> > +
> > + /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> > + PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> > + if (!PS)
> > + goto error_P;
> > +
>
> ditto

OK, I will allocate PS with EM_tmp.
Thanks!

>
> > + for (i = 0; i < (emLen - tLen - 3); i++)
> > + PS[i] = 0xff;
> > +
> > + /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> > + * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> > + */
> > + EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> > + if (!EM_tmp)
> > + goto error_EM;
> > +
> > + EM_tmp[0] = 0x00;
> > + EM_tmp[1] = 0x01;
> > + memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> > + EM_tmp[2 + emLen - tLen - 3] = 0x00;
> > + memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> > +
> > + *EM = EM_tmp;
> > +
> > + kfree(PS);
> > + kfree(T);
>
> get rid of it...
>

OK!

>
> - Dmitry
>
> > +
> > + return 0;
> > +
> > +error_EM:
> > + kfree(PS);
> > +error_P:
> > +error_emLen:
> > + kfree(T);
> > +error_T:
> > +error_shash:
> > + kfree(digest);
> > +error_digest:
> > + crypto_free_shash(tfm);
> > + return ret;
> > +}
> > +
> > +/*
> > * Perform the RSA signature verification.
> > * @H: Value of hash of data and metadata
> > * @EM: The computed signature value
> > @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> > const struct private_key *key, u8 *M,
> > enum pkey_hash_algo hash_algo, const bool hash)
> > {
> > + struct public_key_signature *pks;
> > + u8 *EM = NULL;
> > + size_t emLen;
> > + int ret;
> > +
> > pr_info("RSA_generate_signature start\n");
> >
> > - return 0;
> > + ret = -ENOMEM;
> > + pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> > + if (!pks)
> > + goto error_no_pks;
> > +
> > + /* 1): EMSA-PKCS1-v1_5 encoding: */
> > + /* Use the private key modulus size to be EM length */
> > + emLen = mpi_get_nbits(key->rsa.n);
> > + emLen = (emLen + 7) / 8;
> > +
> > + ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> > + if (ret < 0)
> > + goto error_v1_5_encode;
> > +
> > + /* TODO 2): m = OS2IP (EM) */
> > +
> > + /* TODO 3): s = RSASP1 (K, m) */
> > +
> > + /* TODO 4): S = I2OSP (s, k) */
> > +
> > + /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> > + pks->S = EM; /* TODO: temporary set S to EM */
> > +
> > + return pks;
> > +
> > +error_v1_5_encode:
> > + kfree(pks);
> > +error_no_pks:
> > + pr_info("<==%s() = %d\n", __func__, ret);
> > + return ERR_PTR(ret);
> > }
> >
> > const struct public_key_algorithm RSA_public_key_algorithm = {
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index d44b29f..1cdf457 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> > struct public_key_signature {
> > u8 *digest;
> > u8 digest_size; /* Number of bytes in digest */
> > + u8 *S; /* signature S of length k octets */
> > + size_t k; /* length k of signature S */
> > u8 nr_mpi; /* Occupancy of mpi[] */
> > enum pkey_hash_algo pkey_hash_algo : 8;
> > union {
> > --
> > 1.6.0.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>

Thanks a lot!
Joey Lee

2013-09-18 13:45:39

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

On Sun 2013-09-15 08:56:59, 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.

This series is big enough already... and who is going to test it?
There's no need to make hash configurable. Just select one that works.

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-23 16:50:00

by Phil Carmody

[permalink] [raw]
Subject: Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

On Sun, Sep 15, 2013 at 08:56:48AM +0800, Lee, Chun-Yi wrote:
> Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
>
> This patch is temporary set emLen to pks->k, and temporary set EM to
> pks->S for debugging. We will replace the above values to real signature
> after implement RSASP1.
>
> The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> at them with spec.
>
> Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
>
> V2:

You're now at V4.

> - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
>
> Cc: Pavel Machek <[email protected]>
> Reviewed-by: Jiri Kosina <[email protected]>
> Signed-off-by: Lee, Chun-Yi <[email protected]>
> ---
> crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
> include/crypto/public_key.h | 2 +
> 2 files changed, 164 insertions(+), 1 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> index 47f3be4..352ba45 100644
> --- a/crypto/asymmetric_keys/rsa.c
> +++ b/crypto/asymmetric_keys/rsa.c
> @@ -13,6 +13,7 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/slab.h>
> +#include <crypto/hash.h>
> #include "public_key.h"
> #include "private_key.h"
>
> @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> }
>
> /*
> + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> + * @M: message to be signed, an octet string
> + * @emLen: intended length in octets of the encoded message
> + * @hash_algo: hash function (option)
> + * @hash: true means hash M, otherwise M is already a digest
> + * @EM: encoded message, an octet string of length emLen
> + *
> + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> + *
> + * The variables used in this function accord PKCS#1 spec but not follow kernel
> + * naming convention, it useful when look at them with spec.
> + */
> +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> + enum pkey_hash_algo hash_algo, const bool hash,
> + u8 **EM, struct public_key_signature *pks)
> +{
> + u8 *digest;
> + struct crypto_shash *tfm;
> + struct shash_desc *desc;
> + size_t digest_size, desc_size;
> + size_t tLen;
> + u8 *T, *PS, *EM_tmp;
> + int i, ret;
> +
> + pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> +
> + if (!RSA_ASN1_templates[hash_algo].data)
> + ret = -ENOTSUPP;

...

> + else
> + pks->pkey_hash_algo = hash_algo;
> +
> + /* 1) Apply the hash function to the message M to produce a hash value H */
> + tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> + if (IS_ERR(tfm))
> + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> +
> + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> + digest_size = crypto_shash_digestsize(tfm);
> +
> + ret = -ENOMEM;

The earlier "ret = -ENOTSUPP;" is either unused because you return at the IS_ERR,
or unused because you overwrite it here. I'm a little disappointed that
the compiler didn't recognise that something was assigned to a value that
is never used.

Phil

> +
> + digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
> + if (!digest)
> + goto error_digest;
> + pks->digest = digest;
> + pks->digest_size = digest_size;
> +
> + if (hash) {
> + 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;
> + ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> + if (ret < 0)
> + goto error_shash;
> + } else {
> + memcpy(pks->digest, M, pks->digest_size);
> + pks->digest_size = digest_size;
> + }
> + crypto_free_shash(tfm);
> +
> + /* 2) Encode the algorithm ID for the hash function and the hash value into
> + * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> + * the DigestInfo value and let tLen be the length in octets of T.
> + */
> + tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> + T = kmalloc(tLen, GFP_KERNEL);
> + if (!T)
> + goto error_T;
> +
> + memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> + memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> +
> + /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> + if (emLen < tLen + 11) {
> + ret = -EINVAL;
> + goto error_emLen;
> + }
> +
> + /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> + PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> + if (!PS)
> + goto error_P;
> +
> + for (i = 0; i < (emLen - tLen - 3); i++)
> + PS[i] = 0xff;
> +
> + /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> + * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> + */
> + EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> + if (!EM_tmp)
> + goto error_EM;
> +
> + EM_tmp[0] = 0x00;
> + EM_tmp[1] = 0x01;
> + memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> + EM_tmp[2 + emLen - tLen - 3] = 0x00;
> + memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> +
> + *EM = EM_tmp;
> +
> + kfree(PS);
> + kfree(T);
> +
> + return 0;
> +
> +error_EM:
> + kfree(PS);
> +error_P:
> +error_emLen:
> + kfree(T);
> +error_T:
> +error_shash:
> + kfree(digest);
> +error_digest:
> + crypto_free_shash(tfm);
> + return ret;
> +}
> +
> +/*
> * Perform the RSA signature verification.
> * @H: Value of hash of data and metadata
> * @EM: The computed signature value
> @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> const struct private_key *key, u8 *M,
> enum pkey_hash_algo hash_algo, const bool hash)
> {
> + struct public_key_signature *pks;
> + u8 *EM = NULL;
> + size_t emLen;
> + int ret;
> +
> pr_info("RSA_generate_signature start\n");
>
> - return 0;
> + ret = -ENOMEM;
> + pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> + if (!pks)
> + goto error_no_pks;
> +
> + /* 1): EMSA-PKCS1-v1_5 encoding: */
> + /* Use the private key modulus size to be EM length */
> + emLen = mpi_get_nbits(key->rsa.n);
> + emLen = (emLen + 7) / 8;
> +
> + ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> + if (ret < 0)
> + goto error_v1_5_encode;
> +
> + /* TODO 2): m = OS2IP (EM) */
> +
> + /* TODO 3): s = RSASP1 (K, m) */
> +
> + /* TODO 4): S = I2OSP (s, k) */
> +
> + /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> + pks->S = EM; /* TODO: temporary set S to EM */
> +
> + return pks;
> +
> +error_v1_5_encode:
> + kfree(pks);
> +error_no_pks:
> + pr_info("<==%s() = %d\n", __func__, ret);
> + return ERR_PTR(ret);
> }
>
> const struct public_key_algorithm RSA_public_key_algorithm = {
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index d44b29f..1cdf457 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> struct public_key_signature {
> u8 *digest;
> u8 digest_size; /* Number of bytes in digest */
> + u8 *S; /* signature S of length k octets */
> + size_t k; /* length k of signature S */
> u8 nr_mpi; /* Occupancy of mpi[] */
> enum pkey_hash_algo pkey_hash_algo : 8;
> union {
> --
> 1.6.0.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>

2013-09-25 21:05:16

by David Howells

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot


I have pushed some keyrings patches that will likely affect this to:

http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel

I intend to ask James to pull these into his next branch. If he's happy to do
so, I can look at pulling at least your asymmetric keys patch on top of them.

It'd be helpful if you could see if you need to make any updates.

David

2013-09-25 21:25:41

by Alan Stern

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Wed, 25 Sep 2013, David Howells wrote:

> I have pushed some keyrings patches that will likely affect this to:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
>
> I intend to ask James to pull these into his next branch. If he's happy to do
> so, I can look at pulling at least your asymmetric keys patch on top of them.

This suggests a point that I raised at the Linux Plumbers conference:

Why are asymmetric keys used for verifying the hibernation image? It
seems that a symmetric key would work just as well. And it would be a
lot quicker to generate, because it wouldn't need any high-precision
integer computations.

Alan Stern

2013-09-25 22:17:01

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
> On Wed, 25 Sep 2013, David Howells wrote:
>
> > I have pushed some keyrings patches that will likely affect this to:
> >
> > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> >
> > I intend to ask James to pull these into his next branch. If he's happy to do
> > so, I can look at pulling at least your asymmetric keys patch on top of them.
>
> This suggests a point that I raised at the Linux Plumbers conference:
>
> Why are asymmetric keys used for verifying the hibernation image? It
> seems that a symmetric key would work just as well. And it would be a
> lot quicker to generate, because it wouldn't need any high-precision
> integer computations.

The reason is the desire to validate that the previous kernel created
something which it passed on to the current kernel (in this case, the
hibernation image) untampered with. To do that, something must be
passed to the prior kernel that can be validated but *not* recreated by
the current kernel.

The scheme for doing this is a public/private key pair generated for
each boot incarnation N as a pair P_N (public key) and K_N (private
key). Then the Nth boot incarnation gets P_{N-1} and K_N (the boot
environment holds P_N in inaccessible BS variables for passing into the
next kernel) so the Nth kernel can validate information from the N-1th
kernel using P_{N-1} and create information for passing on in a
validated fashion to the next kernel using K_N.

This scheme doesn't work with symmetric keys unless you have a
modification I haven't seen?

James


2013-09-26 00:27:34

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Wed 2013-09-25 15:16:54, James Bottomley wrote:
> On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
> > On Wed, 25 Sep 2013, David Howells wrote:
> >
> > > I have pushed some keyrings patches that will likely affect this to:
> > >
> > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> > >
> > > I intend to ask James to pull these into his next branch. If he's happy to do
> > > so, I can look at pulling at least your asymmetric keys patch on top of them.
> >
> > This suggests a point that I raised at the Linux Plumbers conference:
> >
> > Why are asymmetric keys used for verifying the hibernation image? It
> > seems that a symmetric key would work just as well. And it would be a
> > lot quicker to generate, because it wouldn't need any high-precision
> > integer computations.
>
> The reason is the desire to validate that the previous kernel created
> something which it passed on to the current kernel (in this case, the
> hibernation image) untampered with. To do that, something must be
> passed to the prior kernel that can be validated but *not* recreated by
> the current kernel.

I don't get this. Why is it important that current kernel can't
recreate the signature?

Current kernel is not considered malicious (if it were, you have worse
problems).

Pavel

PS: And yes, it would be nice to have
Documentation/power/swsusp-uefi.txt (or something) explaining the
design.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-26 01:11:57

by Alan Stern

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Wed, 25 Sep 2013, James Bottomley wrote:

> > Why are asymmetric keys used for verifying the hibernation image? It
> > seems that a symmetric key would work just as well. And it would be a
> > lot quicker to generate, because it wouldn't need any high-precision
> > integer computations.
>
> The reason is the desire to validate that the previous kernel created
> something which it passed on to the current kernel (in this case, the
> hibernation image) untampered with. To do that, something must be
> passed to the prior kernel that can be validated but *not* recreated by
> the current kernel.

As Pavel pointed out, this seems like a futile approach. If the
current kernel is going to do the validating, then of course it can
create something that it will validate.

Or to put it another way, how come you don't trust the current kernel
not to modify the image but you do trust it to validate the image?

> The scheme for doing this is a public/private key pair generated for
> each boot incarnation N as a pair P_N (public key) and K_N (private
> key). Then the Nth boot incarnation gets P_{N-1} and K_N (the boot

Where does it get them from? Some place in the firmware, presumably.

> environment holds P_N in inaccessible BS variables for passing into the
> next kernel) so the Nth kernel can validate information from the N-1th
> kernel using P_{N-1} and create information for passing on in a
> validated fashion to the next kernel using K_N.

So kernel N gets P_{N-1} and an image that has been signed by K_{N-1}.
What's to prevent kernel N from creating a bogus pair of keys (K',P')
and a bogus image, signing that image with K', and then pretending it
got P' from the firmware instead of P_{N-1}?

However... Let's assume that you _do_ trust kernel N. Then consider
this alternative approach:

A symmetric key S_N is created for boot incarnation N. Kernel N
receives S_{N-1} from the firmware and uses it to verify the signature
attached to the hibernation image. When kernel N wants to create the
next hibernation image, it signs the image with S_N (also obtained
from the firmware).

> This scheme doesn't work with symmetric keys unless you have a
> modification I haven't seen?

Obviously these two schemes are different. Do these differences have
any security implications?

Alan Stern

2013-09-26 01:36:53

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 三,2013-09-25 於 22:04 +0100,David Howells 提到:
> I have pushed some keyrings patches that will likely affect this to:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
>

Thanks for your point out, I will respin my asymmetric keys patch base
on this tree.

> I intend to ask James to pull these into his next branch. If he's happy to do
> so, I can look at pulling at least your asymmetric keys patch on top of them.
>
> It'd be helpful if you could see if you need to make any updates.
>
> David
>

In LPC, Alan and Vojtech raised another thinking is using symmetric key
to protect the hash of snapshot. It's simpler then using RSA private key
to sign it.

Even finally we use the symmetric key solution, I will still respin and
resent the patch for add the leading zero byte:

[PATCH V4 07/15] asymmetric keys: explicitly add the leading zero byte
to encoded message

I think keys-devel tree need it.


Thanks a lot!
Joey Lee

2013-09-26 01:43:42

by joeyli

[permalink] [raw]
Subject: Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

於 三,2013-09-18 於 15:45 +0200,Pavel Machek 提到:
> On Sun 2013-09-15 08:56:59, 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.
>
> This series is big enough already... and who is going to test it?

The hash config not just for testing, it's relate to the performance and
secure between different hash algorithms.

There have person raised in LPC say he don't like SHA algorithm.

> There's no need to make hash configurable. Just select one that works.
>
> Pavel
>

SHA1 has good performance, and SHA512 has better security, which one you
like it?


Thanks a lot!
Joey Lee


2013-09-26 02:19:22

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 三,2013-09-25 於 17:25 -0400,Alan Stern 提到:
> On Wed, 25 Sep 2013, David Howells wrote:
>
> > I have pushed some keyrings patches that will likely affect this to:
> >
> > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> >
> > I intend to ask James to pull these into his next branch. If he's happy to do
> > so, I can look at pulling at least your asymmetric keys patch on top of them.
>
> This suggests a point that I raised at the Linux Plumbers conference:
>
> Why are asymmetric keys used for verifying the hibernation image? It
> seems that a symmetric key would work just as well. And it would be a
> lot quicker to generate, because it wouldn't need any high-precision
> integer computations.
>
> Alan Stern
>
>

Per my understood, it's like add salt to snapshot when generate
signature, then remove the salt when store the snapshot to swap. (or
pass snapshot to userland).

Let me explain the symmetric key solution base on my understand:

+ EFI stub kernel generate a hash value from a random seed, then store
it to EFi boot varaible. It should protected by UEFI secure boot
environment.

+ When hibernate launched:
- Kernel create the snapshot image of memory. It's included the
random hash value(salt) that generated in EFI stub stage.
- Then kernel hash the snapshot image, put the hash to snapshot
header, just like current asymmetric keys solution.
- Kernel erase the salt in snapshot image before it go to swap or
pass to userspace tool.

+ When hibernate resume:
- Kernel or userspace tool load the snapshot(without salt) from swap
to temporary memory space.
- Kernel fill the salt back to snapshot image in memory, hash it.
- Kernel compare the hash with the hash that put in snapshot header.
- Verification done! The follow-up action as current solution.

Please current me if I missed anything.


Thanks a lot!
Joey Lee

2013-09-26 02:33:08

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, 2013-09-26 at 02:27 +0200, Pavel Machek wrote:
> On Wed 2013-09-25 15:16:54, James Bottomley wrote:
> > On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
> > > On Wed, 25 Sep 2013, David Howells wrote:
> > >
> > > > I have pushed some keyrings patches that will likely affect this to:
> > > >
> > > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> > > >
> > > > I intend to ask James to pull these into his next branch. If he's happy to do
> > > > so, I can look at pulling at least your asymmetric keys patch on top of them.
> > >
> > > This suggests a point that I raised at the Linux Plumbers conference:
> > >
> > > Why are asymmetric keys used for verifying the hibernation image? It
> > > seems that a symmetric key would work just as well. And it would be a
> > > lot quicker to generate, because it wouldn't need any high-precision
> > > integer computations.
> >
> > The reason is the desire to validate that the previous kernel created
> > something which it passed on to the current kernel (in this case, the
> > hibernation image) untampered with. To do that, something must be
> > passed to the prior kernel that can be validated but *not* recreated by
> > the current kernel.
>
> I don't get this. Why is it important that current kernel can't
> recreate the signature?

The thread model is an attack on the saved information (i.e. the suspend
image) between it being saved by the old kernel and used by the new one.
The important point isn't that the new kernel doesn't have access to
K_{N-1} it's that no-one does: the key is destroyed as soon as the old
kernel terminates however the verification public part P_{N-1} survives.

James

> Current kernel is not considered malicious (if it were, you have worse
> problems).
>
> Pavel
>
> PS: And yes, it would be nice to have
> Documentation/power/swsusp-uefi.txt (or something) explaining the
> design.
>


2013-09-26 04:40:10

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 四,2013-09-26 於 02:27 +0200,Pavel Machek 提到:
> On Wed 2013-09-25 15:16:54, James Bottomley wrote:
> > On Wed, 2013-09-25 at 17:25 -0400, Alan Stern wrote:
> > > On Wed, 25 Sep 2013, David Howells wrote:
> > >
> > > > I have pushed some keyrings patches that will likely affect this to:
> > > >
> > > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> > > >
> > > > I intend to ask James to pull these into his next branch. If he's happy to do
> > > > so, I can look at pulling at least your asymmetric keys patch on top of them.
> > >
> > > This suggests a point that I raised at the Linux Plumbers conference:
> > >
> > > Why are asymmetric keys used for verifying the hibernation image? It
> > > seems that a symmetric key would work just as well. And it would be a
> > > lot quicker to generate, because it wouldn't need any high-precision
> > > integer computations.
> >
> > The reason is the desire to validate that the previous kernel created
> > something which it passed on to the current kernel (in this case, the
> > hibernation image) untampered with. To do that, something must be
> > passed to the prior kernel that can be validated but *not* recreated by
> > the current kernel.
>
> I don't get this. Why is it important that current kernel can't
> recreate the signature?
>
> Current kernel is not considered malicious (if it were, you have worse
> problems).
>

Current boot kernel should not malicious especially when UEFI secure
boot enabled.

> Pavel
>
> PS: And yes, it would be nice to have
> Documentation/power/swsusp-uefi.txt (or something) explaining the
> design.
>

Thanks for your suggestion, I will write the swsusp-uefi.txt to
explaining the design in next version.


Thanks a lot!
Joey Lee

2013-09-26 06:24:54

by Jiri Kosina

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Wed, 25 Sep 2013, James Bottomley wrote:

> > I don't get this. Why is it important that current kernel can't
> > recreate the signature?
>
> The thread model is an attack on the saved information (i.e. the suspend
> image) between it being saved by the old kernel and used by the new one.
> The important point isn't that the new kernel doesn't have access to
> K_{N-1} it's that no-one does: the key is destroyed as soon as the old
> kernel terminates however the verification public part P_{N-1} survives.

James,

could you please describe the exact scenario you think that the symmetric
keys aproach doesn't protect against, while the assymetric key aproach
does?

The crucial points, which I believe make the symmetric key aproach work
(and I feel quite embarassed by the fact that I haven't realized this
initially when coming up with the assymetric keys aproach) are:

- the kernel that is performing the actual resumption is trusted in the
secure boot model, i.e. you trust it to perform proper verification

- potentially malicious userspace (which is what we are protecting against
-- malicious root creating fake hibernation image and issuing reboot)
doesn't have access to the symmetric key

--
Jiri Kosina
SUSE Labs

2013-09-26 07:08:26

by joeyli

[permalink] [raw]
Subject: Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa

Hi Phil,

First! Thanks for your time to review my patch!

於 一,2013-09-23 於 19:49 +0300,Phil Carmody 提到:
> On Sun, Sep 15, 2013 at 08:56:48AM +0800, Lee, Chun-Yi wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
>
> You're now at V4.

The V4 is for whole patchset, I didn't do any modify in this patch in
this version.
The version define maybe confuse between separate and whole patchset, I
will avoid it.

>
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <[email protected]>
> > Reviewed-by: Jiri Kosina <[email protected]>
> > Signed-off-by: Lee, Chun-Yi <[email protected]>
> > ---
> > crypto/asymmetric_keys/rsa.c | 163 +++++++++++++++++++++++++++++++++++++++++-
> > include/crypto/public_key.h | 2 +
> > 2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> > #include <linux/module.h>
> > #include <linux/kernel.h>
> > #include <linux/slab.h>
> > +#include <crypto/hash.h>
> > #include "public_key.h"
> > #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> > }
> >
> > /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > + enum pkey_hash_algo hash_algo, const bool hash,
> > + u8 **EM, struct public_key_signature *pks)
> > +{
> > + u8 *digest;
> > + struct crypto_shash *tfm;
> > + struct shash_desc *desc;
> > + size_t digest_size, desc_size;
> > + size_t tLen;
> > + u8 *T, *PS, *EM_tmp;
> > + int i, ret;
> > +
> > + pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > + if (!RSA_ASN1_templates[hash_algo].data)
> > + ret = -ENOTSUPP;
>
> ...
>
> > + else
> > + pks->pkey_hash_algo = hash_algo;
> > +
> > + /* 1) Apply the hash function to the message M to produce a hash value H */
> > + tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > + if (IS_ERR(tfm))
> > + return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > + desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > + digest_size = crypto_shash_digestsize(tfm);
> > +
> > + ret = -ENOMEM;
>
> The earlier "ret = -ENOTSUPP;" is either unused because you return at the IS_ERR,
> or unused because you overwrite it here. I'm a little disappointed that
> the compiler didn't recognise that something was assigned to a value that
> is never used.
>
> Phil

Yes, Dmitry also pointed out this issue, I should not go on the hash
process if the hash algorithm didn't support.

I will change fix this problem in next version.


Thanks a lot!
Joey Lee

2013-09-26 08:21:52

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm

Hi!

> > On Sun 2013-09-15 08:56:59, 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.
> >
> > This series is big enough already... and who is going to test it?
>
> The hash config not just for testing, it's relate to the performance and
> secure between different hash algorithms.

I'm not saying it is for testing. I'm saying that selection makes
testing harder.

> There have person raised in LPC say he don't like SHA algorithm.

Well, I don't like the config option.

> > There's no need to make hash configurable. Just select one that works.
>
> SHA1 has good performance, and SHA512 has better security, which one you
> like it?

Use SHA1. It is completely adequate for what you are trying to do.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-26 10:44:14

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 四,2013-09-26 於 10:19 +0800,joeyli 提到:
> 於 三,2013-09-25 於 17:25 -0400,Alan Stern 提到:
> > On Wed, 25 Sep 2013, David Howells wrote:
> >
> > > I have pushed some keyrings patches that will likely affect this to:
> > >
> > > http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-devel
> > >
> > > I intend to ask James to pull these into his next branch. If he's happy to do
> > > so, I can look at pulling at least your asymmetric keys patch on top of them.
> >
> > This suggests a point that I raised at the Linux Plumbers conference:
> >
> > Why are asymmetric keys used for verifying the hibernation image? It
> > seems that a symmetric key would work just as well. And it would be a
> > lot quicker to generate, because it wouldn't need any high-precision
> > integer computations.
> >
> > Alan Stern
> >
> >
>
> Per my understood, it's like add salt to snapshot when generate
> signature, then remove the salt when store the snapshot to swap. (or
> pass snapshot to userland).
>
> Let me explain the symmetric key solution base on my understand:
>
> + EFI stub kernel generate a hash value from a random seed, then store
> it to EFi boot varaible. It should protected by UEFI secure boot
> environment.
>
> + When hibernate launched:
> - Kernel create the snapshot image of memory. It's included the
> random hash value(salt) that generated in EFI stub stage.
> - Then kernel hash the snapshot image, put the hash to snapshot
> header, just like current asymmetric keys solution.
> - Kernel erase the salt in snapshot image before it go to swap or
> pass to userspace tool.
>
> + When hibernate resume:
> - Kernel or userspace tool load the snapshot(without salt) from swap
> to temporary memory space.
> - Kernel fill the salt back to snapshot image in memory, hash it.
> - Kernel compare the hash with the hash that put in snapshot header.
> - Verification done! The follow-up action as current solution.
>
> Please current me if I missed anything.
>
>
> Thanks a lot!
> Joey Lee
>

For the symmetric key solution, I will try HMAC (Hash Message
Authentication Code). It's already used in networking, hope the
performance is not too bad to a big image.


Thanks
Joey Lee

2013-09-26 12:06:29

by Pavel Machek

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

Hi!

> For the symmetric key solution, I will try HMAC (Hash Message
> Authentication Code). It's already used in networking, hope the
> performance is not too bad to a big image.

Kernel already supports crc32 of the hibernation image, you may want
to take a look how that is done.

Maybe you want to replace crc32 with cryptographics hash (sha1?) and
then use only hash for more crypto? That way speed of whatever crypto
you do should not be an issue.

Actually...

Is not it as simple as storing hash of hibernation image into NVRAM
and then verifying the hash matches the value in NVRAM on next
startup? No encryption needed.

And that may even be useful for non-secure-boot people, as it ensures
you boot right image after resume, boot it just once, etc...

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2013-09-26 12:21:39

by Michal Marek

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On 26.9.2013 14:06, Pavel Machek wrote:
> Actually...
>
> Is not it as simple as storing hash of hibernation image into NVRAM
> and then verifying the hash matches the value in NVRAM on next
> startup? No encryption needed.

I think that part of the exercise is to minimize the number of writes to
the NVRAM. The hash changes with every hibernation, obviously.

Michal

2013-09-26 12:22:15

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, Sep 26, 2013 at 02:06:21PM +0200, Pavel Machek wrote:

> > For the symmetric key solution, I will try HMAC (Hash Message
> > Authentication Code). It's already used in networking, hope the
> > performance is not too bad to a big image.
>
> Kernel already supports crc32 of the hibernation image, you may want
> to take a look how that is done.
>
> Maybe you want to replace crc32 with cryptographics hash (sha1?) and
> then use only hash for more crypto? That way speed of whatever crypto
> you do should not be an issue.

Well, yes, one could skip the CRC when the signing is enabled to gain a
little speedup.

> Actually...
>
> Is not it as simple as storing hash of hibernation image into NVRAM
> and then verifying the hash matches the value in NVRAM on next
> startup? No encryption needed.

First, there is no encryption going on. Only doing a HMAC (digest (hash)
using a key) of the image.

Second, since NVRAM is accessible through efivarsfs, storing the hash in
NVRAM wouldn't prevent an attacker from modifying the hash to match a
modified image.

There is a reason why the key for the HMAC is stored in the NVRAM in a
BootServices variable that isn't accessible from the OS and is
write-protected on hardware level from the OS.

> And that may even be useful for non-secure-boot people, as it ensures
> you boot right image after resume, boot it just once, etc...

The HMAC approach isn't much more complicated, and it gives you all
these benefits even with secure boot disabled.

--
Vojtech Pavlik
Director SUSE Labs

2013-09-26 12:23:07

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, Sep 26, 2013 at 02:21:23PM +0200, Michal Marek wrote:

> > Is not it as simple as storing hash of hibernation image into NVRAM
> > and then verifying the hash matches the value in NVRAM on next
> > startup? No encryption needed.
>
> I think that part of the exercise is to minimize the number of writes to
> the NVRAM. The hash changes with every hibernation, obviously.

The key should, too.

--
Vojtech Pavlik
Director SUSE Labs

2013-09-26 12:56:25

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 四,2013-09-26 於 14:06 +0200,Pavel Machek 提到:
> Hi!
>
> > For the symmetric key solution, I will try HMAC (Hash Message
> > Authentication Code). It's already used in networking, hope the
> > performance is not too bad to a big image.
>
> Kernel already supports crc32 of the hibernation image, you may want
> to take a look how that is done.

In current kernel design, The crc32 is only for the LZO in-kernel
hibernate, doesn't apply to non-compress hibernate and userspace
hibernate.

Put signature to snapshot header can support any kind of caller that's
trigger hibernate. Any userspace hibernate tool will take the snapshot
image from kernel, so, we need put the signature(or hash result) to
snapshot header before userspace write it to anywhere.

>
> Maybe you want to replace crc32 with cryptographics hash (sha1?) and
> then use only hash for more crypto? That way speed of whatever crypto
> you do should not be an issue.

That speed of hash is calculated from non-compress snapshot image, does
not overlap with crc32.

>
> Actually...
>
> Is not it as simple as storing hash of hibernation image into NVRAM
> and then verifying the hash matches the value in NVRAM on next
> startup? No encryption needed.
>
> And that may even be useful for non-secure-boot people, as it ensures
> you boot right image after resume, boot it just once, etc...
>
> Pavel

The HMAC approach will not encrypt, just put the key of HMAC to boottime
variable.

If user doesn't enable UEFI secure boot, that's fine, the key of HMAC
still cannot access in OS runtime.
If user enable UEFI secure boot, then that's better! Because all EFI
file will signed by the manufacturers or OSVs to make sure the code is
secure, will not pass the key to runtime.


Thanks a lot!
Joey Lee

2013-09-26 13:21:08

by joeyli

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

於 四,2013-09-26 於 14:22 +0200,Vojtech Pavlik 提到:
> On Thu, Sep 26, 2013 at 02:06:21PM +0200, Pavel Machek wrote:
>
> > > For the symmetric key solution, I will try HMAC (Hash Message
> > > Authentication Code). It's already used in networking, hope the
> > > performance is not too bad to a big image.
> >
> > Kernel already supports crc32 of the hibernation image, you may want
> > to take a look how that is done.
> >
> > Maybe you want to replace crc32 with cryptographics hash (sha1?) and
> > then use only hash for more crypto? That way speed of whatever
> crypto
> > you do should not be an issue.
>
> Well, yes, one could skip the CRC when the signing is enabled to gain
> a
> little speedup.

In current kernel, CRC is for check the integrity of LZO compressed
image, the purpose is different to check the integrity of snapshot
image.
So, CRC will not in non-compress hibernate or userspace hibernate code
path

On the other hand, attacker can easily change the CRC code in the header
of LZO hibernate image.

Thanks a lot!
Joey Lee

2013-09-26 14:44:20

by James Bottomley

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, 2013-09-26 at 08:24 +0200, Jiri Kosina wrote:
> On Wed, 25 Sep 2013, James Bottomley wrote:
>
> > > I don't get this. Why is it important that current kernel can't
> > > recreate the signature?
> >
> > The thread model is an attack on the saved information (i.e. the suspend
> > image) between it being saved by the old kernel and used by the new one.
> > The important point isn't that the new kernel doesn't have access to
> > K_{N-1} it's that no-one does: the key is destroyed as soon as the old
> > kernel terminates however the verification public part P_{N-1} survives.
>
> James,
>
> could you please describe the exact scenario you think that the symmetric
> keys aproach doesn't protect against, while the assymetric key aproach
> does?
>
> The crucial points, which I believe make the symmetric key aproach work
> (and I feel quite embarassed by the fact that I haven't realized this
> initially when coming up with the assymetric keys aproach) are:
>
> - the kernel that is performing the actual resumption is trusted in the
> secure boot model, i.e. you trust it to perform proper verification
>
> - potentially malicious userspace (which is what we are protecting against
> -- malicious root creating fake hibernation image and issuing reboot)
> doesn't have access to the symmetric key

OK, so the scheme is to keep a symmetric key in BS that is passed into
the kernel each time (effectively a secret key) for signing and
validation?

The only two problems I see are

1. The key isn't generational (any compromise obtains it). This
can be fixed by using a set of keys generated on each boot and
passing in both K_{N-1} and K_N
2. No external agency other than the next kernel can do the
validation since the validating key has to be secret

The importance of 2 is just tripwire like detection ... perhaps it
doesn't really matter in a personal computer situation. It would matter
in an enterprise where images are stored and re-used but until servers
have UEFI secure boot, that's not going to be an issue.

James


2013-09-26 14:48:13

by Jiri Kosina

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, 26 Sep 2013, James Bottomley wrote:

> > could you please describe the exact scenario you think that the symmetric
> > keys aproach doesn't protect against, while the assymetric key aproach
> > does?
> >
> > The crucial points, which I believe make the symmetric key aproach work
> > (and I feel quite embarassed by the fact that I haven't realized this
> > initially when coming up with the assymetric keys aproach) are:
> >
> > - the kernel that is performing the actual resumption is trusted in the
> > secure boot model, i.e. you trust it to perform proper verification
> >
> > - potentially malicious userspace (which is what we are protecting against
> > -- malicious root creating fake hibernation image and issuing reboot)
> > doesn't have access to the symmetric key
>
> OK, so the scheme is to keep a symmetric key in BS that is passed into
> the kernel each time (effectively a secret key) for signing and
> validation?

Exactly.

> The only two problems I see are
>
> 1. The key isn't generational (any compromise obtains it). This
> can be fixed by using a set of keys generated on each boot and
> passing in both K_{N-1} and K_N

I think this could be easily made optional, leaving the user with choice
of faster or "safer" boot.

> 2. No external agency other than the next kernel can do the
> validation since the validating key has to be secret

This is true, but as you said, the relevance of this seems to be rather
questionable.

--
Jiri Kosina
SUSE Labs

2013-09-26 14:56:42

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Thu, Sep 26, 2013 at 04:48:00PM +0200, Jiri Kosina wrote:

> > The only two problems I see are
> >
> > 1. The key isn't generational (any compromise obtains it). This
> > can be fixed by using a set of keys generated on each boot and
> > passing in both K_{N-1} and K_N
>
> I think this could be easily made optional, leaving the user with choice
> of faster or "safer" boot.

Ideally, the key should be regenerated on each true reboot and kept the
same if it is just a resume. Unfortunately, I don't see a way to
distinguish those before we call ExitBootServices().

The reasoning behind that is that in the case of a kernel compromise, a
suspended-and-resumed kernel will still be compromised, so there is no
value in passing it a new key. A freshly booted kernel, though, should
get a new key, exactly because the attacker could have obtained a key
from the previous, compromised one.

This speeds up the ususal suspend-and-resume cycle, but provides full
security once the user performs a full reboot.

The question that remains is how to tell in advance.

> > 2. No external agency other than the next kernel can do the
> > validation since the validating key has to be secret
>
> This is true, but as you said, the relevance of this seems to be rather
> questionable.

Indeed, it's hard to imagine a scenario that is also valid within the
secure boot threat model.

--
Vojtech Pavlik
Director SUSE Labs

2013-10-17 14:07:06

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [RFC V4 PATCH 00/15] Signature verification of hibernate snapshot

On Sunday, September 15, 2013 08:56:46 AM Lee, Chun-Yi wrote:
> 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 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.

I wonder what the status of this work is? Is it considered ready for inclusion
or are you still going to work on it and resubmit?

Rafael