commit f3f2d74c590e7540e364dfa888065ad54f1a92f3 Author: Jacob Welsh AuthorDate: Fri Sep 8 02:38:27 2023 +0000 Commit: Jacob Welsh CommitDate: Fri Sep 8 02:45:42 2023 +0000 auth, anvil: remove ill-conceived and ill-functioning feature for penalizing failed logins with delayed responses. It's an open invitation to denial-of-service attack, especially when used behind a webmail or similar gateway, and the workarounds suggested for supporting that use case are variously broken and ridiculous. Some related pieces are not fully removed, search on "penalty" for details, but everything builds. http://jfxpt.com/2023/jwrd-logs-for-Sep-2023/#9198 diff --git a/src/Makefile b/src/Makefile index 0776828456..7573d8b387 100644 --- a/src/Makefile +++ b/src/Makefile @@ -140,7 +140,6 @@ LIBEXEC_PROGS = \ util/script-login \ TEST_PROGS = \ - anvil/test-penalty \ auth/test-libpassword \ auth/test-auth-cache \ auth/test-auth \ @@ -626,7 +625,6 @@ LIBDOVECOT_OBJS += \ auth/auth-client-connection.o \ auth/auth-fields.o \ auth/auth-master-connection.o \ - auth/auth-penalty.o \ auth/auth-policy.o \ auth/auth-request-fields.o \ auth/auth-request-handler.o \ @@ -1525,10 +1523,8 @@ anvil/anvil: \ anvil/anvil-connection.o \ anvil/anvil-settings.o \ anvil/connect-limit.o \ - anvil/main.o \ - anvil/penalty.o + anvil/main.o $(LINK) -anvil/test-penalty: anvil/test-penalty.o anvil/penalty.o auth/auth: auth/main.o $(LINK) diff --git a/src/anvil/anvil-connection.c b/src/anvil/anvil-connection.c index 20e859b5c3..69767add88 100644 --- a/src/anvil/anvil-connection.c +++ b/src/anvil/anvil-connection.c @@ -8,7 +8,6 @@ #include "master-service.h" #include "master-interface.h" #include "connect-limit.h" -#include "penalty.h" #include "anvil-connection.h" #include @@ -48,8 +47,7 @@ anvil_connection_request(struct anvil_connection *conn, const char *const *args, const char **error_r) { const char *cmd = args[0]; - unsigned int value, checksum; - time_t stamp; + unsigned int value; pid_t pid; args++; @@ -101,36 +99,9 @@ anvil_connection_request(struct anvil_connection *conn, value = connect_limit_lookup(connect_limit, args[0]); o_stream_nsend_str(conn->output, t_strdup_printf("%u\n", value)); - } else if (strcmp(cmd, "PENALTY-GET") == 0) { - if (args[0] == NULL) { - *error_r = "PENALTY-GET: Not enough parameters"; - return -1; - } - value = penalty_get(penalty, args[0], &stamp); - o_stream_nsend_str(conn->output, - t_strdup_printf("%u %s\n", value, dec2str(stamp))); - } else if (strcmp(cmd, "PENALTY-INC") == 0) { - if (args[0] == NULL || args[1] == NULL || args[2] == NULL) { - *error_r = "PENALTY-INC: Not enough parameters"; - return -1; - } - if (str_to_uint(args[1], &checksum) < 0 || - str_to_uint(args[2], &value) < 0 || - value > PENALTY_MAX_VALUE || - (value == 0 && checksum != 0)) { - *error_r = "PENALTY-INC: Invalid parameters"; - return -1; - } - penalty_inc(penalty, args[0], checksum, value); - } else if (strcmp(cmd, "PENALTY-SET-EXPIRE-SECS") == 0) { - if (args[0] == NULL || str_to_uint(args[0], &value) < 0) { - *error_r = "PENALTY-SET-EXPIRE-SECS: " - "Invalid parameters"; - return -1; - } - penalty_set_expire_secs(penalty, value); } else if (strcmp(cmd, "PENALTY-DUMP") == 0) { - penalty_dump(penalty, conn->output); + /* Penalty interface removed but 'doveadm penalty' could still query it, so return an empty result. We could perhaps stub out the doveadm command too. */ + o_stream_nsend(conn->output, "\n", 1); } else { *error_r = t_strconcat("Unknown command: ", cmd, NULL); return -1; diff --git a/src/anvil/anvil-settings.c b/src/anvil/anvil-settings.c index a94823e02f..51529ff449 100644 --- a/src/anvil/anvil-settings.c +++ b/src/anvil/anvil-settings.c @@ -10,11 +10,9 @@ /* */ static struct file_listener_settings anvil_unix_listeners_array[] = { { "anvil", 0600, "", "" }, - { "anvil-auth-penalty", 0600, "", "" } }; static struct file_listener_settings *anvil_unix_listeners[] = { - &anvil_unix_listeners_array[0], - &anvil_unix_listeners_array[1] + &anvil_unix_listeners_array[0] }; static buffer_t anvil_unix_listeners_buf = { { { anvil_unix_listeners, sizeof(anvil_unix_listeners) } } diff --git a/src/anvil/common.h b/src/anvil/common.h index f9a44bd576..4f3b3da1e1 100644 --- a/src/anvil/common.h +++ b/src/anvil/common.h @@ -4,7 +4,6 @@ #include "lib.h" extern struct connect_limit *connect_limit; -extern struct penalty *penalty; extern bool anvil_restarted; #endif diff --git a/src/anvil/main.c b/src/anvil/main.c index 7e4050bc59..8d7cf0cc2f 100644 --- a/src/anvil/main.c +++ b/src/anvil/main.c @@ -10,13 +10,11 @@ #include "master-service-settings.h" #include "master-interface.h" #include "connect-limit.h" -#include "penalty.h" #include "anvil-connection.h" #include struct connect_limit *connect_limit; -struct penalty *penalty; bool anvil_restarted; static struct io *log_fdpass_io; @@ -74,7 +72,6 @@ int main(int argc, char *argv[]) master_service_set_die_with_master(master_service, FALSE); connect_limit = connect_limit_init(); - penalty = penalty_init(); log_fdpass_io = io_add(MASTER_ANVIL_LOG_FDPASS_FD, IO_READ, log_fdpass_input, NULL); master_service_init_finish(master_service); @@ -82,7 +79,6 @@ int main(int argc, char *argv[]) master_service_run(master_service, client_connected); io_remove(&log_fdpass_io); - penalty_deinit(&penalty); connect_limit_deinit(&connect_limit); anvil_connections_destroy_all(); master_service_deinit(&master_service); diff --git a/src/anvil/penalty.c b/src/anvil/penalty.c deleted file mode 100644 index 2ab6da16aa..0000000000 --- a/src/anvil/penalty.c +++ /dev/null @@ -1,273 +0,0 @@ -/* Copyright (c) 2009-2018 Dovecot authors, see the included COPYING file */ - -/* The idea behind checksums is that the same username+password doesn't - increase the penalty, because it's most likely a user with a misconfigured - account. */ - -#include "lib.h" -#include "ioloop.h" -#include "hash.h" -#include "str.h" -#include "strescape.h" -#include "llist.h" -#include "ostream.h" -#include "penalty.h" - -#include - -#define PENALTY_DEFAULT_EXPIRE_SECS (60*60) -#define PENALTY_CHECKSUM_SAVE_COUNT -#define CHECKSUM_VALUE_COUNT 2 -#define CHECKSUM_VALUE_PTR_COUNT 10 - -#define LAST_UPDATE_BITS 15 - -struct penalty_rec { - /* ordered by last_update */ - struct penalty_rec *prev, *next; - - char *ident; - unsigned int last_penalty; - - unsigned int penalty:16; - unsigned int last_update:LAST_UPDATE_BITS; /* last_penalty + n */ - bool checksum_is_pointer:1; - /* we use value up to two different checksums. - after that switch to pointer. */ - union { - unsigned int value[CHECKSUM_VALUE_COUNT]; - unsigned int *value_ptr; - } checksum; -}; - -struct penalty { - /* ident => penalty_rec */ - HASH_TABLE(char *, struct penalty_rec *) hash; - struct penalty_rec *oldest, *newest; - - unsigned int expire_secs; - struct timeout *to; -}; - -struct penalty *penalty_init(void) -{ - struct penalty *penalty; - - penalty = i_new(struct penalty, 1); - hash_table_create(&penalty->hash, default_pool, 0, str_hash, strcmp); - penalty->expire_secs = PENALTY_DEFAULT_EXPIRE_SECS; - return penalty; -} - -static void penalty_rec_free(struct penalty *penalty, struct penalty_rec *rec) -{ - DLLIST2_REMOVE(&penalty->oldest, &penalty->newest, rec); - if (rec->checksum_is_pointer) - i_free(rec->checksum.value_ptr); - i_free(rec->ident); - i_free(rec); -} - -void penalty_deinit(struct penalty **_penalty) -{ - struct penalty *penalty = *_penalty; - - *_penalty = NULL; - - while (penalty->oldest != NULL) - penalty_rec_free(penalty, penalty->oldest); - hash_table_destroy(&penalty->hash); - - timeout_remove(&penalty->to); - i_free(penalty); -} - -void penalty_set_expire_secs(struct penalty *penalty, unsigned int expire_secs) -{ - penalty->expire_secs = expire_secs; -} - -static bool -penalty_bump_checksum(struct penalty_rec *rec, unsigned int checksum) -{ - unsigned int *checksums; - unsigned int i, count; - - if (!rec->checksum_is_pointer) { - checksums = rec->checksum.value; - count = CHECKSUM_VALUE_COUNT; - } else { - checksums = rec->checksum.value_ptr; - count = CHECKSUM_VALUE_PTR_COUNT; - } - - for (i = 0; i < count; i++) { - if (checksums[i] == checksum) { - if (i > 0) { - memmove(checksums + 1, checksums, - sizeof(checksums[0]) * i); - checksums[0] = checksum; - } - return TRUE; - } - } - return FALSE; -} - -static void penalty_add_checksum(struct penalty_rec *rec, unsigned int checksum) -{ - unsigned int *checksums; - - i_assert(checksum != 0); - - if (!rec->checksum_is_pointer) { - if (rec->checksum.value[CHECKSUM_VALUE_COUNT-1] == 0) { - memcpy(rec->checksum.value + 1, rec->checksum.value, - sizeof(rec->checksum.value[0]) * - (CHECKSUM_VALUE_COUNT-1)); - rec->checksum.value[0] = checksum; - return; - } - - /* switch to using a pointer */ - checksums = i_new(unsigned int, CHECKSUM_VALUE_PTR_COUNT); - memcpy(checksums, rec->checksum.value, - sizeof(checksums[0]) * CHECKSUM_VALUE_COUNT); - rec->checksum.value_ptr = checksums; - rec->checksum_is_pointer = TRUE; - } - - memmove(rec->checksum.value_ptr + 1, rec->checksum.value_ptr, - sizeof(rec->checksum.value_ptr[0]) * - (CHECKSUM_VALUE_PTR_COUNT-1)); - rec->checksum.value_ptr[0] = checksum; -} - -unsigned int penalty_get(struct penalty *penalty, const char *ident, - time_t *last_penalty_r) -{ - struct penalty_rec *rec; - - rec = hash_table_lookup(penalty->hash, ident); - if (rec == NULL) { - *last_penalty_r = 0; - return 0; - } - - *last_penalty_r = rec->last_penalty; - return rec->penalty; -} - -static void penalty_timeout(struct penalty *penalty) -{ - struct penalty_rec *rec; - time_t rec_last_update, expire_time; - unsigned int diff; - - timeout_remove(&penalty->to); - - expire_time = ioloop_time - penalty->expire_secs; - while (penalty->oldest != NULL) { - rec = penalty->oldest; - - rec_last_update = rec->last_penalty + rec->last_update; - if (rec_last_update > expire_time) { - diff = rec_last_update - expire_time; - penalty->to = timeout_add(diff * 1000, - penalty_timeout, penalty); - break; - } - hash_table_remove(penalty->hash, rec->ident); - penalty_rec_free(penalty, rec); - } -} - -void penalty_inc(struct penalty *penalty, const char *ident, - unsigned int checksum, unsigned int value) -{ - struct penalty_rec *rec; - time_t diff; - - i_assert(value > 0 || checksum == 0); - i_assert(value <= INT_MAX); - - rec = hash_table_lookup(penalty->hash, ident); - if (rec == NULL) { - rec = i_new(struct penalty_rec, 1); - rec->ident = i_strdup(ident); - hash_table_insert(penalty->hash, rec->ident, rec); - } else { - DLLIST2_REMOVE(&penalty->oldest, &penalty->newest, rec); - } - - if (checksum == 0) { - rec->penalty = value; - rec->last_penalty = ioloop_time; - } else { - if (penalty_bump_checksum(rec, checksum)) - rec->penalty = value - 1; - else { - penalty_add_checksum(rec, checksum); - rec->penalty = value; - rec->last_penalty = ioloop_time; - } - } - - diff = ioloop_time - rec->last_penalty; - if (diff >= (1 << LAST_UPDATE_BITS)) { - rec->last_update = (1 << LAST_UPDATE_BITS) - 1; - rec->last_penalty = ioloop_time - rec->last_update; - } else { - rec->last_update = diff; - } - - DLLIST2_APPEND(&penalty->oldest, &penalty->newest, rec); - - if (penalty->to == NULL) { - penalty->to = timeout_add(penalty->expire_secs * 1000, - penalty_timeout, penalty); - } -} - -bool penalty_has_checksum(struct penalty *penalty, const char *ident, - unsigned int checksum) -{ - struct penalty_rec *rec; - const unsigned int *checksums; - unsigned int i, count; - - rec = hash_table_lookup(penalty->hash, ident); - if (rec == NULL) - return FALSE; - - if (!rec->checksum_is_pointer) { - checksums = rec->checksum.value; - count = CHECKSUM_VALUE_COUNT; - } else { - checksums = rec->checksum.value_ptr; - count = CHECKSUM_VALUE_PTR_COUNT; - } - - for (i = 0; i < count; i++) { - if (checksums[i] == checksum) - return TRUE; - } - return FALSE; -} - -void penalty_dump(struct penalty *penalty, struct ostream *output) -{ - const struct penalty_rec *rec; - string_t *str = t_str_new(256); - - for (rec = penalty->oldest; rec != NULL; rec = rec->next) { - str_truncate(str, 0); - str_append_tabescaped(str, rec->ident); - str_printfa(str, "\t%u\t%u\t%u\n", - rec->penalty, rec->last_penalty, - rec->last_penalty + rec->last_update); - if (o_stream_send(output, str_data(str), str_len(str)) < 0) - break; - } - o_stream_nsend(output, "\n", 1); -} diff --git a/src/anvil/penalty.h b/src/anvil/penalty.h deleted file mode 100644 index 23a182cde4..0000000000 --- a/src/anvil/penalty.h +++ /dev/null @@ -1,22 +0,0 @@ -#ifndef PENALTY_H -#define PENALTY_H - -#define PENALTY_MAX_VALUE ((1 << 16)-1) - -struct penalty *penalty_init(void); -void penalty_deinit(struct penalty **penalty); - -void penalty_set_expire_secs(struct penalty *penalty, unsigned int expire_secs); - -unsigned int penalty_get(struct penalty *penalty, const char *ident, - time_t *last_penalty_r); -/* if checksum is non-zero and it already exists for ident, the value - is set to "value-1", otherwise it's set to "value". */ -void penalty_inc(struct penalty *penalty, const char *ident, - unsigned int checksum, unsigned int value); - -bool penalty_has_checksum(struct penalty *penalty, const char *ident, - unsigned int checksum); -void penalty_dump(struct penalty *penalty, struct ostream *output); - -#endif diff --git a/src/anvil/test-penalty.c b/src/anvil/test-penalty.c deleted file mode 100644 index 438bf9eb0a..0000000000 --- a/src/anvil/test-penalty.c +++ /dev/null @@ -1,64 +0,0 @@ -/* Copyright (c) 2010-2018 Dovecot authors, see the included COPYING file */ - -#include "lib.h" -#include "ioloop.h" -#include "penalty.h" -#include "test-common.h" - -static void test_penalty_checksum(void) -{ - struct penalty *penalty; - struct ioloop *ioloop; - time_t t; - unsigned int i, j; - - test_begin("penalty"); - - ioloop = io_loop_create(); - penalty = penalty_init(); - - test_assert(penalty_get(penalty, "foo", &t) == 0); - for (i = 1; i <= 10; i++) { - ioloop_time = 12345678 + i; - penalty_inc(penalty, "foo", i, 5+i); - - for (j = I_MIN(1, i-1); j <= i; j++) { - test_assert(penalty_get(penalty, "foo", &t) == 5+i); - test_assert(t == (time_t)(12345678 + i)); - test_assert(penalty_has_checksum(penalty, "foo", i)); - } - test_assert(penalty_get(penalty, "foo", &t) == 5+i); - test_assert(t == (time_t)(12345678 + i)); - test_assert(!penalty_has_checksum(penalty, "foo", j)); - } - test_assert(penalty_get(penalty, "foo2", &t) == 0); - - /* overflows checksum array */ - ioloop_time = 12345678 + i; - penalty_inc(penalty, "foo", i, 5 + i); - penalty_inc(penalty, "foo", i, 5 + i); - penalty_inc(penalty, "foo", 0, 5 + i); - - test_assert(penalty_get(penalty, "foo", &t) == 5+i); - test_assert(t == (time_t)(12345678 + i)); - test_assert(!penalty_has_checksum(penalty, "foo", 1)); - - for (j = 2; j <= i; j++) { - test_assert(penalty_get(penalty, "foo", &t) == 5+i); - test_assert(t == (time_t)(12345678 + i)); - test_assert(penalty_has_checksum(penalty, "foo", i)); - } - - penalty_deinit(&penalty); - io_loop_destroy(&ioloop); - test_end(); -} - -int main(void) -{ - static void (*const test_functions[])(void) = { - test_penalty_checksum, - NULL - }; - return test_run(test_functions); -} diff --git a/src/auth/auth-common.h b/src/auth/auth-common.h index 5ebe8c489a..75385594b0 100644 --- a/src/auth/auth-common.h +++ b/src/auth/auth-common.h @@ -6,7 +6,6 @@ extern bool worker, worker_restart_request; extern time_t process_start_time; -extern struct auth_penalty *auth_penalty; extern struct event_category event_category_auth; extern struct event *auth_event; diff --git a/src/auth/auth-penalty.c b/src/auth/auth-penalty.c deleted file mode 100644 index 3816902a0a..0000000000 --- a/src/auth/auth-penalty.c +++ /dev/null @@ -1,176 +0,0 @@ -/* Copyright (c) 2009-2018 Dovecot authors, see the included COPYING file */ - -#include "lib.h" -#include "ioloop.h" -#include "net.h" -#include "crc32.h" -#include "master-service.h" -#include "anvil-client.h" -#include "auth-request.h" -#include "auth-penalty.h" - -#include - -/* We don't want IPv6 hosts being able to flood our penalty - tracking with tons of different IPs. */ -#define PENALTY_IPV6_MASK_BITS 48 - -struct auth_penalty_request { - struct auth_request *auth_request; - struct anvil_client *client; - auth_penalty_callback_t *callback; -}; - -struct auth_penalty { - struct anvil_client *client; - - bool disabled:1; -}; - -struct auth_penalty *auth_penalty_init(const char *path) -{ - struct auth_penalty *penalty; - - penalty = i_new(struct auth_penalty, 1); - penalty->client = anvil_client_init(path, NULL, - ANVIL_CLIENT_FLAG_HIDE_ENOENT); - if (anvil_client_connect(penalty->client, TRUE) < 0) - penalty->disabled = TRUE; - else { - anvil_client_cmd(penalty->client, t_strdup_printf( - "PENALTY-SET-EXPIRE-SECS\t%u", AUTH_PENALTY_TIMEOUT)); - } - return penalty; -} - -void auth_penalty_deinit(struct auth_penalty **_penalty) -{ - struct auth_penalty *penalty = *_penalty; - - *_penalty = NULL; - anvil_client_deinit(&penalty->client); - i_free(penalty); -} - -unsigned int auth_penalty_to_secs(unsigned int penalty) -{ - unsigned int i, secs = AUTH_PENALTY_INIT_SECS; - - for (i = 0; i < penalty; i++) - secs *= 2; - return secs < AUTH_PENALTY_MAX_SECS ? secs : AUTH_PENALTY_MAX_SECS; -} - -static void auth_penalty_anvil_callback(const char *reply, void *context) -{ - struct auth_penalty_request *request = context; - unsigned int penalty = 0; - unsigned long last_penalty = 0; - unsigned int secs, drop_penalty; - - if (reply == NULL) { - /* internal failure. */ - if (!anvil_client_is_connected(request->client)) { - /* we probably didn't have permissions to reconnect - back to anvil. need to restart ourself. */ - master_service_stop(master_service); - } - } else if (sscanf(reply, "%u %lu", &penalty, &last_penalty) != 2) { - e_error(request->auth_request->event, - "Invalid PENALTY-GET reply: %s", reply); - } else { - if ((time_t)last_penalty > ioloop_time) { - /* time moved backwards? */ - last_penalty = ioloop_time; - } - - /* update penalty. */ - drop_penalty = AUTH_PENALTY_MAX_PENALTY; - while (penalty > 0) { - secs = auth_penalty_to_secs(drop_penalty); - if (ioloop_time - last_penalty < secs) - break; - drop_penalty--; - penalty--; - } - } - - request->callback(penalty, request->auth_request); - auth_request_unref(&request->auth_request); - i_free(request); -} - -static const char * -auth_penalty_get_ident(struct auth_request *auth_request) -{ - struct ip_addr ip; - - ip = auth_request->fields.remote_ip; - if (IPADDR_IS_V6(&ip)) { - memset(ip.u.ip6.s6_addr + PENALTY_IPV6_MASK_BITS/CHAR_BIT, 0, - sizeof(ip.u.ip6.s6_addr) - - PENALTY_IPV6_MASK_BITS/CHAR_BIT); - } - return net_ip2addr(&ip); -} - -void auth_penalty_lookup(struct auth_penalty *penalty, - struct auth_request *auth_request, - auth_penalty_callback_t *callback) -{ - struct auth_penalty_request *request; - const char *ident; - - ident = auth_penalty_get_ident(auth_request); - if (penalty->disabled || ident == NULL || - auth_request->fields.no_penalty) { - callback(0, auth_request); - return; - } - - request = i_new(struct auth_penalty_request, 1); - request->auth_request = auth_request; - request->client = penalty->client; - request->callback = callback; - auth_request_ref(auth_request); - - T_BEGIN { - anvil_client_query(penalty->client, - t_strdup_printf("PENALTY-GET\t%s", ident), - auth_penalty_anvil_callback, request); - } T_END; -} - -static unsigned int -get_userpass_checksum(struct auth_request *auth_request) -{ - return auth_request->mech_password == NULL ? 0 : - crc32_str_more(crc32_str(auth_request->mech_password), - auth_request->fields.user); -} - -void auth_penalty_update(struct auth_penalty *penalty, - struct auth_request *auth_request, unsigned int value) -{ - const char *ident; - - ident = auth_penalty_get_ident(auth_request); - if (penalty->disabled || ident == NULL || - auth_request->fields.no_penalty) - return; - - if (value > AUTH_PENALTY_MAX_PENALTY) { - /* even if the actual value doesn't change, the last_change - timestamp does. */ - value = AUTH_PENALTY_MAX_PENALTY; - } - T_BEGIN { - const char *cmd; - unsigned int checksum; - - checksum = value == 0 ? 0 : get_userpass_checksum(auth_request); - cmd = t_strdup_printf("PENALTY-INC\t%s\t%u\t%u", - ident, checksum, value); - anvil_client_cmd(penalty->client, cmd); - } T_END; -} diff --git a/src/auth/auth-penalty.h b/src/auth/auth-penalty.h deleted file mode 100644 index 96783e4f58..0000000000 --- a/src/auth/auth-penalty.h +++ /dev/null @@ -1,28 +0,0 @@ -#ifndef AUTH_PENALTY_H -#define AUTH_PENALTY_H - -struct auth_request; - -#define AUTH_PENALTY_INIT_SECS 2 -#define AUTH_PENALTY_MAX_SECS 15 -/* timeout specifies how long it takes for penalty to be irrelevant. */ -#define AUTH_PENALTY_TIMEOUT \ - (AUTH_PENALTY_INIT_SECS + 4 + 8 + AUTH_PENALTY_MAX_SECS) -#define AUTH_PENALTY_MAX_PENALTY 4 - -/* If lookup failed, penalty and last_update are both zero */ -typedef void auth_penalty_callback_t(unsigned int penalty, - struct auth_request *request); - -struct auth_penalty *auth_penalty_init(const char *path); -void auth_penalty_deinit(struct auth_penalty **penalty); - -unsigned int auth_penalty_to_secs(unsigned int penalty); - -void auth_penalty_lookup(struct auth_penalty *penalty, - struct auth_request *auth_request, - auth_penalty_callback_t *callback); -void auth_penalty_update(struct auth_penalty *penalty, - struct auth_request *auth_request, unsigned int value); - -#endif diff --git a/src/auth/auth-policy.c b/src/auth/auth-policy.c index 951f85e6f8..de70b533ab 100644 --- a/src/auth/auth-policy.c +++ b/src/auth/auth-policy.c @@ -14,7 +14,6 @@ #include "master-service.h" #include "master-service-ssl-settings.h" #include "auth-request.h" -#include "auth-penalty.h" #include "auth-settings.h" #include "auth-policy.h" #include "auth-common.h" diff --git a/src/auth/auth-request-handler.c b/src/auth/auth-request-handler.c index d4bf53c276..646766633c 100644 --- a/src/auth/auth-request-handler.c +++ b/src/auth/auth-request-handler.c @@ -2,8 +2,6 @@ #include "auth-common.h" #include "ioloop.h" -#include "array.h" -#include "aqueue.h" #include "base64.h" #include "hash.h" #include "net.h" @@ -11,7 +9,6 @@ #include "strescape.h" #include "str-sanitize.h" #include "master-interface.h" -#include "auth-penalty.h" #include "auth-request.h" #include "auth-token.h" #include "auth-client-connection.h" @@ -20,14 +17,6 @@ #include "auth-request-handler-private.h" #include "auth-policy.h" -#define AUTH_FAILURE_DELAY_CHECK_MSECS 500 -static ARRAY(struct auth_request *) auth_failures_arr; -static struct aqueue *auth_failures; -static struct timeout *to_auth_failures; - -static void auth_failure_timeout(void *context) ATTR_NULL(1); - - static void auth_request_handler_default_reply_callback(struct auth_request *request, enum auth_client_result result, @@ -221,12 +210,6 @@ auth_request_handle_failure(struct auth_request *request, const char *reply) /* handle failure here */ auth_request_log_finished(request); - if (request->in_delayed_failure_queue) { - /* we came here from flush_failures() */ - handler->callback(reply, handler->conn); - return; - } - /* remove the request from requests-list */ auth_request_ref(request); auth_request_handler_remove(handler, request); @@ -234,30 +217,8 @@ auth_request_handle_failure(struct auth_request *request, const char *reply) if (request->set->policy_report_after_auth) auth_policy_report(request); - if (auth_fields_exists(request->fields.extra_fields, "nodelay")) { - /* passdb specifically requested not to delay the reply. */ - handler->callback(reply, handler->conn); - auth_request_unref(&request); - return; - } - - /* failure. don't announce it immediately to avoid - a) timing attacks, b) flooding */ - request->in_delayed_failure_queue = TRUE; - handler->refcount++; - - if (auth_penalty != NULL) { - auth_penalty_update(auth_penalty, request, - request->last_penalty + 1); - } - - auth_request_refresh_last_access(request); - aqueue_append(auth_failures, &request); - if (to_auth_failures == NULL) { - to_auth_failures = - timeout_add_short(AUTH_FAILURE_DELAY_CHECK_MSECS, - auth_failure_timeout, NULL); - } + handler->callback(reply, handler->conn); + auth_request_unref(&request); } static void @@ -268,11 +229,6 @@ auth_request_handler_reply_success_finish(struct auth_request *request) auth_request_log_finished(request); - if (request->last_penalty != 0 && auth_penalty != NULL) { - /* reset penalty */ - auth_penalty_update(auth_penalty, request, 0); - } - /* sanitize these fields, since the login code currently assumes they are exactly in this format. */ auth_fields_booleanize(request->fields.extra_fields, "nologin"); @@ -336,11 +292,6 @@ auth_request_handler_reply_failure_finish(struct auth_request *request) } } - if (auth_fields_exists(request->fields.extra_fields, "nodelay")) { - /* this is normally a hidden field, need to add it explicitly */ - str_append(str, "\tnodelay"); - } - if (code != NULL) { str_append(str, "\tcode="); str_append(str, code); @@ -368,7 +319,6 @@ void auth_request_handler_reply(struct auth_request *request, { struct auth_request_handler *handler = request->handler; - request->handler_pending_reply = FALSE; handler->reply_callback(request, result, auth_reply, reply_size); } @@ -442,15 +392,6 @@ auth_request_handler_default_reply_continue(struct auth_request *request, reply, reply_size); } -void auth_request_handler_abort(struct auth_request *request) -{ - i_assert(request->handler_pending_reply); - - /* request destroyed while waiting for auth_request_penalty_finish() - to be called. */ - auth_request_handler_unref(&request->handler); -} - static void auth_request_handler_auth_fail_code(struct auth_request_handler *handler, struct auth_request *request, @@ -497,29 +438,6 @@ static void auth_request_timeout(struct auth_request *request) auth_request_handler_remove(request->handler, request); } -static void auth_request_penalty_finish(struct auth_request *request) -{ - timeout_remove(&request->to_penalty); - auth_request_initial(request); -} - -static void -auth_penalty_callback(unsigned int penalty, struct auth_request *request) -{ - unsigned int secs; - - request->last_penalty = penalty; - - if (penalty == 0) - auth_request_initial(request); - else { - secs = auth_penalty_to_secs(penalty); - request->to_penalty = timeout_add(secs * 1000, - auth_request_penalty_finish, - request); - } -} - bool auth_request_handler_auth_begin(struct auth_request_handler *handler, const char *args) { @@ -683,10 +601,8 @@ bool auth_request_handler_auth_begin(struct auth_request_handler *handler, /* handler is referenced until auth_request_handler_reply() is called. */ handler->refcount++; - request->handler_pending_reply = TRUE; - /* before we start authenticating, see if we need to wait first */ - auth_penalty_lookup(auth_penalty, request, auth_penalty_callback); + auth_request_initial(request); return TRUE; } @@ -911,75 +827,3 @@ void auth_request_handler_cancel_request(struct auth_request_handler *handler, if (request != NULL) auth_request_handler_remove(handler, request); } - -void auth_request_handler_flush_failures(bool flush_all) -{ - struct auth_request **auth_requests, *auth_request; - unsigned int i, j, count; - time_t diff; - - count = aqueue_count(auth_failures); - if (count == 0) { - timeout_remove(&to_auth_failures); - return; - } - - auth_requests = array_front_modifiable(&auth_failures_arr); - /* count the number of requests that we need to flush */ - for (i = 0; i < count; i++) { - auth_request = auth_requests[aqueue_idx(auth_failures, i)]; - - /* FIXME: assumes that failure_delay is always the same. */ - diff = ioloop_time - auth_request->last_access; - if (diff < (time_t)auth_request->set->failure_delay && - !flush_all) - break; - } - - /* shuffle these requests to try to prevent any kind of timing attacks - where attacker performs multiple requests in parallel and attempts - to figure out results based on the order of replies. */ - count = i; - for (i = 0; i < count; i++) { - j = random() % (count - i) + i; - auth_request = auth_requests[aqueue_idx(auth_failures, i)]; - - /* swap i & j */ - auth_requests[aqueue_idx(auth_failures, i)] = - auth_requests[aqueue_idx(auth_failures, j)]; - auth_requests[aqueue_idx(auth_failures, j)] = auth_request; - } - - /* flush the requests */ - for (i = 0; i < count; i++) { - auth_request = auth_requests[aqueue_idx(auth_failures, 0)]; - aqueue_delete_tail(auth_failures); - - i_assert(auth_request != NULL); - i_assert(auth_request->state == AUTH_REQUEST_STATE_FINISHED); - auth_request_handler_reply(auth_request, - AUTH_CLIENT_RESULT_FAILURE, - uchar_empty_ptr, 0); - auth_request_unref(&auth_request); - } -} - -static void auth_failure_timeout(void *context ATTR_UNUSED) -{ - auth_request_handler_flush_failures(FALSE); -} - -void auth_request_handler_init(void) -{ - i_array_init(&auth_failures_arr, 128); - auth_failures = aqueue_init(&auth_failures_arr.arr); -} - -void auth_request_handler_deinit(void) -{ - auth_request_handler_flush_failures(TRUE); - array_free(&auth_failures_arr); - aqueue_deinit(&auth_failures); - - timeout_remove(&to_auth_failures); -} diff --git a/src/auth/auth-request-handler.h b/src/auth/auth-request-handler.h index ceba9356c5..780e8d236a 100644 --- a/src/auth/auth-request-handler.h +++ b/src/auth/auth-request-handler.h @@ -50,7 +50,6 @@ void auth_request_handler_reply(struct auth_request *request, const void *reply, size_t reply_size); void auth_request_handler_reply_continue(struct auth_request *request, const void *reply, size_t reply_size); -void auth_request_handler_abort(struct auth_request *request); unsigned int auth_request_handler_get_request_count(struct auth_request_handler *handler); @@ -61,9 +60,4 @@ bool auth_request_handler_master_request(struct auth_request_handler *handler, void auth_request_handler_cancel_request(struct auth_request_handler *handler, unsigned int client_id); -void auth_request_handler_flush_failures(bool flush_all); - -void auth_request_handler_init(void); -void auth_request_handler_deinit(void); - #endif diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c index ee89e75308..7aa5f97022 100644 --- a/src/auth/auth-request.c +++ b/src/auth/auth-request.c @@ -331,9 +331,6 @@ void auth_request_unref(struct auth_request **_request) i_assert(array_count(&request->authdb_event) == 0); - if (request->handler_pending_reply) - auth_request_handler_abort(request); - event_unref(&request->mech_event); event_unref(&request->event); auth_request_stats_send(request); diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h index 79cf76b585..9dba941646 100644 --- a/src/auth/auth-request.h +++ b/src/auth/auth-request.h @@ -139,7 +139,6 @@ struct auth_request { struct timeout *to_abort, *to_penalty; unsigned int policy_penalty; - unsigned int last_penalty; size_t initial_response_len; const unsigned char *initial_response; @@ -185,10 +184,8 @@ struct auth_request { bool userdbs_seen_internal_failure:1; /* current state: */ - bool handler_pending_reply:1; bool accept_cont_input:1; bool prefer_plain_credentials:1; - bool in_delayed_failure_queue:1; bool removed_from_handler:1; bool snapshot_have_userdb_prefetch_set:1; /* username was changed by this passdb/userdb lookup. Used by diff --git a/src/auth/auth.h b/src/auth/auth.h index 3ca5a9bb12..aab6f07bd2 100644 --- a/src/auth/auth.h +++ b/src/auth/auth.h @@ -76,8 +76,6 @@ struct auth { struct auth_userdb *userdbs; }; -extern struct auth_penalty *auth_penalty; - struct auth *auth_find_service(const char *name); struct auth *auth_default_service(void); diff --git a/src/auth/main.c b/src/auth/main.c index de4a8263cc..b2ec7f81ea 100644 --- a/src/auth/main.c +++ b/src/auth/main.c @@ -22,7 +22,6 @@ #include "otp.h" #include "mech-otp-common.h" #include "auth.h" -#include "auth-penalty.h" #include "auth-token.h" #include "auth-request-handler.h" #include "auth-request-stats.h" @@ -35,8 +34,6 @@ #include #include -#define AUTH_PENALTY_ANVIL_PATH "anvil-auth-penalty" - enum auth_socket_type { AUTH_SOCKET_UNKNOWN = 0, AUTH_SOCKET_CLIENT, @@ -55,7 +52,6 @@ struct auth_socket_listener { bool worker = FALSE, worker_restart_request = FALSE; time_t process_start_time; -struct auth_penalty *auth_penalty; static pool_t auth_set_pool; static struct module *modules = NULL; @@ -172,8 +168,6 @@ static void main_preinit(void) services = read_global_settings(); - if (!worker) - auth_penalty = auth_penalty_init(AUTH_PENALTY_ANVIL_PATH); auth_request_stats_init(); mech_init(global_auth_settings); mech_reg = mech_register_init(global_auth_settings); @@ -211,7 +205,6 @@ static void main_init(void) child_wait_init(); auth_worker_server_init(); auths_init(); - auth_request_handler_init(); auth_policy_init(); if (worker) { @@ -232,16 +225,10 @@ static void main_deinit(void) { struct auth_socket_listener *l; - if (auth_penalty != NULL) { - /* cancel all pending anvil penalty lookups */ - auth_penalty_deinit(&auth_penalty); - } /* deinit auth workers, which aborts pending requests */ auth_worker_server_deinit(); /* deinit passdbs and userdbs. it aborts any pending async requests. */ auths_deinit(); - /* flush pending requests */ - auth_request_handler_deinit(); /* there are no more auth requests */ auths_free(); dict_drivers_unregister_builtin(); diff --git a/src/auth/test-mech.c b/src/auth/test-mech.c index 265ed37cf3..ad77458e1c 100644 --- a/src/auth/test-mech.c +++ b/src/auth/test-mech.c @@ -145,6 +145,7 @@ static void test_mech_prepare_request(struct auth_request **request_r, request->userdb = auth->userdbs; handler->refcount = 1; + /* nodelay is no longer meaningful (we no longer indulge in the rude security theatrics of delayed reporting of login failure), but it might as well stay in the test code since it might be used by existing systems. */ auth_fields_add(request->fields.extra_fields, "nodelay", "", 0); auth_request_ref(request); auth_request_state_count[AUTH_REQUEST_STATE_NEW] = 1; diff --git a/src/auth/test-mock.c b/src/auth/test-mock.c index 9584912f35..cdfe91c30c 100644 --- a/src/auth/test-mock.c +++ b/src/auth/test-mock.c @@ -4,7 +4,6 @@ #include "auth-common.h" #include "passdb.h" -struct auth_penalty *auth_penalty; time_t process_start_time; bool worker, worker_restart_request; static struct passdb_module *mock_passdb_mod = NULL;