DKIM implementations #5620

DorianCoding · 2025-02-20T09:54:43Z

Hello,

I would like to implement DKIM on Rust emails, so SPF and DKIM can be fulfilled and allows better transmission of mails.

Thanks.

tessus · 2025-02-20T10:06:19Z

May I ask why you want to implement this on the client? By client I mean the mail client. vw server acts as a mail client (unless the sendmail binary is directly used).

DKIM and SPF are usually handled by the mail server. Also, what happens if you use DKIM on the client and then on the server again? Or actually it's worse, when using DKIM on the client but not on the server. The server will add headers that in turn will render the DKIM signature created on the client invalid.
Have you tested all possible combinations?

BlackDex · 2025-02-20T10:52:00Z

@tessus, sendmail is nothing more then a client in our case. It's how the sendmail is configured which might make it differently

DorianCoding · 2025-02-20T11:58:28Z

Hello,

Yes I know but some person might use DKIM on their own without their server and it would be wise to allow people to do it if they want to, because the implementation is minimal as you can see. It might not be useful for everyone, I fully agree and if the server already manages SPF/DKIM and so on, there is no need to implement anything more but it might not be the case and people should be able to implement it. I've not tested all combinations but rapid tests shows it can work if people are actually configuring it themselves before and wants to continue.

src/mail.rs

DorianCoding · 2025-02-22T09:07:18Z

@williamdes Sorry, I've forgot I can implement fs::read_to_string which automatically manages the reading and the allocation, it would be way easier. What do you think?

williamdes

Nice

DorianCoding · 2025-02-22T14:31:05Z

Weird the changes I have made to put Rust 2024 has been pushed. Please just push changes to mail if you proceed. Sorry and thanks.

stefan0xC

I'm not sure I agree that this should be added but I added some comments. The way to specify the signing key should be better documented (and configuration options should also be added to the .env.template).

stefan0xC · 2025-08-08T07:45:16Z

src/mail.rs

