Vladimir Kotal wrote: > > Hi all, > > I'd like to ask for a review of CR 6786946 which addresses the first > part of SCF's sub-optimality in terms of handling operations with keys > (or key attributes) longer than what a provider is capable of. > > Fix for CR 6786946 checks the key length for all reasonable operations > against mechanism information for given provider and signals > CRYPTO_KEY_SIZE_RANGE up so metaslot can fall back to softtoken (this > worked previously but possibly with different error code which came from > the driver of given provider) or the error is returned to the caller (in > case of direct access). The main thing is that the check is now done in > KcF instead of the driver for particular provider.
Before I go and look at the code in detail I have some higher level issues that need to be addressed: 1) Why does crypto_check_prov_mech_keylen() need to be part of the API ? ie why is it a crypto_ function rather than in internal kcf_ function ? Why would any caller of of the KCF API want to call this on its own ? 2) What is the performance impact of all this extra checking ? No performance regression is acceptable for kcf here. 3) Why are we not doing the checking in metaslot itself up in userland rather than sending the jobs to the kernel and having them fail ? Doing the check in userland means it will be of benefit even to other providers plugged into libpkcs11. Where as this fix appears to only benefit providers plugged into kcf - which is useful though. 4) Since you are changing the DES implementation to support 2DES (that is what you are doing right ?) you should do so consistently and update pkcs11_softtoken to support it as well. This change for supporting 2DES has test suite impact if a new mech is advertised. -- Darren J Moffat
