8cffd2
From 022c2a55ec9578867b331e419fad547092d94192 Mon Sep 17 00:00:00 2001
8cffd2
From: Alexander Bokovoy <abokovoy@redhat.com>
8cffd2
Date: Fri, 19 Feb 2021 15:37:47 +0200
8cffd2
Subject: [PATCH] ipa-kdb: do not use OpenLDAP functions with NULL LDAP context
8cffd2
8cffd2
Calling to ipadb_get_connection() will remove LDAP context if any error
8cffd2
happens. This means upper layers must always verify that LDAP context
8cffd2
exists after such calls.
8cffd2
8cffd2
ipadb_get_user_auth() may re-read global configuration and that may fail
8cffd2
and cause IPA context to have NULL LDAP context.
8cffd2
8cffd2
Fixes: https://pagure.io/freeipa/issue/8681
8cffd2
8cffd2
Signed-off-by: Alexander Bokovoy <abokovoy@redhat.com>
8cffd2
Reviewed-By: Robbie Harwood <rharwood@redhat.com>
8cffd2
Reviewed-By: Rob Crittenden <rcritten@redhat.com>
8cffd2
Reviewed-By: Florence Blanc-Renaud <frenaud@redhat.com>
8cffd2
---
8cffd2
 daemons/ipa-kdb/ipa_kdb.c            |  1 +
8cffd2
 daemons/ipa-kdb/ipa_kdb_mspac.c      | 32 +++++++++++++++-------------
8cffd2
 daemons/ipa-kdb/ipa_kdb_principals.c | 26 ++++++++++++++++------
8cffd2
 3 files changed, 37 insertions(+), 22 deletions(-)
8cffd2
8cffd2
diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c
8cffd2
index 0dcc74263263423da6b1f4d8441ee149bce24c58..eb18a037e41bd778b3f522644acf8d793ddf70e7 100644
8cffd2
--- a/daemons/ipa-kdb/ipa_kdb.c
8cffd2
+++ b/daemons/ipa-kdb/ipa_kdb.c
8cffd2
@@ -56,6 +56,7 @@ static void ipadb_context_free(krb5_context kcontext,
8cffd2
         /* ldap free lcontext */
8cffd2
         if ((*ctx)->lcontext) {
8cffd2
             ldap_unbind_ext_s((*ctx)->lcontext, NULL, NULL);
8cffd2
+            (*ctx)->lcontext = NULL;
8cffd2
         }
8cffd2
         free((*ctx)->supp_encs);
8cffd2
         free((*ctx)->def_encs);
8cffd2
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
8cffd2
index b6e7516859ce59232364f6ae93dee3063914ecf4..b231844250b8c65337bf4e7523d6a800dfe0767d 100644
8cffd2
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
8cffd2
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
8cffd2
@@ -386,7 +386,6 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
                                         TALLOC_CTX *memctx,
8cffd2
                                         struct netr_SamInfo3 *info3)
