43899226b2
An issue was discovered in the auth component in Dovecot 2.2 and 2.3 before 2.3.20. When two passdb configuration entries exist with the same driver and args settings, incorrect username_filter and mechanism settings can be applied to passdb definitions. These incorrectly applied settings can lead to an unintended security configuration and can permit privilege escalation in certain configurations. The documentation does not advise against the use of passdb definitions that have the same driver and args settings. One such configuration would be where an administrator wishes to use the same PAM configuration or passwd file for both normal and master users but use the username_filter setting to restrict which of the users is able to be a master user. https://dovecot.org/pipermail/dovecot-news/2022-July/000477.html Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
137 lines
4.9 KiB
Diff
137 lines
4.9 KiB
Diff
From 7bad6a24160e34bce8f10e73dbbf9e5fbbcd1904 Mon Sep 17 00:00:00 2001
|
|
From: Timo Sirainen <timo.sirainen@open-xchange.com>
|
|
Date: Mon, 9 May 2022 15:23:33 +0300
|
|
Subject: [PATCH] auth: Fix handling passdbs with identical driver/args but
|
|
different mechanisms/username_filter
|
|
|
|
The passdb was wrongly deduplicated in this situation, causing wrong
|
|
mechanisms or username_filter setting to be used. This would be a rather
|
|
unlikely configuration though.
|
|
|
|
Fixed by moving mechanisms and username_filter from struct passdb_module
|
|
to struct auth_passdb, which is where they should have been in the first
|
|
place.
|
|
|
|
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
|
|
---
|
|
src/auth/auth-request.c | 6 +++---
|
|
src/auth/auth.c | 18 ++++++++++++++++++
|
|
src/auth/auth.h | 5 +++++
|
|
src/auth/passdb.c | 15 ++-------------
|
|
src/auth/passdb.h | 4 ----
|
|
5 files changed, 28 insertions(+), 20 deletions(-)
|
|
|
|
diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c
|
|
index cd08b1fa02..0ca29f3674 100644
|
|
--- a/src/auth/auth-request.c
|
|
+++ b/src/auth/auth-request.c
|
|
@@ -534,8 +534,8 @@ auth_request_want_skip_passdb(struct auth_request *request,
|
|
struct auth_passdb *passdb)
|
|
{
|
|
/* if mechanism is not supported, skip */
|
|
- const char *const *mechs = passdb->passdb->mechanisms;
|
|
- const char *const *username_filter = passdb->passdb->username_filter;
|
|
+ const char *const *mechs = passdb->mechanisms;
|
|
+ const char *const *username_filter = passdb->username_filter;
|
|
const char *username;
|
|
|
|
username = request->fields.user;
|
|
@@ -548,7 +548,7 @@ auth_request_want_skip_passdb(struct auth_request *request,
|
|
return TRUE;
|
|
}
|
|
|
|
- if (passdb->passdb->username_filter != NULL &&
|
|
+ if (passdb->username_filter != NULL &&
|
|
!auth_request_username_accepted(username_filter, username)) {
|
|
auth_request_log_debug(request,
|
|
request->mech != NULL ? AUTH_SUBSYS_MECH
|
|
diff --git a/src/auth/auth.c b/src/auth/auth.c
|
|
index f2f3fda20c..9f6c4ba60c 100644
|
|
--- a/src/auth/auth.c
|
|
+++ b/src/auth/auth.c
|
|
@@ -99,6 +99,24 @@ auth_passdb_preinit(struct auth *auth, const struct auth_passdb_settings *set,
|
|
auth_passdb->override_fields_tmpl =
|
|
passdb_template_build(auth->pool, set->override_fields);
|
|
|
|
+ if (*set->mechanisms == '\0') {
|
|
+ auth_passdb->mechanisms = NULL;
|
|
+ } else if (strcasecmp(set->mechanisms, "none") == 0) {
|
|
+ auth_passdb->mechanisms = (const char *const[]){ NULL };
|
|
+ } else {
|
|
+ auth_passdb->mechanisms =
|
|
+ (const char *const *)p_strsplit_spaces(auth->pool,
|
|
+ set->mechanisms, " ,");
|
|
+ }
|
|
+
|
|
+ if (*set->username_filter == '\0') {
|
|
+ auth_passdb->username_filter = NULL;
|
|
+ } else {
|
|
+ auth_passdb->username_filter =
|
|
+ (const char *const *)p_strsplit_spaces(auth->pool,
|
|
+ set->username_filter, " ,");
|
|
+ }
|
|
+
|
|
/* for backwards compatibility: */
|
|
if (set->pass)
|
|
auth_passdb->result_success = AUTH_DB_RULE_CONTINUE;
|
|
diff --git a/src/auth/auth.h b/src/auth/auth.h
|
|
index f700e29d5c..460a179765 100644
|
|
--- a/src/auth/auth.h
|
|
+++ b/src/auth/auth.h
|
|
@@ -41,6 +41,11 @@ struct auth_passdb {
|
|
struct passdb_template *default_fields_tmpl;
|
|
struct passdb_template *override_fields_tmpl;
|
|
|
|
+ /* Supported authentication mechanisms, NULL is all, {NULL} is none */
|
|
+ const char *const *mechanisms;
|
|
+ /* Username filter, NULL is no filter */
|
|
+ const char *const *username_filter;
|
|
+
|
|
enum auth_passdb_skip skip;
|
|
enum auth_db_rule result_success;
|
|
enum auth_db_rule result_failure;
|
|
diff --git a/src/auth/passdb.c b/src/auth/passdb.c
|
|
index eb4ac8ae82..f5eed1af4f 100644
|
|
--- a/src/auth/passdb.c
|
|
+++ b/src/auth/passdb.c
|
|
@@ -224,19 +224,8 @@ passdb_preinit(pool_t pool, const struct auth_passdb_settings *set)
|
|
passdb->id = ++auth_passdb_id;
|
|
passdb->iface = *iface;
|
|
passdb->args = p_strdup(pool, set->args);
|
|
- if (*set->mechanisms == '\0') {
|
|
- passdb->mechanisms = NULL;
|
|
- } else if (strcasecmp(set->mechanisms, "none") == 0) {
|
|
- passdb->mechanisms = (const char *const[]){NULL};
|
|
- } else {
|
|
- passdb->mechanisms = (const char* const*)p_strsplit_spaces(pool, set->mechanisms, " ,");
|
|
- }
|
|
-
|
|
- if (*set->username_filter == '\0') {
|
|
- passdb->username_filter = NULL;
|
|
- } else {
|
|
- passdb->username_filter = (const char* const*)p_strsplit_spaces(pool, set->username_filter, " ,");
|
|
- }
|
|
+ /* NOTE: if anything else than driver & args are added here,
|
|
+ passdb_find() also needs to be updated. */
|
|
array_push_back(&passdb_modules, &passdb);
|
|
return passdb;
|
|
}
|
|
diff --git a/src/auth/passdb.h b/src/auth/passdb.h
|
|
index 2e95328e5c..e466a9fdb6 100644
|
|
--- a/src/auth/passdb.h
|
|
+++ b/src/auth/passdb.h
|
|
@@ -63,10 +63,6 @@ struct passdb_module {
|
|
/* Default password scheme for this module.
|
|
If default_cache_key is set, must not be NULL. */
|
|
const char *default_pass_scheme;
|
|
- /* Supported authentication mechanisms, NULL is all, [NULL] is none*/
|
|
- const char *const *mechanisms;
|
|
- /* Username filter, NULL is no filter */
|
|
- const char *const *username_filter;
|
|
|
|
/* If blocking is set to TRUE, use child processes to access
|
|
this passdb. */
|
|
--
|
|
2.30.2
|
|
|