padata - is serial actually serial?

[Jason A. Donenfeld]

Hi Steffen & Folks,

I submit a job to padata_do_parallel(). When the parallel() function
triggers, I do some things, and then call padata_do_serial(). Finally
the serial() function triggers, where I complete the job (check a
nonce, etc).

The padata API is very appealing because not only does it allow for
parallel computation, but it claims that the serial() functions will
execute in the order that jobs were originally submitted to
padata_do_parallel().

Unfortunately, in practice, I'm pretty sure I'm seeing deviations from
this. When I submit tons and tons of tasks at rapid speed to
padata_do_parallel(), it seems like the serial() function isn't being
called in the exactly the same order that tasks were submitted to
padata_do_parallel().

Is this known (expected) behavior? Or have I stumbled upon a potential
bug that's worthwhile for me to investigate more?

Thanks,
Jason
--
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 v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

[Mat Martineau]

Stephan,

On Sat, 14 May 2016, Tadeusz Struk wrote:


From: Stephan Mueller 

This patch adds the user space interface for asymmetric ciphers. The
interface allows the use of sendmsg as well as vmsplice to provide data.

This version has been rebased on top of 4.6 and a few chackpatch issues
have been fixed.

Signed-off-by: Stephan Mueller 
Signed-off-by: Tadeusz Struk 
---
crypto/algif_akcipher.c |  542 +++
1 file changed, 542 insertions(+)
create mode 100644 crypto/algif_akcipher.c

diff --git a/crypto/algif_akcipher.c b/crypto/algif_akcipher.c
new file mode 100644
index 000..6342b6e
--- /dev/null
+++ b/crypto/algif_akcipher.c
+
+static int akcipher_sendmsg(struct socket *sock, struct msghdr *msg,
+   size_t size)
+{
+   struct sock *sk = sock->sk;
+   struct alg_sock *ask = alg_sk(sk);
+   struct akcipher_ctx *ctx = ask->private;
+   struct akcipher_sg_list *sgl = >tsgl;
+   struct af_alg_control con = {};
+   long copied = 0;
+   int op = 0;
+   bool init = 0;
+   int err;
+
+   if (msg->msg_controllen) {
+   err = af_alg_cmsg_send(msg, );
+   if (err)
+   return err;
+
+   init = 1;
+   switch (con.op) {
+   case ALG_OP_VERIFY:
+   case ALG_OP_SIGN:
+   case ALG_OP_ENCRYPT:
+   case ALG_OP_DECRYPT:
+   op = con.op;
+   break;
+   default:
+   return -EINVAL;
+   }
+   }
+
+   lock_sock(sk);
+   if (!ctx->more && ctx->used)
+   goto unlock;


err might be uninitialised at this goto. Should it be set to something 
like -EALREADY to indicate that data is already queued for a different 
crypto op?





+unlock:
+   akcipher_data_wakeup(sk);
+   release_sock(sk);
+
+   return err ?: copied;
+}


Regards,

--
Mat Martineau
Intel OTC
--
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 v8 1/3] crypto: Key-agreement Protocol Primitives API (KPP)

[Benedetto, Salvatore]

> -Original Message-
> From: Herbert Xu [mailto:herb...@gondor.apana.org.au]
> Sent: Tuesday, June 14, 2016 12:35 PM
> To: Benedetto, Salvatore 
> Cc: linux-crypto@vger.kernel.org
> Subject: Re: [PATCH v8 1/3] crypto: Key-agreement Protocol Primitives API
> (KPP)
> 
> On Mon, Jun 13, 2016 at 10:55:46PM +0100, Salvatore Benedetto wrote:
> >
> > +struct kpp_alg {
> > +   int (*set_secret)(struct crypto_kpp *tfm, void *buffer);
> 
> Sorry I think we need to change this.  Leaving this with no type checking
> between the user and the driver is a recipe for disaster.
> 
> I think the easiest solution is to use either BER encoding like rsa.c or 
> netlink
> encoding like authenc.c.
>

My very first patch used PKCS3 and there were some objections to that.
https://patchwork.kernel.org/patch/8311881/
 
Both Bluetooth or keyctl KEYCTL_DH_COMPUTE would have to first pack the
key to whatever format we choose and I don't see that very convinient. We
only want to provide the acceleration here, without bounding the user to a
certain key format.
 
 akcipher is different as PKCS1 is a recognized standard for RSA keys.
 
 Please don't get me wrong, it's not much of an issue for me to respin the
 patchset and change that to PKCS3 for example, but I see no harm in leaving
 it as it is and moving the key check format to whatever upper layer is using us
 (like BT and keyctl). Just more work for who is using the API.
 
 Could you reconsider that?
 
 Thanks,
 Salvatore
--
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 v5] crypto: rsa - return raw integers for the ASN.1 parser

