[Fedora-directory-commits] mod_admserv mod_admserv.c, 1.30, 1.31 mod_admserv.h, 1.4, 1.5

Richard Allen Megginson (rmeggins) fedora-directory-commits at redhat.com
Fri Jun 22 22:37:48 UTC 2007


Author: rmeggins

Update of /cvs/dirsec/mod_admserv
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv28739/mod_admserv

Modified Files:
	mod_admserv.c mod_admserv.h 
Log Message:
Resolves: bug 245369
Bug Description: mod_admserv: Task cache refresh uses wrong credentials
Reviewed by: nkinder (Thanks!)
Fix Description: When the user requests a Task url, the admin server first
figures out which server instance (or product) the request is for, then checks
to see if it has seen that server or product before.  If not, it uses the 
function sync_task_sie_data() to read the task data from the SIEs and ISIEs.
However, it needs to use the credentials of the currently authenticated user
to do so, because the tasks are protected by ACIs, and the user should only be
allowed to read those tasks the user has access to.  The interface to read
these tasks is not great.  It expects the SIE is a user with a password, and
it attempts to bind as that user, instead of the currently authenticated user.
I had to hack it to force it to use the current userdn and password instead
of the SIE DN and SIE password.
The SIE DN and password are now deprecated for binding.  There were a couple
of places where the SIE was used for both the bind DN and the SIE DN.  I've
created another structure member for the admservSieDN for use as the SIE (the
configuration base DN) instead of as a bind DN, and deprecated the use of the
SIE as the bind DN elsewhere in the code.
Platforms tested: RHEL4
Flag Day: no
Doc impact: no



Index: mod_admserv.c
===================================================================
RCS file: /cvs/dirsec/mod_admserv/mod_admserv.c,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- mod_admserv.c	19 Jun 2007 23:31:12 -0000	1.30
+++ mod_admserv.c	22 Jun 2007 22:37:46 -0000	1.31
@@ -758,7 +758,7 @@
            and will set up all of the TLS/SSL stuff */
         /* if we are acting as simply a TLS/SSL client to the directory server, 
            we still have to perform our own TLS/SSL client init */
