huan / docker-simple-mail-forwarder

Simplest and Smallest Email Forward Service based on Docker.
https://hub.docker.com/r/zixia/simple-mail-forwarder/
Apache License 2.0
544 stars 86 forks source link

Adds DKIM support for multiple domains #89

Closed dgraziotin closed 3 years ago

dgraziotin commented 3 years ago

Fixes #88 and merges #83.

Creates DKIM public/private keys and puts them into use.

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

dgraziotin commented 3 years ago

FYI, I am using this PR in production (😨) and testing it with dgraziotin/simple-mail-forwarder-dkim. I have four domains plus HOSTNAME and all score 10/10 on http://mail-tester.com and https://dkimvalidator.com is happy with DKIM for all of them.

dgraziotin commented 3 years ago

@petslane this last push merges #83 and #89 following my approach. You are right, it looks cleaner now. No config variable needed. Please review, though, and test. I hope I am covering all edge cases.

Edit: I am now making sure that config lines are never entered twice accidentally. I once found duplicated entries in a config file. This never repeated, but you made me suspicious it might happen sometimes. Now it won't happen.

petslane commented 3 years ago

I like this solution a lot better!

dgraziotin commented 3 years ago

@petslane I am very satisfied with this PR now. Many thanks!

dgraziotin commented 3 years ago

@huan I think we are done here. I edited the PR description to reflect what's in the code right now. In the end it got way simpler than originally submitted. Code is complete, tests are complete. Feel free to merge or ask anything.

huan commented 3 years ago

@dgraziotin It's great to see this new feature has been implemented, and thank your effort for reviewing this PR to make it better @petslane

Please help this PR to get at least one approvement before we have enough confidence to merge it.

petslane commented 3 years ago

Tried to test, but I get errors on startup:

forwarder_1  | >> ARGV arguments found. value:[start]
forwarder_1  | >> SMF_CONFIG found in ENV. use this settings for forward maps.
forwarder_1  | >> Setting password[xxx] for user @exaple.com ...
forwarder_1  | >> Setting password[xxx] for user yyy@yyy.yy ...
forwarder_1  | >> Setting password[xxx] for user yyy@yyy.yy ...
forwarder_1  | postmap: warning: /etc/postfix/virtual.db: duplicate entry: "@exaple.com
forwarder_1  | >> Set hostname to exaple.com
forwarder_1  | Migrating exaple.com keys to /var/db/dkim/exaple.com/
forwarder_1  | Inserting exaple.com data to /etc/opendkim/{KeyTable, SigningTable, TrustedHosts}
forwarder_1  | 'No such file or directory' messages might be displayed here. This is normal.
forwarder_1  | grep: /etc/opendkim/KeyTable: No such file or directory
forwarder_1  | grep: /etc/opendkim/SigningTable: No such file or directory
forwarder_1  | grep: /etc/opendkim/TrustedHosts: No such file or directory
forwarder_1  | OpenDKIM: this TXT record for petslane.eu should be present:

Looks like grep fails if these files are missing.

petslane commented 3 years ago

But then again, I can see that default keys were moved to domain-specific folders and https://www.mail-tester.com/ test gets 10/10.

Maybe grep should use -s, --no-messages argument? I think this should suppress "No such file or directory" errors.

dgraziotin commented 3 years ago

Tried to test, but I get errors on startup:

forwarder_1  | >> ARGV arguments found. value:[start]
forwarder_1  | >> SMF_CONFIG found in ENV. use this settings for forward maps.
forwarder_1  | >> Setting password[xxx] for user @exaple.com ...
forwarder_1  | >> Setting password[xxx] for user yyy@yyy.yy ...
forwarder_1  | >> Setting password[xxx] for user yyy@yyy.yy ...
forwarder_1  | postmap: warning: /etc/postfix/virtual.db: duplicate entry: "@exaple.com
forwarder_1  | >> Set hostname to exaple.com
forwarder_1  | Migrating exaple.com keys to /var/db/dkim/exaple.com/
forwarder_1  | Inserting exaple.com data to /etc/opendkim/{KeyTable, SigningTable, TrustedHosts}
forwarder_1  | 'No such file or directory' messages might be displayed here. This is normal.
forwarder_1  | grep: /etc/opendkim/KeyTable: No such file or directory
forwarder_1  | grep: /etc/opendkim/SigningTable: No such file or directory
forwarder_1  | grep: /etc/opendkim/TrustedHosts: No such file or directory
forwarder_1  | OpenDKIM: this TXT record for petslane.eu should be present:

Looks like grep fails if these files are missing.

Yeah, these aren't exactly errors. Mostly unwanted output. That's why I put 'No such file or directory' messages might be displayed here. This is normal.. It's working, just ugly output. For some reason, the -q options of grep is not working.

But then again, I can see that default keys were moved to domain-specific folders and https://www.mail-tester.com/ test gets 10/10.

Maybe grep should use -s, --no-messages argument? I think this should suppress "No such file or directory" errors.

That was it, thanks. Was not aware of -s. Pushed.

petslane commented 3 years ago

No warnings anymore. LGTM.

huan commented 3 years ago

@petslane thank your very much for the testing, reviewing, and approvement!

Appreciate for all efforts from @dgraziotin and @petslane to make the SMF better, thank you very much!

P.S. I have bumped the version to 1.4 , the docker image should be published shortly.