Re: [PATCH v2] crypto: AF_ALG - limit mask and type

[Herbert Xu]

On Fri, Dec 22, 2017 at 08:41:10AM +0100, Stephan Mueller wrote:
>
> Shouldn't we then rather use a white list instead of a black list?
> > 
> > Most other problems however would be bugs in the template code.
> > The first thing a template does when it creates an instance is
> > to check whether the resulting algorithm would fulfil the requested
> > type/mask using crypto_check_attr_type.  So if that's not working
> > then we should fix it there as it may also be triggered via other
> > code paths that can create instances.
> 
> I think I understand that, but does user space need to utilize this logic?

I'm open to a white list as well.  But we should certainly fix
the underlying bugs rather than just papering over them as they
could come back via other channels.

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

Re: [PATCH v2] crypto: AF_ALG - limit mask and type

[Stephan Mueller]

Am Freitag, 22. Dezember 2017, 08:36:07 CET schrieb Herbert Xu:

Hi Herbert,

> On Tue, Dec 19, 2017 at 07:25:04AM +0100, Stephan Müller wrote:
> > The user space interface allows specifying the type and the mask field
> > used to allocate the cipher. As user space can precisely select the
> > desired cipher by using either the name or the driver name, additional
> > selection options for cipher are not considered necessary and relevant
> > for user space.
> > 
> > This fixes a bug where user space is able to cause one cipher to be
> > registered multiple times potentially exhausting kernel memory.
> > 
> > Reported-by: syzbot 
> > Cc: 
> > Signed-off-by: Stephan Mueller 
> 
> This will break users of CRYPTO_ALG_KERN_DRIVER_ONLY.  I think
> we should add CRYPTO_ALG_TESTED to the blacklist since there is
> no sane reason to use it here.

Shouldn't we then rather use a white list instead of a black list?
> 
> Most other problems however would be bugs in the template code.
> The first thing a template does when it creates an instance is
> to check whether the resulting algorithm would fulfil the requested
> type/mask using crypto_check_attr_type.  So if that's not working
> then we should fix it there as it may also be triggered via other
> code paths that can create instances.

I think I understand that, but does user space need to utilize this logic?
> 
> Thanks,



Ciao
Stephan

Re: [PATCH v2] crypto: AF_ALG - limit mask and type

[Herbert Xu]

On Tue, Dec 19, 2017 at 07:25:04AM +0100, Stephan Müller wrote:
> The user space interface allows specifying the type and the mask field
> used to allocate the cipher. As user space can precisely select the
> desired cipher by using either the name or the driver name, additional
> selection options for cipher are not considered necessary and relevant
> for user space.
> 
> This fixes a bug where user space is able to cause one cipher to be
> registered multiple times potentially exhausting kernel memory.
> 
> Reported-by: syzbot 
> Cc: 
> Signed-off-by: Stephan Mueller 

This will break users of CRYPTO_ALG_KERN_DRIVER_ONLY.  I think
we should add CRYPTO_ALG_TESTED to the blacklist since there is
no sane reason to use it here.

Most other problems however would be bugs in the template code.
The first thing a template does when it creates an instance is
to check whether the resulting algorithm would fulfil the requested
type/mask using crypto_check_attr_type.  So if that's not working
then we should fix it there as it may also be triggered via other
code paths that can create instances.

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