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