Re: [PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-05-01 Thread Tadeusz Struk 
On 05/01/2015 09:21 AM, David Howells wrote:
>> +.verify = RSA_verify_signature,
>> > +  .capabilities = PKEY_CAN_VERIFY,
> Can we keep .verify_signature as the name of the first.  The second is
> redundant given the function pointers.

I'm thinking that .verify will be more generic. If in the future
we would like to implement something that verifies not a signature, but
for instance is a number is a prime, then we can register a "prime" alg
that implements verify and returns true if a number is a prime.   

> 
> Given that X.509 certs can hang around for a very long time, having a tfm in
> the cert is probably a bad idea as it may pin resources such as crypto h/w.
> 
>> > -  ctx->cert->pub->pkey_algo = PKEY_ALGO_RSA;
>> > -
> I think you need this rather than the above.  You should only get the tfm when
> you actually need it.
> 

That's a good point.
Thank you David for all your comments. I'll rework my patches and send v2 soon.
I'll also try to integrate it with your sign-file as you suggested.
Thanks
T
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-05-01 Thread David Howells 
Tadeusz Struk  wrote:

> +Additionally public key algorithm names are defined:
> +#define PKEY_ALGO_DSA "dsa"
> +#define PKEY_ALGO_RSA "rsa"
> +These will be used to allocate public key tfm instances.

These should be a blank line either side of the two #defines and the #defines
should be indented a tab.

> - BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA);
> + BUG_ON(strcmp(ctx->sinfo->sig.pkey_algo, PKEY_ALGO_RSA));

If you make PKEY_ALGO_RSA a const char [] can you use != here?

Oh, and can you do either != 0 or == 0 on the end of your strcmp()?  It's a
bit more obvious since strcmp()'s return is sort of inverse.

> + .verify = RSA_verify_signature,
> + .capabilities = PKEY_CAN_VERIFY,

Can we keep .verify_signature as the name of the first.  The second is
redundant given the function pointers.

> + if (cert->pub && !IS_ERR(cert->pub->tfm))
> + crypto_free_pke(cert->pub->tfm);
> ...
> +
> + cert->pub->tfm = crypto_alloc_pke(ctx->cert->sig.pkey_algo, 0, 0);
> + if (IS_ERR(cert->pub->tfm)) {
> + pr_err("Failed to alloc pkey algo %s\n",
> +ctx->cert->sig.pkey_algo);
> + goto error_decode;
> + }
> +

Given that X.509 certs can hang around for a very long time, having a tfm in
the cert is probably a bad idea as it may pin resources such as crypto h/w.

> - ctx->cert->pub->pkey_algo = PKEY_ALGO_RSA;
> -

I think you need this rather than the above.  You should only get the tfm when
you actually need it.

> - pr_devel("Cert Key Algo: %s\n", pkey_algo_name[cert->pub->pkey_algo]);
> + pr_devel("Cert Key Algo: %s\n", pke_alg_name(cert->pub->tfm));

pkey_algo_name() perhaps?

