lib-imap-client: imapc-connection - Obtain authentication settings exclusively from event

The imap client is now configured as follows:

imapc_host = imap.example.com
imapc_port = 143
imapc_ssl = no

imapc {
  client_auth PLAIN {
    authid = imap-frontend
    password = frop
  }
}

The following settings are renamed:

imapc_user        -> imapc/client_auth_authzid
imapc_master_user -> imapc/client_auth_authid
imapc_password    -> imapc/client_auth_password

The following settings are deprecated:

imapc_sasl_mechanisms

src/auth/passdb-imap.c

@@ -114,7 +114,7 @@ passdb_imap_verify_plain(struct auth_request *auth_request,
 	struct settings_instance *set_instance;
 
 	set_instance = settings_instance_new_from_event(event);
-	settings_override(set_instance, "imapc_password", password,
+	settings_override(set_instance, "imapc/client_auth_password", password,
 			  SETTINGS_OVERRIDE_TYPE_CODE);
 	event_set_ptr(event, SETTINGS_EVENT_INSTANCE, set_instance);
 

src/lib-imap-client/imapc-client-private.h

@@ -23,6 +23,7 @@ struct imapc_client {
 	int refcount;
 
 	struct event *event;
+	struct settings_instance *set_instance;
 	const struct imapc_settings *set;
 	struct imapc_parameters params;
 	enum imapc_client_ssl_mode ssl_mode;

src/lib-imap-client/imapc-client.c

@@ -6,6 +6,7 @@
 #include "ioloop.h"
 #include "safe-mkstemp.h"
 #include "settings.h"
+#include "client-auth-settings.h"
 #include "dsasl-client.h"
 #include "imapc-msgmap.h"
 #include "imapc-connection.h"
@@ -26,6 +27,41 @@ default_untagged_callback(const struct imapc_untagged_reply *reply ATTR_UNUSED,
 {
 }
 
+static struct settings_instance *
+imapc_client_init_settings(struct event *event,
+			   struct settings_instance **set_instance)
+{
+	if (*set_instance != NULL)
+		return *set_instance;
+
+	*set_instance = settings_instance_new_from_event(event);
+	event_set_ptr(event, SETTINGS_EVENT_INSTANCE, *set_instance);
+	return *set_instance;
+}
+
+static void
+imapc_client_handle_deprecated_settings(
+	struct event *event, const struct imapc_settings *set,
+	struct settings_instance **set_instance)
+{
+	/* SASL */
+
+	const char *const *mech_p =
+		settings_boollist_get(&set->imapc_sasl_mechanisms);
+	while (*mech_p != NULL) {
+		struct settings_instance *instance =
+			imapc_client_init_settings(event, set_instance);
+
+		settings_override(instance, "client_auth+", *mech_p,
+				  SETTINGS_OVERRIDE_TYPE_CODE);
+		settings_override(instance,
+				  t_strdup_printf("client_auth/%s/mechanism",
+						  *mech_p),
+				  *mech_p, SETTINGS_OVERRIDE_TYPE_CODE);
+		mech_p++;
+	}
+}
+
 int imapc_client_init(const struct imapc_parameters *params,
 		      struct event *event_parent,
 		      struct imapc_client **client_r, const char **error_r)
@@ -33,6 +69,7 @@ int imapc_client_init(const struct imapc_parameters *params,
 	struct event *event;
 	struct imapc_settings *set;
 	const struct ssl_settings *ssl_set = NULL;
+	struct settings_instance *set_instance = NULL;
 	struct dsasl_client *sasl_client;
 	struct imapc_client *client;
 	pool_t pool;
@@ -58,31 +95,16 @@ int imapc_client_init(const struct imapc_parameters *params,
 		return -1;
 	}
 
+	imapc_client_handle_deprecated_settings(event, set,
+						&set_instance);
+
 	sasl_client = dsasl_client_create(event);
-	if (!array_is_empty(&set->imapc_sasl_mechanisms)) {
-		const char *mech_name;
-		array_foreach_elem(&set->imapc_sasl_mechanisms, mech_name) {
-			const struct dsasl_client_mech *mech =
-				dsasl_client_mech_find(sasl_client, mech_name);
-			if (mech == NULL) {
-				*error_r = t_strdup_printf(
-					"imapc_sasl_mechanism: "
-					"'%s' mechanism is not supported",
-					mech_name);
-				dsasl_client_unref(&sasl_client);
-				settings_free(set);
-				event_unref(&event);
-				return -1;
-			}
-			if (dsasl_client_mech_uses_password(mech) &&
-				  *set->imapc_password == '\0') {
-				*error_r = "Missing imapc_password";
-				dsasl_client_unref(&sasl_client);
-				settings_free(set);
-				event_unref(&event);
-				return -1;
-			}
-		}
+	if (dsasl_client_check_settings(sasl_client, error_r) < 0) {
+		dsasl_client_unref(&sasl_client);
+		settings_instance_free(&set_instance);
+		settings_free(set);
+		event_unref(&event);
+		return -1;
 	}
 
 	pool = pool_alloconly_create("imapc client", 1024);
@@ -93,6 +115,7 @@ int imapc_client_init(const struct imapc_parameters *params,
 	client->event = event;
 	client->set = set;
 	client->ssl_set = ssl_set;
+	client->set_instance = set_instance;
 	client->sasl_client = sasl_client;
 
 	client->params.session_id_prefix =
@@ -137,6 +160,7 @@ void imapc_client_unref(struct imapc_client **_client)
 
 	dsasl_client_unref(&client->sasl_client);
 	event_unref(&client->event);
+	settings_instance_free(&client->set_instance);
 	pool_unref(&client->pool);
 }
 
@@ -170,28 +194,52 @@ int imapc_client_settings_match(struct imapc_client *client,
 {
 	const struct imapc_settings *imapc_set;
 	const struct ssl_settings *ssl_set;
-	bool match;
+	struct settings_instance *set_instance = NULL;
+	struct event *event;
+	int ret = 1;
 
-	if (settings_get(event_other, &imapc_setting_parser_info, 0,
+	/* Look up the settings exactly like imapc_client_init() does, so also
+	   the settings inside the imapc { } filter are seen. */
+	event = event_create(event_other);
+	settings_event_add_filter_name(event, IMAPC_SETTINGS_FILTER);
+
+	if (settings_get(event, &imapc_setting_parser_info, 0,
 			 &imapc_set, error_r) < 0) {
-		/* The error is reported when the new storage is created. */
+		event_unref(&event);
 		return -1;
 	}
-	match = settings_equal(&imapc_setting_parser_info, client->set,
-			       imapc_set, NULL);
+	if (!settings_equal(&imapc_setting_parser_info, client->set,
+			    imapc_set, NULL))
+		ret = 0;
+	else {
+		/* The client translated its deprecated settings into
+		   client_auth settings, so the same must be done here before
+		   they are compared. */
+		imapc_client_handle_deprecated_settings(event, imapc_set,
+							&set_instance);
+	}
 	settings_free(imapc_set);
-	if (!match || client->ssl_set == NULL) {
-		/* Either the settings differ, or imapc_ssl=no in both of them
-		   and there are no SSL settings to compare. */
-		return (match ? 1 : 0);
-	}
 
-	if (ssl_client_settings_get(event_other, &ssl_set, error_r) < 0)
-		return -1;
-	match = settings_equal(&ssl_setting_parser_info,
-			       client->ssl_set, ssl_set, NULL);
-	settings_free(ssl_set);
-	return (match ? 1 : 0);
+	if (ret > 0 && client->ssl_set != NULL) {
+		/* imapc_ssl is not "no", so compare also the SSL settings. */
+		if (ssl_client_settings_get(event, &ssl_set, error_r) < 0)
+			ret = -1;
+		else {
+			if (!settings_equal(&ssl_setting_parser_info,
+					    client->ssl_set, ssl_set, NULL))
+				ret = 0;
+			settings_free(ssl_set);
+		}
+	}
+	if (ret > 0) {
+		/* The authentication settings are used only when the
+		   connection is created, so they must be compared here as
+		   well. */
+		ret = client_auth_settings_equal(client->event, event, error_r);
+	}
+	event_unref(&event);
+	settings_instance_free(&set_instance);
+	return ret;
 }
 
 void imapc_client_register_untagged(struct imapc_client *client,

src/lib-imap-client/imapc-connection.c

@@ -95,6 +95,8 @@ struct imapc_connection {
 	struct timeout *to;
 	struct timeout *to_output;
 	struct dns_lookup *dns_lookup;
+
+	const struct client_auth_settings *auth_set;
 	struct dsasl_client_session *sasl_session;
 
 	struct ssl_iostream *ssl_iostream;
@@ -480,6 +482,7 @@ void imapc_connection_disconnect_full(struct imapc_connection *conn,
 	if (conn->dns_lookup != NULL)
 		dns_lookup_abort(&conn->dns_lookup);
 	dsasl_client_session_destroy(&conn->sasl_session);
+	settings_free(conn->auth_set);
 	imapc_connection_lfiles_free(conn);
 	imapc_connection_literal_reset(&conn->literal);
 	timeout_remove(&conn->to_output);
@@ -935,18 +938,19 @@ imapc_connection_proxyauth_login_cb(const struct imapc_command_reply *reply,
 				    void *context)
 {
 	struct imapc_connection *conn = context;
-	const struct imapc_settings *set = conn->client->set;
 	struct imapc_command *cmd;
 
 	if (reply->state == IMAPC_COMMAND_STATE_OK) {
 		cmd = imapc_connection_cmd(conn, imapc_connection_login_cb,
 					   conn);
 		imapc_command_set_flags(cmd, IMAPC_COMMAND_FLAG_PRELOGIN);
-		imapc_command_sendf(cmd, "PROXYAUTH %s", set->imapc_user);
+		imapc_command_sendf(cmd, "PROXYAUTH %s",
+				    conn->auth_set->authzid);
 		imapc_command_send_more(conn);
 	} else {
 		imapc_connection_auth_finish(conn, reply);
 	}
+	settings_free(conn->auth_set);
 }
 
 static void
@@ -994,20 +998,6 @@ imapc_connection_authenticate_cb(const struct imapc_command_reply *reply,
 	imapc_connection_disconnect(conn);
 }
 
-static bool imapc_connection_have_auth(struct imapc_connection *conn,
-				       const char *mech_name)
-{
-	const char *auth;
-	char *const *capa;
-
-	for (capa = conn->capabilities_list; *capa != NULL; capa++) {
-		if (str_begins_icase(*capa, "AUTH=", &auth) &&
-		    strcasecmp(auth, mech_name) == 0)
-			return TRUE;
-	}
-	return FALSE;
-}
-
 static const char *const *
 imapc_connection_get_auth_mechs(struct imapc_connection *conn,
 				bool *login_disabled_r)
@@ -1030,27 +1020,6 @@ imapc_connection_get_auth_mechs(struct imapc_connection *conn,
 	return array_idx(&mechs, 0);
 }
 
-static int
-imapc_connection_get_sasl_mech(struct imapc_connection *conn,
-			       const char **mech_name_r, const char **error_r)
-{
-	struct imapc_client *client = conn->client;
-	const struct imapc_settings *set = client->set;
-	const char *const *mechanisms =
-		settings_boollist_get(&set->imapc_sasl_mechanisms);
-
-	/* find one of the specified SASL mechanisms */
-	for (; *mechanisms != NULL; mechanisms++) {
-		if (imapc_connection_have_auth(conn, *mechanisms)) {
-			*mech_name_r = *mechanisms;
-			return 0;
-		}
-	}
-	*error_r = t_strdup_printf("IMAP server doesn't support any of the requested SASL mechanisms: %s",
-				   t_strarray_join(mechanisms, ", "));
-	return -1;
-}
-
 static int
 imapc_connection_channel_bind_callback(const char *type, void *context,
 				       const buffer_t **data_r,
@@ -1092,62 +1061,61 @@ static void imapc_connection_authenticate(struct imapc_connection *conn)
 	struct imapc_command *cmd;
 	const char *const *server_mechanisms;
 	bool login_disabled = FALSE;
-	struct client_auth_settings sasl_set;
-	const char *mech_name = NULL, *error;
+	const char *error;
 	int ret;
 
-	if (array_not_empty(&set->imapc_sasl_mechanisms)) {
-		if (imapc_connection_get_sasl_mech(conn, &mech_name,
-						   &error) < 0) {
-			struct imapc_command_reply reply = {
-				.state = IMAPC_COMMAND_STATE_DISCONNECTED,
-				.text_full = "",
-			};
-			imapc_auth_failed(conn, &reply, error);
-			imapc_connection_disconnect(conn);
-			return;
-		}
+	i_assert(conn->auth_set == NULL);
+	if (settings_get(conn->event, &client_auth_setting_parser_info, 0,
+			 &conn->auth_set, &error) < 0) {
+		struct imapc_command_reply reply = {
+			.state = IMAPC_COMMAND_STATE_DISCONNECTED,
+			.text_full = "",
+		};
+		imapc_auth_failed(conn, &reply, error);
+		imapc_connection_disconnect(conn);
+		return;
 	}
 
 	server_mechanisms = imapc_connection_get_auth_mechs(conn, &login_disabled);
 
 	if ((set->parsed_features & IMAPC_FEATURE_PROXYAUTH) != 0 &&
-	    *set->imapc_master_user != '\0') {
+	    *conn->auth_set->authid != '\0' &&
+	    *conn->auth_set->authzid != '\0') {
 		/* We can use LOGIN command */
 		e_debug(conn->event, "Logging in as %s (PROXYAUTH)",
-			set->imapc_master_user);
+			conn->auth_set->authid);
 		cmd = imapc_connection_cmd(conn, imapc_connection_proxyauth_login_cb,
 					   conn);
 		imapc_command_set_flags(cmd, IMAPC_COMMAND_FLAG_PRELOGIN);
 		imapc_command_sendf(cmd, "LOGIN %s %s",
-				    set->imapc_master_user, set->imapc_password);
+				    conn->auth_set->authid,
+				    conn->auth_set->password);
 		return;
 	}
-	if (!login_disabled && mech_name == NULL &&
-	    ((*set->imapc_master_user == '\0' &&
-	      !need_literal(set->imapc_user) &&
-	      !need_literal(set->imapc_password)) ||
+
+	const char *login_user = conn->auth_set->authzid;
+	const char *login_master = conn->auth_set->authid;
+	if (*login_user == '\0') {
+		login_user = login_master;
+		login_master = "";
+	}
+	if (!login_disabled && *login_user != '\0' &&
+	    !client_auth_settings_mechanism_configured(conn->auth_set) &&
+	    ((*login_master == '\0' &&
+	      !need_literal(login_user) &&
+	      !need_literal(conn->auth_set->password)) ||
 	     (conn->capabilities & IMAPC_CAPABILITY_AUTH_PLAIN) == 0)) {
 		/* We can use LOGIN command */
-		e_debug(conn->event, "Logging in as %s", set->imapc_user);
+		e_debug(conn->event, "Logging in as %s", login_user);
 		cmd = imapc_connection_cmd(conn, imapc_connection_login_cb,
 					   conn);
 		imapc_command_set_flags(cmd, IMAPC_COMMAND_FLAG_PRELOGIN);
 		imapc_command_sendf(cmd, "LOGIN %s %s",
-				    set->imapc_user, set->imapc_password);
+				    login_user, conn->auth_set->password);
+		settings_free(conn->auth_set);
 		return;
 	}
 
-	i_zero(&sasl_set);
-	sasl_set.mechanism = mech_name;
-	if (*set->imapc_master_user == '\0')
-		sasl_set.authid = set->imapc_user;
-	else {
-		sasl_set.authid = set->imapc_master_user;
-		sasl_set.authzid = set->imapc_user;
-	}
-	sasl_set.password = set->imapc_password;
-
 	struct dsasl_client_parameters sasl_params = {
 		.protocol = "imap",
 		.host = set->imapc_host,
@@ -1156,8 +1124,9 @@ static void imapc_connection_authenticate(struct imapc_connection *conn)
 	};
 
 	ret = dsasl_client_session_create(client->sasl_client, conn->event,
-					  &sasl_params, &sasl_set,
+					  &sasl_params, conn->auth_set,
 					  &conn->sasl_session, &error);
+	settings_free(conn->auth_set);
 	if (ret <= 0) {
 		e_error(conn->event, "Failed to create SASL session: %s",
 			error);
@@ -1169,7 +1138,9 @@ static void imapc_connection_authenticate(struct imapc_connection *conn)
 		imapc_connection_disconnect(conn);
 		return;
 	}
-	mech_name = dsasl_client_session_get_mech_name(conn->sasl_session);
+
+	const char *mech_name =
+		dsasl_client_session_get_mech_name(conn->sasl_session);
 
 	if (conn->ssl_iostream != NULL) {
 		dsasl_client_session_enable_channel_binding(

src/lib-imap-client/imapc-settings.c

@@ -23,12 +23,6 @@ static const struct setting_define imapc_setting_defines[] = {
 	{ .type = SET_FILTER_NAME, .key = "layout_imapc" },
 	DEF(STR, imapc_host),
 	DEF(IN_PORT, imapc_port),
-
-	DEF(STR, imapc_user),
-	DEF(STR, imapc_master_user),
-	DEF(STR, imapc_password),
-	DEF(BOOLLIST, imapc_sasl_mechanisms),
-
 	DEF(ENUM, imapc_ssl),
 
 	DEF(BOOLLIST, imapc_features),
@@ -43,18 +37,15 @@ static const struct setting_define imapc_setting_defines[] = {
 
 	DEF(STR, pop3_deleted_flag),
 
+	/* deprecated: */
+	DEF(BOOLLIST_HIDDEN, imapc_sasl_mechanisms),
+
 	SETTING_DEFINE_LIST_END
 };
 
 static const struct imapc_settings imapc_default_settings = {
 	.imapc_host = "",
 	.imapc_port = 143,
-
-	.imapc_user = "%{owner_user}",
-	.imapc_master_user = "",
-	.imapc_password = "",
-	.imapc_sasl_mechanisms = ARRAY_INIT,
-
 	.imapc_ssl = "no:imaps:starttls",
 
 	.imapc_features = ARRAY_INIT,
@@ -68,10 +59,14 @@ static const struct imapc_settings imapc_default_settings = {
 	.imapc_max_line_length = SET_SIZE_UNLIMITED,
 
 	.pop3_deleted_flag = "",
+
+	/* deprecated: */
+	.imapc_sasl_mechanisms = ARRAY_INIT,
 };
 
 static const struct setting_keyvalue imapc_default_settings_keyvalue[] = {
 	{ "imapc/mailbox_list_layout", "imapc" },
+	{ "imapc/client_auth_authzid", "%{owner_user}" },
 	/* We want to have all imapc mailboxes accessible, so escape them if
 	   necessary. */
 	{ "layout_imapc/mailbox_list_visible_escape_char", "~" },

src/lib-imap-client/imapc-settings.h

@@ -67,12 +67,6 @@ struct imapc_settings {
 	pool_t pool;
 	const char *imapc_host;
 	in_port_t imapc_port;
-
-	const char *imapc_user;
-	const char *imapc_master_user;
-	const char *imapc_password;
-	ARRAY_TYPE(const_string) imapc_sasl_mechanisms;
-
 	const char *imapc_ssl;
 
 	ARRAY_TYPE(const_string) imapc_features;
@@ -93,6 +87,9 @@ struct imapc_settings {
 	unsigned int throttle_init_msecs;
 	unsigned int throttle_max_msecs;
 	unsigned int throttle_shrink_min_msecs;
+
+	/* deprecated: */
+	ARRAY_TYPE(const_string) imapc_sasl_mechanisms;
 };
 
 extern const struct setting_parser_info imapc_setting_parser_info;

src/lib-imap-client/test-imapc-client.c

@@ -56,8 +56,9 @@ test_generate_settings(const char *port, const char *connection_timeout_interval
 	static const char *settings[] = {
 		"imapc_host", "127.0.0.1",
 		"imapc_port", "",
-		"imapc_user", "testuser",
-		"imapc_password", "testpass",
+		"imapc/client_auth_authid", "",
+		"imapc/client_auth_authzid", "testuser",
+		"imapc/client_auth_password", "testpass",
 		"imapc_connection_timeout_interval", "5s",
 		"imapc_connection_retry_count", "3",
 		"imapc_connection_retry_interval", "10ms",
@@ -68,7 +69,7 @@ test_generate_settings(const char *port, const char *connection_timeout_interval
 	if (port != NULL)
 		settings[3] = port;
 	if (connection_timeout_interval != NULL)
-		settings[9] = connection_timeout_interval;
+		settings[11] = connection_timeout_interval;
 
 	return settings;
 }

src/lib-settings/settings-history-core.txt

@@ -2,6 +2,10 @@
 # rename <old-setting-key> <new-setting-key> <ce-version> <pro-version>
 # value <old-setting-key>=<old-value> <new-setting-key>=<new-value> <ce-version> <pro-version>
 # deprecate <setting-key> <ce-version> <pro-version>
+rename	imapc_user	imapc/client_auth_authzid	2.4.6	3.3.0
+rename	imapc_master_user	imapc/client_auth_authid	2.4.6	3.3.0
+rename	imapc_password	imapc/client_auth_password	2.4.6	3.3.0
+deprecate	imapc_sasl_mechanisms	2.4.6	3.3.0
 default	textcat_filter_languages	no	2.4.6	3.3.0
 value	shutdown_clients=yes	service_shutdown_clients_timeout=0	2.4.6	3.3.0
 value	shutdown_clients=no	service_shutdown_clients_timeout=infinite	2.4.6	3.3.0

src/lib-storage/index/imapc/Makefile.am

@@ -8,6 +8,7 @@ AM_CPPFLAGS = \
 	-I$(top_srcdir)/src/lib-mail \
 	-I$(top_srcdir)/src/lib-imap \
 	-I$(top_srcdir)/src/lib-imap-client \
+	-I$(top_srcdir)/src/lib-auth/ \
 	-I$(top_srcdir)/src/lib-sasl/ \
 	-I$(top_srcdir)/src/lib-index \
 	-I$(top_srcdir)/src/lib-storage \