After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 564465 - Wrong signature marked as valid on modified mail messages
Wrong signature marked as valid on modified mail messages
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: Mailer
2.22.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
: 576734 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-12-14 10:59 UTC by Yves-Alexis Perez
Modified: 2010-03-25 09:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
CA for the signatures (1.84 KB, text/plain)
2008-12-14 11:00 UTC, Yves-Alexis Perez
  Details
S/MIME signed mail (invalid in openssl, valid in evolution) (2.26 KB, text/plain)
2008-12-14 11:01 UTC, Yves-Alexis Perez
  Details
Proposed patch (1.59 KB, patch)
2009-02-16 20:20 UTC, Matthew Barnes
committed Details | Review
fix for a break? (600 bytes, patch)
2009-03-04 15:16 UTC, Milan Crha
committed Details | Review
Fix, backported to 2.24.5 (3.71 KB, patch)
2009-03-07 20:15 UTC, Daniel Gryniewicz
reviewed Details | Review
Fix backported patch to 2.24.5 (3.88 KB, patch)
2009-03-08 15:05 UTC, Yves-Alexis Perez
reviewed Details | Review
proposed eds patch (1.03 KB, patch)
2009-03-31 11:51 UTC, Milan Crha
committed Details | Review

Description Yves-Alexis Perez 2008-12-14 10:59:36 UTC
Please describe the problem:
Hi,

a Debian user reported (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508479) that in some cases, a S/MIME signature could be marked as valid while it is not (the message is a modified version of a correctly signed one).

Openssl smime shows it as invalid, while Evolution marks it a ok.

I'm not sure it can be exploited yet.

Steps to reproduce:
1. Import the CA in the certificates store
2. Import the mail in an evolution folder



Actual results:
Signature is marked as valid

