Bug 331991 - GnuPG signatures broken by "Pipe Through" filter action
Summary: GnuPG signatures broken by "Pipe Through" filter action
Status: RESOLVED FIXED
Alias: None
Product: Akonadi
Classification: Frameworks and Libraries
Component: Mail Filter Agent (show other bugs)
Version: 4.12
Platform: Ubuntu Linux
: NOR major
Target Milestone: ---
Assignee: Sandro Knauß
URL:
Keywords:
: 322932 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-03-10 19:33 UTC by xor
Modified: 2015-03-15 20:02 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.16.7
Sentry Crash Report:


Attachments
Attached is a sample mail which a valid signature which will be damaged when using "Pipe Through". (4.55 KB, application/octet-stream)
2014-03-10 19:34 UTC, xor
Details

Note You need to log in before you can comment on or make changes to this bug.
Description xor 2014-03-10 19:33:47 UTC
KMail modifies the mail on its when using "pipe through" filter action, even though the only thing modifying it should be the software through which the mail is piped.

This causes the GnuPG signed part of mails to be modified in a way which makes the signature invalid.

Reproducible: Always

Steps to Reproduce:
Used Ubuntu (lsb_release -a): Kubuntu 13.10
Architecture (uname -a): x86_64
Used kmail (dpkg --status): Version: 4:4.11.5-0ubuntu0.1

Steps to reproduce:
- Generate a bogofilter spam filter using KMail's own wizard

- This will add a filter called "Bogofilter Check" which uses the "pipe through" action to pipe the mail through "bogofilter -p -e"

- Now have someone send you a GnuPG signed mail (using OpenPGP/MIME, i.e. which contains a text part which begins similar to this one (will try to upload an attachment which contains a whole sample message):
'--nextPart4479402.yrgGCOTjb2
Content-Type: Text/Plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit'
Actual Results:  
- After the mail has been piped through the filter, the text part will be mangled to this:
'--nextPart4479402.yrgGCOTjb2
Content-Type: text/plain; charset="iso-8859-15"
Content-Transfer-Encoding: 7bit'

- As the header of the text part is part of the GnuPG signature, the signature is broken now! There should not be ANY modifications on it for GPG signature validation to succeed.

- If you pipe the same mail manually through "bogofilter -p -e" on the terminal, and diff the before and after, you will notice that this does NOT happen. This means that KMail is the one to blame for mangling the header of the text part. In fact, it mangles a lot of more stuff about the mail. Check the diff. 

Expected Results:  
The header of the text part should not be modified by KMail.
IMHO, when doing "pipe through", KMail itself should not do ANY modifications on the mail at all.
Comment 1 xor 2014-03-10 19:34:48 UTC
Created attachment 85516 [details]
Attached is a sample mail which a valid signature which will be damaged when using "Pipe Through".

Notice the user agent of the mail: It's KMail, so KMail even damages its own mails.
Comment 2 xor 2014-03-10 19:35:34 UTC
Downstream bug report at Ubuntu: https://bugs.launchpad.net/ubuntu/+source/kdepim/+bug/1290506
Comment 3 Sandro Knauß 2014-03-28 22:49:22 UTC
I can confirm with kmail 4.13rc.
Comment 4 Sandro Knauß 2014-03-30 17:27:01 UTC
It can be tracked down to the mark as action (mailcommon/filter/filteractionsetstatus.cpp).

So all mark as Ham, Spam or Important via Filter are affected.

The bug lies in the mailmodifyjob, that modifies the mail in a way that it breaks:
agents/mailfilteragent/filtermanager.cpp (line 467):
      if ( context.needsPayloadStore() || context.needsFlagStore() ) {
          Akonadi::Item item = context.item();
          //the item might be in a new collection with a different remote id, so don't try to force on it
          //the previous remote id. Example: move to another collection on another resource => new remoteId, but our context.item()
          //remoteid still holds the old one. Without clearing it, we try to enforce that on the new location, which is
          //anything but good (and the server replies with "NO Only resources can modify remote identifiers"
          item.setRemoteId(QString());
          Akonadi::ItemModifyJob *modifyJob = new Akonadi::ItemModifyJob( item, this );
          modifyJob->disableRevisionCheck(); //no conflict handling for mails as no other process could change the mail body and we don't care about flag conflicts
          //The below is a safety check to ignore modifying payloads if it was not requested,
          //as in that case we might change the payload to an invalid one
          modifyJob->setIgnorePayload( !context.needsFullPayload() );
          connect( modifyJob, SIGNAL(result(KJob*)), SLOT(modifyJobResult(KJob*)));
      }


Using the Menu (Message->Mark Message) the signatuer is not getting broken.
Comment 5 xor 2014-03-31 09:12:46 UTC
It is also important to mention that this NOT only applies to pressing the buttons. It also happens upon the *automatic* filtering at arrival of the mail.

So effectively, this makes spam detection unusable for people who need GnuPG, or vice versa.
Comment 6 xor 2014-05-17 18:56:21 UTC
Given that we live in the age of the NSA scandal, I would be really glad if you could fix this soon. 

As developers, I think we can all agree tahat the whole open source world relies upon GPG for mail signatures, and it is really annoying to get lots of broken signatures on development mailing lists. I ought to NOT trust mails of other developers with broken signatures - how am I supposed to work like that? :(
Comment 7 xor 2014-08-26 11:45:39 UTC
Still happens with kmail Version: 4:4.13.3-0ubuntu0.1
Comment 8 xor 2014-10-03 22:04:59 UTC
I am now offering a bounty of $ 20, paid in Bitcoin, to whoever gets a fix merged. 
To claim the bounty:
- Use signed Git commits (git commit --gpg-sign) on the fixes.
- Post an URL which shows that the commits are merged into the main repository.
- Post your Bitcoin address GPG-signed with the same key as the commits.

Thank you.
Comment 9 Sandro Knauß 2014-10-04 16:11:17 UTC
Hey,

well I just answer you, 'cause I'm the dev that was/is fixing gpg bugs in 
kmail:) I would have fixed it in the past, if I had time to fix it... but as I 
wrote before, the problem live in the modifyitemjobs. Till november I won't 
find time to fix this bug but after I can focus again at upstream bugs :)

