From 43d63bd5a1744917706f87d798eb1323bc9d3153 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Tue, 14 Dec 2021 20:00:48 +0000 Subject: [PATCH] puppet: Always set the RabbitMQ nodename to zulip@localhost. This is required in order to lock down the RabbitMQ port to only listen on localhost. If the nodename is `rabbit@hostname`, in most circumstances the hostname will resolve to an external IP, which the rabbitmq port will not be bound to. Installs which used `rabbit@hostname`, due to RabbitMQ having been installed before Zulip, would not have functioned if the host or RabbitMQ service was restarted, as the localhost restrictions in the RabbitMQ configuration would have made rabbitmqctl (and Zulip cron jobs that call it) unable to find the rabbitmq server. The previous commit ensures that configure-rabbitmq is re-run after the nodename has changed. However, rabbitmq needs to be stopped before `rabbitmq-env.conf` is changed; we use an `onlyif` on an `exec` to print the warning about the node change, and let the subsequent config change and notify of the service and configure-rabbitmq to complete the re-configuration. --- docs/overview/changelog.md | 5 ++ docs/production/deployment.md | 6 --- .../rabbitmq/rabbitmq-env.conf} | 8 +--- puppet/zulip/manifests/profile/rabbitmq.pp | 47 +++++++++++-------- scripts/lib/install | 23 --------- scripts/lib/warn-rabbitmq-nodename-change | 29 ++++++++++++ 6 files changed, 63 insertions(+), 55 deletions(-) rename puppet/zulip/{templates/rabbitmq-env.conf.template.erb => files/rabbitmq/rabbitmq-env.conf} (73%) create mode 100755 scripts/lib/warn-rabbitmq-nodename-change diff --git a/docs/overview/changelog.md b/docs/overview/changelog.md index b71f8a52c3..6a87c63c95 100644 --- a/docs/overview/changelog.md +++ b/docs/overview/changelog.md @@ -121,6 +121,11 @@ log][commit-log] for an up-to-date list of raw changes. ## Zulip 4.x series +- Removed the `rabbitmq.nodename` configuration in `zulip.conf`; all + RabbitMQ instances will be reconfigured to have a nodename of + `zulip@localhost`. You can remove this setting from your + `zulip.conf` configuration file, if it exists. + ## Zulip 4.8 -- 2021-12-01 - CVE-2021-43791: Zulip could fail to enforce expiration dates diff --git a/docs/production/deployment.md b/docs/production/deployment.md index c0edb81858..d847c11555 100644 --- a/docs/production/deployment.md +++ b/docs/production/deployment.md @@ -728,12 +728,6 @@ connections. The version of PostgreSQL that is in use. Do not set by hand; use the [PostgreSQL upgrade tool](../production/upgrade-or-modify.html#upgrading-postgresql). -### `[rabbitmq]` - -#### `nodename` - -The name used to identify the local RabbitMQ server; do not modify. - ### `[memcached]` #### `memory` diff --git a/puppet/zulip/templates/rabbitmq-env.conf.template.erb b/puppet/zulip/files/rabbitmq/rabbitmq-env.conf similarity index 73% rename from puppet/zulip/templates/rabbitmq-env.conf.template.erb rename to puppet/zulip/files/rabbitmq/rabbitmq-env.conf index 5119b4c7d5..094b983a80 100644 --- a/puppet/zulip/templates/rabbitmq-env.conf.template.erb +++ b/puppet/zulip/files/rabbitmq/rabbitmq-env.conf @@ -3,12 +3,8 @@ # combination. See the clustering on a single machine guide for details: # http://www.rabbitmq.com/clustering.html#single-machine # -# By default, we set nodename to rabbit@localhost so it will always resolve -<% if @rabbitmq_nodename != '' -%> -NODENAME=<%= @rabbitmq_nodename %> -<% else -%> -NODENAME=rabbit -<% end -%> +# By default, we set nodename to zulip@localhost so it will always resolve +NODENAME=zulip@localhost # By default RabbitMQ will bind to all interfaces, on IPv4 and IPv6 if # available. Set this if you only want to bind to one network interface or# diff --git a/puppet/zulip/manifests/profile/rabbitmq.pp b/puppet/zulip/manifests/profile/rabbitmq.pp index 80131e6116..193732292f 100644 --- a/puppet/zulip/manifests/profile/rabbitmq.pp +++ b/puppet/zulip/manifests/profile/rabbitmq.pp @@ -16,6 +16,12 @@ class zulip::profile::rabbitmq { ensure => absent, } + file { '/etc/rabbitmq': + ensure => 'directory', + owner => 'root', + group => 'root', + mode => '0755', + } file { '/etc/rabbitmq/rabbitmq.config': ensure => file, require => Package[rabbitmq-server], @@ -24,26 +30,27 @@ class zulip::profile::rabbitmq { mode => '0644', source => 'puppet:///modules/zulip/rabbitmq/rabbitmq.config', } - - $rabbitmq_nodename = zulipconf('rabbitmq', 'nodename', '') - if $rabbitmq_nodename != '' { - file { '/etc/rabbitmq': - ensure => 'directory', - owner => 'root', - group => 'root', - mode => '0755', - } - - file { '/etc/rabbitmq/rabbitmq-env.conf': - ensure => file, - require => File['/etc/rabbitmq'], - before => [Package[rabbitmq-server], Service[rabbitmq-server]], - notify => Exec['configure-rabbitmq'], - owner => 'root', - group => 'root', - mode => '0644', - content => template('zulip/rabbitmq-env.conf.template.erb'), - } + exec { 'warn-rabbitmq-nodename-change': + command => "${::zulip_scripts_path}/lib/warn-rabbitmq-nodename-change", + onlyif => '[ -f /etc/rabbitmq/rabbitmq-env.conf ] && ! grep -xq NODENAME=zulip@localhost /etc/rabbitmq/rabbitmq-env.conf', + before => [ + File['/etc/rabbitmq/rabbitmq-env.conf'], + Service['rabbitmq-server'], + ], + logoutput => true, + loglevel => 'warning', + } + file { '/etc/rabbitmq/rabbitmq-env.conf': + ensure => file, + owner => 'root', + group => 'root', + mode => '0644', + source => 'puppet:///modules/zulip/rabbitmq/rabbitmq-env.conf', + before => Package['rabbitmq-server'], + notify => [ + Service['rabbitmq-server'], + Exec['configure-rabbitmq'], + ], } # epmd doesn't have an init script, so we just check if it is # running, and if it isn't, start it. Even in case of a race, this diff --git a/scripts/lib/install b/scripts/lib/install index 06715b9cc8..972f401173 100755 --- a/scripts/lib/install +++ b/scripts/lib/install @@ -433,29 +433,6 @@ EOF if [ -n "$POSTGRESQL_DATABASE_USER" ]; then crudini --set /etc/zulip/zulip.conf postgresql database_user "$POSTGRESQL_DATABASE_USER" fi - - # Note: there are four dpkg-query outputs to consider: - # - # root@host# dpkg-query --showformat '${Status}\n' -W rabbitmq-server 2>/dev/null - # root@host# apt install rabbitmq-server - # root@host# dpkg-query --showformat '${Status}\n' -W rabbitmq-server 2>/dev/null - # install ok installed - # root@host# apt remove rabbitmq-server - # root@host# dpkg-query --showformat '${Status}\n' -W rabbitmq-server 2>/dev/null - # deinstall ok config-files - # root@host# apt purge rabbitmq-server - # root@host# dpkg-query --showformat '${Status}\n' -W rabbitmq-server 2>/dev/null - # unknown ok not-installed - # - # (There are more possibilities in the case of dpkg errors.) Here - # we are checking for either empty or not-installed. - if ! dpkg-query --showformat '${Status}\n' -W rabbitmq-server 2>/dev/null | grep -vq ' not-installed$'; then - cat <>/etc/zulip/zulip.conf - -[rabbitmq] -nodename = zulip@localhost -EOF - fi fi if has_class "zulip::app_frontend_base"; then diff --git a/scripts/lib/warn-rabbitmq-nodename-change b/scripts/lib/warn-rabbitmq-nodename-change new file mode 100755 index 0000000000..63c4d388c9 --- /dev/null +++ b/scripts/lib/warn-rabbitmq-nodename-change @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +# For security reasons, we need to configure RabbitMQ to listen only +# on localhost, which we cannot do if the nodename contains the +# hostname (e.g. rabbit@zulip-host). Containing the hostname also +# makes it brittle to hostname changes, which are (for example) common +# in containerized environments. + +set -eu + +# Try to find the current nodename +CURRENT=$(sh -c 'cd /usr/lib/rabbitmq/bin/ && . ./rabbitmq-env && echo $NODENAME') +if [ "$CURRENT" != "zulip@localhost" ]; then + cat <