gen-smtp / gen_smtp

The extensible Erlang SMTP client and server library.
Other
683 stars 266 forks source link

DKIM with Ed25519 key #275

Closed cw789 closed 3 years ago

cw789 commented 3 years ago

Do I understand the code correctly that DKIM currently only will work with rsa-sha256? What need to be done to make it compatible with ed25519?

Would I need to enable passing the DKIM tag a?

seriyps commented 3 years ago

Hi. Yes, it currently only supports rsa-sha256 (it is hardcoded):

https://github.com/gen-smtp/gen_smtp/blob/dd7a0008f8466f1e3d0b8104e5d51344d1c772dc/src/mimemail.erl#L1143

https://github.com/gen-smtp/gen_smtp/blob/dd7a0008f8466f1e3d0b8104e5d51344d1c772dc/src/mimemail.erl#L1093

Is there some RFC mentioning ed25519? i see https://datatracker.ietf.org/doc/html/rfc6376/#section-3.3 only mentions sha-256 and sha1..

Anyway, it should not be difficult to add support for other algorithms, just make a option configurable via mimemail:dkim_options()

seriyps commented 3 years ago

I guess this RFC? https://datatracker.ietf.org/doc/html/rfc8463

seriyps commented 3 years ago

Also, Erlang's crypto mentions this algorithm https://erlang.org/doc/man/crypto.html#type-ec_named_curve So, I guess it's indeed should be quite easy to add.

cw789 commented 3 years ago

RFC 8463 right. Ok. This will then be OTP ≥ 21.x. Should I go for it?

seriyps commented 3 years ago

yep, if you would like to make a PR, I would suggest introducing {a, 'rsa-sha256' | 'ed25519'} option (so, atom values spelled the same way as in RFCs) with default being rsa-sha256' and then map this atom to the appropriate atom option in OTP crypto. And either have another mapping from atom to binary value of a=XXXX header or just call atom_to_binary/2. You can use -ifdef(OTP_RELEASE). to detect OTP-21+ (this macro have been introduced in OTP-21).

cw789 commented 3 years ago

Ok. I'll try it (first time in Erlang). But I think it should be {a, 'rsa-sha256' | 'ed25519-sha256'}?

seriyps commented 3 years ago

yes, you are right, {a, 'rsa-sha256' | 'ed25519-sha256'}!

cw789 commented 3 years ago

Close in favour of pull request.