Skip to content

Add SkipDBWalk config option#226

Closed
r-a-z-v-a-n wants to merge 0 commit intotrusteddomainproject:developfrom
r-a-z-v-a-n:develop
Closed

Add SkipDBWalk config option#226
r-a-z-v-a-n wants to merge 0 commit intotrusteddomainproject:developfrom
r-a-z-v-a-n:develop

Conversation

@r-a-z-v-a-n
Copy link

SkipDBWalk skips the dkimf_db_walk when starting opendkim. This allows for faster startup when there are large databases.

According to simple tests, bad keys will not crash opendkim, but will instead output an error.

@r-a-z-v-a-n
Copy link
Author

Simple test results:
If the private key is bad:

Sep 08 19:32:56 bookworm01 opendkim[759]: OpenDKIM Filter v2.11.0 starting
Sep 08 19:59:13 bookworm01 opendkim[759]: 349634048D: SSL error:0680008E:asn1 encoding routines::not enough data
Sep 08 19:59:13 bookworm01 opendkim[759]: 349634048D: dkim_eom(): resource unavailable: d2i_PrivateKey_bio() failed

451 4.7.1 Service unavailable - try again later

If selector is NULL (should be prevented by sql not null option):

451 4.7.1 Service unavailable - try again later
Sep 08 20:06:12 bookworm01 postfix/cleanup[1546]: BCE424048F: milter-reject: END-OF-MESSAGE from localhost[::1]: 4.7.1 Service unavailable - try again later; from=<xxxx@linux>

Sep 08 20:06:12 bookworm01 opendkim[759]: KeyTable entry for '1' corrupt
Sep 08 20:06:12 bookworm01 opendkim[759]: BCE424048F: error loading key '1'

If key is NULL

Sep 08 20:09:16 bookworm01 opendkim[759]: KeyTable entry for '1' corrupt
Sep 08 20:09:16 bookworm01 opendkim[759]: 527A24048D: error loading key '1'

Sep 08 20:09:16 bookworm01 postfix/cleanup[1573]: 527A24048D: milter-reject: END-OF-MESSAGE from localhost[::1]: 4.7.1 Service unavailable - try again later; from=<xxx@linux>

in neither case did opendkim crash. Furthermore, you can solve a lot of these issues with db table constraints.

@futatuki
Copy link

Thank you for the implementation.

The logic looks good to me, however I have some points on my mind.

  • The option name "SkipDBWalk" may not be clear for users, and consistency check of Signing table itself can be skip by default. How about rename the option to "CheckSigningTable" (or "SigningTableCheck", regarding the order in the sample config file and man page), with inverse logic?
  • the place for adding the new code (in structure member and in dkimf_config_load()) among the other options. However I can't see what it should be. If you can explain why they are there, please let me know.
  • Indentation in the file. Some lines in newly added seems using expanded spaces for the indentation. It seems the original style uses tab for logical indentation + spaces for continuation line, adjusting offset from top column of the logical line.

Also it is nice if it have an command line option to check the table consistency only (not run milter), or dedicated command line tool to check it, like opendkim-testkey(8).

@r-a-z-v-a-n
Copy link
Author

I closed this commit by accident, but I created a clean pull in
#228

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.

2 participants