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

Support for DKIM dual-signing #1

Open
yitzhaq opened this issue Jun 14, 2020 · 11 comments
Open

Support for DKIM dual-signing #1

yitzhaq opened this issue Jun 14, 2020 · 11 comments

Comments

@yitzhaq
Copy link

yitzhaq commented Jun 14, 2020

Nice little project, thanks for your efforts!

One thing I'd love to see is the possibility to easily DKIM sign with more than one key/selector.

I tried to look at using signconf.xml to accomplish this, but since I guess you can't use $domain there, this would be a much less flexible solution when you want to use global keys across all domains.

If instead of hardcoding global.key filename and global selector, you could loop through /var/qmail/control/dkim/*.key, and do signing for each of <filename>.key (using <filename> as selector), this should be easily accomplishable.
So if in /var/qmail/control/dkim/ you have first.key and second.key, the message would get two DKIM-Signature headers, one with s=first and one with s=second.

In addition to other possible desirable dual-signing scenarios, this would make it very easy to support RFC 8463 in a nice and backwards compatible way, as described in this article on how to use DKIM with Ed25519.

Thanks in advance!

@pflanze
Copy link
Owner

pflanze commented Jun 14, 2020

Nice if you can find use!

I'll have a look a bit later today or tomorrow when I find time; this project adapted a previous project (see Git history), signconf.xml came from there. This was the first time I used DKIM, and I wasn't sure how to properly use keys for domains, and I remember I adapted it just in the simplest way possible and deferred finding out what's needed to "later". If your instructions can help me get it to a proper state that will be cool. I'll have a look at your links and get back to you.

@pflanze
Copy link
Owner

pflanze commented Jun 14, 2020

(I'm currently using it with a single DKIM key for all of my domains.)

@yitzhaq
Copy link
Author

yitzhaq commented Jun 14, 2020

Sounds like we both came from the same place in terms of DKIM implementation :) Although it was actually Hashcash that brought me here - I was gonna look into adding it to a(nother) wrapper anyway, and thanks to you, now I don't have to - yay!

I guess the "proper" way to use keys is to have a unique key for each domain, but this can be somewhat of a PITA to manage in shared hosting environments. The easier route is the "global" one (I must admit to doing this myself), which probably should be fine as long as you rotate the key every now and then, and obviously keep it safe, as with any key.

So while my proposal won't really help you to "properly" handle keys, as such, it would allow you to dual-sign, which at least I would find quite useful, and which can help adoption of the much nicer Ed25519 keys. Plus I'm sure there are other scenarios in which dual-signing could potentially be useful, although I lack the imagination to think of them right now.

BTW, one more thing, while I'm here; the script also hardcodes method as "simple", which dictates strict canonicalization of both headers and body, so even a spurious whitespace can effectively invalidate the signature. While in a perfect world simple should work fine, the real world certainly is less forgiving. So if a value is gonna be hardcoded, I would propose changing it to "relaxed/relaxed", which is what nearly everyone uses anyway. This has been nicely summarized by others far more eloquent than me.
(I guess this should be a separate issue, but laziness wins yet again :o)

@yitzhaq
Copy link
Author

yitzhaq commented Jun 14, 2020

Oh, I should also mention that, if you choose to implement this, it might potentially break usage of unique, per-domain keys rather than global key(s). But since you haven't ripped out the signconf.xml logic, and since it appears this file will still be used (exclusively) if present, I imagine it should still be possible to support that for anyone who wants it, by using signconf.xml instead.

So it's an either or (that I think your code already handles):

  1. If signconf.xml exists, use it, don't do anything global, only do what the file stipulates
  2. If signconf.xml does not exist, sign every message with every key

@pflanze
Copy link
Owner

pflanze commented Jun 19, 2020

Working on it now, was being held up.

pflanze added a commit that referenced this issue Jun 19, 2020
@pflanze
Copy link
Owner

pflanze commented Jun 19, 2020

I've added support for multiple keys, see "issue1" branch, v0.3.8pre2 at this time, https://github.com/pflanze/better-qmail-remote/tree/issue1

I still haven't studied the links that you sent me, I need a break first. In case you already see that this solves your problem, let me know.

Note that I've updated to the current functional-perl libraries, so you'll need to run "git submodule update" in existing checkouts. Also it does have a "make test" functionality now, it would be interesting if that succeeds in your environment.

@pflanze
Copy link
Owner

pflanze commented Jun 20, 2020

I've now pushed v0.3.8pre3 which should support EdDSA keys, but the problem is now that current Mail::DKIM version 1.20200513.1 doesn't support EdDSA. You can see this in the Mail::DKIM sources:

lib/Mail/DKIM/ARC/Signer.pm: unless $self->{Algorithm} eq 'rsa-sha256'; # add ed25519 sometime

So we'll need to check with the Mail::DKIM author, or consider alternatives like Mail::OpenDKIM, although I can't see any hint in that latter module either about it supporting EdDSA keys. Any other ideas?

better-qmail-remote also supports configuration of the method now and defaults to relaxed/relaxed (see README.md)

@yitzhaq
Copy link
Author

yitzhaq commented Jun 20, 2020

Thanks a lot for your efforts! Although I must admit I didn't check first whether Mail::DKIM actually supports RFC 8463, I just went ahead and assumed it does, considering its recent updates.. D'oh! :|
I guess your approach should work as soon as support is added, though?

If you're interested in continuing to chase this, there is a third option, though the scope could be wider than simply wrapping a lib or issuing a simple command. Rspamd I know specifically does support EdDSA and dual-signing - to the point that examples for doing so are explicitly mentioned in its docs - and it does offer a signing only mode, which could potentially accomplish the desired effect. It might be possible to invoke rspamc to just sign a message and exit, or use it in LDA mode where it takes care of execing qmail-remote.orig after processing, though I haven't tried.

That said, Rspamd is a ridiculously powerful piece of kit, which I dearly miss in the qmail universe. "Proper" integration against it would be a huge win. It is specifically designed to also work well for outgoing mail, and has features - like ratelimiting - which could potentially make some of the other functionality of your script superfluous, or nicely complement it. The signconf.xml stuff could certainly be ripped out, as Rspamd supports everything it would, and much more.

Gaining access to Rspamd's rich feature set would be like an instant modern upgrade for any qmail server IMHO. While Rspamd might seem daunting at first, given everything it can do and the myriad ways it can do them, initial setup is actually surprisingly easy. Given its focus on shipping with sane defaults, and the very neat and tidy config system, it works very well out of the box, without any need for advanced tweaking. The config wizard guides you through the few things you should decide on before use, and DKIM signing - dual or otherwise - can be accomplished in just a few lines of config. Its selector system makes it easy to continue using global key(s) if you wish, or otherwise do just about anything else you may wish.

@pflanze
Copy link
Owner

pflanze commented Jun 20, 2020

Rspamd is a huge code base in C (much larger than Qmail itself, ~300k lines including libraries, ~100k lines just those in the src/ directory, plus another ~60k lines of Lua code) which makes me wary. Also I'm using qpsmtpd with spamassassin to handle spam. I've basically minimized the amount of C code that runs: the smtpd and spam checker are Perl (with -T ?), Mail::DKIM is directly using openssl with no other C code (other than hashing and maybe a couple others) and runs in the qmail-remote wrapper with -T. Using Rspamd would be quite the departure from that objective. Maybe there's still reason to do it, I'm not sure, but I'd prefer to wait and see first. It might not even be hard to extend Mail::DKIM, I've had a look, making it fully backwards compatible may pose some challenges (subclassing Mail::DKIM::{Private,Public}Key might do it, if a bit ugly), more testing, and being careful, that should be about it.

I'm happy to help you if you would like to tie Rspamd into it anyway yourself, but until I know the alternatives I'd like to avoid spending time on learning it and getting it to work correctly myself.

@yitzhaq
Copy link
Author

yitzhaq commented Jun 21, 2020

Yeah, if your goal is to avoid C, then Rspamd would certainly be a step in the opposite direction. And Rspamd is a large project, though with excellent performance and a great number of highly configurable features. I obviously agree it would be a much heavier dependency just for the DKIM signing - perhaps too heavy - I personally want it just as much for all the other features, if not even more so.

I'll see if I get around to trying to tie Rspamd into things. Until then, I guess we wait for the EdDSA support in Mail::DKIM (unless you get sufficiently inspired that a pull request turns up :o)

Thanks again!

@pflanze
Copy link
Owner

pflanze commented Jul 7, 2020

Mail::DKIM's author has responded to the issue (vaguely). I'm planning to write to the qmail mailing list in a day or two and see what other suggestions people have.

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

No branches or pull requests

2 participants