From 036e62f7842147f7a7cf937fafc452567d325847 Mon Sep 17 00:00:00 2001 From: yixiangzhike Date: Tue, 11 Oct 2022 17:47:10 +0800 Subject: [PATCH] backport upstream patches Signed-off-by: yixiangzhike --- ...ort-Add-additional-KRB5_TRACE-points.patch | 570 +++++++++ ...aks-on-error-in-kadm5-init-functions.patch | 665 +++++++++++ backport-Fix-many-unlikely-memory-leaks.patch | 1054 +++++++++++++++++ ...memory-leak-in-OTP-kdcpreauth-module.patch | 51 + ...-UPN-handling-in-PKINIT-client-certs.patch | 57 + ...port-Fix-uncommon-PKINIT-memory-leak.patch | 57 + ...e-verto-context-later-in-KDC-cleanup.patch | 44 + ...rt-Modernize-pkinit_get_certs_pkcs11.patch | 320 +++++ ...b5int_open_plugin-for-PKCS-11-module.patch | 145 +++ krb5.spec | 14 +- 10 files changed, 2976 insertions(+), 1 deletion(-) create mode 100644 backport-Add-additional-KRB5_TRACE-points.patch create mode 100644 backport-Fix-leaks-on-error-in-kadm5-init-functions.patch create mode 100644 backport-Fix-many-unlikely-memory-leaks.patch create mode 100644 backport-Fix-memory-leak-in-OTP-kdcpreauth-module.patch create mode 100644 backport-Fix-multiple-UPN-handling-in-PKINIT-client-certs.patch create mode 100644 backport-Fix-uncommon-PKINIT-memory-leak.patch create mode 100644 backport-Free-verto-context-later-in-KDC-cleanup.patch create mode 100644 backport-Modernize-pkinit_get_certs_pkcs11.patch create mode 100644 backport-Use-krb5int_open_plugin-for-PKCS-11-module.patch diff --git a/backport-Add-additional-KRB5_TRACE-points.patch b/backport-Add-additional-KRB5_TRACE-points.patch new file mode 100644 index 0000000..48696f7 --- /dev/null +++ b/backport-Add-additional-KRB5_TRACE-points.patch @@ -0,0 +1,570 @@ +From 34625d594c339a077899fa01fc4b5c331a1647d0 Mon Sep 17 00:00:00 2001 +From: Ken Hornstein +Date: Wed, 17 Mar 2021 17:44:46 -0400 +Subject: [PATCH] Add additional KRB5_TRACE points + +Add additional tracing points to the PKINIT plugin for use with the +KRB5_TRACE facility, replacing many (but not all) of the existing +pkiDebug() trace points. + +Fix a memory leak of an errinfo structure when loading a PKCS11 module +fails. + +Rename pkinit_pkcs11_code_to_text() to pkcs11err() for brevity, and +modify it for thread safety. + +[ghudson@mit.edu: added certificate index to regexp match trace +points; renamed various identifiers; edited commit message] + +ticket: 8999 (new) +--- + .../preauth/pkinit/pkinit_crypto_openssl.c | 114 ++++++++++-------- + src/plugins/preauth/pkinit/pkinit_identity.c | 3 +- + src/plugins/preauth/pkinit/pkinit_matching.c | 39 +++--- + src/plugins/preauth/pkinit/pkinit_trace.h | 51 +++++++- + 4 files changed, 129 insertions(+), 78 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index 263910480..2f2dec599 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -32,6 +32,7 @@ + #include "k5-int.h" + #include "pkinit_crypto_openssl.h" + #include "k5-buf.h" ++#include "k5-err.h" + #include "k5-hex.h" + #include + #include +@@ -101,8 +102,6 @@ static krb5_error_code pkinit_login + CK_TOKEN_INFO *tip, const char *password); + static krb5_error_code pkinit_open_session + (krb5_context context, pkinit_identity_crypto_context id_cryptoctx); +-static struct plugin_file_handle *pkinit_C_LoadModule +-(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p); + #ifdef SILLYDECRYPT + CK_RV pkinit_C_Decrypt + (pkinit_identity_crypto_context id_cryptoctx, +@@ -143,8 +142,8 @@ static int + wrap_signeddata(unsigned char *data, unsigned int data_len, + unsigned char **out, unsigned int *out_len); + +-static char * +-pkinit_pkcs11_code_to_text(int err); ++static const char * ++pkcs11err(int err); + + + #ifdef HAVE_OPENSSL_CMS +@@ -1195,11 +1194,10 @@ cms_signeddata_create(krb5_context context, + } + certstack = X509_STORE_CTX_get1_chain(certctx); + size = sk_X509_num(certstack); +- pkiDebug("size of certificate chain = %d\n", size); + for(i = 0; i < size - 1; i++) { + X509 *x = sk_X509_value(certstack, i); + X509_NAME_oneline(X509_get_subject_name(x), buf, sizeof(buf)); +- pkiDebug("cert #%d: %s\n", i, buf); ++ TRACE_PKINIT_CERT_CHAIN_NAME(context, (int)i, buf); + sk_X509_push(cert_stack, X509_dup(x)); + } + X509_STORE_CTX_free(certctx); +@@ -1988,21 +1986,17 @@ crypto_retrieve_X509_sans(krb5_context context, + + X509_NAME_oneline(X509_get_subject_name(cert), + buf, sizeof(buf)); +- pkiDebug("%s: looking for SANs in cert = %s\n", __FUNCTION__, buf); + + l = X509_get_ext_by_NID(cert, NID_subject_alt_name, -1); + if (l < 0) + return 0; + + if (!(ext = X509_get_ext(cert, l)) || !(ialt = X509V3_EXT_d2i(ext))) { +- pkiDebug("%s: found no subject alt name extensions\n", __FUNCTION__); ++ TRACE_PKINIT_SAN_CERT_NONE(context, buf); + goto cleanup; + } + num_sans = sk_GENERAL_NAME_num(ialt); + +- pkiDebug("%s: found %d subject alt name extension(s)\n", __FUNCTION__, +- num_sans); +- + /* OK, we're likely returning something. Allocate return values */ + if (princs_ret != NULL) { + princs = calloc(num_sans + 1, sizeof(krb5_principal)); +@@ -2089,6 +2083,8 @@ crypto_retrieve_X509_sans(krb5_context context, + } + sk_GENERAL_NAME_pop_free(ialt, GENERAL_NAME_free); + ++ TRACE_PKINIT_SAN_CERT_COUNT(context, (int)num_sans, p, u, d, buf); ++ + retval = 0; + if (princs != NULL && *princs != NULL) { + *princs_ret = princs; +@@ -3547,34 +3543,47 @@ prepare_enc_data(const uint8_t *indata, int indata_len, uint8_t **outdata, + + #ifndef WITHOUT_PKCS11 + static struct plugin_file_handle * +-pkinit_C_LoadModule(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p) ++load_pkcs11_module(krb5_context context, const char *modname, ++ CK_FUNCTION_LIST_PTR_PTR p11p) + { +- struct plugin_file_handle *handle; ++ struct plugin_file_handle *handle = NULL; + CK_RV (*getflist)(CK_FUNCTION_LIST_PTR_PTR); + struct errinfo einfo = EMPTY_ERRINFO; ++ const char *errmsg = NULL; + void (*sym)(); + long err; + CK_RV rv; + +- pkiDebug("loading module \"%s\"... ", modname); +- if (krb5int_open_plugin(modname, &handle, &einfo) != 0) { +- pkiDebug("not found\n"); +- return NULL; ++ 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); ++ goto error; + } + + err = krb5int_get_plugin_func(handle, "C_GetFunctionList", &sym, &einfo); +- k5_clear_error(&einfo); +- if (!err) { +- getflist = (CK_RV (*)())sym; +- rv = (*getflist)(p11p); ++ if (err) { ++ errmsg = k5_get_error(&einfo, err); ++ TRACE_PKINIT_PKCS11_GETSYM_FAILED(context, errmsg); ++ goto error; + } +- if (err || rv != CKR_OK) { +- krb5int_close_plugin(handle); +- pkiDebug("failed\n"); +- return NULL; ++ ++ getflist = (CK_RV (*)())sym; ++ rv = (*getflist)(p11p); ++ if (rv != CKR_OK) { ++ TRACE_PKINIT_PKCS11_GETFLIST_FAILED(context, pkcs11err(rv)); ++ goto error; + } +- pkiDebug("ok\n"); ++ + return handle; ++ ++error: ++ k5_free_error(&einfo, errmsg); ++ k5_clear_error(&einfo); ++ if (handle != NULL) ++ krb5int_close_plugin(handle); ++ return NULL; + } + + static krb5_error_code +@@ -3631,7 +3640,7 @@ pkinit_login(krb5_context context, + (u_char *) rdat.data, rdat.length); + + if (r != CKR_OK) { +- pkiDebug("C_Login: %s\n", pkinit_pkcs11_code_to_text(r)); ++ TRACE_PKINIT_PKCS11_LOGIN_FAILED(context, pkcs11err(r)); + r = KRB5KDC_ERR_PREAUTH_FAILED; + } + } +@@ -3657,22 +3666,24 @@ pkinit_open_session(krb5_context context, + return 0; /* session already open */ + + /* Load module */ +- cctx->p11_module = +- pkinit_C_LoadModule(cctx->p11_module_name, &cctx->p11); ++ cctx->p11_module = load_pkcs11_module(context, cctx->p11_module_name, ++ &cctx->p11); + if (cctx->p11_module == NULL) + return KRB5KDC_ERR_PREAUTH_FAILED; + + /* Init */ + if ((r = cctx->p11->C_Initialize(NULL)) != CKR_OK) { +- pkiDebug("C_Initialize: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_Initialize: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + + /* Get the list of available slots */ + if (cctx->p11->C_GetSlotList(TRUE, NULL, &count) != CKR_OK) + return KRB5KDC_ERR_PREAUTH_FAILED; +- if (count == 0) ++ if (count == 0) { ++ TRACE_PKINIT_PKCS11_NO_TOKEN(context); + return KRB5KDC_ERR_PREAUTH_FAILED; ++ } + slotlist = calloc(count, sizeof(CK_SLOT_ID)); + if (slotlist == NULL) + return ENOMEM; +@@ -3688,13 +3699,13 @@ pkinit_open_session(krb5_context context, + /* Open session */ + if ((r = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, + NULL, NULL, &cctx->session)) != CKR_OK) { +- pkiDebug("C_OpenSession: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_OpenSession: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + + /* Get token info */ + if ((r = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo)) != CKR_OK) { +- pkiDebug("C_GetTokenInfo: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_GetTokenInfo: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -3706,8 +3717,8 @@ pkinit_open_session(krb5_context context, + } + label_len = cp - tinfo.label; + +- pkiDebug("open_session: slotid %d token \"%.*s\"\n", +- (int)slotlist[i], (int)label_len, tinfo.label); ++ TRACE_PKINIT_PKCS11_SLOT(context, (int)slotlist[i], (int)label_len, ++ tinfo.label); + if (cctx->token_label == NULL || + (strlen(cctx->token_label) == label_len && + memcmp(cctx->token_label, tinfo.label, label_len) == 0)) +@@ -3716,7 +3727,7 @@ pkinit_open_session(krb5_context context, + } + if (i >= count) { + free(slotlist); +- pkiDebug("open_session: no matching token found\n"); ++ TRACE_PKINIT_PKCS11_NO_MATCH_TOKEN(context); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + cctx->slotid = slotlist[i]; +@@ -3825,13 +3836,13 @@ pkinit_find_private_key(pkinit_identity_crypto_context id_cryptoctx, + r = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, nattrs); + if (r != CKR_OK) { + pkiDebug("krb5_pkinit_sign_data: C_FindObjectsInit: %s\n", +- pkinit_pkcs11_code_to_text(r)); ++ pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + + r = 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, pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("found %d private keys (%s)\n", (int)count, pkcs11err(r)); + if (r != CKR_OK || count < 1) + return KRB5KDC_ERR_PREAUTH_FAILED; + return 0; +@@ -3944,7 +3955,7 @@ pkinit_decode_data_pkcs11(krb5_context context, + r = pkinit_C_Decrypt(id_cryptoctx, (CK_BYTE_PTR) data, (CK_ULONG) data_len, + cp, &len); + if (r != CKR_OK) { +- pkiDebug("C_Decrypt: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_Decrypt: %s\n", pkcs11err(r)); + if (r == CKR_BUFFER_TOO_SMALL) + pkiDebug("decrypt %d needs %d\n", (int) data_len, (int) len); + return KRB5KDC_ERR_PREAUTH_FAILED; +@@ -4024,7 +4035,7 @@ pkinit_sign_data_pkcs11(krb5_context context, + + if ((r = id_cryptoctx->p11->C_SignInit(id_cryptoctx->session, &mech, + obj)) != CKR_OK) { +- pkiDebug("C_SignInit: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_SignInit: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -4047,7 +4058,7 @@ pkinit_sign_data_pkcs11(krb5_context context, + (CK_ULONG) data_len, cp, &len); + } + if (r != CKR_OK) { +- pkiDebug("C_Sign: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_Sign: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + pkiDebug("sign %d -> %d\n", (int) data_len, (int) len); +@@ -4578,7 +4589,7 @@ pkinit_get_certs_pkcs11(krb5_context context, + #else + if ((r = id_cryptoctx->p11->C_GetMechanismList(id_cryptoctx->slotid, NULL, + &count)) != CKR_OK || count <= 0) { +- pkiDebug("C_GetMechanismList: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_GetMechanismList: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + mechp = malloc(count * sizeof (CK_MECHANISM_TYPE)); +@@ -4635,7 +4646,7 @@ pkinit_get_certs_pkcs11(krb5_context context, + + r = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, nattrs); + if (r != CKR_OK) { +- pkiDebug("C_FindObjectsInit: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -4661,7 +4672,7 @@ pkinit_get_certs_pkcs11(krb5_context context, + + if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session, + obj, attrs, 2)) != CKR_OK && r != CKR_BUFFER_TOO_SMALL) { +- pkiDebug("C_GetAttributeValue: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + cert = (CK_BYTE_PTR) malloc((size_t) attrs[0].ulValueLen + 1); +@@ -4679,7 +4690,7 @@ pkinit_get_certs_pkcs11(krb5_context context, + + if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session, + obj, attrs, 2)) != CKR_OK) { +- pkiDebug("C_GetAttributeValue: %s\n", pkinit_pkcs11_code_to_text(r)); ++ pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -5348,12 +5359,12 @@ crypto_load_cas_and_crls(krb5_context context, + { + switch (idtype) { + case IDTYPE_FILE: +- TRACE_PKINIT_LOAD_FROM_FILE(context); ++ TRACE_PKINIT_LOAD_FROM_FILE(context, id); + return load_cas_and_crls(context, plg_cryptoctx, req_cryptoctx, + id_cryptoctx, catype, id); + break; + case IDTYPE_DIR: +- TRACE_PKINIT_LOAD_FROM_DIR(context); ++ TRACE_PKINIT_LOAD_FROM_DIR(context, id); + return load_cas_and_crls_dir(context, plg_cryptoctx, req_cryptoctx, + id_cryptoctx, catype, id); + break; +@@ -5784,19 +5795,18 @@ print_pubkey(BIGNUM * key, char *msg) + } + #endif + +-static char * +-pkinit_pkcs11_code_to_text(int err) ++static const char * ++pkcs11err(int err) + { + int i; +- static char uc[32]; + + for (i = 0; pkcs11_errstrings[i].text != NULL; i++) + if (pkcs11_errstrings[i].code == err) + break; + if (pkcs11_errstrings[i].text != NULL) + return (pkcs11_errstrings[i].text); +- snprintf(uc, sizeof(uc), _("unknown code 0x%x"), err); +- return (uc); ++ ++ return "unknown PKCS11 error"; + } + + /* +diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c +index 62b2cf7a1..3c1778a3b 100644 +--- a/src/plugins/preauth/pkinit/pkinit_identity.c ++++ b/src/plugins/preauth/pkinit/pkinit_identity.c +@@ -387,8 +387,7 @@ process_option_identity(krb5_context context, + int idtype; + krb5_error_code retval = 0; + +- pkiDebug("%s: processing value '%s'\n", +- __FUNCTION__, value ? value : "NULL"); ++ TRACE_PKINIT_IDENTITY_OPTION(context, value); + if (value == NULL) + return EINVAL; + +diff --git a/src/plugins/preauth/pkinit/pkinit_matching.c b/src/plugins/preauth/pkinit/pkinit_matching.c +index b042c74d3..b42485a50 100644 +--- a/src/plugins/preauth/pkinit/pkinit_matching.c ++++ b/src/plugins/preauth/pkinit/pkinit_matching.c +@@ -448,25 +448,26 @@ cleanup: + } + + static int +-regexp_match(krb5_context context, rule_component *rc, char *value) ++regexp_match(krb5_context context, rule_component *rc, char *value, int idx) + { + int code; + +- pkiDebug("%s: checking %s rule '%s' with value '%s'\n", +- __FUNCTION__, keyword2string(rc->kw_type), rc->regsrc, value); +- + code = regexec(&rc->regexp, value, 0, NULL, 0); + +- pkiDebug("%s: the result is%s a match\n", __FUNCTION__, +- code == REG_NOMATCH ? " NOT" : ""); ++ if (code == 0) { ++ TRACE_PKINIT_REGEXP_MATCH(context, keyword2string(rc->kw_type), ++ rc->regsrc, value, idx); ++ } else { ++ TRACE_PKINIT_REGEXP_NOMATCH(context, keyword2string(rc->kw_type), ++ rc->regsrc, value, idx); ++ } + + return (code == 0 ? 1: 0); + } + + static int +-component_match(krb5_context context, +- rule_component *rc, +- pkinit_cert_matching_data *md) ++component_match(krb5_context context, rule_component *rc, ++ pkinit_cert_matching_data *md, int idx) + { + int match = 0; + int i; +@@ -476,21 +477,21 @@ component_match(krb5_context context, + case kwvaltype_regexp: + switch (rc->kw_type) { + case kw_subject: +- match = regexp_match(context, rc, md->subject_dn); ++ match = regexp_match(context, rc, md->subject_dn, idx); + break; + case kw_issuer: +- match = regexp_match(context, rc, md->issuer_dn); ++ match = regexp_match(context, rc, md->issuer_dn, idx); + break; + case kw_san: + for (i = 0; md->sans != NULL && md->sans[i] != NULL; i++) { + krb5_unparse_name(context, md->sans[i], &princ_string); +- match = regexp_match(context, rc, princ_string); ++ match = regexp_match(context, rc, princ_string, idx); + krb5_free_unparsed_name(context, princ_string); + if (match) + break; + } + for (i = 0; md->upns != NULL && md->upns[i] != NULL; i++) { +- match = regexp_match(context, rc, md->upns[i]); ++ match = regexp_match(context, rc, md->upns[i], idx); + if (match) + break; + } +@@ -574,7 +575,7 @@ check_all_certs(krb5_context context, + pkiDebug("%s: subject: '%s'\n", __FUNCTION__, md->subject_dn); + certs_checked++; + for (rc = rs->crs; rc != NULL; rc = rc->next) { +- comp_match = component_match(context, rc, md); ++ comp_match = component_match(context, rc, md, i); + if (comp_match) { + pkiDebug("%s: match for keyword type %s\n", + __FUNCTION__, keyword2string(rc->kw_type)); +@@ -600,8 +601,7 @@ check_all_certs(krb5_context context, + nextcert: + continue; + } +- pkiDebug("%s: After checking %d certs, we found %d matches\n", +- __FUNCTION__, certs_checked, total_cert_matches); ++ TRACE_PKINIT_CERT_NUM_MATCHING(context, certs_checked, total_cert_matches); + if (total_cert_matches == 1) { + *match_found = 1; + *match_index = save_index; +@@ -642,7 +642,7 @@ pkinit_cert_matching(krb5_context context, + + /* parse each rule line one at a time and check all the certs against it */ + for (x = 0; rules[x] != NULL; x++) { +- pkiDebug("%s: Processing rule '%s'\n", __FUNCTION__, rules[x]); ++ TRACE_PKINIT_CERT_RULE(context, rules[x]); + + /* Free rules from previous time through... */ + if (rs != NULL) { +@@ -652,8 +652,7 @@ pkinit_cert_matching(krb5_context context, + retval = parse_rule_set(context, rules[x], &rs); + if (retval) { + if (retval == EINVAL) { +- pkiDebug("%s: Ignoring invalid rule pkinit_cert_match = '%s'\n", +- __FUNCTION__, rules[x]); ++ TRACE_PKINIT_CERT_RULE_INVALID(context, rules[x]); + continue; + } + goto cleanup; +@@ -737,7 +736,7 @@ pkinit_client_cert_match(krb5_context context, + goto cleanup; + + for (rc = rs->crs; rc != NULL; rc = rc->next) { +- comp_match = component_match(context, rc, md); ++ comp_match = component_match(context, rc, md, 0); + if ((comp_match && rs->relation == relation_or) || + (!comp_match && rs->relation == relation_and)) { + break; +diff --git a/src/plugins/preauth/pkinit/pkinit_trace.h b/src/plugins/preauth/pkinit/pkinit_trace.h +index bba3226bd..259e95c6c 100644 +--- a/src/plugins/preauth/pkinit/pkinit_trace.h ++++ b/src/plugins/preauth/pkinit/pkinit_trace.h +@@ -93,6 +93,25 @@ + #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) ++ + #define TRACE_PKINIT_SERVER_CERT_AUTH(c, modname) \ + TRACE(c, "PKINIT server authorizing cert with module {str}", \ + modname) +@@ -123,16 +142,28 @@ + TRACE(c, "PKINIT server could not parse UPN \"{str}\": {kerr}", \ + upn, ret) + ++#define TRACE_PKINIT_CERT_CHAIN_NAME(c, index, name) \ ++ TRACE(c, "PKINIT chain cert #{int}: {str}", index, name) ++#define TRACE_PKINIT_CERT_NUM_MATCHING(c, total, nummatch) \ ++ TRACE(c, "PKINIT client checked {int} certs, found {int} matches", \ ++ total, nummatch) ++#define TRACE_PKINIT_CERT_RULE(c, rule) \ ++ TRACE(c, "PKINIT client matching rule '{str}' against certificates", rule) ++#define TRACE_PKINIT_CERT_RULE_INVALID(c, rule) \ ++ TRACE(c, "PKINIT client ignoring invalid rule '{str}'", rule) ++ + #define TRACE_PKINIT_EKU(c) \ + TRACE(c, "PKINIT found acceptable EKU and digitalSignature KU") + #define TRACE_PKINIT_EKU_NO_KU(c) \ + TRACE(c, "PKINIT found acceptable EKU but no digitalSignature KU") ++#define TRACE_PKINIT_IDENTITY_OPTION(c, name) \ ++ TRACE(c, "PKINIT loading identity {str}", name) + #define TRACE_PKINIT_LOADED_CERT(c, name) \ + TRACE(c, "PKINIT loaded cert and key for {str}", name) +-#define TRACE_PKINIT_LOAD_FROM_FILE(c) \ +- TRACE(c, "PKINIT loading CA certs and CRLs from FILE") +-#define TRACE_PKINIT_LOAD_FROM_DIR(c) \ +- TRACE(c, "PKINIT loading CA certs and CRLs from DIR") ++#define TRACE_PKINIT_LOAD_FROM_FILE(c, name) \ ++ TRACE(c, "PKINIT loading CA certs and CRLs from FILE {str}", name) ++#define TRACE_PKINIT_LOAD_FROM_DIR(c, name) \ ++ TRACE(c, "PKINIT loading CA certs and CRLs from DIR {str}", name) + #define TRACE_PKINIT_NO_CA_ANCHOR(c, file) \ + TRACE(c, "PKINIT no anchor CA in file {str}", file) + #define TRACE_PKINIT_NO_CA_INTERMEDIATE(c, file) \ +@@ -162,6 +193,18 @@ + TRACE(c, "PKINIT second PKCS12_parse with password failed") + #define TRACE_PKINIT_PKCS_PROMPT_FAIL(c) \ + TRACE(c, "PKINIT failed to prompt for PKCS12 password") ++#define TRACE_PKINIT_REGEXP_MATCH(c, keyword, comp, value, idx) \ ++ TRACE(c, "PKINIT matched {str} rule '{str}' with " \ ++ "value '{str}' in cert #{int}", keyword, comp, value, (idx) + 1) ++#define TRACE_PKINIT_REGEXP_NOMATCH(c, keyword, comp, value, idx) \ ++ TRACE(c, "PKINIT didn't match {str} rule '{str}' with " \ ++ "value '{str}' in cert #{int}", keyword, comp, value, (idx) + 1) ++#define TRACE_PKINIT_SAN_CERT_COUNT(c, count, princ, upns, dns, cert) \ ++ TRACE(c, "PKINIT client found {int} SANs ({int} princs, {int} " \ ++ "UPNs, {int} DNS names) in certificate {str}", count, princ, \ ++ upns, dns, cert) ++#define TRACE_PKINIT_SAN_CERT_NONE(c, cert) \ ++ TRACE(c, "PKINIT client found no SANs in certificate {str}", cert) + + #define TRACE_CERTAUTH_VTINIT_FAIL(c, ret) \ + TRACE(c, "certauth module failed to init vtable: {kerr}", ret) +-- +2.27.0 + diff --git a/backport-Fix-leaks-on-error-in-kadm5-init-functions.patch b/backport-Fix-leaks-on-error-in-kadm5-init-functions.patch new file mode 100644 index 0000000..2bab8dd --- /dev/null +++ b/backport-Fix-leaks-on-error-in-kadm5-init-functions.patch @@ -0,0 +1,665 @@ +From 552d7b7626450f963b8e37345c472420c842402c Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Wed, 23 Jun 2021 16:53:16 -0400 +Subject: [PATCH] Fix leaks on error in kadm5 init functions + +In the GENERIC_CHECK_HANDLE function, separate out the +version-checking logic so we can call it in the init functions before +allocating resources. + +In the client and server library initialization functions, use a +single exit path after argument validation, and share the destruction +code with kadm5_destroy() via a helper. +--- + src/lib/kadm5/admin_internal.h | 39 ++++--- + src/lib/kadm5/clnt/client_init.c | 174 +++++++++++----------------- + src/lib/kadm5/srv/server_init.c | 191 ++++++++++--------------------- + 3 files changed, 145 insertions(+), 259 deletions(-) + +diff --git a/src/lib/kadm5/admin_internal.h b/src/lib/kadm5/admin_internal.h +index faf8e9c36..9be53883a 100644 +--- a/src/lib/kadm5/admin_internal.h ++++ b/src/lib/kadm5/admin_internal.h +@@ -11,29 +11,32 @@ + + #define KADM5_SERVER_HANDLE_MAGIC 0x12345800 + +-#define GENERIC_CHECK_HANDLE(handle, old_api_version, new_api_version) \ ++#define CHECK_VERSIONS(struct_version, api_version, old_api_err, new_api_err) \ + { \ +- kadm5_server_handle_t srvr = \ +- (kadm5_server_handle_t) handle; \ +- \ +- if (! srvr) \ +- return KADM5_BAD_SERVER_HANDLE; \ +- if (srvr->magic_number != KADM5_SERVER_HANDLE_MAGIC) \ +- return KADM5_BAD_SERVER_HANDLE; \ +- if ((srvr->struct_version & KADM5_MASK_BITS) != \ +- KADM5_STRUCT_VERSION_MASK) \ ++ if ((struct_version & KADM5_MASK_BITS) != KADM5_STRUCT_VERSION_MASK) \ + return KADM5_BAD_STRUCT_VERSION; \ +- if (srvr->struct_version < KADM5_STRUCT_VERSION_1) \ ++ if (struct_version < KADM5_STRUCT_VERSION_1) \ + return KADM5_OLD_STRUCT_VERSION; \ +- if (srvr->struct_version > KADM5_STRUCT_VERSION_1) \ ++ if (struct_version > KADM5_STRUCT_VERSION_1) \ + return KADM5_NEW_STRUCT_VERSION; \ +- if ((srvr->api_version & KADM5_MASK_BITS) != \ +- KADM5_API_VERSION_MASK) \ ++ if ((api_version & KADM5_MASK_BITS) != KADM5_API_VERSION_MASK) \ + return KADM5_BAD_API_VERSION; \ +- if (srvr->api_version < KADM5_API_VERSION_2) \ +- return old_api_version; \ +- if (srvr->api_version > KADM5_API_VERSION_4) \ +- return new_api_version; \ ++ if (api_version < KADM5_API_VERSION_2) \ ++ return old_api_err; \ ++ if (api_version > KADM5_API_VERSION_4) \ ++ return new_api_err; \ ++ } ++ ++#define GENERIC_CHECK_HANDLE(handle, old_api_err, new_api_err) \ ++ { \ ++ kadm5_server_handle_t srvr = handle; \ ++ \ ++ if (srvr == NULL) \ ++ return KADM5_BAD_SERVER_HANDLE; \ ++ if (srvr->magic_number != KADM5_SERVER_HANDLE_MAGIC) \ ++ return KADM5_BAD_SERVER_HANDLE; \ ++ CHECK_VERSIONS(srvr->struct_version, srvr->api_version, \ ++ old_api_err, new_api_err); \ + } + + /* +diff --git a/src/lib/kadm5/clnt/client_init.c b/src/lib/kadm5/clnt/client_init.c +index 0aaca701f..75614bb19 100644 +--- a/src/lib/kadm5/clnt/client_init.c ++++ b/src/lib/kadm5/clnt/client_init.c +@@ -138,6 +138,36 @@ kadm5_init_with_skey(krb5_context context, char *client_name, + server_handle); + } + ++static kadm5_ret_t ++free_handle(kadm5_server_handle_t handle) ++{ ++ kadm5_ret_t ret = 0; ++ OM_uint32 minor_stat; ++ krb5_ccache ccache; ++ ++ if (handle == NULL) ++ return 0; ++ ++ if (handle->destroy_cache && handle->cache_name != NULL) { ++ ret = krb5_cc_resolve(handle->context, handle->cache_name, &ccache); ++ if (!ret) ++ ret = krb5_cc_destroy(handle->context, ccache); ++ } ++ free(handle->cache_name); ++ (void)gss_release_cred(&minor_stat, &handle->cred); ++ if (handle->clnt != NULL && handle->clnt->cl_auth != NULL) ++ AUTH_DESTROY(handle->clnt->cl_auth); ++ if (handle->clnt != NULL) ++ clnt_destroy(handle->clnt); ++ if (handle->client_socket != -1) ++ close(handle->client_socket); ++ free(handle->lhandle); ++ kadm5_free_config_params(handle->context, &handle->params); ++ free(handle); ++ ++ return ret; ++} ++ + static kadm5_ret_t + init_any(krb5_context context, char *client_name, enum init_type init_type, + char *pass, krb5_ccache ccache_in, char *service_name, +@@ -145,36 +175,34 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + krb5_ui_4 api_version, char **db_args, void **server_handle) + { + int fd = -1; +- OM_uint32 minor_stat; + krb5_boolean iprop_enable; + int port; + rpcprog_t rpc_prog; + rpcvers_t rpc_vers; +- krb5_ccache ccache; + krb5_principal client = NULL, server = NULL; + struct timeval timeout; + +- kadm5_server_handle_t handle; ++ kadm5_server_handle_t handle = NULL; + kadm5_config_params params_local; + +- int code = 0; ++ krb5_error_code code; + generic_ret r = { 0, 0 }; + + initialize_ovk_error_table(); + initialize_ovku_error_table(); + +- if (! server_handle) { ++ if (server_handle == NULL || client_name == NULL) + return EINVAL; +- } + +- if (! (handle = malloc(sizeof(*handle)))) { +- return ENOMEM; +- } +- memset(handle, 0, sizeof(*handle)); +- if (! (handle->lhandle = malloc(sizeof(*handle)))) { +- free(handle); +- return ENOMEM; +- } ++ CHECK_VERSIONS(struct_version, api_version, KADM5_OLD_LIB_API_VERSION, ++ KADM5_NEW_LIB_API_VERSION); ++ ++ handle = k5alloc(sizeof(*handle), &code); ++ if (handle == NULL) ++ goto cleanup; ++ handle->lhandle = k5alloc(sizeof(*handle), &code); ++ if (handle->lhandle == NULL) ++ goto cleanup; + + handle->magic_number = KADM5_SERVER_HANDLE_MAGIC; + handle->struct_version = struct_version; +@@ -192,33 +220,20 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + + handle->context = context; + +- if(client_name == NULL) { +- free(handle); +- return EINVAL; +- } +- +- /* +- * Verify the version numbers before proceeding; we can't use +- * CHECK_HANDLE because not all fields are set yet. +- */ +- GENERIC_CHECK_HANDLE(handle, KADM5_OLD_LIB_API_VERSION, +- KADM5_NEW_LIB_API_VERSION); +- + memset(¶ms_local, 0, sizeof(params_local)); + +- if ((code = kadm5_get_config_params(handle->context, 0, +- params_in, &handle->params))) { +- free(handle); +- return(code); +- } ++ code = kadm5_get_config_params(handle->context, 0, params_in, ++ &handle->params); ++ if (code) ++ goto cleanup; + + #define REQUIRED_PARAMS (KADM5_CONFIG_REALM | \ + KADM5_CONFIG_ADMIN_SERVER | \ + KADM5_CONFIG_KADMIND_PORT) + + if ((handle->params.mask & REQUIRED_PARAMS) != REQUIRED_PARAMS) { +- free(handle); +- return KADM5_MISSING_KRB5_CONF_PARAMS; ++ code = KADM5_MISSING_KRB5_CONF_PARAMS; ++ goto cleanup; + } + + code = krb5_parse_name(handle->context, client_name, &client); +@@ -228,6 +243,6 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + code = krb5_parse_name(handle->context, client_name, &client); + if (code) +- goto error; ++ goto cleanup; + + /* + * Get credentials. Also does some fallbacks in case kadmin/fqdn +@@ -239,7 +254,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + code = get_init_creds(handle, client, init_type, pass, ccache_in, + service_name, handle->params.realm, &server); + if (code) +- goto error; ++ goto cleanup; + + /* If the service_name and client_name are iprop-centric, use the iprop + * port and RPC identifiers. */ +@@ -258,7 +273,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + + code = connect_to_server(handle->params.admin_server, port, &fd); + if (code) +- goto error; ++ goto cleanup; + + handle->clnt = clnttcp_create(NULL, rpc_prog, rpc_vers, &fd, 0, 0); + if (handle->clnt == NULL) { +@@ -266,7 +281,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + #ifdef DEBUG + clnt_pcreateerror("clnttcp_create"); + #endif +- goto error; ++ goto cleanup; + } + + /* Set a one-hour timeout. */ +@@ -278,10 +293,6 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + handle->lhandle->clnt = handle->clnt; + handle->lhandle->client_socket = fd; + +- /* now that handle->clnt is set, we can check the handle */ +- if ((code = _kadm5_check_handle((void *) handle))) +- goto error; +- + /* + * The RPC connection is open; establish the GSS-API + * authentication context. +@@ -289,7 +300,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + code = setup_gss(handle, params_in, + (init_type == INIT_CREDS) ? client : NULL, server); + if (code) +- goto error; ++ goto cleanup; + + /* + * Bypass the remainder of the code and return straight away +@@ -297,7 +308,8 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + */ + if (iprop_enable) { + code = 0; +- *server_handle = (void *) handle; ++ *server_handle = handle; ++ handle = NULL; + goto cleanup; + } + +@@ -306,7 +318,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + #ifdef DEBUG + clnt_perror(handle->clnt, "init_2 null resp"); + #endif +- goto error; ++ goto cleanup; + } + /* Drop down to v3 wire protocol if server does not support v4 */ + if (r.code == KADM5_NEW_SERVER_API_VERSION && +@@ -315,7 +327,7 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + memset(&r, 0, sizeof(generic_ret)); + if (init_2(&handle->api_version, &r, handle->clnt)) { + code = KADM5_RPC_ERROR; +- goto error; ++ goto cleanup; + } + } + /* Drop down to v2 wire protocol if server does not support v3 */ +@@ -325,47 +337,21 @@ init_any(krb5_context context, char *client_name, enum init_type init_type, + memset(&r, 0, sizeof(generic_ret)); + if (init_2(&handle->api_version, &r, handle->clnt)) { + code = KADM5_RPC_ERROR; +- goto error; ++ goto cleanup; + } + } + if (r.code) { + code = r.code; +- goto error; ++ goto cleanup; + } + +- *server_handle = (void *) handle; +- +- goto cleanup; +- +-error: +- /* +- * Note that it is illegal for this code to execute if "handle" +- * has not been allocated and initialized. I.e., don't use "goto +- * error" before the block of code at the top of the function +- * that allocates and initializes "handle". +- */ +- if (handle->destroy_cache && handle->cache_name) { +- if (krb5_cc_resolve(handle->context, +- handle->cache_name, &ccache) == 0) +- (void) krb5_cc_destroy (handle->context, ccache); +- } +- if (handle->cache_name) +- free(handle->cache_name); +- (void)gss_release_cred(&minor_stat, &handle->cred); +- if(handle->clnt && handle->clnt->cl_auth) +- AUTH_DESTROY(handle->clnt->cl_auth); +- if(handle->clnt) +- clnt_destroy(handle->clnt); +- if (fd != -1) +- close(fd); +- free(handle->lhandle); +- kadm5_free_config_params(handle->context, &handle->params); ++ *server_handle = handle; ++ handle = NULL; + + cleanup: +- krb5_free_principal(handle->context, client); +- krb5_free_principal(handle->context, server); +- if (code) +- free(handle); ++ krb5_free_principal(context, client); ++ krb5_free_principal(context, server); ++ (void)free_handle(handle); + + return code; + } +@@ -695,38 +681,8 @@ rpc_auth(kadm5_server_handle_t handle, kadm5_config_params *params_in, + kadm5_ret_t + kadm5_destroy(void *server_handle) + { +- OM_uint32 minor_stat; +- krb5_ccache ccache = NULL; +- int code = KADM5_OK; +- kadm5_server_handle_t handle = +- (kadm5_server_handle_t) server_handle; +- + CHECK_HANDLE(server_handle); +- +- if (handle->destroy_cache && handle->cache_name) { +- if ((code = krb5_cc_resolve(handle->context, +- handle->cache_name, &ccache)) == 0) +- code = krb5_cc_destroy (handle->context, ccache); +- } +- if (handle->cache_name) +- free(handle->cache_name); +- if (handle->cred) +- (void)gss_release_cred(&minor_stat, &handle->cred); +- if (handle->clnt && handle->clnt->cl_auth) +- AUTH_DESTROY(handle->clnt->cl_auth); +- if (handle->clnt) +- clnt_destroy(handle->clnt); +- if (handle->client_socket != -1) +- close(handle->client_socket); +- if (handle->lhandle) +- free (handle->lhandle); +- +- kadm5_free_config_params(handle->context, &handle->params); +- +- handle->magic_number = 0; +- free(handle); +- +- return code; ++ return free_handle(server_handle); + } + /* not supported on client */ + kadm5_ret_t kadm5_lock(void *server_handle) +diff --git a/src/lib/kadm5/srv/server_init.c b/src/lib/kadm5/srv/server_init.c +index 3adc4b57d..2c0d51efd 100644 +--- a/src/lib/kadm5/srv/server_init.c ++++ b/src/lib/kadm5/srv/server_init.c +@@ -19,23 +19,6 @@ + #include "osconf.h" + #include "iprop_hdr.h" + +-/* +- * Function check_handle +- * +- * Purpose: Check a server handle and return a com_err code if it is +- * invalid or 0 if it is valid. +- * +- * Arguments: +- * +- * handle The server handle. +- */ +- +-static int check_handle(void *handle) +-{ +- CHECK_HANDLE(handle); +- return 0; +-} +- + static int dup_db_args(kadm5_server_handle_t handle, char **db_args) + { + int count = 0; +@@ -84,6 +67,23 @@ static void free_db_args(kadm5_server_handle_t handle) + } + } + ++static void ++free_handle(kadm5_server_handle_t handle) ++{ ++ if (handle == NULL) ++ return; ++ ++ destroy_pwqual(handle); ++ k5_kadm5_hook_free_handles(handle->context, handle->hook_handles); ++ ulog_fini(handle->context); ++ krb5_db_fini(handle->context); ++ krb5_free_principal(handle->context, handle->current_caller); ++ kadm5_free_config_params(handle->context, &handle->params); ++ free(handle->lhandle); ++ free_db_args(handle); ++ free(handle); ++} ++ + kadm5_ret_t kadm5_init_with_password(krb5_context context, char *client_name, + char *pass, char *service_name, + kadm5_config_params *params, +@@ -163,8 +163,8 @@ kadm5_ret_t kadm5_init(krb5_context context, char *client_name, char *pass, + char **db_args, + void **server_handle) + { +- int ret; +- kadm5_server_handle_t handle; ++ krb5_error_code ret; ++ kadm5_server_handle_t handle = NULL; + kadm5_config_params params_local; /* for v1 compat */ + + if (! server_handle) +@@ -173,18 +173,18 @@ kadm5_ret_t kadm5_init(krb5_context context, char *client_name, char *pass, + if (! client_name) + return EINVAL; + +- if (! (handle = (kadm5_server_handle_t) malloc(sizeof *handle))) +- return ENOMEM; +- memset(handle, 0, sizeof(*handle)); +- +- ret = dup_db_args(handle, db_args); +- if (ret) { +- free(handle); +- return ret; +- } ++ CHECK_VERSIONS(struct_version, api_version, KADM5_OLD_SERVER_API_VERSION, ++ KADM5_NEW_SERVER_API_VERSION); + ++ handle = k5alloc(sizeof(*handle), &ret); ++ if (handle == NULL) ++ goto cleanup; + handle->context = context; + ++ ret = dup_db_args(handle, db_args); ++ if (ret) ++ goto cleanup; ++ + initialize_ovk_error_table(); + initialize_ovku_error_table(); + +@@ -192,13 +192,6 @@ kadm5_ret_t kadm5_init(krb5_context context, char *client_name, char *pass, + handle->struct_version = struct_version; + handle->api_version = api_version; + +- /* +- * Verify the version numbers before proceeding; we can't use +- * CHECK_HANDLE because not all fields are set yet. +- */ +- GENERIC_CHECK_HANDLE(handle, KADM5_OLD_SERVER_API_VERSION, +- KADM5_NEW_SERVER_API_VERSION); +- + /* + * Acquire relevant profile entries. Merge values + * in params_in with values from profile, based on +@@ -208,11 +201,8 @@ kadm5_ret_t kadm5_init(krb5_context context, char *client_name, char *pass, + + ret = kadm5_get_config_params(handle->context, 1, params_in, + &handle->params); +- if (ret) { +- free_db_args(handle); +- free(handle); +- return(ret); +- } ++ if (ret) ++ goto cleanup; + + #define REQUIRED_PARAMS (KADM5_CONFIG_REALM | KADM5_CONFIG_DBNAME | \ + KADM5_CONFIG_ENCTYPE | \ +@@ -226,132 +216,69 @@ kadm5_ret_t kadm5_init(krb5_context context, char *client_name, char *pass, + KADM5_CONFIG_IPROP_PORT) + + if ((handle->params.mask & REQUIRED_PARAMS) != REQUIRED_PARAMS) { +- kadm5_free_config_params(handle->context, &handle->params); +- free_db_args(handle); +- free(handle); +- return KADM5_MISSING_CONF_PARAMS; ++ ret = KADM5_MISSING_CONF_PARAMS; ++ goto cleanup; + } + if ((handle->params.mask & KADM5_CONFIG_IPROP_ENABLED) == KADM5_CONFIG_IPROP_ENABLED + && handle->params.iprop_enabled) { + if ((handle->params.mask & IPROP_REQUIRED_PARAMS) != IPROP_REQUIRED_PARAMS) { +- kadm5_free_config_params(handle->context, &handle->params); +- free_db_args(handle); +- free(handle); +- return KADM5_MISSING_CONF_PARAMS; ++ ret = KADM5_MISSING_CONF_PARAMS; ++ goto cleanup; + } + } + + ret = krb5_set_default_realm(handle->context, handle->params.realm); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ if (ret) ++ goto cleanup; + + ret = krb5_db_open(handle->context, db_args, + KRB5_KDB_OPEN_RW | KRB5_KDB_SRV_TYPE_ADMIN); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- free_db_args(handle); +- free(handle); +- return(ret); +- } ++ if (ret) ++ goto cleanup; + +- if ((ret = krb5_parse_name(handle->context, client_name, +- &handle->current_caller))) { +- kadm5_free_config_params(handle->context, &handle->params); +- krb5_db_fini(handle->context); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ ret = krb5_parse_name(handle->context, client_name, ++ &handle->current_caller); ++ if (ret) ++ goto cleanup; + +- if (! (handle->lhandle = malloc(sizeof(*handle)))) { +- kadm5_free_config_params(handle->context, &handle->params); +- krb5_db_fini(handle->context); +- free_db_args(handle); +- free(handle); +- return ENOMEM; +- } ++ handle->lhandle = k5alloc(sizeof(*handle), &ret); ++ if (handle->lhandle == NULL) ++ goto cleanup; + *handle->lhandle = *handle; + handle->lhandle->api_version = KADM5_API_VERSION_4; + handle->lhandle->struct_version = KADM5_STRUCT_VERSION; + handle->lhandle->lhandle = handle->lhandle; + +- /* can't check the handle until current_caller is set */ +- ret = check_handle((void *) handle); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- free_db_args(handle); +- free(handle); +- return ret; +- } +- + ret = kdb_init_master(handle, handle->params.realm, + (handle->params.mask & KADM5_CONFIG_MKEY_FROM_KBD) + && handle->params.mkey_from_kbd); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- krb5_db_fini(handle->context); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ if (ret) ++ goto cleanup; + + ret = kdb_init_hist(handle, handle->params.realm); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- krb5_db_fini(handle->context); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ if (ret) ++ goto cleanup; + + ret = k5_kadm5_hook_load(context,&handle->hook_handles); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- krb5_db_fini(handle->context); +- krb5_free_principal(handle->context, handle->current_caller); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ if (ret) ++ goto cleanup; + + ret = init_pwqual(handle); +- if (ret) { +- kadm5_free_config_params(handle->context, &handle->params); +- k5_kadm5_hook_free_handles(context, handle->hook_handles); +- krb5_db_fini(handle->context); +- krb5_free_principal(handle->context, handle->current_caller); +- free_db_args(handle); +- free(handle); +- return ret; +- } ++ if (ret) ++ goto cleanup; + +- *server_handle = (void *) handle; ++ *server_handle = handle; ++ handle = NULL; + +- return KADM5_OK; ++cleanup: ++ free_handle(handle); ++ return ret; + } + + kadm5_ret_t kadm5_destroy(void *server_handle) + { +- kadm5_server_handle_t handle = server_handle; +- + CHECK_HANDLE(server_handle); +- +- destroy_pwqual(handle); +- +- k5_kadm5_hook_free_handles(handle->context, handle->hook_handles); +- ulog_fini(handle->context); +- krb5_db_fini(handle->context); +- krb5_free_principal(handle->context, handle->current_caller); +- kadm5_free_config_params(handle->context, &handle->params); +- handle->magic_number = 0; +- free(handle->lhandle); +- free_db_args(handle); +- free(handle); +- ++ free_handle(server_handle); + return KADM5_OK; + } + +-- +2.27.0 + diff --git a/backport-Fix-many-unlikely-memory-leaks.patch b/backport-Fix-many-unlikely-memory-leaks.patch new file mode 100644 index 0000000..a1f3aa0 --- /dev/null +++ b/backport-Fix-many-unlikely-memory-leaks.patch @@ -0,0 +1,1054 @@ +From a06945b4ec267e8b80e5e8c95edd89930ff12103 Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Wed, 26 May 2021 17:35:06 -0400 +Subject: [PATCH] Fix many unlikely memory leaks + +These are on error paths and often require allocation failures, so are +unlikely to be issues in practice. Reported by Coverity and cppcheck. +--- + src/kadmin/dbutil/kadm5_create.c | 19 +++--- + src/kadmin/dbutil/kdb5_create.c | 23 +++---- + src/kadmin/ktutil/ktutil_funcs.c | 3 +- + src/kdc/do_tgs_req.c | 27 ++++---- + src/kprop/kpropd.c | 26 ++++---- + src/kprop/kproplog.c | 25 ++++--- + src/lib/gssapi/spnego/spnego_mech.c | 6 +- + src/lib/kadm5/srv/pwqual_dict.c | 11 +++- + src/lib/kdb/kdb5.c | 5 +- + src/lib/kdb/kdb_convert.c | 65 +++++++++++-------- + src/lib/krad/remote.c | 7 +- + src/lib/krb5/krb/gic_keytab.c | 3 +- + src/lib/krb5/krb/s4u_authdata.c | 4 +- + src/lib/krb5/krb/send_tgs.c | 6 +- + src/lib/krb5/os/dnssrv.c | 2 +- + src/lib/krb5/os/sendto_kdc.c | 4 +- + src/plugins/audit/kdc_j_encode.c | 5 +- + src/plugins/kdb/db2/adb_policy.c | 10 +-- + src/plugins/kdb/db2/kdb_db2.c | 2 +- + .../kdb/ldap/libkdb_ldap/ldap_principal.c | 14 ++-- + .../kdb/ldap/libkdb_ldap/ldap_principal2.c | 8 +-- + .../preauth/pkinit/pkinit_crypto_openssl.c | 58 ++++++++++------- + src/plugins/preauth/pkinit/pkinit_identity.c | 10 +-- + src/util/profile/prof_get.c | 13 ++-- + 24 files changed, 198 insertions(+), 158 deletions(-) + +diff --git a/src/kadmin/dbutil/kadm5_create.c b/src/kadmin/dbutil/kadm5_create.c +index 42b45aa2d..db12cac36 100644 +--- a/src/kadmin/dbutil/kadm5_create.c ++++ b/src/kadmin/dbutil/kadm5_create.c +@@ -185,21 +185,24 @@ static int add_admin_princs(void *handle, krb5_context context, char *realm) + int add_admin_princ(void *handle, krb5_context context, + char *name, char *realm, int attrs, int lifetime) + { +- char *fullname; ++ char *fullname = NULL; + krb5_error_code ret; + kadm5_principal_ent_rec ent; + long flags; ++ int fret; + + memset(&ent, 0, sizeof(ent)); + + if (asprintf(&fullname, "%s@%s", name, realm) < 0) { + com_err(progname, ENOMEM, _("while appending realm to principal")); +- return ERR; ++ fret = ERR; ++ goto cleanup; + } + ret = krb5_parse_name(context, fullname, &ent.principal); + if (ret) { + com_err(progname, ret, _("while parsing admin principal name")); +- return(ERR); ++ fret = ERR; ++ goto cleanup; + } + ent.max_life = lifetime; + ent.attributes = attrs; +@@ -210,13 +213,13 @@ int add_admin_princ(void *handle, krb5_context context, + ret = kadm5_create_principal(handle, &ent, flags, NULL); + if (ret && ret != KADM5_DUP) { + com_err(progname, ret, _("while creating principal %s"), fullname); +- krb5_free_principal(context, ent.principal); +- free(fullname); +- return ERR; ++ fret = ERR; ++ goto cleanup; + } + ++ fret = OK; ++cleanup: + krb5_free_principal(context, ent.principal); + free(fullname); +- +- return OK; ++ return fret; + } +diff --git a/src/kadmin/dbutil/kdb5_create.c b/src/kadmin/dbutil/kdb5_create.c +index efdb8adb0..f9205f84d 100644 +--- a/src/kadmin/dbutil/kdb5_create.c ++++ b/src/kadmin/dbutil/kdb5_create.c +@@ -393,7 +393,7 @@ add_principal(context, princ, op, pblock) + struct realm_info *pblock; + { + krb5_error_code retval; +- krb5_db_entry *entry; ++ krb5_db_entry *entry = NULL; + krb5_kvno mkey_kvno; + krb5_timestamp now; + struct iterate_args iargs; +@@ -410,20 +410,20 @@ add_principal(context, princ, op, pblock) + entry->expiration = pblock->expiration; + + if ((retval = krb5_copy_principal(context, princ, &entry->princ))) +- goto error_out; ++ goto cleanup; + + if ((retval = krb5_timeofday(context, &now))) +- goto error_out; ++ goto cleanup; + + if ((retval = krb5_dbe_update_mod_princ_data(context, entry, + now, &db_create_princ))) +- goto error_out; ++ goto cleanup; + + switch (op) { + case MASTER_KEY: + if ((entry->key_data=(krb5_key_data*)malloc(sizeof(krb5_key_data))) + == NULL) +- goto error_out; ++ goto cleanup; + memset(entry->key_data, 0, sizeof(krb5_key_data)); + entry->n_key_data = 1; + +@@ -435,7 +435,7 @@ add_principal(context, princ, op, pblock) + if ((retval = krb5_dbe_encrypt_key_data(context, pblock->key, + &master_keyblock, NULL, + mkey_kvno, entry->key_data))) +- return retval; ++ goto cleanup; + /* + * There should always be at least one "active" mkey so creating the + * KRB5_TL_ACTKVNO entry now so the initial mkey is active. +@@ -445,11 +445,11 @@ add_principal(context, princ, op, pblock) + /* earliest possible time in case system clock is set back */ + actkvno.act_time = 0; + if ((retval = krb5_dbe_update_actkvno(context, entry, &actkvno))) +- return retval; ++ goto cleanup; + + /* so getprinc shows the right kvno */ + if ((retval = krb5_dbe_update_mkvno(context, entry, mkey_kvno))) +- return retval; ++ goto cleanup; + + break; + case TGT_KEY: +@@ -464,10 +464,11 @@ add_principal(context, princ, op, pblock) + 1, + tgt_keysalt_iterate, + (krb5_pointer) &iargs))) +- return retval; ++ goto cleanup; + break; + case NULL_KEY: +- return EOPNOTSUPP; ++ retval = EOPNOTSUPP; ++ goto cleanup; + default: + break; + } +@@ -479,7 +480,7 @@ add_principal(context, princ, op, pblock) + + retval = krb5_db_put_principal(context, entry); + +-error_out: ++cleanup: + krb5_db_free_principal(context, entry); + return retval; + } +diff --git a/src/kadmin/ktutil/ktutil_funcs.c b/src/kadmin/ktutil/ktutil_funcs.c +index e2e005d22..56bed1bbc 100644 +--- a/src/kadmin/ktutil/ktutil_funcs.c ++++ b/src/kadmin/ktutil/ktutil_funcs.c +@@ -256,7 +256,8 @@ krb5_error_code ktutil_add(context, list, princ_str, fetch, kvno, + *last = lp; + + cleanup: +- krb5_kt_free_entry(context, entry); ++ krb5_free_keytab_entry_contents(context, entry); ++ free(entry); + zapfree(password.data, password.length); + krb5_free_data_contents(context, &salt); + krb5_free_data_contents(context, ¶ms); +diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c +index 6d244ffd4..582e497cc 100644 +--- a/src/kdc/do_tgs_req.c ++++ b/src/kdc/do_tgs_req.c +@@ -162,17 +162,14 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt, + } + + errcode = kdc_make_rstate(kdc_active_realm, &state); +- if (errcode !=0) { +- krb5_free_kdc_req(kdc_context, request); +- return errcode; +- } ++ if (errcode != 0) ++ goto cleanup; + + /* Initialize audit state. */ + errcode = kau_init_kdc_req(kdc_context, request, from, &au_state); +- if (errcode) { +- krb5_free_kdc_req(kdc_context, request); +- return errcode; +- } ++ if (errcode) ++ goto cleanup; ++ + /* Seed the audit trail with the request ID and basic information. */ + kau_tgs_req(kdc_context, TRUE, au_state); + +@@ -733,11 +730,13 @@ cleanup: + if (errcode) + emsg = krb5_get_error_message (kdc_context, errcode); + +- au_state->status = status; +- if (!errcode) +- au_state->reply = &reply; +- kau_tgs_req(kdc_context, errcode ? FALSE : TRUE, au_state); +- kau_free_kdc_req(au_state); ++ if (au_state != NULL) { ++ au_state->status = status; ++ if (!errcode) ++ au_state->reply = &reply; ++ kau_tgs_req(kdc_context, errcode ? FALSE : TRUE, au_state); ++ kau_free_kdc_req(au_state); ++ } + + log_tgs_req(kdc_context, from, request, &reply, cprinc, + sprinc, altcprinc, authtime, +@@ -747,7 +746,7 @@ cleanup: + emsg = NULL; + } + +- if (errcode) { ++ if (errcode && state != NULL) { + int got_err = 0; + if (status == 0) { + status = krb5_get_error_message (kdc_context, errcode); +diff --git a/src/kprop/kpropd.c b/src/kprop/kpropd.c +index 498ca599a..46bb9d1f7 100644 +--- a/src/kprop/kpropd.c ++++ b/src/kprop/kpropd.c +@@ -632,7 +632,7 @@ krb5_error_code + do_iprop() + { + kadm5_ret_t retval; +- krb5_principal iprop_svc_principal; ++ krb5_principal iprop_svc_principal = NULL; + void *server_handle = NULL; + char *iprop_svc_princstr = NULL, *primary_svc_princstr = NULL; + unsigned int pollin, backoff_time; +@@ -652,16 +652,13 @@ do_iprop() + if (pollin == 0) + pollin = 10; + +- if (primary_svc_princstr == NULL) { +- retval = kadm5_get_kiprop_host_srv_name(kpropd_context, realm, +- &primary_svc_princstr); +- if (retval) { +- com_err(progname, retval, +- _("%s: unable to get kiprop host based " +- "service name for realm %s\n"), +- progname, realm); +- return retval; +- } ++ retval = kadm5_get_kiprop_host_srv_name(kpropd_context, realm, ++ &primary_svc_princstr); ++ if (retval) { ++ com_err(progname, retval, _("%s: unable to get kiprop host based " ++ "service name for realm %s\n"), ++ progname, realm); ++ goto done; + } + + retval = sn2princ_realm(kpropd_context, NULL, KIPROP_SVC_NAME, realm, +@@ -669,7 +666,7 @@ do_iprop() + if (retval) { + com_err(progname, retval, + _("while trying to construct host service principal")); +- return retval; ++ goto done; + } + + retval = krb5_unparse_name(kpropd_context, iprop_svc_principal, +@@ -677,10 +674,8 @@ do_iprop() + if (retval) { + com_err(progname, retval, + _("while canonicalizing principal name")); +- krb5_free_principal(kpropd_context, iprop_svc_principal); +- return retval; ++ goto done; + } +- krb5_free_principal(kpropd_context, iprop_svc_principal); + + reinit: + /* +@@ -995,6 +990,7 @@ error: + done: + free(iprop_svc_princstr); + free(primary_svc_princstr); ++ krb5_free_principal(kpropd_context, iprop_svc_principal); + krb5_free_default_realm(kpropd_context, def_realm); + kadm5_destroy(server_handle); + krb5_db_fini(kpropd_context); +diff --git a/src/kprop/kproplog.c b/src/kprop/kproplog.c +index 0c025f0eb..9d3a91070 100644 +--- a/src/kprop/kproplog.c ++++ b/src/kprop/kproplog.c +@@ -398,28 +398,36 @@ print_update(kdb_hlog_t *ulog, uint32_t entry, uint32_t ulogentries, + } + } + +-/* Return a read-only mmap of the ulog, or NULL on failure. Assumes fd is +- * released on process exit. */ ++/* Return a read-only mmap of the ulog, or NULL on failure. */ + static kdb_hlog_t * +-map_ulog(const char *filename) ++map_ulog(const char *filename, int *fd_out) + { + int fd; + struct stat st; +- kdb_hlog_t *ulog; ++ kdb_hlog_t *ulog = MAP_FAILED; ++ ++ *fd_out = -1; + + fd = open(filename, O_RDONLY); + if (fd == -1) + return NULL; +- if (fstat(fd, &st) < 0) ++ if (fstat(fd, &st) < 0) { ++ close(fd); + return NULL; ++ } + ulog = mmap(0, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); +- return (ulog == MAP_FAILED) ? NULL : ulog; ++ if (ulog == MAP_FAILED) { ++ close(fd); ++ return NULL; ++ } ++ *fd_out = fd; ++ return ulog; + } + + int + main(int argc, char **argv) + { +- int c; ++ int c, ulog_fd = -1; + unsigned int verbose = 0; + bool_t headeronly = FALSE, reset = FALSE; + uint32_t entry = 0; +@@ -480,7 +488,7 @@ main(int argc, char **argv) + goto done; + } + +- ulog = map_ulog(params.iprop_logfile); ++ ulog = map_ulog(params.iprop_logfile, &ulog_fd); + if (ulog == NULL) { + fprintf(stderr, _("Unable to map log file %s\n\n"), + params.iprop_logfile); +@@ -546,6 +554,7 @@ main(int argc, char **argv) + printf("\n"); + + done: ++ close(ulog_fd); + kadm5_free_config_params(context, ¶ms); + krb5_free_context(context); + return 0; +diff --git a/src/lib/gssapi/spnego/spnego_mech.c b/src/lib/gssapi/spnego/spnego_mech.c +index d74dc2b1b..ba7765cb4 100644 +--- a/src/lib/gssapi/spnego/spnego_mech.c ++++ b/src/lib/gssapi/spnego/spnego_mech.c +@@ -3485,11 +3485,9 @@ get_mech_set(OM_uint32 *minor_status, unsigned char **buff_in, + + major_status = gss_add_oid_set_member(minor_status, + temp, &returned_mechSet); +- if (major_status == GSS_S_COMPLETE) { ++ if (major_status == GSS_S_COMPLETE) + set_length += returned_mechSet->elements[i].length +2; +- if (generic_gss_release_oid(minor_status, &temp)) +- map_errcode(minor_status); +- } ++ generic_gss_release_oid(minor_status, &temp); + } + + return (returned_mechSet); +diff --git a/src/lib/kadm5/srv/pwqual_dict.c b/src/lib/kadm5/srv/pwqual_dict.c +index e3ac20eb8..bdd44c23a 100644 +--- a/src/lib/kadm5/srv/pwqual_dict.c ++++ b/src/lib/kadm5/srv/pwqual_dict.c +@@ -121,11 +121,16 @@ init_dict(dict_moddata dict, const char *dict_file) + close(fd); + return errno; + } +- if ((dict->word_block = malloc(sb.st_size + 1)) == NULL) ++ dict->word_block = malloc(sb.st_size + 1); ++ if (dict->word_block == NULL) { ++ (void)close(fd); + return ENOMEM; +- if (read(fd, dict->word_block, sb.st_size) != sb.st_size) ++ } ++ if (read(fd, dict->word_block, sb.st_size) != sb.st_size) { ++ (void)close(fd); + return errno; +- (void) close(fd); ++ } ++ (void)close(fd); + dict->word_block[sb.st_size] = '\0'; + + p = dict->word_block; +diff --git a/src/lib/kdb/kdb5.c b/src/lib/kdb/kdb5.c +index 11e2430c4..c49721821 100644 +--- a/src/lib/kdb/kdb5.c ++++ b/src/lib/kdb/kdb5.c +@@ -1449,8 +1449,11 @@ krb5_db_setup_mkey_name(krb5_context context, const char *keyname, + if (asprintf(&fname, "%s%s%s", keyname, REALM_SEP_STRING, realm) < 0) + return ENOMEM; + +- if ((retval = krb5_parse_name(context, fname, principal))) ++ retval = krb5_parse_name(context, fname, principal); ++ if (retval) { ++ free(fname); + return retval; ++ } + if (fullname) + *fullname = fname; + else +diff --git a/src/lib/kdb/kdb_convert.c b/src/lib/kdb/kdb_convert.c +index e1bf1919f..59952f55e 100644 +--- a/src/lib/kdb/kdb_convert.c ++++ b/src/lib/kdb/kdb_convert.c +@@ -550,7 +550,7 @@ krb5_error_code + ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kdb_incr_update_t *update) + { +- krb5_db_entry *ent; ++ krb5_db_entry *ent = NULL; + int replica; + krb5_principal mod_princ = NULL; + int i, j, cnt = 0, mod_time = 0, nattrs; +@@ -572,23 +572,20 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + */ + nattrs = update->kdb_update.kdbe_t_len; + +- dbprincstr = malloc((update->kdb_princ_name.utf8str_t_len + 1) +- * sizeof (char)); ++ dbprincstr = k5memdup0(update->kdb_princ_name.utf8str_t_val, ++ update->kdb_princ_name.utf8str_t_len, &ret); + if (dbprincstr == NULL) +- return (ENOMEM); +- strncpy(dbprincstr, (char *)update->kdb_princ_name.utf8str_t_val, +- update->kdb_princ_name.utf8str_t_len); +- dbprincstr[update->kdb_princ_name.utf8str_t_len] = 0; ++ goto cleanup; + + ret = krb5_parse_name(context, dbprincstr, &dbprinc); + free(dbprincstr); + if (ret) +- return (ret); ++ goto cleanup; + + ret = krb5_db_get_principal(context, dbprinc, 0, &ent); + krb5_free_principal(context, dbprinc); + if (ret && ret != KRB5_KDB_NOENTRY) +- return (ret); ++ goto cleanup; + is_add = (ret == KRB5_KDB_NOENTRY); + + /* +@@ -643,8 +640,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + + case AT_PRINC: + tmpprinc = conv_princ_2db(context, &u.av_princ); +- if (tmpprinc == NULL) +- return ENOMEM; ++ if (tmpprinc == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + krb5_free_principal(context, ent->princ); + ent->princ = tmpprinc; + break; +@@ -661,8 +660,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + /* Allocate one extra key data to avoid allocating zero bytes. */ + newptr = realloc(ent->key_data, (ent->n_key_data + 1) * + sizeof(krb5_key_data)); +- if (newptr == NULL) +- return ENOMEM; ++ if (newptr == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + ent->key_data = newptr; + + /* BEGIN CSTYLED */ +@@ -677,7 +678,8 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kp->key_data_ver = (krb5_int16)kv->k_ver; + kp->key_data_kvno = (krb5_ui_2)kv->k_kvno; + if (kp->key_data_ver > 2) { +- return EINVAL; /* XXX ? */ ++ ret = EINVAL; /* XXX ? */ ++ goto cleanup; + } + + for (cnt = 0; cnt < kp->key_data_ver; cnt++) { +@@ -685,8 +687,10 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + kp->key_data_length[cnt] = (krb5_int16)kv->k_contents.k_contents_val[cnt].utf8str_t_len; + newptr = realloc(kp->key_data_contents[cnt], + kp->key_data_length[cnt]); +- if (newptr == NULL) +- return ENOMEM; ++ if (newptr == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + kp->key_data_contents[cnt] = newptr; + + (void) memset(kp->key_data_contents[cnt], 0, +@@ -704,22 +708,26 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + newtl.tl_data_length = (krb5_int16)u.av_tldata.av_tldata_val[j].tl_data.tl_data_len; + newtl.tl_data_contents = (krb5_octet *)u.av_tldata.av_tldata_val[j].tl_data.tl_data_val; + newtl.tl_data_next = NULL; +- if ((ret = krb5_dbe_update_tl_data(context, ent, &newtl))) +- return (ret); ++ ret = krb5_dbe_update_tl_data(context, ent, &newtl); ++ if (ret) ++ goto cleanup; + } + break; + /* END CSTYLED */ + } + case AT_PW_LAST_CHANGE: +- if ((ret = krb5_dbe_update_last_pwd_change(context, ent, +- u.av_pw_last_change))) +- return (ret); ++ ret = krb5_dbe_update_last_pwd_change(context, ent, ++ u.av_pw_last_change); ++ if (ret) ++ goto cleanup; + break; + + case AT_MOD_PRINC: + tmpprinc = conv_princ_2db(context, &u.av_mod_princ); +- if (tmpprinc == NULL) +- return ENOMEM; ++ if (tmpprinc == NULL) { ++ ret = ENOMEM; ++ goto cleanup; ++ } + mod_princ = tmpprinc; + break; + +@@ -743,14 +751,17 @@ ulog_conv_2dbentry(krb5_context context, krb5_db_entry **entry, + if (mod_time && mod_princ) { + ret = krb5_dbe_update_mod_princ_data(context, ent, + mod_time, mod_princ); +- krb5_free_principal(context, mod_princ); +- mod_princ = NULL; + if (ret) +- return (ret); ++ goto cleanup; + } + + *entry = ent; +- return (0); ++ ent = NULL; ++ ret = 0; ++cleanup: ++ krb5_db_free_principal(context, ent); ++ krb5_free_principal(context, mod_princ); ++ return ret; + } + + +diff --git a/src/lib/krad/remote.c b/src/lib/krad/remote.c +index a938665f6..7e491e994 100644 +--- a/src/lib/krad/remote.c ++++ b/src/lib/krad/remote.c +@@ -445,7 +445,7 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + { + krad_packet *tmp = NULL; + krb5_error_code retval; +- request *r; ++ request *r, *new_request = NULL; + + if (rr->info->ai_socktype == SOCK_STREAM) + retries = 0; +@@ -464,7 +464,7 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + } + + timeout = timeout / (retries + 1); +- retval = request_new(rr, tmp, timeout, retries, cb, data, &r); ++ retval = request_new(rr, tmp, timeout, retries, cb, data, &new_request); + if (retval != 0) + goto error; + +@@ -472,12 +472,13 @@ kr_remote_send(krad_remote *rr, krad_code code, krad_attrset *attrs, + if (retval != 0) + goto error; + +- K5_TAILQ_INSERT_TAIL(&rr->list, r, list); ++ K5_TAILQ_INSERT_TAIL(&rr->list, new_request, list); + if (pkt != NULL) + *pkt = tmp; + return 0; + + error: ++ free(new_request); + krad_packet_free(tmp); + return retval; + } +diff --git a/src/lib/krb5/krb/gic_keytab.c b/src/lib/krb5/krb/gic_keytab.c +index d20457069..b8b7c1506 100644 +--- a/src/lib/krb5/krb/gic_keytab.c ++++ b/src/lib/krb5/krb/gic_keytab.c +@@ -182,7 +182,7 @@ krb5_init_creds_set_keytab(krb5_context context, + krb5_init_creds_context ctx, + krb5_keytab keytab) + { +- krb5_enctype *etype_list; ++ krb5_enctype *etype_list = NULL; + krb5_error_code ret; + struct canonprinc iter = { ctx->request->client, .subst_defrealm = TRUE }; + krb5_const_principal canonprinc; +@@ -212,6 +212,7 @@ krb5_init_creds_set_keytab(krb5_context context, + free_canonprinc(&iter); + if (ret) { + TRACE_INIT_CREDS_KEYTAB_LOOKUP_FAILED(context, ret); ++ free(etype_list); + return 0; + } + TRACE_INIT_CREDS_KEYTAB_LOOKUP(context, ctx->request->client, etype_list); +diff --git a/src/lib/krb5/krb/s4u_authdata.c b/src/lib/krb5/krb/s4u_authdata.c +index a2300def8..c33a50a56 100644 +--- a/src/lib/krb5/krb/s4u_authdata.c ++++ b/src/lib/krb5/krb/s4u_authdata.c +@@ -170,8 +170,10 @@ s4u2proxy_export_authdata(krb5_context kcontext, + return code; + + authdata[0] = k5alloc(sizeof(krb5_authdata), &code); +- if (authdata[0] == NULL) ++ if (authdata[0] == NULL) { ++ free(authdata); + return code; ++ } + + code = encode_krb5_ad_signedpath(&sp, &data); + if (code != 0) { +diff --git a/src/lib/krb5/krb/send_tgs.c b/src/lib/krb5/krb/send_tgs.c +index 3dda2fdaa..7a11864fa 100644 +--- a/src/lib/krb5/krb/send_tgs.c ++++ b/src/lib/krb5/krb/send_tgs.c +@@ -261,8 +261,10 @@ k5_make_tgs_req(krb5_context context, + pa->length = in_padata[i]->length; + pa->contents = k5memdup(in_padata[i]->contents, in_padata[i]->length, + &ret); +- if (pa->contents == NULL) ++ if (pa->contents == NULL) { ++ free(pa); + goto cleanup; ++ } + padata[i + 1] = pa; + } + req.padata = padata; +@@ -289,7 +291,7 @@ cleanup: + krb5_free_data(context, authdata_asn1); + krb5_free_data(context, req_body_asn1); + krb5_free_data(context, ap_req_asn1); +- krb5_free_pa_data(context, req.padata); ++ krb5_free_pa_data(context, padata); + krb5_free_ticket(context, sec_ticket); + krb5_free_data_contents(context, &authdata_enc.ciphertext); + krb5_free_keyblock(context, subkey); +diff --git a/src/lib/krb5/os/dnssrv.c b/src/lib/krb5/os/dnssrv.c +index 02ba87955..5992a9bdf 100644 +--- a/src/lib/krb5/os/dnssrv.c ++++ b/src/lib/krb5/os/dnssrv.c +@@ -217,7 +217,7 @@ k5_make_uri_query(krb5_context context, const krb5_data *realm, + /* rdlen - 4 bytes remain after the priority and weight. */ + uri->host = k5memdup0(p, rdlen - 4, &ret); + if (uri->host == NULL) { +- ret = errno; ++ free(uri); + goto out; + } + +diff --git a/src/lib/krb5/os/sendto_kdc.c b/src/lib/krb5/os/sendto_kdc.c +index 0eedec175..8620ffa45 100644 +--- a/src/lib/krb5/os/sendto_kdc.c ++++ b/src/lib/krb5/os/sendto_kdc.c +@@ -722,8 +722,10 @@ add_connection(struct conn_state **conns, k5_transport transport, + + if (*udpbufp == NULL) { + *udpbufp = malloc(MAX_DGRAM_SIZE); +- if (*udpbufp == 0) ++ if (*udpbufp == NULL) { ++ free(state); + return ENOMEM; ++ } + } + state->in.buf = *udpbufp; + state->in.bufsize = MAX_DGRAM_SIZE; +diff --git a/src/plugins/audit/kdc_j_encode.c b/src/plugins/audit/kdc_j_encode.c +index 265e95bc4..fb4a4ed73 100755 +--- a/src/plugins/audit/kdc_j_encode.c ++++ b/src/plugins/audit/kdc_j_encode.c +@@ -748,9 +748,8 @@ req_to_value(krb5_kdc_req *req, const krb5_boolean ev_success, + if (ret) + goto error; + ret = addr_to_obj(req->addresses[i], tmpa); +- if (ret) +- goto error; +- ret = k5_json_array_add(arra, tmpa); ++ if (!ret) ++ ret = k5_json_array_add(arra, tmpa); + k5_json_release(tmpa); + if (ret) + goto error; +diff --git a/src/plugins/kdb/db2/adb_policy.c b/src/plugins/kdb/db2/adb_policy.c +index 9bf1931da..221473bb0 100644 +--- a/src/plugins/kdb/db2/adb_policy.c ++++ b/src/plugins/kdb/db2/adb_policy.c +@@ -337,16 +337,16 @@ osa_adb_iter_policy(osa_adb_policy_t db, osa_adb_iter_policy_func func, + } + + while (ret == 0) { +- if (!(entry = (osa_policy_ent_t) malloc(sizeof(osa_policy_ent_rec)))) { +- ret = ENOMEM; ++ entry = k5alloc(sizeof(osa_policy_ent_rec), &ret); ++ if (entry == NULL) + goto error; +- } + + aligned_data = k5memdup(dbdata.data, dbdata.size, &ret); +- if (aligned_data == NULL) ++ if (aligned_data == NULL) { ++ free(entry); + goto error; ++ } + +- memset(entry, 0, sizeof(osa_policy_ent_rec)); + xdrmem_create(&xdrs, aligned_data, dbdata.size, XDR_DECODE); + if(!xdr_osa_policy_ent_rec(&xdrs, entry)) { + xdr_destroy(&xdrs); +diff --git a/src/plugins/kdb/db2/kdb_db2.c b/src/plugins/kdb/db2/kdb_db2.c +index 1a476b586..2c163d91c 100644 +--- a/src/plugins/kdb/db2/kdb_db2.c ++++ b/src/plugins/kdb/db2/kdb_db2.c +@@ -1258,7 +1258,7 @@ krb5_db2_destroy(krb5_context context, char *conf_section, char **db_args) + goto cleanup; + status = osa_adb_destroy_db(polname, plockname, OSA_ADB_POLICY_DB_MAGIC); + if (status) +- return status; ++ goto cleanup; + + status = krb5_db2_fini(context); + +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c +index ce28bf61f..b5a4e5f76 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c ++++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal.c +@@ -188,20 +188,22 @@ krb5_ldap_iterate(krb5_context context, char *match_expr, + for (i=0; values[i] != NULL; ++i) { + if (krb5_ldap_parse_principal_name(values[i], &princ_name) != 0) + continue; +- if (krb5_parse_name(context, princ_name, &principal) != 0) ++ st = krb5_parse_name(context, princ_name, &principal); ++ free(princ_name); ++ if (st) + continue; ++ + if (is_principal_in_realm(ldap_context, principal)) { +- if ((st = populate_krb5_db_entry(context, ldap_context, ld, ent, principal, +- &entry)) != 0) ++ st = populate_krb5_db_entry(context, ldap_context, ld, ++ ent, principal, &entry); ++ krb5_free_principal(context, principal); ++ if (st) + goto cleanup; + (*func)(func_arg, &entry); + krb5_dbe_free_contents(context, &entry); +- (void) krb5_free_principal(context, principal); +- free(princ_name); + break; + } + (void) krb5_free_principal(context, principal); +- free(princ_name); + } + ldap_value_free(values); + } +diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +index 8d97a29b6..ff705a2cc 100644 +--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c ++++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c +@@ -785,6 +785,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + char *polname = NULL; + OPERATION optype; + krb5_boolean found_entry = FALSE; ++ char *filter = NULL; + + /* Clear the global error string */ + krb5_clear_error_message(context); +@@ -833,7 +834,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if (entry->mask & KADM5_LOAD) { + unsigned int tree = 0; + int numlentries = 0; +- char *filter = NULL; + + /* A load operation is special, will do a mix-in (add krbprinc + * attrs to a non-krb object entry) if an object exists with a +@@ -864,7 +864,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if (st == LDAP_SUCCESS) { + numlentries = ldap_count_entries(ld, result); + if (numlentries > 1) { +- free(filter); + st = EINVAL; + k5_setmsg(context, st, + _("operation can not continue, more than one " +@@ -880,7 +879,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + if ((principal_dn = ldap_get_dn(ld, ent)) == NULL) { + ldap_get_option (ld, LDAP_OPT_RESULT_CODE, &st); + st = set_ldap_error (context, st, 0); +- free(filter); + goto cleanup; + } + } +@@ -889,7 +887,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + } else if (st != LDAP_NO_SUCH_OBJECT) { + /* could not perform search, return with failure */ + st = set_ldap_error (context, st, 0); +- free(filter); + goto cleanup; + } + ldap_msgfree(result); +@@ -900,8 +897,6 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + */ + } /* end for (tree = 0; principal_dn == ... */ + +- free(filter); +- + if (found_entry == FALSE && principal_dn != NULL) { + /* + * if principal_dn is null then there is code further down to +@@ -1450,6 +1445,7 @@ krb5_ldap_put_principal(krb5_context context, krb5_db_entry *entry, + } + + cleanup: ++ free(filter); + if (user) + free(user); + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index ce4233953..0204ad8ba 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -3653,14 +3653,15 @@ static krb5_error_code + pkinit_open_session(krb5_context context, + pkinit_identity_crypto_context cctx) + { +- CK_ULONG i, r; ++ CK_ULONG i, pret; + unsigned char *cp; + size_t label_len; + CK_ULONG count = 0; +- CK_SLOT_ID_PTR slotlist; ++ CK_SLOT_ID_PTR slotlist = NULL; + CK_TOKEN_INFO tinfo; +- char *p11name; ++ char *p11name = NULL; + const char *password; ++ krb5_error_code ret; + + if (cctx->p11_module != NULL) + return 0; /* session already open */ +@@ -3672,8 +3673,9 @@ pkinit_open_session(krb5_context context, + return KRB5KDC_ERR_PREAUTH_FAILED; + + /* Init */ +- if ((r = cctx->p11->C_Initialize(NULL)) != CKR_OK) { +- pkiDebug("C_Initialize: %s\n", pkcs11err(r)); ++ pret = cctx->p11->C_Initialize(NULL); ++ if (pret != CKR_OK) { ++ pkiDebug("C_Initialize: %s\n", pkcs11err(pret)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +@@ -3687,8 +3689,10 @@ pkinit_open_session(krb5_context context, + slotlist = calloc(count, sizeof(CK_SLOT_ID)); + if (slotlist == NULL) + return ENOMEM; +- if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ if (cctx->p11->C_GetSlotList(TRUE, slotlist, &count) != CKR_OK) { ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; ++ } + + /* Look for the given token label, or if none given take the first one */ + for (i = 0; i < count; i++) { +@@ -3697,16 +3701,20 @@ pkinit_open_session(krb5_context context, + continue; + + /* Open session */ +- if ((r = cctx->p11->C_OpenSession(slotlist[i], CKF_SERIAL_SESSION, +- NULL, NULL, &cctx->session)) != CKR_OK) { +- pkiDebug("C_OpenSession: %s\n", pkcs11err(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ 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; ++ goto cleanup; + } + + /* Get token info */ +- if ((r = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo)) != CKR_OK) { +- pkiDebug("C_GetTokenInfo: %s\n", pkcs11err(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ pret = cctx->p11->C_GetTokenInfo(slotlist[i], &tinfo); ++ if (pret != CKR_OK) { ++ pkiDebug("C_GetTokenInfo: %s\n", pkcs11err(pret)); ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; + } + + /* tinfo.label is zero-filled but not necessarily zero-terminated. +@@ -3726,12 +3734,11 @@ pkinit_open_session(krb5_context context, + cctx->p11->C_CloseSession(cctx->session); + } + if (i >= count) { +- free(slotlist); + TRACE_PKINIT_PKCS11_NO_MATCH_TOKEN(context); +- return KRB5KDC_ERR_PREAUTH_FAILED; ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; + } + cctx->slotid = slotlist[i]; +- free(slotlist); + pkiDebug("open_session: slotid %d (%lu of %d)\n", (int)cctx->slotid, + i + 1, (int) count); + +@@ -3751,23 +3758,26 @@ pkinit_open_session(krb5_context context, + (int)label_len, tinfo.label) < 0) + p11name = NULL; + } +- } else { +- p11name = NULL; + } + if (cctx->defer_id_prompt) { + /* Supply the identity name to be passed to the responder. */ + pkinit_set_deferred_id(&cctx->deferred_ids, + p11name, tinfo.flags, NULL); +- free(p11name); +- return 0; ++ ret = 0; ++ goto cleanup; + } + /* Look up a responder-supplied password for the token. */ + password = pkinit_find_deferred_id(cctx->deferred_ids, p11name); +- free(p11name); +- r = pkinit_login(context, cctx, &tinfo, password); ++ ret = pkinit_login(context, cctx, &tinfo, password); ++ if (ret) ++ goto cleanup; + } + +- return r; ++ ret = 0; ++cleanup: ++ free(slotlist); ++ free(p11name); ++ return ret; + } + + /* +diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c +index 3c1778a3b..a5a979f27 100644 +--- a/src/plugins/preauth/pkinit/pkinit_identity.c ++++ b/src/plugins/preauth/pkinit/pkinit_identity.c +@@ -310,17 +310,17 @@ parse_fs_options(krb5_context context, + const char *residual) + { + char *certname, *keyname, *save; +- char *cert_filename = NULL, *key_filename = NULL; ++ char *copy = NULL, *cert_filename = NULL, *key_filename = NULL; + krb5_error_code retval = ENOMEM; + + if (residual == NULL || residual[0] == '\0' || residual[0] == ',') + return EINVAL; + +- certname = strdup(residual); +- if (certname == NULL) ++ copy = strdup(residual); ++ if (copy == NULL) + goto cleanup; + +- certname = strtok_r(certname, ",", &save); ++ certname = strtok_r(copy, ",", &save); + if (certname == NULL) + goto cleanup; + keyname = strtok_r(NULL, ",", &save); +@@ -341,7 +341,7 @@ parse_fs_options(krb5_context context, + retval = 0; + + cleanup: +- free(certname); ++ free(copy); + free(cert_filename); + free(key_filename); + return retval; +diff --git a/src/util/profile/prof_get.c b/src/util/profile/prof_get.c +index 16a1762df..0e14200ca 100644 +--- a/src/util/profile/prof_get.c ++++ b/src/util/profile/prof_get.c +@@ -157,7 +157,7 @@ profile_get_values(profile_t profile, const char *const *names, + char ***ret_values) + { + errcode_t retval; +- void *state; ++ void *state = NULL; + char *value; + struct profile_string_list values; + +@@ -172,8 +172,9 @@ profile_get_values(profile_t profile, const char *const *names, + &state))) + return retval; + +- if ((retval = init_list(&values))) +- return retval; ++ retval = init_list(&values); ++ if (retval) ++ goto cleanup; + + do { + if ((retval = profile_node_iterator(&state, 0, 0, &value))) +@@ -187,11 +188,9 @@ profile_get_values(profile_t profile, const char *const *names, + goto cleanup; + } + +- end_list(&values, ret_values); +- return 0; +- + cleanup: +- end_list(&values, 0); ++ end_list(&values, retval ? NULL : ret_values); ++ profile_node_iterator_free(&state); + return retval; + } + +-- +2.27.0 + diff --git a/backport-Fix-memory-leak-in-OTP-kdcpreauth-module.patch b/backport-Fix-memory-leak-in-OTP-kdcpreauth-module.patch new file mode 100644 index 0000000..a73c8c0 --- /dev/null +++ b/backport-Fix-memory-leak-in-OTP-kdcpreauth-module.patch @@ -0,0 +1,51 @@ +From 5ad465bc8e0d957a4945218bea487b77622bf433 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Fri, 3 Jun 2022 14:30:42 -0400 +Subject: [PATCH] Fix memory leak in OTP kdcpreauth module + +In otp_edata(), free the generated nonce. + +ticket: 9063 (new) +tags: pullup +target_version: 1.20-next +target_version: 1.19-next +--- + src/plugins/preauth/otp/main.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/src/plugins/preauth/otp/main.c b/src/plugins/preauth/otp/main.c +index 119714f99..0e682aae5 100644 +--- a/src/plugins/preauth/otp/main.c ++++ b/src/plugins/preauth/otp/main.c +@@ -228,7 +228,7 @@ otp_edata(krb5_context context, krb5_kdc_req *request, + krb5_pa_otp_challenge chl; + krb5_pa_data *pa = NULL; + krb5_error_code retval; +- krb5_data *encoding; ++ krb5_data *encoding, nonce = empty_data(); + char *config; + + /* Determine if otp is enabled for the user. */ +@@ -256,9 +256,10 @@ otp_edata(krb5_context context, krb5_kdc_req *request, + ti.iteration_count = -1; + + /* Generate the nonce. */ +- retval = nonce_generate(context, armor_key->length, &chl.nonce); ++ retval = nonce_generate(context, armor_key->length, &nonce); + if (retval != 0) + goto out; ++ chl.nonce = nonce; + + /* Build the output pa-data. */ + retval = encode_krb5_pa_otp_challenge(&chl, &encoding); +@@ -275,6 +276,7 @@ otp_edata(krb5_context context, krb5_kdc_req *request, + free(encoding); + + out: ++ krb5_free_data_contents(context, &nonce); + (*respond)(arg, retval, pa); + } + +-- +2.27.0 + diff --git a/backport-Fix-multiple-UPN-handling-in-PKINIT-client-certs.patch b/backport-Fix-multiple-UPN-handling-in-PKINIT-client-certs.patch new file mode 100644 index 0000000..b3273b7 --- /dev/null +++ b/backport-Fix-multiple-UPN-handling-in-PKINIT-client-certs.patch @@ -0,0 +1,57 @@ +From 4e325cadee4f5511e494f0b4fd9faeb24e7b7c08 Mon Sep 17 00:00:00 2001 +From: Ken Hornstein +Date: Wed, 17 Mar 2021 17:44:46 -0400 +Subject: [PATCH] Fix multiple UPN handling in PKINIT client certs + +Commit 0f26c1c7504777d6e7bfa1d3dee575c504ab6c05 neglected to increment +the array index when storing UPN strings. Also remove the unused +num_found variable. + +[ghudson@mit.edu: pulled from a larger commit; added removal of +num_found; wrote commit message] + +ticket: 9000 (new) +--- + src/plugins/preauth/pkinit/pkinit_crypto_openssl.c | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index fbbdab510..263910480 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -1964,7 +1964,7 @@ crypto_retrieve_X509_sans(krb5_context context, + krb5_principal *princs = NULL; + char **upns = NULL; + unsigned char **dnss = NULL; +- unsigned int i, num_found = 0, num_sans = 0; ++ unsigned int i, num_sans = 0; + X509_EXTENSION *ext = NULL; + GENERAL_NAMES *ialt = NULL; + GENERAL_NAME *gen = NULL; +@@ -2047,7 +2047,6 @@ crypto_retrieve_X509_sans(krb5_context context, + __FUNCTION__); + } else { + p++; +- num_found++; + } + } else if (upns != NULL && + OBJ_cmp(plgctx->id_ms_san_upn, +@@ -2058,6 +2057,7 @@ crypto_retrieve_X509_sans(krb5_context context, + upns[u] = k5memdup0(name.data, name.length, &ret); + if (upns[u] == NULL) + goto cleanup; ++ u++; + } else { + pkiDebug("%s: unrecognized othername oid in SAN\n", + __FUNCTION__); +@@ -2079,7 +2079,6 @@ crypto_retrieve_X509_sans(krb5_context context, + __FUNCTION__); + } else { + d++; +- num_found++; + } + } + break; +-- +2.27.0 + diff --git a/backport-Fix-uncommon-PKINIT-memory-leak.patch b/backport-Fix-uncommon-PKINIT-memory-leak.patch new file mode 100644 index 0000000..4502d6e --- /dev/null +++ b/backport-Fix-uncommon-PKINIT-memory-leak.patch @@ -0,0 +1,57 @@ +From 883415036a4b4e0372b84a5a6e46c10b3a67aba0 Mon Sep 17 00:00:00 2001 +From: sashan +Date: Sun, 29 May 2022 10:32:57 +0200 +Subject: [PATCH] Fix uncommon PKINIT memory leak + +PKINIT per-request module data objects are normally created by +pkinit_server_verify_padata() and freed by +pkinit_server_return_padata(). In some unusual circumstances, the KDC +may not call the return_padata method after verification succeeds. +Add a free_modreq method and free the object there instead. + +[ghudson@mit.edu: rewrote commit message] + +ticket: 9065 (new) +tags: pullup +target_version: 1.20-next +target_version: 1.19-next +--- + src/plugins/preauth/pkinit/pkinit_srv.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_srv.c b/src/plugins/preauth/pkinit/pkinit_srv.c +index 1147a8fc2..865c543c4 100644 +--- a/src/plugins/preauth/pkinit/pkinit_srv.c ++++ b/src/plugins/preauth/pkinit/pkinit_srv.c +@@ -1022,7 +1022,6 @@ pkinit_server_return_padata(krb5_context context, + (*send_pa)->contents = (krb5_octet *) out_data->data; + + cleanup: +- pkinit_fini_kdc_req_context(context, reqctx); + free(scratch.data); + free(out_data); + if (encoded_dhkey_info != NULL) +@@ -1612,6 +1611,13 @@ pkinit_fini_kdc_req_context(krb5_context context, void *ctx) + free(reqctx); + } + ++static void ++pkinit_free_modreq(krb5_context context, krb5_kdcpreauth_moddata moddata, ++ krb5_kdcpreauth_modreq modreq) ++{ ++ pkinit_fini_kdc_req_context(context, modreq); ++} ++ + krb5_error_code + kdcpreauth_pkinit_initvt(krb5_context context, int maj_ver, int min_ver, + krb5_plugin_vtable vtable); +@@ -1633,5 +1639,6 @@ kdcpreauth_pkinit_initvt(krb5_context context, int maj_ver, int min_ver, + vt->edata = pkinit_server_get_edata; + vt->verify = pkinit_server_verify_padata; + vt->return_padata = pkinit_server_return_padata; ++ vt->free_modreq = pkinit_free_modreq; + return 0; + } +-- +2.27.0 + diff --git a/backport-Free-verto-context-later-in-KDC-cleanup.patch b/backport-Free-verto-context-later-in-KDC-cleanup.patch new file mode 100644 index 0000000..84bcf5e --- /dev/null +++ b/backport-Free-verto-context-later-in-KDC-cleanup.patch @@ -0,0 +1,44 @@ +From 8dcace04945723cd6a3c8ea2c1ba467c22eb6584 Mon Sep 17 00:00:00 2001 +From: Greg Hudson +Date: Fri, 3 Jun 2022 14:38:45 -0400 +Subject: [PATCH] Free verto context later in KDC cleanup + +The KDC supplies the verto context to kdcpreauth modules via the loop +method (added in commit 83b4ecd20e50ad330cd761977d5dadefe30a785b). +This context should remain valid until kdcpreauth modules are +unloaded, as modules might refer to it during cleanup. In particular, +the OTP module references the verto context when freeing the RADIUS +client object (commit e89abc2d4ea1fea1ec28d470f297514b828e4842), which +can cause a memory error during KDC shutdown without this change. + +ticket: 9064 (new) +tags: pullup +target_version: 1.20-next +target_version: 1.19-next +--- + src/kdc/main.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/kdc/main.c b/src/kdc/main.c +index be6e361b8..bfdfef5c4 100644 +--- a/src/kdc/main.c ++++ b/src/kdc/main.c +@@ -1037,7 +1037,6 @@ int main(int argc, char **argv) + kau_kdc_start(kcontext, TRUE); + + verto_run(ctx); +- loop_free(ctx); + kau_kdc_stop(kcontext, TRUE); + krb5_klog_syslog(LOG_INFO, _("shutting down")); + unload_preauth_plugins(kcontext); +@@ -1051,6 +1050,7 @@ int main(int argc, char **argv) + #ifndef NOCACHE + kdc_free_lookaside(kcontext); + #endif ++ loop_free(ctx); + krb5_free_context(kcontext); + return errout; + } +-- +2.27.0 + diff --git a/backport-Modernize-pkinit_get_certs_pkcs11.patch b/backport-Modernize-pkinit_get_certs_pkcs11.patch new file mode 100644 index 0000000..af663c1 --- /dev/null +++ b/backport-Modernize-pkinit_get_certs_pkcs11.patch @@ -0,0 +1,320 @@ +From cddd5c589b1a13561e75c647cd51067c16a5697d Mon Sep 17 00:00:00 2001 +From: Robbie Harwood +Date: Sat, 29 May 2021 14:54:56 -0400 +Subject: [PATCH] Modernize pkinit_get_certs_pkcs11 + +Remove unusable PKINIT_USE_MECH_LIST code since there's no build +system support and it would leak mechp. Factor out the code to load a +cert from the card. Avoid mixing PKCS11 and krb5 error codes. Fix +leaks of cert and cert_id reported by Coverity. +--- + .../preauth/pkinit/pkinit_crypto_openssl.c | 230 +++++++++--------- + 1 file changed, 113 insertions(+), 117 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index 2f2dec599..ce4233953 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -4510,6 +4510,89 @@ reassemble_pkcs11_name(pkinit_identity_opts *idopts) + return ret; + } + ++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) ++{ ++ krb5_error_code ret; ++ CK_ATTRIBUTE attrs[2]; ++ CK_BYTE_PTR cert = NULL, cert_id = NULL; ++ CK_RV pret; ++ const unsigned char *cp; ++ CK_OBJECT_HANDLE obj; ++ CK_ULONG count; ++ X509 *x = NULL; ++ pkinit_cred_info cred; ++ ++ *cred_out = NULL; ++ ++ /* Look for X.509 cert. */ ++ pret = p11->C_FindObjects(session, &obj, 1, &count); ++ if (pret != CKR_OK || count <= 0) ++ return 0; ++ ++ /* Get cert and id len. */ ++ attrs[0].type = CKA_VALUE; ++ attrs[0].pValue = NULL; ++ attrs[0].ulValueLen = 0; ++ 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; ++ goto cleanup; ++ } ++ ++ /* Allocate buffers and read the cert and id. */ ++ cert = k5alloc(attrs[0].ulValueLen + 1, &ret); ++ if (cert == NULL) ++ goto cleanup; ++ cert_id = k5alloc(attrs[1].ulValueLen + 1, &ret); ++ if (cert_id == NULL) ++ goto cleanup; ++ attrs[0].type = CKA_VALUE; ++ 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; ++ goto cleanup; ++ } ++ ++ pkiDebug("cert: size %d, id %d, idlen %d\n", (int)attrs[0].ulValueLen, ++ (int)cert_id[0], (int)attrs[1].ulValueLen); ++ ++ cp = (unsigned char *)cert; ++ x = d2i_X509(NULL, &cp, (int)attrs[0].ulValueLen); ++ if (x == NULL) { ++ ret = KRB5KDC_ERR_PREAUTH_FAILED; ++ goto cleanup; ++ } ++ ++ cred = k5alloc(sizeof(struct _pkinit_cred_info), &ret); ++ if (cred == NULL) ++ goto cleanup; ++ ++ cred->name = reassemble_pkcs11_name(idopts); ++ cred->cert = x; ++ cred->key = NULL; ++ cred->cert_id = cert_id; ++ cred->cert_id_len = attrs[1].ulValueLen; ++ ++ *cred_out = cred; ++ cert_id = NULL; ++ ret = 0; ++ ++cleanup: ++ free(cert); ++ free(cert_id); ++ return ret; ++} ++ + static krb5_error_code + pkinit_get_certs_pkcs11(krb5_context context, + pkinit_plg_crypto_context plg_cryptoctx, +@@ -4518,20 +4601,13 @@ pkinit_get_certs_pkcs11(krb5_context context, + pkinit_identity_crypto_context id_cryptoctx, + krb5_principal princ) + { +-#ifdef PKINIT_USE_MECH_LIST +- CK_MECHANISM_TYPE_PTR mechp; +- CK_MECHANISM_INFO info; +-#endif + CK_OBJECT_CLASS cls; +- CK_OBJECT_HANDLE obj; + CK_ATTRIBUTE attrs[4]; +- CK_ULONG count; + CK_CERTIFICATE_TYPE certtype; +- CK_BYTE_PTR cert = NULL, cert_id; +- const unsigned char *cp; +- int i, r; ++ int i; + unsigned int nattrs; +- X509 *x = NULL; ++ krb5_error_code ret; ++ CK_RV pret; + + /* Copy stuff from idopts -> id_cryptoctx */ + if (idopts->p11_module_name != NULL) { +@@ -4552,20 +4628,20 @@ pkinit_get_certs_pkcs11(krb5_context context, + } + /* Convert the ascii cert_id string into a binary blob */ + if (idopts->cert_id_string != NULL) { +- r = k5_hex_decode(idopts->cert_id_string, +- &id_cryptoctx->cert_id, &id_cryptoctx->cert_id_len); +- if (r != 0) { ++ ret = k5_hex_decode(idopts->cert_id_string, &id_cryptoctx->cert_id, ++ &id_cryptoctx->cert_id_len); ++ if (ret) { + pkiDebug("Failed to convert certid string [%s]\n", + idopts->cert_id_string); +- return r; ++ return ret; + } + } + id_cryptoctx->slotid = idopts->slotid; + id_cryptoctx->pkcs11_method = 1; + +- r = pkinit_open_session(context, id_cryptoctx); +- if (r != 0) +- return r; ++ ret = pkinit_open_session(context, id_cryptoctx); ++ if (ret) ++ return ret; + if (id_cryptoctx->defer_id_prompt) { + /* + * We need to reset all of the PKCS#11 state, so that the next time we +@@ -4577,56 +4653,23 @@ pkinit_get_certs_pkcs11(krb5_context context, + return 0; + } + +-#ifndef PKINIT_USE_MECH_LIST + /* + * We'd like to use CKM_SHA1_RSA_PKCS for signing if it's available, but + * many cards seems to be confused about whether they are capable of + * this or not. The safe thing seems to be to ignore the mechanism list, + * always use CKM_RSA_PKCS and calculate the sha1 digest ourselves. + */ +- + id_cryptoctx->mech = CKM_RSA_PKCS; +-#else +- if ((r = id_cryptoctx->p11->C_GetMechanismList(id_cryptoctx->slotid, NULL, +- &count)) != CKR_OK || count <= 0) { +- pkiDebug("C_GetMechanismList: %s\n", pkcs11err(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; +- } +- mechp = malloc(count * sizeof (CK_MECHANISM_TYPE)); +- if (mechp == NULL) +- return ENOMEM; +- if ((r = id_cryptoctx->p11->C_GetMechanismList(id_cryptoctx->slotid, +- mechp, &count)) != CKR_OK) +- return KRB5KDC_ERR_PREAUTH_FAILED; +- for (i = 0; i < count; i++) { +- if ((r = id_cryptoctx->p11->C_GetMechanismInfo(id_cryptoctx->slotid, +- mechp[i], &info)) != CKR_OK) +- return KRB5KDC_ERR_PREAUTH_FAILED; +-#ifdef DEBUG_MECHINFO +- pkiDebug("mech %x flags %x\n", (int) mechp[i], (int) info.flags); +- if ((info.flags & (CKF_SIGN|CKF_DECRYPT)) == (CKF_SIGN|CKF_DECRYPT)) +- pkiDebug(" this mech is good for sign & decrypt\n"); +-#endif +- if (mechp[i] == CKM_RSA_PKCS) { +- /* This seems backwards... */ +- id_cryptoctx->mech = +- (info.flags & CKF_SIGN) ? CKM_SHA1_RSA_PKCS : CKM_RSA_PKCS; +- } +- } +- free(mechp); +- +- pkiDebug("got %d mechs from card\n", (int) count); +-#endif + + cls = CKO_CERTIFICATE; + attrs[0].type = CKA_CLASS; + attrs[0].pValue = &cls; +- attrs[0].ulValueLen = sizeof cls; ++ attrs[0].ulValueLen = sizeof(cls); + + certtype = CKC_X_509; + attrs[1].type = CKA_CERTIFICATE_TYPE; + attrs[1].pValue = &certtype; +- attrs[1].ulValueLen = sizeof certtype; ++ attrs[1].ulValueLen = sizeof(certtype); + + nattrs = 2; + +@@ -4644,80 +4687,33 @@ pkinit_get_certs_pkcs11(krb5_context context, + nattrs++; + } + +- r = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, nattrs); +- if (r != CKR_OK) { +- pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(r)); ++ pret = id_cryptoctx->p11->C_FindObjectsInit(id_cryptoctx->session, attrs, ++ nattrs); ++ if (pret != CKR_OK) { ++ pkiDebug("C_FindObjectsInit: %s\n", pkcs11err(pret)); + return KRB5KDC_ERR_PREAUTH_FAILED; + } + +- for (i = 0; ; i++) { +- if (i >= MAX_CREDS_ALLOWED) +- return KRB5KDC_ERR_PREAUTH_FAILED; +- +- /* Look for x.509 cert */ +- if ((r = id_cryptoctx->p11->C_FindObjects(id_cryptoctx->session, +- &obj, 1, &count)) != CKR_OK || count <= 0) { +- id_cryptoctx->creds[i] = NULL; +- break; +- } +- +- /* Get cert and id len */ +- attrs[0].type = CKA_VALUE; +- attrs[0].pValue = NULL; +- attrs[0].ulValueLen = 0; +- +- attrs[1].type = CKA_ID; +- attrs[1].pValue = NULL; +- attrs[1].ulValueLen = 0; +- +- if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session, +- obj, attrs, 2)) != CKR_OK && r != CKR_BUFFER_TOO_SMALL) { +- pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; +- } +- cert = (CK_BYTE_PTR) malloc((size_t) attrs[0].ulValueLen + 1); +- cert_id = (CK_BYTE_PTR) malloc((size_t) attrs[1].ulValueLen + 1); +- if (cert == NULL || cert_id == NULL) +- return ENOMEM; +- +- /* Read the cert and id off the card */ +- +- attrs[0].type = CKA_VALUE; +- attrs[0].pValue = cert; +- +- attrs[1].type = CKA_ID; +- attrs[1].pValue = cert_id; +- +- if ((r = id_cryptoctx->p11->C_GetAttributeValue(id_cryptoctx->session, +- obj, attrs, 2)) != CKR_OK) { +- pkiDebug("C_GetAttributeValue: %s\n", pkcs11err(r)); +- return KRB5KDC_ERR_PREAUTH_FAILED; +- } +- +- pkiDebug("cert %d size %d id %d idlen %d\n", i, +- (int) attrs[0].ulValueLen, (int) cert_id[0], +- (int) attrs[1].ulValueLen); +- +- cp = (unsigned char *) cert; +- x = d2i_X509(NULL, &cp, (int) attrs[0].ulValueLen); +- if (x == NULL) +- return KRB5KDC_ERR_PREAUTH_FAILED; +- id_cryptoctx->creds[i] = malloc(sizeof(struct _pkinit_cred_info)); ++ for (i = 0; i < MAX_CREDS_ALLOWED; i++) { ++ ret = load_one_cert(id_cryptoctx->p11, id_cryptoctx->session, idopts, ++ &id_cryptoctx->creds[i]); ++ if (ret) ++ return ret; + if (id_cryptoctx->creds[i] == NULL) +- return KRB5KDC_ERR_PREAUTH_FAILED; +- id_cryptoctx->creds[i]->name = reassemble_pkcs11_name(idopts); +- id_cryptoctx->creds[i]->cert = x; +- id_cryptoctx->creds[i]->key = NULL; +- id_cryptoctx->creds[i]->cert_id = cert_id; +- id_cryptoctx->creds[i]->cert_id_len = attrs[1].ulValueLen; +- free(cert); ++ break; + } ++ if (i == MAX_CREDS_ALLOWED) ++ return KRB5KDC_ERR_PREAUTH_FAILED; ++ + id_cryptoctx->p11->C_FindObjectsFinal(id_cryptoctx->session); +- if (cert == NULL) ++ ++ /* Check if we found no certs. */ ++ if (id_cryptoctx->creds[0] == NULL) + return KRB5KDC_ERR_PREAUTH_FAILED; + return 0; + } +-#endif ++ ++#endif /* !WITHOUT_PKCS11 */ + + + static void +-- +2.27.0 + diff --git a/backport-Use-krb5int_open_plugin-for-PKCS-11-module.patch b/backport-Use-krb5int_open_plugin-for-PKCS-11-module.patch new file mode 100644 index 0000000..e0c490f --- /dev/null +++ b/backport-Use-krb5int_open_plugin-for-PKCS-11-module.patch @@ -0,0 +1,145 @@ +From c5c11839e02c7993eb78f2c94c75c10cf93f2195 Mon Sep 17 00:00:00 2001 +From: Ken Hornstein +Date: Sun, 14 Mar 2021 22:18:53 -0400 +Subject: [PATCH] Use krb5int_open_plugin for PKCS#11 module + +Instead of calling dlopen() directly, use the krb5 cross-platform +interfaces (krb5int_open_plugin()). + +The goal here is to eventually support pkinit on Windows; this is just +the first small step in that direction. + +[ghudson@mit.edu: fixed memory leak; changed type of p11_module field; +added intermediate sym variable for strict aliasing conformance; +simplified out pkinit_C_UnloadModule() wrapper] +--- + src/plugins/preauth/pkinit/pkinit_clnt.c | 1 - + .../preauth/pkinit/pkinit_crypto_openssl.c | 37 ++++++++++--------- + .../preauth/pkinit/pkinit_crypto_openssl.h | 2 +- + src/plugins/preauth/pkinit/pkinit_identity.c | 1 - + 4 files changed, 20 insertions(+), 21 deletions(-) + +diff --git a/src/plugins/preauth/pkinit/pkinit_clnt.c b/src/plugins/preauth/pkinit/pkinit_clnt.c +index d29b03dfb..b6266b4b5 100644 +--- a/src/plugins/preauth/pkinit/pkinit_clnt.c ++++ b/src/plugins/preauth/pkinit/pkinit_clnt.c +@@ -34,7 +34,6 @@ + #include "k5-json.h" + + #include +-#include + #include + + /** +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +index e5940a513..fbbdab510 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.c +@@ -33,7 +33,6 @@ + #include "pkinit_crypto_openssl.h" + #include "k5-buf.h" + #include "k5-hex.h" +-#include + #include + #include + #include +@@ -102,8 +101,8 @@ static krb5_error_code pkinit_login + CK_TOKEN_INFO *tip, const char *password); + static krb5_error_code pkinit_open_session + (krb5_context context, pkinit_identity_crypto_context id_cryptoctx); +-static void * pkinit_C_LoadModule(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p); +-static CK_RV pkinit_C_UnloadModule(void *handle); ++static struct plugin_file_handle *pkinit_C_LoadModule ++(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p); + #ifdef SILLYDECRYPT + CK_RV pkinit_C_Decrypt + (pkinit_identity_crypto_context id_cryptoctx, +@@ -1006,7 +1005,7 @@ pkinit_fini_pkcs11(pkinit_identity_crypto_context ctx) + ctx->p11 = NULL; + } + if (ctx->p11_module != NULL) { +- pkinit_C_UnloadModule(ctx->p11_module); ++ krb5int_close_plugin(ctx->p11_module); + ctx->p11_module = NULL; + } + free(ctx->p11_module_name); +@@ -3548,21 +3547,30 @@ prepare_enc_data(const uint8_t *indata, int indata_len, uint8_t **outdata, + } + + #ifndef WITHOUT_PKCS11 +-static void * ++static struct plugin_file_handle * + pkinit_C_LoadModule(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p) + { +- void *handle; ++ struct plugin_file_handle *handle; + CK_RV (*getflist)(CK_FUNCTION_LIST_PTR_PTR); ++ struct errinfo einfo = EMPTY_ERRINFO; ++ void (*sym)(); ++ long err; ++ CK_RV rv; + + pkiDebug("loading module \"%s\"... ", modname); +- handle = dlopen(modname, RTLD_NOW); +- if (handle == NULL) { ++ if (krb5int_open_plugin(modname, &handle, &einfo) != 0) { + pkiDebug("not found\n"); + return NULL; + } +- getflist = (CK_RV (*)(CK_FUNCTION_LIST_PTR_PTR)) dlsym(handle, "C_GetFunctionList"); +- if (getflist == NULL || (*getflist)(p11p) != CKR_OK) { +- dlclose(handle); ++ ++ err = krb5int_get_plugin_func(handle, "C_GetFunctionList", &sym, &einfo); ++ k5_clear_error(&einfo); ++ if (!err) { ++ getflist = (CK_RV (*)())sym; ++ rv = (*getflist)(p11p); ++ } ++ if (err || rv != CKR_OK) { ++ krb5int_close_plugin(handle); + pkiDebug("failed\n"); + return NULL; + } +@@ -3570,13 +3578,6 @@ pkinit_C_LoadModule(const char *modname, CK_FUNCTION_LIST_PTR_PTR p11p) + return handle; + } + +-static CK_RV +-pkinit_C_UnloadModule(void *handle) +-{ +- dlclose(handle); +- return CKR_OK; +-} +- + static krb5_error_code + pkinit_login(krb5_context context, + pkinit_identity_crypto_context id_cryptoctx, +diff --git a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h +index 957c3def4..ea28b8edc 100644 +--- a/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h ++++ b/src/plugins/preauth/pkinit/pkinit_crypto_openssl.h +@@ -84,7 +84,7 @@ struct _pkinit_identity_crypto_context { + char *token_label; + char *cert_label; + /* These are crypto-specific */ +- void *p11_module; ++ struct plugin_file_handle *p11_module; + CK_SESSION_HANDLE session; + CK_FUNCTION_LIST_PTR p11; + uint8_t *cert_id; +diff --git a/src/plugins/preauth/pkinit/pkinit_identity.c b/src/plugins/preauth/pkinit/pkinit_identity.c +index cee448db9..4c8e8434c 100644 +--- a/src/plugins/preauth/pkinit/pkinit_identity.c ++++ b/src/plugins/preauth/pkinit/pkinit_identity.c +@@ -30,7 +30,6 @@ + */ + + #include "pkinit.h" +-#include + #include + + static void +-- +2.27.0 + diff --git a/krb5.spec b/krb5.spec index cbcff9f..398b91c 100644 --- a/krb5.spec +++ b/krb5.spec @@ -3,7 +3,7 @@ Name: krb5 Version: 1.19.2 -Release: 2 +Release: 3 Summary: The Kerberos network authentication protocol License: MIT URL: http://web.mit.edu/kerberos/www/ @@ -27,6 +27,15 @@ Patch4: fix-debuginfo-with-y.tab.c.patch Patch5: Remove-3des-support.patch Patch6: FIPS-with-PRNG-and-RADIUS-and-MD4.patch Patch7: backport-CVE-2021-37750.patch +Patch8: backport-Use-krb5int_open_plugin-for-PKCS-11-module.patch +Patch9: backport-Fix-multiple-UPN-handling-in-PKINIT-client-certs.patch +Patch10: backport-Add-additional-KRB5_TRACE-points.patch +Patch11: backport-Fix-leaks-on-error-in-kadm5-init-functions.patch +Patch12: backport-Fix-many-unlikely-memory-leaks.patch +Patch13: backport-Modernize-pkinit_get_certs_pkcs11.patch +Patch14: backport-Fix-uncommon-PKINIT-memory-leak.patch +Patch15: backport-Fix-memory-leak-in-OTP-kdcpreauth-module.patch +Patch16: backport-Free-verto-context-later-in-KDC-cleanup.patch BuildRequires: gettext BuildRequires: gcc make automake autoconf pkgconfig pam-devel libselinux-devel byacc @@ -318,6 +327,9 @@ make -C src check || : %{_mandir}/man8/* %changelog +* Tue Oct 11 2022 yixiangzhike - 1.19.2-3 +- Backport upstream patches + * Tue Mar 8 2022 yixiangzhike - 1.19.2-2 - Add ExecStartPost option to krb5kdc.service for solving error message when krb5kdc starting -- Gitee