From a07f64a463e16c1ca123c02013f99c6d2d9fa2f3 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 fff385213a..99bcc0b970 100644 --- a/docs/overview/changelog.md +++ b/docs/overview/changelog.md @@ -7,6 +7,11 @@ 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 145f6a5e45..601fabd63c 100644 --- a/docs/production/deployment.md +++ b/docs/production/deployment.md @@ -641,12 +641,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 05160c02f4..3bd27c2640 100755 --- a/scripts/lib/install +++ b/scripts/lib/install @@ -410,29 +410,6 @@ EOF if ! has_class "zulip::postgresql_common" || [ "$package_system" != apt ]; then crudini --del /etc/zulip/zulip.conf postgresql 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 <