Redirect mail (Resend/redirect/bounce) rfc5322 #1739

the0ne · 2016-10-20T20:36:01Z

this is a working proposal for the implementation of rfc5322

philipwhiuk

Found a few errors.

Some level of automated testing would be nice to see.

philipwhiuk · 2016-10-20T21:00:10Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

+                DateTimeField field = (DateTimeField)DefaultFieldParser.parse("Resent-Date: "
+                        + MimeUtility.unfoldAndDecode(getFirstHeader("Resent-Date")));
+                mResentDate = field.getDate();
+            } catch (Exception e) {


Empty catch block

the function tries to get the Resent-Date header from the headers list.
if there aint one or an error happens, then the internal one will be returned.
it's a perfect clone of the existing function getSentDate()
`

@Override public Date getSentDate() { if (mSentDate == null) { try { DateTimeField field = (DateTimeField)DefaultFieldParser.parse("Date: " + MimeUtility.unfoldAndDecode(getFirstHeader("Date"))); mSentDate = field.getDate(); } catch (Exception e) { } } return mSentDate; }

`

philipwhiuk · 2016-10-20T21:00:53Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

+                if (mResentTo == null) {
+                    mResentTo = Address.parse(MimeUtility.unfold(getFirstHeader("Resent-To")));
+                }
+                return mTo;


I think this is an error.

philipwhiuk · 2016-10-20T21:01:46Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

+                if (mResentCc == null) {
+                    mResentCc = Address.parse(MimeUtility.unfold(getFirstHeader("Resent-CC")));
+                }
+                return mCc;


philipwhiuk · 2016-10-20T21:01:55Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

+                if (mResentBcc == null) {
+                    mResentBcc = Address.parse(MimeUtility.unfold(getFirstHeader("Resent-BCC")));
+                }
+                return mBcc;


the0ne · 2016-10-21T04:07:36Z

thanks for the review, i have fixed the errors!
i agree that automated testing would be nice.
i'll be off a few days - i possibly find the time in a week or so.

Valodim

Remailing is a nice advanced feature. I wonder though if it is problematic with modern anti-spam techniques, since it essentially spoofs the sender address, doesn't it?

Possibly relevant: https://wiki.list.org/DEV/DMARC

I like the feature idea, but we should have a clear picture of these considerations before we merge this

Valodim · 2016-10-26T19:24:21Z

k9mail/src/main/java/com/fsck/k9/mailstore/LocalMessage.java

@@ -509,6 +488,17 @@ public LocalMessage clone() {
        return message;
    }

+    public MimeMessage cloneAsSuper( boolean clearUid) {


Code style issues

Valodim · 2016-10-26T19:29:23Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MessageExtractor.java

@@ -142,7 +144,10 @@ public static void findViewablesAndAttachments(Part part,
        }

        Body body = part.getBody();
-        if (body instanceof Multipart) {
+        if (body instanceof BinaryMemoryBody) {


What's the purpose of this? Seems like a hotfix to fix a very specific problem, which will have deep implications on content extraction behavior

the diff here is shown misleading - please check the diff in MessageExtractor.java manually.
i had just added handling of BinaryMemoryBody for the specific case of redirecting.
i have reverted the change with a later commit.

Valodim · 2016-10-26T19:38:23Z

k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java

@@ -159,6 +160,7 @@
     */
    private Account mAccount;

+    private com.fsck.k9.mail.Body mRedirectBody = null;


no point fully qualifying this type

Valodim · 2016-10-26T19:42:08Z

k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java

@@ -1489,12 +1533,28 @@ public void onMessageBuildSuccess(MimeMessage message, boolean isDraft) {
            }
        } else {
            currentMessageBuilder = null;
+
+            if (mAction == Action.REDIRECT) {


This piece of logic is misplaced here, redirected messages should also be built by a MessageBuilder

the whole point of redirecting a message is to not modify the headers at all - except for adding resent-xxx headers.

It doesn't matter what should or shouldn't be done to the message, there should be no logic that changes (or creates) the message here. that's the MessageBuilder's job

i have added a new builder for resending messages - please review

Valodim · 2016-10-26T19:46:23Z

k9mail-library/src/main/java/com/fsck/k9/mail/internet/MimeMessage.java

+        setRecipients(type, addresses, true);
+    }
+
+    public void setRecipients(RecipientType type, Address[] addresses, boolean setHeader) {


the setHeader paramter allows desync of fields from the rfc2045 headers, which seems contrary to the purpose of MimeMessage

the whole code had been nearly duplicated for localmessage vs. mimemessage - with the exception of calling setHeader().
by adding the parameter now localmessage can use the same functionality without duplicating it, just by giving the setHeader parameter.

Valodim · 2016-10-26T19:51:18Z

k9mail/src/main/java/com/fsck/k9/activity/MessageCompose.java

+                LocalMessage referenceMessage = mMessageReference.restoreToLocalMessage(getApplicationContext());
+                message = referenceMessage.cloneAsSuper(true);
+                message.setResentDate(new Date(), K9.hideTimeZone());
+                message.setRecipients(RecipientType.RESENT_TO, toArray(recipientPresenter.getToAddresses()));


this sets the resent-to header, but does it influence who the mail is actually sent to by way of the smtp-transport?

thats what the whole thing is about.
of cause k-9 only has to support the client part of adding the headers.

The question was whether the RCPT in the smtp transport end up with the correct values.

i haven't found any hint about the RCPT being wrong - nor about it would matter at all when using resent-headers.
do you have a server where you could test the behaviour?

Maybe I'm just missing something here. Just to help me understand, at the end of this block of code the message that is queued for sending will be the originally referenced one, except with added Resent- headers, right? But the addresses where the message will be actually delivered are the addresses in To, Cc and Bcc (see SmtpTransport), so doesn't that mean it will be sent to the same recipients as original one, rather than the ones they should be redirected to?

K-9 opens the connection to the mail server of the identity.
All headers (and later the body) are sent to the server.
If the server finds Resent-xxx headers then those will be used instead of the adresses in To, Cc, Bcc.
Not to the ones in To, Cc, Bcc.
Only to those in Resent-To, Resent-Cc, Resent-Bcc.

Example: Let's assume you send a message to my email1 and my email2. And i redirect from email2 to email3. Then you, email1 and email2 will not get the message again. It will only be sent to email3 and email3 will only see you as the sender and email2 and email3 as recipients.

That is by design and exactly what many people other than me seem to require as well =)

i retested with an external server and found some additional changes were necessary to the code if we want to forward the message to a server other than our own.
committed as 50cf537..05cf210

the0ne · 2016-11-03T12:45:10Z

@Valodim & @philipwhiuk - any idea why the SmtpTransportTest unit tests fail?

Valodim · 2016-11-03T12:50:04Z

Run them locally, and check out the html report file mentioned in the output, that should point you to the failing assertion.

I'm also not really convinced yet that remailing is a use case that is still fine with how current mail transport networks work. Check out the DMARC FAQ, and maybe here and the wiki link above.

Valodim · 2016-11-03T13:04:14Z

I'd like to reassert that the DMARC problem mentioned above is a showstopper here. Without a good answer that clears up this problem, we won't be able to merge this PR, I suggest you look into this before investing more work

the0ne · 2016-11-03T13:05:48Z

Travis did test against latest whereas my source was on the branched version.
After pulling from upstream i could fix the unit test.

neufeind · 2016-11-03T13:11:02Z

In some cases bouncing a message can interfer with antispam-techniques when used "publically" - as your server will send with a "forged" sender (the original one). One possible way would imho be to at least set the smtp-envelope correctly to the re-sender but keep the mail itself as is.

We use bouncing internally, locally on the same mailserver and with smtp-auth. In that scenario you can relax antispam-measurements for authenticated users and bouncing will work fine. Having bounce-functionality would be totally awesome.

the0ne · 2016-11-03T13:51:51Z

@neufeind absolutely, i agree about the smtp-envelope.
Return-Path is now set accordingly.

Valodim · 2016-11-03T13:57:15Z

Um, I think I'd like to see more of a rationale here than "might work in a special scenario or two, let's roll!"

neufeind · 2016-11-03T13:58:32Z

imho it's more a "might fail for certain people in certain scenarios" ... as will SPF when using normal email-forwarding :-)

Valodim · 2016-11-03T14:03:54Z

How about "might fail in the gmail case, as pointed out in a link above, and which is half of the email user base"? Ignoring this does not fill me with confidence that this has been thought through as much as it needs to be.

And just to be clear, spf in some cases breaks "forwarding" as in the thing recipients set up ("forward incoming mail to other address" on a mail server), not as in the thing that happens when you click "forward" in an email client. Not sure how this is relevant in a discussion about clients though.

the0ne · 2016-11-03T14:43:06Z

With the latest commit 7e4f5d7 resending to Gmail addresses definitely works as expected.

the0ne · 2016-11-04T10:12:41Z

@philipwhiuk
i have added the requested unit test.
and i have added the two missing spaces in SmtpTransport.java

cketti

Please clean up this branch so it only contains relevant commits.

the0ne · 2016-12-13T08:56:30Z

sure. please let me know the details.

philipwhiuk · 2016-12-13T10:35:28Z

One way is:

Pull from upstream and make sure your master branch is the same as K-9's
Checkout the redirect_mail branch
Generate a diff against master (e.g. git diff master --no-ext-diff > ~/redirectMail.diff
Hard reset redirect_mail to master (git reset --hard master)
Apply the diff (git apply ~/redirectMail.diff)
Do a commit (git commit -a -m "Redirect mail")
Force push the branch (git push -f origin redirect_mail)

the0ne · 2016-12-13T15:38:19Z

please let me know if this is what you requested.

Valodim · 2016-12-13T15:51:00Z

Looking good! Thanks so far 👍

the0ne · 2016-12-14T15:17:02Z

@philipwhiuk Thanks! 😃
Please also review #1736

the0ne · 2016-12-29T12:53:00Z

@cketti The branch has been cleaned up as requested - please review.

cketti · 2017-01-01T01:09:34Z

Thanks for cleaning up the branch which made it easier to see the changes.
However, I've thought some more about this feature and decided that it's not functionality I want to add to the app at this point in time.

ArchangeGabriel · 2017-01-01T01:11:13Z

@cketti Do you mind sharing your reasons?

cketti · 2017-01-01T01:24:09Z

It's functionality that is not commonly used/supported and has the potential to confuse non-expert users. Also, I believe clients should simply not be able to "resend" a message.
All use cases can be handled by "forward as attachment". That makes a very clear distinction between original message and the forwarding operation. Also, it doesn't mix original headers with ones for the resend operation.
Sure, it might require additional work on the receiving side to extract the original message. But that's not something we should "fix" on the client side.

Valodim · 2017-01-01T01:27:00Z

There's also the DKIM/SPF/DMARC troubles. SMTP servers can't be assumed to deliver messages if they have From addresses your sending server isn't authorized for, and these policies are getting more and more strict these days.

ArchangeGabriel · 2017-01-01T01:39:03Z

OK, make sense. Did K-9 gain support for attached email recently (both opening and forwarding as such)? Last time I checked that wasn’t working.

cketti · 2017-01-01T01:41:45Z

Support for displaying message/rfc822 parts needs to get better. For forwarding messages as attachment there's PR #1728.

the0ne · 2017-01-02T18:13:11Z

I think having the forwarding functionality off by default and having to enable it in settings would be a better option.
More people than you would believe are waiting for this.
Many power users are utilizing this well-documented technique without problems for years.

darkdragon-001 · 2017-01-02T19:17:37Z

I agree with @the0ne. I am watching this issue for quite some time and can't wait to see this implemented.

cketti · 2017-01-02T19:19:57Z

We've done that in the past. But adding all kinds of functionality with a setting to turn it on/off doesn't scale. The maintenance cost is real and part of what slows down development of the app.

the0ne · 2017-12-29T15:57:37Z

@neufeind and @darkdragon-001 feel free to build
https://github.com/the0ne/k-9/tree/5.4-MAINT which has the following added functionality:

supports sending messages as attachments
supports remotely learning messages as spam or ham
supports redirecting messages using "Resent-..." headers

I've been using these since a year now and the additions work pretty well.
Upgraded to 5.4 just now, builds and runs on my devices.

darkdragon-001 · 2018-01-08T12:28:01Z

@the0ne please enable issues on your fork. While I can build the original 5.4-MAINT branch from this project via tools/debian_build.sh, this is not possible for your fork. I tried to fix some dependencies but did not succeed...

heinlein-support · 2019-01-12T15:05:20Z

I'd like to re-open that discussion since I also requested that feature in 2017 and I miss it every day (yes, every day!).

Not every use case could be handled by forwarding a mail as an attachment. I often redirect mails from customers to employes or into ticket systems and there it's very important to keep the original from/sender address to send ticket notifications to the right people. I also prefer having mails from customers redirected to me, because then I'm able to search directly for a customer's mail instead of having thousands of mails that "seem" to come from my team and where the original sender isn't directly visible.

Also DKIM/DMARC isn't really a problem here, since also redirected mails could get a new DKIM signature and/or the original DKIM signatur could/should stay in that mail. At the end, redirecting a mail is nothing else then redirecting mails with a virtual table on Postfix SMTP servers (which is a standard feature there for >15 years and working). SPF /could/ be a problem, but SPF is broken in general (so don't blame redirecting mails for problems if all kinds of forwardings do have problems with SPF) and also redirecting mails SHOULD change the SMTP envelope sender address (which is mostly matched against SPF) -- while the header sender/from isn't changed.

Last but not least: The Redirect Plugin in Thunderbird is working perfectly for years now, but since K9 doesn't have this feature, I have to mark mails in K9 during the day to redirect them in thunderbird later when I'm back in the my office or back at home. I'm really missing that feature and there's no way to substitute it in a good way.

the0ne · 2019-01-13T16:33:34Z

@heinlein-support feel free to use k9mail-debug-5.600.apk.zip which includes the above implementation based on k9mail 5.600.

It intentionally is the debug version so you can install and use it parallel to your regular k9mail if you want. Personally I did uninstall the regular k9mail on all of my devices quite a while ago as I feel same as you:
Without "redirect" k9mail is not half as valuable as it could be.

I heavily use the redirect functionality and it works as well as the Thunderbird Redirect plugin.

heinlein-support · 2019-01-13T19:54:29Z

Hi the0ne,
thanks for the perfect link, I'll try it. Anyway, it's always better to have it in the main version to get all updates automatically...
Thanks,
Peer

heinlein-support · 2019-03-01T07:27:33Z

No chance to get this great and unproblematic feature into K9?

basti122303 · 2022-09-09T11:58:25Z

Any News? It would be great to Add this.

HeikoSchlittermann · 2022-09-09T12:42:04Z

I'd like to see this in K9. Yes, the use cases are limited (spoofed sender). But bouncing a message from e.g. my personal company account to my company's ticket system is expected to work (depending on the setup of my company's mail system).

This was referenced Oct 20, 2016

forward a message as attachment #1728

Closed

Resend/redirect/bounce mail #622

Closed

philipwhiuk suggested changes Oct 20, 2016

View reviewed changes

Valodim reviewed Oct 26, 2016

View reviewed changes

cketti requested changes Dec 12, 2016

View reviewed changes

cketti added the status: needs work Issues that are pending further action or development label Dec 12, 2016

the0ne force-pushed the redirect_mail branch from 3b992a9 to c51c1e8 Compare December 13, 2016 15:35

Redirect mail

c51c1e8

Redirect mail

9be66b7

philipwhiuk approved these changes Dec 14, 2016

View reviewed changes

cketti closed this Jan 1, 2017

Redirect mail (Resend/redirect/bounce) rfc5322 #1739

Redirect mail (Resend/redirect/bounce) rfc5322 #1739

Conversation

the0ne commented Oct 20, 2016

philipwhiuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the0ne commented Oct 21, 2016

Valodim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the0ne Oct 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the0ne Oct 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the0ne Oct 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the0ne Oct 27, 2016 • edited

Choose a reason for hiding this comment

the0ne Oct 28, 2016 • edited

Choose a reason for hiding this comment

the0ne commented Nov 3, 2016

Valodim commented Nov 3, 2016 • edited

Valodim commented Nov 3, 2016

the0ne commented Nov 3, 2016 • edited

neufeind commented Nov 3, 2016

the0ne commented Nov 3, 2016 • edited

Valodim commented Nov 3, 2016

neufeind commented Nov 3, 2016

Valodim commented Nov 3, 2016

the0ne commented Nov 3, 2016

the0ne commented Nov 4, 2016 • edited

cketti left a comment

Choose a reason for hiding this comment

the0ne commented Dec 13, 2016 • edited

philipwhiuk commented Dec 13, 2016 • edited

the0ne commented Dec 13, 2016

Valodim commented Dec 13, 2016

the0ne commented Dec 14, 2016

the0ne commented Dec 29, 2016

cketti commented Jan 1, 2017

ArchangeGabriel commented Jan 1, 2017

cketti commented Jan 1, 2017

Valodim commented Jan 1, 2017 • edited

ArchangeGabriel commented Jan 1, 2017

cketti commented Jan 1, 2017

the0ne commented Jan 2, 2017

darkdragon-001 commented Jan 2, 2017

cketti commented Jan 2, 2017

the0ne commented Dec 29, 2017

darkdragon-001 commented Jan 8, 2018

heinlein-support commented Jan 12, 2019

the0ne commented Jan 13, 2019

heinlein-support commented Jan 13, 2019

heinlein-support commented Mar 1, 2019

basti122303 commented Sep 9, 2022

HeikoSchlittermann commented Sep 9, 2022

the0ne Oct 27, 2016 •

edited

the0ne Oct 27, 2016 •

edited

the0ne Oct 27, 2016 •

edited

the0ne Oct 27, 2016 •

edited

the0ne Oct 28, 2016 •

edited

Valodim commented Nov 3, 2016 •

edited

the0ne commented Nov 3, 2016 •

edited

the0ne commented Nov 3, 2016 •

edited

the0ne commented Nov 4, 2016 •

edited

the0ne commented Dec 13, 2016 •

edited

philipwhiuk commented Dec 13, 2016 •

edited

Valodim commented Jan 1, 2017 •

edited