8cffd2
 {
8cffd2
-    LDAP *lcontext = ipactx->lcontext;
8cffd2
     LDAPDerefRes *deref_results = NULL;
8cffd2
     struct dom_sid sid;
8cffd2
     gid_t prigid = -1;
8cffd2
@@ -403,7 +402,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
     krb5_principal princ;
8cffd2
     krb5_data *data;
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_strlist(lcontext, lentry, "objectClass",
8cffd2
+    ret = ipadb_ldap_attr_to_strlist(ipactx->lcontext, lentry, "objectClass",
8cffd2
                                      &objectclasses);
8cffd2
     if (ret == 0 && objectclasses != NULL) {
8cffd2
         for (c = 0; objectclasses[c] != NULL; c++) {
8cffd2
@@ -427,7 +426,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
     }
8cffd2
 
8cffd2
     if (is_host) {
8cffd2
-        ret = ipadb_ldap_attr_to_str(lcontext, lentry, "fqdn", &strres);
8cffd2
+        ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "fqdn", &strres);
8cffd2
         if (ret) {
8cffd2
             /* fqdn is mandatory for hosts */
8cffd2
             return ret;
8cffd2
@@ -440,7 +439,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
             return ENOENT;
8cffd2
         }
8cffd2
     } else if (is_service) {
8cffd2
-        ret = ipadb_ldap_attr_to_str(lcontext, lentry, "krbPrincipalName", &strres);
8cffd2
+        ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
+                                     "krbCanonicalName", &strres);
8cffd2
         if (ret) {
8cffd2
             /* krbPrincipalName is mandatory for services */
8cffd2
             return ret;
8cffd2
@@ -489,7 +489,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
             return ENOENT;
8cffd2
         }
8cffd2
     } else {
8cffd2
-        ret = ipadb_ldap_attr_to_str(lcontext, lentry, "uid", &strres);
8cffd2
+        ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "uid", &strres);
8cffd2
         if (ret) {
8cffd2
             /* uid is mandatory */
8cffd2
             return ret;
8cffd2
@@ -502,7 +502,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
     if (is_host || is_service) {
8cffd2
         prigid = 515; /* Well known RID for domain computers group */
8cffd2
     } else {
8cffd2
-        ret = ipadb_ldap_attr_to_int(lcontext, lentry, "gidNumber", &intres);
8cffd2
+        ret = ipadb_ldap_attr_to_int(ipactx->lcontext, lentry,
8cffd2
+                                     "gidNumber", &intres);
8cffd2
         if (ret) {
8cffd2
             /* gidNumber is mandatory */
8cffd2
             return ret;
8cffd2
@@ -533,7 +534,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
     info3->base.kickoff_time = -1;
8cffd2
 #endif
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_time_t(lcontext, lentry,
8cffd2
+    ret = ipadb_ldap_attr_to_time_t(ipactx->lcontext, lentry,
8cffd2
                                     "krbLastPwdChange", &timeres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
@@ -550,7 +551,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
     info3->base.allow_password_change = 0;
8cffd2
     info3->base.force_password_change = -1;
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_str(lcontext, lentry, "cn", &strres);
8cffd2
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, "cn", &strres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
         info3->base.full_name.string = talloc_strdup(memctx, strres);
8cffd2
@@ -563,7 +564,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         return ret;
8cffd2
     }
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_str(lcontext, lentry,
8cffd2
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
                                  "ipaNTLogonScript", &strres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
@@ -577,7 +578,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         return ret;
8cffd2
     }
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_str(lcontext, lentry,
8cffd2
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
                                  "ipaNTProfilePath", &strres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
@@ -591,7 +592,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         return ret;
8cffd2
     }
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_str(lcontext, lentry,
8cffd2
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
                                  "ipaNTHomeDirectory", &strres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
@@ -605,7 +606,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         return ret;
8cffd2
     }
8cffd2
 
8cffd2
-    ret = ipadb_ldap_attr_to_str(lcontext, lentry,
8cffd2
+    ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
                                  "ipaNTHomeDirectoryDrive", &strres);
8cffd2
     switch (ret) {
8cffd2
     case 0:
8cffd2
@@ -626,7 +627,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         /* Well know RID of domain controllers group */
8cffd2
         info3->base.rid = 516;
8cffd2
     } else {
8cffd2
-        ret = ipadb_ldap_attr_to_str(lcontext, lentry,
8cffd2
+        ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
8cffd2
                                      "ipaNTSecurityIdentifier", &strres);
8cffd2
         if (ret) {
8cffd2
             /* SID is mandatory */
8cffd2
@@ -643,7 +644,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
8cffd2
         }
8cffd2
     }
8cffd2
 
8cffd2
-    ret = ipadb_ldap_deref_results(lcontext, lentry, &deref_results);
8cffd2
+    ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, &deref_results);
8cffd2
     switch (ret) {
8cffd2
     LDAPDerefRes *dres;
8cffd2
     LDAPDerefVal *dval;
8cffd2
@@ -2433,7 +2434,7 @@ static krb5_error_code
8cffd2
 ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
8cffd2
 {
8cffd2
     struct ipadb_adtrusts *t;
8cffd2
-    LDAP *lc = ipactx->lcontext;
8cffd2
+    LDAP *lc = NULL;
8cffd2
     char *attrs[] = { "cn", "ipaNTTrustPartner", "ipaNTFlatName",
8cffd2
                       "ipaNTTrustedDomainSID", "ipaNTSIDBlacklistIncoming",
8cffd2
                       "ipaNTSIDBlacklistOutgoing", "ipaNTAdditionalSuffixes", NULL };
8cffd2
@@ -2467,6 +2468,7 @@ ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
8cffd2
         goto done;
8cffd2
     }
8cffd2
 
8cffd2
+    lc = ipactx->lcontext;
8cffd2
     for (le = ldap_first_entry(lc, res); le; le = ldap_next_entry(lc, le)) {
8cffd2
         dnstr = ldap_get_dn(lc, le);
8cffd2
 
8cffd2
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c
8cffd2
index 3b78970b118cb6d8bd97da82a79226d20ea784e1..e1e86a6102886051cdafa3d885bf75c2718b8575 100644
8cffd2
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
8cffd2
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
8cffd2
@@ -341,6 +341,11 @@ static enum ipadb_user_auth ipadb_get_user_auth(struct ipadb_context *ipactx,
8cffd2
     if (gcfg != NULL)
8cffd2
         gua = gcfg->user_auth;
8cffd2
 
8cffd2
+    /* lcontext == NULL means ipadb_get_global_config() failed to load
8cffd2
+     * global config and cleared the ipactx */
8cffd2
+    if (ipactx->lcontext == NULL)
8cffd2
+        return IPADB_USER_AUTH_NONE;
8cffd2
+
8cffd2
     /* Get the user's user_auth settings if not disabled. */
8cffd2
     if ((gua & IPADB_USER_AUTH_DISABLED) == 0)
8cffd2
         ipadb_parse_user_auth(ipactx->lcontext, lentry, &ua);
8cffd2
@@ -555,8 +560,16 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
8cffd2
         free(entry);
8cffd2
         return KRB5_KDB_DBNOTINITED;
8cffd2
     }
8cffd2
-    lcontext = ipactx->lcontext;
8cffd2
-    if (!lcontext) {
8cffd2
+
8cffd2
+    entry->magic = KRB5_KDB_MAGIC_NUMBER;
8cffd2
+    entry->len = KRB5_KDB_V1_BASE_LENGTH;
8cffd2
+
8cffd2
+    /* Get User Auth configuration. */
8cffd2
+    ua = ipadb_get_user_auth(ipactx, lentry);
8cffd2
+
8cffd2
+    /* ipadb_get_user_auth() calls into ipadb_get_global_config()
8cffd2
+     * and that might fail, causing lcontext to become NULL */
8cffd2
+    if (!ipactx->lcontext) {
8cffd2
         krb5_klog_syslog(LOG_INFO,
8cffd2
                          "No LDAP connection in ipadb_parse_ldap_entry(); retrying...\n");
8cffd2
         ret = ipadb_get_connection(ipactx);
8cffd2
@@ -568,11 +581,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext,
8cffd2
         }
8cffd2
     }
8cffd2
 
8cffd2
-    entry->magic = KRB5_KDB_MAGIC_NUMBER;
8cffd2
-    entry->len = KRB5_KDB_V1_BASE_LENGTH;
8cffd2
-
8cffd2
-    /* Get User Auth configuration. */
8cffd2
-    ua = ipadb_get_user_auth(ipactx, lentry);
8cffd2
+    /* If any code below would result in invalidating ipactx->lcontext,
8cffd2
+     * lcontext must be updated with the new ipactx->lcontext value.
8cffd2
+     * We rely on the fact that none of LDAP-parsing helpers does it. */
8cffd2
+    lcontext = ipactx->lcontext;
8cffd2
 
8cffd2
     /* ignore mask for now */
8cffd2
 
8cffd2
-- 
8cffd2
2.26.3
8cffd2