From 022c2a55ec9578867b331e419fad547092d94192 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy@redhat.com>
Date: Fri, 19 Feb 2021 15:37:47 +0200
Subject: [PATCH] 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>
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
---
daemons/ipa-kdb/ipa_kdb.c | 1 +
daemons/ipa-kdb/ipa_kdb_mspac.c | 32 +++++++++++++++-------------
daemons/ipa-kdb/ipa_kdb_principals.c | 26 ++++++++++++++++------
3 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
index 0dcc74263263423da6b1f4d8441ee149bce24c58..eb18a037e41bd778b3f522644acf8d793ddf70e7 100644
--- a/daemons/ipa-kdb/ipa_kdb.c
+++ b/daemons/ipa-kdb/ipa_kdb.c
@@ -56,6 +56,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);
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index b6e7516859ce59232364f6ae93dee3063914ecf4..b231844250b8c65337bf4e7523d6a800dfe0767d 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -386,7 +386,6 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
TALLOC_CTX *memctx,
struct netr_SamInfo3 *info3)
{
- LDAP *lcontext = ipactx->lcontext;
LDAPDerefRes *deref_results = NULL;
struct dom_sid sid;
gid_t prigid = -1;
@@ -403,7 +402,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
krb5_principal princ;
krb5_data *data;
- 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++) {
@@ -427,7 +426,7 @@ 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;
@@ -440,7 +439,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ENOENT;
}
} else if (is_service) {
- ret = ipadb_ldap_attr_to_str(lcontext, lentry, "krbPrincipalName", &strres);
+ ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
+ "krbCanonicalName", &strres);
if (ret) {
/* krbPrincipalName is mandatory for services */
return ret;
@@ -489,7 +489,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;
@@ -502,7 +502,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;
@@ -533,7 +534,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
info3->base.kickoff_time = -1;
#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:
@@ -550,7 +551,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
info3->base.allow_password_change = 0;
info3->base.force_password_change = -1;
- 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);
@@ -563,7 +564,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:
@@ -577,7 +578,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:
@@ -591,7 +592,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:
@@ -605,7 +606,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:
@@ -626,7 +627,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
/* Well know RID of domain controllers group */
info3->base.rid = 516;
} 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 */
@@ -643,7 +644,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;
@@ -2433,7 +2434,7 @@ static 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 };
@@ -2467,6 +2468,7 @@ 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);
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
index 3b78970b118cb6d8bd97da82a79226d20ea784e1..e1e86a6102886051cdafa3d885bf75c2718b8575 100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -341,6 +341,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);
@@ -555,8 +560,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);
@@ -568,11 +581,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 */
--
2.26.3