crypto: user - crypto_alg_match removal

crypto: user - crypto_alg_match removal

[Stephan Mueller]

Hi Steffen, Herbert,

may I ask for the reasons why crypto_alg_match exists? Doesn't it 
implement crypto_alg_lookup -- and that not even complete? Is there a 
particular reason why this exact flag of crypto_alg_match is really 
needed in the context of crypto_user?

Unless there is such valid reason, may I ask whether we can remove 
crypto_alg_match and simply use crypto_alg_lookup in all instances where 
crypto_alg_match is invoked using the following replacement:

alg = crypto_alg_lookup(p->cru_name, p->cru_type, p->cru_mask)

The only problem with this replacement is that p->cru_driver_name is not 
considered with that. Do you think a double invocation of 
crypto_alg_lookup should be done or that even the user space interface 
should be changed such that cru_driver_name is removed from it?

Note, this change would now imply that crypto_user follows the kernel-
internal crypto API invocation approach.

Thanks
Stephan

Re: crypto: user - crypto_alg_match removal

[Steffen Klassert]

Hi Stephan.

On Sat, Nov 22, 2014 at 11:25:05PM +0100, Stephan Mueller wrote:
> Hi Steffen, Herbert,
> 
> may I ask for the reasons why crypto_alg_match exists? Doesn't it 
> implement crypto_alg_lookup -- and that not even complete? Is there a 
> particular reason why this exact flag of crypto_alg_match is really 
> needed in the context of crypto_user?
> 
> Unless there is such valid reason, may I ask whether we can remove 
> crypto_alg_match and simply use crypto_alg_lookup in all instances where 
> crypto_alg_match is invoked using the following replacement:
> 
> alg = crypto_alg_lookup(p->cru_name, p->cru_type, p->cru_mask)
> 
> The only problem with this replacement is that p->cru_driver_name is not 
> considered with that. 

With crypto_alg_lookup() we don't know whether the match is based on
the driver or the algorithm name. That's why we have crypto_alg_match(),
here we can ask for a driver or an algorithm match. In some situations it
is important to have an exact match on the crypto driver name. For example
if a user wants to instantiate or delete a certain inplementation of an
algorithm. In this case we need to know whether this exact algorithm
driver is registered in the system.

> Do you think a double invocation of 
> crypto_alg_lookup should be done or that even the user space interface 
> should be changed such that cru_driver_name is removed from it?

Whatever we do, we can't remove cru_driver_name as this is exported
to userspace and tools already use it.

Re: crypto: user - crypto_alg_match removal

[Stephan Mueller]

Am Montag, 24. November 2014, 08:22:46 schrieb Steffen Klassert:

Hi Steffen,

> Hi Stephan.
> 
> On Sat, Nov 22, 2014 at 11:25:05PM +0100, Stephan Mueller wrote:
> > Hi Steffen, Herbert,
> > 
> > may I ask for the reasons why crypto_alg_match exists? Doesn't it
> > implement crypto_alg_lookup -- and that not even complete? Is there a
> > particular reason why this exact flag of crypto_alg_match is really
> > needed in the context of crypto_user?
> > 
> > Unless there is such valid reason, may I ask whether we can remove
> > crypto_alg_match and simply use crypto_alg_lookup in all instances where
> > crypto_alg_match is invoked using the following replacement:
> > 
> > alg = crypto_alg_lookup(p->cru_name, p->cru_type, p->cru_mask)
> > 
> > The only problem with this replacement is that p->cru_driver_name is not
> > considered with that.
> 
> With crypto_alg_lookup() we don't know whether the match is based on
> the driver or the algorithm name. That's why we have crypto_alg_match(),
> here we can ask for a driver or an algorithm match. In some situations it
> is important to have an exact match on the crypto driver name. For example
> if a user wants to instantiate or delete a certain inplementation of an
> algorithm. In this case we need to know whether this exact algorithm
> driver is registered in the system.

I understand. But going with the logic of the kernel crypto API, if one needs 
an exact match, you pick the driver name. Otherwise the generic name. 
crypto_alg_lookup returns the exact algo when you supply a driver name. It 
returns the algo with the highest prio when you supply a generic name.

I do not see a difference for the scenarios you describe. What I am worried 
about is that the logic of how a name versus a driver name is applied differs 
between the kernel crypto API and crypto_user.


> 
> > Do you think a double invocation of
> > crypto_alg_lookup should be done or that even the user space interface
> > should be changed such that cru_driver_name is removed from it?
> 
> Whatever we do, we can't remove cru_driver_name as this is exported
> to userspace and tools already use it.

That is definitely an issue. But the more I think about it, the more I see 
that we do not need to change the interface.

Something like that would work in the kernel:

if (p->cru_driver_name[0])
	alg = crypto_alg_lookup(p->cru_driver_name, p->cru_type, p->cru_mask)
