Replaced auth_worker_max_request_count setting with passdb pam { args = max_requests=n }

--HG--
branch : HEAD

diff --git a/dovecot-example.conf b/dovecot-example.conf
index 9b1e650e229b2484228a261eac196204757dd8c1..1a6a36a2e8ebe305cf8919f034a0249cf0fba938 100644 (file)
--- a/dovecot-example.conf
+++ b/dovecot-example.conf
@@ -798,10 +798,6 @@ protocol lda {
 # automatically created and destroyed as needed.
 #auth_worker_max_count = 30
 
-# Number of auth requests to handle before destroying the process. This may
-# be useful if PAM plugins leak memory.
-#auth_worker_max_request_count = 0
-
 # Host name to use in GSSAPI principal names. The default is to use the
 # name returned by gethostname(). Use "$ALL" to allow all keytab entries.
 #auth_gssapi_hostname =
@@ -859,7 +855,7 @@ auth default {
   # REMEMBER: You'll need /etc/pam.d/dovecot file created for PAM
   # authentication to actually work. <doc/wiki/PasswordDatabase.PAM.txt>
   passdb pam {
-    # [session=yes] [setcred=yes] [failure_show_msg=yes]
+    # [session=yes] [setcred=yes] [failure_show_msg=yes] [max_requests=<n>]
     # [cache_key=<key>] [<service name>]
     #
     # session=yes makes Dovecot open and immediately close PAM session. Some
@@ -869,6 +865,10 @@ auth default {
     # need that. They aren't ever deleted though, so this isn't enabled by
     # default.
     #
+    # max_requests specifies how many PAM lookups to do in one process before
+    # recreating the process. The default is 100, because many PAM plugins
+    # leak memory.
+    #
     # cache_key can be used to enable authentication caching for PAM
     # (auth_cache_size also needs to be set). It isn't enabled by default
     # because PAM modules can do all kinds of checks besides checking password,

diff --git a/src/auth/auth-worker-client.c b/src/auth/auth-worker-client.c
index 3d4fb029b3a5cee8694e78771e11c3af9ff2a6a7..26f54eaa6654e27b01a5462e83379ae544c3525a 100644 (file)
--- a/src/auth/auth-worker-client.c
+++ b/src/auth/auth-worker-client.c
@@ -81,6 +81,14 @@ add_userdb_replies(struct auth_stream_reply *reply,
        }
 }
 
+static void auth_worker_send_reply(struct auth_worker_client *client,
+                                  string_t *str)
+{
+       if (shutdown_request)
+               o_stream_send_str(client->output, "SHUTDOWN\n");
+       o_stream_send(client->output, str_data(str), str_len(str));
+}
+
 static void verify_plain_callback(enum passdb_result result,
                                  struct auth_request *request)
 {
@@ -118,7 +126,7 @@ static void verify_plain_callback(enum passdb_result result,
        }
        str = auth_stream_reply_get_str(reply);
        str_append_c(str, '\n');
-       o_stream_send(client->output, str_data(str), str_len(str));
+       auth_worker_send_reply(client, str);
 
        auth_request_unref(&request);
        auth_worker_client_check_throttle(client);
@@ -217,7 +225,7 @@ lookup_credentials_callback(enum passdb_result result,
        }
        str = auth_stream_reply_get_str(reply);
        str_append_c(str, '\n');
-       o_stream_send(client->output, str_data(str), str_len(str));
+       auth_worker_send_reply(client, str);
 
        auth_request_unref(&request);
        auth_worker_client_check_throttle(client);
@@ -282,7 +290,7 @@ set_credentials_callback(bool success, struct auth_request *request)
 
        str = t_str_new(64);
        str_printfa(str, "%u\t%s\n", request->id, success ? "OK" : "FAIL");
-       o_stream_send(client->output, str_data(str), str_len(str));
+       auth_worker_send_reply(client, str);
 
        auth_request_unref(&request);
        auth_worker_client_check_throttle(client);
@@ -357,7 +365,7 @@ lookup_user_callback(enum userdb_result result,
        }
        str_append_c(str, '\n');
 
-       o_stream_send(client->output, str_data(str), str_len(str));
+       auth_worker_send_reply(client, str);
 
        auth_request_unref(&auth_request);
        auth_worker_client_check_throttle(client);

diff --git a/src/auth/auth-worker-server.c b/src/auth/auth-worker-server.c
index c04031685999447e02a40b73d83e6abee3dc89e6..f52fffaa36af8dbbdf01a139568c28f3027151da 100644 (file)
--- a/src/auth/auth-worker-server.c
+++ b/src/auth/auth-worker-server.c
@@ -35,16 +35,15 @@ struct auth_worker_connection {
        struct ostream *output;
        struct timeout *to;
 
+       struct auth_worker_request *request;
        unsigned int id_counter;
-        struct auth_worker_request *request;
 
-       unsigned int requests_left;
+       unsigned int shutdown:1;
 };
 
 static ARRAY_DEFINE(connections, struct auth_worker_connection *) = ARRAY_INIT;
 static unsigned int idle_count;
 static unsigned int auth_workers_max;
-static unsigned int auth_workers_max_request_count;
 
 static ARRAY_DEFINE(worker_request_array, struct auth_worker_request *);
 static struct aqueue *worker_request_queue;
@@ -78,8 +77,6 @@ static void auth_worker_request_send(struct auth_worker_connection *conn,
 {
        struct const_iovec iov[3];
 
-       i_assert(conn->requests_left > 0);
-
        if (ioloop_time - request->created > AUTH_WORKER_DELAY_WARN_SECS &&
            ioloop_time - auth_worker_last_warn >
            AUTH_WORKER_DELAY_WARN_MIN_INTERVAL_SECS) {
@@ -102,7 +99,6 @@ static void auth_worker_request_send(struct auth_worker_connection *conn,
        o_stream_sendv(conn->output, iov, 3);
 
        conn->request = request;
-       conn->requests_left--;
 
        timeout_remove(&conn->to);
        conn->to = timeout_add(AUTH_WORKER_LOOKUP_TIMEOUT_SECS * 1000,
@@ -163,7 +159,6 @@ static struct auth_worker_connection *auth_worker_create(void)
                                         FALSE);
        conn->output = o_stream_create_fd(fd, (size_t)-1, FALSE);
        conn->io = io_add(fd, IO_READ, worker_input, conn);
-       conn->requests_left = auth_workers_max_request_count;
        conn->to = timeout_add(AUTH_WORKER_MAX_IDLE_SECS * 1000,
                               auth_worker_idle_timeout, conn);
 
@@ -272,6 +267,10 @@ static void worker_input(struct auth_worker_connection *conn)
        }
 
        while ((line = i_stream_next_line(conn->input)) != NULL) {
+               if (strcmp(line, "SHUTDOWN") == 0) {
+                       conn->shutdown = TRUE;
+                       continue;
+               }
                id_str = line;
                line = strchr(line, '\t');
                if (line == NULL)
@@ -295,7 +294,7 @@ static void worker_input(struct auth_worker_connection *conn)
                }
        }
 
-       if (conn->requests_left == 0)
+       if (conn->shutdown && conn->request == NULL)
                auth_worker_destroy(&conn, "Max requests limit", TRUE);
        else
                auth_worker_request_send_next(conn);
@@ -354,13 +353,6 @@ void auth_worker_server_init(void)
                i_fatal("AUTH_WORKER_MAX_COUNT environment not set");
        auth_workers_max = atoi(env);
 
-       env = getenv("AUTH_WORKER_MAX_REQUEST_COUNT");
-       if (env == NULL)
-               i_fatal("AUTH_WORKER_MAX_REQUEST_COUNT environment not set");
-       auth_workers_max_request_count = atoi(env);
-       if (auth_workers_max_request_count == 0)
-               auth_workers_max_request_count = (unsigned int)-1;
-
        i_array_init(&worker_request_array, 128);
        worker_request_queue = aqueue_init(&worker_request_array.arr);
 

diff --git a/src/auth/common.h b/src/auth/common.h
index 2b7fae01c3ca686119d42793de435c4fddbbc111..3ca08837b84ba1322fac78e1adf058bdb321ad27 100644 (file)
--- a/src/auth/common.h
+++ b/src/auth/common.h
@@ -9,7 +9,7 @@
 #define WORKER_SERVER_FD 4
 
 extern struct ioloop *ioloop;
-extern bool standalone, worker;
+extern bool standalone, worker, shutdown_request;
 extern time_t process_start_time;
 
 #endif

diff --git a/src/auth/main.c b/src/auth/main.c
index 5281f5319f0a9115c5187f24b8f126718e0e4b7a..5bb6b88d27dfaa47dd60d29b27abda8c6b8dc04f 100644 (file)
--- a/src/auth/main.c
+++ b/src/auth/main.c
@@ -31,7 +31,7 @@
 #include <sys/stat.h>
 
 struct ioloop *ioloop;
-bool standalone = FALSE, worker = FALSE;
+bool standalone = FALSE, worker = FALSE, shutdown_request = FALSE;
 time_t process_start_time;
 
 static struct module *modules = NULL;

diff --git a/src/auth/passdb-pam.c b/src/auth/passdb-pam.c
index 39787c248d227db582b350a688eda1179c7c3bcb..65dfcbd116adba81392f4916824bbd135e0c5056 100644 (file)
--- a/src/auth/passdb-pam.c
+++ b/src/auth/passdb-pam.c
@@ -42,10 +42,13 @@
 #endif
 typedef linux_const void *pam_item_t;
 
+#define PASSDB_PAM_DEFAULT_MAX_REQUESTS 100
+
 struct pam_passdb_module {
        struct passdb_module module;
 
        const char *service_name, *pam_cache_key;
+       unsigned int requests_left;
 
        unsigned int pam_setcred:1;
        unsigned int pam_session:1;
@@ -309,6 +312,11 @@ pam_verify_plain(struct auth_request *request, const char *password,
        string_t *expanded_service;
        const char *service;
 
+       if (module->requests_left > 0) {
+               if (--module->requests_left == 0)
+                       shutdown_request = TRUE;
+       }
+
        expanded_service = t_str_new(64);
        var_expand(expanded_service, module->service_name,
                   auth_request_get_var_expand_table(request, NULL));
@@ -333,6 +341,7 @@ pam_preinit(struct auth_passdb *auth_passdb, const char *args)
           given by the auth mechanism */
        module->module.default_pass_scheme = "PLAIN";
        module->module.blocking = TRUE;
+       module->requests_left = PASSDB_PAM_DEFAULT_MAX_REQUESTS;
 
        t_args = t_strsplit_spaces(args, " ");
        for(i = 0; t_args[i] != NULL; i++) {
@@ -353,6 +362,8 @@ pam_preinit(struct auth_passdb *auth_passdb, const char *args)
                } else if (strcmp(t_args[i], "*") == 0) {
                        /* for backwards compatibility */
                        module->service_name = "%Ls";
+               } else if (strncmp(t_args[i], "max_requests=", 13) == 0) {
+                       module->requests_left = atoi(t_args[i] + 13);
                } else if (t_args[i+1] == NULL) {
                        module->service_name =
                                p_strdup(auth_passdb->auth->pool, t_args[i]);

diff --git a/src/master/auth-process.c b/src/master/auth-process.c
index 597c425ef099ac5f879a099c390aa12c2e55eece..8d9be506885952db24bbb9abed0a6c8baff7515f 100644 (file)
--- a/src/master/auth-process.c
+++ b/src/master/auth-process.c
@@ -600,8 +600,6 @@ static int create_auth_process(struct auth_process_group *group)
                                dec2str(getpid())));
        env_put(t_strdup_printf("AUTH_WORKER_MAX_COUNT=%u",
                                group->set->worker_max_count));
-       env_put(t_strdup_printf("AUTH_WORKER_MAX_REQUEST_COUNT=%u",
-                               group->set->worker_max_request_count));
 
        /* make sure we don't leak syslog fd, but do it last so that
           any errors above will be logged */

diff --git a/src/master/master-settings.c b/src/master/master-settings.c
index 9f09f194c374f7564380c8a028cd3b87ebc95fb5..a2bc583230da93e0b925c89bf3a83a9db594db55 100644 (file)
--- a/src/master/master-settings.c
+++ b/src/master/master-settings.c
@@ -94,7 +94,6 @@ static struct setting_def auth_setting_defs[] = {
 
        DEF_INT(count),
        DEF_INT(worker_max_count),
-       DEF_INT(worker_max_request_count),
        DEF_INT(process_size),
 
        { 0, NULL, 0 }
@@ -332,7 +331,6 @@ struct auth_settings default_auth_settings = {
 
        MEMBER(count) 1,
        MEMBER(worker_max_count) 30,
-       MEMBER(worker_max_request_count) 0,
        MEMBER(process_size) 256,
 
        /* .. */

diff --git a/src/master/master-settings.h b/src/master/master-settings.h
index 7ef74c32b1acfcf7253c289750640e92a10f4130..17fdba39e624597764ccf096d0a38aa6e50681c5 100644 (file)
--- a/src/master/master-settings.h
+++ b/src/master/master-settings.h
@@ -221,7 +221,6 @@ struct auth_settings {
 
        unsigned int count;
        unsigned int worker_max_count;
-       unsigned int worker_max_request_count;
        unsigned int process_size;
 
        /* .. */