Bug 246124 - adminutil: findSIEDNByIDSSL() uses wrong credentials
adminutil: findSIEDNByIDSSL() uses wrong credentials
Status: CLOSED ERRATA
Product: 389
Classification: Community
Component: Admin (Show other bugs)
1.1.0
All Linux
low Severity low
: ---
: ---
Assigned To: Nathan Kinder
Orla Hegarty
:
Depends On:
Blocks: 240316 FDS1.1.0
  Show dependency treegraph
 
Reported: 2007-06-28 13:57 EDT by Nathan Kinder
Modified: 2008-01-03 13:05 EST (History)
0 users

See Also:
Fixed In Version: 1.1.3-1.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-07-25 01:22:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


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

  None (edit)
Description Nathan Kinder 2007-06-28 13:57:37 EDT
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 13:57:38 EDT
Created attachment 158146 [details]
CVS Diffs
Comment 2 Noriko Hosoi 2007-06-28 14:06:24 EDT
Looks good.
Comment 3 Nathan Kinder 2007-06-28 14:15:34 EDT
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 14:25:15 EDT
admldapGetSIEDN() returns malloc'ed memory - you should call PL_strfree() when
done with the value.
Comment 5 Nathan Kinder 2007-06-28 14:45:25 EDT
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 16:48:39 EDT
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 01:21:39 EDT
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.