Commit graph

4 commits

Author SHA1 Message Date
Brennan Kinney e9f04cf8a7
chore: Change setup config dkim default key size to 2048 (open-dkim) (#3508)
* 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.
2023-08-29 09:40:02 +12:00
Brennan Kinney 88767f7cc8
tests(refactor): open_dkim.bats (#3060)
* 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>
2023-02-10 00:18:06 +13:00
Brennan Kinney 835056d707 tests(chore): Use REPOSITORY_ROOT export var from Makefile
Allows for using `load` with an absolute path instead of a relative one, which makes it possible to group tests into different directories.

Parallel tests differ slightly, loading the newer `helper/common.bash` and `helper/setup.bash` files instead of the older `test_helper/common.bash` which serial tests continue to use.
2022-11-26 14:52:42 +13:00
Georg Lauterbach 59127e2b25 tests(chore): Rename test files to serial and parallel types
- `test_helper.bats` needs more work than this PR provides to be compatible with parallel tests, so must remain as a serial test for now.
- `spam_bounced.bats` had failures as a serial test, but works well converted to a parallel test in a future commit.
2022-11-26 14:52:42 +13:00
Renamed from test/open_dkim.bats (Browse further)