Darren J Moffat wrote: <snip>
> 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 ? It doesn't have to be. > 2) What is the performance impact of all this extra checking ? > No performance regression is acceptable for kcf here. Let's see: for each operation the new function will perform mech_info lookup via KCF_TO_PROV_MECHINFO macro which is direct access to an array. Then it will do a integer comparison and some arithmetics to see the units and finally it will do 2 integer comparisons to see if the key length fits the interval. I'd say other than the function call performance impact is negligible (see how kcf_get_hardware_provider() works, e.g.). That said, I can easily convert the function to a macro of make it inline. > 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. See 6784968, 6266218 and 6762069. > 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. Actually, I'm only fixing things up in terms of what providers claim about themselves. v.
