- 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>
* refactor `mail_pop3.bats`
* refactor `mail_with_imap.bats`
* refactor `mail_with_relays.bats`
* moved test that that did not belong into POP3 test
* slightly clean up `no_container.bats`
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
* chore: Extract out accounts test cases from `tests.bats`
Standard test file format, the test cases have been copied over unmodified.
* chore: Revise test case descriptions
* tests(refactor): `accounts.bats`
Revised test cases:
- Some common test case logic extracted to test methods.
- Update direct user management commands to use the `setup email ...` variants.
- Improved assertions.
- Removed `sleep 2` lines as the need for that is ambiguous (may no longer be relevant?)
- Additional commentary for maintaining
- Two test cases for missing `postfix-accounts.cf` opted to just run the image without any volumes instead, as the `without-accounts/` folder was empty anyway. 2nd test case can instead use a single `docker run` to check the newly created`postfix-accounts.cf` content.
- `test/config/without-accounts/` remains as `open_dkim.bats` presently uses it.
* chore: Remove unnecessary account removal assert
Traced this back to the original PR where it appears to have been a typo and was probably intended as a cleanup on the `user4` account. Not necessary, removing.
* chore: Rename `accounts.bat` -> `account_management.bats`
---------
* feedback: Avoid `ls` for detecting directories
Replace `ls -d` approach from original test cases
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* feedback: Remove asserting empty output on failure
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* tests: Migrate and combine ENV tests for `*_INET_PROTOCOLS`
These two features + tests were introduced years apart but serve the same purpose for both Postfix and Dovecot.
Using `--openssl` uses the native `openssl` package within the image instead of the older `1.0.2` bundled from `testssl.sh`.
The test is only testing cipher suite compatibility is what we expect it to be, thus we do not need to run `testssl.sh` with a broader range of ciphers.
* add functionality for filtering mail log by ID
This was not planned, but as @polarthene mentioned in
https://github.com/docker-mailserver/docker-mailserver/pull/3033#issuecomment-1407169569
, filtering the mail log by email ID would be (the only) correct
approach for the Rspamd test (to eliminate race conditions).
I asserted the currect state, and came to the conclusion that this might
(or actually is) something we want in more than one place. So I went
ahead and implemented a solution.
The solution for acquiring the ID is a bit slower because it ensures the
mail queue is empty _before_ and _after_ the mail is sent. This is the
tradeoff one has to make if they want to send multiple emails in one
test file and get their IDs.
I hope you like this approach. I will provide another PR that adjusts
our current tests to use these new functions.
* added note about our helper functions in the docs
I think our work for our custom test framework should be noted in the
docs for newcomers to better understand what they should do.
* adjust Rspamd test to use new helpers for sending
* improve filter helpers further
* add sanity check when acquiring mail ID
* re-add `refute_output` to test which should now work well
This doesn't make any difference to the tests performed here (_partly due to `--preference`_).
It would make a difference if performing a test for receiving a grade, which would otherwise fail due to chain of trust not being verifiable for a self-signed certificate (_or a signed certificate without a CA public key to verify against_)
* chore: Use a common method to check domain and fqdn config
* chore: Shift other test cases into shared test methods
* chore: Add another shared method for checking mail headers
* chore: Add another shared method for checking hostname
* refactor: Improve quality of shared test methods
Based on changes from an earlier closed hostname PR from Oct 2021 with additional revision to use `assert_output` and more thorough checking of values expected in output.
* chore: Move clean shutdown test to `process-check-restart.bats`
This was originally a single test case in `tests.bats` intended for `supervisord` testing.
It seems at some point it got reassigned to a hostname override test container, and then migrated to separate test file for hostname override test by accident.
It now belongs in the correct place again, as hostname config should have nothing to do with a graceful shutdown?
* chore: Prepare for migrating to use `test/helper/setup.bash`
* chore: Rename containers and configured FQDN settings
* chore: Convert to using common container setup helpers
Wait for SMTP port is left at the end to avoid additional start-up delays.
* chore: Use `_run_in_container_bash` helper
* chore: Be more specific on matching mail headers
- I could do multiple container grep calls instead, but opted to match by lines in file. This better ensures values are being matched to the correct lines.
- Renamed the test case descriptions.
- Expanded test coverage of the 4th container as it represents another DNS config, while the 3rd is just the 4th container with the `SRS_DOMAINNAME` env added, no value in more coverage there.
* chore: Remove redundant test coverage in `tests.bats`
These checks are performed in `mail_hostname.bats` with better coverage.
* chore: Move each containers setup into it's own test-case instead
* chore: Re-arrange container name IDs
The original `fqdn-with-subdomain` is now `with-nis-domain` which is more accurate. A new test case will properly cover the default `--hostname` only config that is not a bare domain.
* chore: Re-arrange test cases to align with new ID ordering
This commit just shifts the test cases, no new changes to any content beyond that.
* chore: Add new test case for default config
* chore: Review feedback `_run_in_container_bash` to `_run_in_container`
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* chore: Additional review feedback
- Fix a suggested change bug with quote wrapping an interpolated variable.
- Convert two other `_bash` methods that were missed from review.
- Apply the last two suggested changes from review.
* chore: `_exec_in_container_bash` to `_exec_in_container`
The `| head -n 1` can be dropped if we know for sure it's only one line, which is what we expect. Quotes can then be dropped too.
---------
Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com>
* tests(fix): `spam_junk_folder.bats` Wait on Amavis port to be ready
Postfix can potentially be ready before Amavis is. This caused test failures as mail was sent before Amavis was ready to process it.
Both test cases shared the same test logic, except for the expected location to deliver the spam to. Extracted into a shared test method, and moved the port conditions into there.
* tests(chore): `spam_junk_folder.bats` minor revisions
Test case descriptions, container names and test prefix are now more descriptive of what is under test here (an ENV for Amavis).
* tests(chore): Move Amavis bounce test into `spam_junk_folder.bats`
These two tests seem to be related to the same feature. Grouping them into a single test file instead.
* tests(refactor): Split shared method into smaller methods
Now it can be better shared with the bounce test case.
* tests(chore): Shift test cases to match their CONTAINER_NAME order
No changes to code, just cut + paste of the `CONTAINER3_NAME` test case to shift it to the last test case position.
* added options to toggle OpenDKIM & OpenDMARC
rspamd can provide DKIM signing and DMARC checking itself, so users
should be able to disable OpenDKIM & OpenDMARC. The default is left at
1, so users have to to opt-in when the want to disable the features.
* misc small enhancements
* adjusted start of rspamd
The order of starting redis + rspamd was reversed (now correct) and
rspamd now starts with the correct user.
* adjusted rspamd core configuration
The main configuration was revised. This includes AV configuration as
well as worker/proxy/controller configuration used to control the main
rspamd processes.
The configuration is not tested extensively, but well enough that I am
confident to go forward with it until we declare rspamd support as
stable.
* update & improve the documentation
* add tests
These are some initial tests which test the most basic functionality.
* tests(refactor): Improve consistency and documentation for test helpers (#3012)
* added `ALWAYS_RUN` target `Makefile` recipies (#3013)
This ensures the recipies are always run.
Co-authored-by: georglauterbach <44545919+georglauterbach@users.noreply.github.com>
* adjusted rspamd test to refactored test helper functions
* improve documentation
* apply suggestions from code review (no. 1 by @polarthene)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
* streamline heredoc (EOM -> EOF)
* adjust rspamd test (remove unnecessary run arguments)
Co-authored-by: Brennan Kinney <5098581+polarathene@users.noreply.github.com>
