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.

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

Add DKIM support #83

Merged
merged 3 commits into from Jan 26, 2021
Merged

Add DKIM support #83

merged 3 commits into from Jan 26, 2021

Conversation

petslane
Copy link
Contributor

@petslane petslane commented Oct 7, 2020

This change is based on @xriser fork - https://github.com/xriser/docker-simple-mail-forwarder

  • Added DKIM configuration based on @xriser work.
  • Updated docker entrypoint to generate DKIM keypair if it's missing and set OpenDKIM Domain config value to $SMF_DOMAIN
  • Added test to verify DNS TXT record match with DKIM keypair - but skip this test on docker build workflow as this test requires DNS TXT record
  • Updated readme

@xriser please review this change

refs #41 #19

@petslane petslane changed the title WIP: Add DKIM support Add DKIM support Oct 7, 2020
@lxsameer
Copy link

Is it possible to generate DKIM pair per smpt account ? I mean create a pair for each domain in SMF_CONFIG which has a smtp password.

@petslane
Copy link
Contributor Author

I am not expert of these email server technologies, but as i understand then you should just set default._domainkey TXT record for both domains in DNS. But I'm not sure of SMF supports receiving emails for multiple domains.

@lxsameer
Copy link

lxsameer commented Dec 5, 2020

@huan hey, what's the status of this

@huan
Copy link
Owner

huan commented Dec 5, 2020

@lxsameer I'd like to merge this new feature.

However, I'm not very familiar with the DKIM so I need someone to help me to confirm this PR is ok.

The code looks good to me, so I believe if there have one or two more approvement for this PR, I will have enough confidence to merge it.

@lxsameer
Copy link

lxsameer commented Dec 5, 2020

It seems fine to me as well. I can give it a shot and report back

@mcknight89
Copy link

Really need SRS working as gmail considers everything spam, is there a development container somewhere I can use until this is pulled in?

@lxsameer
Copy link

@huan I confirm that this PR is good to go. I'm using it in production for about 45 days now

@huan
Copy link
Owner

huan commented Jan 26, 2021

@lxsameer great to hear from you, your feedback is very valuable!

One more thing: can we confirm that this PR will not affect other users of SMF?

I mean we have to make sure that the new code will not block anyone if they are not using DKIM.

This PR will ready to be merged after we confirmed that.

Thank you very much for the efforts pushing this PR moving forward, appreciate it!

@lxsameer
Copy link

@huan It doesn't, It creates the key and everything and everything operate as usual. In order to activate it users need to take care of their DNS records

@huan huan merged commit 507186d into huan:master Jan 26, 2021
@huan
Copy link
Owner

huan commented Jan 26, 2021

Thank you very much for the confirmation @lxsameer .

Merged and should be published as v1.3.2 shortly.

@dgraziotin
Copy link
Contributor

I have missed this one when I commented previously. Do I understand it correctly that sent e-mails (when using SMF as SMTP server) still miss DKIM?

@dgraziotin
Copy link
Contributor

@huan found the issue. Right now DKIM is supported only for the domain in $SMF_DOMAIN. We are however allowed to send for several domains, and all others will miss DKIM signature. I see if I can do something for this.

@huan
Copy link
Owner

huan commented Feb 9, 2021

Glad to know that you found the issue!

Any improvement for our SMF will be welcome, thanks!

@petslane
Copy link
Contributor Author

petslane commented Feb 9, 2021

@dgraziotin Does setting the same default._domainkey TXT record on all the domains help? Probably not, as opendkim-genkey that generates these pub/priv keys have a domain as input, so probably every domain needs its own pub/priv keys.

Currently, SMF_DOMAIN env variable is used to generate these keys for that domain, but maybe these domains should be extracted from SMF_CONFIG and generate keys for every domain. But I don't know how to configure opendkim.conf for multi-domain.

@petslane
Copy link
Contributor Author

petslane commented Feb 9, 2021

https://serverfault.com/a/1004858

looks like listing all domains in Domain in opendkim.conf file should work, needs testing. So somehow using the same pub/priv keys for multiple domains is possible.

@dgraziotin
Copy link
Contributor

@petslane working on it :-) I'm extracting all ${virtualDomain} and generating one key each. Will come back here once I have something working.

@dgraziotin
Copy link
Contributor

Looking good already! Will now see how to test it properly and make a PR.

Screen Shot 2021-02-09 at 18 56 31

@dgraziotin
Copy link
Contributor

@petslane @huan feel free to review #89!

dgraziotin added a commit to dgraziotin/docker-simple-mail-forwarder that referenced this pull request Feb 10, 2021
huan added a commit that referenced this pull request Feb 13, 2021
* generating a DKIM key for all virtualDomains

* including HOSTNAME in folder of domains for DKIM