+    if let Some(sig) = dkim {
+        dkim_sign(&mut email, &sig);


Suggested change

if let Some(sig) = dkim {

dkim_sign(&mut email, &sig);

if let Some(dkim_config) = dkim {

dkim_sign(&mut email, &dkim_config);

stefan0xC · 2025-08-08T07:46:45Z

src/config.rs

+        /// Dkim algo (true if RSA else ed25519)
+        dkim_algo:                     bool,   true,   option;
+        /// Dkim infos (selector:domain)
+        dkim_infos:                    String, true,   option;


Why not let users configure selector and domain separately?

stefan0xC · 2025-08-08T07:49:01Z

src/config.rs

+        /// Dkim signature (type:privatekey). Private must be base64-encoded ed key or PKCS#1 format RSA key.
+        dkim_signature:                String, true,   option;


What does type:privatekey mean? Nothing indicates that this should be a path to a file. If you want to configure it via a file you should be able to configure a DKIM_SIGNATURE_FILE.

Also this should probably called dkim_signing_key instead.

stefan0xC · 2025-08-08T07:50:14Z

src/mail.rs

+                let sig = match fs::read_to_string(sig) {
+                    Err(e) => {
+                        debug!("Cannot read DKIM file. Err is {:?}", e);
+                            None
+                    },


We also already have a mechanism for loading config values from files so this should not be necessary.

stefan0xC · 2025-08-08T07:55:14Z

src/mail.rs

 async fn send_email(address: &str, subject: &str, body_html: String, body_text: String) -> EmptyResult {
    let smtp_from = &CONFIG.smtp_from();
-
+    let dkim = match (CONFIG.dkim_signature(), CONFIG.dkim_infos()) {


The validation should be done at startup (in config.rs) instead of when sending a mail.

stefan0xC · 2025-08-08T07:59:46Z

Cargo.toml

+edition = "2024"
+rust-version = "1.85.0"


Weird the changes I have made to put Rust 2024 has been pushed. Please just push changes to mail if you proceed. Sorry and thanks.

Why don't you remove those changes from your PR if you don't want them in it?

Besides that, it this PR needs a rebase on main, and the edition should not be changed!

stefan0xC · 2025-08-08T08:02:54Z

src/config.rs

+        /// Dkim algo (true if RSA else ed25519)
+        dkim_algo:                     bool,   true,   option;


Do we have to support both methods? Also the naming is wrong if this is a toggle to use an RSA key. (I would call it dkim_use_rsa instead (and say that the default is ed25519 otherwise).

stefan0xC · 2025-08-08T08:29:09Z

Also you should probably run cargo fmt and cargo clippy otherwise the automatic checks (that have not run yet) will fail.

BlackDex · 2025-08-08T10:03:04Z

rustfmt.toml

@@ -1,4 +1,4 @@
-edition = "2021"
+edition = "2024"


This should not be changed!

BlackDex · 2025-08-08T10:04:49Z

rust-toolchain.toml

This file should not be edited, and you probably want to rebase on our current main branch.

BlackDex · 2025-08-08T10:05:52Z

Cargo.toml

+edition = "2024"
+rust-version = "1.85.0"


Besides that, it this PR needs a rebase on main, and the edition should not be changed!

tessus · 2025-08-08T17:37:32Z

I am still a bit fuzzy on the why and how.

but some person might use DKIM on their own without their server and it would be wise to allow people to do it if they want to

How do people use DKIM on their own?

DKIM requires the DNS record to be updated with TXT entries. Do you expect the vw admin to update the mail server's DNS record? Or do you plan to send emails via a random mail server, but let people think they are coming from vaultwarden and add the DKIM TXT entries to the vw DNS record?

DorianCoding · 2025-08-14T10:34:25Z

Hello,

Thanks everyone for your review, I have redo my changes in light of the new version. I've taken into account your comments.

How do people use DKIM on their own?

DKIM requires the DNS record to be updated with TXT entries. Do you expect the vw admin to update the mail server's DNS record? Or do you plan to send emails via a random mail server, but let people think they are coming from vaultwarden and add the DKIM TXT entries to the vw DNS record?

TXT is the public key. You just need vaultwarden to get the private key and sign. However, depending on the MTA, it might be useless or wrong. However if the MTA does not implement it and does not change header, DKIM implementation by the client would work. It is for me really important as some people might need it in their configuration and email sending by Vaultwarden should pass filters to avoid issues and ensure security.

By default, From/Subject/To/Date are signed. Those are headers that are unlikely to be changed by default MTA.

tessus · 2025-08-14T20:57:49Z

TXT is the public key. You just need vaultwarden to get the private key and sign. However, depending on the MTA, it might be useless or wrong. However if the MTA does not implement it and does not change header, DKIM implementation by the client would work. It is for me really important as some people might need it in their configuration and email sending by Vaultwarden should pass filters to avoid issues and ensure security.

My thought process went into another direction and maybe I did express myself poorly. Let me try to unpack this.

You can derive the public key from the private key, but not the other way around. This means the person who setups vw to use DKIM, must be the person who has access to the private keys AND must add the public keys to the DNS record.
You can't send emails from your home network, because providers (at least in North America and Europe) block outgoing mails. And even if you could, dynamic IP ranges are included in various DSNBL lists, which means the receiving email server will most likely discard them.
Using a public mail server like google/yahoo/protonmail/whatever is fragile, because they sign the mail with DKIM. In theory this should be fine according to the DKIM specs, but in reality is not so straightforward (double signing can be a mess). Apart from that some mailservers add a footer (similar to what mailing lists sometimes do), which makes your locally signed mail invalid.
Other mail servers use 7bit encoding. This alone screws up the signature.

This means you either have to be in control (the admin) of the mail server or know exactly how that mail server behaves (no mail body or header changes, transfer encoding, ...).

If it is the former, why not setup DKIM on the mail server? Even if you have never done it before, it takes less than 15 minutes to do so. My question here is: what is the gain? 15 minutes? Let me remind you, the vw admin has to change the DNS record to add the public keys. So the only gain is not having to add dkim to the mail server - 15 minutes.

If it is the latter, don't you think the group of people who have all this type of info is rather small? How many people would actually be able to use your PR? All I am saying is that this use case is very specific and the target audience probably small.

Please don’t take this the wrong way, I do not want to belittle your PR or your use case. I just do not understand why it would warrant adding something that only a few people are able to use.
But it is also very possible that I misunderstood your use case.
Maybe people have access to mail servers that do not sign email, use the right transfer encoding, don't change body and headers, follow all specs to the letter, ...
My comment was not meant to deter anyone from merging your work (which I couldn't, even if I wanted to, since my opinion does not matter at all). It's about me. I am the one, who does not understand your use case, which is why I asked for the why and how.
I am curious about the gain and reusability. That's all.

By default, From/Subject/To/Date are signed. Those are headers that are unlikely to be changed by default MTA.

This is unfortunately not always the case. e.g. mail servers sometimes use the mail server's timestamp, instead of just forwarding the date: header. As mentioned before, if you were to use your local MTA to send email, you would have to use a smart host to submit the email via an SMTP relay. Once again, you have to know exactly how all the parts in the chain behave.

DorianCoding · 2025-08-15T09:35:37Z

Hello,

I agree with nearly all your comments.

If it is the former, why not setup DKIM on the mail server? Even if you have never done it before, it takes less than 15 minutes to do so.

If you relay your mail like you say on a server that you don't control. Maybe that server don't do DKIM. Maybe you don't want this server to sign for you. Maybe it is not free to implement DKIM... Even this, if you use a relay that sends your email, you or this provider needs DNS access, unless you don't send mails from your domain.

My question here is: what is the gain? 15 minutes? Let me remind you, the vw admin has to change the DNS record to add the public keys. So the only gain is not having to add dkim to the mail server - 15 minutes.

The gain is where the person cannot or does not want to implement DKIM on the MTA but has domain control and can know headers signed (the one I've sent to you) are not altered by their MTA.

If it is the latter, don't you think the group of people who have all this type of info is rather small? How many people would actually be able to use your PR? All I am saying is that this use case is very specific and the target audience probably small.
Please don’t take this the wrong way, I do not want to belittle your PR or your use case. I just do not understand why it would warrant adding something that only a few people are able to use.

I don't take the wrong way at all and your comments are legitimate. The target audience might be small. This implementation is not optimal but it is a work-around for specific people and deliverability of emails sent by Vaultwarden is crucial for many reasons. As such, it is just an addition of a feature from a crate that already exists in mail. If necessary, we could also implement it as a feature. In conclusion, even if DKIM support should not be done from the client side, it has some applications for some people and the minimum changes required to achieve this should give us reasons to implement it.

tessus · 2025-08-15T10:03:35Z

The gain is where the person cannot or does not want to implement DKIM on the MTA but has domain control and can know headers signed (the one I've sent to you) are not altered by their MTA.

Ok, this makes sense, at least the "cannot implement" part.

For me it was rather a given that people who had server control (for vw) and control over DNS, also had control over a mail server. But I guess it's possible tha this might not be the case. Weird, but legitimately ok.

My question is answered. Thank you.

As for the implementation I think the current approach is fine. It's not a huge change, the binary size wouldn't increase by a lot, and there are no performace implications, thus a feature is most likely an overkill.

DorianCoding force-pushed the main branch from 374fc69 to 30d9001 Compare February 20, 2025 12:01

williamdes reviewed Feb 20, 2025

View reviewed changes

src/mail.rs Outdated Show resolved Hide resolved

williamdes approved these changes Feb 22, 2025

View reviewed changes

stefan0xC suggested changes Aug 8, 2025

View reviewed changes

BlackDex requested changes Aug 8, 2025

View reviewed changes

DorianCoding closed this Aug 14, 2025

DorianCoding force-pushed the main branch from 99cdc04 to 8e7eeab Compare August 14, 2025 09:02

changes to pull requests + rebase

3f40fc4

DorianCoding reopened this Aug 14, 2025

DorianCoding requested review from BlackDex and stefan0xC August 14, 2025 10:32

		/// Dkim signature (type:privatekey). Private must be base64-encoded ed key or PKCS#1 format RSA key.
		dkim_signature: String, true, option;

		/// Dkim algo (true if RSA else ed25519)
		dkim_algo: bool, true, option;

Uh oh!

DKIM implementations #5620

Are you sure you want to change the base?

DKIM implementations #5620

Uh oh!

Conversation

DorianCoding commented Feb 20, 2025

Uh oh!

tessus commented Feb 20, 2025

Uh oh!

BlackDex commented Feb 20, 2025

Uh oh!

DorianCoding commented Feb 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

DorianCoding commented Feb 22, 2025

Uh oh!

williamdes left a comment

Choose a reason for hiding this comment

Uh oh!

DorianCoding commented Feb 22, 2025

Uh oh!

stefan0xC left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefan0xC Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefan0xC commented Aug 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tessus commented Aug 8, 2025

Uh oh!

DorianCoding commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tessus commented Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DorianCoding commented Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tessus commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DorianCoding commented Feb 20, 2025 •

edited

Loading

stefan0xC Aug 8, 2025 •

edited

Loading

DorianCoding commented Aug 14, 2025 •

edited

Loading

tessus commented Aug 14, 2025 •

edited

Loading

DorianCoding commented Aug 15, 2025 •

edited

Loading