[Fedora-directory-commits] mod_admserv mod_admserv.c,1.18,1.19

Richard Allen Megginson (rmeggins) fedora-directory-commits at redhat.com
Wed Jan 18 02:26:34 UTC 2006


Author: rmeggins

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

Modified Files:
	mod_admserv.c 
Log Message:
Bug(s) fixed: 175170
Bug Description: Directory Server Admin Server Dies after Secure Bind to Directory Server
Reviewed by: Rob C. (Thanks!)
Files: mod_admserv.c
Branch: HEAD
Fix Description: This fix makes the assumption that mod_nss will always be used.  It is possible to use mod_admserv without mod_nss - this would mean that the admin server accepts http, but uses ldaps to communicate with the DS.  However, I don't forsee that happening, so in order to simplify things, this fix makes mod_nss resposible for initializing NSS and shutting it down properly.
Another problem was the memory and resource leaks.  pset's have to be disposed of after use.  This appears to have been a problem in the old NES libAdmservPlugin as well since most of the code was just copied/pasted.  There were also a couple of other memory leaks.
NOTE: This is only part of the total fix, which will involve changes to mod_nss, ldap sdk, and admin server components.
Platforms tested: FC4
Flag Day: no
Doc impact: no



Index: mod_admserv.c
===================================================================
RCS file: /cvs/dirsec/mod_admserv/mod_admserv.c,v
retrieving revision 1.18
retrieving revision 1.19
diff -u -r1.18 -r1.19
--- mod_admserv.c	18 Nov 2005 21:18:42 -0000	1.18
+++ mod_admserv.c	18 Jan 2006 02:26:25 -0000	1.19
@@ -16,7 +16,11 @@
 /*
  * mod_admserv.c: Provides communication link between Console and Directory.
  *
+ * Authors (in alphabetical order)
  * Rob Crittenden
+ * Miodrag Kekic
+ * Rich Megginson
+ * Adam Prishtina
  *
  */
 
@@ -736,76 +740,19 @@
     return TRUE;
 }
 
-static int onlyOnceSwitch = 0;
-
-#if defined(WINNT)
-static char const prefixMask[] = "\\alias\\%s-";
-static char const secmodName[] = "\\alias\\secmod.db";
-#else
-static char const prefixMask[] = "/alias/%s-";
-static char const secmodName[] = "/alias/secmod.db";
-#endif
-
 static int
-sslinit(AdmldapInfo info, char const * svrroot, int forceInit)
+sslinit()
 {
-    int rc = PR_FALSE;
-  
-    if (onlyOnceSwitch) {
-        rc = PR_TRUE;
+    if (!NSS_IsInitialized()) {
+        ap_log_error(APLOG_MARK, APLOG_CRIT, 0 /* status */, NULL,
+                     "sslinit: mod_nss has not been started and initialized: cannot start server");
+        exit(1);
     } else {
-        char * x;
-        /*
-         * Usually one would find that the path to the cert and key db
-         * is alias/admin-serv-hostname- but one can not assume that
-         * to be the case. Thus we must disassemble the SIEDN to find
-         * the instance's actual name.
-         */
-        x = admldapGetSIEDN(info);
-        if (x) {
-            if (x[0]) {
-                int i;
-                for (i=1; x[i] && !(x[i] == '=' && x[i-1] != '\\'); ++i); /* scan past attr name */
-                if (x[i] == '=') {
-                    char * sie;
-                    for (++i; x[i] && x[i] == ' '; ++i); /* drop value's leading spaces */
-                    sie = &x[i];
-                    for ( ; x[i] && !(x[i] == ',' && x[i-1] != '\\'); ++i); /* till end of value */
-                    if (x[i] == ',') {
-                        char * scratch;
-                        for (x[i--] = 0; x[i] == ' '; --i) x[i] = 0; /* drop value's trailing spaces */
-                        scratch = (char *) malloc(sizeof(prefixMask) + strlen(sie));
-                        if (scratch) {
-                            sprintf(scratch, prefixMask, sie);
-                            if (SECSuccess == NSS_Initialize(serverroot, scratch, scratch, secmodName, 0)) {
-                                if (SSL_OptionSetDefault(SSL_ENABLE_SSL2, PR_FALSE)
-                                    || SSL_OptionSetDefault(SSL_ENABLE_SSL3, PR_TRUE)) {
-                                    ap_log_error(APLOG_MARK, APLOG_ERR, 0 /* status */, NULL,
-                                                 "Failed to enable SSL3 and disable SSL2");
-                                } else {
-                                    if (SSLPLCY_Install() == PR_SUCCESS) {
-                                        onlyOnceSwitch = 1;
-                                        rc = PR_TRUE;
-                                    } else {
-                                        ap_log_error(APLOG_MARK, APLOG_ERR, 0 /* status */, NULL,
-                                                     "SSLPLCY_Install() failed.");
-                                    }
-                                }
-                            } else {
-                                ap_log_error(APLOG_MARK, APLOG_CRIT, 0 /* status */, NULL,
-                                             "Failed to do NSS_Initialize using path %s and prefix %s",
-                                             serverroot, scratch);
-                            }
-                            free((void *) scratch);
-                        }
-                    }
-                }
-            }
-            free((void *) x);
-        }
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0 /* status */, NULL,
+                     "sslinit: mod_nss has been started and initialized");
     }
 
-    return rc;
+    return 1;
 }
 
 static int