Regads,

sandro

--

Am Freitag, 3. Oktober 2014, 22:04:59 schrieben Sie:
> https://bugs.kde.org/show_bug.cgi?id=331991
> 
> --- Comment #8 from xor <xor@gmx.li> ---
> I am now offering a bounty of $ 20, paid in Bitcoin, to whoever gets a fix
> merged.
> To claim the bounty:
> - Use signed Git commits (git commit --gpg-sign) on the fixes.
> - Post an URL which shows that the commits are merged into the main
> repository. - Post your Bitcoin address GPG-signed with the same key as the
> commits.
> 
> Thank you.
Comment 10 xor 2014-12-01 04:14:58 UTC
I am rasing the bounty to fix this from $ 20 to $ 30, paid in Bitcoin, to whoever gets a fix merged. To claim the bounty:
- Use signed Git commits (git commit --gpg-sign) on the fixes.
- Post an URL which shows that the commits are merged into the main repository.
- Post your Bitcoin address GPG-signed with the same key as the commits. Thank you.
Comment 11 Sandro Knauß 2015-03-14 20:40:48 UTC
I have now two review requests, that fixes the problem together:
https://git.reviewboard.kde.org/r/122961
https://git.reviewboard.kde.org/r/122933

Btw. the key of the mail is expired in meanwhile:

GpgME::VerificationResult(
 error:      GpgME::Error(0 (Erfolg))
 fileName:   
 signatures:
GpgME::Signature(
 Summary:                   GpgME::Signature::Summary(KeyExpired )
 Fingerprint:               285BD3152424E17EC4E2FB8A61435B7375941D88
 Status:                    GpgME::Error(117440665 (Schlüssel abgelaufen))
 creationTime:              1347385792
 expirationTime:            0
 isWrongKeyUsage:           0
 isVerifiedUsingChainModel: 0
 pkaStatus:                 GpgME::Signature::PKAStatus()
 pkaAddress:                <null>
 validity:                  ?
 nonValidityReason:         GpgME::Error(0 (Erfolg))
 publicKeyAlgorithm:        DSA
 hashAlgorithm:             SHA256
 policyURL:                 <null>
 notations:
)
Comment 12 Sandro Knauß 2015-03-15 12:59:19 UTC
Git commit 02fc4cfd8f3e976a26d7748fd612fce398051b4c by Sandro Knauß.
Committed on 14/03/2015 at 20:14.
Pushed by knauss into branch 'KDE/4.14'.

Do not change mails, if use pipe through filter.

For signed mails the body of the mail has not to change not even slightly.
FIXED-IN: 4.16.7
REVIEW: 122961

M  +17   -0    mailcommon/filter/autotests/CMakeLists.txt
A  +343  -0    mailcommon/filter/autotests/actionpipethrough.cpp     [License: LGPL (v2+)]
A  +47   -0    mailcommon/filter/autotests/actionpipethrough.h     [License: LGPL (v2+)]
M  +9    -3    mailcommon/filter/filteractions/filteractionwithcommand.cpp

http://commits.kde.org/kdepim/02fc4cfd8f3e976a26d7748fd612fce398051b4c
Comment 13 Sandro Knauß 2015-03-15 20:02:30 UTC
*** Bug 322932 has been marked as a duplicate of this bug. ***