Fix checksum race condition in check-for-changes.sh

If a change to one of the tracked files happened soon after (<1 second?)
a previously detected change, it could end up going undetected. In
particular, this could cause integration tests to fail (see next
commits).

Fixed by computing the new checksum file _before_ checking for changes.

target/check-for-changes.sh

@@ -16,7 +16,6 @@ if [ ! -f postfix-accounts.cf ]; then
 fi
 
 # Verify checksum file exists; must be prepared by start-mailserver.sh
-CHKSUM_FILE=/tmp/docker-mailserver-config-chksum
 if [ ! -f $CHKSUM_FILE ]; then
    echo "${log_date} ${CHKSUM_FILE} is missing! Start script failed? Exit!"
    exit
@@ -32,12 +31,6 @@ fi
 PM_ADDRESS="${POSTMASTER_ADDRESS:=postmaster@${DOMAINNAME}}"
 echo "${log_date} Using postmaster address ${PM_ADDRESS}"
 
-# Create an array of files to monitor, must be the same as in start-mailserver.sh
-declare -a cf_files=()
-for file in postfix-accounts.cf postfix-virtual.cf postfix-aliases.cf dovecot-quotas.cf /etc/letsencrypt/acme.json "/etc/letsencrypt/live/$HOSTNAME/key.pem" "/etc/letsencrypt/live/$HOSTNAME/fullchain.pem"; do
-  [ -f "$file" ] && cf_files+=("$file")
-done
-
 # Wait to make sure server is up before we start
 sleep 10
 
@@ -48,10 +41,12 @@ while true; do
 log_date=$(date +"%Y-%m-%d %H:%M:%S ")
 
 # Get chksum and check it, no need to lock config yet
-chksum=$(sha512sum -c --ignore-missing $CHKSUM_FILE)
+monitored_files_checksums >"$CHKSUM_FILE.new"
 
-if [[ $chksum == *"FAIL"* ]]; then
+if ! cmp --silent -- "$CHKSUM_FILE" "$CHKSUM_FILE.new"; then
 	echo "${log_date} Change detected"
+  changed=$(grep -Fxvf "$CHKSUM_FILE" "$CHKSUM_FILE.new" | sed 's/^[^ ]\+  //')
+  mv "$CHKSUM_FILE.new" "$CHKSUM_FILE"
 
 	# Bug alert! This overwrites the alias set by start-mailserver.sh
 	# Take care that changes in one script are propagated to the other
@@ -63,13 +58,18 @@ if [[ $chksum == *"FAIL"* ]]; then
         (
           flock -e 200
 
-  if [[ $chksum == *"/etc/letsencrypt/acme.json: FAILED"* ]]; then
-    for certdomain in $SSL_DOMAIN $HOSTNAME $DOMAINNAME; do
-      if extractCertsFromAcmeJson "$certdomain"; then
-        break
-      fi
-    done
-  fi
+  for file in $changed; do
+    case $file in
+    /etc/letsencrypt/acme.json)
+      for certdomain in $SSL_DOMAIN $HOSTNAME $DOMAINNAME; do
+        if extractCertsFromAcmeJson "$certdomain"; then
+          break
+        fi
+      done
+      ;;
+    #TODO: Perform updates below conditionally as well.
+    esac
+  done
 
 	#regen postix aliases.
 	echo "root: ${PM_ADDRESS}" > /etc/aliases
@@ -211,9 +211,6 @@ if [[ $chksum == *"FAIL"* ]]; then
 		supervisorctl restart dovecot
 	fi
 
-	echo "${log_date} Update checksum"
-	sha512sum ${cf_files[@]/#/--tag } >$CHKSUM_FILE
-
         ) 200<postfix-accounts.cf # end lock
 fi
 

target/helper_functions.sh

@@ -73,3 +73,23 @@ for key, value in acme.items():
     return 1
   fi
 }
+
+# File storing the checksums of the monitored files.
+CHKSUM_FILE=/tmp/docker-mailserver-config-chksum
+
+# Compute checksums of monitored files.
+function monitored_files_checksums() {
+  (
+    cd /tmp/docker-mailserver
+    # (2>/dev/null to ignore warnings about files that don't exist)
+    exec sha512sum 2>/dev/null -- \
+           postfix-accounts.cf \
+           postfix-virtual.cf \
+           postfix-aliases.cf \
+           dovecot-quotas.cf \
+           /etc/letsencrypt/acme.json \
+           "/etc/letsencrypt/live/$HOSTNAME/key.pem" \
+           "/etc/letsencrypt/live/$HOSTNAME/fullchain.pem"
+  )
+  return 0
+}

target/start-mailserver.sh

@@ -500,19 +500,9 @@ function _setup_file_permissions() {
 function _setup_chksum_file() {
         notify 'task' "Setting up configuration checksum file"
 
-
         if [ -d /tmp/docker-mailserver ]; then
-          pushd /tmp/docker-mailserver
-
-          declare -a cf_files=()
-          for file in postfix-accounts.cf postfix-virtual.cf postfix-aliases.cf dovecot-quotas.cf /etc/letsencrypt/acme.json "/etc/letsencrypt/live/$HOSTNAME/key.pem" "/etc/letsencrypt/live/$HOSTNAME/fullchain.pem"; do
-            [ -f "$file" ] && cf_files+=("$file")
-          done
-
           notify 'inf' "Creating $CHKSUM_FILE"
-          sha512sum ${cf_files[@]/#/--tag } >$CHKSUM_FILE
-
-          popd
+          monitored_files_checksums >"$CHKSUM_FILE"
         else
           # We could just skip the file, but perhaps config can be added later?
           # If so it must be processed by the check for changes script

test/mail_ssl_letsencrypt.bats

@@ -118,7 +118,7 @@ function teardown_file() {
   assert_output --partial "Cert found in /etc/letsencrypt/acme.json for *.example.com"
   assert_output --partial "postfix: stopped"
   assert_output --partial "postfix: started"
-  assert_output --partial "Update checksum"
+  assert_output --partial "Change detected"
 
   run docker exec mail_lets_acme_json /bin/bash -c "cat /etc/letsencrypt/live/mail.my-domain.com/key.pem"
   assert_output "$(cat "`pwd`/test/config/letsencrypt/changed/key.pem")"

test/tests.bats

@@ -20,7 +20,7 @@ function wait_for_service() {
 
 function count_processed_changes() {
   containerName=$1
-  docker exec $containerName cat /var/log/supervisor/changedetector.log | grep "Update checksum" | wc -l
+  docker exec $containerName cat /var/log/supervisor/changedetector.log | grep "Change detected" | wc -l
 }
 
 #