Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds DKIM support for multiple domains #89

Merged
merged 18 commits into from
Feb 13, 2021
Merged

Adds DKIM support for multiple domains #89

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
498b549
generating a DKIM key for all virtualDomains
dgraziotin Feb 9, 2021
ae3f4a4
including HOSTNAME in folder of domains for DKIM
dgraziotin Feb 9, 2021
4db0f91
KeyTable, SigningTable, TrustedHosts for HOSTNAME and all virtualDomain
dgraziotin Feb 9, 2021
4906208
Generate new DKIM data only if keys do not exist yet
dgraziotin Feb 9, 2021
38e1ec3
disabled opendkim.conf settings for single domain, added KeyTable,Sig…
dgraziotin Feb 9, 2021
3a989e0
Correct permissions of DKIM files regardless of prior creation
dgraziotin Feb 9, 2021
0105cf9
Added test for multiple domains and DKIM. Ready for #88
dgraziotin Feb 9, 2021
05e462e
Updated README on DKIM for multiple domains
dgraziotin Feb 9, 2021
99f2776
Fixed indentation on entrypoint
dgraziotin Feb 10, 2021
42a7fe7
Fixed wrong indentation (style)
dgraziotin Feb 10, 2021
6686475
Cleaner handling of multiple DKIM keys. No settings required. Renders…
dgraziotin Feb 10, 2021
926d640
Making sure we never insert the same config twice #89
dgraziotin Feb 10, 2021
0c4c988
Forgot one last mention of SMF_DKIM_ALL
dgraziotin Feb 10, 2021
100115e
Better tld naming for DKIM in README
dgraziotin Feb 11, 2021
10a9f9b
DKIM test no longer changes working directory
dgraziotin Feb 11, 2021
9147207
More elegant generation of DKIM entries for HOSTNAME and virtual domains
dgraziotin Feb 11, 2021
93f3a14
Correct switch to suppress grep complains when files miss
dgraziotin Feb 12, 2021
24e4d32
Update VERSION
huan Feb 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,15 @@ It is highly advised to mount `/var/db/dkim/` folder to host, so generated keypa
docker run -e SMF_CONFIG="$SMF_CONFIG" -p 25:25 -v $(pwd)/dkim:/var/db/dkim/ zixia/simple-mail-forwarder
```

DKIM and multiple domains
=========================

If `$SMF_DKIM_ALL` is defined (any value will do, including `1`), SMF will generate private/public keypairs for `$SMF_DOMAIN` and for all source domains contained in `SMF_CONFIG`. All keys will be stored in `/var/db/dkim/<domain.ext>/`.

This will enable DKIM for multiple domains and test for their validity on SMF startup.

If a DKIM key was already present for `$SMF_DOMAIN` under `/var/db/dkim/`, it will be copied under `/var/db/dkim/{$SMF_DOMAIN}`.

Helper Scripts
--------------------

Expand Down
52 changes: 50 additions & 2 deletions entrypoint.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Environment Variables:
SMF_CONFIG - mail forward addresses mapping list.
SMF_MYNETWORKS - configure relaying from trusted IPs, see http://www.postfix.org/postconf.5.html#mynetworks
SMF_RELAYHOST - configure a relayhost
SMF_DKIM_ALL - If defined, generate a DKIM key for all domains found in SMF_CONFIG, in addition to the one in SMF_DOMAIN

this creates a new smtp server which listens on port 25,
forward all email from
Expand Down Expand Up @@ -190,8 +191,7 @@ function start_postfix {

postfix start


# DKIM
# DKIM only for $HOSTNAME
if [ ! -f /var/db/dkim/default.private ]; then
mkdir -p /var/db/dkim
echo "OpenDKIM: Keys not found, generating..."
Expand All @@ -203,8 +203,56 @@ function start_postfix {
echo "OpenDKIM: Add TXT record to DNS:"
cat /var/db/dkim/default.txt
fi


sed -n -e '/^Domain\s/!p' -e '$aDomain '$HOSTNAME -i /etc/opendkim/opendkim.conf
# DKIM for all virtual domains and $HOSTNAME
if [ "$SMF_DKIM_ALL" != "" ]; then
if [ ! -f /var/db/dkim/$HOSTNAME/default.private ]; then
echo "Moving ${HOSTNAME} keys to /var/db/dkim/$HOSTNAME/"
mkdir -p /var/db/dkim/$HOSTNAME
cp /var/db/dkim/default.* /var/db/dkim/$HOSTNAME
fi
chmod 400 /var/db/dkim/default.private
chown opendkim:opendkim /var/db/dkim/default.private
echo "Inserting ${HOSTNAME} data to /etc/opendkim/{KeyTable, SigningTable, TrustedHosts}"
echo "default._domainkey.${HOSTNAME} ${HOSTNAME}:default:/var/db/dkim/${HOSTNAME}/default.private
" >> /etc/opendkim/KeyTable

echo "${HOSTNAME} default._domainkey.${HOSTNAME}" >> /etc/opendkim/SigningTable

echo "${HOSTNAME}" >> /etc/opendkim/TrustedHosts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the intention. And if you append with >>, does it mean that on every SMF restart new lines will be added to /etc/opendkim/{KeyTable, SigningTable, TrustedHosts}?

Copy link
Contributor Author

@dgraziotin dgraziotin Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking! Indentation fixed at line 220, unless you meant something else.

The >> is required, because for all files {KeyTable, SigningTable, TrustedHosts} I am inserting infos on multiple occasions:

  • first for the entry your PR creates (the ${HOSTNAME})
  • over a loop for all virtualDomain in $virtualDomains

All new files in /etc/opendkim/ are re-created at each restart or down/up. They are dynamic according to the inserted domains, so there is no danger of inserting new lines.

docker exec -it smf cat /etc/opendkim/KeyTable | wc -l
10
docker-compose restart
docker exec -it smf cat /etc/opendkim/KeyTable | wc -l
10

Copy link
Contributor

@petslane petslane Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a good fix, same thing on line 234.

But I meant that the whole block of code from lines 216-223 and 231-242 is intented:
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whops! Thanks for pointing it out. Fixed.


for virtualDomain in $virtualDomains; do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HOSTNAME is probably also in $virtualDomains, so I suspect that configuration for HOSTNAME is done twice.

Copy link
Contributor Author

@dgraziotin dgraziotin Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand from here, HOSTNAME is not part of $virtualDomains. I did check for this empirically, to be sure. A folder /var/db/dkim/${HOSTNAME}/ was not created in a for loop over $virtualDomains.

In any case, I think that this handles the case of reverting to a single domain (your PR), because HOSTNAME and all forwarded domains are kept separately.

Copy link
Contributor

@petslane petslane Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

    - SMF_CONFIG=@example.com:xxx@gmail.com:pwd
    - SMF_DOMAIN=example.com

This is my config, based on https://github.com/dgraziotin/docker-simple-mail-forwarder/blob/99f27761e49e131a52b03159d69e7c652d25ffff/entrypoint.sh#L101 virtualDomains is a list of domains extracted from emailFrom which is the first element when split with :. I have to actually test but I'm quite sure that with my config, example.com is in virtualDomains and SMF_DOMAIN.

And what do you think, is #83 actually needed? Maybe dkim should always use a per-domain path style? Even if there is only one domain? This would mean removing this and removing SMF_DKIM_ALL here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petslane it was great that you pointed out your case, because it showcases my different use case! Luckily, both seem covered.

I think that we are both right: your HOSTNAME is part of virtualDomains because it is extracted from SMF_CONFIG. Normally people will have addresses @${HOSTNAME} in SMF_CONFIG.

I do not. My HOSTNAME is dedicated to the SMTP server and does not appear in SMF_CONFIG. My domains use HOSTNAME in their MX entries. HOSTNAME is not used for sending or forwarding.

In light of this, I believe that both solutions should stay. #83 is needed for those with a single domain for everything (which I believe will be most people). #89 is for more complex cases. Everything is handled in entrypoint.sh and, if I did not make any mistake, things should stay clean all in all. @huan, what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In light of this, I believe that both solutions should stay. #83 is needed for those with a single domain for everything (which I believe will be most people). #89 is for more complex cases.

I have only one domain, why shouldn't I use your way of setting DKIM keys? From the user's point of view, it's currently complicated as there are 2 ways to set up DKIM, what option should the user choose? Why not have only one way to set up DKIM for SMF, your way, and there is no difference in how many domains are used (I expect that your way should work also with only 1 domain).

Copy link
Contributor Author

@dgraziotin dgraziotin Feb 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be OK to have both solutions merged, but I am not sure I could deliver this in short. PR to the PR are welcome, I guess 😄

Edit: I might have some time, after all.

if [ ! -f /var/db/dkim/${virtualDomain}/default.private ]; then
mkdir -p /var/db/dkim/${virtualDomain}
echo "OpenDKIM: Keys for ${virtualDomain} not found, generating..."
opendkim-genkey -b 2048 -d ${virtualDomain} -D /var/db/dkim/${virtualDomain} -s default -v
fi
chmod 400 /var/db/dkim/${virtualDomain}/default.private
chown opendkim:opendkim /var/db/dkim/${virtualDomain}/default.private
echo "Inserting ${virtualDomain} data to /etc/opendkim/{KeyTable, SigningTable, TrustedHosts}"
echo "default._domainkey.${virtualDomain} ${virtualDomain}:default:/var/db/dkim/${virtualDomain}/default.private
" >> /etc/opendkim/KeyTable

echo "${virtualDomain} default._domainkey.${virtualDomain}" >> /etc/opendkim/SigningTable

echo "${virtualDomain}" >> /etc/opendkim/TrustedHosts

echo "OpenDKIM: Add TXT record to DNS for ${virtualDomain}:"
cat /var/db/dkim/${virtualDomain}/default.txt

done
echo "Moving from single DKIM key settings to multiple DKIM key settings."
sed -e '/KeyFile/ s/^#*/#/' -i /etc/opendkim/opendkim.conf
sed -e '/Selector/ s/^#*/#/' -i /etc/opendkim/opendkim.conf
sed -e '/Domain/ s/^#*/#/' -i /etc/opendkim/opendkim.conf
echo "KeyTable /etc/opendkim/KeyTable" >> /etc/opendkim/opendkim.conf
echo "SigningTable /etc/opendkim/SigningTable" >> /etc/opendkim/opendkim.conf
echo "ExternalIgnoreList /etc/opendkim/TrustedHosts" >> /etc/opendkim/opendkim.conf
echo "InternalHosts /etc/opendkim/TrustedHosts" >> /etc/opendkim/opendkim.conf
fi

}

#
Expand Down
8 changes: 7 additions & 1 deletion test/simple-mail-forwarder.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,13 @@
if [[ "$SKIP_TEST" == *"DKIM"* ]]; then
skip "This test will fail on docker build workflow"
fi
echo "Validating DKIM for $SMF_DOMAIN"
opendkim-testkey -d $SMF_DOMAIN -s default -vvv

if [ "$SMF_DKIM_ALL" != "" ]; then
cd /var/db/dkim/ && for domain in */ ; do
echo "Validating DKIM for ${domain::-1}"
opendkim-testkey -d ${domain::-1} -s default -vvv
done
fi
[ $? -eq 0 ]
}