* fix: `supervisor-app.conf` - Correct `postgrey` log location
Looks like this should have been like every other service and reference a log file(s) based on program name in the supervisor log directory.
* tests: Adjust log location for `postgrey_enabled.bats`
* chore: Extract out Dovecot Quota test cases into new test file
Test cases are just cut + paste, no logic changed there yet.
* chore: Rename test case descriptions
* chore: Use `setup ...` methods instead of direct calls
* chore: Adjust `_run_in_container_bash` to `_run_in_container`
Plus some additional bug fixes in the disabled test case
* tests(refactor): Revise ENV test cases for max mailbox and message sizes
* tests(refactor): Revise ENV test cases for mailbox and message limits v2
Removes the extra variables and filtering in favour of explicit values instead of matching for comparison.
- Easier at a glance to know what is actually expected.
- Additionally reworks the quota limit checks in other test cases. Using a different formatter for `doveadm` is easier to match the desired value (`Limit`).
* chore: Sync improvement from `tests.bats` master
---
NOTE: This PR has been merged to avoid additional maintenance burden without losing the improvements. It was not considered complete, but remaining tasks were not documented in the PR.
* outsource Rspamd ENVs into explicit helper
This will allow us to uniformly source the helper and get the values
from everywhere consistently. This is more than desirable since we will
be using these values not only for the Rspamd setup, but also for DKIM
management and during change-detection.
* integrate Rspamd into changedetection
We outsource one more function to reside in the helper script for Rspamd
so that we can call this function from the Rspamd setup and from the
changedetection functionality too.
* realize deprecation of old commands file for Rspamd
THIS IS A BREAKING CHANGE!
This change realizes the log message: "Using old file location now
(deprecated) - this will prevent startup in v13.0.0" Startup will now
fail.
* added '--force' option to Rspamd DKIM script
* use new helper to get ENVs for Rspamd in DKIM script
* remove the need for linking directories
This was unnecessary, as explained in
https://github.com/docker-mailserver/docker-mailserver/pull/3597#discussion_r1369413599
* Apply suggestions from code review
review by @polarathene
* apply more review feedback from @polarathene
- <https://github.com/docker-mailserver/docker-mailserver/pull/3599#discussion_r1370885519>
- <https://github.com/docker-mailserver/docker-mailserver/pull/3599#discussion_r1370904201>
* update documentation
---------
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Instead of using `etc/rspamd/override.d/dkim_signing.conf`, we will now
be using `/tmp/docker-mailserver/rspamd/override.d/dkim_signing.conf`.
The new location is persisted (and linked again during startup) and
hence better suited.
Some deployment scenarios are not compatible with `5000:5000` static vmail user with `/var/mail`. This feature allows adjusting the defaults to a UID / GID that is compatible.
Signed-off-by: vincent <vincent@ducamps.win>
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
Instead of exit status of `124` (_signifies timeout_), it should fail with `1` (failure) like the others. Handled via using `_run_in_container_bash()` (_`timeout` failure `124` does not propagate and is treated as `1` instead_).
In this case we are waiting on the status of the mail being sent, the pattern provided to `grep` is too specific and results in a timeout. Instead since we only expect the one log entry, match any status and assert the expected pattern afterwards.
This provides a more helpful failure output that informs us that mail was at least processed by Postfix, but the sent status is not what we expected.
### Before
```
✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [60327]
(from function `assert_success' in file test/test_helper/bats-assert/src/assert_success.bash, line 42,
in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 47)
`assert_success' failed
-- command failed --
status : 124
output :
--
```
### After
```
✗ [ENV] (POSTFIX_DAGENT) delivers mail to existing account [1425]
(from function `assert_output' in file test/test_helper/bats-assert/src/assert_output.bash, line 178,
in test file test/tests/parallel/set3/mta/lmtp_ip.bats, line 48)
`assert_output --regexp "${MATCH_LOG_LINE}=sent .* Saved)"' failed
-- regular expression does not match output --
regexp : postfix/lmtp.* status=sent .* Saved)
output : Sep 28 04:12:52 mail postfix/lmtp[721]: 23701B575: to=<user1@localhost.localdomain>, relay=127.0.0.1[127.0.0.1]:24, delay=0.08, delays=0.07/0/0.01/0, dsn=4.2.0, status=deferred (host 127.0.0.1[127.0.0.1] said: 451 4.2.0 <user1@localhost.localdomain> Internal error occurred. Refer to server log for more information. [2023-09-28 04:12:52] (in reply to end of DATA command))
--
```
The expected pattern is logged as `assert_success` confirms a valid match for the log line of interest was found, and we have the mismatched value to debug the failure against.
This was missed during original review.
On a linux host, processes running within a container have been visible via commands like `pgrep`. This is does not appear to be the case with WSL2 + Docker Desktop (Windows), resulting in test failure.
The command should have been run from within the container regardless.
* chore: Drop management of `SASLAUTHD_*` ENV
- `variables-stack.sh` does not need to manage all these extra ENV or store them. They're not used anywhere else.
- `saslauthd.sh` is the only consumer of these ENV which are effectively direct key/value mappings, with some defaults provided / inherited.
Instead of trying to conditionally support key/value pairs when ENV is set, we could instead use `sed` to delete lines with empty values.
* chore: Drop fallbacks + update configs to match docs
- Drop deprecated support:
- `DOVECOT_HOSTS` is an ENV deprecated since v10.
- Fallback for missing URI scheme introduced for Dovecot and SASLAuthd in v10.
- Adding error log message when no LDAP URI scheme is detected for the supported ENV (when set).
- Docs updated for ENV to reflect the mandatory requirement. `mailserver.env` partially synced equivalent sections.
- Provided base LDAP configs (for overriding) likewise updated from `domain.com` to `example.com`.
- LDAP test updated for required `ldap://` URI scheme. Common ENV shared across LDAP configs hoisted out of the Postfix group.
* chore: Remove unset lines in generated `saslauthd.conf`
* chore: Use white-space in query filters to improve readability
* tests: LDAP ENV query filters documented
- These filters remain roughly the same as they were before. The conditions are the same, but restructured to make the complimentary constraints more separated from the actual target attribtues.
- The DOMAIN query additionally includes the `mailAlias` from `PostfixBookMailAccount` class as well.
- Heavy inline documentation breaking down how the filters work for any maintainer to reference. This will likely be migrated after revision into our user docs for LDAP. Some quirks have also been noted with advice for future changes.
* tests: LDAP - Support test-case specific containers
A bit more complicated than other test files due to the larger ENV config array that most containers may need to share.
Example introduced with the test-case checking custom config file support.
* tests: Adjust LDAP test configs
- Paths for `.ldif` files used with volumes shortened
- Postfix LDAP `.cf` files adjusted to conventions used in LDAP tests.
- The `uniqueIdentifier` attribute is not appropriate and was relying on `objectClass: extensibleObject` as a workaround to allow it. A more appropriate attribute to use instead is `userID` (_short name: `uid`_).
- Removing `extensibleObject` now requires switching the user accounts to use `inetOrgPerson` class (_which inherits from `organizationalPerson`_). which allows the attributes `givenName`, `userID` and `mail` (_also provided via the `PostfixBookMailAccount` class_).
- The LDAP root object now uses `dc` attributes for `example.test` instead of `localhost.localdomain`. This has nothing to do with DMS or LDAP containers networking config, nor the users mail addresses.
- Users are now grouped under the organizational unit of `users` instead of `people`. Purely a naming change out of preference, no functional difference.
The LDAP test ENV has been updated to accommodate the above changes. An additional ENV override was required for SASLAuthd to switch an attribute set for `ldap_filter` in `/etc/saslauthd.conf` from the implicit default of `uniqueIdentifier` (_that we set during startup as an ENV default for fallback_) to the `userID` attribute.
* chore: Adjust default DKIM size (`open-dkim`) from 4096-bit to 2048-bit
4096-bit is excessive in size for DKIM key. 2048-bit is plenty.
* chore: Additional revisions to `open-dkim` command help output
- The examples use `keysize 2048`, but as that's the new default it makes sense to change that.
- Other help text was also revised.
- Last example for domains did not need to demonstrate the other options. Changed example domains to more appropriate values.
* docs: Revise DKIM docs
Primarily for the change in default key size, but does revise some text to better communicate to the user.
- While the referenced RFC advises 512-bit to 2048-bit key size, we now explicitly discourage `512-bit` as it's not secure. `1024-bit` is still likely safe for most, but `2048-bit` is a good default for those not rotating their keys.
- Adjusted the domains example to match the new `setup config dkim domain` domains example.
- Tip for changing default key size changed to "info" with added clarity of lowering security or increasing it (excessively).
- Rspamd section is minor formatting changes, with the exception of clarifying the "main domain" for the mail accounts is assumed as the DMS FQDN with any subdomain (like `mail.`) stripped away. This is not great, but a legacy issue that needs to be addressed in future.
- `docs-rspamd-override-d` ref removed, and usage replaced with equivalent ref `docs-rspamd-config-dropin`, while `docs-rspamd-config-declarative` ref was not in use and also removed.
- Revised the `<selector>.txt` DNS formatting info section to better communicate with the reader. Additionally it had mixed usage of default `mail` and custom `dkim-rsa` selectors (_file content and output_).
* docs: Sync DKIM commands help messages and update DKIM docs for LDAP
- Adopt the help options format style from the `rspamd-dkim` into `open-dkim` command. And convert `./setup.sh` to `setup`. `selector` option has been implemented. for a while now.
- Update `rspamd-dkim` examples help output to align with `open-dkim` command examples.
- Give both DKIM command tools a consistent description. The two tools differ in support for the `domain` option (_implicit domain sourcing for default account provisioner, and support for multiple domains as input_).
- DKIM docs for LDAP domain support revised to better communicate when explicit domain config is necessary.
* tests: Adjust test-cases for `setup config dkim` change
`rspamd_dkim.bats`:
- Update assert for command help output.
- Don't bother creating a DKIM key at 512-bit size.
`setup_cli.bats`:
- Update assert for command help output of the `setup config dkim` (OpenDKIM) command.
* docs: Update DKIM section for large keys to newer RFC
The linked discussion from 2021 does mention this updated RFC over the original. That removes outdated advice about `512-bit` key length support.
The discussion link is still kept to reference a comment for the reader to better understand the security strength of 2048-bit RSA keys and why larger keys are not worthwhile, especially for DKIM.
* docs: Extract out common DKIM generation command from content tabs
Should be fine to be DRY here, not specific to `open-dkim` or `rspamd` generation/support. Previously rspamd lacked support of an equivalent command in DMS.
* docs: DKIM refactoring
- Shifted out the info admonition on key size advice out of the content tabs as it's now generic information.
- Indented the 4096-bit warning into this, which is less of a concern as the default for our DKIM generation tools is consistently 2048-bit now.
- Reworked the LDAP and Rspamd multi-domain advice. To avoid causing a bad diff, these sections haven't been moved/merged yet.
* docs: Revise DKIM docs
Advice for managing domains individually with LDAP and Rspamd extracted out of the content tabs. Default domain behaviour explained with extra info about OpenDKIM + FILE provisioner sourcing extra domains implicitly.
**TL;DR:**
- New image is actively maintained vs existing one that is over 5 years old.
- Slight improvement to LDAP tree config via `.ldif` files.
- No more `Dockerfile` required to build, we can just rely on `docker run`.
`osixia/openldap` has not seen any activity since Feb 2021, while our `Dockerfile` was fixed to v1.1.6` (Feb 2018).
Startup time for this new image is around 5 seconds? (_The LDAP test uses a standard 20 second timeout check to wait until the server is ready before continuing with starting the DMS image_).
This commit migrates to `bitnami/openldap` which required modifying the `01_mail-tree.ldif` to also include adding the root object to start successfully. This image is actively maintained and one of the most popular OpenLDAP images on DockerHub.
The user account `.ldif` files have minimal changes:
- Lines moved around for better organization
- Additional comments for context
- Removal of inherited `objectClass` attributes (`person`, `top`) from the `orgnizationalPerson` class. Attribute `sn` changed to long form `surname` and values corrected with `givenName`. `changetype: add` was also not necessary.
Additionally the image does not support the `.schema` format, they must be converted to `.ldif` which has been done for `postfix-book.schema`.
See PR for more details.
* tests: Switch to setup helper and conventions
* tests: Adapt run command to new conventions
- We have two helper methods with implicit `CONTAINER_NAME` reference, which is a bit more DRY and improves readability.
- `wc -l` + `assert_output 1` converted to use helper `_should_output_number_of_lines 1`
- `DOMAIN` var changed from `my-domain.com` to local testing domain `example.test`.
* tests: Refactor `setup_file()`
- Test wide ENV defined at the top
- OpenLDAP build and run logic grouped together. Added notes, network alias and tty not required.
- Extracted out special LDAP Postfix/Dovecot ENV into separate array. LDAP specific provisioning / auth ENV also included, with comments + linebreak to better group related ENV.
- Likewise additional ENV to support test cases has been extracted to a separate array with additional comments for context.
- Those two arrays are expanded back into the main `CUSTOM_SETUP_ARGUMENTS` that configure hostname and network for the DMS container.
* tests: Refactor the LDAP account table query testcase
- Covers 3 accounts to test from LDAP.
- 2 are the same query against users/aliases/groups tables in Postfix, only differing by account queried (and expected as returned result).
- 1 separate test to ensure a difference in config is supported correctly.
- Extracted repeated test logic into a helper method.
- Added additional context in comments about the creation source of these LDAP accounts and their related Postfix config / interaction. Direct reference to special case PR (since `git blame` will be less useful).
* tests: Use iteration for `grep` setting checks
More DRY approach. With a bit more helpful failure context via `assert_output` (_and only grepping the key_). Simpler to grok what's being covered.
* tests: DRY test email delivery
A bit more verbose with the new helper method. `test-email.txt` template is only used by the LDAP test, as is the `sendmail` command.
Helper will take two args to support the testcases, but at a later date should be refactored to be consistent with the `_send_email()` helper (_which presently uses `nc` that is required for plain-text mail/auth, otherwise we'd have used `openssl`, bigger refactor required_).
* tests: Slight revisions and relocating testcases
- Dovecot quota plugin testcase revised to check files exist instead of rely on `ls` failure.
- Moved Postfix quota plugin testcase into prior dovecot testcase for quota plugin check. Better error output by only querying the `smtpd_recipient_restrictions` setting (_which should be the only one configured for the service_).
- Moved the saslauthd and pflogsumm testcases (_no changes beyond revised comments_) above the `ATTENTION` comment, and one testcase below the comment that belonged to that group.
* tests: Simplify openldap `docker build` command
- `--no-cache` was creating a new image on the Docker host each time the test is run. Shouldn't be any need to build without cache.
- No need to use `pushd` + `popd`, can just provide the path context directly, and the `./Dockerfile` is an implicit default thus `-f` not required either.
Additionally removed the old `checking` prefix from testcase names.
* tests: Move LDAP specific config into `test/config/ldap/`
- No changes to any of these config files, just better isolation as not relevant to any other tests.
- Section heading in `setup_file()` added to distinguish the remainder of the function is dedicated to the DMS container setup.
- Comment providing some context about the `mv` to maintainers, this should be done after defaults are initialized but before starting up the container.
* chore: Appease the lint gods
* Apply suggestions from code review
* add new home dir for Dovecot
I tried changing the mail dir, but this is a _very_ disruptive change,
so I took approach 3 on
<https://doc.dovecot.org/configuration_manual/home_directories_for_virtual_users/>,
whereby the home directory is now inside the mail directory.
The MDBOX/SDBOX formats are not touched by this change. The change
itself could be considered breaking though.
* adjust Sieve tests accordingly
* Update target/dovecot/10-mail.conf
* Update target/dovecot/auth-passwdfile.inc
---------
Co-authored-by: Casper <casperklein@users.noreply.github.com>
* adjust learning of ham
See #3333
When moving a mail from the Junk folder to the Trash folder, the mail
previously classified as ham due to the wildcard match of `*`. Because
the syntax does not allow for negation, we can only change the behavior
in a way that mails are learned as ham when they are moved into `INBOX`
from `Junk`. This is reasonable though.
* adjust tests accordingly
* adjust docs accordingly
* add sanity check for Clam size & adjusted MaxScanSize
The second part is of special importance! See
<https://askubuntu.com/a/1448525>, which explains that the maximum scan
size is important as well. We previously just set the maximum file size,
which actually is pretty insecure as we silently not scan mile bigger
than `MaxScanSize`. This is corrected now.
* add SlamAV size configuration to Rspamd
* move modules adjustment file to new location
Because we link `/tmp/docker-mailserver/rspamd/override.d` to
`/etc/rspamd/override.d`, I think it makes sense to move the modules
adjustment file into `/tmp/docker-mailserver/rspamd/` as well.
I write the code in a way that it is backwards compatible for now, so
this is NOT a breaking change.
* minor improvement to `__rspamd__handle_user_modules_adjustments`
The expansion of `ARGUMENT3` is now done in a way that only adds the
whitespace in case the variable is set and not null.
* move test file structure to respect latest changes
Because we're now linking `rspamd/override.d/`, we can simplify the
setup a bit. But this requires a change in directory structure.
The current Rspamd test will be renamed to `rspamd_full.bats`, because I
plan on adding more tests in different files for different feature sets.
This is done to make this feature well-tested!
* improved and added tests to Rspamd-full
FYI: The line
```bats
_run_in_container grep 'sieve_global_extensions.*\+vnd\.dovecot\.pipe'
"${SIEVE_CONFIG_FILE}"
```
was testing a condition that should actually not be met, but when I
started working on this feature, I thought this was the correct
configuration. Adding the `assert_success` statements revealed this
wrong line.
I also added tests to check whether `override.d` is linked correctly.
* renamed: `rspamd.bats` => `rspamd_full.bats`
* added new tests for incomplete Rspamd feature set
We now test that warnings are emitted & features are disabled correctly.
* update documentation
* added checks whether OpenDKIM/OpenDMARC/policyd-spf are enabled
* added functions to check if VAR is 0/0 or an int
and also added tests.
I also adjusted the test file to not run in a container, because there
is no need. This also decreases test time, which, in turn, increases
maintainers' happiness.
* added more checks to Rspamd setup
I added the helpers from the previous commit to the Rspamd setup to make
the whole setup more robust, and indicate to the user that an ENV
variable's value is incorrect.
While we did not issues for this in the past, I believe it to be
worthwhile for the future.
* added canonical directory for users to place files in
This dir is canonical with DMS's optional configuration dirs, as it
lives in well-known volume mounts. Hence, users will not need to adjust
`/etc/rspamd/override.d` manually anymore, or mount a volume to this
place.
The docs explain this now, but the DKIM page needs a slight update on
this too I guess. I will follow-up here.
* misc minor improvements
* use variables for common directories
When a new version of docker-mailserver is available the account used in this
tests also gets the postmaster notification for the new version. The mailbox
then may contain 2 mails but only one with 'This is a test mail.'.
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* postfix: remove smtpd_sasl_auth_enable global setting
* tests: disable auth on 25 port
* tests: revert ldap-smtp-auth-spoofed-sender-with-filter-exception.txt
* Skip failing test
The test seems to have been broken from the beginning.
Sadly, no LDAP maintainers can verify. Added a TODO item if ever a LDAP maintainer comes around.
* Apply PR feedback
---------
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* chore: Only replace `CHKSUM_FILE` when a change has been processed
* chore: Change Detection service should be the last daemon started
* chore: Remove 10 second startup delay for change detector
There should be no concern with conflicts as any writes should have already been done by the time this daemon service is started.
* tests(fix): `smtp_delivery.bats` must wait for Amavis
The change event for adding a user can be processed much sooner now, which means Amavis may not yet be ready.
Added extra condition to wait on at least the Amavis port being reachable, and some failure asserts with the mail queue to better catch / debug when this problem occurs.
* chore: Add some minor delay to avoid Amavis failing to connect
* tests(refactor): Make test cases for opendkim keysizes DRY
- These all do roughly the same logic that can be split into two separate methods.
- `_should_generate_dkim_key()` covers a bit more logic as it can be leveraged to handle other test cases that also perform the same logic.
- The `config/opendkim/` doesn't seem necessary for tests. Only the first few test cases here are testing against it, so we can conditionally make that available. `process_check_restart.bats` also depended on it to run OpenDKIM successfully, but this was due to the `setup-stack.sh` config defaults failing to find an "empty" file forcing `supervisord` to constantly restart the process..
- With this, there we inverse the default opendkim config, so we don't have to mount unique / empty subfolders for each test case, followed by copying over the two extra configs.
* tests(refactor): DRY up more test cases
All the remaining test cases but the last one were refactored here for a clean commit diff. The last test case will be refactored in the following commit.
Plenty of repeated logic spread across these test cases, now condensed into shared methods.
* tests(refactor): Make final test case DRY
* chore: Migrate to new testing helpers
* chore: Revise test case descriptions
* tests(refactor): Improve and simplify assertions
* tests(refactor): Use common container setup instead of `docker run`
- As the majority of test cases are only running `open-dkim` helper, we don't actually have to wait for a full container setup. So an alternative container start is called.
- Also improves assertions a bit more instead of just counting lines.
- Some test cases don't bind mount all of `/tmp/docker-mailserver` contents, thus don't raise permission errors on subsequent test runs.
- Instead of `rm -f` on some config files, have opted to mount them read-only instead, or alternatively mount an anonymous empty volume instead.
- Collapsed the first three test cases into one, thus no `setup_file()` necessary.
- Shift the `_wait_for_finished_setup_in_container()` method into `_common_container_setup()` instead since nothing else is using `_common_container_start()` yet, this allows for avoiding the wait.
* tests(refactor): Collapse dkim key size test cases into single test case
This makes these tests a bit more DRY, and enhances the raised quality issue with these tests. Now not only is the domain checked in the generated DNS dkim record, but we also verify the key size is corrected in the public and private keys via openssl.
* chore: Revise container names
* chore: Swap order of test case 1 and 2
* tests(refactor): Assert generated log output
- `__should_have_tables_trustedhosts_for_domain` shifted in each test case to just after generating the domains keys.
- Asserts `open-dkim` logs instead of just counting them.
- Added checks for domains that should not be present in a test case.
- Additional coverage and notes about the alias from vhost `@localdomain.com`
- Single assert statement with switch statement as all are using common args.
* chore: Minor changes
* tests(refactor): Share `find` logic in helpers and tests
* tests(fix): Listing file content does not need to match line order
The order printed from local system vs CI differed causing the CI to fail. The order of lines is irrelevant so `--index` is not required.
Additionally correct the prefix of the called method to be only one `_` now that it's a `common.bash` helper method.
* chore: Collapse custom DKIM selector test into custom DKIM domain test
These cover the same test logic for the most part, the first domain could also be testing the custom selector.
`special_use_folders.bats` + `mailbox_format_dbox` can assert lines instead, removing the need for `--partial`.
* Apply suggestions from code review
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* chore: Split switch statement method into wrapper methods
---------
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
