From 2a1df4ddb2419fd67e72400b15bc9517099de061 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Thu, 5 Nov 2020 12:05:07 +0000 Subject: [PATCH] Breaking: Drop privileges after loading the configuration This change means privileges are dropped before loading certificates, starting logging, creating the pid file etc. are carried out, so all of those actions must now be changed to ensure that the unprivileged user can carry them out. --- ChangeLog.txt | 14 +++++++++ README-letsencrypt.md | 17 +++++++++++ misc/letsencrypt/mosquitto-copy.sh | 28 +++++++++++++++++ src/logging.c | 4 --- src/mosquitto.c | 49 ++++++------------------------ src/mosquitto_broker_internal.h | 3 +- 6 files changed, 70 insertions(+), 45 deletions(-) create mode 100644 README-letsencrypt.md create mode 100755 misc/letsencrypt/mosquitto-copy.sh diff --git a/ChangeLog.txt b/ChangeLog.txt index add4924f..60c1f5b8 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -22,6 +22,20 @@ Breaking changes: method, or set `allow_anonymous true`. When the broker is run without a configured listener, and so binds to the loopback interface, anonymous connections are allowed. +- If Mosquitto is run on as root on a unix like system, it will attempt to + drop privileges as soon as the configuration file has been read. This is in + contrast to the previous behaviour where elevated privileges were only + dropped after listeners had been started (and hence TLS certificates loaded) + and logging had been started. The change means that clients will never be + able to connect to the broker when it is running as root, unless the user + explicitly sets it to run as root, which is not advised. It also means that + all locations that the broker needs to access must be available to the + unprivileged user. In particular those people using TLS certificates from + Lets Encrypt will need to do something to allow Mosquitto to access + those certificates. An example deploy renewal hook script to help with this + is at `misc/letsencrypt/mosquitto-copy.sh`. + The user that Mosquitto will change to are the one provided in the + configuration, `mosquitto`, or `nobody`, in order of availability. - The `pid_file` option will now always attempt to write a pid file, regardless of whether the `-d` argument is used when running the broker. - The `tls_version` option now defines the *minimum* TLS protocol version to diff --git a/README-letsencrypt.md b/README-letsencrypt.md new file mode 100644 index 00000000..5fbddfc8 --- /dev/null +++ b/README-letsencrypt.md @@ -0,0 +1,17 @@ +# Using Lets Encrypt with Mosquitto + +On Unix like operating systems, Mosquitto will attempt to drop root access as +soon as it has loaded its configuration file, but before it has activated any +of that configuration. This means that if you are using Lets Encrypt TLS +certificates, it will be unable to access the certificates and private keys +typically located in /etc/letsencrypt/live/ + +To help with this problem there is an example `deploy` renewal hook script in +`misc/letsencrypt/mosquitto-copy.sh` which shows how the certificate and +private key for a mosquitto broker can be copied to /etc/mosquitto/certs/ and +given the correct ownership and permissions so the broker can access them, but +no other user can. It then signals Mosquitto to reload the certificates. + +Use of this script allows you to happily use Lets Encrypt certificates with +Mosquitto without needing root access for Mosquitto, and without having to +restart Mosquitto. diff --git a/misc/letsencrypt/mosquitto-copy.sh b/misc/letsencrypt/mosquitto-copy.sh new file mode 100755 index 00000000..c9482913 --- /dev/null +++ b/misc/letsencrypt/mosquitto-copy.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +# This is an example deploy renewal hook for certbot that copies newly updated +# certificates to the Mosquitto certificates directory and sets the ownership +# and permissions so only the mosquitto user can access them, then signals +# Mosquitto to reload certificates. + +# RENEWED_DOMAINS will match the domains being renewed for that certificate, so +# may be just "example.com", or multiple domains "www.example.com example.com" +# depending on your certificate. + +# Place this script in /etc/letsencrypt/renewal-hoots/deploy/ and make it +# executable after editing it to your needs. + +if [ ${RENEWED_DOMAINS} == "my-mosquitto-domain" ]; then + # Copy new certificate to Mosquitto directory + cp ${RENEWED_LINEAGE}/fullchain.pem /etc/mosquitto/certs/server.pem + cp ${RENEWED_LINEAGE}/privkey.pem /etc/mosquitto/certs/server.key + + # Set ownership to Mosquitto + chown mosquitto: /etc/mosquitto/certs/server.pem /etc/mosquitto/certs/server.key + + # Ensure permissions are restrictive + chmod 0600 /etc/mosquitto/certs/server.pem /etc/mosquitto/certs/server.key + + # Tell Mosquitto to reload certificates and configuration + pkill -HUP -x mosquitto +fi diff --git a/src/logging.c b/src/logging.c index 3c26d8fe..3877577e 100644 --- a/src/logging.c +++ b/src/logging.c @@ -130,9 +130,6 @@ int log__init(struct mosquitto__config *config) } if(log_destinations & MQTT3_LOG_FILE){ - if(drop_privileges(config, true)){ - return 1; - } config->log_fptr = mosquitto__fopen(config->log_file, "at", true); if(config->log_fptr){ setvbuf(config->log_fptr, NULL, _IOLBF, 0); @@ -141,7 +138,6 @@ int log__init(struct mosquitto__config *config) log_priorities = MOSQ_LOG_ERR; log__printf(NULL, MOSQ_LOG_ERR, "Error: Unable to open log file %s for writing.", config->log_file); } - restore_privileges(); } #ifdef WITH_DLT dlt_fifo_check(); diff --git a/src/mosquitto.c b/src/mosquitto.c index 427bd715..70a3fe3a 100644 --- a/src/mosquitto.c +++ b/src/mosquitto.c @@ -88,7 +88,7 @@ struct mosquitto_db *mosquitto__get_db(void) * Note that setting config->user to "root" does not produce an error, but it * strongly discouraged. */ -int drop_privileges(struct mosquitto__config *config, bool temporary) +int drop_privileges(struct mosquitto__config *config) { #if !defined(__CYGWIN__) && !defined(WIN32) struct passwd *pwd; @@ -122,21 +122,13 @@ int drop_privileges(struct mosquitto__config *config, bool temporary) log__printf(NULL, MOSQ_LOG_ERR, "Error setting groups whilst dropping privileges: %s.", err); return 1; } - if(temporary){ - rc = setegid(pwd->pw_gid); - }else{ - rc = setgid(pwd->pw_gid); - } + rc = setgid(pwd->pw_gid); if(rc == -1){ err = strerror(errno); log__printf(NULL, MOSQ_LOG_ERR, "Error setting gid whilst dropping privileges: %s.", err); return 1; } - if(temporary){ - rc = seteuid(pwd->pw_uid); - }else{ - rc = setuid(pwd->pw_uid); - } + rc = setuid(pwd->pw_uid); if(rc == -1){ err = strerror(errno); log__printf(NULL, MOSQ_LOG_ERR, "Error setting uid whilst dropping privileges: %s.", err); @@ -151,31 +143,6 @@ int drop_privileges(struct mosquitto__config *config, bool temporary) return MOSQ_ERR_SUCCESS; } -int restore_privileges(void) -{ -#if !defined(__CYGWIN__) && !defined(WIN32) - char *err; - int rc; - - if(getuid() == 0){ - rc = setegid(0); - if(rc == -1){ - err = strerror(errno); - log__printf(NULL, MOSQ_LOG_ERR, "Error setting gid whilst restoring privileges: %s.", err); - return 1; - } - rc = seteuid(0); - if(rc == -1){ - err = strerror(errno); - log__printf(NULL, MOSQ_LOG_ERR, "Error setting uid whilst restoring privileges: %s.", err); - return 1; - } - } -#endif - return MOSQ_ERR_SUCCESS; -} - - void mosquitto__daemonise(void) { #ifndef WIN32 @@ -485,6 +452,13 @@ int main(int argc, char *argv[]) if(rc != MOSQ_ERR_SUCCESS) return rc; int_db.config = &config; + /* Drop privileges permanently immediately after the config is loaded. + * This requires the user to ensure that all certificates, log locations, + * etc. are accessible my the `mosquitto` or other unprivileged user. + */ + rc = drop_privileges(&config); + if(rc != MOSQ_ERR_SUCCESS) return rc; + if(config.daemon){ mosquitto__daemonise(); } @@ -533,9 +507,6 @@ int main(int argc, char *argv[]) if(listeners__start(&int_db, &listensock, &listensock_count)) return 1; - rc = drop_privileges(&config, false); - if(rc != MOSQ_ERR_SUCCESS) return rc; - signal__setup(); #ifdef WITH_BRIDGE diff --git a/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 8d9f2cfe..70de751a 100644 --- a/src/mosquitto_broker_internal.h +++ b/src/mosquitto_broker_internal.h @@ -643,8 +643,7 @@ int config__read(struct mosquitto_db *db, struct mosquitto__config *config, bool void config__cleanup(struct mosquitto__config *config); int config__get_dir_files(const char *include_dir, char ***files, int *file_count); -int drop_privileges(struct mosquitto__config *config, bool temporary); -int restore_privileges(void); +int drop_privileges(struct mosquitto__config *config); /* ============================================================ * Server send functions