Re: [PATCH v6 1/5] random: Blocking API for accessing nonblocking_pool

2015-06-04 Thread Herbert Xu
On Tue, May 19, 2015 at 10:18:05PM +0800, Herbert Xu wrote:
> On Tue, May 19, 2015 at 09:50:28AM -0400, Theodore Ts'o wrote:
> >
> > Finally, this is only going to block *once*, when the system is
> > initially botting up.  Why is it so important that we get the
> > asynchronous nature of this right, and why can't we solve it simply by
> > just simply doing the work in a workqueue, with a completion barrier
> > getting triggered once /dev/random initializes itself, and just simply
> > blocking the module unload until /dev/random is initialized?
> 
> I guess I'm still thinking of the old work queue code before
> Tejun's cmwq work.  Yes blocking in a work queue should be fine
> as there is usually just one DRBG instance.

It looks like waiting for it in a workqueue isn't good enough
after all.  I just got this in a KVM machine:

INFO: task kworker/0:1:121 blocked for more than 120 seconds.
  Tainted: G   O4.1.0-rc1+ #34
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
kworker/0:1 D 88001eb47d18 0   121  2 0x
Workqueue: events drbg_async_seed [drbg]
 88001eb47d18 88001e1bcec0 88001eb84010 0246
 88001eb48000 0020 88001d54ea50 
 88001f613080 88001eb47d38 813fe692 0020
Call Trace:
 [] schedule+0x32/0x80
 [] get_blocking_random_bytes+0x65/0xa0
 [] ? add_wait_queue+0x60/0x60
 [] drbg_async_seed+0x2c/0xc0 [drbg]
 [] process_one_work+0x129/0x310
 [] worker_thread+0x119/0x430
 [] ? __schedule+0x7fb/0x85e
 [] ? process_scheduled_works+0x40/0x40
 [] kthread+0xc4/0xe0
 [] ? proc_cap_handler+0x180/0x1b0
 [] ? kthread_freezable_should_stop+0x60/0x60
 [] ret_from_fork+0x42/0x70
 [] ? kthread_freezable_should_stop+0x60/0x60

Steffen, I think we need to revisit the idea of having a list
of callbacks.

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: Crypto driver -DCP

2015-06-04 Thread Herbert Xu
On Thu, Jun 04, 2015 at 05:34:39PM +0200, Marek Vasut wrote:
> 
> Is this really a valid way to go about crypto -- introduce all kinds
> of obscure nuances into the API which are driver specific at best ?

So what do you suggest?
-- 
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: [v2 PATCH 5/13] crypto: testmgr - Switch to new AEAD interface

2015-06-04 Thread Herbert Xu
On Thu, Jun 04, 2015 at 03:15:19PM -0700, Tadeusz Struk wrote:
> Hi Herbert,
> On 05/22/2015 01:30 AM, Herbert Xu wrote:
> > This patch makes use of the new AEAD interface which uses a single
> > SG list instead of separate lists for the AD and plain text.
> 
> The fact the src and assoc point to the same sgl causes some inconsistency. 
> The input I'm getting is:
> req->old = 1
> req->src_nents = 1
> req->src_len = 80
> req->dst_nents = 1
> req->dst_len = 80
> req->assoclen = 0
> 
> but
> 
> req->assoc_nents = 1
> req->assoc_len = 80
> 
> I presume req->assoc is obsolete now and drivers need to use req->assoclen. 
> right?
> Currently I just loop over req->assoc to get the AD.

Existing AEAD implementations should be completely unaware of
the new interface because we recreate the old req->assoc in the
crypto API.

However, if you are creating a new AEAD implementation then yes
you should stop using req->assoc and fetch it from req->src instead.

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: [v2 PATCH 5/13] crypto: testmgr - Switch to new AEAD interface

2015-06-04 Thread Tadeusz Struk
Hi Herbert,
On 05/22/2015 01:30 AM, Herbert Xu wrote:
> This patch makes use of the new AEAD interface which uses a single
> SG list instead of separate lists for the AD and plain text.

The fact the src and assoc point to the same sgl causes some inconsistency. The 
input I'm getting is:
req->old = 1
req->src_nents = 1
req->src_len = 80
req->dst_nents = 1
req->dst_len = 80
req->assoclen = 0

but

req->assoc_nents = 1
req->assoc_len = 80

