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

Per request sign algorithm (A part of issue #6) #202

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

futatuki
Copy link

@futatuki futatuki commented Mar 1, 2024

This extends KeyTable value fields as the issue comment #6 (comment) said.

With an extended KeyTable and lua script, I could confirm that the milter can both rsa-sha256 and ed25519-sha256 signature.

(Edit)
Now, this PR depends on PR #207 for conversion table for dkim_alg_t and dkim_canon_t (but not including those commits).

@futatuki
Copy link
Author

futatuki commented Mar 1, 2024

I didn't care opendkim-testkey, so I'll update it later.

@futatuki
Copy link
Author

futatuki commented Mar 3, 2024

I didn't care opendkim-testkey, so I'll update it later.

Done. However library function dkim_test_key() does not support ed25519 keys, so opendkim-testkey still reports "keys do not match" at ed25519 key entry.

@futatuki
Copy link
Author

futatuki commented Mar 3, 2024

As commit message of commit 2fb3b18 is wrong, rebase latest 2 commit.

@futatuki futatuki force-pushed the per-request-sign-algorithm branch from 0abd128 to 5c47d2c Compare March 3, 2024 04:19
@futatuki
Copy link
Author

futatuki commented Mar 3, 2024

As commit message of commit 2fb3b18 is wrong, rebase latest 2 commit.

Done.

@futatuki
Copy link
Author

futatuki commented Mar 9, 2024

I separated lookup functions and tables from opendkim/opendkim.c into opendkim/opendkim-const.c because opendkim/opendkim-testkey.c uses conversion from sign algorithm string to sign algorithm internal code.

However I found it has been provided in libopendkim/dkim-tables.h, so I'll remove last 2 commit and I'll use dkim_name_to_code in opendkim/opendkim-testkey.c.

I think lookups for dkimf_canon and dkimf_sign in opendkim/opendkim.c should also use dkim_name_to_code or dkim_code_to_name with canonicalizations table and algorithms, but it is another issue.

@futatuki futatuki marked this pull request as draft March 9, 2024 18:42
@futatuki
Copy link
Author

It turned out that all nametable s which should be used with dkim_code_to_name() or dkim_name_to_code() are all not exposed outside of the library because of their name, while the functions are exposed.

I believe those nametables are worth exposed. Also, there is no bad side effect if names of the tables are renamed by adding prefix 'dkim_', because all macros in the tables are already exposed.

…ield

* opendkim/opendkim.c
  (): Add forward declaration for dkimf_lookup_strtoint
  (dkimf_add_signrequest): Extract sign algorithm from 4th field in key
   table and set it in the sign request.
  (dkimf_config_load): Allow 4th field in key table value.
futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Mar 23, 2024
@futatuki futatuki force-pushed the per-request-sign-algorithm branch from 5c47d2c to a05fead Compare March 23, 2024 08:08
@futatuki futatuki marked this pull request as ready for review March 23, 2024 08:13
@futatuki
Copy link
Author

Now, it uses lookup table for dkim_alg_t and dkim_canon_t in libopendkim (by using PR #207).

futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Mar 24, 2024
futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Apr 9, 2024
futatuki added a commit to futatuki/OpenDKIM that referenced this pull request Apr 26, 2024
…-sign-algorithm

Extend KeyTable to hold optionally signing algorithm per key entry.
@futatuki futatuki changed the title Per request sign algorithm (A pert of issue #6) Per request sign algorithm (A part of issue #6) Jun 21, 2024
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.

1 participant