From 98afb314d13939cbee19c69885dcb655db8460da Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Fri, 23 Feb 2024 13:51:26 -0500 Subject: Improve PKCS11 error reporting in PKINIT Create a helper p11err() to set extended error message for failed PKCS11 operations, and use it instead of pkiDebug() and pkcs11error(). ticket: 9113 (new) --- src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 262 +++++++++++---------- src/plugins/preauth/pkinit/pkinit_trace.h | 9 - 2 files changed, 142 insertions(+), 129 deletions(-) diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c index 18f1ffe..d9cc248 100644 --- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c @@ -161,9 +161,11 @@ static krb5_error_code pkinit_create_sequence_of_principal_identifiers int type, krb5_pa_data ***e_data_out); #ifndef WITHOUT_PKCS11 -static krb5_error_code pkinit_find_private_key -(pkinit_identity_crypto_context, CK_ATTRIBUTE_TYPE usage, - CK_OBJECT_HANDLE *objp); +static krb5_error_code +pkinit_find_private_key(krb5_context context, + pkinit_identity_crypto_context id_cryptoctx, + CK_ATTRIBUTE_TYPE usage, + CK_OBJECT_HANDLE *objp); static krb5_error_code pkinit_login (krb5_context context, pkinit_identity_crypto_context id_cryptoctx, CK_TOKEN_INFO *tip, const char *password); @@ -180,6 +182,8 @@ static krb5_error_code pkinit_sign_data_pkcs11 (krb5_context context, pkinit_identity_crypto_context id_cryptoctx, unsigned char *data, unsigned int data_len, unsigned char **sig, unsigned int *sig_len); + +static krb5_error_code p11err(krb5_context context, CK_RV rv, const char *op); #endif /* WITHOUT_PKCS11 */ static krb5_error_code pkinit_sign_data_fs @@ -197,9 +201,6 @@ create_krb5_invalidCertificates(krb5_context context, static krb5_error_code create_identifiers_from_stack(STACK_OF(X509) *sk, krb5_external_principal_identifier *** ids); -static const char * -pkcs11err(int err); - #if OPENSSL_VERSION_NUMBER < 0x10100000L @@ -944,8 +945,9 @@ cleanup: #endif /* OPENSSL_VERSION_NUMBER < 0x30000000L */ +#ifndef WITHOUT_PKC11 static struct pkcs11_errstrings { - short code; + CK_RV code; char *text; } pkcs11_errstrings[] = { { 0x0, "ok" }, @@ -1035,6 +1037,7 @@ static struct pkcs11_errstrings { { 0x200, "function rejected" }, { -1, NULL } }; +#endif MAKE_INIT_FUNCTION(pkinit_openssl_init); @@ -1563,6 +1566,8 @@ pkinit_fini_pkcs11(pkinit_identity_crypto_context ctx) free(ctx->token_label); free(ctx->cert_id); free(ctx->cert_label); + ctx->p11_module_name = ctx->token_label = ctx->cert_label = NULL; + ctx->cert_id = NULL; #endif } @@ -3360,48 +3365,53 @@ pkinit_pkcs7type2oid(pkinit_plg_crypto_context cryptoctx, int pkcs7_type) } #ifndef WITHOUT_PKCS11 -static struct plugin_file_handle * +static krb5_error_code load_pkcs11_module(krb5_context context, const char *modname, - CK_FUNCTION_LIST_PTR_PTR p11p) + struct plugin_file_handle **handle_out, + CK_FUNCTION_LIST_PTR_PTR p11_out) { struct plugin_file_handle *handle = NULL; - CK_RV (*getflist)(CK_FUNCTION_LIST_PTR_PTR); + CK_RV rv, (*getflist)(CK_FUNCTION_LIST_PTR_PTR); struct errinfo einfo = EMPTY_ERRINFO; - const char *errmsg = NULL; + const char *errmsg = NULL, *failure; void (*sym)(void); long err; - CK_RV rv; TRACE_PKINIT_PKCS11_OPEN(context, modname); err = krb5int_open_plugin(modname, &handle, &einfo); if (err) { - errmsg = k5_get_error(&einfo, err); - TRACE_PKINIT_PKCS11_OPEN_FAILED(context, errmsg); + failure = _("Cannot load PKCS11 module"); goto error; } err = krb5int_get_plugin_func(handle, "C_GetFunctionList", &sym, &einfo); if (err) { - errmsg = k5_get_error(&einfo, err); - TRACE_PKINIT_PKCS11_GETSYM_FAILED(context, errmsg); + failure = _("Cannot find C_GetFunctionList in PKCS11 module"); goto error; } getflist = (CK_RV (*)(CK_FUNCTION_LIST_PTR_PTR))sym; - rv = (*getflist)(p11p); + rv = (*getflist)(p11_out); if (rv != CKR_OK) { - TRACE_PKINIT_PKCS11_GETFLIST_FAILED(context, pkcs11err(rv)); + failure = _("Cannot retrieve function list in PKCS11 module"); goto error; } - return handle; + *handle_out = handle; + return 0; error: - k5_free_error(&einfo, errmsg); + if (err) { + errmsg = k5_get_error(&einfo, err); + k5_setmsg(context, err, _("%s: %s"), failure, errmsg); + } else { + err = KRB5KDC_ERR_PREAUTH_FAILED; + k5_setmsg(context, err, "%s", failure); + } k5_clear_error(&einfo); if (handle != NULL) krb5int_close_plugin(handle); - return NULL; + return err; } static krb5_error_code @@ -3409,12 +3419,13 @@ pkinit_login(krb5_context context, pkinit_identity_crypto_context id_cryptoctx, CK_TOKEN_INFO *tip, const char *password) { + krb5_error_code ret = 0; + CK_RV rv; krb5_data rdat; char *prompt; const char *warning; krb5_prompt kprompt; krb5_prompt_type prompt_type; - int r = 0; if (tip->flags & CKF_PROTECTED_AUTHENTICATION_PATH) { rdat.data = NULL; @@ -3423,7 +3434,7 @@ pkinit_login(krb5_context context, rdat.data = strdup(password); rdat.length = strlen(password); } else if (id_cryptoctx->prompter == NULL) { - r = KRB5_LIBOS_CANTREADPWD; + ret = KRB5_LIBOS_CANTREADPWD; rdat.data = NULL; } else { if (tip->flags & CKF_USER_PIN_LOCKED) @@ -3447,31 +3458,28 @@ pkinit_login(krb5_context context, /* PROMPTER_INVOCATION */ k5int_set_prompt_types(context, &prompt_type); - r = (*id_cryptoctx->prompter)(context, id_cryptoctx->prompter_data, - NULL, NULL, 1, &kprompt); + ret = (*id_cryptoctx->prompter)(context, id_cryptoctx->prompter_data, + NULL, NULL, 1, &kprompt); k5int_set_prompt_types(context, 0); free(prompt); } - if (r == 0) { - r = id_cryptoctx->p11->C_Login(id_cryptoctx->session, CKU_USER, - (u_char *) rdat.data, rdat.length); - - if (r != CKR_OK) { - TRACE_PKINIT_PKCS11_LOGIN_FAILED(context, pkcs11err(r)); - r = KRB5KDC_ERR_PREAUTH_FAILED; - } + if (!ret) { + rv = id_cryptoctx->p11->C_Login(id_cryptoctx->session, CKU_USER, + (uint8_t *)rdat.data, rdat.length); + if (rv != CKR_OK) + ret = p11err(context, rv, "C_Login"); } free(rdat.data); - return r; + return ret; } static krb5_error_code pkinit_open_session(krb5_context context, pkinit_identity_crypto_context cctx) { - CK_ULONG i, pret; + CK_ULONG i, rv; unsigned char *cp; size_t label_len; CK_ULONG count = 0; @@ -3485,30 +3493,35 @@ pkinit_open_session(krb5_context context, return 0; /* session already open */ /* Load module */ - cctx->p11_module = load_pkcs11_module(context, cctx->p11_module_name, - &cctx->p11); - if (cctx->p11_module == NULL) - return KRB5KDC_ERR_PREAUTH_FAILED; + ret = load_pkcs11_module(context, cctx->p11_module_name, &cctx->p11_module, + &cctx->p11); + if (ret) + goto cleanup; /* Init */ - pret = cctx->p11->C_Initialize(NULL); - if (pret != CKR_OK) { - pkiDebug("C_Initialize: %s\n", pkcs11err(pret)); - return KRB5KDC_ERR_PREAUTH_FAILED; + rv = cctx->p11->C_Initialize(NULL); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_Initialize"); + goto cleanup; } /* Get the list of available slots */ - if (cctx->p11->C_GetSlotList(TRUE, NULL, &count) != CKR_OK) - return KRB5KDC_ERR_PREAUTH_FAILED; + rv = cctx->p11->C_GetSlotList(TRUE, NULL, &count); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_GetSlotList"); + goto cleanup; + } if (count == 0) { TRACE_PKINIT_PKCS11_NO_TOKEN(context); - return KRB5KDC_ERR_PREAUTH_FAILED; + ret = KRB5KDC_ERR_PREAUTH_FAILED; + goto cleanup; } - slotlist = calloc(count, sizeof(CK_SLOT_ID)); + slotlist = k5calloc(count, sizeof(CK_SLOT_ID), &ret); if (slotlist == NULL) - return ENOMEM; - if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) { - ret = KRB5KDC_ERR_PREAUTH_FAILED; + goto cleanup; + rv = cctx->p11->C_GetSlotList(TRUE, slotlist, &count); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_GetSlotList"); goto cleanup; } @@ -3519,19 +3532,17 @@ pkinit_open_session(krb5_context context, continue; /* Open session */ - pret = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, - NULL, NULL, &cctx->session); - if (pret != CKR_OK) { - pkiDebug("C_OpenSession: %s\n", pkcs11err(pret)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, + NULL, NULL, &cctx->session); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_OpenSession"); goto cleanup; } /* Get token info */ - pret = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo); - if (pret != CKR_OK) { - pkiDebug("C_GetTokenInfo: %s\n", pkcs11err(pret)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_GetTokenInfo"); goto cleanup; } @@ -3593,6 +3604,10 @@ pkinit_open_session(krb5_context context, ret = 0; cleanup: + /* On error, finalize the PKCS11 fields to ensure that we don't mistakenly + * short-circuit with success on the next call. */ + if (ret) + pkinit_fini_pkcs11(cctx); free(slotlist); free(p11name); return ret; @@ -3614,16 +3629,17 @@ cleanup: * If there are more than one, we just take the first one. */ -krb5_error_code -pkinit_find_private_key(pkinit_identity_crypto_context id_cryptoctx, +static krb5_error_code +pkinit_find_private_key(krb5_context context, + pkinit_identity_crypto_context id_cryptoctx, CK_ATTRIBUTE_TYPE usage, CK_OBJECT_HANDLE *objp) { CK_OBJECT_CLASS cls; CK_ATTRIBUTE attrs[4]; CK_ULONG count; + CK_RV rv; unsigned int nattrs = 0; - int r; #ifdef PKINIT_USE_KEY_USAGE CK_BBOOL true_false; #endif @@ -3653,18 +3669,21 @@ pkinit_find_private_key(pkinit_identity_crypto_context id_cryptoctx, attrs[nattrs].ulValueLen = id_cryptoctx->cert_id_len; nattrs++; - r = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, nattrs); - if (r != CKR_OK) { - pkiDebug("krb5_pkinit_sign_data: C_FindObjectsInit: %s\n", - pkcs11err(r)); - return KRB5KDC_ERR_PREAUTH_FAILED; - } + rv = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, + nattrs); + if (rv != CKR_OK) + return p11err(context, rv, _("C_FindObjectsInit")); - r = id_cryptoctx->p11->C_FindObjects(id_cryptoctx->session, objp, 1, &count); + rv = id_cryptoctx->p11->C_FindObjects(id_cryptoctx->session, objp, 1, + &count); id_cryptoctx->p11->C_FindObjectsFinal(id_cryptoctx->session); - pkiDebug("found %d private keys (%s)\n", (int)count, pkcs11err(r)); - if (r != CKR_OK || count < 1) + if (rv != CKR_OK) + return p11err(context, rv, _("C_FindObjects")); + if (count < 1) { + k5_setmsg(context, KRB5KDC_ERR_PREAUTH_FAILED, + _("Found no private keys in PKCS11 token")); return KRB5KDC_ERR_PREAUTH_FAILED; + } return 0; } #endif @@ -3812,34 +3831,32 @@ pkinit_sign_data_pkcs11(krb5_context context, CK_FUNCTION_LIST_PTR p11; CK_ATTRIBUTE attr; CK_KEY_TYPE keytype; + CK_RV rv; EVP_MD_CTX *ctx; const EVP_MD *md = EVP_sha256(); unsigned int mdlen; uint8_t mdbuf[EVP_MAX_MD_SIZE], *dinfo = NULL, *sigbuf = NULL, *input; size_t dinfo_len, input_len; - int r; *sig = NULL; *sig_len = 0; - if (pkinit_open_session(context, id_cryptoctx)) { - pkiDebug("can't open pkcs11 session\n"); - return KRB5KDC_ERR_PREAUTH_FAILED; - } + ret = pkinit_open_session(context, id_cryptoctx); + if (ret) + return ret; p11 = id_cryptoctx->p11; session = id_cryptoctx->session; - ret = pkinit_find_private_key(id_cryptoctx, CKA_SIGN, &obj); + ret = pkinit_find_private_key(context, id_cryptoctx, CKA_SIGN, &obj); if (ret) return ret; attr.type = CKA_KEY_TYPE; attr.pValue = &keytype; attr.ulValueLen = sizeof(keytype); - r = p11->C_GetAttributeValue(session, obj, &attr, 1); - if (r) { - pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = p11->C_GetAttributeValue(session, obj, &attr, 1); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_GetAttributeValue"); goto cleanup; } @@ -3881,10 +3898,9 @@ pkinit_sign_data_pkcs11(krb5_context context, mech.pParameter = NULL; mech.ulParameterLen = 0; - r = p11->C_SignInit(session, &mech, obj); - if (r != CKR_OK) { - pkiDebug("C_SignInit: %s\n", pkcs11err(r)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = p11->C_SignInit(session, &mech, obj); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_SignInit"); goto cleanup; } @@ -3897,18 +3913,17 @@ pkinit_sign_data_pkcs11(krb5_context context, if (sigbuf == NULL) goto cleanup; - r = p11->C_Sign(session, input, input_len, sigbuf, &len); - if (r == CKR_BUFFER_TOO_SMALL || (r == CKR_OK && len >= PK_SIGLEN_GUESS)) { + rv = p11->C_Sign(session, input, input_len, sigbuf, &len); + if (rv == CKR_BUFFER_TOO_SMALL || + (rv == CKR_OK && len >= PK_SIGLEN_GUESS)) { free(sigbuf); - pkiDebug("C_Sign realloc %d\n", (int) len); sigbuf = k5alloc(len, &ret); if (sigbuf == NULL) goto cleanup; - r = p11->C_Sign(session, input, input_len, sigbuf, &len); + rv = p11->C_Sign(session, input, input_len, sigbuf, &len); } - if (r != CKR_OK) { - pkiDebug("C_Sign: %s\n", pkcs11err(r)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_Sign"); goto cleanup; } @@ -4364,13 +4379,14 @@ reassemble_pkcs11_name(pkinit_identity_opts *idopts) } static krb5_error_code -load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session, - pkinit_identity_opts *idopts, pkinit_cred_info *cred_out) +load_one_cert(krb5_context context, CK_FUNCTION_LIST_PTR p11, + CK_SESSION_HANDLE session, pkinit_identity_opts *idopts, + pkinit_cred_info *cred_out) { krb5_error_code ret; CK_ATTRIBUTE attrs[2]; CK_BYTE_PTR cert = NULL, cert_id = NULL; - CK_RV pret; + CK_RV rv; const unsigned char *cp; CK_OBJECT_HANDLE obj; CK_ULONG count; @@ -4380,8 +4396,8 @@ load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session, *cred_out = NULL; /* Look for X.509 cert. */ - pret = p11->C_FindObjects(session, &obj, 1, &count); - if (pret != CKR_OK || count <= 0) + rv = p11->C_FindObjects(session, &obj, 1, &count); + if (rv != CKR_OK || count <= 0) return 0; /* Get cert and id len. */ @@ -4391,10 +4407,9 @@ load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session, attrs[1].type = CKA_ID; attrs[1].pValue = NULL; attrs[1].ulValueLen = 0; - pret = p11->C_GetAttributeValue(session, obj, attrs, 2); - if (pret != CKR_OK && pret != CKR_BUFFER_TOO_SMALL) { - pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(pret)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = p11->C_GetAttributeValue(session, obj, attrs, 2); + if (rv != CKR_OK && rv != CKR_BUFFER_TOO_SMALL) { + ret = p11err(context, rv, "C_GetAttributeValue"); goto cleanup; } @@ -4409,10 +4424,9 @@ load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session, attrs[0].pValue = cert; attrs[1].type = CKA_ID; attrs[1].pValue = cert_id; - pret = p11->C_GetAttributeValue(session, obj, attrs, 2); - if (pret != CKR_OK) { - pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(pret)); - ret = KRB5KDC_ERR_PREAUTH_FAILED; + rv = p11->C_GetAttributeValue(session, obj, attrs, 2); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_GetAttributeValue"); goto cleanup; } @@ -4422,7 +4436,8 @@ load_one_cert(CK_FUNCTION_LIST_PTR p11, CK_SESSION_HANDLE session, cp = (unsigned char *)cert; x = d2i_X509(NULL, &cp, (int)attrs[0].ulValueLen); if (x == NULL) { - ret = KRB5KDC_ERR_PREAUTH_FAILED; + ret = oerr(context, 0, + _("Failed to decode X509 certificate from PKCS11 token")); goto cleanup; } @@ -4460,7 +4475,7 @@ pkinit_get_certs_pkcs11(krb5_context context, int i; unsigned int nattrs; krb5_error_code ret; - CK_RV pret; + CK_RV rv; /* Copy stuff from idopts -> id_cryptoctx */ if (idopts->p11_module_name != NULL) { @@ -4532,16 +4547,16 @@ pkinit_get_certs_pkcs11(krb5_context context, nattrs++; } - pret = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, - nattrs); - if (pret != CKR_OK) { - pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(pret)); + rv = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, + nattrs); + if (rv != CKR_OK) { + ret = p11err(context, rv, "C_FindObjectsInit"); return KRB5KDC_ERR_PREAUTH_FAILED; } for (i = 0; i < MAX_CREDS_ALLOWED; i++) { - ret = load_one_cert(id_cryptoctx->p11, id_cryptoctx->session, idopts, - &id_cryptoctx->creds[i]); + ret = load_one_cert(context, id_cryptoctx->p11, id_cryptoctx->session, + idopts, &id_cryptoctx->creds[i]); if (ret) return ret; if (id_cryptoctx->creds[i] == NULL) @@ -5526,19 +5541,26 @@ print_pubkey(BIGNUM * key, char *msg) } #endif -static const char * -pkcs11err(int err) +#ifndef WITHOUT_PKCS11 +static krb5_error_code +p11err(krb5_context context, CK_RV rv, const char *op) { + krb5_error_code code = KRB5KDC_ERR_PREAUTH_FAILED; int i; + const char *msg; - for (i = 0; pkcs11_errstrings[i].text != NULL; i++) - if (pkcs11_errstrings[i].code == err) + for (i = 0; pkcs11_errstrings[i].text != NULL; i++) { + if (pkcs11_errstrings[i].code == rv) break; - if (pkcs11_errstrings[i].text != NULL) - return (pkcs11_errstrings[i].text); + } + msg = pkcs11_errstrings[i].text; + if (msg == NULL) + msg = "unknown PKCS11 error"; - return "unknown PKCS11 error"; + krb5_set_error_message(context, code, _("PKCS11 error (%s): %s"), op, msg); + return code; } +#endif /* * Add an item to the pkinit_identity_crypto_context's list of deferred diff --git a/src/plugins/preauth/pkinit/pkinit_trace.h b/src/plugins/preauth/pkinit/pkinit_trace.h index ab23b68..c0b1e2c 100644 --- a/src/plugins/preauth/pkinit/pkinit_trace.h +++ b/src/plugins/preauth/pkinit/pkinit_trace.h @@ -99,21 +99,12 @@ #define TRACE_PKINIT_OPENSSL_ERROR(c, msg) \ TRACE(c, "PKINIT OpenSSL error: {str}", msg) -#define TRACE_PKINIT_PKCS11_GETFLIST_FAILED(c, errstr) \ - TRACE(c, "PKINIT PKCS11 C_GetFunctionList failed: {str}", errstr) -#define TRACE_PKINIT_PKCS11_GETSYM_FAILED(c, errstr) \ - TRACE(c, "PKINIT unable to find PKCS11 plugin symbol " \ - "C_GetFunctionList: {str}", errstr) -#define TRACE_PKINIT_PKCS11_LOGIN_FAILED(c, errstr) \ - TRACE(c, "PKINIT PKCS11 C_Login failed: {str}", errstr) #define TRACE_PKINIT_PKCS11_NO_MATCH_TOKEN(c) \ TRACE(c, "PKINIT PKCS#11 module has no matching tokens") #define TRACE_PKINIT_PKCS11_NO_TOKEN(c) \ TRACE(c, "PKINIT PKCS#11 module shows no slots with tokens") #define TRACE_PKINIT_PKCS11_OPEN(c, name) \ TRACE(c, "PKINIT opening PKCS#11 module \"{str}\"", name) -#define TRACE_PKINIT_PKCS11_OPEN_FAILED(c, errstr) \ - TRACE(c, "PKINIT PKCS#11 module open failed: {str}", errstr) #define TRACE_PKINIT_PKCS11_SLOT(c, slot, len, label) \ TRACE(c, "PKINIT PKCS#11 slotid {int} token {lenstr}", \ slot, len, label) -- cgit v1.1