* KeyTable, SigningTable, TrustedHosts for HOSTNAME and all virtualDomain

* Generate new DKIM data only if keys do not exist yet

* disabled opendkim.conf settings for single domain, added KeyTable,SigningTable,ExternalIgnoreList,InternalHosts

* Correct permissions of DKIM files regardless of prior creation

* Added test for multiple domains and DKIM. Ready for #88

* Updated README on DKIM for multiple domains

* Fixed indentation on entrypoint

* Fixed wrong indentation (style)

* Cleaner handling of multiple DKIM keys. No settings required. Renders #83 redundant

* Making sure we never insert the same config twice #89

* Forgot one last mention of SMF_DKIM_ALL

* Better tld naming for DKIM in README

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* DKIM test no longer changes working directory

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* More elegant generation of DKIM entries for HOSTNAME and virtual domains

* Correct switch to suppress grep complains when files miss

Co-authored-by: Peeter N <petslane@users.noreply.github.com>
haratosan pushed a commit to haratosan/docker-simple-mail-forwarder that referenced this pull request Apr 5, 2021
* Add DKIM support

* 1.2.11

Co-authored-by: Huan (李卓桓) <zixia@zixia.net>
haratosan pushed a commit to haratosan/docker-simple-mail-forwarder that referenced this pull request Apr 5, 2021
haratosan pushed a commit to haratosan/docker-simple-mail-forwarder that referenced this pull request Apr 5, 2021
* generating a DKIM key for all virtualDomains

* including HOSTNAME in folder of domains for DKIM

* KeyTable, SigningTable, TrustedHosts for HOSTNAME and all virtualDomain

* Generate new DKIM data only if keys do not exist yet

* disabled opendkim.conf settings for single domain, added KeyTable,SigningTable,ExternalIgnoreList,InternalHosts

* Correct permissions of DKIM files regardless of prior creation

* Added test for multiple domains and DKIM. Ready for huan#88

* Updated README on DKIM for multiple domains

* Fixed indentation on entrypoint

* Fixed wrong indentation (style)

* Cleaner handling of multiple DKIM keys. No settings required. Renders huan#83 redundant

* Making sure we never insert the same config twice huan#89

* Forgot one last mention of SMF_DKIM_ALL

* Better tld naming for DKIM in README

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* DKIM test no longer changes working directory

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* More elegant generation of DKIM entries for HOSTNAME and virtual domains

* Correct switch to suppress grep complains when files miss

Co-authored-by: Peeter N <petslane@users.noreply.github.com>
huan added a commit that referenced this pull request Jul 6, 2021
…103)

* Use alpine:latest as base image

