Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Herbert Xu]

On Thu, Apr 14, 2016 at 05:38:08PM +0200, Stephan Mueller wrote:
>
> > I don't think this is really needed. memzero_explicit is used only on stack
> > variables that get cleared just before they go out of scope.
> 
> Are you so sure that a compiler is not getting smart on seeing a memset 
> followed by a free without marking the pointer as volatile? You free the 
> pointer immediately after memset(). I would not want to bet anything that a 
> compiler would leave the memset for non-volatile pointers.
> 
> Besides, memzero_expicit does not cost anything -- it does not add any 
> instruction but convinces the compiler to not optimize it away.

memzero_explicit is only meant for stack pointers, so there is
no need to use it here.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Stephan Mueller]

Am Donnerstag, 14. April 2016, 15:25:17 schrieb Tudor-Dan Ambarus:

Hi Tudor,

> > 
> > > +{
> > > + if (key->d) {
> > > + memset(key->d, '\0', key->n_sz);
> > 
> > memzero_explicit, please
> 
> I don't think this is really needed. memzero_explicit is used only on stack
> variables that get cleared just before they go out of scope.

Are you so sure that a compiler is not getting smart on seeing a memset 
followed by a free without marking the pointer as volatile? You free the 
pointer immediately after memset(). I would not want to bet anything that a 
compiler would leave the memset for non-volatile pointers.

Besides, memzero_expicit does not cost anything -- it does not add any 
instruction but convinces the compiler to not optimize it away.

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Stephan Mueller]

Am Freitag, 8. April 2016, 13:09:02 schrieb Jeffrey Walton:

Hi Jeffrey,

> On Fri, Apr 8, 2016 at 12:55 PM, Stephan Mueller  
wrote:
> > Am Freitag, 8. April 2016, 12:54:10 schrieb Jeffrey Walton:
> > 
> > Hi Jeffrey,
> > 
> >> > +int rsa_check_key_length(unsigned int len)
> >> > +{
> >> > +   switch (len) {
> >> > +   case 512:
> >> > +   case 1024:
> >> > +   case 1536:
> >> > +   case 2048:
> >> > +   case 3072:
> >> > +   case 4096:
> >> > +   return 0;
> >> > +   }
> >> > +
> >> > +   return -EINVAL;
> >> > +}
> >> 
> >> That's an unusual restriction.
> >> 
> >> > +   key->n_sz = vlen;
> >> > +   /* In FIPS mode only allow key size 2K & 3K */
> >> > +   if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
> >> > +   dev_err(ctx->dev, "RSA: key size not allowed in FIPS
> >> > mode\n"); +   goto err;
> >> > +   }
> >> 
> >> That's an unusual restriction, too. As far as I know, FIPS does not
> >> place that restriction.
> > 
> > It does, see SP80-131A and the requirements on CAVS.
> 
> I believe the controlling document is SP800-56B. SP800-131 is just a
> guide, and it digests the information from SP800-56B. For current FIPS
> 140 requirements (SP800-56B), RSA is a Finite Filed (FF) system, and
> the requirement is |N| >= 2048.

To be clear, SP800-131A requires that only 2k or higher is allowed. The second 
constraint comes in with CAVS: you can only test 2k and 3k (and lately 4k) 
RSA. As the requirement is to have CAVS certs, you can therefore only get 
2k/3k/4k CAVS certs.

> 
> Also, I did not see the restriction listed in SP800-131A Rev 1. Cf.,
> http://csrc.nist.gov/publications/drafts/800-131A/sp800-131a_r1_draft.pdf.
> 
> Jeff


Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Jeffrey Walton]

On Fri, Apr 8, 2016 at 12:55 PM, Stephan Mueller  wrote:
> Am Freitag, 8. April 2016, 12:54:10 schrieb Jeffrey Walton:
>
> Hi Jeffrey,
>
>> > +int rsa_check_key_length(unsigned int len)
>> > +{
>> > +   switch (len) {
>> > +   case 512:
>> > +   case 1024:
>> > +   case 1536:
>> > +   case 2048:
>> > +   case 3072:
>> > +   case 4096:
>> > +   return 0;
>> > +   }
>> > +
>> > +   return -EINVAL;
>> > +}
>>
>> That's an unusual restriction.
>>
>> > +   key->n_sz = vlen;
>> > +   /* In FIPS mode only allow key size 2K & 3K */
>> > +   if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
>> > +   dev_err(ctx->dev, "RSA: key size not allowed in FIPS
>> > mode\n"); +   goto err;
>> > +   }
>>
>> That's an unusual restriction, too. As far as I know, FIPS does not
>> place that restriction.
>
> It does, see SP80-131A and the requirements on CAVS.

