mark powers wrote: > kcf_callprov.c > -------------- > mcp-0 line 119 I don't think the assert is that useful since > key != NULL is checked before the function is called.
I added it just for the case of someone calling kcf_check_prov_mech_keylen() in the future without checking the key pointer first. It is just a safety belt. It's not necessary so I have removed it. > mcp-0 lines 122-129 This code is duplicated three times in this > function. It could be moved to before the > actual check on line 180. Those instances are not the same. For CRYPTO_KEY_RAW key types it uses key->ck_length, for CRYPTO_KEY_ATTR_LIST it has to get the attribute length first. Also, CRYPTO_KEY_RAW key lengths are converted from bits to bytes using the CRYPTO_BITS2BYTES macro. In fact, they are doing different conversions - shifting versus multiplication. Lastly, moving the whole block (assuming generic macro for both types) before the switch statement would be suboptimal for CRYPTO_KEY_REFERENCE key type and other types. I can make a macro just for the CRYPTO_KEY_ATTR_LIST key type assignments where there is indeed a duplication - would this be okay ? > mcp-3 Related to Krishna's comment (KY-1), wouldn't it be better to > always return CRYPTO_KEY_SIZE_RANGE instead of > CRYPTO_MECH_NOT_SUPPORTED? This is easy to do for kcf_get_hardware_provider() in case the provider is not logical provider. As for the other cases, I am not sure - the key length check is now part of provider selection (it's one of the criteria for selection) so from this perspective it should fall under CRYPTO_MECH_NOT_SUPPORTED. Incremental webrev (on top of already approved changes) is here: http://cr.opensolaris.org/~vkotal/kcf-keylen_check-6786946.onnv.Mark/ v.
