Re: [Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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.
Regards, Adam
Petr^2 Spacek
bind-dyndb-ldap-pspacek-0019-2-Add-proper-DN-escaping-before-LDAP-library-calls.patch
From 0291e1b63e077af06b84374c7aa0b15bd71aeb7d Mon Sep 17 00:00:00 2001
From: Petr Spacekpspa...@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 Spacekpspa...@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 @@
#includeisc/buffer.h
#includeisc/mem.h
#includeisc/util.h
+#includeisc/string.h
#includedns/name.h
#includedns/rdatatype.h
@@ -32,6 +33,7 @@
#includeerrno.h
#includestrings.h
+#includectype.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);
Please use 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);
Please put the space between number/variable and an operator (3 *
dns_str_len + 1 in this case).
+ 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;
ditto
+ 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 */
ditto
+ log_bug(dns string too short);
+
Re: [Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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
Regards, Adam
Petr^2 Spacek
bind-dyndb-ldap-pspacek-0019-2-Add-proper-DN-escaping-before-LDAP-library-calls.patch
From 1db133bf26a3c8bec7da3b045258bad630378d50 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 | 109 --
src/zone_register.c |7 +++
src/zone_register.h |3 +
3 files changed, 114 insertions(+), 5 deletions(-)
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index 6405a98fda942cbba10b4c862351c4e3695aba85..6cb761db97cff12b999263e2fcb4f31603ccd733 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,96 @@ 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 */
+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
Re: [Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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
[Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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,
Re: [Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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
Re: [Freeipa-devel] [PATCH 0019] Add proper DN escaping before LDAP library calls
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