I presume req->assoc is obsolete now and drivers need to use req->assoclen. 
right?
Currently I just loop over req->assoc to get the AD.
regards,
T

--
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 3/9] crypto: Add a generic Poly1305 authenticator implementation

2015-06-04 Thread Martin Willi
Herbert,

> I just realised that this doesn't quite work.  The key is shared
> by all users of the tfm, yet in your case you need it to be local

I agree, as Poly1305 uses a different key for each tag the current
approach doesn't work.

> I think the simplest solution is to make the key the beginning
> of the hashed text instead.  So the first two blocks that you
> process get used as the key.

Yes, that makes sense. I'll prepare a fix, might require some days,
though.

Thanks!
Martin

--
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 RFC v3 2/3] crypto: RSA: KEYS: convert rsa and public key to new PKE API

2015-06-04 Thread Tadeusz Struk
On 06/03/2015 11:53 PM, Herbert Xu wrote:
> I'd like to see this split into multiple patches.  First of all
> the new crypto_akcipher implementation should coexist with the
> existing code.  That way the exiting users can be converted over
> one-by-one.
> 
> Also you should implement the crypto_akcipher completely before
> converting anybody over, that means doing encoding/wrapping in
> addition to the crypto.
> 
> That way we don't have to have craziness like converting in and
> out of MPI multiple times.

That's cool. It will make it easier for me. I wanted to convert
one to show that the interface is working.

> 
> Lastly you should consider adding an MPI helper that writes to
> an existing buffer instead of allocating a new one and copying
> it over.

I'll think about something.

--
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 RFC v3 1/3] crypto: add PKE API

2015-06-04 Thread Tadeusz Struk
Hi Herbert,
On 06/03/2015 11:49 PM, Herbert Xu wrote:
> Because the caller is going to be allocating memory for the output,
> we need to provide a way for them to know how much memory to
> allocate.
> 
> This presumably will depend on the key size.
> 
> So something like
> 
>   int (*maxsize)(struct crypto_akcipher *tfm);
> 
> is needed.
> 
> You should also provide setkey here.  You can't just save a pointer
> to the key.  The transform must hold the key physically as the
> original may go away.  It should also ensure that the key is
> actually valid for the transform.
> 
> base already has ctx so you should get rid of ctx and move base
> to the end of the struct.

right, will do that.
Thanks for quick response.




--
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 RFC v3 3/3] crypto: add tests vectors for RSA

2015-06-04 Thread Tadeusz Struk
Hi Stephan
On 06/03/2015 05:15 PM, Stephan Mueller wrote:
> May I ask that the outbuf_enc is memcmp()ed with an expected value? This 
> check 
> is required for FIPS 140-2 compliance. Without that memcmp, FIPS 140-2 
> validations will not be successful.

Sure, I will do that. I wasn't aware that this was required.

> 
> Sorry for bringing that one up just now: 512 and 1024 bit test vectors will 
> not be helpful for several use cases, including FIPS. I can offer to give you 
> 2k or 3k vectors.

I have one 2K vector from openSSL fips so I'll use it instead of the 512 one.

> Besides, wouldn't one vector be sufficient?

I think there is no harm to have these 3 vectors to make sure an implementation
is well tested.
--
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 02/13] dmaengine: Introduce dma_request_slave_channel_compat_reason()

2015-06-04 Thread Peter Ujfalusi
Vinod,

On 06/02/2015 03:55 PM, Vinod Koul wrote:
> On Fri, May 29, 2015 at 05:32:50PM +0300, Peter Ujfalusi wrote:
>> On 05/29/2015 01:18 PM, Vinod Koul wrote:
>>> On Fri, May 29, 2015 at 11:42:27AM +0200, Geert Uytterhoeven wrote:
 On Fri, May 29, 2015 at 11:33 AM, Vinod Koul  wrote:
> On Tue, May 26, 2015 at 04:25:57PM +0300, Peter Ujfalusi wrote:
>> dma_request_slave_channel_compat() 'eats' up the returned error codes 
>> which
>> prevents drivers using the compat call to be able to do deferred probing.
>>
>> The new wrapper is identical in functionality but it will return with 
>> error
>> code in case of failure and will pass the -EPROBE_DEFER to the caller in
>> case dma_request_slave_channel_reason() returned with it.
> This is okay but am worried about one more warpper, how about fixing
> dma_request_slave_channel_compat()

 Then all callers of dma_request_slave_channel_compat() have to be
 modified to handle ERR_PTR first.

 The same is true for (the existing) dma_request_slave_channel_reason()
 vs. dma_request_slave_channel().