@@ -838,7 +785,7 @@
     }
 
     if (admldapGetSecurity(info)) {
-        sslinit(info, serverroot, PR_FALSE);
+        sslinit();
         if (admldapBuildInfoSSL(info, &error)) {
         } else {
             char *host = admldapGetHost(info);
@@ -1109,6 +1056,9 @@
                 int errorcode;
                 char*  serverid = psetGetAttrSingleValue(tmp, SERVER_ID_ATTRIBUTE, &errorcode);
 
+                psetDelete(tmp);
+                tmp = NULL;
+
                 if (!serverid) {
                     ap_log_error(APLOG_MARK, APLOG_WARNING, 0, NULL,
                                  "sync_task_sie_data: Unable to find serverid for dn=\"%s\" (error code = %d)",
@@ -1117,6 +1067,7 @@
                 }
 
                 task_register_server(serverid, serverlist[i]);
+                PL_strfree(serverid);
                 ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
                              "sync_task_sie_data: registered server [%s] dn [%s]",
                              serverid, serverlist[i]);
@@ -1127,6 +1078,7 @@
                              serverlist[i], errorCode);
             }
         }
+        deleteAttrNameList(serverlist);
     } else {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, NULL,
                      "sync_task_sie_data: no servers found");
@@ -1146,6 +1098,7 @@
                          productID, productDN);
             ii++;
         }
+        deleteAttributeList(installlist);
     }
 
     destroyAdmldap(ldapInfo);
@@ -1342,9 +1295,11 @@
     }
 
     if (admldapGetSecurity(ldapInfo)) {
-        sslinit(ldapInfo, serverroot, PR_FALSE);
+        sslinit();
     }
 
+    destroyAdmldap(ldapInfo);
+
     binddn = apr_table_get(r->notes, RQ_NOTES_USERDN);
     bindpw = apr_table_get(r->notes, RQ_NOTES_USERPW);
 
@@ -1363,15 +1318,15 @@
     }
 
     errorCode = psetSetSingleValueAttr(pset, (char*)"userpassword", pwd);
+    psetDelete(pset);
+    pset = NULL;
 
     if (errorCode) {
-        psetDelete(pset);
         ap_log_rerror(APLOG_MARK, APLOG_CRIT, 0, r,
                       "PSET Set Failed for attribute userpassword, err=%d",
                       errorCode);
         return 0;
     }
-    psetDelete(pset);
     return 1;
 }
 
@@ -1977,16 +1932,18 @@
     }
   
     if (admldapGetSecurity(info)) {
-        sslinit(info, serverroot, PR_FALSE);
+        sslinit();
         if (admldapBuildInfoSSL(info, &error)) {
         } else {
             ap_log_error(APLOG_MARK, APLOG_CRIT, 0, base_server,
                          "host_ip_init(): unable to create secure AdmldapInfo (error code = %d)",
                          error);
+            destroyAdmldap(info);
             return DONE;
         }
     }
 
+    destroyAdmldap(info);
 #ifdef CHANGE_EUID
     /* make sure pset creates the cache file owned by the server uid, not root */
     if (geteuid() == 0) {
@@ -2021,12 +1978,15 @@
         ap_log_error(APLOG_MARK, APLOG_CRIT, 0, base_server,
                      "host_ip_init(): PSET failure: Could not retrieve access hosts attribute (pset error = %s)",
                      psetErrorString(error, NULL));
+        psetDelete(pset);
         return DONE;
     }
 
     accessHosts = apr_pstrdup(module_pool, val);
 
     val = psetGetAttrSingleValue(pset, NS_ADMIN_ACCESS_ADDRESSES, &error);
