From cdab42e840da7e7dfbb8d3a194d6a0aa8a181d92 Mon Sep 17 00:00:00 2001 From: Titanium Date: Mon, 25 Mar 2024 01:51:19 +0800 Subject: [PATCH] [#422] Improve the `restart_server` function Implement restart_server function for daemon with conditional restarts and warnings for configuration changes, including TLS updates. --- AUTHORS | 3 +- doc/ARCHITECTURE.md | 1 + doc/CONFIGURATION.md | 8 +++--- doc/man/pgagroal.conf.5.rst | 8 +++--- src/libpgagroal/configuration.c | 51 ++++++++++++++++++++++++++++++++- 5 files changed, 61 insertions(+), 10 deletions(-) diff --git a/AUTHORS b/AUTHORS index a81701ca..8b37779a 100644 --- a/AUTHORS +++ b/AUTHORS @@ -9,4 +9,5 @@ Nikita Bugrovsky Lawrence Wu Yongting You <2010youy01@gmail.com> Ashutosh Sharma -Henrique de Carvalho \ No newline at end of file +Henrique de Carvalho +Yihe Lu \ No newline at end of file diff --git a/doc/ARCHITECTURE.md b/doc/ARCHITECTURE.md index 10e6373e..ecb78122 100644 --- a/doc/ARCHITECTURE.md +++ b/doc/ARCHITECTURE.md @@ -243,6 +243,7 @@ However, some configuration settings requires a full restart of `pgagroal` in or * `unix_socket_dir` * `pidfile` * Limit rules defined by `pgagroal_databases.conf` +* TLS rules defined by server section The configuration can also be reloaded using `pgagroal-cli -c pgagroal.conf reload`. The command is only supported over the local interface, and hence doesn't work remotely. diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 72c234b7..74e0bf89 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -105,10 +105,10 @@ There can be up to `64` host sections, each with an unique name and different co | host | | String | Yes | The address of the PostgreSQL instance | | port | | Int | Yes | The port of the PostgreSQL instance | | primary | | Bool | No | Identify the instance as primary (hint) | -| tls | `off` | Bool | No | Enable Transport Layer Security (TLS) support (Experimental - no pooling) | -| tls_cert_file | | String | No | Certificate file for TLS. This file must be owned by either the user running pgagroal or root. | -| tls_key_file | | String | No | Private key file for TLS. This file must be owned by either the user running pgagroal or root. Additionally permissions must be at least `0640` when owned by root or `0600` otherwise. | -| tls_ca_file | | String | No | Certificate Authority (CA) file for TLS. This file must be owned by either the user running pgagroal or root. | +| tls | `off` | Bool | No | Enable Transport Layer Security (TLS) support (Experimental - no pooling). Changes require restart. | +| tls_cert_file | | String | No | Certificate file for TLS. This file must be owned by either the user running pgagroal or root. Changes require restart. | +| tls_key_file | | String | No | Private key file for TLS. This file must be owned by either the user running pgagroal or root. Additionally permissions must be at least `0640` when owned by root or `0600` otherwise.Changes require restart. | +| tls_ca_file | | String | No | Certificate Authority (CA) file for TLS. This file must be owned by either the user running pgagroal or root. Changes require restart. | Note, that if `host` starts with a `/` it represents a path and `pgagroal` will connect using a Unix Domain Socket. diff --git a/doc/man/pgagroal.conf.5.rst b/doc/man/pgagroal.conf.5.rst index c36ed259..d0aeb1de 100644 --- a/doc/man/pgagroal.conf.5.rst +++ b/doc/man/pgagroal.conf.5.rst @@ -129,16 +129,16 @@ failover_script The failover script tls - Enable Transport Layer Security (TLS). Default is false + Enable Transport Layer Security (TLS). Default is false. Changes require restart in the server section. tls_cert_file - Certificate file for TLS + Certificate file for TLS. Changes require restart in the server section. tls_key_file - Private key file for TLS + Private key file for TLS. Changes require restart in the server section. tls_ca_file - Certificate Authority (CA) file for TLS + Certificate Authority (CA) file for TLS. Changes require restart in the server section. libev The libev backend to use. Valid options: auto, select, poll, epoll, iouring, devpoll and port. Default is auto diff --git a/src/libpgagroal/configuration.c b/src/libpgagroal/configuration.c index 2cdc64e7..bdd5ebca 100644 --- a/src/libpgagroal/configuration.c +++ b/src/libpgagroal/configuration.c @@ -79,12 +79,14 @@ static void copy_server(struct server* dst, struct server* src); static void copy_hba(struct hba* dst, struct hba* src); static void copy_user(struct user* dst, struct user* src); static int restart_int(char* name, int e, int n); +static int restart_bool(char* name, bool e, bool n); static int restart_string(char* name, char* e, char* n, bool skip_non_existing); static int restart_limit(char* name, struct configuration* config, struct configuration* reload); static int restart_server(struct server* src, struct server* dst); static bool is_empty_string(char* s); static bool is_same_server(struct server* s1, struct server* s2); +static bool is_same_tls(struct server* s1, struct server* s2); static bool key_in_section(char* wanted, char* section, char* key, bool global, bool* unknown); static bool is_comment_line(char* line); @@ -2395,6 +2397,26 @@ is_same_server(struct server* s1, struct server* s2) } } +/** + * Checks if TLS configurations are same. + * @return true if the TLS configurations are same +*/ +static bool +is_same_tls(struct server* src, struct server* dst) +{ + if (src->tls == dst->tls && + !strncmp(src->tls_cert_file, dst->tls_cert_file, MISC_LENGTH) && + !strncmp(src->tls_key_file, dst->tls_key_file, MISC_LENGTH) && + !strncmp(src->tls_ca_file, dst->tls_ca_file, MISC_LENGTH)) + { + return true; + } + else + { + return false; + } +} + static void copy_server(struct server* dst, struct server* src) { @@ -2446,6 +2468,22 @@ restart_int(char* name, int e, int n) return 0; } +/** + * Utility function prints a line in the log when a restart is required. + * @return 0 when parameter values are same, 1 when a restart required. +*/ +static int +restart_bool(char* name, bool e, bool n) +{ + if (e != n) + { + pgagroal_log_info("Restart required for %s - Existing %s New %s", name, e ? "true" : "false", n ? "true" : "false"); + return 1; + } + + return 0; +} + /** * Utility function to notify when a string parameter in the * configuration requires a restart. @@ -2524,7 +2562,18 @@ restart_server(struct server* src, struct server* dst) restart_string(restart_message, dst->host, src->host, false); snprintf(restart_message, sizeof(restart_message), "Server <%s>, parameter ", src->name); restart_int(restart_message, dst->port, src->port); - /* TODO - TLS */ + return 1; + } + else if (!is_same_tls(src, dst)) + { + snprintf(restart_message, sizeof(restart_message), "Server <%s>, parameter ", src->name); + restart_bool(restart_message, dst->tls, src->tls); + snprintf(restart_message, sizeof(restart_message), "Server <%s>, parameter ", src->name); + restart_string(restart_message, dst->tls_cert_file, src->tls_cert_file, false); + snprintf(restart_message, sizeof(restart_message), "Server <%s>, parameter ", src->name); + restart_string(restart_message, dst->tls_key_file, src->tls_key_file, false); + snprintf(restart_message, sizeof(restart_message), "Server <%s>, parameter ", src->name); + restart_string(restart_message, dst->tls_ca_file, src->tls_ca_file, false); return 1; }