>>> Good point, looking again, I think we should rather fix
>>> dma_request_slave_channel_reason() as it was expected to return err code and
>>> add new users. Anyway users of this API do expect the reason...
>>
>> Hrm, they are for different use.dma_request_slave_channel()/_reason() is for
>> drivers only working via DT or ACPI while
>> dma_request_slave_channel_compat()/_reason() is for drivers expected to run 
>> in
>> DT/ACPI or legacy mode as well.
>>
>> I added the dma_request_slave_channel_compat_reason() because OMAP/daVinci
>> drivers are using this to request channels - they need to support DT and
>> legacy mode.
> I think we should hide these things behind the API and do this behind the
> hood for ACPI/DT systems.
> 
> Also it makes sense to use right API and mark rest as depricated

So to convert the dma_request_slave_channel_compat() and not to create _reason
variant?

Or to have single API to request channel? The problem with that is that we
need different parameters for legacy and DT for example.

>>
>> But it is doable to do this for both the non _compat and _compat version:
>> 1. change all users to check IS_ERR_OR_NULL(chan)
>>  return the PTR_ERR if not NULL, or do whatever the driver was doing in case
>> of chan == NULL.
>> 2. change the non _compat and _compat versions to do the same as the _reason
>> variants, #define the _reason ones to the non _reason names
>> 3. Rename the _reason use to non _reason function in drivers
>> 4. Remove the #defines for the _reason functions
>> 5. Change the IS_ERR_OR_NULL(chan) to IS_ERR(chan) in all drivers
>> The result:
>> Both dma_request_slave_channel() and dma_request_slave_channel_compat() will
>> return ERR_PTR in case of failure or in success they will return the pinter 
>> to
>> chan.
>>
>> Is this what you were asking?
>> It is a bit broader than what this series was doing: taking care of
>> OMAP/daVinci drivers for deferred probing regarding to dmaengine ;)
> Yes but it would make sense right? I know it is a larger work but then we
> wouldn't want another dma_request_slave_xxx API, at some point we have stop
> it exapnding, perhpas now :)

Yes, it make sense to get rid if the _reason() things and have the
dma_request_slave_channel() and dma_request_slave_channel_compat() return with
error code

One thing we need to do for this is to change the error codes coming back from
the _dt() and _acpi() calls when we boot in legacy mode. Right now the only
error code which comes back is -ENODEV and -EPROBE_DEFER. We need to
differentiate between 'real' errors and from the fact that we did not booted
with DT or the ACPI is not available.
IMHO if we boot with DT and the channel request fails with other than
-EPROBE_DEFER we should not go and try to get the channel via legacy API.

> Yes I am all ears to stage this work and not do transition gardually..
> 


-- 
Péter
--
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: Crypto driver -DCP

2015-06-04 Thread Marek Vasut
On Thursday, June 04, 2015 at 05:24:00 AM, Herbert Xu wrote:
> On Wed, Jun 03, 2015 at 03:02:13PM -0500, Jay Monkman wrote:
> > That would be one use, but a more likely use would be to prevent
> > access to the keys. A system could write keys to the key slots in
> > the bootloader or in a TrustZone secure world. Then those keys could
> > be used for crypto operations in Linux without ever exposing them.
> > Key slots can be written to, but cannot be read from.
> > 
> > Even with keys stored in key slots, other keys may be used. For
> > 
> > example, someone could do:
> > operation w/ key in slot 1
> > operation w/ key provided in descriptor
> > operation w/ key in slot 1
> > 
> > I don't think an LRU scheme would allow something like that.
> 
> In that case I would suggest using setkey with a length other
> than that of a valid AES key.  For example, you could use a one-
> byte value to select the key slot.

Is this really a valid way to go about crypto -- introduce all kinds
of obscure nuances into the API which are driver specific at best ?

Best regards,
Marek Vasut
--
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 3/9] crypto: Add a generic Poly1305 authenticator implementation

