2019-02-20 04:06:35

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC PATCH] crypto: pcrypt - forbid recursive instantiation

On Wed, Apr 18, 2018 at 07:35:33AM +0200, Steffen Klassert wrote:
>
> Yes sure, I just wanted to know if it is worth to think about
> preventing template recursions. If there is a valid usecase,
> then we don't even need to think in this direction.
>
> While I think each pcrypt instance should have it's own
> padata instance on the long run, it would be good to have
> a not so intrusive fix that can be backported to the stable
> trees.

Steffen, has there been any progress on this work?

We need to fix this soon or we'll have to disable pcrypt because
it is a security issue.

It's not just about nested templates either. You can trigger
the same issue where a pcrypt instance over an AEAD algorithm
that uses a fallback which also happens to be pcrypt.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt


2019-02-20 11:42:17

by Steffen Klassert

[permalink] [raw]
Subject: Re: [RFC PATCH] crypto: pcrypt - forbid recursive instantiation

On Wed, Feb 20, 2019 at 12:06:28PM +0800, Herbert Xu wrote:
> On Wed, Apr 18, 2018 at 07:35:33AM +0200, Steffen Klassert wrote:
> >
> > Yes sure, I just wanted to know if it is worth to think about
> > preventing template recursions. If there is a valid usecase,
> > then we don't even need to think in this direction.
> >
> > While I think each pcrypt instance should have it's own
> > padata instance on the long run, it would be good to have
> > a not so intrusive fix that can be backported to the stable
> > trees.
>
> Steffen, has there been any progress on this work?

I had a look on what we need to use separate padata
instances for each pcrypt instance. But that's comlicated
and will create incompatibilities on the sysfs cpuset
configuration options. So that's not really a thing that
could be a fix.

>
> We need to fix this soon or we'll have to disable pcrypt because
> it is a security issue.
>
> It's not just about nested templates either. You can trigger
> the same issue where a pcrypt instance over an AEAD algorithm
> that uses a fallback which also happens to be pcrypt.

Would it be possible to forbid pcrypt for algorithms
that needs a fallback?

If I see this correct, only crypto algorithms used by
HW crypto accelerators need a fallback to software
crypto. HW crypto can't benefit from pcrypt anyway,
so it would be no loss to disable pcrypt in that case.

We could use the initial fix from Eric in combination
with disableing pcrypt for algorithms that need a fallback.


2019-03-01 03:38:11

by Herbert Xu

[permalink] [raw]
Subject: Re: [RFC PATCH] crypto: pcrypt - forbid recursive instantiation

On Wed, Feb 20, 2019 at 12:42:08PM +0100, Steffen Klassert wrote:
>
> I had a look on what we need to use separate padata
> instances for each pcrypt instance. But that's comlicated
> and will create incompatibilities on the sysfs cpuset
> configuration options. So that's not really a thing that
> could be a fix.

I don't see why it'll create incompatibilities for the cpu mask
configuration. You can just maintain two global configuration
masks as you do now, and read them from each instance. What's
the problem with that?

> > We need to fix this soon or we'll have to disable pcrypt because
> > it is a security issue.
> >
> > It's not just about nested templates either. You can trigger
> > the same issue where a pcrypt instance over an AEAD algorithm
> > that uses a fallback which also happens to be pcrypt.
>
> Would it be possible to forbid pcrypt for algorithms
> that needs a fallback?

This is complicated and not fool-proof. pcrypt itself might
be embedded into some other algorithm directly rather than as
a fallback.

Basically anyone who does a crypto_alloc_aead could end up with
a pcrypt instance unwittingly. If they were then used as part
of another AEAD algorithm's encrypt/decrypt path then we'd have
a dead-lock as we do now.

Granted we may not have this situation right now but relying on
it to never occur is not a good choice IMHO.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt