Dependency Graph

Dependency Graph
related to related to child of child of duplicate of duplicate of

View Issue Details

IDProjectCategoryView StatusLast Update
0002152SOGoSOPEpublic2013-01-31 21:52
ReporterAdam Tkac Assigned To 
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version2.0.3a 
Target Version2.0.4Fixed in Version2.0.4 
Summary0002152: Users with commas and spaces in their LDAP DNs cannot login
Description

When user DN in LDAP contains comma immediately followed by space, SOGo fails to construct correct LDAP binddn for such user.

Example:

dn: CN=Tkac\, Adam,OU=ITZ,DC=geodis,DC=cz

I captured LDAP network traffic and found that SOGo tries to bind with following binddn (note that the space character is missing)

dn: CN=Tkac\,Adam,OU=ITZ,DC=geodis,DC=cz

So the LDAP bind obviously fails because DN doesn't exist and SOGo web interface writes that user specified wrong credentials.

I'm using the latest SOGo/SOPE:
sope49-ldap-4.9-20121206_1664.el6.1.x86_64
sogo-2.0.3a-1.centos6.x86_64

Additional Information

After inspection the main issue is in sope, in sope-ldap/NGLdap/NSString+DN.m:dnComponents method. This method thinks that every comma splits DN. However this is not true when comma is escaped.

After incorrect split this method removes starting and trailing whitespaces from attributes, so the space from DN disappears.

I will attach proposed patch, which uses ldap_explode_dn to split DN into RDNs.

TagsNo tags attached.

Relationships

has duplicate 0001407 resolvedludovic LDAP authentication agains ActiveDirectory fails when backslash in distinguishedName 

Activities

2012-12-19 21:55

 

0001-NSString-DN.m-dnComponents-method-failed-to-parse-DN.patch (2,370 bytes)   
From 21f18217d638c3b11dd9800def3613512366899b Mon Sep 17 00:00:00 2001
From: Adam Tkac <vonsch@gmail.com>
Date: Wed, 19 Dec 2012 21:08:05 +0100
Subject: [PATCH] NSString+DN.m:dnComponents method failed to parse DNs which
 contain commas

NSString+DN.m:dnComponents failed to parse for example following DN:

dn: CN=Tkac\, Adam,OU=ITZ,DC=geodis,DC=cz

The DN was splitted into

"CN=Tkac\", "Adam", "OU=ITZ", "DC=geodis", "DC=cz"

which is apparently wrong. Now the DN is splitted correctly into

"CN=Tkac\, Adam", "OU=ITZ", "DC=geodis", "DC=cz"

Signed-off-by: Adam Tkac <vonsch@gmail.com>
---
 sope-ldap/NGLdap/NSString+DN.m | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/sope-ldap/NGLdap/NSString+DN.m b/sope-ldap/NGLdap/NSString+DN.m
index eddc067..e5a979f 100644
--- a/sope-ldap/NGLdap/NSString+DN.m
+++ b/sope-ldap/NGLdap/NSString+DN.m
@@ -22,6 +22,9 @@
 
 #import <Foundation/NSCharacterSet.h>
 
+#define LDAP_DEPRECATED 1
+#include <ldap.h>
+
 #include "NSString+DN.h"
 #include <NGExtensions/NSString+Ext.h>
 #include "common.h"
@@ -48,6 +51,43 @@ static NSArray *cleanDNComponents(NSArray *_components) {
   return _components;
 }
 
+static NSArray *explodeDN(const char *dn) {
+  char **exploded;
+  unsigned i;
+  NSMutableArray *array;
+  NSArray *ret;
+  id *cs;
+
+  if (dn == NULL)
+    return nil;
+
+  if (dn[0] == '\0') {
+    return [NSArray arrayWithObjects: @"", nil];
+  }
+
+  exploded = ldap_explode_dn(dn, 0);
+  if (exploded == NULL)
+    return nil;
+
+  /* Count number of RDNs */
+  for (i = 0; exploded[i] != NULL; i++);
+
+  cs = calloc(i, sizeof(id));
+
+  array = [[NSMutableArray alloc] initWithCapacity:i];
+  for (i = 0; exploded[i] != NULL; i++) {
+    [array addObject: [NSString stringWithCString:exploded[i]]];
+  }
+
+  ldap_value_free(exploded);
+
+  ret = [array copy];
+
+  if (cs != NULL) { free(cs); cs = NULL; }
+
+  return cleanDNComponents(ret);
+}
+
 @implementation NSString(DNSupport)
 
 + (NSString *)dnWithComponents:(NSArray *)_components {
@@ -55,7 +95,7 @@ static NSArray *cleanDNComponents(NSArray *_components) {
 }
 
 - (NSArray *)dnComponents {
-  return cleanDNComponents([self componentsSeparatedByString:dnSeparator]);
+  return explodeDN([self cString]);
 }
 
 - (NSString *)stringByAppendingDNComponent:(NSString *)_component {
-- 
1.8.0.2

Adam Tkac

Adam Tkac

2012-12-19 21:56

reporter   ~0005079

The patch fixes the login issue in my case, I already verified it. Sorry that it is not so clean but I have no experience with objective-c.

ludovic

ludovic

2013-01-11 16:52

administrator   ~0005138

Fix slightly modified and pushed, thanks!

https://github.com/inverse-inc/sope/commit/134db61df78edbd367c8b740513802c5fcb25571
https://github.com/inverse-inc/sope/commit/52d80264db6e5d4d8aedd50f3bb76d015b8394d6

jraby

jraby

2013-01-31 21:52

viewer   ~0005303

New fix as this patch broke utf8 encoding of DNs : https://github.com/inverse-inc/sope/commit/7a8cb886103dc9b4569151ae185b9e3bc2e95b4e

Issue History

Date Modified Username Field Change
2012-12-19 21:54 Adam Tkac New Issue
2012-12-19 21:55 Adam Tkac File Added: 0001-NSString-DN.m-dnComponents-method-failed-to-parse-DN.patch
2012-12-19 21:56 Adam Tkac Note Added: 0005079
2012-12-20 13:51 ludovic Target Version => 2.0.4
2013-01-11 16:52 ludovic Note Added: 0005138
2013-01-11 16:52 ludovic Status new => closed
2013-01-11 16:52 ludovic Resolution open => fixed
2013-01-11 16:52 ludovic Fixed in Version => 2.0.4
2013-01-18 19:59 ludovic Relationship added has duplicate 0001407
2013-01-31 21:52 jraby Note Added: 0005303