* Roll back to sillelien/base-alpine:0.10 (#23)

* fix doc

* Update README.md

* Update README.md

* Update Base to Alpine 3.8

* Update base image to Alpine 3.8
* Install s6 process manager directly
* Upgraded BATS to 1.1.0
* Install syslog-ng for postfix logging to stdout

* syslog-ng: Disable statistic messages

These spam the console too much, so disable them.

* Fix typo in README

couse => course

* Upgrade circleci from v1 to v2

* add ide config

* fix circleci config

* fix circleci config

* fix circleci config

* fix circleci config

* fix yml

* fix yml add docker run type

* fix yml add machine run type

* fix yml

* year 2019

* Add voice from Paweł Czochański

* EC key support (#51)

* Fix nickname typo

* Add support for EC keys

* Update README.md

* Update README.md

* add ec key support

* Fix layout

* Timezone tzdata packagge (#57)

Add custom timezone support

* make circler yaml linter happy

* Fix leak of EC Cert/Key problem (#58)

* code clean

* only generate not existing files (#51 #58)

* one line -> one-line

* Timezone supported

* Update author & copyright

* Update master changelog

* fix chinese charactor bug

* v1.1

* Update README.md timezone (#59)

Update README.md for Timezone support

* clean doc

* Fix H1 title

* Update README.md

* Update README.md

* Fix typo (#66)

* add auth for relayhost (#68)

* add auth for relayhost

* indent fix

* example for AUTH

* remove excess line

* environment var typo fix

* fix for mail log not displaying

* Enable GitHub Actions

* Add Actions Badge

* basic "proofreading" (#69)

* basic "proofreading"

fixed some grammatical and spelling errors, made the descriptions flow a little better

* PR revisions

* Update README.md

* add hall of flame

* Upgrade BATS & S6, with multiple-platform docker image published with version 1.2 (#76)

* v1.2

* Upgrade Alpine to 3.8 (#77)

* upgrade base image to alpine 3.8

* v1.3

* Deploy docker image arm platform from github action

* test

* test

* checkout before deploy

* clean

* use buildx as default bugild

* republish v1.2 for amd64 with s6 fix (#79)

* republish v1.3 for amd64 with s6 fix

* fix comment

* v1.2

* Add test for deleting test user (#82)

* Add makefile

* makefile

* makefile

* makefile

* 1.2.14

* add make version

* 1.2.15

* v1.3.0 for multi platforms image

* Use script to install s6 with right platforms (arm/x86) (#76)

* use aarch64 for s6 release (#76)

* v1.3.1

* Add DKIM support (#83)

* Add DKIM support

* 1.2.11

Co-authored-by: Huan (李卓桓) <zixia@zixia.net>

* generating a DKIM key for all virtualDomains

* including HOSTNAME in folder of domains for DKIM

* KeyTable, SigningTable, TrustedHosts for HOSTNAME and all virtualDomain

* Generate new DKIM data only if keys do not exist yet

* disabled opendkim.conf settings for single domain, added KeyTable,SigningTable,ExternalIgnoreList,InternalHosts

* Correct permissions of DKIM files regardless of prior creation

* Added test for multiple domains and DKIM. Ready for #88

* Updated README on DKIM for multiple domains

* Fixed indentation on entrypoint

* Fixed wrong indentation (style)

* Cleaner handling of multiple DKIM keys. No settings required. Renders #83 redundant

* Making sure we never insert the same config twice #89

* Forgot one last mention of SMF_DKIM_ALL

* Better tld naming for DKIM in README

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* DKIM test no longer changes working directory

Co-authored-by: Peeter N <petslane@users.noreply.github.com>

* More elegant generation of DKIM entries for HOSTNAME and virtual domains

* Correct switch to suppress grep complains when files miss

* Update VERSION

* Strips sender details (IP, client, user agent) when sending (#91)

* Strips sender's IP, client, and user agent headers

* Bumping patch verison number

* Allow for setting any Postfix variables in the config file (both main.cf and master.cf) (#93)

* Strips sender's IP, client, and user agent headers

* Bumping patch verison number

* SMF_POSTFIXMAIN_* to set custom postfix main.cf entries

* SMF_POSTFIXMASTER_* to set custom postfix master.cf entries

* Using sed to handle master.cf custom variables

* README.md explains env variables for custom main.cf and master.cf

* Tests for custom main.cf and master.cf

* Fixes #92

* Simplify docker run command with SMF_CONFIG

* v1.4.3 (#94)

* Add VERSION & Update README.md (#94)

* fix ignore

* 1.4.4

* add v1.4 changelog

* clean

* show version

* 1.4.5

* layout

* Update configuration after variables has been injected to the main configuration (#98)

* Add an option to override postfix's default logging configuration

* Add tests

* Update README.md

* push to build

* fix overwriting variable

* update with postfix-configuration

* delete drone for pull request

Co-authored-by: Tamaro Skaljic <49238587+tamaro-skaljic@users.noreply.github.com>

* Add an option to override postfix's default logging configuration (#97)

* Add an option to override postfix's default logging configuration

* Add tests

* Update README.md

* v1.4.6

* Change Postfix logging configuration tests behaviour (#99)

* fix default postfix logging configuration test

* Change logfile path in custom postfix logging configuration test

* Make postfix logging configuration tests restore the preconditions

* start PostSRSd and generate Secret

* start PostSRSd and generate Secret

* start PostSRSd if  is set

* start PostSRSd if  is set

* only start PostSRSd if  is set

* only start PostSRSd if  is set

* Updated README.md

* Updated README.md

* spelling...

* spelling...

* Updated the if-statement for PostSRSd

* Updated the if-statement for PostSRSd

Co-authored-by: Martijn Rondeel <martijn@rondeel.email>
Co-authored-by: Huan LI <zixia@zixia.net>
Co-authored-by: Chris Blake <chrisrblake93@gmail.com>
Co-authored-by: universeroc <universeroc@gmail.com>
Co-authored-by: Paweł Czochański <czochanski@gmail.com>
Co-authored-by: me1299 <50422731+me1299@users.noreply.github.com>
Co-authored-by: David Gonzalez <davidgg666@gmail.com>
Co-authored-by: Choon-Siang Lai <mycyberpet@yahoo.com>
Co-authored-by: Bailey <bailey.riezebos@gmail.com>
Co-authored-by: Peeter N <petslane@users.noreply.github.com>
Co-authored-by: Daniel Graziotin <daniel.graziotin@iste.uni-stuttgart.de>
Co-authored-by: Daniel Graziotin <daniel@ineed.coffee>
Co-authored-by: Cenk Kılıç <cenk1cenk2cenk3@gmail.com>
Co-authored-by: Tamaro Skaljic <49238587+tamaro-skaljic@users.noreply.github.com>
Co-authored-by: Linux User <harato@alpine.members.linode.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants