Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[Steffen Klassert]

On Thu, Apr 06, 2017 at 01:58:32PM -0700, David Miller wrote:
> From: Herbert Xu 
> Date: Thu, 6 Apr 2017 16:15:09 +0800
> 
> > As the final patch depends on all three it would be easiest if
> > we pushed the xfrm patch through the crypto tree.  Steffen/David?
> 
> No objections from me for this going through the crypto tree.

I have no objections too.

Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[David Miller]

From: Herbert Xu 
Date: Thu, 6 Apr 2017 16:15:09 +0800

> As the final patch depends on all three it would be easiest if
> we pushed the xfrm patch through the crypto tree.  Steffen/David?

No objections from me for this going through the crypto tree.

Re: [PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[Alexander Sverdlin]

Hi!

On 06/04/17 10:15, Herbert Xu wrote:
> On Thu, Mar 16, 2017 at 03:16:29PM +0100, Alexander Sverdlin wrote:
>> This is a regression caused by 856e3f4092
>> ("crypto: seqiv - Add support for new AEAD interface")
>>
>> As I've said above, I can offer one of the two solutions, which patch should 
>> I send?
>> Or do you see any better alternatives?
> Here is a series of patches which should fix the problem.
> 
> The first three patches prepare the user-space interfaces to deal
> with longer names.  The final patch extends it.
> 
> Note that with crypto_user I haven't actually extended it to
> configure longer names.  It'll only be able to configure names
> less than 64 bytes.  However, it should be able to dump/read
> algorithms with longer names, albeit the name will be truncated
> to 64 bytes length.
> 
> Steffen, when convenient could you look into extending the crypto
> user interface to handle longer names (preferably arbitraty length
> since netlink should be able to deal with that)?
> 
> Likewise xfrm is still fixed to 64 bytes long.  But this should
> be OK as the problematic case only arises with IV generators for
> now and we do not allow IV generators to be specified through xfrm.
> 
> af_alg on the other hand now allows arbitrarily long names.

I'm not sure about patch 2 (as I've replied separately), but I've applied
and tested the whole series and it at least solves the original problem
with long algorithm name.

> As the final patch depends on all three it would be easiest if
> we pushed the xfrm patch through the crypto tree.  Steffen/David?

-- 
Best regards,
Alexander Sverdlin.

[PATCH 0/4] crypto: CRYPTO_MAX_ALG_NAME is too low

[Herbert Xu]

On Thu, Mar 16, 2017 at 03:16:29PM +0100, Alexander Sverdlin wrote:
>
> This is a regression caused by 856e3f4092
> ("crypto: seqiv - Add support for new AEAD interface")
> 
> As I've said above, I can offer one of the two solutions, which patch should 
> I send?
> Or do you see any better alternatives?

Here is a series of patches which should fix the problem.

The first three patches prepare the user-space interfaces to deal
with longer names.  The final patch extends it.

Note that with crypto_user I haven't actually extended it to
configure longer names.  It'll only be able to configure names
less than 64 bytes.  However, it should be able to dump/read
algorithms with longer names, albeit the name will be truncated
to 64 bytes length.

Steffen, when convenient could you look into extending the crypto
user interface to handle longer names (preferably arbitraty length
since netlink should be able to deal with that)?

Likewise xfrm is still fixed to 64 bytes long.  But this should
be OK as the problematic case only arises with IV generators for
now and we do not allow IV generators to be specified through xfrm.

af_alg on the other hand now allows arbitrarily long names.

As the final patch depends on all three it would be easiest if
we pushed the xfrm patch through the crypto tree.  Steffen/David?

Thanks,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: CRYPTO_MAX_ALG_NAME is too low

[Alexander Sverdlin]

Hello!

On 10/03/17 12:55, Alexander Sverdlin wrote:
> Hello crypto maintainers!
> 
> We've found and example of the ipsec algorithm combination, which doesn't fit
> into CRYPTO_MAX_ALG_NAME long buffers:
> 
> ip x s add src 1.1.1.1 dst 1.1.1.2 proto esp spi 0 mode tunnel enc des3_ede 
> 0x0 auth sha256 0x0 flag esn replay-window 256
> 
> produces "echainiv(authencesn(hmac(sha256-generic),cbc(des3_ede-generic)))"
> on the machines without optimized crypto drivers, which doesn't fit into 
> current
> 64-bytes buffers.
> 
> I see two possible options:
> 
> a) split CRYPTO_MAX_ALG_NAME into CRYPTO_MAX_ALG_NAME + CRYPTO_MAX_DRV_NAME 
> pair
> and make later, say, 96, because the former probably cannot be changed 
> because of
> numerous user-space exports. And change half of the code to use new define.
> 
> b) rename *-generic algorithms to *-gen, so that cra_driver_name will be 
> shortened,
> while MODULE_ALIAS_CRYPTO() could still be maintained in old and new form.
> 
> What are your thoughts?

Any?

This is a regression caused by 856e3f4092
("crypto: seqiv - Add support for new AEAD interface")

As I've said above, I can offer one of the two solutions, which patch should I 
send?
Or do you see any better alternatives?

-- 
Best regards,
Alexander Sverdlin.

CRYPTO_MAX_ALG_NAME is too low

[Alexander Sverdlin]

Hello crypto maintainers!

We've found and example of the ipsec algorithm combination, which doesn't fit
into CRYPTO_MAX_ALG_NAME long buffers:

ip x s add src 1.1.1.1 dst 1.1.1.2 proto esp spi 0 mode tunnel enc des3_ede 0x0 
auth sha256 0x0 flag esn replay-window 256

produces "echainiv(authencesn(hmac(sha256-generic),cbc(des3_ede-generic)))"
on the machines without optimized crypto drivers, which doesn't fit into current
64-bytes buffers.

I see two possible options:

a) split CRYPTO_MAX_ALG_NAME into CRYPTO_MAX_ALG_NAME + CRYPTO_MAX_DRV_NAME pair
and make later, say, 96, because the former probably cannot be changed because 
of
numerous user-space exports. And change half of the code to use new define.

b) rename *-generic algorithms to *-gen, so that cra_driver_name will be 
shortened,
while MODULE_ALIAS_CRYPTO() could still be maintained in old and new form.

What are your thoughts?

-- 
Best regards,
Alexander Sverdlin.