+    psetDelete(pset);
+    pset = NULL;
     if(val) {
     } else {
         ap_log_error(APLOG_MARK, APLOG_CRIT, 0, base_server,
@@ -2121,6 +2081,17 @@
 }
 
 /*
+ * NSS caches SSL client session information - this cache must be cleared, otherwise
+ * NSS_Shutdown will give an error.  mod_nss also does this (along with the NSS_Shutdown)
+ * It is ok to call SSL_ClearSessionCache multiple times.
+ */
+static
+apr_status_t mod_admserv_unload(void *data)
+{
+    SSL_ClearSessionCache();
+}
+
+/*
  * This is where we do the rest of our initialization, that depends
  * on configuration settings
  */
@@ -2141,6 +2112,12 @@
     auth_users = HashTableCreate();
     auth_tasks = HashTableCreate();
 
+    /* 
+     * Let us cleanup on restarts and exists
+     */
+    apr_pool_cleanup_register(p, base_server,
+                              mod_admserv_unload,
+                              apr_pool_cleanup_null);
 
     /* if nsroot was not set in the config, get from the environment */
     srv_cfg = our_sconfig(base_server);
@@ -2482,17 +2459,6 @@
   apr_table_set(r->notes, RQ_NOTES_USERDN, userdn);
   apr_table_set(r->notes, AUTHENTICATION_LDAP_URL, ldapURL = formLdapURL(data, r->pool));
   apr_table_set(r->notes, RQ_NOTES_USERPW, pw);
-#if 0
-  pblock_nvinsert("userdn", userdn, rq->vars);
-  pblock_nvinsert("auth-type", "basic", rq->vars);
-  pblock_nvinsert("auth-password", pw, rq->vars);
-  if (uid) pblock_nvinsert("auth-user", uid, rq->vars);
-  pblock_nvinsert(AUTHENTICATION_LDAP_URL, ldapURL = formLdapURL(data, r->p), rq->vars);
-  if (pw_expiring >= 0) {
-    sprintf(pw_expiring_str, "%d", pw_expiring);
-    pblock_nvinsert("auth-password-expire", pw_expiring_str, rq->vars);
-  }
-#endif
 
   create_auth_users_cache_entry(user, userdn, pw, ldapURL);
 
@@ -2562,15 +2528,7 @@
 
     /* That failed too. The last resort is to fall back to the standard
      * Apache basic-auth using admpw.
-     */ 
-
-#if 0
-    ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r,
-       "dn is %s", dn);
-    apr_table_set(r->notes, RQ_NOTES_USERDN, dn);
-    apr_table_set(r->notes, RQ_NOTES_USERPW, sent_pw);
-#endif
-
+     */
     return DECLINED;
 }
 
@@ -2644,7 +2602,7 @@
      * be set in admserv.conf (or httpd.conf)
      */
     if (!string) {
-#define ADMSERV_VERSION_STRING "Fedora-Administrator/7.0"
+#define ADMSERV_VERSION_STRING "Fedora-Administrator/1.0"
         string = ADMSERV_VERSION_STRING;
     }
     apr_table_setn(r->headers_out, "Admin-Server", string);
@@ -2700,13 +2658,15 @@
 {
     /* Do basic auth after our own auth */
     static const char * const aszPost[] = { "mod_auth.c", NULL };
+    /* Make sure mod_nss has been configured before us */
+    static const char * const aszPre[] = { "mod_nss.c", NULL };
 
     /* handler for /admin-serv/authenticate requests */
     ap_hook_handler(userauth, NULL, NULL, APR_HOOK_MIDDLE);
     /* handler for /admin-serv/commands */
     ap_hook_handler(admserv_command_handler, NULL, NULL, APR_HOOK_MIDDLE);
-    ap_hook_pre_config(mod_admserv_pre_config, NULL, NULL, APR_HOOK_MIDDLE);
-    ap_hook_post_config(mod_admserv_post_config, NULL, NULL, APR_HOOK_MIDDLE);
+    ap_hook_pre_config(mod_admserv_pre_config, aszPre, NULL, APR_HOOK_MIDDLE);
+    ap_hook_post_config(mod_admserv_post_config, aszPre, NULL, APR_HOOK_MIDDLE);
     /* called at read_request phase to block clients from disallowed hosts */
     ap_hook_post_read_request(admserv_host_ip_check, NULL, NULL, APR_HOOK_MIDDLE);
     ap_hook_check_user_id(admserv_check_user_id, NULL, aszPost, APR_HOOK_MIDDLE);




More information about the Fedora-directory-commits mailing list