On 05/09/2012 02:17 PM, Adam Tkac wrote:
On 05/09/2012 02:11 PM, Petr Spacek wrote:
On 05/09/2012 01:24 PM, Adam Tkac wrote:
On 05/03/2012 03:46 PM, Petr Spacek wrote:
On 05/03/2012 11:25 AM, Petr Spacek wrote:
Hello,

this patch adds missing DNS->LDAP escaping conversion. It's necessary to
prevent (potential) LDAP injection attacks in future.

Code isn't very nice, because DNS users decimal escaping \123, LDAP uses
hexadecimal escaping \ab and set of escaped characters is smaller in DNS
than
in LDAP.

Any improvements are welcome.

Petr^2 Spacek

Here is second version of the patch.

Changes:
- comments
- order of [._-] in if()
- function was renamed to dns_to_ldap_dn_escape()

Escaping logic itself wasn't changed.

Hello Peter,

please check my comments inside the patch.
Oh, I feel so ashamed. All errors were corrected, see attachment.

Petr^2 Spacek
Ack, please push it to master.
Pushed with minor change as discussed on IRC: log_error() was substituted by REQUIRE().

https://fedorahosted.org/bind-dyndb-ldap/changeset/3d43fd66aa68ef275855391a94e47e9d2f30309d

Petr^2 Spacek



Regards, Adam


Petr^2 Spacek

bind-dyndb-ldap-pspacek-0019-2-Add-proper-DN-escaping-before-LDAP-library-calls.patch


bind-dyndb-ldap-pspacek-0019-3-Add-proper-DN-escaping-before-LDAP-library-calls.patch


From 3000d2165d3eae332476d223a286f620c5be08bc Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 23 Apr 2012 11:38:43 +0200
Subject: [PATCH] Add proper DN escaping before LDAP library calls.
 Signed-off-by: Petr Spacek <pspa...@redhat.com>

---
 src/ldap_convert.c  |  105 ++++++++++++++++++++++++++++++++++++++++++++++++--
 src/zone_register.c |    7 +++
 src/zone_register.h |    3 +
 3 files changed, 110 insertions(+), 5 deletions(-)

diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index 6405a98fda942cbba10b4c862351c4e3695aba85..6b4e321ea6b14128fcfdfef91fb400a222ee2211 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -21,6 +21,7 @@
 #include <isc/buffer.h>
 #include <isc/mem.h>
 #include <isc/util.h>
+#include <isc/string.h>
 
 #include <dns/name.h>
 #include <dns/rdatatype.h>
@@ -32,6 +33,7 @@
 
 #include <errno.h>
 #include <strings.h>
+#include <ctype.h>
 
 #include "str.h"
 #include "ldap_convert.h"
@@ -189,6 +191,92 @@ cleanup:
 	return result;
 }
 