I believe the controlling document is SP800-56B. SP800-131 is just a
guide, and it digests the information from SP800-56B. For current FIPS
140 requirements (SP800-56B), RSA is a Finite Filed (FF) system, and
the requirement is |N| >= 2048.

Also, I did not see the restriction listed in SP800-131A Rev 1. Cf.,
http://csrc.nist.gov/publications/drafts/800-131A/sp800-131a_r1_draft.pdf.

Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Stephan Mueller]

Am Freitag, 8. April 2016, 12:54:10 schrieb Jeffrey Walton:

Hi Jeffrey,

> > +int rsa_check_key_length(unsigned int len)
> > +{
> > +   switch (len) {
> > +   case 512:
> > +   case 1024:
> > +   case 1536:
> > +   case 2048:
> > +   case 3072:
> > +   case 4096:
> > +   return 0;
> > +   }
> > +
> > +   return -EINVAL;
> > +}
> 
> That's an unusual restriction.
> 
> > +   key->n_sz = vlen;
> > +   /* In FIPS mode only allow key size 2K & 3K */
> > +   if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
> > +   dev_err(ctx->dev, "RSA: key size not allowed in FIPS
> > mode\n"); +   goto err;
> > +   }
> 
> That's an unusual restriction, too. As far as I know, FIPS does not
> place that restriction.

It does, see SP80-131A and the requirements on CAVS.

Very lately they added 4k too, hence my question.
> 
> Jeff


Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Jeffrey Walton]

> +int rsa_check_key_length(unsigned int len)
> +{
> +   switch (len) {
> +   case 512:
> +   case 1024:
> +   case 1536:
> +   case 2048:
> +   case 3072:
> +   case 4096:
> +   return 0;
> +   }
> +
> +   return -EINVAL;
> +}

That's an unusual restriction.

> +   key->n_sz = vlen;
> +   /* In FIPS mode only allow key size 2K & 3K */
> +   if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
> +   dev_err(ctx->dev, "RSA: key size not allowed in FIPS mode\n");
> +   goto err;
> +   }

That's an unusual restriction, too. As far as I know, FIPS does not
place that restriction.

Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Stephan Mueller]

Am Mittwoch, 6. April 2016, 16:37:05 schrieb Tudor Ambarus:

Hi Tudor,

