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
From 10bc76498072e554ccfb1504d81f3166a14b79a5 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 | 95 ++++++++++++++++++++++++++++++++++++++++++++++++---
src/zone_register.c | 7 ++++
src/zone_register.h | 3 ++
3 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index 6405a98fda942cbba10b4c862351c4e3695aba85..32c17f92fc2d2f0f9a5237fd8756f8d4e1a2d476 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,84 @@ cleanup:
return result;
}
+/**
+ * Convert string from DNS escaping to LDAP escaping.
+ * DNS escaping: "\123" = ASCII value 123 (decimal)
+ * LDAP escaping: "\xy" = ASCII value xy (hexadecimal)
+ * DNS to text functions from ISC libraries do not convert certain characters
+ * (e.g. ","). These characters are converted from raw ASCII form to \xy form.
+ * Any character other than [a-zA-Z0-9.-] will be converted to \xy.
+ *
+ * If dns_str consists only from [a-zA-Z0-9.-], it will be checked & copied to
+ * output buffer, without any magic.
+ */
+isc_result_t
+dns_to_ldap_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);
+ 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 */
+ if (dns_str_len <= dns_idx+3) { /* this should never happen */
+ log_bug("dns string too short");
+ result = ISC_R_NOSPACE;
+ goto cleanup;
+ }
+ 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 +323,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 +348,26 @@ 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_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 +412,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