> + pr_devel("Cert Signature: %s + %s\n", cert->sig.pkey_algo,

Split line at that comma please.  That way all the arguments line up.

> - cert->pub->algo = pkey_algo[cert->pub->pkey_algo];

Might still need this.

> -enum pkey_algo {
> - PKEY_ALGO_DSA,
> - PKEY_ALGO_RSA,
> - PKEY_ALGO__LAST
> -};

This represents a value seen external to the kernel - at least for the
moment.  Switching to PKCS#7 module sigs would cure that.

> +#define PKEY_ALGO_DSA "dsa"
> +#define PKEY_ALGO_RSA "rsa"

const char []

> +int public_key_verify_signature(const struct public_key *pk,
> + const struct public_key_signature *sig);

Retain the extern please and the following blank line.

> +static const char *const pkey_algo_name[] = {
> + PKEY_ALGO_DSA, PKEY_ALGO_RSA
> +};
> +

Split the list over multiple lines, please.  Better still, move to PKCS#7.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-05-01 Thread David Howells 
Tadeusz Struk tadeusz.st...@intel.com wrote:

 +Additionally public key algorithm names are defined:
 +#define PKEY_ALGO_DSA dsa
 +#define PKEY_ALGO_RSA rsa
 +These will be used to allocate public key tfm instances.

These should be a blank line either side of the two #defines and the #defines
should be indented a tab.

 - BUG_ON(ctx-sinfo-sig.pkey_algo != PKEY_ALGO_RSA);
 + BUG_ON(strcmp(ctx-sinfo-sig.pkey_algo, PKEY_ALGO_RSA));

If you make PKEY_ALGO_RSA a const char [] can you use != here?

Oh, and can you do either != 0 or == 0 on the end of your strcmp()?  It's a
bit more obvious since strcmp()'s return is sort of inverse.

 + .verify = RSA_verify_signature,
 + .capabilities = PKEY_CAN_VERIFY,

Can we keep .verify_signature as the name of the first.  The second is
redundant given the function pointers.

 + if (cert-pub  !IS_ERR(cert-pub-tfm))
 + crypto_free_pke(cert-pub-tfm);
 ...
 +
 + cert-pub-tfm = crypto_alloc_pke(ctx-cert-sig.pkey_algo, 0, 0);
 + if (IS_ERR(cert-pub-tfm)) {
 + pr_err(Failed to alloc pkey algo %s\n,
 +ctx-cert-sig.pkey_algo);
 + goto error_decode;
 + }
 +

Given that X.509 certs can hang around for a very long time, having a tfm in
the cert is probably a bad idea as it may pin resources such as crypto h/w.

 - ctx-cert-pub-pkey_algo = PKEY_ALGO_RSA;
 -

I think you need this rather than the above.  You should only get the tfm when
you actually need it.

 - pr_devel(Cert Key Algo: %s\n, pkey_algo_name[cert-pub-pkey_algo]);
 + pr_devel(Cert Key Algo: %s\n, pke_alg_name(cert-pub-tfm));

pkey_algo_name() perhaps?

 + pr_devel(Cert Signature: %s + %s\n, cert-sig.pkey_algo,

Split line at that comma please.  That way all the arguments line up.

 - cert-pub-algo = pkey_algo[cert-pub-pkey_algo];

Might still need this.

 -enum pkey_algo {
 - PKEY_ALGO_DSA,
 - PKEY_ALGO_RSA,
 - PKEY_ALGO__LAST
 -};

This represents a value seen external to the kernel - at least for the
moment.  Switching to PKCS#7 module sigs would cure that.

 +#define PKEY_ALGO_DSA dsa
 +#define PKEY_ALGO_RSA rsa

const char []

 +int public_key_verify_signature(const struct public_key *pk,
 + const struct public_key_signature *sig);

Retain the extern please and the following blank line.

 +static const char *const pkey_algo_name[] = {
 + PKEY_ALGO_DSA, PKEY_ALGO_RSA
 +};
 +

Split the list over multiple lines, please.  Better still, move to PKCS#7.

David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-05-01 Thread Tadeusz Struk 
On 05/01/2015 09:21 AM, David Howells wrote:
 +.verify = RSA_verify_signature,
  +  .capabilities = PKEY_CAN_VERIFY,
 Can we keep .verify_signature as the name of the first.  The second is
 redundant given the function pointers.

I'm thinking that .verify will be more generic. If in the future
we would like to implement something that verifies not a signature, but
for instance is a number is a prime, then we can register a prime alg
that implements verify and returns true if a number is a prime.   

 
 Given that X.509 certs can hang around for a very long time, having a tfm in
 the cert is probably a bad idea as it may pin resources such as crypto h/w.
 
  -  ctx-cert-pub-pkey_algo = PKEY_ALGO_RSA;
  -
 I think you need this rather than the above.  You should only get the tfm when
 you actually need it.
 

That's a good point.
Thank you David for all your comments. I'll rework my patches and send v2 soon.
I'll also try to integrate it with your sign-file as you suggested.
Thanks
T
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-04-30 Thread Tadeusz Struk 
Change the existing rsa and public key code to integrate it
with the new Public Key Encryption API.

Signed-off-by: Tadeusz Struk 
---
 Documentation/crypto/asymmetric-keys.txt  |   10 +++-
 crypto/asymmetric_keys/Kconfig|1 
 crypto/asymmetric_keys/pkcs7_parser.c |4 +-
 crypto/asymmetric_keys/pkcs7_trust.c  |2 -
 crypto/asymmetric_keys/pkcs7_verify.c |5 +-
 crypto/asymmetric_keys/public_key.c   |   73 +
 crypto/asymmetric_keys/public_key.h   |   36 --
 crypto/asymmetric_keys/rsa.c  |   47 +++
 crypto/asymmetric_keys/x509_cert_parser.c |   14 --
 crypto/asymmetric_keys/x509_public_key.c  |   14 ++
 include/crypto/public_key.h   |   24 +++---
 kernel/module_signing.c   |9 +++-
 12 files changed, 124 insertions(+), 115 deletions(-)
 delete mode 100644 crypto/asymmetric_keys/public_key.h

diff --git a/Documentation/crypto/asymmetric-keys.txt 
b/Documentation/crypto/asymmetric-keys.txt
index b767590..47bb5fb 100644
--- a/Documentation/crypto/asymmetric-keys.txt
+++ b/Documentation/crypto/asymmetric-keys.txt
@@ -89,11 +89,9 @@ inclusion is required:
#include 
 
 This gives access to functions for dealing with asymmetric / public keys.
-Three enums are defined there for representing public-key cryptography
+Two enums are defined there for representing public-key cryptography
 algorithms:
 
-   enum pkey_algo
-
 digest algorithms used by those:
 
enum pkey_hash_algo
@@ -102,6 +100,11 @@ and key identifier representations:
 
enum pkey_id_type
 
+Additionally public key algorithm names are defined:
+#define PKEY_ALGO_DSA "dsa"
+#define PKEY_ALGO_RSA "rsa"
+These will be used to allocate public key tfm instances.
+
 Note that the key type representation types are required because key
 identifiers from different standards aren't necessarily compatible.  For
 instance, PGP generates key identifiers by hashing the key data plus some
@@ -131,6 +134,7 @@ transferred the relevant bits to the structure pointed to 
by sig.
 
struct public_key_signature {
u8 *digest;
+   char *pkey_algo;
u8 digest_size;
enum pkey_hash_algo pkey_hash_algo : 8;
u8 nr_mpi;
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 4870f28..1ad10f1 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -23,6 +23,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
 config PUBLIC_KEY_ALGO_RSA
tristate "RSA public-key algorithm"
select MPILIB
+   select CRYPTO_PKE
help
  This option enables support for the RSA algorithm (PKCS#1, RFC3447).
 
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c 
b/crypto/asymmetric_keys/pkcs7_parser.c
index 3bd5a1e..d37a608 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -15,7 +15,7 @@
 #include 
 #include 
 #include 
-#include "public_key.h"
+#include 
 #include "pkcs7_parser.h"
 #include "pkcs7-asn1.h"
 
@@ -376,7 +376,7 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen,
struct pkcs7_parse_context *ctx = context;
MPI mpi;
 
-   BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA);
+   BUG_ON(strcmp(ctx->sinfo->sig.pkey_algo, PKEY_ALGO_RSA));
 
mpi = mpi_read_raw_data(value, vlen);
if (!mpi)
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c 
b/crypto/asymmetric_keys/pkcs7_trust.c
index 1d29376..68ebae2 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -17,7 +17,7 @@
 #include 
 #include 
 #include 
-#include "public_key.h"
+#include 
 #include "pkcs7_parser.h"
 
 /**
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c 
b/crypto/asymmetric_keys/pkcs7_verify.c
index cd45545..28f4a77 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -16,7 +16,7 @@
 #include 
 #include 
 #include 
-#include "public_key.h"
+#include 
 #include "pkcs7_parser.h"
 
 /*
@@ -144,7 +144,8 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
 sinfo->index, certix);
 
-   if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
+   if (strcmp(pke_alg_name(x509->pub->tfm),
+  sinfo->sig.pkey_algo)) {
pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't 
match\n",
sinfo->index);
continue;
diff --git a/crypto/asymmetric_keys/public_key.c 
b/crypto/asymmetric_keys/public_key.c
index 2f6e4fb..031112a 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -18,30 +18,21 @@
 #include 
 #include 
 #include 
-#include "public_key.h"
+#include

[PATCH RFC 2/2] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-04-30 Thread Tadeusz Struk 
Change the existing rsa and public key code to integrate it
with the new Public Key Encryption API.

Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---
 Documentation/crypto/asymmetric-keys.txt  |   10 +++-
 crypto/asymmetric_keys/Kconfig|1 
 crypto/asymmetric_keys/pkcs7_parser.c |4 +-
 crypto/asymmetric_keys/pkcs7_trust.c  |2 -
 crypto/asymmetric_keys/pkcs7_verify.c |5 +-
 crypto/asymmetric_keys/public_key.c   |   73 +
 crypto/asymmetric_keys/public_key.h   |   36 --
 crypto/asymmetric_keys/rsa.c  |   47 +++
 crypto/asymmetric_keys/x509_cert_parser.c |   14 --
 crypto/asymmetric_keys/x509_public_key.c  |   14 ++
 include/crypto/public_key.h   |   24 +++---
 kernel/module_signing.c   |9 +++-
 12 files changed, 124 insertions(+), 115 deletions(-)
 delete mode 100644 crypto/asymmetric_keys/public_key.h

diff --git a/Documentation/crypto/asymmetric-keys.txt 
b/Documentation/crypto/asymmetric-keys.txt
index b767590..47bb5fb 100644
--- a/Documentation/crypto/asymmetric-keys.txt
+++ b/Documentation/crypto/asymmetric-keys.txt
@@ -89,11 +89,9 @@ inclusion is required:
#include crypto/public_key.h
 
 This gives access to functions for dealing with asymmetric / public keys.
-Three enums are defined there for representing public-key cryptography
+Two enums are defined there for representing public-key cryptography
 algorithms:
 
-   enum pkey_algo
-
 digest algorithms used by those:
 
enum pkey_hash_algo
@@ -102,6 +100,11 @@ and key identifier representations:
 
enum pkey_id_type
 
+Additionally public key algorithm names are defined:
+#define PKEY_ALGO_DSA dsa
+#define PKEY_ALGO_RSA rsa
+These will be used to allocate public key tfm instances.
+
 Note that the key type representation types are required because key
 identifiers from different standards aren't necessarily compatible.  For
 instance, PGP generates key identifiers by hashing the key data plus some
@@ -131,6 +134,7 @@ transferred the relevant bits to the structure pointed to 
by sig.
 
struct public_key_signature {
u8 *digest;
+   char *pkey_algo;
u8 digest_size;
enum pkey_hash_algo pkey_hash_algo : 8;
u8 nr_mpi;
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 4870f28..1ad10f1 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -23,6 +23,7 @@ config ASYMMETRIC_PUBLIC_KEY_SUBTYPE
 config PUBLIC_KEY_ALGO_RSA
tristate RSA public-key algorithm
select MPILIB
+   select CRYPTO_PKE
help
  This option enables support for the RSA algorithm (PKCS#1, RFC3447).
 
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c 
b/crypto/asymmetric_keys/pkcs7_parser.c
index 3bd5a1e..d37a608 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -15,7 +15,7 @@
 #include linux/slab.h
 #include linux/err.h
 #include linux/oid_registry.h
-#include public_key.h
+#include crypto/public_key.h
 #include pkcs7_parser.h
 #include pkcs7-asn1.h
 
@@ -376,7 +376,7 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen,
struct pkcs7_parse_context *ctx = context;
MPI mpi;
 
-   BUG_ON(ctx-sinfo-sig.pkey_algo != PKEY_ALGO_RSA);
+   BUG_ON(strcmp(ctx-sinfo-sig.pkey_algo, PKEY_ALGO_RSA));
 
mpi = mpi_read_raw_data(value, vlen);
if (!mpi)
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c 
b/crypto/asymmetric_keys/pkcs7_trust.c
index 1d29376..68ebae2 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -17,7 +17,7 @@
 #include linux/asn1.h
 #include linux/key.h
 #include keys/asymmetric-type.h
-#include public_key.h
+#include crypto/public_key.h
 #include pkcs7_parser.h
 
 /**
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c 
b/crypto/asymmetric_keys/pkcs7_verify.c
index cd45545..28f4a77 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -16,7 +16,7 @@
 #include linux/err.h
 #include linux/asn1.h
 #include crypto/hash.h
-#include public_key.h
+#include crypto/public_key.h
 #include pkcs7_parser.h
 
 /*
@@ -144,7 +144,8 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
pr_devel(Sig %u: Found cert serial match X.509[%u]\n,
 sinfo-index, certix);
 
-   if (x509-pub-pkey_algo != sinfo-sig.pkey_algo) {
+   if (strcmp(pke_alg_name(x509-pub-tfm),
+  sinfo-sig.pkey_algo)) {
pr_warn(Sig %u: X.509 algo and PKCS#7 sig algo don't 
match\n,
sinfo-index);
continue;
diff --git a/crypto/asymmetric_keys/public_key.c 
b/crypto/asymmetric_keys/public_key.c
index 2f6e4fb..031112a 100644
---