add CheckSigningTable config option #228

r-a-z-v-a-n · 2024-09-15T19:05:43Z

When CheckSigningTable is set to no, the keys in KeyTable are no longer verified when config is loaded.
This helps with large databases. This commit only adds support for USE_ODBX.

When CheckSigningTable is set to no, the keys in KeyTable are no longer verified when config is loaded. This helps with large databases. This commit only adds support for USE_ODBX.

when comparing if (conf->conf_checksigningtable == TRUE) and CheckSigningTable no in the config, no acts as TRUE. Therefore, it is better to use != FALSE.

r-a-z-v-a-n · 2024-09-15T20:02:15Z

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.

r-a-z-v-a-n · 2024-09-15T20:05:32Z

this is continued from #226

r-a-z-v-a-n · 2024-09-15T20:08:56Z

also should there be a command line option for it ? example,
opendkim/opendkim.c:15594: /* process command line options */
if yes, which letter may I use ?

opendkim/opendkim.conf.5.in

As advised by futatuki, specify SigningTable instead of the "database" in the description.

futatuki · 2024-09-15T20:36:29Z

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

I'll take a look later, if it can avoid crash even if the key entry is corrupted. (But it is another issue)

futatuki · 2024-09-15T20:45:57Z

also should there be a command line option for it ? example, opendkim/opendkim.c:15594: /* process command line options */ if yes, which letter may I use ?

Currently 'C' is not in use, however it might be the time we shoud consider to use word options, although getopt_long() is not in POSIX...

r-a-z-v-a-n · 2024-09-15T20:54:20Z

can we consider the long options issue as a separate issue?
one commit at a time so we dont get overloaded.

futatuki · 2024-09-15T21:04:47Z

can we consider the long options issue as a separate issue? one commit at a time so we dont get overloaded.

ya, sorry, I think it is a separate issue. It is not good that multiple issue in a PR.

futatuki · 2024-09-15T21:35:25Z

This feature may useful even if ODBX feature is not used, so I think it is not need to restrict it on USE_ODBX is true. No other things to say, it looks good to me, for the commits till 906a8b4.

You can freely determine the command line option letter for this feature, if you implement it (perhaps no one complains about it). However if I implement a command line option for checking SiginigTable consistency and exit, I'll select 'C' for it if it is still not in use :)

Allow the use of -C to disable CheckSigningTable (or set to no).

Allow disabling of CheckSigningTable for other databases such as LDAP. This option disables the walking of SigningTable to look for missing keys in KeyTable when the config gets loaded.

r-a-z-v-a-n · 2024-09-15T23:03:41Z

see commit 898f6ec for args option -C
see commit 35f13b1 to allow for disabling CheckSigningTable for all databases.

r-a-z-v-a-n · 2024-09-15T23:08:07Z

lhy is the develop branch so far ahead of master branch ?
There are 127 pending commits, last one 2 years ago.
The master branch has not been updated in 6 years.
When is the next planned release ?

As requested by futatuki, I will use -g for CheckSigningTable and reserve -C for future option to check the database tables.

r-a-z-v-a-n · 2024-09-16T00:31:37Z

You can freely determine the command line option letter for this feature, if you implement it (perhaps no one complains about it). However if I implement a command line option for checking SiginigTable consistency and exit, I'll select 'C' for it if it is still not in use :)

oops i just realized what you said.
I changed the option to -g in commit ee40b42

futatuki · 2024-09-16T04:18:34Z

As you added a new command line option, it should be described in opendkim(8) :)

lhy is the develop branch so far ahead of master branch ?
There are 127 pending commits, last one 2 years ago.
The master branch has not been updated in 6 years.
When is the next planned release ?

I think this project is not working any more. So I made my own branch, maintain it, and I use here as a collection center of issues and proposals of the changes.

r-a-z-v-a-n · 2024-09-16T05:29:46Z

Thank you @futatuki . I would like to commit these changes to your mirror as well.
I see your stable branch is 212 commits ahead : )

futatuki · 2024-09-16T12:04:05Z

It would be great if you could add a description about new "-g" option to opendkim/opendkim.8.in? I'd like to see your commit for it, not mine :)

r-a-z-v-a-n · 2024-09-16T19:09:39Z

see commit 3fc8cb7 for -g in opendkim(8) man page.

futatuki · 2024-09-16T20:23:24Z

opendkim/opendkim.8.in

+.I \-g
+Set CheckSigningTable to no.  This means that when the config is loaded,
+The SigningTable will not be checked for any missing keys in
+the KeyTable.


A typo " the SiningTable"

The mention to CheckSigingTable option is not so important for understanding what this commandline option does. So it is good to move it after the last sentence of the paragraph, with reference information to opendkim.conf(5).

I see a typo, but in your comment, not the code.
Anyway I re-wrote it based on your suggestion.
See commit 3dabd5f

futatuki · 2024-09-16T20:25:05Z

