From 5fa61e17e9d96bf3ceea1eace10caeb4ea94e9f5 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 13 Dec 2024 17:41:54 +0200 Subject: [PATCH] lib-ldap, auth: Change ldap_set_*() API to return error message Instead of i_fatal()ing on error. --- src/auth/db-ldap.c | 22 ++++++--- src/lib-ldap/ldap-connection.c | 8 +-- src/lib-ldap/ldap-private.h | 1 - src/lib-ldap/ldap-utils.c | 89 ++++++++++++++++++++++------------ src/lib-ldap/ldap-utils.h | 14 +++--- 5 files changed, 84 insertions(+), 50 deletions(-) diff --git a/src/auth/db-ldap.c b/src/auth/db-ldap.c index d015de4169..6d168ad57c 100644 --- a/src/auth/db-ldap.c +++ b/src/auth/db-ldap.c @@ -877,6 +877,7 @@ db_ldap_del_connection_callback(LDAP *ld ATTR_UNUSED, Sockbuf *sb ATTR_UNUSED, static void ldap_set_options(struct ldap_connection *conn) { + const char *error; int ret; struct ldap_conncb *cb = p_new(conn->pool, struct ldap_conncb, 1); @@ -898,20 +899,25 @@ static void ldap_set_options(struct ldap_connection *conn) conn->log_prefix, ldap_err2string(ret)); #endif - ldap_set_opt(conn->log_prefix, conn->ld, LDAP_OPT_DEREF, &conn->set->parsed_deref, - "ldap_deref", conn->set->deref); + if (ldap_set_opt(conn->ld, LDAP_OPT_DEREF, &conn->set->parsed_deref, + "ldap_deref", conn->set->deref, &error) < 0) + i_fatal("%s%s", conn->log_prefix, error); #ifdef LDAP_OPT_DEBUG_LEVEL if (conn->set->debug_level != 0) { - ldap_set_opt(conn->log_prefix, NULL, LDAP_OPT_DEBUG_LEVEL, &conn->set->debug_level, - "ldap_debug_level", dec2str(conn->set->debug_level)); + if (ldap_set_opt(NULL, LDAP_OPT_DEBUG_LEVEL, &conn->set->debug_level, + "ldap_debug_level", dec2str(conn->set->debug_level), &error) < 0) + i_fatal("%s%s", conn->log_prefix, error); event_set_forced_debug(conn->event, TRUE); } #endif - ldap_set_opt(conn->log_prefix, conn->ld, LDAP_OPT_PROTOCOL_VERSION, - &conn->set->version, "ldap_version", dec2str(conn->set->version)); - ldap_set_tls_options(conn->log_prefix, conn->ld, conn->set->starttls, - conn->set->uris, conn->ssl_set); + if (ldap_set_opt(conn->ld, LDAP_OPT_PROTOCOL_VERSION, + &conn->set->version, "ldap_version", + dec2str(conn->set->version), &error) < 0) + i_fatal("%s%s", conn->log_prefix, error); + if (ldap_set_tls_options(conn->ld, conn->set->starttls, + conn->set->uris, conn->ssl_set, &error) < 0) + i_fatal("%s%s", conn->log_prefix, error); } static void db_ldap_init_ld(struct ldap_connection *conn) diff --git a/src/lib-ldap/ldap-connection.c b/src/lib-ldap/ldap-connection.c index 36c76fa01d..2e2fac954b 100644 --- a/src/lib-ldap/ldap-connection.c +++ b/src/lib-ldap/ldap-connection.c @@ -54,8 +54,9 @@ int ldap_connection_setup(struct ldap_connection *conn, const char **error_r) return -1; } - ldap_set_tls_options(conn->log_prefix, conn->conn, conn->set->starttls, - conn->set->uris, conn->ssl_set); + if (ldap_set_tls_options(conn->conn, conn->set->starttls, + conn->set->uris, conn->ssl_set, error_r) < 0) + return -1; #ifdef LDAP_OPT_X_TLS_PROTOCOL_MIN /* refuse to connect to SSLv2 as it's completely insecure */ @@ -112,7 +113,8 @@ int ldap_connection_init(struct ldap_client *client, conn->pool = pool; conn->event = event_create(ldap_client_get_event(client)); conn->client = client; - conn->log_prefix = p_strdup_printf(pool, "ldap(%s): ", set->uris); + event_set_append_log_prefix(conn->event, + t_strdup_printf("ldap(%s): ", set->uris)); pool_ref(set->pool); pool_ref(ssl_set->pool); diff --git a/src/lib-ldap/ldap-private.h b/src/lib-ldap/ldap-private.h index 8ec31f86d1..c35936770c 100644 --- a/src/lib-ldap/ldap-private.h +++ b/src/lib-ldap/ldap-private.h @@ -61,7 +61,6 @@ struct ldap_connection { const struct ldap_client_settings *set; const struct ssl_settings *ssl_set; - char *log_prefix; struct aqueue *request_queue; ARRAY(struct ldap_op_queue_entry *) request_array; diff --git a/src/lib-ldap/ldap-utils.c b/src/lib-ldap/ldap-utils.c index d646e623b6..5a7e94839f 100644 --- a/src/lib-ldap/ldap-utils.c +++ b/src/lib-ldap/ldap-utils.c @@ -5,36 +5,45 @@ #include "ssl-settings.h" #include "settings-parser.h" -void ldap_set_opt(const char *prefix, LDAP *ld, int opt, const void *value, - const char *optname, const char *value_str) +int ldap_set_opt(LDAP *ld, int opt, const void *value, + const char *optname, const char *value_str, + const char **error_r) { int ret; ret = ldap_set_option(ld, opt, value); if (ret != LDAP_SUCCESS) { - i_fatal("%sCan't set option %s to %s: %s", - prefix, optname, value_str, ldap_err2string(ret)); + *error_r = t_strdup_printf("Can't set option %s to %s: %s", + optname, value_str, ldap_err2string(ret)); + return -1; } + return 0; } -void ldap_set_opt_str(const char *prefix, LDAP *ld, int opt, const char *value, - const char *optname) +int ldap_set_opt_str(LDAP *ld, int opt, const char *value, + const char *optname, const char **error_r) { if (*value != '\0') - ldap_set_opt(prefix, ld, opt, value, optname, value); + return ldap_set_opt(ld, opt, value, optname, value, error_r); + else + return 0; } #ifndef LDAP_OPT_X_TLS -void ldap_set_tls_options(const char *prefix ATTR_UNUSED, LDAP *ld ATTR_UNUSED, - bool starttls ATTR_UNUSED, const char *uris ATTR_UNUSED, - const struct ssl_settings *ssl_set ATTR_UNUSED) { } +int ldap_set_tls_options(LDAP *ld ATTR_UNUSED, bool starttls ATTR_UNUSED, + const char *uris ATTR_UNUSED, + const struct ssl_settings *ssl_set ATTR_UNUSED, + const char **error_r ATTR_UNUSED) +{ + return 0; +} #else - -void ldap_set_tls_options(const char *prefix, LDAP *ld, bool starttls, - const char *uris, const struct ssl_settings *ssl_set) +int ldap_set_tls_options(LDAP *ld, bool starttls, const char *uris, + const struct ssl_settings *ssl_set, + const char **error_r) { if (!starttls && strstr(uris, "ldaps:") == NULL) - return; + return 0; struct settings_file key_file, cert_file, ca_file; settings_file_get(ssl_set->ssl_client_key_file, @@ -44,31 +53,47 @@ void ldap_set_tls_options(const char *prefix, LDAP *ld, bool starttls, settings_file_get(ssl_set->ssl_client_ca_file, unsafe_data_stack_pool, &ca_file); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_CACERTFILE, - ca_file.path, "ssl_client_ca_file"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_CACERTDIR, - ssl_set->ssl_client_ca_dir, "ssl_client_ca_dir"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_CERTFILE, - cert_file.path, "ssl_client_cert_file"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_KEYFILE, - key_file.path, "ssl_client_key_file"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_CIPHER_SUITE, - ssl_set->ssl_cipher_list, "ssl_cipher_list"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_PROTOCOL_MIN, - ssl_set->ssl_min_protocol, "ssl_min_protocol"); - ldap_set_opt_str(prefix, ld, LDAP_OPT_X_TLS_ECNAME, - ssl_set->ssl_curve_list, "ssl_curve_list"); + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_CACERTFILE, + ca_file.path, "ssl_client_ca_file", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_CACERTDIR, + ssl_set->ssl_client_ca_dir, + "ssl_client_ca_dir", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_CERTFILE, cert_file.path, + "ssl_client_cert_file", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_KEYFILE, key_file.path, + "ssl_client_key_file", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_CIPHER_SUITE, + ssl_set->ssl_cipher_list, + "ssl_cipher_list", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_PROTOCOL_MIN, + ssl_set->ssl_min_protocol, + "ssl_min_protocol", error_r) < 0) + return -1; + if (ldap_set_opt_str(ld, LDAP_OPT_X_TLS_ECNAME, + ssl_set->ssl_curve_list, + "ssl_curve_list", error_r) < 0) + return -1; bool requires = ssl_set->ssl_client_require_valid_cert; int opt = requires ? LDAP_OPT_X_TLS_HARD : LDAP_OPT_X_TLS_ALLOW; /* required for Bookworm */ - ldap_set_opt(prefix, NULL, LDAP_OPT_X_TLS_REQUIRE_CERT, &opt, - "ssl_client_require_valid_cert", requires ? "yes" : "no" ); + if (ldap_set_opt(NULL, LDAP_OPT_X_TLS_REQUIRE_CERT, &opt, + "ssl_client_require_valid_cert", + requires ? "yes" : "no", error_r) < 0) + return -1; /* required for RHEL9 */ - ldap_set_opt(prefix, ld, LDAP_OPT_X_TLS_REQUIRE_CERT, &opt, - "ssl_client_require_valid_cert", requires ? "yes" : "no"); + if (ldap_set_opt(ld, LDAP_OPT_X_TLS_REQUIRE_CERT, &opt, + "ssl_client_require_valid_cert", + requires ? "yes" : "no", error_r) < 0) + return -1; + return 0; } static int ldap_set_tls_validate_file(const char *file, const char *name, diff --git a/src/lib-ldap/ldap-utils.h b/src/lib-ldap/ldap-utils.h index c42f636d1e..53e8ce0147 100644 --- a/src/lib-ldap/ldap-utils.h +++ b/src/lib-ldap/ldap-utils.h @@ -5,14 +5,16 @@ struct ssl_settings; -void ldap_set_opt(const char *prefix, LDAP *ld, int opt, const void *value, - const char *optname, const char *value_str); +int ldap_set_opt(LDAP *ld, int opt, const void *value, + const char *optname, const char *value_str, + const char **error_r); -void ldap_set_opt_str(const char *prefix, LDAP *ld, int opt, const char *value, - const char *optname); +int ldap_set_opt_str(LDAP *ld, int opt, const char *value, + const char *optname, const char **error_r); -void ldap_set_tls_options(const char *prefix, LDAP *ld, bool starttls, - const char *uris, const struct ssl_settings *ssl_set); +int ldap_set_tls_options(LDAP *ld, bool starttls, const char *uris, + const struct ssl_settings *ssl_set, + const char **error_r); int ldap_set_tls_validate(const struct ssl_settings *set, const char **error_r); -- 2.47.3