hooks: Add dkimpy support

This patch adds support in the default hook for using dkimpy for DKIM
signing.

Unfortunately, dkimpy binaries have the same name as driusan/dkim's, so
we need to use --help to disambiguate. It's not pretty but it should
work, and is quite self contained.

Also, for the integration tests, we still need driusan/dkim because
dkimpy lacks the features needed. Specifically, dkimpy's dkimverify
can't be made to use custom DNS, or override the TXT values in any way,
so we can't verify that the generated signature is reasonable.

Thanks to ne9z@github for suggesting this change and providing an
alternative patch in https://github.com/albertito/chasquid/pull/19.

docs/dkim.md

@@ -8,8 +8,8 @@ mechanism.
 ## Signing
 
 The example hook in this repository contains an example of integration with
-[driusan/dkim](https://github.com/driusan/dkim) tools, and assumes the
-following:
+[driusan/dkim](https://github.com/driusan/dkim) and
+[dkimpy](https://launchpad.net/dkimpy/), and assumes the following:
 
 - The [selector](https://tools.ietf.org/html/rfc6376#section-3.1) for a domain
   can be found in the file `domains/$DOMAIN/dkim_selector`.

etc/chasquid/hooks/post-data

@@ -7,7 +7,7 @@
 #  - spamc (from Spamassassin) to filter spam.
 #  - rspamc (from rspamd) or chasquid-rspamd to filter spam.
 #  - clamdscan (from ClamAV) to filter virus.
-#  - dkimsign (from driusan/dkim) to do DKIM signing.
+#  - dkimsign (from driusan/dkim or dkimpy) to do DKIM signing.
 #
 # If it exits with code 20, it will be considered a permanent error.
 # Otherwise, temporary.
@@ -79,7 +79,7 @@ if command -v clamdscan >/dev/null; then
         echo "X-Virus-Scanned: pass"
 fi
 
-# DKIM sign with https://github.com/driusan/dkim.
+# DKIM sign with either driusan/dkim or dkimpy.
 #
 # Do it only if all the following are true:
 #  - User has authenticated.
@@ -90,12 +90,34 @@ fi
 # Note this has not been thoroughly tested, so might need further adjustments.
 if [ "$AUTH_AS" != "" ] && command -v dkimsign >/dev/null; then
 	DOMAIN=$( echo "$MAIL_FROM" | cut -d '@' -f 2 )
+
 	if [ -f "domains/$DOMAIN/dkim_selector" ] \
-			&& [ -f "certs/$DOMAIN/dkim_privkey.pem" ]; then
-		dkimsign -n -hd \
-			-key "certs/$DOMAIN/dkim_privkey.pem" \
-			-s "$(cat "domains/$DOMAIN/dkim_selector")" \
-			-d "$DOMAIN" \
-			< "$TF"
+		&& [ -f "certs/$DOMAIN/dkim_privkey.pem" ];
+	then
+		# driusan/dkim and dkimpy both provide the same binary (dkimsign) but
+		# take different arguments, so we need to tell them apart.
+		# This is awful but it should work reasonably well.
+		if dkimsign --help 2>&1 | grep -q -- --identity; then
+			# dkimpy
+			dkimsign \
+				"$(cat "domains/$DOMAIN/dkim_selector")" \
+				"$DOMAIN" \
+				"certs/$DOMAIN/dkim_privkey.pem" \
+				< "$TF" > "$TF.dkimout"
+			# dkimpy doesn't provide a way to just show the new
+			# headers, so we have to compute the difference.
+			# ALSOCHANGE(test/t-19-dkimpy/config/hooks/post-data)
+			! diff --changed-group-format='%>' \
+				--unchanged-group-format='' \
+				"$TF" "$TF.dkimout"
+			rm "$TF.dkimout"
+		else
+			# driusan/dkim
+			dkimsign -n -hd \
+				-key "certs/$DOMAIN/dkim_privkey.pem" \
+				-s "$(cat "domains/$DOMAIN/dkim_selector")" \
+				-d "$DOMAIN" \
+				< "$TF"
+		fi
 	fi
 fi

test/Dockerfile

@@ -26,7 +26,8 @@ RUN apt-get install -y -q python3 msmtp
 RUN apt-get install -y -q \
 	gettext-base dovecot-imapd \
 	exim4-daemon-light \
-	haproxy
+	haproxy \
+	python3-dkim
 
 # Install sudo, needed for the docker entrypoint.
 RUN apt-get install -y -q sudo

test/t-19-dkimpy/config/chasquid.conf

@@ -0,0 +1,9 @@
+smtp_address: ":1025"
+submission_address: ":1587"
+monitoring_address: ":1099"
+
+mail_delivery_agent_bin: "test-mda"
+mail_delivery_agent_args: "%to%"
+
+data_dir: "../.data"
+mail_log_path: "../.logs/mail_log"

test/t-19-dkimpy/config/domains/testserver/dkim_selector

@@ -0,0 +1 @@
+testselector1

test/t-19-dkimpy/config/hooks/post-data

@@ -0,0 +1,43 @@
+#!/bin/bash
+
+# If authenticated, sign; otherwise, verify.
+#
+# It is not recommended that we fail delivery on dkim verification failures,
+# but leave it to the MUA to handle verifications.
+# https://tools.ietf.org/html/rfc6376#section-2.2
+#
+# We do a verification here so we have a stronger integration test (check
+# encodings/dot-stuffing/etc. works ok), but it's not recommended for general
+# purposes.
+
+set -e
+
+TF="$(mktemp --tmpdir post-data-XXXXXXXXXX)"
+trap 'rm "$TF"' EXIT
+
+# Save the message to the temporary file.
+cat > "$TF"
+
+if [ "$AUTH_AS" != "" ]; then
+	DOMAIN=$( echo "$MAIL_FROM" | cut -d '@' -f 2 )
+
+	# Call /usr/bin/dkimsign directly to prevent a conflict with
+	# driusan/dkim, which the integration tests install in ~/go/bin.
+	/usr/bin/dkimsign \
+		"$(cat "domains/$DOMAIN/dkim_selector")" \
+		"$DOMAIN" \
+		"../.dkimcerts/private.key" \
+		< "$TF" > "$TF.dkimout"
+	# dkimpy doesn't provide a way to just show the new headers, so we
+	# have to compute the difference.
+	# ALSOCHANGE(etc/chasquid/hooks/post-data)
+	! diff --changed-group-format='%>' \
+		--unchanged-group-format='' \
+		"$TF" "$TF.dkimout"
+	rm "$TF.dkimout"
+else
+	# NOTE: This is using driusan/dkim instead of dkimpy, because dkimpy can't be
+	# overriden to get the DNS information from anywhere else (text file or custom
+	# DNS server).
+	dkimverify -txt ../.dkimcerts/private.dns < "$TF"
+fi

test/t-19-dkimpy/content

@@ -0,0 +1,9 @@
+Subject: Prueba desde el test
+To: someone@testserver
+
+Crece desde el test el futuro
+Crece desde el test
+
+.
+
+El punto de arriba testea el dot-stuffing, que es importante para DKIM.

test/t-19-dkimpy/hosts

@@ -0,0 +1 @@
+testserver localhost

test/t-19-dkimpy/msmtprc

@@ -0,0 +1,14 @@
+account default
+
+host testserver
+port 1587
+
+tls on
+tls_trust_file config/certs/testserver/fullchain.pem
+
+from user@testserver
+
+auth on
+user user@testserver
+password secretpassword
+

test/t-19-dkimpy/run.sh

@@ -0,0 +1,64 @@
+#!/bin/bash
+#
+# Test integration with dkimpy.
+
+set -e
+. $(dirname ${0})/../util/lib.sh
+
+init
+check_hostaliases
+
+# Check if dkimpy tools are installed in /usr/bin, and driusan/dkim is
+# installed somewhere else in $PATH.
+#
+# Unfortunately we need both because dkimpy's dkimverify lacks the features
+# needed to use it in integration testing.
+#
+# We need to run them and check the help because there are other binaries with
+# the same name.
+# This is really hacky but the most practical way to handle it, since they
+# both have the same binary names.
+if ! /usr/bin/dkimsign --help 2>&1 | grep -q -- --identity; then
+	skip "/usr/bin/dkimsign is not dkimpy's"
+fi
+if ! dkimverify --help 2>&1 < /dev/null | grep -q -- "-txt string"; then
+	skip "dkimverify is not driusan/dkim's"
+fi
+
+generate_certs_for testserver
+( mkdir -p .dkimcerts; cd .dkimcerts; dknewkey private > log 2>&1 )
+
+add_user user@testserver secretpassword
+add_user someone@testserver secretpassword
+
+mkdir -p .logs
+chasquid -v=2 --logfile=.logs/chasquid.log --config_dir=config &
+wait_until_ready 1025
+
+# Authenticated: user@testserver -> someone@testserver
+# Should be signed.
+run_msmtp someone@testserver < content
+wait_for_file .mail/someone@testserver
+mail_diff content .mail/someone@testserver
+grep -q "DKIM-Signature:" .mail/someone@testserver
+
+# Verify the signature manually, just in case.
+# NOTE: This is using driusan/dkim instead of dkimpy, because dkimpy can't be
+# overriden to get the DNS information from anywhere else (text file or custom
+# DNS server).
+dkimverify -txt .dkimcerts/private.dns < .mail/someone@testserver
+
+# Save the signed mail so we can verify it later.
+# Drop the first line ("From blah") so it can be used as email contents.
+tail -n +2 .mail/someone@testserver > .signed_content
+
+# Not authenticated: someone@testserver -> someone@testserver
+smtpc.py --server=localhost:1025 < .signed_content
+
+# Check that the signature fails on modified content.
+echo "Added content, invalid and not signed" >> .signed_content
+if smtpc.py --server=localhost:1025 < .signed_content 2> /dev/null; then
+	fail "DKIM verification succeeded on modified content"
+fi
+
+success