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

Replace rspamd with OpenDKIM #186

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Replace rspamd with OpenDKIM #186

merged 3 commits into from
Jan 24, 2024

Conversation

link2xt
Copy link
Contributor

@link2xt link2xt commented Jan 19, 2024

OpenDKIM configuration
has two Lua scripts defining strict DKIM policy.

screen.lua filters out signatures that do not correspond
to the From: domain so they are not even checked.
final.lua rejects mail if it is not outgoing
and has no valid DKIM signatures.

OpenDKIM is configured as a milter on port 25 smtpd
to check DKIM signatures
and on mail reinjecting smtpd
to sign outgoing messages with DKIM signatures.

Closes #179
Closes #182

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

WIP, need to add signature verification now.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

Now it checks at least something but probably needs http://www.opendkim.org/opendkim-lua.3.html scripts to ensure there is at least one valid signature corresponding to the From: field. We should also make sure it signs Autocrypt headers and other headers we care about even if they are not present.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

I have added ScreenPolicyScript, but now it fails with:

Jan 19 13:50:21 c20 systemd[1]: Starting opendkim.service - OpenDKIM Milter...
Jan 19 13:50:21 c20 systemd[1]: Started opendkim.service - OpenDKIM Milter.
Jan 19 13:50:21 c20 opendkim[514403]: OpenDKIM Filter v2.11.0 starting
Jan 19 13:50:32 c20 opendkim[514403]: EE2721F6A9: dkimf_lua_screen_hook() failed: processing error
Jan 19 13:50:32 c20 opendkim[514403]: EF9A31F7AC: dkimf_lua_screen_hook() failed: processing error
Jan 19 13:50:32 c20 opendkim[514403]: F11291F7AD: dkimf_lua_screen_hook() failed: processing error
Jan 19 13:50:32 c20 opendkim[514403]: B656C1F6A9: dkimf_lua_screen_hook() failed: processing error
Jan 19 13:50:32 c20 opendkim[514403]: B661D1F7AC: dkimf_lua_screen_hook() failed: processing error
Jan 19 13:50:32 c20 opendkim[514403]: B6D891F7AD: dkimf_lua_screen_hook() failed: processing error

Edit: fixed the problem and opened upstream issue
trusteddomainproject/OpenDKIM#195

@link2xt link2xt self-assigned this Jan 19, 2024
@link2xt link2xt marked this pull request as ready for review January 19, 2024 16:40
@link2xt link2xt force-pushed the link2xt/opendkim branch 2 times, most recently from 60f485d to 1bc3670 Compare January 19, 2024 16:50
@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

This is ready for review. I tested that it can reject mails by making a condition for skipping signatures in screen.lua always true.
I can send and receive mails between c20.testrun.org running this and nine.testrun.org.

@missytake
Copy link
Contributor

I received the following DKIM signature from c20.testrun.org:

DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=c20.testrun.org;
	s=opendkim; t=1705684900;
	bh=9nI2wlDG0Ykq3gVRJxWi2HowGwYlGYJr4KFoLXtD3bE=;
	h=Subject:From:To:Date:In-Reply-To:References:From;
	b=BT9br06BVJXpKq/hG4TimZaXcOyoouXjQng4Uckxj+omtutf+lvL1Rvf8ttPaSD3d
	 HW71e3tW26ouvU+ttX4hmGAuvoAAWpM2eONtwNkxKAgpIA3BryV3j8+wzMlWYrH8rO
	 wTiObK2/EVyW/sfvdWHOAO+C4KW9t+iRVk50WUw+Yk7ybkJrSrumopG07mRQ/z4PQw
	 V4SDcicpadm3TqRpKhuD4D7dU5tFoLlXYkqb4cBoKAmPUAw0N1kUeKuq7WHGVetMXS
	 kUmDIVmseT7MWNJFC8zn12bs9AGNfnJuub7HO3t6YvUJ1hX4J4rVnj3GJTIq7s1fuO
	 imB3Bzqqy2WXw==

does that mean the autocrypt header was not signed?

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

Yes, does not seem to sign Autocrypt by default. Needs to be added to SignHeaders and possibly OversignHeaders.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

I tested, now Autocrypt is signed.

Copy link
Contributor

@missytake missytake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work nicely :) one thing I noticed is that senders get a nice error message if their server doesn't have working DKIM, but opendkim doesn't log the rejection in journalctl. Can we add that?

c9.testrun.org is a test host without DKIM if you want to test it.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 19, 2024

one thing I noticed is that senders get a nice error message if their server doesn't have working DKIM, but opendkim doesn't log the rejection in journalctl. Can we add that?

Can add LogResults yes to config, then you get:

Jan 19 20:14:46 c20 opendkim[549352]: 841B41F696: signature=HOaum5JZ domain=c9.testrun.org selector=dkim result="key not found in DNS"

But even without this postfix logs already:

Jan 19 20:14:46 c20 postfix/cleanup[549296]: 841B41F696: milter-reject: END-OF-MESSAGE from unknown[2a01:4f9:c012:b695::1]: 5.7.1 No valid DKIM signature found; from=<[email protected]> to=<[email protected]> proto=ESMTP helo=<c9.testrun.org>

So I think additional logs in OpenDKIM are not necessary.

@link2xt link2xt mentioned this pull request Jan 20, 2024
@link2xt link2xt requested a review from missytake January 20, 2024 17:22
This was referenced Jan 20, 2024
Comment on lines 499 to 511
systemd.service(
name="Start and enable rspamd",
service="rspamd.service",
name="Start and enable OpenDKIM",
service="opendkim.service",
running=True,
enabled=True,
restarted=rspamd_need_restart,
restarted=opendkim_need_restart,
)
Copy link
Contributor

