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 SkipDBWalk config option #226

Closed
wants to merge 0 commits into from

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