Hello,

On 18.2.2015 10:36, Tomas Hozza wrote:
> On 12/16/2014 04:32 PM, Petr Spacek wrote:
>> Hello,
>>
>> Fix crash triggered by zone objects with unexpected DN.
>>
>> https://fedorahosted.org/bind-dyndb-ldap/ticket/148
>>
> NACK.
> 
> The patch seems to make no difference when using the reproducer from ticket 
> 148
> 
> 18-Feb-2015 10:34:09.067 running
> 18-Feb-2015 10:34:09.139 ldap_helper.c:4876: INSIST(task == inst->task) 
> failed, back trace
> 18-Feb-2015 10:34:09.139 #0 0x555555587a80 in ??
> 18-Feb-2015 10:34:09.139 #1 0x7ffff620781a in ??
> 18-Feb-2015 10:34:09.139 #2 0x7ffff20b00b2 in ??
> 18-Feb-2015 10:34:09.140 #3 0x7ffff1e7ccf9 in ??
> 18-Feb-2015 10:34:09.140 #4 0x7ffff1e7d992 in ??
> 18-Feb-2015 10:34:09.140 #5 0x7ffff20a7f3b in ??
> 18-Feb-2015 10:34:09.140 #6 0x7ffff5dda52a in ??
> 18-Feb-2015 10:34:09.140 #7 0x7ffff508d79d in ??
> 18-Feb-2015 10:34:09.140 exiting (due to assertion failure)
> 
> Program received signal SIGABRT, Aborted.
> [Switching to Thread 0x7fffea7cd700 (LWP 1719)]
> 0x00007ffff4fc18c7 in __GI_raise (sig=sig@entry=6) at 
> ../sysdeps/unix/sysv/linux/raise.c:55
> 55      return INLINE_SYSCALL (tgkill, 3, pid, selftid, sig);
> Missing separate debuginfos, use: debuginfo-install 
> cyrus-sasl-gssapi-2.1.26-19.fc21.x86_64 cyrus-sasl-lib-2.1.26-19.fc21.x86_64 
> cyrus-sasl-md5-2.1.26-19.fc21.x86_64 cyrus-sasl-plain-2.1.26-19.fc21.x86_64 
> gssproxy-0.3.1-4.fc21.x86_64 keyutils-libs-1.5.9-4.fc21.x86_64 
> libattr-2.4.47-9.fc21.x86_64 libdb-5.3.28-9.fc21.x86_64 
> libgcc-4.9.2-6.fc21.x86_64 libselinux-2.3-5.fc21.x86_64 
> nspr-4.10.8-1.fc21.x86_64 nss-3.17.4-1.fc21.x86_64 
> nss-softokn-freebl-3.17.4-1.fc21.x86_64 nss-util-3.17.4-1.fc21.x86_64 
> pcre-8.35-8.fc21.x86_64 sssd-client-1.12.3-4.fc21.x86_64 
> xz-libs-5.1.2-14alpha.fc21.x86_64
> (gdb) bt
> #0  0x00007ffff4fc18c7 in __GI_raise (sig=sig@entry=6) at 
> ../sysdeps/unix/sysv/linux/raise.c:55
> #1  0x00007ffff4fc352a in __GI_abort () at abort.c:89
> #2  0x0000555555587c29 in assertion_failed (file=<optimized out>, 
> line=<optimized out>, type=<optimized out>, cond=<optimized out>) at 
> ./main.c:220
> #3  0x00007ffff620781a in isc_assertion_failed 
> (file=file@entry=0x7ffff20bad2a "ldap_helper.c", line=line@entry=4876, 
> type=type@entry=isc_assertiontype_insist,
>     cond=cond@entry=0x7ffff20baf04 "task == inst->task") at assertions.c:57
> #4  0x00007ffff20b00b2 in syncrepl_update (chgtype=1, entry=0x7ffff0125590, 
> inst=0x7ffff7fa3160) at ldap_helper.c:4876
> #5  ldap_sync_search_entry (ls=<optimized out>, msg=<optimized out>, 
> entryUUID=<optimized out>, phase=LDAP_SYNC_CAPI_ADD) at ldap_helper.c:5031
> #6  0x00007ffff1e7ccf9 in ldap_sync_search_entry (ls=ls@entry=0x7fffe40008c0, 
> res=0x7fffe4003870) at ldap_sync.c:228
> #7  0x00007ffff1e7d992 in ldap_sync_init (ls=0x7fffe40008c0, 
> mode=mode@entry=3) at ldap_sync.c:792
> #8  0x00007ffff20a7f3b in ldap_syncrepl_watcher (arg=0x7ffff7fa3160) at 
> ldap_helper.c:5247
> #9  0x00007ffff5dda52a in start_thread (arg=0x7fffea7cd700) at 
> pthread_create.c:310
> #10 0x00007ffff508d79d in clone () at 
> ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Thank you for catching this! I was using slightly different test which
triggered the new code but by using different code path.

