I think that retrying key generation when it fails with the default value (= unset) with CKA_PRIVATE=true is probably the best solution.

The additional parameter is confusing and formalises what according to AWS their docs is an incompatibility in the CloudHSM API, as default behaviour.

@ties: This PR preserves the existing Krill behaviour by default for backward compatibility with earlier versions of Krill, while adding a new configuration setting to allow the operator to explicitly control the behaviour as needed.

I considered the automatic fallback behaviour but am concerned that it is magic and unclear to operators unless they happen to notice a log entry that we might add indicating that fallback behaviour had been used. We discussed this internally and felt that an explicit configuration setting was preferable.

I decided to investigate further what it would take to do the magic fallback approach instead, i.e. on CKR_INCONSISTENT_TEMPLATE retry omitting the CKA_PRIVATE attribute in the public key generation template.

One problem I hadn't foreseen with this approach is that the current signer registration model is signer implementation independent, there's no way to store a specific flag for a PKCS#11 signer as the same common fields are stored for OpenSSL, KMIP and PKCS#11 signers. I could force the flag into a string type, maybe even force it into the descriptive strings held for each signer, but strong typing is much preferred over basing behaviour on string content.

Of course it could be done, but it would be even more work than I thought. For example we would also have to support existing signer registration details stored on disk that lack the extra field and assume that the absence of the field means that CKA_PRIVATE should be set to CK_TRUE, or "upgrade" the signer registration store on first start of the newer version of Krill to include the missing field.

I can see the attraction of the automatic fallback, hence why I proposed it as an option in the first place, but I'm still wary of the magic quality of this behaviour vs the explicit nature of a config setting.

One improvement I could make to the current implementation is on CKR_INCONSISTENT_TEMPLATE error when probing a signer, to log a helpful error such as "One possible cause for this error is that the signer doesn't support authenticated pubkey_access. If this is the case, setting pubkey_access to token-default or unauthenticated in your Krill configuration file may be enough to make the signer usable by Krill.".

One improvement I could make to the current implementation is on CKR_INCONSISTENT_TEMPLATE error when probing a signer, to log a helpful error such as "One possible cause for this error is that the signer doesn't support authenticated pubkey_access. If this is the case, setting pubkey_access to token-default or unauthenticated in your Krill configuration file may be enough to make the signer usable by Krill.".

This sounds like the right fix.

One improvement I could make to the current implementation is on CKR_INCONSISTENT_TEMPLATE error when probing a signer, to log a helpful error such as "One possible cause for this error is that the signer doesn't support authenticated pubkey_access. If this is the case, setting pubkey_access to token-default or unauthenticated in your Krill configuration file may be enough to make the signer usable by Krill.".

This sounds like the right fix.

I have pushed a commit to this PR to add an error message similar to this. It was more work than I expected because I discovered that (a) the underlying error was not accessible to higher level logic even within the context of the PKCS#11 signer, and (b) that once augmented the modified error message was being wrongly dropped and never included in the logs. I also made some warnings about signer registration/detection failure into errors to be consistent with other nearby logged messages which were already errors, and because if you configure a signer it is a pretty serious error if it cannot be used.

Update: I tested this locally using SoftHSMv2. As I couldn't find a way to make SoftHSMv2 return the CKR_INCONSISTENT_TEMPLATE error when C_GenerateKeyPair() is invoked I instead temporarily modified my code to wrongly pass an additional CKA_CLASS attribute with value CKO_PRIVATE_KEY in the public key template, and modified the error catching code to catch the resulting CKR_ATTRIBUTE_VALUE_INVALID error. I was then able to see the error in the log by starting Krill and attempting to create a CA through the UI.

@timbru : It might be good if you look at this again given my last commit and comment above.

Note that the config setting added in this PR was replaced by different settings added in PR #1020.