Bug 246124

Summary: adminutil: findSIEDNByIDSSL() uses wrong credentials
Product: [Retired] 389 Reporter: Nathan Kinder <nkinder>
Component: AdminAssignee: Nathan Kinder <nkinder>
Status: CLOSED ERRATA QA Contact: Orla Hegarty <ohegarty>
Severity: low Docs Contact:
Priority: low    
Version: 1.1.0   
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.1.3-1.fc7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-07-25 05:22:00 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 240316, 427409    
Attachments:
Description Flags
CVS Diffs
none
Revised Diffs none

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.