Implement support for ALWAYS_AUTH and interactive prompting when the caller did not provide pin #309
Conversation
src/session.c
Outdated
| ret = CKR_GENERAL_ERROR; | ||
| goto done; | ||
| /* this error can mean anything from the user canceling the prompt to no UI method provided | ||
| * Fall back to our prompt here */ |
There was a problem hiding this comment.
if we did have a callback I do not think we should repeat the prompting,
we should prompt on our own if no callback is present, gated behind a config option that enables this behavior.
There was a problem hiding this comment.
As described in this comment, from store, we are always getting a callback, but if the calling application does not provide the UI, it just fails. I think we are getting the pointer to ossl_pw_get_passphrase() function from crypto/passphrase.c unconditionally. And based on what application provided, it works of fails with No password method specified:
https://github.com/openssl/openssl/blob/cf6342bc024868f5a55f2225f2e083415fb1329a/crypto/passphrase.c#L275
src/session.c
Outdated
| goto done; | ||
| /* We are asking the user off-band for the user consent -- from store | ||
| * we will always receive non-null (but unusable) callback */ | ||
| ret = p11prov_session_get_pin(slot, (char *)cb_pin, &cb_pin_len); |
Jakuje
force-pushed
the
always_auth
branch
3 times, most recently
from
e12ca0e to
9aec79f
Compare
Jakuje
force-pushed
the
always_auth
branch
2 times, most recently
from
c88fae3 to
a69e05f
Compare
| } | ||
| } else { | ||
| /* We are asking the user off-band for the user consent -- from | ||
| * store we will always receive non-null (but unusable) callback |
There was a problem hiding this comment.
Not sure why this is unusable? When we receive a callback it is always usable, the problem is that often we just do not have one, and we can't store the callback we were provided for the store operation for later operations as the calling application may not expect it.
There was a problem hiding this comment.
As discussed above, from the store we will always get a callback (at least in OpenSSL in Fedora 38), but the callback just fails with the error mentioned above.
There was a problem hiding this comment.
Sorry I do not get it, if pw_cb is empty it means we got a NULL callback, not an unusable one, so I am not sure what this comment means.
I guess we can change it later, but I would relaly like to understand what you meant to convey here.
There was a problem hiding this comment.
Its the conversion from the UI methods on the store API to the low-level callbacks on the provider level. What you describe would make sense and I would be assuming this, but this is OpenSSL. So when the store API is called without the UI method, the provider gets non-null callback, but that one fails because there is no assigned UI method. Sounds as stupid as it is (or I followed the gdb wrongly -- there is obviously no documentation. You can double-check that this wont work if you remove this code branch).
Fixes latchset#42 Signed-off-by: Simo Sorce <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
Fixes: latchset#295 Signed-off-by: Jakub Jelen <[email protected]>
Signed-off-by: Jakub Jelen <[email protected]>
So lets keep it for now without the configuration option discussed previously? |
|
Noting here, that the macos CI picked up gnutls 3.8.2 with the following regression in handling ed25519 keys: https://gitlab.com/gnutls/gnutls/-/issues/1515 |
And now we have the broken gnutls also in Fedora 39. The update to Fedora 38 is on the way too: https://bodhi.fedoraproject.org/updates/FEDORA-2023-e075ac32be |
|
Merged, and opened a followup issue for the gating config option. |
Based on #298. The current version passes all existing tests and the default UI is not gated with a configuration option (but this can be added if needed).
Fixes #295 and #42