-        if (ADMSSL_Init(info, configdir, 0)) {
+        if (ADMSSL_Init(info, (char *)configdir, 0)) {
             ap_log_error(APLOG_MARK, APLOG_CRIT, 0 /* status */, NULL,
                          "sslinit: NSS is required to use LDAPS, but security initialization failed.  Cannot start server");
             exit(1);
@@ -816,6 +816,7 @@
     userGroupServer.port   = 0;
     userGroupServer.secure = 0;
     userGroupServer.baseDN = NULL;
+    userGroupServer.admservSieDN = NULL;
 
     if (NULL == admldapGetLDAPHndl(info)) {
         /* LDAP is not available; gather info from the cache */
@@ -1035,6 +1036,7 @@
     return DONE;
 }
 
+
 static int
 sync_task_sie_data(const char *name, char *query, void *arg, request_rec *r)
 {
@@ -1046,7 +1048,8 @@
     int servercnt, i;
     UserCacheEntry *cache_entry = NULL;
     char *siedn = NULL;
-    char *passwd = NULL;
+    const char *userdn = apr_table_get(r->notes, RQ_NOTES_USERDN);
+    const char *passwd = apr_table_get(r->notes, RQ_NOTES_USERPW);
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
                  "sync_task_sie_data: getting ldap info for [%s]",
@@ -1062,22 +1065,24 @@
 
     siedn = admldapGetSIEDN(ldapInfo);
     task_register_server(ADMIN_SERVER_ID, siedn);
-    PL_strfree(siedn);
 
-    passwd = admldapGetSIEPWD(ldapInfo);
+    /* HACK HACK HACK */
+    /* getServerDNListSSL uses the siedn as the binddn - so we temporarily
+       replace the siedn with the userdn - fortunately it doesn't use the
+       siedn as the SIE DN */
+    admldapSetSIEDN(ldapInfo, userdn);
     if (NULL == passwd) { /* use the passwd in cache if possible */
-        char *userdn = admldapGetUserDN(ldapInfo, NULL);
         cache_entry = (UserCacheEntry*)HashTableFind(auth_users, userdn);
         if (cache_entry) {
-            admSetCachedSIEPWD(cache_entry->userPW);
+            passwd = cache_entry->userPW;
         }
-        PL_strfree(userdn);
-    } else {
-        PL_strfree(passwd);
     }
-    
+    admSetCachedSIEPWD(passwd);
 
     serverlist = getServerDNListSSL(ldapInfo);
+    /* HACK HACK HACK - reset after getServerDNListSSL */
+    admldapSetSIEDN(ldapInfo, siedn);
+
     servercnt=0;
     if (serverlist) {
         while (serverlist[servercnt]) servercnt++;
@@ -1086,19 +1091,15 @@
         for (i=0; i < servercnt; i++) {
             /* Create Pset for each individual server */
             char *host = admldapGetHost(ldapInfo);
-            char *siedn = admldapGetSIEDN(ldapInfo);
-            char *siepwd = admldapGetSIEPWD(ldapInfo);
             tmp = psetRealCreateSSL(host,
                                     admldapGetPort(ldapInfo),
                                     admldapGetSecurity(ldapInfo),
                                     serverlist[i],
-                                    siedn,
-                                    siepwd,
+                                    (char *)userdn,
+                                    (char *)passwd,
                                     NULL,
                                     &errorCode);
             PL_strfree(host);
-            PL_strfree(siedn);
-            PL_strfree(siepwd);
             if (tmp) {
 #          define SERVER_ID_ATTRIBUTE (char*)"nsServerID"
 
@@ -1136,6 +1137,11 @@
     /* DT 02/02/98
      * Register installed product tasks
      */
+    /* HACK HACK HACK */
+    /* getInstalledServerDNListSSL uses the siedn as the binddn - so we temporarily
+       replace the siedn with the userdn - fortunately it doesn't use the
+       siedn as the SIE DN */
+    admldapSetSIEDN(ldapInfo, userdn);
     if ((installlist = getInstalledServerDNListSSL(ldapInfo))) {
         int ii = 0;
         while (installlist[ii]) {
@@ -1149,13 +1155,15 @@
         }
         deleteAttributeList(installlist);
     }
+    /* HACK HACK HACK - reset after getInstalledServerDNListSSL */
+    admldapSetSIEDN(ldapInfo, siedn);
+    PL_strfree(siedn);
 
     destroyAdmldap(ldapInfo);
 
     return TRUE;
 }
 
-static int update_ds(char *admroot, char *pwd, request_rec *r);
 static int update_admpwd(char *admroot, char *newuid, char *newpw);
 static int update_adm_conf(char *admroot, char *newpw);
 
@@ -1231,7 +1239,6 @@
    ldapURL = formLdapURL(&registryServer, r->pool);
    create_auth_users_cache_entry(uid, (char *)userDN, password, ldapURL);
 
-   registryServer.bindPW = password;
    rval = 1;
 bailout:
    closeLDAPConnection(ld);
@@ -1253,7 +1260,6 @@
     char       inbuf[BIG_LINE];
     char       outbuf[64];  /* needs at least 36 bytes */
     char       *origpw = (char *)apr_table_get(r->notes, RQ_NOTES_USERPW);
-    int        ds_done = 0;
     int        admpwd_done = 0;
 
     apr_snprintf(filename, sizeof(filename), "%s/admpw", configdir);
@@ -1286,12 +1292,6 @@
     
     uid = inbuf; *col=0; pw=col+1;
     
-    if (!update_ds(configdir, newpw, r))  {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL,
-                     "failed to update siepwd on DS");
-        return 0;
-    }
-    ds_done = 1;
     apr_sha1_base64(newpw, strlen(newpw), outbuf);
     if (!update_admpwd(configdir, uid, outbuf)) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, NULL,
@@ -1309,9 +1309,6 @@
     return 1;
 
 recover:
-    if (ds_done) {
-        update_ds(configdir, origpw, r);
-    }
     if (admpwd_done) {
         apr_sha1_base64(origpw, strlen(origpw), outbuf);
         update_admpwd(configdir, uid, outbuf);
@@ -1354,64 +1351,6 @@
     return 1;
 }
 
-/*
- * Modify userpassword in the DS
- */
-static int
-update_ds(char *admroot, char *pwd, request_rec *r)
-{ 
-    int   errorCode;
-    PsetHndl pset;
-    const char *binddn = 0;
-    const char *bindpw = 0;
-    AdmldapInfo ldapInfo = NULL;
-
-    /* Initialize the pset  */
-
-    ldapInfo = admldapBuildInfo(admroot, &errorCode);
-
-    if (!ldapInfo) {
-        ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
-                      "AdmInit: Failed to read data from adm.conf");
-        return 0;
-    }
-
-    if (admldapGetSecurity(ldapInfo)) {
-        sslinit(ldapInfo, admroot);
-    }
-
-    destroyAdmldap(ldapInfo);
-
-    binddn = apr_table_get(r->notes, RQ_NOTES_USERDN);
-    bindpw = apr_table_get(r->notes, RQ_NOTES_USERPW);
-
-    pset = psetCreateSSL((char*)"admin-serv", 
-                         admroot,
-                         (char*)binddn,
-                         (char*)bindpw,
-                         &errorCode);
-
-    if (pset) {
-    } else {
-        ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
-                      "PSET Creation Failed for binddn [%s], err=%d",
-                      binddn, errorCode);
-        return 0;
-    }
-
-    errorCode = psetSetSingleValueAttr(pset, (char*)"userpassword", pwd);
-    psetDelete(pset);
-    pset = NULL;
-
-    if (errorCode) {
-        ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
-                      "PSET Set Failed for attribute userpassword, err=%d",
-                      errorCode);
-        return 0;
-    }
-    return 1;
-}
-
 static void
 populate_tasks_from_server(char *serverid, const void *sieDN,  void *userdata)
 {
@@ -1434,8 +1373,8 @@
             return;
         }
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
-                     "populate_tasks_from_server(): Opened new LDAPConnection to [%s:%d] user [%s]",
-                     registryServer.host, registryServer.port, registryServer.bindDN);
+                     "populate_tasks_from_server(): Opened new LDAPConnection to [%s:%d]",
+                     registryServer.host, registryServer.port);
         data->server = server;
     }
 
