code reviews for 6474964 and 6545665

[Mark Powers]

Darren J Moffat wrote:
> Mark Powers wrote:
>> Folks,
>>
>> I need a code review for:
>>
>> 6474964 cryptoadm(1M) displays duplicate mechanisms for hardware 
>> providers
>
> Is this really solving the real issue here ?  Is the real issue that 
> we are returning duplicate mechanisms over the ioctl or is it that we 
> allowed registration of duplicate mechanisms ?  Is this the best place 
> to weed out the duplicates ?  Or put another way are the duplicates 
> useful to kcf but not to pkcs11_kernel ?

The same mechanism can be used for encryption/decryption
and wrap/unwrap. In the case of the SCA-6000:
CKM_DES_CBC encrypt/decrypt is registered for mca0:CB provider and
CKM_DES_CBC wrap/unwrap is registered for mca0:OM provider.

The ioctl that returns the mechanisms supported by a device reports
all mechanisms from all registrations of that device without checking
for duplicates. I believe it is the ioctl (kernel) that needs to be fixed.
>
>> 6545665 make CKM_AES_CTR available to non-kernel users
>
> The code changese look just fine I'm happy to be listed as a 
> codereviewer.
>
> One nit on the import of the new header files you updated the string 
> to number table for  pkcs11_mech2str() libcryptotuil but you only did 
> so for CKM_AES_CTR not all the new mechanisms in the new PKCS#11 
> header file.  That mapping table is supposed to contain all the mechs 
> even if we don't implement them since it is used by cryptoadm to 
> display the mechanism names a provider has registered.

You're right. I should have noticed all of those mechanisms we don't 
support.
I'll add add the new mechanisms.

>
> The bug should be moved to pkcs11_kernel sub-category since it doesn't 
> apply to pkcs11_softtoken.  If it doesn't already exists please log a 
> bug for having AES_CTR available from pkcs11_softtoken and cross ref 
> to this one.

Okay.

Thanks for the review Darren!