Bug 246124 - adminutil: findSIEDNByIDSSL() uses wrong credentials
Summary: adminutil: findSIEDNByIDSSL() uses wrong credentials
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: 389
Classification: Retired
Component: Admin
Version: 1.1.0
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Nathan Kinder
QA Contact: Orla Hegarty
URL:
Whiteboard:
Depends On:
Blocks: 240316 FDS1.1.0
TreeView+ depends on / blocked
 
Reported: 2007-06-28 17:57 UTC by Nathan Kinder
Modified: 2008-01-03 18:05 UTC (History)
0 users

Fixed In Version: 1.1.3-1.fc7
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-07-25 05:22:00 UTC
Embargoed:


Attachments (Terms of Use)
CVS Diffs (1.82 KB, patch)
2007-06-28 17:57 UTC, Nathan Kinder
no flags Details | Diff
Revised Diffs (1.25 KB, patch)
2007-06-28 18:45 UTC, Nathan Kinder
no flags Details | Diff

Description Nathan Kinder 2007-06-28 17:57:37 UTC
The findSIEDNByIDSSL() function in adminutil needs to use the credentials used
to authenticate to the webserver instead of the siedn when calling
getServerDNListSSL().

The attached diffs save the siedn off before calling getServerDNListSSL() and
set the userdn and password in the AdmLDAPInfo to the credentials supplied by
the password pipe.  There's also an unrelated change in the code that parses
admpw.  On my machine (FC6 i386), the parsing of admpw was giving incorrect
results as viewed in gdb.  For some reason, the pointer was being incremented
before assignment.  Noriko did not observe this on her RHEL4 machine, but we
agreed that the changes I made are more safe as we will always get the intended
results.

Comment 1 Nathan Kinder 2007-06-28 17:57:38 UTC
Created attachment 158146 [details]
CVS Diffs

Comment 2 Noriko Hosoi 2007-06-28 18:06:24 UTC
Looks good.

Comment 3 Nathan Kinder 2007-06-28 18:15:34 UTC
Rich pointed out an error in the increment of the pointer when parsing admpw. 
Here's the diff that addresses the issue he pointed out.

retrieving revision 1.8
diff -u -5 -t -r1.8 admutil.c
--- lib/libadminutil/admutil.c  8 May 2007 19:13:25 -0000       1.8
+++ lib/libadminutil/admutil.c  28 Jun 2007 18:11:25 -0000
@@ -1245,11 +1245,12 @@
     break;
   case 1:
     /* EOF */
   default:
     password = strchr(buf, ':');
-    *password++ = '\0';
+    *password = '\0';
+    password++;
     while (*password) {
       if (*password == ' ') password++;
       else break;
     }


Comment 4 Rich Megginson 2007-06-28 18:25:15 UTC
admldapGetSIEDN() returns malloc'ed memory - you should call PL_strfree() when
done with the value.

Comment 5 Nathan Kinder 2007-06-28 18:45:25 UTC
Created attachment 158149 [details]
Revised Diffs

This new set of diffs addresses Rich's comment about the need to free the
siedn.	I also did some more tests around the parsing of the admpw contents
since the orignal code is correct and should work as intended.	I found that
the original code does in fact work on the same machine where I saw it fail
before.  I'm thinking that this was due to memory corruption from another bug
(bug 245396) that I was running up against in adminutil at that time.  I've
backed my changes to this area of code out.

Comment 6 Nathan Kinder 2007-06-28 20:48:39 UTC
Checked into adminutil (HEAD).  Thanks to Rich and Noriko for their reviews!

Checking in lib/libadmsslutil/srvutilssl.c;
/cvs/dirsec/adminutil/lib/libadmsslutil/srvutilssl.c,v  <--  srvutilssl.c
new revision: 1.6; previous revision: 1.5
done

Comment 7 Fedora Update System 2007-07-25 05:21:39 UTC
adminutil-1.1.3-1.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.