@missytake missytake Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, starting the opendkim service during cmdeploy fails for me (on c1):

Jan 23 17:06:42 c1 systemd[1]: Starting opendkim.service - OpenDKIM Milter...
Jan 23 17:06:42 c1 opendkim[359568]: opendkim: milter socket must be specified
Jan 23 17:06:42 c1 opendkim[359568]:         (use "-?" for help)
Jan 23 17:06:42 c1 systemd[1]: opendkim.service: Control process exited, code=exited, status=78/CONFIG
Jan 23 17:06:42 c1 systemd[1]: opendkim.service: Failed with result 'exit-code'.
Jan 23 17:06:42 c1 systemd[1]: Failed to start opendkim.service - OpenDKIM Milter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But /etc/opendkim.conf has it:

Socket                  local:/var/spool/postfix/opendkim/opendkim.sock

The file however does not exist:

ls /var/spool/postfix/opendkim/opendkim.sock
ls: cannot access '/var/spool/postfix/opendkim/opendkim.sock': No such file or directory

Maybe it is created by postfix? Should we maybe reconfigure postfix first so it creates the socket?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I restarted opendkim with systemctl restart opendkim, it started and created the socket. :/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problem deploying to c2:

...
--> Starting operation: apt install opendkim opendkim-tools 
    [c2.testrun.org] Success

--> Starting operation: Files/Template (src=/home/user/src/deltachat/chatmail/cmdeploy/src/cmdeploy/opendkim/opendkim.conf, dest=/etc/opendkim.conf, user=root, group=root, mode=644, config={'domain_name': 'c2.testrun.org', 'opendkim_selector': 'opendkim'})
    [c2.testrun.org] Success

--> Starting operation: Files/Put (src=/home/user/src/deltachat/chatmail/cmdeploy/src/cmdeploy/opendkim/screen.lua, dest=/etc/opendkim/screen.lua, user=root, group=root, mode=644)
    [c2.testrun.org] Success

--> Starting operation: Files/Put (src=/home/user/src/deltachat/chatmail/cmdeploy/src/cmdeploy/opendkim/final.lua, dest=/etc/opendkim/final.lua, user=root, group=root, mode=644)
    [c2.testrun.org] Success

--> Starting operation: Add opendkim directory to /etc 
    [c2.testrun.org] No changes

--> Starting operation: Files/Template (src=/home/user/src/deltachat/chatmail/cmdeploy/src/cmdeploy/opendkim/KeyTable, dest=/etc/dkimkeys/KeyTable, user=opendkim, group=opendkim, mode=644, config={'domain_name': 'c2.testrun.org', 'opendkim_selector': 'opendkim'})
    [c2.testrun.org] Success

--> Starting operation: Files/Template (src=/home/user/src/deltachat/chatmail/cmdeploy/src/cmdeploy/opendkim/SigningTable, dest=/etc/dkimkeys/SigningTable, user=opendkim, group=opendkim, mode=644, config={'domain_name': 'c2.testrun.org', 'opendkim_selector': 'opendkim'})
    [c2.testrun.org] Success

--> Starting operation: Add opendkim socket directory to /var/spool/postfix 
    [c2.testrun.org] Success

--> Starting operation: Generate OpenDKIM domain keys 
    [c2.testrun.org] Success

--> Starting operation: Start and enable OpenDKIM 
    [c2.testrun.org] Job for opendkim.service failed because the control process exited with error code.
    [c2.testrun.org] See "systemctl status opendkim.service" and "journalctl -xeu opendkim.service" for details.
    [c2.testrun.org] Error: executed 0/2 commands
--> pyinfra error: No hosts remaining!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply running cmdeploy run second time works, but I cannot figure out why /etc/opendkim.conf is consistently not found and read the first time.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 23, 2024

@missytake I pushed another commit that apparently fixes the bug. Deployed ccc20.testrun.org from main, then deployed this PR.

Could you try the same thing on another domain?

@link2xt
Copy link
Contributor Author

link2xt commented Jan 24, 2024

Rebased

@missytake
Copy link
Contributor

@missytake I pushed another commit that apparently fixes the bug. Deployed ccc20.testrun.org from main, then deployed this PR.

Could you try the same thing on another domain?

sounds good, will try tomorrow... my eyes hurt already^^

Copy link
Contributor

@hpk42 hpk42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, a nice simplifying improvement. Didn't test it myself though and also am not sure about the one "you need to run it twice" issue. That probably needs to be addressed before merging.

@link2xt
Copy link
Contributor Author

link2xt commented Jan 24, 2024

That probably needs to be addressed before merging.

It is addressed as I commented above, needs someone to test this again by installing a new server using main branch and then deploying this branch on top.

OpenDKIM configuration
has two Lua scripts defining strict DKIM policy.

screen.lua filters out signatures that do not correspond
to the From: domain so they are not even checked.
final.lua rejects mail if it is not outgoing
and has no valid DKIM signatures.

OpenDKIM is configured as a milter on port 25 smtpd
to check DKIM signatures
and on mail reinjecting smtpd
to sign outgoing messages with DKIM signatures.
@link2xt
Copy link
Contributor Author

link2xt commented Jan 24, 2024

Rebased again to fix conflict in master.cf

Copy link
Contributor

@missytake missytake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works :) I first deployed main on c4 and then deployed this PR. A test still expected the old error message, but apart from that it worked.

One thing which could be improved is: printing "please run cmdeploy dns again, your DKIM setup changed" if apt.packages(packages="rspamd", present=False).changed == True. That would help admins notice that they need to update their DKIM record, I guess. But it's no blocker.

@link2xt link2xt merged commit da268b5 into main Jan 24, 2024
@link2xt link2xt deleted the link2xt/opendkim branch January 26, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants