On 6/27/2019 5:54 PM, Ard Biesheuvel wrote:
> On Thu, 27 Jun 2019 at 16:50, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>>
>> On Thu, 27 Jun 2019 at 16:44, Horia Geanta <horia.geanta@xxxxxxx> wrote:
>>>
>>> On 6/27/2019 3:03 PM, Ard Biesheuvel wrote:
>>>> n my effort to remove crypto_alloc_cipher() invocations from non-crypto
>>>> code, i ran into a DES call in the CIFS driver. This is addressed in
>>>> patch #30.
>>>>
>>>> The other patches are cleanups for the quirky DES interface, and lots
>>>> of duplication of the weak key checks etc.
>>>>
>>>> Changes since vpub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/fixes1/RFC:
>>>> - fix build errors in various drivers that i failed to catch in my
>>>> initial testing
>>>> - put all caam changes into the correct patch
>>>> - fix weak key handling error flagged by the self tests, as reported
>>>> by Eric.
>>> I am seeing a similar (?) issue:
>>> alg: skcipher: ecb-des-caam setkey failed on test vector 4; expected_error=-22, actual_error=-126, flags=0x100101
>>>
>>> crypto_des_verify_key() in include/crypto/internal/des.h returns -ENOKEY,
>>> while testmgr expects -EINVAL (setkey_error = -EINVAL in the test vector).
>>>
>>> I assume crypto_des_verify_key() should return -EINVAL, not -ENOKEY.
>>>
>>
>> Yes, but I tried to keep handling of the crypto_tfm flags out of the
>> library interface.
>>
>> I will try to find a way to solve this cleanly.
>
> Actually, it should be as simple as
>
> diff --git a/include/crypto/internal/des.h b/include/crypto/internal/des.h
> index dfe5e8f92270..522c09c08002 100644
> --- a/include/crypto/internal/des.h
> +++ b/include/crypto/internal/des.h
> @@ -27,10 +27,12 @@ static inline int crypto_des_verify_key(struct
> crypto_tfm *tfm, const u8 *key)
> int err;
>
> err = des_expand_key(&tmp, key, DES_KEY_SIZE);
> - if (err == -ENOKEY &&
> - !(crypto_tfm_get_flags(tfm) & CRYPTO_TFM_REQ_FORBID_WEAK_KEYS))
> - err = 0;
> -
> + if (err == -ENOKEY) {
> + if (crypto_tfm_get_flags(tfm) & CRYPTO_TFM_REQ_FORBID_WEAK_KEYS)
> + err = -EINVAL;
> + else
> + err = 0;
> + }
> if (err)
> crypto_tfm_set_flags(tfm, CRYPTO_TFM_RES_WEAK_KEY);
>
Confirming this works for me (on CAAM accelerator).
Thanks,
Horia
