-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement support for CKA_ALWAYS_AUTHENTICATE #298
Conversation
Fixes latchset#42 Signed-off-by: Simo Sorce <[email protected]>
I think in softhsm, this could be automated as when you create the key in softhsm you can provide the private key template and OpenSC's pkcs11-tool has |
@Jakuje once NSS in F38 is fixed and we get CI working again, would you be willing to attempt writing a test with that option ? |
Sure, I can give it a try. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added couple of commits implementing a test (unfortunately, there is no simple way to check the context specific login happens except for reading through the logs now) for this and fixing some issues noticed on the way in:
https://github.com/Jakuje/pkcs11-provider/commits/always_auth
Feel free to pull the changes if you consider them useful.
} | ||
|
||
if (always_auth) { | ||
ret = p11prov_context_specific_login(session, NULL, NULL, NULL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the p11prov_get_session
failed above (for example because of unknown algorithm), you are passing here the NULL
session, which leads to crashing in p11prov_context_specific_login
when you dereference the session
pointer.
I think the above switch needs to be extended with goto done
in case of failure.
The current implementation also prevents any interactive prompting for the PIN when it is not provided through any other means (URI/config/cached) as the callbacks are null here. Generally, we can assume PDF signing software, where the user needs to provide consent in the form of PIN to sign some (possibly legally binding) document. In this use case, the pin will not be in the URI, config and I believe it should not be cached either and the application should provide a PIN prompt for the signature. Not sure if this is simply possible though as I see that the UI method is passed only to the store API ...
Closing in favor of #309 |
Fixes #42
Unfortunately I do not have a way to add a test for this in CI just yet, so some manual testing will be required before merging.