Email Stuffer flaw when creating DKIM signatures w/ multi-part emails

[SteveTheTechie]

This module has a flaw when trying to produce DKIM signatures w/ multi-part emails. The problem is that too many of the methods call $self->email, which resets the parts and recreates the boundary markers (new marker text). The former is not a problem with creating DKIM sigs, but the latter will cause DKIM verification failures if the boundary markers are recreated after the DKIM signature is created.

For example, consider the following steps:

1. Use Email::Stuffer->new to create the "stuffed" email.
2. Use ->as_string (which calls $self->email) to produce string version of email for Mail::DKIM::Signer
3. Use ->header() to insert the DKIM Signature. This does not change the email body.
4. Use ->send (which calls $self->email) to send the email.
   Because ->send calls $self->email, the boundary markers are changed which invalidates the DKIM signature just inserted. I keep getting "body altered" DKIM verification failures because of this.

Method $self->email is called too many times unnecessarily, particularly when the Email::MIME email object is cached in $self->{email} A better approach would be to use the cached object as much as possible, with certain methods "undefing" the cached object (cache invalidation) in {email} to force it to be recreated when needed. This would solve the DKIM verification problem for multi-part emails, and would make the module more performant.

[pali]

IMO that is probably design of Email::Stuffer and Email::MIME and their APIs. Basically calling any of set method on their objects invalidates any signature.

Probably the best would be to add DKIM support into Email::MIME (or Email::Simple?? as DKIM is not related to MIME). Email::MIME is layer which do any Unicode encoding/decoding, but DKIM is working on raw data. So also philosophically it problematic with Email::MIME.

Moreover, this request is for Email::Stuffer module which is another layer (on top of Email::MIME) therefore I would suggest to first solve how to handle DKIM in Email::MIME and later decide how to handle it in Email::Stuffer.

[SteveTheTechie]

Actually, I have working integrations between Mail::DKIM and Email::MIME and Email::Stuffer. My integrations handle signing over 300,000 emails a week, some of which are system generated and some of which come from user's email agents through our list server. Some of the code between the modules is obviously redundant, but I was happy to get something working for now until somebody else will take the leap into the bowels of Email::MIME ( or Email::Simple) I am moderately familiar enough with the Email::MIME code to know that I do not want to take that plunge.

I had to do a work-around w/ Email Stuffer that basically renders it just to be an email creator module. I would like to address that. I would be willing to do a PR w/ the understanding that my changes are "enabling changes" that provider for simpler integrations.

Improving the potential for integration is actually easy... it would only take 3 statement adds and 1 statement modifications in the Email::Stuffer module to simplify the integration. However, I do not want to invest any time if is just going to be dismissed out of hand.

[pali]

How to handle DKIM in this Email:: module architecture is question for maintainers, so for @rjbs.

If decision would be on me, I would probably design something like this:

• Create a new Email::something module which would take Email::Simple object (or any inherited) and add their headers DKIM signature.

• Add new decorator method into Email::Simple which would take name of another module which would provide specific functions/method and that decorator method would be responsible for doing needed thing. Then e.g. external module would be able to insert needed headers into Email::Simple object, without need to implement DKIM in Email::Simple (as I believe DKIM handling is too high-level for Email::Simple). Email::MIME should not have DKIM methods too as DKIM is not related to MIME.

• Patch Email::Stuffer to be compatible with above changes and add new method for Email::Stuffer which generate DKIM signature. For me it looks like Email::Stuffer is the only module which should directly have functions/methods for DKIM support, as it is the only high-level module for creating emails.

But I'm still not sure if this design is really good. It would provide unified interface for implementing modules which changes existing Email::Simple objects (or inherited) in some way, not only DKIM.

[rjbs]

I agree that Email::Stuffer makes this a pain. One possibility is to create a subclass that has a signed_email method and a sign_and_send method.

I'm not in a rush. It's easy to use the email method, sign the email, and send that.

[SteveTheTechie]

I agree that Email::Stuffer makes this a pain. One possibility is to create a subclass that has a signed_email method and a sign_and_send method.

I'm not in a rush. It's easy to use the email method, sign the email, and send that.

@rjbs As I note above, the Stuffer send calls the email method again which changes the boundary markers rendering the DKIM signature invalid. This is the crux of the problem with using Stuffer w/ DKIM, and why I suggested a a modification was needed.

Regardless of whether a subclass is done or not, the manner in which the boundary markers are created (more than once) by Stuffer is problematic.

[Frank071]

This is a rather old issue, but yet I have spent an evening bumping my head what happened... Anyhow, I found a rather simple solution patching the Email::Stuffer code. Instead of attaching the parts to the Email::MIME object every time the sub email is called, I changed the code so it does this only on the (each) first call after creation of the object or (possible) tampering with the parts. @Rjbs - if you are interested and still maintaining I can create a patch.

[rjbs]

@Frank071 That sounds good. I believe what you're saying is: the Email::MIME object is cached once it's generated, and that cache is cleared when (and only when) some property of the email is changed that would change the email object in a substantive way.

[Frank071]

@rjbs that is correct - I introduced a helper value in $self that is set when the email is built and reset whenever content is attached. However I can't create a proper patch since the version I obtained from cpan seems different from the one on Github. It contains a line "$Email::Stuffer::VERSION = '0.018';" at the beginning and all pod-lines are starting with #pod ...?