Expected results:
The signature is wrong (as confirmed by openssl smime -CAfile key.pem -in mail.txt -verify

Does this happen every time?
Yes

Other information:
Downstream bug report is available at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508479 and I'll add the attachments.
Comment 1 Yves-Alexis Perez 2008-12-14 11:00:45 UTC
Created attachment 124634 [details]
CA for the signatures
Comment 2 Yves-Alexis Perez 2008-12-14 11:01:36 UTC
Created attachment 124635 [details]
S/MIME signed mail (invalid in openssl, valid in evolution)
Comment 3 Matthew Barnes 2009-02-10 22:49:15 UTC
Trying to follow the logic in camel-smime-context.c, I -think- what's happening is we're just verifying the signature, which is unchanged and still good.  But we're not doing a data integrity check (i.e. running a checksum on the signed message and comparing it with the signature), so we never notice that the message has been altered.
Comment 4 Robert Buchholz 2009-02-13 17:52:23 UTC
CVE-2009-0547 has been assigned to this vulnerability.
Comment 5 Matthew Barnes 2009-02-16 20:20:23 UTC
Created attachment 128857 [details] [review]
Proposed patch

Patch written by Nalin Dahyabhai.

To paraphrase bits of discussion with Nalin:

The current flow adds digests to the SignedData, using the plaintext, if none
are found (apparently expecting that a list of digest algorithms is present,
which I'm not sure is normal even for well-formed data).  If digests are
already present, then they are left alone.  The signatures are then verified.
They verify, and evolution happily displays the copy that wasn't protected by
the signature.

The proposed patch overwrites the digests used for signature verification
using locally-generated values computed over the not-encapsulated parts,
effectively ignoring encapsulated data.
Comment 6 Srinivasa Ragavan 2009-02-25 07:59:46 UTC
Matt, for trunk?
Comment 7 Matthew Barnes 2009-02-26 18:37:38 UTC
Committed to trunk (revision 10106).
Comment 8 Daniel Gryniewicz 2009-02-27 20:09:28 UTC
When I tried to backport this to 2.24.5, it causes every S/MIME signed message I have to fail with:

Error verifying signature
Cannot set message digests

Is this correct?  Are they all messed up somehow?
Comment 9 Milan Crha 2009-03-02 18:38:33 UTC
Reopening per last comment and because I see this too. Though I can reproduce this with the above data, I cannot do that with newly created messages in evolution, those are correctly recognized as invalid signatures when I add something to the message body. I wonder how much related it is with a fact that the above certificate doesn't offer any email or common name itself.
Comment 10 Joachim Breitner 2009-03-02 22:41:05 UTC
It might be that my test data is not perfect – it’s the output of a script I wrote myself (otherwise I woundn’t have spotted the bug, I guess). IIRC, the unmodified mail got verified as ok valid in Thunderbird, though.
Comment 11 Milan Crha 2009-03-03 09:33:43 UTC
(In reply to comment #10)
> IIRC, the unmodified mail got verified as ok valid in Thunderbird, though.

And the above attached as invalid?
Comment 12 Joachim Breitner 2009-03-03 20:04:51 UTC
It was detected as invalid, IIRC.
Comment 13 Milan Crha 2009-03-04 15:16:27 UTC
Created attachment 130025 [details] [review]
fix for a break?

for evolution-data-server;

this change helped me to see the above mail as invalid and old (evo-generated) mails as valid/invalid based on their correctness. Also, when I tried with the correct email, from the debian bug, then it shows signature as valid, thus this change should be fine.
Comment 14 Matthew Barnes 2009-03-04 20:36:29 UTC
That works.  I guess we were being too strict about digests.

Fixed up the code a bit and committed to trunk (revision 10132).
Comment 15 Yves-Alexis Perez 2009-03-04 21:17:00 UTC
Is there a change this could be backported to 2.24?
Comment 16 Matthew Barnes 2009-03-04 21:58:17 UTC
I can commit it to the gnome-2-24 branch but there are no more 2.24 releases.
Distros will have to distribute it.
Comment 17 Yves-Alexis Perez 2009-03-04 22:39:31 UTC
Yes, that's why I'm asking. I'm the de facto evolution maintainer in debian and I'm not sure how the 2.26 stuff will be handled, so in the meantime I'd prefer trying to setup a correct 2.24 one.
Comment 18 Daniel Gryniewicz 2009-03-07 20:15:30 UTC
Created attachment 130246 [details] [review]
Fix, backported to 2.24.5

Here's the committed version, backported to 2.24.5. This is what I'm going to push out on Gentoo.
Comment 19 Yves-Alexis Perez 2009-03-08 10:04:14 UTC
Thanks, it work pretty fine :)
Comment 20 Yves-Alexis Perez 2009-03-08 13:37:48 UTC
Hmhm except that now evolution doesn't build anymore against eds because of:

cc -g -O2 -g -Wall -O2 -fPIC -Wall -Wmissing-prototypes -Wno-sign-compare -o .libs/test-calendar test-calendar.o -pthread  ./.libs/libemiscwidgets.so /tmp/buildd/evolution-2.24.5/widgets/text/.libs/libetext.so /tmp/buildd/evolution-2.24.5/widgets/table/.libs/libetable.so /tmp/buildd/evolution-2.24.5/a11y/widgets/.libs/libevolution-widgets-a11y.so /tmp/buildd/evolution-2.24.5/a11y/.libs/libevolution-a11y.so /tmp/buildd/evolution-2.24.5/e-util/.libs/libeutil.so -lcamel-provider-1.2 -lgtkhtml-editor -lgtkhtml-3.14 /usr/lib/libenchant.so -lnss3 -lnssutil3 -lsmime3 -lssl3 ../../e-util/.libs/libeutil.so -lcamel-1.2 /usr/lib/libsqlite3.so -lplds4 -lplc4 -lnspr4 -lX11 -lgpilotd -lgpilotdcm -lgpilotdconduit /usr/lib/libpisync.so /usr/lib/libpisock.so /usr/lib/libusb.so -lpthread /usr/lib/libbluetooth.so /usr/lib/libgnomeui-2.so -lSM -lICE /usr/lib/libbonoboui-2.so /usr/lib/libgnomevfs-2.so /usr/lib/libgnomecanvas-2.so /usr/lib/libart_lgpl_2.so -ledataserverui-1.2 /usr/lib/libglade-2.0.so -lebook-1.2 /usr/lib/libgtk-x11-2.0.so /usr/lib/libgdk-x11-2.0.so /usr/lib/libatk-1.0.so /usr/lib/libpangoft2-1.0.so /usr/lib/libgdk_pixbuf-2.0.so -lm /usr/lib/libpangocairo-1.0.so /usr/lib/libcairo.so /usr/lib/libpango-1.0.so /usr/lib/libfreetype.so -lz -lfontconfig /usr/lib/libgnome-2.so /usr/lib/libpopt.so -ledataserver-1.2 /usr/lib/libxml2.so /usr/lib/libgconf-2.so -lsoup-2.4 /usr/lib/libbonobo-2.so /usr/lib/libgio-2.0.so /usr/lib/libbonobo-activation.so /usr/lib/libORBit-2.so /usr/lib/libgmodule-2.0.so -ldl /usr/lib/libgthread-2.0.so -lrt /usr/lib/libgobject-2.0.so /usr/lib/libglib-2.0.so  -Wl,--rpath -Wl,/usr/lib/evolution/2.24 -Wl,--rpath -Wl,/usr/lib
/usr/lib/gcc/x86_64-linux-gnu/4.3.3/../../../../lib/libcamel-provider-1.2.so: undefined reference to `set_nss_error'
Comment 21 Yves-Alexis Perez 2009-03-08 15:05:55 UTC
Created attachment 130282 [details] [review]
Fix backported patch to 2.24.5

set_nss_error() doesn't exist in 2.24.5. This fixed version seems to work fine, replacing set_nss_error() by camel_exception_set()
Comment 22 Daniel Gryniewicz 2009-03-08 18:07:24 UTC
Good call.  I hadn't tried to rebuild my evo.  Thanks, I've updated gentoo's patch.
Comment 23 Srinivasa Ragavan 2009-03-09 04:03:46 UTC
Clearning off patches from my radar.
Comment 24 Milan Crha 2009-03-31 11:48:40 UTC
Last committed change here
http://svn.gnome.org/viewvc/evolution-data-server/trunk/camel/camel-smime-context.c?r1=10106&r2=10132
introduces a regression, signed and encrypted S/MIME messages are not displayed correctly, as was reported in the downstream bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=492852
Comment 25 Milan Crha 2009-03-31 11:51:46 UTC
Created attachment 131771 [details] [review]
proposed eds patch

for evolution-data-server;
Comment 26 Milan Crha 2009-03-31 11:55:24 UTC
Committed to trunk. Committed revision 10194.
Comment 27 Akhil Laddha 2010-03-25 09:37:32 UTC
*** Bug 576734 has been marked as a duplicate of this bug. ***