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.
Petr^2 Spacek
From 0291e1b63e077af06b84374c7aa0b15bd71aeb7d 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..1e7c31097b532a8641a34de589ef0926df6bf45f 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,94 @@ 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);
+ 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 +333,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 +358,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_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