This new version should be more robust. Please re-test it, thank you!

-- 
Petr^2 Spacek
From bfe9f7767ed86cdd562b36b801ce3ee6e6f2063b Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 24 Feb 2015 14:45:42 +0100
Subject: [PATCH] Fix crash triggered by zone objects with unexpected DN.

https://fedorahosted.org/bind-dyndb-ldap/ticket/148
---
 src/ldap_convert.c | 24 ++++++++++++++++++++++++
 src/ldap_convert.h |  4 ++++
 src/ldap_helper.c  |  3 +++
 3 files changed, 31 insertions(+)

diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index b51d402492415d6630a42435b823925c8246a06f..dde10d6e989159e9f6f5086a4a12bbd165b73646 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -193,6 +193,30 @@ cleanup:
 }
 
 /**
+ * Evaluate if DN has/does not have expected format with one or two components
+ * and error out if a mismatch is detected.
+ *
+ * @param[in] prefix      Prefix for error messages, usually a function name.
+ * @param[in] dn
+ * @param[in] dniszone    Boolean returned by dn_to_dnsname for given DN.
+ * @param[in] classiszone ISC_TRUE if DN should be a zone, ISC_FALSE otherwise.
+ * @retval ISC_R_SUCCESS or ISC_R_UNEXPECTED if values do not match.
+ */
+isc_result_t
+dn_want_zone(const char * const prefix, const char * const dn,
+	     isc_boolean_t dniszone, isc_boolean_t classiszone) {
+	if (dniszone != classiszone) {
+		log_error("%s: object '%s' does%s have a zone object class "
+			  "but DN format suggests that it is%s a zone",
+			  prefix, dn, classiszone ? "" : " not",
+			  dniszone ? "" : " not");
+		return ISC_R_UNEXPECTED;
+	}
+
+	return ISC_R_SUCCESS;
+}
+
+/**
  * WARNING! This function is used to mangle input from network
  *          and it is security sensitive.
  *
diff --git a/src/ldap_convert.h b/src/ldap_convert.h
index f0b09262dbbe588c5c12b074242a9f7db4361880..daa0db0b311dd51c05ade688715550d9f70759ac 100644
--- a/src/ldap_convert.h
+++ b/src/ldap_convert.h
@@ -42,6 +42,10 @@ isc_result_t dn_to_dnsname(isc_mem_t *mctx, const char *dn,
 			   isc_boolean_t *iszone)
 			   ATTR_NONNULL(1, 2, 3) ATTR_CHECKRESULT;
 
+isc_result_t dn_want_zone(const char * const prefix, const char * const dn,
+			  isc_boolean_t dniszone, isc_boolean_t classiszone)
+			  ATTR_NONNULLS ATTR_CHECKRESULT;
+
 isc_result_t dnsname_to_dn(zone_register_t *zr, dns_name_t *name, dns_name_t *zone,
 			   ld_string_t *target) ATTR_NONNULLS ATTR_CHECKRESULT;
 
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 9427dfbee800bacf3960c623ede2a10a7bc988cb..42efc8c0889e60636a1f7bed193b1b45eb279907 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -4856,6 +4856,7 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 	CHECK(ldap_dn_compare(ldap_base, entry->dn, &isbase));
 	if (isbase == ISC_TRUE) {
 		class = LDAP_ENTRYCLASS_CONFIG;
+		iszone = ISC_FALSE;
 	} else {
 		CHECK(dn_to_dnsname(inst->mctx, dn, &entry_name, &entry_origin,
 				    &iszone));
@@ -4925,6 +4926,8 @@ syncrepl_update(ldap_instance_t *inst, ldap_entry_t *entry, int chgtype)
 		result = ISC_R_NOTIMPLEMENTED;
 		goto cleanup;
 	}
+	CHECK(dn_want_zone(__func__, dn, iszone,
+			   ISC_TF(class & (LDAP_ENTRYCLASS_MASTER | LDAP_ENTRYCLASS_FORWARD))));
 
 	/* All events for single zone are handled by one task, so we don't
 	 * need to spend time with normal records. */
-- 
2.1.0

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to