@@ -1542,6 +1481,8 @@
     int             ldapError;
     char            entryDN[LINE_LENGTH];
     char           *p;
+    char           *siedn; /* this is looked up from the serverid, which is
+                              extracted from the uri */
     LDAP           *server;
     const char     *userdn;
     const char     *pw;
@@ -1578,14 +1519,14 @@
     saveduri = apr_pstrdup(r->pool, uri);
     *p       = '\0';
 
-    if (!(p = (char*)HashTableFind(servers, serverid))) {
+    if (!(siedn = (char*)HashTableFind(servers, serverid))) {
         /* DT 4/6/98 -- If we're seeing a serverid for the first time, then we try to do
          *              a resync to pull in new data for the serverid. If it still fails,
          *              then the client is out of luck. */
       
-        admserv_runtime_command_exec(RUNTIME_RESYNC_COMMAND, NULL, NULL);
+        admserv_runtime_command_exec(RUNTIME_RESYNC_COMMAND, r->args, r);
       
-        if (!(p = (char*)HashTableFind(servers, serverid))) {
+        if (!(siedn = (char*)HashTableFind(servers, serverid))) {
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                           "admserv_check_authz(): unable to find registered server (%s)",
                           serverid);
@@ -1601,7 +1542,7 @@
         return OK;
     }
 
-    if (!build_full_DN(&storage, entryDN+LINE_LENGTH, uri, p)) {
+    if (!build_full_DN(&storage, entryDN+LINE_LENGTH, uri, siedn)) {
         ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                       "admserv_check_authz(): unable to build DN from URL - bad URL [%s]",
                       uri);
@@ -2264,8 +2205,9 @@
     registryServer.port   = (admldapGetPort(info) < 0) ? 389 : admldapGetPort(info);
     registryServer.secure = (admldapGetSecurity(info)) ? 1 : 0;
     registryServer.baseDN = admldapGetBaseDN(info);
-    registryServer.bindDN = admldapGetSIEDN(info);
-    registryServer.bindPW = admldapGetSIEPWD(info);
+    registryServer.bindDN = ""; /* deprecated - use user credentials */
+    registryServer.bindPW = ""; /* deprecated - use user credentials */
+    registryServer.admservSieDN = admldapGetSIEDN(info);
   
     destroyAdmldap(info);
     info = NULL;
@@ -2283,7 +2225,7 @@
     }
   
     /* Register the admin server tasks */
-    task_register_server(ADMIN_SERVER_ID, registryServer.bindDN);
+    task_register_server(ADMIN_SERVER_ID, registryServer.admservSieDN);
 
     /* Populate the auth_tasks cache for the Local Admin */
 
@@ -2326,7 +2268,7 @@
         TaskCacheEntry *cache_entry;
 
         if (!build_full_DN(&storage, startds+LINE_LENGTH, uri,
-                           registryServer.bindDN)) {
+                           registryServer.admservSieDN)) {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, base_server,
                 "mod_admserv_post_config: unable to build DN from URL - bad URL [%s]",
                 uri?uri:"none");
@@ -2496,7 +2438,7 @@
     ap_rprintf(r, "ldapPort: %d\n", registryServer.port);
     ap_rprintf(r, "ldapSecurity: %s\n", (registryServer.secure == 1) ? "on" : "off");
     ap_rprintf(r, "ldapBaseDN: %s\n", registryServer.baseDN);
-    ap_rprintf(r, "SIE: %s\n", registryServer.bindDN);
+    ap_rprintf(r, "SIE: %s\n", registryServer.admservSieDN);
     ap_rputs("NMC_Status: 0\n", r);
 
     return OK;
@@ -2532,7 +2474,9 @@
   {
       closeLDAPConnection(server);
       ap_log_rerror(APLOG_MARK, APLOG_NOTICE|APLOG_NOERRNO, 0, r,
-          "unable to bind to server [%s:%d] as [%s]", data->host, data->port, data->bindDN); /*i18n*/
+          "unable to bind to server [%s:%d] as [%s]",
+          data->host, data->port,
+          (data->bindDN && *data->bindDN) ? data->bindDN : "(anonymous)"); /*i18n*/
       return DECLINED;
   }
 