+/**
+ * Convert a string from DNS escaping to LDAP escaping.
+ * The Input string dns_str is expected to be the result of dns_name_tostring().
+ * The DNS label can contain any binary data as described in
+ * http://tools.ietf.org/html/rfc2181#section-11 .
+ *
+ * DNS escaping uses form   "\123" = ASCII value 123 (decimal)
+ * LDAP escaping users form "\7b"  = ASCII value 7b (hexadecimal)
+ *
+ * Input (DNS escaped) example  : _aaa,bbb\255\000ccc.555.ddd-eee
+ * Output (LDAP escaped) example: _aaa\2cbbb\ff\00ccc.555.ddd-eee
+ *
+ * The DNS to text functions from ISC libraries do not convert certain
+ * characters (e.g. ","). This function converts \123 form to \7b form in all
+ * cases. Other characters (not escaped by ISC libraries) will be additionally
+ * converted to the LDAP escape form.
+ * Input characters [a-zA-Z0-9._-] are left in raw ASCII form.
+ *
+ * If dns_str consists only of the characters in the [a-zA-Z0-9._-] set, it
+ * will be checked & copied to the output buffer, without any additional escaping.
+ */
+isc_result_t
+dns_to_ldap_dn_escape(isc_mem_t *mctx, const char const * dns_str, char ** ldap_name) {
+	isc_result_t result = ISC_R_FAILURE;
+	char * esc_name = NULL;
+	int idx_symb_first = -1; /* index of first "nice" printable symbol in dns_str */
+	int dns_idx = 0;
+	int esc_idx = 0;
+
+	REQUIRE(dns_str != NULL);
+	REQUIRE(ldap_name != NULL && *ldap_name == NULL);
+
+	int dns_str_len = strlen(dns_str);
+
+	/**
+	 * In worst case each symbol from DNS dns_str will be represented
+	 * as "\xy" in ldap_name. (xy are hexadecimal digits)
+	 */
+	CHECKED_MEM_ALLOCATE(mctx, *ldap_name, 3 * dns_str_len + 1);
+	esc_name = *ldap_name;
+
+	for (dns_idx = 0; dns_idx < dns_str_len; dns_idx++) {
+		if (isalnum(dns_str[dns_idx]) || dns_str[dns_idx] == '.'
+				|| dns_str[dns_idx] == '-' || dns_str[dns_idx] == '_' ) {
+			if (idx_symb_first == -1)
+				idx_symb_first = dns_idx;
+			continue;
+		} else { /* some not very nice symbols */
+			int ascii_val;
+			if (idx_symb_first != -1) { /* copy previous nice part */
+				int length_ok = dns_idx - idx_symb_first;
+				memcpy(esc_name + esc_idx, dns_str + idx_symb_first, length_ok);
+				esc_idx += length_ok;
+				idx_symb_first = -1;
+			}
+			if (dns_str[dns_idx] != '\\') { /* not nice raw value, e.g. ',' */
+				ascii_val = dns_str[dns_idx];
+			} else { /* not nice value in DNS \123 decimal format */
+				/* check if input length <= expected size */
+				REQUIRE (dns_str_len > dns_idx + 3); /* this problem should never happen */
+				ascii_val = 100 * (dns_str[dns_idx + 1] - '0')
+						+ 10 * (dns_str[dns_idx + 2] - '0')
+						+ (dns_str[dns_idx + 3] - '0');
+				dns_idx += 3;
+			}
+			/* LDAP uses \xy escaping. "xy" represent two hexadecimal digits.*/
+			/* TODO: optimize to bit mask & rotate & dec->hex table? */
+			CHECK(isc_string_printf(esc_name + esc_idx, 4, "\\%02x", ascii_val));
+			esc_idx += 3; /* isc_string_printf wrote 4 bytes including '\0' */
+		}
+	}
+	if (idx_symb_first != -1) { /* copy last nice part */
+		int length_ok = dns_idx - idx_symb_first;
+		memcpy(esc_name + esc_idx, dns_str + idx_symb_first, dns_idx - idx_symb_first);
+		esc_idx += length_ok;
+		idx_symb_first = -1;
+	}
+	esc_name[esc_idx] = '\0';
+	return ISC_R_SUCCESS;
+
+cleanup:
+	if (*ldap_name)
+		isc_mem_free(mctx, *ldap_name);
+	return result;
+}
+
 static isc_result_t
 explode_dn(const char *dn, char ***explodedp, int notypes)
 {
@@ -243,11 +331,15 @@ dnsname_to_dn(zone_register_t *zr, dns_name_t *name, ld_string_t *target)
 	isc_result_t result;
 	int label_count;
 	const char *zone_dn = NULL;
+	char *dns_str = NULL;
+	char *escaped_name = NULL;
 
 	REQUIRE(zr != NULL);
 	REQUIRE(name != NULL);
 	REQUIRE(target != NULL);
 
+	isc_mem_t * mctx = zr_get_mctx(zr);
+
 	/* Find the DN of the zone we belong to. */
 	{
 		DECLARE_BUFFERED_NAME(zone);
@@ -264,26 +356,28 @@ dnsname_to_dn(zone_register_t *zr, dns_name_t *name, ld_string_t *target)
 
 	str_clear(target);
 	if (label_count > 0) {
-		DECLARE_BUFFER(buffer, DNS_NAME_MAXTEXT);
 		dns_name_t labels;
 
-		INIT_BUFFER(buffer);
 		dns_name_init(&labels, NULL);
-
 		dns_name_getlabelsequence(name, 0, label_count, &labels);
-		CHECK(dns_name_totext(&labels, ISC_TRUE, &buffer));
+		CHECK(dns_name_tostring(&labels, &dns_str, mctx));
 
+		CHECK(dns_to_ldap_dn_escape(mctx, dns_str, &escaped_name));
 		CHECK(str_cat_char(target, "idnsName="));
-		CHECK(str_cat_isc_buffer(target, &buffer));
+		CHECK(str_cat_char(target, escaped_name));
 		/* 
 		 * Modification of following line can affect modify_ldap_common().
 		 * See line with: char *zone_dn = strstr(str_buf(owner_dn),", ") + 1;  
 		 */
 		CHECK(str_cat_char(target, ", "));
 	}
 	CHECK(str_cat_char(target, zone_dn));
 
 cleanup:
+	if (dns_str)
+		isc_mem_free(mctx, dns_str);
+	if (escaped_name)
+		isc_mem_free(mctx, escaped_name);
 	return result;
 }
 
@@ -328,3 +422,4 @@ rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, const char **target)
 
 	return ISC_R_SUCCESS;
 }
+
diff --git a/src/zone_register.c b/src/zone_register.c
index fc6dc076ac91ae2f21ecf934d0d6c837f1581766..81d208fc6e3c0dba6efb72ae10db54a79c336eca 100644
--- a/src/zone_register.c
+++ b/src/zone_register.c
@@ -61,6 +61,13 @@ zr_get_rbt(zone_register_t *zr)
 	return zr->rbt;
 }
 
+isc_mem_t *
+zr_get_mctx(zone_register_t *zr) {
+	REQUIRE(zr);
+
+	return zr->mctx;
+}
+
 /*
  * Create a new zone register.
  */
diff --git a/src/zone_register.h b/src/zone_register.h
index e2408cbf8630effc0036c3765535a84381f83117..fa8ef255ef9cf212bca04aaafba0e6450d40a5c6 100644
--- a/src/zone_register.h
+++ b/src/zone_register.h
@@ -45,4 +45,7 @@ zr_get_zone_ptr(zone_register_t *zr, dns_name_t *name, dns_zone_t **zonep);
 dns_rbt_t *
 zr_get_rbt(zone_register_t *zr);
 
+isc_mem_t *
+zr_get_mctx(zone_register_t *zr);
+
 #endif /* !_LD_ZONE_REGISTER_H_ */
-- 
1.7.7.6

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

Reply via email to