Support import certificate using softhsm2-util

[alonbl]

The softhsm2-util already support importing keys, why not also import
certificates?

Useful for test scripts that require both keys and certificates.

Add --import-type parameter, depreciate the --aes parameter.

Signed-off-by: Alon Bar-Lev alon.barlev@gmail.com

[rijswijk]

Thank you for your contribution, I have some things to consider before merging:

Certificate import may be slightly more complex, since certificates are often associated with keys. Different applications link
keys and certificates in different ways (e.g. one example might be to have matching CKA_ID attributes, but other options also exist). I'm not entirely sure whether adding this type of support to softhsm2-util is the right way forward, it might be too narrowly scoped to a particular use case.

@halderen your opinion would be welcome.

If we do decide to proceed with this, I think the following changes/features would be necessary:

• Don't do an immediate deprecation of --import, instead, clearly mark it as deprecated in the documentation (usage, man page) and tag it for future removal (while you still support it, it is immediately deprecated without documentation)
• Support Botan as well (to keep the two builds in sync)

[alonbl]

Hi,

Thank you for the review. I altered the implementation and added --import-type instead of deprecating the --import, I hope you like it better.

For the matching between key and certificate, as far as I know the best practice is to match the CKA_ID or read the entire token and match based on public key. The option to even having a basic import is significant for the use case of using the provider as test framework. If an application has special needs it can always perform the import by its own.

I have been around for years in PKCS#11 domain and never saw a setup in which the CKA_ID is not used for matching.

If you ACK I will update the man page and try to hack the botan as well.

Regards,
Alon

[alonbl]

The botan part was actually simpler than I thought, so I applied the botan part and the man part.

[rijswijk]

Thanks, looks good, one small request: the PR is still essentially deprecating the current mode of use of --import (which essentially assumes it's always a key pair unless --aes is specified). Furthermore, it looks like you removed the documentation for --aes from the manual page while the option is still silently supported. To really make it a "nice" deprecation, I suggest the following:

• If --aes is specified, print a warning to stderr that says something like "Warning: use of --aes is deprecated, this flag will be removed in a future version"
• If --import is specified without --import-type, print a warning to stderr that says something like "Warning: use of --import without specifying the object type with --import-type is deprecated, assuming "keypair"; future versions will give an error in this case."
• Clear flag the deprecated behaviour in the manual page

Again, thanks for your contribution, I think with the changes above we'd be good to go for a review by @halderen and a merge.

[alonbl]

Hi, I left the —aes in the man page, just removed it from the first line. And added depreciation notice in parameter usage, so I do not think anything is missing... just less endorsement for the parameter. :) The past addition of the —aes was very confusing, I only realized it’s usage when reading the code. For the stderr messages, I do not like it, it may break existing scripts. As long as we support backward compatibility, I thing the risk of breakage due to output is wrong. I believe documentation should be sufficient. What do you think? Thanks, Alon

[rijswijk]

I think I agree, I'm going to ask @halderen to review and merge. Thanks!

[alonbl]

ping?

[alonbl]

Hi @halderen,
Can you please also checkout this one?
Thanks!

[davydsantos]

+1 on the feature. This can be accomplished by using opensc/pkcs11-tool but it would be easier to rely on one single tool for both tasks. Another possible feature could be to allow importing a p12/pfx directly without the need of breaking it up in 2 different files and it would handle everything internally.

[alonbl]

Rebased.

Hi @halderen,
Can you please also checkout this one? I will be glad to accept any feedback.
Thanks!

[alonbl]

Hi @halderen,
Do you have time to review this?
Thanks.

[alonbl]

ping?
I see almost no activity in project, any chance to merge it? feature was acked and change was reviewed.
thanks!

[alonbl]

Hello @halderen, can you please look at this patch? It would be great if we can use the softhsm2-util to import certificates for unit tests without additional software.

[tougantium]

@halderen: I would also appreciate this PR very much, could you please consider merging.

[alonbl]

Rebased.

@halderen: can you please review? this is super handy utility for unittest of PKCS#11 enabled application.

[jschlyter]

Please rebase on develop and mark as ready when ready.

[alonbl]

Please rebase on develop and mark as ready when ready.

Hi @jschlyter,
Done :)
Thanks!

[alonbl]

Hello @ijsf ,

Thank you for reviewing.
I pushed a new revision, I hope it will address your concerns.

Regards,

[alonbl]

Hello @jschlyter, is there anything more I can do to push this forward?

[jschlyter]

I think we're good, but I'd appreciate if @bjosv could take a look as well.

[bjosv]

Looks useful, just have a few comment before a merge.

[alonbl]

@bjosv thank you for spotting this leftover!

I hope all is good now.

[bjosv]

LGTM!

[jschlyter]

One more thing @alonbl, could you add a test case for the import function in a separate PR?

[alonbl]

Hello @jschlyter,

I do not see any test cases for the usage of the utilities.
I may miss this, if there is infrastructure for this, please refer me and I will add this specific test case.

Thanks,
Alon