@@ -2545,15 +2489,16 @@
 
       tries = 0;
       do {
-        ldapError = ldapu_find_userdn(server, user, baseDN ? baseDN : data->baseDN, &userdn);
+        /* first try the basedn in the ldap server data, then the basedn param as a fallback */
+        ldapError = ldapu_find_userdn(server, user, data->baseDN ? data->baseDN : baseDN, &userdn);
         if(ldapError != LDAP_SERVER_DOWN && ldapError != LDAP_CONNECT_ERROR)
           break;
   
         closeLDAPConnection(server);
         if(!(server = openLDAPConnection(data)))
           ap_log_rerror(APLOG_MARK, APLOG_NOTICE|APLOG_NOERRNO, 0, r,
-                        "unable to find user [%s] in server [%s:%d]",
-                        user, data->host, data->port);
+                        "unable to find user [%s] in server [%s:%d] under base DN [%s]",
+                        user, data->host, data->port, data->baseDN ? data->baseDN : baseDN);
           return DECLINED;
       } while (server != NULL && ++tries < 2);
 


Index: mod_admserv.h
===================================================================
RCS file: /cvs/dirsec/mod_admserv/mod_admserv.h,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- mod_admserv.h	15 Nov 2006 21:31:40 -0000	1.4
+++ mod_admserv.h	22 Jun 2007 22:37:46 -0000	1.5
@@ -131,9 +131,12 @@
    char *host;
    int   port;
    int 	 secure;  /* track whether the server is running in secure mode */
-   char *baseDN;
-   char *bindDN;
-   char *bindPW;
+   char *baseDN;  /* for the config ds, usually o=NetscapeRoot
+                     for the user/group ds, this is the default
+                     suffix e.g. dc=example,dc=com */
+   char *bindDN;  /* deprecated since the SIE cannot bind anymore */
+   char *bindPW;  /* deprecated since the SIE cannot bind anymore */
+   char *admservSieDN; /* SIE DN of this admin server */
 } LdapServerData;
 
 typedef struct ServletLookupData {




More information about the Fedora-directory-commits mailing list