2015-06-04 Thread Herbert Xu
On Mon, Jun 01, 2015 at 01:43:58PM +0200, Martin Willi wrote:
>
> +static int poly1305_setkey(struct crypto_shash *tfm,
> +const u8 *key, unsigned int keylen)
> +{
> + struct poly1305_ctx *ctx = crypto_shash_ctx(tfm);
> +
> + if (keylen != POLY1305_KEY_SIZE) {
> + crypto_shash_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> + return -EINVAL;
> + }
> +
> + /* r &= 0xffc0ffc0ffc0fff */
> + ctx->r[0] = (le32_to_cpuvp(key +  0) >> 0) & 0x3ff;
> + ctx->r[1] = (le32_to_cpuvp(key +  3) >> 2) & 0x303;
> + ctx->r[2] = (le32_to_cpuvp(key +  6) >> 4) & 0x3ffc0ff;
> + ctx->r[3] = (le32_to_cpuvp(key +  9) >> 6) & 0x3f03fff;
> + ctx->r[4] = (le32_to_cpuvp(key + 12) >> 8) & 0x00f;
> +
> + ctx->s[0] = le32_to_cpuvp(key + 16);
> + ctx->s[1] = le32_to_cpuvp(key + 20);
> + ctx->s[2] = le32_to_cpuvp(key + 24);
> + ctx->s[3] = le32_to_cpuvp(key + 28);
> +
> + return 0;
> +}

I just realised that this doesn't quite work.  The key is shared
by all users of the tfm, yet in your case you need it to be local
to the shash_desc as otherwise two packets processed in parallel
will overwrite each other's key.

I think the simplest solution is to make the key the beginning
of the hashed text instead.  So the first two blocks that you
process get used as the key.

What do you think?

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] crypto: fix nx-842 pSeries driver minimum buffer size

2015-06-04 Thread Herbert Xu
On Tue, Jun 02, 2015 at 03:22:10PM -0400, Dan Streetman wrote:
> Reduce the nx-842 pSeries driver minimum buffer size from 128 to 8.
> Also replace the single use of IO_BUFFER_ALIGN macro with the standard
> and correct DDE_BUFFER_ALIGN.
> 
> The hw sometimes rejects buffers that contain padding past the end of the
> 8-byte aligned section where it sees the "end" marker.  With the minimum
> buffer size set too high, some highly compressed buffers were being padded
> and the hw was incorrectly rejecting them; this sets the minimum correctly
> so there will be no incorrect padding.
> 
> Signed-off-by: Dan Streetman 

Applied.
-- 
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 1/2] Doc:crypto: Fix typo in crypto-API.tmpl

2015-06-04 Thread Herbert Xu
On Thu, Jun 04, 2015 at 12:01:20AM +0900, Masanari Iida wrote:
> This patch fix some spelling typo found in crypto-API.tmpl
> 
> Signed-off-by: Masanari Iida 

Both applied.
-- 
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 0/9] crypto: Add ChaCha20-Poly1305 AEAD support for IPsec

2015-06-04 Thread Herbert Xu
On Mon, Jun 01, 2015 at 01:43:55PM +0200, Martin Willi wrote:
> This is a first version of a patch series implementing the ChaCha20-Poly1305
> AEAD construction defined in RFC7539. It is based on the current cryptodev 
> tree.
> 
> The first two patches implement the ChaCha20 cipher, the second two the 
> Poly1305
> authenticator, both in portable C for all architectures. Patch 5 and 6
> provide an AEAD construction using the two cipher primitives, named rfc7539.
> 
> Patch 7 and 8 add a variant of the same AEAD that uses additional key material
> as a nonce to shorten the explicit IV to 8 bytes, as defined for use in IPsec
> in draft-ietf-ipsecme-chacha20-poly1305. The last patch exposes that AEAD
> to IPsec users.
> 
> I don't expect any technical changes to draft-ietf-ipsecme-chacha20-poly1305,
> but we don't have an RFC name yet to reference the AEAD. We therefore simply
> name it rfc7539esp, but other suggestions are welcome.
> 
> The AEAD uses the crypto_nivaead_type to make it available to IPsec. However,
> I was unable to run test vectors against this type of AEAD on cryptodev, but
> I've verified the vectors against the same AEAD using crypto_aead_type.
> Additionally IPsec traffic has been tested against our userland ESP backend in
> strongSwan.
> 
> On my x64_64 test setup the IPsec throughput is ~700Mbits/s with these 
> portable
> drivers. Architecture specific drivers subject to a future patchset can 
> improve
> performance, for example with SSE doubling performance is feasible.

All applied.  Thanks a lot!
-- 
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