[Tudor-Dan Ambarus]

Hi Stephan,

> But then I need to refine my question: isn't rsa_parse_priv_key allocating
> the
> MPIs (at least rsa_parse_priv_key seems to hint to that considering the
> error
> code path)? So, shouldn't the MPIs be freed here with free_mpis()? This
> would
> apply to parse_pub_key too.

rsa_parse_priv_key is not allocating the MPIs, mpi_read_raw_data is.

MPIs are freed on error path. On success, they are freed when exiting
the tfm or when setting a new key.

Thanks,
ta
--
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 v5] crypto: rsa - return raw integers for the ASN.1 parser

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 21:38:06 schrieb Herbert Xu:

Hi Herbert,

> On Tue, Jun 14, 2016 at 03:20:06PM +0200, Stephan Mueller wrote:
> > memzero_explicit(raw_key) should be added here in success and failure code
> > paths.
> 
> The raw_key is just a bunch of pointers, do we really need to
> zero it?

You are correct.

But then I need to refine my question: isn't rsa_parse_priv_key allocating the 
MPIs (at least rsa_parse_priv_key seems to hint to that considering the error 
code path)? So, shouldn't the MPIs be freed here with free_mpis()? This would 
apply to parse_pub_key too.

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 v5] crypto: rsa - return raw integers for the ASN.1 parser

[Herbert Xu]

On Tue, Jun 14, 2016 at 03:20:06PM +0200, Stephan Mueller wrote:
>
> memzero_explicit(raw_key) should be added here in success and failure code 
> paths.

The raw_key is just a bunch of pointers, do we really need to
zero it?

Thanks,
-- 
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 v5] crypto: rsa - return raw integers for the ASN.1 parser

[Stephan Mueller]

Am Dienstag, 14. Juni 2016, 16:14:58 schrieb Tudor Ambarus:

Hi Tudor,