> Dedicated to RSA (hardware) implementations that want to use
> raw integers instead of MPI keys.
> 
> Signed-off-by: Tudor Ambarus 
> ---
>  crypto/rsa.c  |  15 
>  crypto/rsa_helper.c   | 182
> ++ include/crypto/internal/rsa.h | 
> 28 +++
>  3 files changed, 210 insertions(+), 15 deletions(-)
> 
> diff --git a/crypto/rsa.c b/crypto/rsa.c
> index 7cb0153..37ac189 100644
> --- a/crypto/rsa.c
> +++ b/crypto/rsa.c
> @@ -235,21 +235,6 @@ err_free_m:
>   return ret;
>  }
> 
> -static int rsa_check_key_length(unsigned int len)
> -{
> - switch (len) {
> - case 512:
> - case 1024:
> - case 1536:
> - case 2048:
> - case 3072:
> - case 4096:
> - return 0;
> - }
> -
> - return -EINVAL;
> -}
> -
>  static int rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
>  unsigned int keylen)
>  {
> diff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c
> index 0149ed3..df1f480 100644
> --- a/crypto/rsa_helper.c
> +++ b/crypto/rsa_helper.c
> @@ -14,6 +14,9 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
>  #include 
>  #include "rsapubkey-asn1.h"
>  #include "rsaprivkey-asn1.h"
> @@ -239,3 +242,182 @@ error:
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(rsa_parse_mpi_priv_key);
> +
> +int rsa_check_key_length(unsigned int len)
> +{
> + switch (len) {
> + case 512:
> + case 1024:
> + case 1536:
> + case 2048:
> + case 3072:
> + case 4096:
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(rsa_check_key_length);

I assume we can remove that length check in the future and you just ported it 
to be en-par with the feature set of the current implementation?
> +
> +void raw_rsa_free_key(struct rsa_raw_key *key)
> +{
> + kzfree(key->d);
> + key->d = NULL;
> +
> + kfree(key->e);
> + key->e = NULL;
> +
> + kfree(key->n);
> + key->n = NULL;
> +
> + key->n_sz = 0;
> + key->e_sz = 0;
> +}
> +EXPORT_SYMBOL_GPL(raw_rsa_free_key);
> +
> +void raw_rsa_free_coherent_key(struct device *dev, struct rsa_raw_key *key)
> +{
> + if (key->d) {
> + memset(key->d, '\0', key->n_sz);

memzero_explicit, please

> + dma_free_coherent(dev, key->n_sz, key->d, key->dma_d);
> + key->d = NULL;
> + }
> +
> + if (key->e) {
> + dma_free_coherent(dev, key->n_sz, key->e, key->dma_e);
> + key->e = NULL;
> + }
> +
> + if (key->n) {
> + dma_free_coherent(dev, key->n_sz, key->n, key->dma_n);
> + key->n = NULL;
> + }
> +
> + key->n_sz = 0;
> + key->e_sz = 0;
> +}
> +EXPORT_SYMBOL_GPL(raw_rsa_free_coherent_key);
> +
> +int raw_rsa_get_n(void *context, const void *value, size_t vlen)
> +{
> + struct rsa_raw_ctx *ctx = context;
> + struct rsa_raw_key *key = >key;
> + const char *ptr = value;
> + int ret = -EINVAL;
> +
> + while (!*ptr && vlen) {
> + ptr++;
> + vlen--;
> + }
> +
> + key->n_sz = vlen;
> + /* In FIPS mode only allow key size 2K & 3K */

Again, this only excludes 4k as this should be done in a subsequent patch, 
right?

> + if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
> + dev_err(ctx->dev, "RSA: key size not allowed in FIPS mode\n");
> + goto err;
> + }
> + /* invalid key size provided */
> + ret = rsa_check_key_length(key->n_sz << 3);
> + if (ret)
> + goto err;
> +
> + if (key->is_coherent)
> + key->n = kzalloc(key->n_sz, key->flags);
> + else
> + key->n = dma_zalloc_coherent(ctx->dev, key->n_sz, >dma_n,
> +  key->flags);
> +
> + if (!key->n) {
> + ret = -ENOMEM;
> + goto err;
> + }
> +
> + memcpy(key->n, ptr, key->n_sz);
> +
> + return 0;
> +err:
> + key->n_sz = 0;
> + key->n = NULL;
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(raw_rsa_get_n);
> +
> +int raw_rsa_get_e(void *context, const void *value, size_t vlen)
> +{
> + struct rsa_raw_ctx *ctx = context;
> + struct rsa_raw_key *key = >key;
> + const char *ptr = value;
> + size_t offset = 0;
> +
> + while (!*ptr && vlen) {
> + ptr++;
> + vlen--;
> + }
> +
> + key->e_sz = vlen;
> +
> + if (!key->n_sz || !vlen || vlen > key->n_sz) {
> + key->e = NULL;
> + return -EINVAL;
> + }
> +
> + if (key->is_coherent) {
> + key->e = kzalloc(key->e_sz, key->flags);
> + } else {
> + key->e = dma_zalloc_coherent(ctx->dev, key->n_sz, >dma_e,
> +  key->flags);
> + offset = key->n_sz - vlen;
> + }
> +
> + if 

[PATCH v3 2/3] crypto: rsa_helper - add raw integer parser actions

[Tudor Ambarus]

Dedicated to RSA (hardware) implementations that want to use
raw integers instead of MPI keys.

Signed-off-by: Tudor Ambarus 
---
 crypto/rsa.c  |  15 
 crypto/rsa_helper.c   | 182 ++
 include/crypto/internal/rsa.h |  28 +++
 3 files changed, 210 insertions(+), 15 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index 7cb0153..37ac189 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -235,21 +235,6 @@ err_free_m:
return ret;
 }
 
-static int rsa_check_key_length(unsigned int len)
-{
-   switch (len) {
-   case 512:
-   case 1024:
-   case 1536:
-   case 2048:
-   case 3072:
-   case 4096:
-   return 0;
-   }
-
-   return -EINVAL;
-}
-
 static int rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
   unsigned int keylen)
 {
diff --git a/crypto/rsa_helper.c b/crypto/rsa_helper.c
index 0149ed3..df1f480 100644
--- a/crypto/rsa_helper.c
+++ b/crypto/rsa_helper.c
@@ -14,6 +14,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 #include 
 #include "rsapubkey-asn1.h"
 #include "rsaprivkey-asn1.h"
@@ -239,3 +242,182 @@ error:
return ret;
 }
 EXPORT_SYMBOL_GPL(rsa_parse_mpi_priv_key);
+
+int rsa_check_key_length(unsigned int len)
+{
+   switch (len) {
+   case 512:
+   case 1024:
+   case 1536:
+   case 2048:
+   case 3072:
+   case 4096:
+   return 0;
+   }
+
+   return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(rsa_check_key_length);
+
+void raw_rsa_free_key(struct rsa_raw_key *key)
+{
+   kzfree(key->d);
+   key->d = NULL;
+
+   kfree(key->e);
+   key->e = NULL;
+
+   kfree(key->n);
+   key->n = NULL;
+
+   key->n_sz = 0;
+   key->e_sz = 0;
+}
+EXPORT_SYMBOL_GPL(raw_rsa_free_key);
+
+void raw_rsa_free_coherent_key(struct device *dev, struct rsa_raw_key *key)
+{
+   if (key->d) {
+   memset(key->d, '\0', key->n_sz);
+   dma_free_coherent(dev, key->n_sz, key->d, key->dma_d);
+   key->d = NULL;
+   }
+
+   if (key->e) {
+   dma_free_coherent(dev, key->n_sz, key->e, key->dma_e);
+   key->e = NULL;
+   }
+
+   if (key->n) {
+   dma_free_coherent(dev, key->n_sz, key->n, key->dma_n);
+   key->n = NULL;
+   }
+
+   key->n_sz = 0;
+   key->e_sz = 0;
+}
+EXPORT_SYMBOL_GPL(raw_rsa_free_coherent_key);
+
+int raw_rsa_get_n(void *context, const void *value, size_t vlen)
+{
+   struct rsa_raw_ctx *ctx = context;
+   struct rsa_raw_key *key = >key;
+   const char *ptr = value;
+   int ret = -EINVAL;
+
+   while (!*ptr && vlen) {
+   ptr++;
+   vlen--;
+   }
+
+   key->n_sz = vlen;
+   /* In FIPS mode only allow key size 2K & 3K */
+   if (fips_enabled && (key->n_sz != 256 && key->n_sz != 384)) {
+   dev_err(ctx->dev, "RSA: key size not allowed in FIPS mode\n");
+   goto err;
+   }
+   /* invalid key size provided */
+   ret = rsa_check_key_length(key->n_sz << 3);
+   if (ret)
+   goto err;
+
+   if (key->is_coherent)
+   key->n = kzalloc(key->n_sz, key->flags);
+   else
+   key->n = dma_zalloc_coherent(ctx->dev, key->n_sz, >dma_n,
+key->flags);
+
+   if (!key->n) {
+   ret = -ENOMEM;
+   goto err;
+   }
+
+   memcpy(key->n, ptr, key->n_sz);
+
+   return 0;
+err:
+   key->n_sz = 0;
+   key->n = NULL;
+   return ret;
+}
+EXPORT_SYMBOL_GPL(raw_rsa_get_n);
+
+int raw_rsa_get_e(void *context, const void *value, size_t vlen)
+{
+   struct rsa_raw_ctx *ctx = context;
+   struct rsa_raw_key *key = >key;
+   const char *ptr = value;
+   size_t offset = 0;
+
+   while (!*ptr && vlen) {
+   ptr++;
+   vlen--;
+   }
+
+   key->e_sz = vlen;
+
+   if (!key->n_sz || !vlen || vlen > key->n_sz) {
+   key->e = NULL;
+   return -EINVAL;
+   }
+
+   if (key->is_coherent) {
+   key->e = kzalloc(key->e_sz, key->flags);
+   } else {
+   key->e = dma_zalloc_coherent(ctx->dev, key->n_sz, >dma_e,
+key->flags);
+   offset = key->n_sz - vlen;
+   }
+
+   if (!key->e)
+   return -ENOMEM;
+
+   memcpy(key->e + offset, ptr, vlen);
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(raw_rsa_get_e);
+
+int raw_rsa_get_d(void *context, const void *value, size_t vlen)
+{
+   struct rsa_raw_ctx *ctx = context;
+   struct rsa_raw_key *key = >key;
+   const char *ptr = value;
+   size_t offset = 0;
+   int ret = -EINVAL;
+
+   while (!*ptr && vlen) {
+   ptr++;
+   vlen--;
+   }
+
+   if