Why delivery_handler, not interceptor?

[duckdalbe]

Is there a specific reason that mail-gpg has been implemented as a delivery_handler, instead of an interceptor?

The latter seems more appropriate to me. And it would also enable to chain it with other custom code that modifies messages, while with the delivery_handler I must implement my own code as delivery_handler that wraps mail-gpg.

Would it be an option to maybe rebuild mail-gpg as an interceptor?

Background: I want to handle specific headers in a certain way (remove them from the encrypted message) in order to protect headers as suggested by https://github.com/autocrypt/memoryhole

[jkraemer]

Actually I have a long-standing item on my todo list that says "look into action mailer interceptors". So if you want to give it a try, I'd be happy to get that done.

[duckdalbe]

Ok, I'll try.

[duckdalbe]

I have some working code now, but there is one big difference in behaviour of which I'm not sure if you're OK with: As an interceptor, mail-gpg must work on the very object, not a copy of it. That means, delievering a Mail::Message-object changes it:

mail = Mail.new { subject 'test'; body 'blabla'; from: '[email protected]'; to '[email protected]' }
mail.gpg encrypt: true
puts mail.body.to_s
=> "blabla"
mail.deliver
puts mail.body.to_s
=> "----- BEGIN PGP MESSAGE -----\n ..."

People who use mail-gpg in loops, e.g. to send a message to multiple recipients, would run into problems, because the lose the original message.

Working around that is simple, you e.g. only have to run mail.dup.deliver. But the behvaiour is rather surprising, I think.

On the other hand, mail-gpg changes some state during encryption already: it deletes :encrypt from the gpg-options (By the way: why's that?).

On the positive side: as an interceptor, mail-gpg doesn't have to copy headers.

What do you think about this?

[duckdalbe]

ping.

[jkraemer]

Sorry for the late reply.

In general I think if we made it clear in the changelog that the message object will be modified and make it a new major version such change in behavior would be ok. However there's a couple more things to consider.

I just found a (quite old) blog post which outlines another problem:

using deliver! [instead of deliver] will call any registered Mail Observers, but not Interceptors - meaning that your mail will be sent unaltered.

https://joshmcarthur.com/howto/2011/10/06/action-mailer-interceptors.html

I am not sure whats the reasoning behind this or if it maybe was just a bug in Rails that's already fixed now, but surely something to keep in mind. I wouldn't want to have such an inconsistency in behavior.

Another change for users will be that they will have to register the interceptor manually because the order multiple interceptors are registered in will matter. Will give the user more control (i.e. only enable it in production...) though.

Regarding the Memory Hole project - isn't what they advertise (putting the headers into the encrypted part) exactly what mail-gpg already does? All that is left to do in order to fully implement this would be formalizing a way to define what headers are sensitive and should be moved into the signed/encrypted part, and which probably should just be copied. The latter might make sense when signing only - leaving headers in the main message will help with clients unaware of the memory hole standard.

That would give you a way to move sensitive headers into the encrypted part, and it might also help with cases like #53 where people explicitly want no headers in there.

The header copying has been a problematic area of mail-gpg all the time, moving from a hard-coded list that had to be maintained to 'encrypt them all' which then made users ask why that is the case. I think implementing a flexible way that probably not only supports header names but also regexps to catch i.e. all X-Redmine-.+ Headers would be a good thing regardless of the Interceptor / DeliveryHandler issue.

What do you think?

[duckdalbe]

Alright, so I'll clean up the interceptor-code.

Regarding the other topics:

• Since 2012-01-24, deliver! calls interceptors (released in mail v2.4.2)
• I would opt to register the Interceptor on inclusion of mail-gpg, and document how to un-register and re-register it in case of the need.
• Memoryhole wants the headers moved in some circumstances, yes. For encryption it's only a little bit more than mail-gpg currently does. For decryption it would raise the question how to communicate to the surrounding code, which headers had been protected (please do not mix signed+encrypted headers with unsigned+cleartext headers  #45). If you're interested, we could tackle that and implement a memoryhole-option in mail-gpg, too! But's (for decryption) it would be more work than just to change a few lines of code, I'm afraid.
• Regardless of memoryhole, I think it would be good to allow surrounding code to influence, which headers will be copied.

[geor-g]

@jkraemer Ping ... :)

[duckdalbe]

@ge-fa I think the ball is on my side. It's only that my pressing problem with this has vanished and I can't find the time to do this nonetheless.

It's still on my agenda, though.

[mkamensky]

The gem does not currently work with ActionMailer, as far as I can tell, since it assigns itself as the delivery_handler, and so Mail::Gpg::DeliveryHandler is not assigned. If I understand correctly, using the interceptor approach should solve it