>  static int rsa_set_priv_key(struct crypto_akcipher *tfm, const void *key,
>   unsigned int keylen)
>  {
> - struct rsa_key *pkey = akcipher_tfm_ctx(tfm);
> + struct rsa_mpi_key *mpi_key = akcipher_tfm_ctx(tfm);
> + struct rsa_key raw_key = {0};
>   int ret;
> 
> - ret = rsa_parse_priv_key(pkey, key, keylen);
> + /* Free the old MPI key if any */
> + rsa_free_mpi_key(mpi_key);
> +
> + ret = rsa_parse_priv_key(_key, key, keylen);
>   if (ret)
>   return ret;
> 
> - if (rsa_check_key_length(mpi_get_size(pkey->n) << 3)) {
> - rsa_free_key(pkey);
> - ret = -EINVAL;
> + mpi_key->d = mpi_read_raw_data(raw_key.d, raw_key.d_sz);
> + if (!mpi_key->d)
> + goto err;
> +
> + mpi_key->e = mpi_read_raw_data(raw_key.e, raw_key.e_sz);
> + if (!mpi_key->e)
> + goto err;
> +
> + mpi_key->n = mpi_read_raw_data(raw_key.n, raw_key.n_sz);
> + if (!mpi_key->n)
> + goto err;
> +
> + if (rsa_check_key_length(mpi_get_size(mpi_key->n) << 3)) {
> + rsa_free_mpi_key(mpi_key);
> + return -EINVAL;
>   }
> - return ret;
> +
> + return 0;
> +
> +err:
> + rsa_free_mpi_key(mpi_key);
> + return -ENOMEM;
>  }

memzero_explicit(raw_key) should be added here in success and failure code 
paths.

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

[PATCH v5] crypto: rsa - return raw integers for the ASN.1 parser

[Tudor Ambarus]

Return the raw key with no other processing so that the caller
can copy it or MPI parse it, etc.

The scope is to have only one ANS.1 parser for all RSA
implementations.

Update the RSA software implementation so that it does
the MPI conversion on top.

Signed-off-by: Tudor Ambarus 
---
Changes in v5:
- store raw keys on stack

Changes in v4:
1. Remove the skipping of leading zeros from rsa_get_n/e/d helper functions.
2. Remove FIPS RSA key length checking for the RSA private exponent.
FIPS check is done only for the RSA modulus.


 crypto/rsa.c  | 105 +--
 crypto/rsa_helper.c   | 111 +++---
 include/crypto/internal/rsa.h |  22 ++---
 3 files changed, 135 insertions(+), 103 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index 77d737f..dc692d4 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -10,16 +10,23 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 
+struct rsa_mpi_key {
+   MPI n;
+   MPI e;
+   MPI d;
+};
+
 /*
  * RSAEP function [RFC3447 sec 5.1.1]
  * c = m^e mod n;
  */
-static int _rsa_enc(const struct rsa_key *key, MPI c, MPI m)
+static int _rsa_enc(const struct rsa_mpi_key *key, MPI c, MPI m)
 {
/* (1) Validate 0 <= m < n */
if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
@@ -33,7 +40,7 @@ static int _rsa_enc(const struct rsa_key *key, MPI c, MPI m)
  * RSADP function [RFC3447 sec 5.1.2]
  * m = c^d mod n;
  */
-static int _rsa_dec(const struct rsa_key *key, MPI m, MPI c)
+static int _rsa_dec(const struct rsa_mpi_key *key, MPI m, MPI c)
 {
/* (1) Validate 0 <= c < n */
if (mpi_cmp_ui(c, 0) < 0 || mpi_cmp(c, key->n) >= 0)
@@ -47,7 +54,7 @@ static int _rsa_dec(const struct rsa_key *key, MPI m, MPI c)
  * RSASP1 function [RFC3447 sec 5.2.1]
  * s = m^d mod n
  */
-static int _rsa_sign(const struct rsa_key *key, MPI s, MPI m)
+static int _rsa_sign(const struct rsa_mpi_key *key, MPI s, MPI m)
 {
/* (1) Validate 0 <= m < n */
if (mpi_cmp_ui(m, 0) < 0 || mpi_cmp(m, key->n) >= 0)
@@ -61,7 +68,7 @@ static int _rsa_sign(const struct rsa_key *key, MPI s, MPI m)
  * RSAVP1 function [RFC3447 sec 5.2.2]
  * m = s^e mod n;
  */
-static int _rsa_verify(const struct rsa_key *key, MPI m, MPI s)
+static int _rsa_verify(const struct rsa_mpi_key *key, MPI m, MPI s)
 {
/* (1) Validate 0 <= s < n */
if (mpi_cmp_ui(s, 0) < 0 || mpi_cmp(s, key->n) >= 0)
@@ -71,7 +78,7 @@ static int _rsa_verify(const struct rsa_key *key, MPI m, MPI 
s)
return mpi_powm(m, s, key->e, key->n);
 }
 
-static inline struct rsa_key *rsa_get_key(struct crypto_akcipher *tfm)
+static inline struct rsa_mpi_key *rsa_get_key(struct crypto_akcipher *tfm)
 {
return akcipher_tfm_ctx(tfm);
 }
@@ -79,7 +86,7 @@ static inline struct rsa_key *rsa_get_key(struct 
crypto_akcipher *tfm)
 static int rsa_enc(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI m, c = mpi_alloc(0);
int ret = 0;
int sign;
@@ -118,7 +125,7 @@ err_free_c:
 static int rsa_dec(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI c, m = mpi_alloc(0);
int ret = 0;
int sign;
@@ -156,7 +163,7 @@ err_free_m:
 static int rsa_sign(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI m, s = mpi_alloc(0);
int ret = 0;
int sign;
@@ -195,7 +202,7 @@ err_free_s:
 static int rsa_verify(struct akcipher_request *req)
 {
struct crypto_akcipher *tfm = crypto_akcipher_reqtfm(req);
-   const struct rsa_key *pkey = rsa_get_key(tfm);
+   const struct rsa_mpi_key *pkey = rsa_get_key(tfm);
MPI s, m = mpi_alloc(0);
int ret = 0;
int sign;
@@ -233,6 +240,16 @@ err_free_m:
return ret;
 }
 
+static void rsa_free_mpi_key(struct rsa_mpi_key *key)
+{
+   mpi_free(key->d);
+   mpi_free(key->e);
+   mpi_free(key->n);
+   key->d = NULL;
+   key->e = NULL;
+   key->n = NULL;
+}
+
 static int rsa_check_key_length(unsigned int len)
 {
switch (len) {
@@ -251,49 +268,87 @@ static int rsa_check_key_length(unsigned int len)
 static int rsa_set_pub_key(struct crypto_akcipher *tfm, const void *key,
   unsigned int keylen)
 {
-   struct rsa_key *pkey = akcipher_tfm_ctx(tfm);
+   struct rsa_mpi_key *mpi_key = akcipher_tfm_ctx(tfm);
+   struct rsa_key raw_key = {0};
int ret;
 
-   ret 

Re: [PATCH v8 1/3] crypto: Key-agreement Protocol Primitives API (KPP)

[Herbert Xu]

On Mon, Jun 13, 2016 at 10:55:46PM +0100, Salvatore Benedetto wrote:
>
> +struct kpp_alg {
> + int (*set_secret)(struct crypto_kpp *tfm, void *buffer);

Sorry I think we need to change this.  Leaving this with no type
checking between the user and the driver is a recipe for disaster.

I think the easiest solution is to use either BER encoding like
rsa.c or netlink encoding like authenc.c.

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 v4] crypto: rsa - return raw integers for the ASN.1 parser

[Herbert Xu]

On Mon, Jun 13, 2016 at 05:12:53PM +0300, Tudor Ambarus wrote:
> +
> +struct rsa_ctx {
> + struct rsa_key key;

This isn't necessary and worse it may lead to bugs in future.
The raw keys will be invalid as soon as the setkey functions
return.

So just store it on the stack.

Thanks,
-- 
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 v2 0/3] hw_random: Add Amlogic Meson SoCs Random Generator driver

[Herbert Xu]

On Mon, Jun 13, 2016 at 03:10:28PM -0700, Kevin Hilman wrote:
> Could you take just the driver please?
> 
> Due to lots of other activity in the DT, I'd prefer to send the DT &
> bindings though the arm-soc (via the amlogic tree.)

OK I will back out these two patches.

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 v6 3/6] crypto: AF_ALG -- add asymmetric cipher interface

[Andrew Zaborowski]

Hi Stephan,

On 14 June 2016 at 07:12, Stephan Mueller  wrote:
> Am Dienstag, 14. Juni 2016, 00:16:11 schrieb Andrew Zaborowski:
>> On 8 June 2016 at 21:14, Mat Martineau
>>
>>  wrote:
>> > On Wed, 8 Jun 2016, Stephan Mueller wrote:
>> >> What is your concern?
>> >
>> > Userspace must allocate larger buffers than it knows are necessary for
>> > expected results.
>> >
>> > It looks like the software rsa implementation handles shorter output
>> > buffers ok (mpi_write_to_sgl will return EOVERFLOW if the the buffer is
>> > too small), however I see at least one hardware rsa driver that requires
>> > the output buffer to be the maximum size. But this inconsistency might be
>> > best addressed within the software cipher or drivers rather than in
>> > recvmsg.
>> Should the hardware drivers fix this instead?  I've looked at the qat
>> and caam drivers, they both require the destination buffer size to be
>> the key size and in both cases there would be no penalty for dropping
>> this requirement as far as I see.  Both do a memmove if the result
>> ends up being shorter than key size.  In case the caller knows it is
>> expecting a specific output size, the driver will have to use a self
>> allocated buffer + a memcpy in those same cases where it would later
>> use memmove instead.  Alternatively the sg passed to dma_map_sg can be
>> prepended with a dummy segment the right size to save the memcpy.
>>
>> akcipher.h only says:
>> @dst_len: Size of the output buffer. It needs to be at least as big as
>> the expected result depending on the operation
>>
>> Note that for random input data the memmove will be done about 1 in
>> 256 times but with PKCS#1 padding the signature always has a leading
>> zero.
>>
>> Requiring buffers bigger than needed makes the added work of dropping
>> the zero bytes from the sglist and potentially re-adding them in the
>> client difficult to justify.  RSA doing this sets a precedent for a
>> future pkcs1pad (or other algorithm) implementation to do the same
>> thing and a portable client having to always know the key size and use
>> key-sized buffers.
>
> I think we have agreed on dropping the length enforcement at the interface
> level.

Separately from this there's a problem with the user being unable to
know if the algorithm is going to fail because of destination buffer
size != key size (including kernel users).  For RSA, the qat
implementation will fail while the software implementation won't.  For
pkcs1pad(...) there's currently just one implementation but the user
can't assume that.

Best regards
--
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