LinuxLists.cc - [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1

2013-09-15 00:56:48

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 <jlee-IBi9RG/[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-17 21:51:15

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:29:55

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:35

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 09:08:00

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 <jlee-IBi9RG/[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 09:08:00

by joeyli

[permalink] [raw]

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

2013-09-18 09:08:00

by joeyli

[permalink] [raw]

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

2013-09-18 09:08:00

by joeyli

[permalink] [raw]

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

2013-09-23 16:49:44

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-26 07:08:30

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 <jlee-IBi9RG/[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

--
To unsubscribe, e-mail: opensuse-kernel+unsubscribe-stAJ6ESoqRxg9hUCZPvPmw@public.gmane.org
To contact the owner, e-mail: [email protected]