see commit 3fc8cb7 for -g in opendkim(8) man page.

Thank you!

Although I 'm not good at writing English, I've reviewed it.

r-a-z-v-a-n · 2024-09-16T21:35:30Z

thank you so much for all your help futatuki!

r-a-z-v-a-n

since we made it global, row doesnt always apply, example LDAP server.

futatuki · 2024-09-19T08:59:46Z

Looks good. Thank you!

…ningTable Add CheckSigningTable config option When CheckSigningTable is set to no, the keys in KeyTable are no longer verified when config is loaded. Also implement a command line option -g for skipping SigningTable verification. trusteddomainproject#228

add CheckSigningTable config option

552aba7

When CheckSigningTable is set to no, the keys in KeyTable are no longer verified when config is loaded. This helps with large databases. This commit only adds support for USE_ODBX.

r-a-z-v-a-n mentioned this pull request Sep 15, 2024

Add SkipDBWalk config option #226

Closed

conf_checksigningtable changed ==TRUE with !=FALSE

cd0a7f4

when comparing if (conf->conf_checksigningtable == TRUE) and CheckSigningTable no in the config, no acts as TRUE. Therefore, it is better to use != FALSE.

r-a-z-v-a-n mentioned this pull request Sep 15, 2024

OpenDKIM times out on startup #209

Open

futatuki reviewed Sep 15, 2024

View reviewed changes

opendkim/opendkim.conf.5.in Outdated Show resolved Hide resolved

CheckSigningTable improve documentation

906a8b4

As advised by futatuki, specify SigningTable instead of the "database" in the description.

r-a-z-v-a-n added 2 commits September 15, 2024 18:56

CheckSigningTable option as -C argument

898f6ec

Allow the use of -C to disable CheckSigningTable (or set to no).

CheckSigningTable make option always available

35f13b1

Allow disabling of CheckSigningTable for other databases such as LDAP. This option disables the walking of SigningTable to look for missing keys in KeyTable when the config gets loaded.

CheckSigningTable use arg -g instead of -C

ee40b42

As requested by futatuki, I will use -g for CheckSigningTable and reserve -C for future option to check the database tables.

r-a-z-v-a-n mentioned this pull request Sep 16, 2024

allow CheckSigningTable option to enable/disable checking for missing keys. r-a-z-v-a-n/OpenDKIM#1

Closed

CheckSigningTable add -g arg to opendkim(8) man page

3fc8cb7

futatuki reviewed Sep 16, 2024

View reviewed changes

CheckSigningTable improve man page

3dabd5f

SigningTable improve man page

c7d845b

r-a-z-v-a-n commented Sep 19, 2024

View reviewed changes

remove extra line

6bea2fa

futatuki mentioned this pull request Oct 3, 2024

Can't connect to memcached #218

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add CheckSigningTable config option #228

add CheckSigningTable config option #228

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024 •

edited

Loading

futatuki commented Sep 15, 2024

futatuki commented Sep 15, 2024 •

edited

Loading

r-a-z-v-a-n commented Sep 15, 2024

futatuki commented Sep 15, 2024

futatuki commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 16, 2024

futatuki commented Sep 16, 2024 •

edited

Loading

r-a-z-v-a-n commented Sep 16, 2024 •

edited

Loading

futatuki commented Sep 16, 2024

r-a-z-v-a-n commented Sep 16, 2024

futatuki Sep 16, 2024

r-a-z-v-a-n Sep 19, 2024

futatuki commented Sep 16, 2024

r-a-z-v-a-n commented Sep 16, 2024

r-a-z-v-a-n left a comment

futatuki commented Sep 19, 2024

add CheckSigningTable config option #228

Are you sure you want to change the base?

add CheckSigningTable config option #228

Conversation

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024 • edited Loading

futatuki commented Sep 15, 2024

futatuki commented Sep 15, 2024 • edited Loading

r-a-z-v-a-n commented Sep 15, 2024

futatuki commented Sep 15, 2024

futatuki commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 15, 2024

r-a-z-v-a-n commented Sep 16, 2024

futatuki commented Sep 16, 2024 • edited Loading

r-a-z-v-a-n commented Sep 16, 2024 • edited Loading

futatuki commented Sep 16, 2024

r-a-z-v-a-n commented Sep 16, 2024

futatuki Sep 16, 2024

Choose a reason for hiding this comment

r-a-z-v-a-n Sep 19, 2024

Choose a reason for hiding this comment

futatuki commented Sep 16, 2024

r-a-z-v-a-n commented Sep 16, 2024

r-a-z-v-a-n left a comment

Choose a reason for hiding this comment

futatuki commented Sep 19, 2024

r-a-z-v-a-n commented Sep 15, 2024 •

edited

Loading

futatuki commented Sep 15, 2024 •

edited

Loading

futatuki commented Sep 16, 2024 •

edited

Loading

r-a-z-v-a-n commented Sep 16, 2024 •

edited

Loading