else
	alg = crypto_alg_lookup(p->cru_name, p->cru_type, p->cru_mask)

-- 
Ciao
Stephan

Re: crypto: user - crypto_alg_match removal

[Steffen Klassert]

On Mon, Nov 24, 2014 at 01:29:10PM +0100, Stephan Mueller wrote:
> Am Montag, 24. November 2014, 08:22:46 schrieb Steffen Klassert:
> 
> > With crypto_alg_lookup() we don't know whether the match is based on
> > the driver or the algorithm name. That's why we have crypto_alg_match(),
> > here we can ask for a driver or an algorithm match. In some situations it
> > is important to have an exact match on the crypto driver name. For example
> > if a user wants to instantiate or delete a certain inplementation of an
> > algorithm. In this case we need to know whether this exact algorithm
> > driver is registered in the system.
> 
> I understand. But going with the logic of the kernel crypto API, if one needs 
> an exact match, you pick the driver name. Otherwise the generic name. 
> crypto_alg_lookup returns the exact algo when you supply a driver name. It 
> returns the algo with the highest prio when you supply a generic name.
> 
> I do not see a difference for the scenarios you describe.

Well, I think there is a small but important difference. If a user
requests a driver name that would match an algorithm name (i.e. cbc(aes)
instead of cbc(aes-asm)) crypto_alg_lookup() returns the algorithm with
the highest priority instead of telling that we don't have a driver with
the name cbc(aes).

Re: crypto: user - crypto_alg_match removal

[Stephan Mueller]

Am Dienstag, 25. November 2014, 09:42:25 schrieb Steffen Klassert:

Hi Steffen,

>On Mon, Nov 24, 2014 at 01:29:10PM +0100, Stephan Mueller wrote:
>> Am Montag, 24. November 2014, 08:22:46 schrieb Steffen Klassert:
>> > With crypto_alg_lookup() we don't know whether the match is based
>> > on
>> > the driver or the algorithm name. That's why we have
>> > crypto_alg_match(), here we can ask for a driver or an algorithm
>> > match. In some situations it is important to have an exact match
>> > on the crypto driver name. For example if a user wants to
>> > instantiate or delete a certain inplementation of an algorithm. In
>> > this case we need to know whether this exact algorithm driver is
>> > registered in the system.
>> 
>> I understand. But going with the logic of the kernel crypto API, if
>> one needs an exact match, you pick the driver name. Otherwise the
>> generic name. crypto_alg_lookup returns the exact algo when you
>> supply a driver name. It returns the algo with the highest prio when
>> you supply a generic name.
>> 
>> I do not see a difference for the scenarios you describe.
>
>Well, I think there is a small but important difference. If a user
>requests a driver name that would match an algorithm name (i.e.
>cbc(aes) instead of cbc(aes-asm)) crypto_alg_lookup() returns the
>algorithm with the highest priority instead of telling that we don't
>have a driver with the name cbc(aes).

Agreed. If this is a use case scenario that is needed, crypto_alg_lookup 
is not suitable.

Thanks for the clarification.

Ciao
Stephan

Re: crypto: user - crypto_alg_match removal

[Herbert Xu]

On Tue, Nov 25, 2014 at 09:42:25AM +0100, Steffen Klassert wrote:
>
> Well, I think there is a small but important difference. If a user
> requests a driver name that would match an algorithm name (i.e. cbc(aes)
> instead of cbc(aes-asm)) crypto_alg_lookup() returns the algorithm with
> the highest priority instead of telling that we don't have a driver with
> the name cbc(aes).

Does this matter though? The current user interface is only used to
query specific driver names which should never be equal to an
algorithm name.  So doing so already invokes undefined behaviour.

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

Re: crypto: user - crypto_alg_match removal

[Steffen Klassert]

On Tue, Nov 25, 2014 at 05:06:01PM +0800, Herbert Xu wrote:
> On Tue, Nov 25, 2014 at 09:42:25AM +0100, Steffen Klassert wrote:
> >
> > Well, I think there is a small but important difference. If a user
> > requests a driver name that would match an algorithm name (i.e. cbc(aes)
> > instead of cbc(aes-asm)) crypto_alg_lookup() returns the algorithm with
> > the highest priority instead of telling that we don't have a driver with
> > the name cbc(aes).
> 
> Does this matter though? The current user interface is only used to
> query specific driver names which should never be equal to an
> algorithm name.  So doing so already invokes undefined behaviour.
> 

Using an algorithm name as a driver name is a misconfiguration.
We currently, catch these kind of misconfigurations beacuse
we match only driver names with crypto_alg_match().

crypto_alg_lookup() would return the algorithm with the highest
priority in this case and this would indeed lead to undefined
behaviour.