ipa-kdb: do not use OpenLDAP functions with NULL LDAP context

Calling to ipadb_get_connection() will remove LDAP context if any error
happens. This means upper layers must always verify that LDAP context
exists after such calls.

ipadb_get_user_auth() may re-read global configuration and that may fail
and cause IPA context to have NULL LDAP context.

Fixes: https://pagure.io/freeipa/issue/8681

Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
This commit is contained in:
Alexander Bokovoy 2021-02-19 15:37:47 +02:00 committed by Rob Crittenden
parent 0b60408dab
commit 47d49aa7b7
3 changed files with 37 additions and 22 deletions

View File

@ -57,6 +57,7 @@ static void ipadb_context_free(krb5_context kcontext,
/* ldap free lcontext */
if ((*ctx)->lcontext) {
ldap_unbind_ext_s((*ctx)->lcontext, NULL, NULL);
(*ctx)->lcontext = NULL;
}
free((*ctx)->supp_encs);
free((*ctx)->def_encs);

View File

@ -418,7 +418,6 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
krb5_timestamp authtime,
struct netr_SamInfo3 *info3)
{
LDAP *lcontext = ipactx->lcontext;
LDAPDerefRes *deref_results = NULL;
struct dom_sid sid;
gid_t prigid = -1;
@ -435,7 +434,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
bool is_idobject = false;
krb5_principal princ;
ret = ipadb_ldap_attr_to_strlist(lcontext, lentry, "objectClass",
ret = ipadb_ldap_attr_to_strlist(ipactx->lcontext, lentry, "objectClass",
&objectclasses);
if (ret == 0 && objectclasses != NULL) {
for (c = 0; objectclasses[c] != NULL; c++) {
@ -472,13 +471,14 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
}
if (is_host) {
ret = ipadb_ldap_attr_to_str(lcontext, lentry, "fqdn", &strres);
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "fqdn", &strres);
if (ret) {
/* fqdn is mandatory for hosts */
return ret;
}
} else if (is_service) {
ret = ipadb_ldap_attr_to_str(lcontext, lentry, "krbCanonicalName", &strres);
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"krbCanonicalName", &strres);
if (ret) {
/* krbCanonicalName is mandatory for services */
return ret;
@ -498,7 +498,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ENOENT;
}
} else {
ret = ipadb_ldap_attr_to_str(lcontext, lentry, "uid", &strres);
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "uid", &strres);
if (ret) {
/* uid is mandatory */
return ret;
@ -511,7 +511,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
if (is_host || is_service) {
prigid = 515; /* Well known RID for domain computers group */
} else {
ret = ipadb_ldap_attr_to_int(lcontext, lentry, "gidNumber", &intres);
ret = ipadb_ldap_attr_to_int(ipactx->lcontext, lentry,
"gidNumber", &intres);
if (ret) {
/* gidNumber is mandatory */
return ret;
@ -544,7 +545,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
info3->base.kickoff_time = INT64_MAX;
#endif
ret = ipadb_ldap_attr_to_time_t(lcontext, lentry,
ret = ipadb_ldap_attr_to_time_t(ipactx->lcontext, lentry,
"krbLastPwdChange", &timeres);
switch (ret) {
case 0:
@ -562,7 +563,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
info3->base.allow_password_change = info3->base.last_password_change;
info3->base.force_password_change = INT64_MAX;
ret = ipadb_ldap_attr_to_str(lcontext, lentry, "cn", &strres);
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "cn", &strres);
switch (ret) {
case 0:
info3->base.full_name.string = talloc_strdup(memctx, strres);
@ -575,7 +576,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ret;
}
ret = ipadb_ldap_attr_to_str(lcontext, lentry,
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTLogonScript", &strres);
switch (ret) {
case 0:
@ -589,7 +590,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ret;
}
ret = ipadb_ldap_attr_to_str(lcontext, lentry,
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTProfilePath", &strres);
switch (ret) {
case 0:
@ -603,7 +604,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ret;
}
ret = ipadb_ldap_attr_to_str(lcontext, lentry,
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTHomeDirectory", &strres);
switch (ret) {
case 0:
@ -617,7 +618,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ret;
}
ret = ipadb_ldap_attr_to_str(lcontext, lentry,
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTHomeDirectoryDrive", &strres);
switch (ret) {
case 0:
@ -648,7 +649,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
info3->base.rid = 515;
}
} else {
ret = ipadb_ldap_attr_to_str(lcontext, lentry,
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTSecurityIdentifier", &strres);
if (ret) {
/* SID is mandatory */
@ -665,7 +666,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
}
}
ret = ipadb_ldap_deref_results(lcontext, lentry, &deref_results);
ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, &deref_results);
switch (ret) {
LDAPDerefRes *dres;
LDAPDerefVal *dval;
@ -2511,7 +2512,7 @@ static void ipadb_free_sid_blacklists(char ***sid_blocklist_incoming, char ***si
krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
{
struct ipadb_adtrusts *t;
LDAP *lc = ipactx->lcontext;
LDAP *lc = NULL;
char *attrs[] = { "cn", "ipaNTTrustPartner", "ipaNTFlatName",
"ipaNTTrustedDomainSID", "ipaNTSIDBlacklistIncoming",
"ipaNTSIDBlacklistOutgoing", "ipaNTAdditionalSuffixes", NULL };
@ -2545,6 +2546,7 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
goto done;
}
lc = ipactx->lcontext;
for (le = ldap_first_entry(lc, res); le; le = ldap_next_entry(lc, le)) {
dnstr = ldap_get_dn(lc, le);

View File

@ -333,6 +333,11 @@ static enum ipadb_user_auth ipadb_get_user_auth(struct ipadb_context *ipactx,
if (gcfg != NULL)
gua = gcfg->user_auth;
/* lcontext == NULL means ipadb_get_global_config() failed to load
* global config and cleared the ipactx */
if (ipactx->lcontext == NULL)
return IPADB_USER_AUTH_NONE;
/* Get the user's user_auth settings if not disabled. */
if ((gua & IPADB_USER_AUTH_DISABLED) == 0)
ipadb_parse_user_auth(ipactx->lcontext, lentry, &ua);
@ -607,8 +612,16 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
free(entry);
return KRB5_KDB_DBNOTINITED;
}
lcontext = ipactx->lcontext;
if (!lcontext) {
entry->magic = KRB5_KDB_MAGIC_NUMBER;
entry->len = KRB5_KDB_V1_BASE_LENGTH;
/* Get User Auth configuration. */
ua = ipadb_get_user_auth(ipactx, lentry);
/* ipadb_get_user_auth() calls into ipadb_get_global_config()
* and that might fail, causing lcontext to become NULL */
if (!ipactx->lcontext) {
krb5_klog_syslog(LOG_INFO,
"No LDAP connection in ipadb_parse_ldap_entry(); retrying...\n");
ret = ipadb_get_connection(ipactx);
@ -620,11 +633,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
}
}
entry->magic = KRB5_KDB_MAGIC_NUMBER;
entry->len = KRB5_KDB_V1_BASE_LENGTH;
/* Get User Auth configuration. */
ua = ipadb_get_user_auth(ipactx, lentry);
/* If any code below would result in invalidating ipactx->lcontext,
* lcontext must be updated with the new ipactx->lcontext value.
* We rely on the fact that none of LDAP-parsing helpers does it. */
lcontext = ipactx->lcontext;
/* ignore mask for now */