Fix for PKCS11_store_certificate generates CKR_ATTRIBUTE_TYPE_INVALID error

[mihaicristiantanase]

This fixes the issue https://github.com/OpenSC/libp11/issues/518

[dengert]

C_GetInfo is called when the module is loaded:

https://github.com/OpenSC/libp11/blob/master/src/p11_load.c#L94-L107

The version number could be save once in PKCS11_CTX, rather then calling C_GetInfo each time the library version is needed. Personally, I would save it as a CK_VERSION library_version structure. Or could be saved as major * 1000 + minor as you are doing.
There may be other places in libp11 that might need the version number too.

@mtrojnar comments?

[mtrojnar]

I fully support @dengert's suggestions.

Also, please do not add another source file for two trivial functions. As @dengert pointed out, it's one PKCS11 API call after the module is loaded and initialized, and one field in the context structure, so no new functions seem to be needed.

[mihaicristiantanase]

Thanks for the feedback.

First, saving the version at library load is a good idea. The only problem I see in reusing CK_VERSION is that I need to include "pkcs11.h" into "libp11.h", which is probably not a good idea. I'm thinking of creating a different type in "libp11.h" (ex: Version) that mirrors CK_VERSION. @dengert, what do you think?

Second, I don't see the issue with adding a new source file. Even if a source file contains trivial content, you don't really want to repeat it in all places it is used. Probably I'm missing something here... Anyway, @mtrojnar, where do you suggest I should add version related logic?

[mtrojnar]

Adding https://github.com/OpenSC/libp11/commit/9ac6919996beb76361c2d00faec1af7051c84c99 was easier than explaining.

Avoid converting them to an integer or adding an encapsulation layer. An additional abstraction layer makes the code harder instead of simpler to understand. Simply create and appropriate condition every time it's needed.

General advice: Use the KISS principle.

[mihaicristiantanase]

https://github.com/OpenSC/libp11/commit/9ac6919996beb76361c2d00faec1af7051c84c99 adds a new type to PKCS11_CTX. Any idea how can this be accessed from a PKCS11_SLOT_private object?

[mtrojnar]

Good point. It shouldn't go into the public API anyway. Apply https://github.com/OpenSC/libp11/commit/c6e69c944dcedf5a57d6d7a363f83c023ba4813e and refer to slot->ctx->cryptoki_version.major and slot->ctx->cryptoki_version.minor.

[mihaicristiantanase]

I've used the simpler CK_VERSION instead of struct _CK_VERSION. @mtrojnar, is there a reason you used the latter in PKCS11_SLOT_private? Also, I've added a version comparison function in p11_cert.c instead of using a new source file. Probably, this will be moved to a separate place when the comparison is needed elsewhere.

[mtrojnar]

Thank you very much for your work.

[mihaicristiantanase]

No problem. Thank you for merging the changes!