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

… file.

…steddomainproject#219

Allow overriding CheckSigningTable config option by command line options

futatuki · 2024-10-11T03:19:46Z

opendkim/opendkim.8.in

@@ -262,6 +263,14 @@ The value may include two different canonicalizations separated by a
 slash ("/") character, in which case the first will be applied to the
 headers and the second to the body.
 .TP
+.I \-G


Keep alphabetic order in the descriptions of command line options :) This should be below of -g option now.

New place of -G is still above of -g. It seems the order is lowercase first. (Other changs looks good to me, thank you!)

futatuki · 2024-10-11T03:20:38Z

opendkim/opendkim.conf.5.in

@@ -179,6 +179,11 @@ requires superuser access. A warning will be generated if
 .I UserID
 is not also set.

+.TP
+.I CheckSigningTable (Boolean)
+If set to yes, it walks the SigningTable on boot when it loads the config


As I commented other place, loading config (and the opportunity of the check) may occure other than on boot, by receiving SIGHUP signal.

…-a-n/CheckSigningTable Add command line option to override CheckSigningTable setting on config file. trusteddomainproject#228

…-a-n/CheckSigningTable reorder parameter list for brevity

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

futatuki added 3 commits October 6, 2024 19:58

opendkim: The -g option overrides CheckSigningTable setting on config…

9f99642

… file.

opendkim: Imprement -C option, just opposit of the -g option.

7836770

opendkim/opendkim.8.in: Update descriptions for -C and -g options.

5dcb091

futatuki mentioned this pull request Oct 6, 2024

Allow overriding CheckSigningTable config option by command line options r-a-z-v-a-n/OpenDKIM#2

Merged

futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Oct 7, 2024

RELEASE_NOTES: Record merge of PR trusteddomainproject#228 and PR tru…

38f16f9

…steddomainproject#219

r-a-z-v-a-n added 2 commits October 10, 2024 19:36

Merge pull request #2 from futatuki/check-signingtable-only

c2ec0d8

Allow overriding CheckSigningTable config option by command line options

change -C to -G to match the opposite of -g

745ebee

futatuki reviewed Oct 11, 2024

View reviewed changes

r-a-z-v-a-n added 2 commits October 11, 2024 00:18

improve documentation and fix typo

ac1d2c6

documentation place -G after -g

dfa3dfe

futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Oct 11, 2024

RELEASE_NOTE: update the entry for merging PR trusteddomainproject#228

e902521

reorder parameter list for brevity

5236d47

futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Dec 22, 2024

Re-merge update of pull request trusteddomainproject#228 from r-a-z-v…

49adf9e

…-a-n/CheckSigningTable reorder parameter list for brevity

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

futatuki Oct 11, 2024

r-a-z-v-a-n Oct 11, 2024

futatuki Oct 11, 2024

r-a-z-v-a-n Oct 11, 2024

futatuki Oct 11, 2024

r-a-z-v-a-n Oct 11, 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

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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