GNOME Bugzilla – Bug 564465
Wrong signature marked as valid on modified mail messages
Last modified: 2010-03-25 09:37:32 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.
Created attachment 124634 [details] CA for the signatures
Created attachment 124635 [details] S/MIME signed mail (invalid in openssl, valid in evolution)
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.
CVE-2009-0547 has been assigned to this vulnerability.
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.
Matt, for trunk?
Committed to trunk (revision 10106).
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?
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.
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.
(In reply to comment #10) > IIRC, the unmodified mail got verified as ok valid in Thunderbird, though. And the above attached as invalid?
It was detected as invalid, IIRC.
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.
That works. I guess we were being too strict about digests. Fixed up the code a bit and committed to trunk (revision 10132).
Is there a change this could be backported to 2.24?
I can commit it to the gnome-2-24 branch but there are no more 2.24 releases. Distros will have to distribute it.
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.
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.
Thanks, it work pretty fine :)
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'
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()
Good call. I hadn't tried to rebuild my evo. Thanks, I've updated gentoo's patch.
Clearning off patches from my radar.
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
Created attachment 131771 [details] [review] proposed eds patch for evolution-data-server;
Committed to trunk. Committed revision 10194.
*** Bug 576734 has been marked as a duplicate of this bug. ***