Skip to content
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 ALWAYS_AUTH and interactive prompting when the caller did not provide pin #309

Merged
merged 7 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/asymmetric_cipher.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ static int p11prov_rsaenc_decrypt(void *ctx, unsigned char *out, size_t *outlen,
CK_OBJECT_HANDLE handle;
CK_ULONG out_size = *outlen;
int result = RET_OSSL_ERR;
bool always_auth = false;
CK_RV ret;

P11PROV_debug("decrypt (ctx=%p)", ctx);
Expand Down Expand Up @@ -296,6 +297,15 @@ static int p11prov_rsaenc_decrypt(void *ctx, unsigned char *out, size_t *outlen,
goto endsess;
}

always_auth =
p11prov_obj_get_bool(encctx->key, CKA_ALWAYS_AUTHENTICATE, false);
if (always_auth) {
ret = p11prov_context_specific_login(session, NULL, NULL, NULL);
if (ret != CKR_OK) {
goto endsess;
}
}

/* Special handling against PKCS#1 1.5 side channel leaking */
if (mechanism.mechanism == CKM_RSA_PKCS) {
CK_ULONG cond;
Expand Down
34 changes: 33 additions & 1 deletion src/objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,35 @@ CK_ATTRIBUTE *p11prov_obj_get_attr(P11PROV_OBJ *obj, CK_ATTRIBUTE_TYPE type)
return NULL;
}

bool p11prov_obj_get_bool(P11PROV_OBJ *obj, CK_ATTRIBUTE_TYPE type, bool def)
{
CK_ATTRIBUTE *attr = NULL;

if (!obj) {
return def;
}

for (int i = 0; i < obj->numattrs; i++) {
if (obj->attrs[i].type == type) {
attr = &obj->attrs[i];
}
}

if (!attr || !attr->pValue) {
return def;
}

if (attr->ulValueLen == sizeof(CK_BBOOL)) {
if (*((CK_BBOOL *)attr->pValue) == CK_FALSE) {
return false;
} else {
return true;
}
}

return def;
}

CK_KEY_TYPE p11prov_obj_get_key_type(P11PROV_OBJ *obj)
{
if (obj) {
Expand Down Expand Up @@ -566,8 +595,9 @@ P11PROV_CTX *p11prov_obj_get_prov_ctx(P11PROV_OBJ *obj)

/* CKA_ID
* CKA_LABEL
* CKA_ALWAYS_AUTHENTICATE
* CKA_ALLOWED_MECHANISMS see p11prov_obj_from_handle() */
#define BASE_KEY_ATTRS_NUM 3
#define BASE_KEY_ATTRS_NUM 4

#define RSA_ATTRS_NUM (BASE_KEY_ATTRS_NUM + 2)
static int fetch_rsa_key(P11PROV_CTX *ctx, P11PROV_SESSION *session,
Expand All @@ -588,6 +618,7 @@ static int fetch_rsa_key(P11PROV_CTX *ctx, P11PROV_SESSION *session,
FA_SET_BUF_ALLOC(attrs, num, CKA_PUBLIC_EXPONENT, true);
FA_SET_BUF_ALLOC(attrs, num, CKA_ID, false);
FA_SET_BUF_ALLOC(attrs, num, CKA_LABEL, false);
FA_SET_BUF_ALLOC(attrs, num, CKA_ALWAYS_AUTHENTICATE, false);
ret = p11prov_fetch_attributes(ctx, session, object, attrs, num);
if (ret != CKR_OK) {
/* free any allocated memory */
Expand Down Expand Up @@ -745,6 +776,7 @@ static CK_RV fetch_ec_key(P11PROV_CTX *ctx, P11PROV_SESSION *session,
}
FA_SET_BUF_ALLOC(attrs, num, CKA_ID, false);
FA_SET_BUF_ALLOC(attrs, num, CKA_LABEL, false);
FA_SET_BUF_ALLOC(attrs, num, CKA_ALWAYS_AUTHENTICATE, false);
ret = p11prov_fetch_attributes(ctx, session, object, attrs, num);
if (ret != CKR_OK) {
/* free any allocated memory */
Expand Down
1 change: 1 addition & 0 deletions src/objects.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ CK_SLOT_ID p11prov_obj_get_slotid(P11PROV_OBJ *obj);
CK_OBJECT_HANDLE p11prov_obj_get_handle(P11PROV_OBJ *obj);
CK_OBJECT_CLASS p11prov_obj_get_class(P11PROV_OBJ *obj);
CK_ATTRIBUTE *p11prov_obj_get_attr(P11PROV_OBJ *obj, CK_ATTRIBUTE_TYPE type);
bool p11prov_obj_get_bool(P11PROV_OBJ *obj, CK_ATTRIBUTE_TYPE type, bool def);
CK_KEY_TYPE p11prov_obj_get_key_type(P11PROV_OBJ *obj);
CK_ULONG p11prov_obj_get_key_bit_size(P11PROV_OBJ *obj);
CK_ULONG p11prov_obj_get_key_size(P11PROV_OBJ *obj);
Expand Down
1 change: 1 addition & 0 deletions src/provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <openssl/proverr.h>
#include <openssl/core_names.h>
#include <openssl/provider.h>
#include <openssl/ui.h>

#define UNUSED __attribute__((unused))
#define RET_OSSL_OK 1
Expand Down
125 changes: 106 additions & 19 deletions src/session.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,10 +387,49 @@ CK_SLOT_ID p11prov_session_slotid(P11PROV_SESSION *session)
return session->slotid;
}

static int p11prov_session_prompt_for_pin(struct p11prov_slot *slot,
char *cb_pin, size_t *cb_pin_len)
{
char *prompt = NULL;
UI *ui = UI_new_method(NULL);
const char *login_info = p11prov_slot_get_login_info(slot);
int ret;

P11PROV_debug("Starting internal PIN prompt slot=%p", slot);

if (ui == NULL) {
ret = RET_OSSL_ERR;
goto err;
}
prompt = UI_construct_prompt(ui, "PIN", login_info);
if (!prompt) {
ret = RET_OSSL_ERR;
goto err;
}
ret = UI_dup_input_string(ui, prompt, UI_INPUT_FLAG_DEFAULT_PWD, cb_pin, 4,
MAX_PIN_LENGTH);
if (ret <= 0) {
ret = RET_OSSL_ERR;
goto err;
}

if (UI_process(ui)) {
ret = RET_OSSL_ERR;
goto err;
}

*cb_pin_len = strlen(cb_pin);

err:
OPENSSL_free(prompt);
UI_free(ui);
return ret;
}

/* returns a locked login_session if _session is not NULL */
static CK_RV token_login(P11PROV_SESSION *session, P11PROV_URI *uri,
OSSL_PASSPHRASE_CALLBACK *pw_cb, void *pw_cbarg,
struct p11prov_slot *slot)
struct p11prov_slot *slot, CK_USER_TYPE user_type)
{
char cb_pin[MAX_PIN_LENGTH + 1] = { 0 };
size_t cb_pin_len = 0;
Expand All @@ -400,6 +439,9 @@ static CK_RV token_login(P11PROV_SESSION *session, P11PROV_URI *uri,
bool cache = false;
CK_RV ret;

P11PROV_debug("Log into the token session=%p uri=%p slot=%p type=%d",
session, uri, slot, user_type);

token = p11prov_slot_get_token(slot);
if (!(token->flags & CKF_PROTECTED_AUTHENTICATION_PATH)) {
const char *cached_pin = p11prov_slot_get_cached_pin(slot);
Expand All @@ -421,18 +463,39 @@ static CK_RV token_login(P11PROV_SESSION *session, P11PROV_URI *uri,
}
if (pin) {
pinlen = strlen((const char *)pin);
} else if (pw_cb) {
const char *login_info = p11prov_slot_get_login_info(slot);
OSSL_PARAM params[2] = {
OSSL_PARAM_DEFN(OSSL_PASSPHRASE_PARAM_INFO,
OSSL_PARAM_UTF8_STRING, (void *)login_info,
strlen(login_info)),
OSSL_PARAM_END,
};
ret = pw_cb(cb_pin, sizeof(cb_pin), &cb_pin_len, params, pw_cbarg);
if (ret != RET_OSSL_OK) {
ret = CKR_GENERAL_ERROR;
goto done;
} else {
if (pw_cb) {
const char *login_info = p11prov_slot_get_login_info(slot);
OSSL_PARAM params[2] = {
OSSL_PARAM_DEFN(OSSL_PASSPHRASE_PARAM_INFO,
OSSL_PARAM_UTF8_STRING, (void *)login_info,
strlen(login_info)),
OSSL_PARAM_END,
};
ret = pw_cb(cb_pin, sizeof(cb_pin), &cb_pin_len, params,
pw_cbarg);
if (ret != RET_OSSL_OK) {
/* this error can mean anything from the user canceling
* the prompt to no UI method provided.
* Fall back to our prompt here */
ret = p11prov_session_prompt_for_pin(slot, (char *)cb_pin,
&cb_pin_len);
if (ret != RET_OSSL_OK) {
/* give up */
ret = CKR_GENERAL_ERROR;
goto done;
}
}
} else {
/* We are asking the user off-band for the user consent -- from
* store we will always receive non-null (but unusable) callback
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

*/
ret = p11prov_session_prompt_for_pin(slot, (char *)cb_pin,
&cb_pin_len);
if (ret != RET_OSSL_OK) {
ret = CKR_GENERAL_ERROR;
goto done;
}
}
if (cb_pin_len == 0) {
ret = CKR_CANCEL;
Expand All @@ -443,16 +506,12 @@ static CK_RV token_login(P11PROV_SESSION *session, P11PROV_URI *uri,
pinlen = cb_pin_len;

cache = p11prov_ctx_cache_pins(session->provctx);

} else {
ret = CKR_CANCEL;
goto done;
}
}

P11PROV_debug("Attempt Login on session %lu", session->session);
/* Supports only USER login sessions for now */
ret = p11prov_Login(session->provctx, session->session, CKU_USER, pin,
ret = p11prov_Login(session->provctx, session->session, user_type, pin,
pinlen);
if (ret == CKR_USER_ALREADY_LOGGED_IN) {
ret = CKR_OK;
Expand Down Expand Up @@ -481,6 +540,34 @@ static CK_RV token_login(P11PROV_SESSION *session, P11PROV_URI *uri,
return ret;
}

CK_RV p11prov_context_specific_login(P11PROV_SESSION *session, P11PROV_URI *uri,
OSSL_PASSPHRASE_CALLBACK *pw_cb,
void *pw_cbarg)
{
P11PROV_SLOTS_CTX *sctx = NULL;
P11PROV_SLOT *slot = NULL;
CK_RV ret;

P11PROV_debug("Providing context specific login session=%p uri=%p", session,
uri);

ret = p11prov_take_slots(session->provctx, &sctx);
if (ret != CKR_OK) {
return CKR_GENERAL_ERROR;
}

slot = p11prov_get_slot_by_id(sctx, p11prov_session_slotid(session));
if (!slot) {
ret = CKR_GENERAL_ERROR;
}

ret =
token_login(session, uri, pw_cb, pw_cbarg, slot, CKU_CONTEXT_SPECIFIC);

p11prov_return_slots(sctx);
return ret;
}

static CK_RV check_slot(P11PROV_CTX *ctx, P11PROV_SLOT *slot, P11PROV_URI *uri,
CK_MECHANISM_TYPE mechtype, bool rw)
{
Expand Down Expand Up @@ -689,7 +776,7 @@ static CK_RV slot_login(P11PROV_SLOT *slot, P11PROV_URI *uri,
/* we seem to already have a valid logged in session */
ret = CKR_OK;
} else {
ret = token_login(session, uri, pw_cb, pw_cbarg, slot);
ret = token_login(session, uri, pw_cb, pw_cbarg, slot, CKU_USER);
}

done:
Expand Down
4 changes: 4 additions & 0 deletions src/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ CK_RV p11prov_take_login_session(P11PROV_CTX *provctx, CK_SLOT_ID slotid,
P11PROV_SESSION **_session);
void p11prov_return_session(P11PROV_SESSION *session);

CK_RV p11prov_context_specific_login(P11PROV_SESSION *session, P11PROV_URI *uri,
OSSL_PASSPHRASE_CALLBACK *pw_cb,
void *pw_cbarg);

typedef CK_RV (*p11prov_session_callback_t)(void *cbarg);
void p11prov_session_set_callback(P11PROV_SESSION *session,
p11prov_session_callback_t cb, void *cbarg);
Expand Down
12 changes: 12 additions & 0 deletions src/signature.c
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,7 @@ static CK_RV p11prov_sig_operate_init(P11PROV_SIG_CTX *sigctx, bool digest_op,
CK_SESSION_HANDLE sess;
CK_SLOT_ID slotid;
bool reqlogin = false;
bool always_auth = false;
CK_RV ret;

P11PROV_debug("called (sigctx=%p, digest_op=%s)", sigctx,
Expand Down Expand Up @@ -832,10 +833,21 @@ static CK_RV p11prov_sig_operate_init(P11PROV_SIG_CTX *sigctx, bool digest_op,
slotid = p11prov_obj_get_slotid(sigctx->key);

ret = mech_fallback_init(sigctx, slotid);
goto done;
simo5 marked this conversation as resolved.
Show resolved Hide resolved
break;
default:
P11PROV_raise(sigctx->provctx, ret,
"Failed to open session on slot %lu", slotid);
goto done;
}

if (reqlogin) {
always_auth =
p11prov_obj_get_bool(sigctx->key, CKA_ALWAYS_AUTHENTICATE, false);
}

if (always_auth) {
ret = p11prov_context_specific_login(session, NULL, NULL, NULL);
}

done:
Expand Down
5 changes: 5 additions & 0 deletions tests/pincache.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ int main(int argc, char *argv[])
int status;

keyuri = getenv("PRIURI");
/* optional first argument is a PKCS#11 uri of the key to test.
* Default is provided by environment variable BASEURI */
if (argc > 1) {
keyuri = argv[1];
}
if (!keyuri) {
fprintf(stderr, "PRIURI not defined\n");
exit(EXIT_FAILURE);
Expand Down
32 changes: 31 additions & 1 deletion tests/setup-softhsm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,30 @@ else
echo ""
fi

title PARA "generate EC key pair with ALWAYS AUTHENTICATE flag, self-signed certificate"
KEYID='0008'
URIKEYID="%00%08"
TSTCRT="${TMPPDIR}/eccert3"
TSTCRTN="ecCert3"

pkcs11-tool --keypairgen --key-type="EC:secp521r1" --login --pin=$PINVALUE \
--module="$P11LIB" --label="${TSTCRTN}" --id="$KEYID" --always-auth
ca_sign $TSTCRT $TSTCRTN "My EC Cert 3" $KEYID

ECBASE3URIWITHPIN="pkcs11:id=${URIKEYID};pin-value=${PINVALUE}"
ECBASE3URI="pkcs11:id=${URIKEYID}"
ECPUB3URI="pkcs11:type=public;id=${URIKEYID}"
ECPRI3URI="pkcs11:type=private;id=${URIKEYID}"
ECCRT3URI="pkcs11:type=cert;object=${TSTCRTN}"

title LINE "EC3 PKCS11 URIS"
echo "${ECBASE3URIWITHPIN}"
echo "${ECBASE3URI}"
echo "${ECPUB3URI}"
echo "${ECPRI3URI}"
echo "${ECCRT3URI}"
echo ""

title PARA "Show contents of softhsm token"
echo " ----------------------------------------------------------------------------------------------------"
pkcs11-tool -O --login --pin=$PINVALUE --module="$P11LIB"
Expand Down Expand Up @@ -393,10 +417,16 @@ export BASE2URI="${BASE2URI}"
export PRI2URI="${PRI2URI}"
export CRT2URI="${CRT2URI}"

export ECBASE2URIWITHPIN="${ECBASEURIWITHPIN}"
export ECBASE2URIWITHPIN="${ECBASE2URIWITHPIN}"
export ECBASE2URI="${ECBASE2URI}"
export ECPRI2URI="${ECPRI2URI}"
export ECCRT2URI="${ECCRT2URI}"

export ECBASE3URIWITHPIN="${ECBASE3URIWITHPIN}"
export ECBASE3URI="${ECBASE3URI}"
export ECPUB3URI="${ECPUB3URI}"
export ECPRI3URI="${ECPRI3URI}"
export ECCRT3URI="${ECCRT3URI}"
DBGSCRIPT

if [ -n "${ECXBASEURI}" ]; then
Expand Down
Loading
Loading