From: Timo Sirainen Date: Tue, 16 Dec 2008 06:06:56 +0000 (+0200) Subject: Replaced auth_worker_max_request_count setting with passdb pam { args = max_requests=n } X-Git-Tag: 1.2.beta1~183 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=798cfe56c9871262770384da1239162b3800cce1;p=thirdparty%2Fdovecot%2Fcore.git 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 9b1e650e22..1a6a36a2e8 100644 --- 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. passdb pam { - # [session=yes] [setcred=yes] [failure_show_msg=yes] + # [session=yes] [setcred=yes] [failure_show_msg=yes] [max_requests=] # [cache_key=] [] # # 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 3d4fb029b3..26f54eaa66 100644 --- 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 c040316859..f52fffaa36 100644 --- 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 2b7fae01c3..3ca08837b8 100644 --- 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 5281f5319f..5bb6b88d27 100644 --- a/src/auth/main.c +++ b/src/auth/main.c @@ -31,7 +31,7 @@ #include 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 39787c248d..65dfcbd116 100644 --- 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 597c425ef0..8d9be50688 100644 --- 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 9f09f194c3..a2bc583230 100644 --- 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 7ef74c32b1..17fdba39e6 100644 --- 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; /* .. */