2017-03-10 11:55:18

by Alexander Sverdlin

[permalink] [raw]
Subject: CRYPTO_MAX_ALG_NAME is too low

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.


2017-03-16 14:16:37

by Alexander Sverdlin

[permalink] [raw]
Subject: Re: CRYPTO_MAX_ALG_NAME is too low

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.

2017-04-06 08:15:35

by Herbert Xu

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

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 <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2017-04-06 15:11:10

by Alexander Sverdlin

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

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.

2017-04-06 20:58:38

by David Miller

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

From: Herbert Xu <[email protected]>
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.

2017-04-06 21:14:35

by Steffen Klassert

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

On Thu, Apr 06, 2017 at 01:58:32PM -0700, David Miller wrote:
> From: Herbert Xu <[email protected]>
> 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.