| msg51995 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2007-02-28 12:04 |
| The body of a multipart/signed message has to remain unmodified for the signature to stay intact. Rewrapping headers of nested MIME parts breaks signatures. So I disabled header rewrapping for multipart/signed, by adding a new handler to the email.Generator class. There still remains an issue about leading spaces in header values breaking signatures. The supplied patch is against the latest subversion sources, r54016. Related issues: Python bug #968430 http://tinyurl.com/2973or Mailman bug #815297 http://tinyurl.com/2a8dta |
|
|
| msg51996 - (view) |
Author: Collin Winter (collinwinter) *  |
Date: 2007-03-08 18:01 |
| Thanks for your patch! Could you work up some tests for this, too? |
|
|
| msg51997 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-03-13 08:34 |
| Assigning to Barry. |
|
|
| msg55343 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2007-08-28 00:32 |
| Looks like I missed your comments on this patch. What kind of tests do you have in mind? Tests demonstrating where current implementation fails and my patch will help? Or rather tests checking that this patch won't break anything else? The former would be easy enough, but for the latter I'm not sure how far this would go, as people tend to make strange assumptions I would never have thought of. |
|
|
| msg55344 - (view) |
Author: Collin Winter (collinwinter) * |
Date: 2007-08-28 00:37 |
| I'd like some tests demonstrating where the current implementation fails and your patch helps, yes. |
|
|
| msg55346 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2007-08-28 01:55 |
| Take the attached test5.eml. Run it through the following python script: import email print (email.message_from_file(open("test5.eml")).as_string(False)) The result will have both instances of the X-Long-Line header rewrapped. As the second instance is included in the digest calculation, the signature verification will now fail. This is a real world signature algorithm, following RFC 3156 (if I didn't make a mistake). If you have an OpenPGP-enabled mailreader (e.g. enigmail for Thunderbird) and have some way of injecting a mail as is into your mail folders (e.g. a maildir-based server), then you can use this setup to verify that the signature was correct in the first place and is broken after parsing and reconstruction by python. If you don't have such a setup available, and you don't believe me that rewrapping the header breaks the signature, then I could either devise some unrealistic but easy-to-check signing process, or could try to get this working with an S/MIME signature using an X.509 certificate. I would rather avoid this, though. |
|
|
| msg97819 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2010-01-15 16:26 |
| Let's get some traction here, please! Attached is a test case which will demonstrate the issue. It includes the content of test5.eml as a string so that it won't require additional files. It produces both human-readable output and a suitable exit status. Turning it into a unit test should be easy as well. It doesn't do signature verification, but uses simple string comparison instead. The rationale is that anything changing the string would break the signature as well. That should be enough for unit tests. Please change stage to "patch review". |
|
|
| msg97822 - (view) |
Author: Brian Curtin (brian.curtin) * |
Date: 2010-01-15 16:40 |
| Martin, can you provide a true unit test? Lib\email\test\test_email.py has many examples, and something like this would fit in there. |
|
|
| msg97831 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2010-01-15 18:44 |
| OK, here is a patch providing both two test cases and the fix for current trunk. Will probably hack something for python 3 as well, although there the Message.as_string approach works due to the new headerlength argument defaulting to 0. So there I'd adjust the patch I also included a second test e-mail together with two disabled test cases in order to address the whitespace issue I mentioned. Would be nice to have a fix for these as well, but I assume you don't want known to fail test cases without a fix in trunk unless it's really serious, right? In any case, the fact that there should be at least four tests, maybe more in the future, all dealing with signature preservation, led me to have a class dedicated to them, instead of only adding methods to one of the existing classes. I hope you agree with that decision. |
|
|
| msg97866 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2010-01-16 09:15 |
| Here is the corresponding path for python3 (py3k branch). It's mostly the same, except for one additional test which ensures header preservation even if the maxheaderlen argument to Message.as_string is greater than 0. It's called test_long_headers_as_string_maxheaderlen. |
|
|
| msg97879 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-01-16 15:00 |
| I've committed fix and the non-disabled tests to trunk in r77517. I updated the comments to point to the relevant RFC and note that the problem is not fixed, just mitigated. I've made a note of the additional tests in my issues list for the email package. I'm not sure we can fix the whitespace problem in the 2.x email package. |
|
|
| msg97882 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2010-01-16 16:00 |
| You missed a digit in the test comment: s/See issue 96843/See issue 968430/ |
|
|
| msg97895 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-01-16 18:42 |
| I actually had the wrong message number entirely. I was trying to reference this one, since it has the additional tests. Fixed in r77525. I backported the fix to 2.6 in r77526 and r77527, forwarded ported to py3k in r77542 (with the addition of the new test), and backported to 3.1 in r77546. I'm going to close this issue, but we aren't forgetting about the whitespace issues. |
|
|
| msg98024 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-01-18 16:25 |
| See issue 1670765 for more discussion of the whitespace issues. |
|
|
| msg98026 - (view) |
Author: Martin von Gagern (gagern) |
Date: 2010-01-18 16:49 |
| Wrong issue number again, or did you deliberately try to catch me in an infinite see-also loop? ;-) I've tried to work out what issue you meant, but failed so far. |
|
|
| msg98028 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-01-18 19:07 |
| Heh. Copy and paste error. I copied the issue number I wanted to reference, then copied the issue number I needed to open in order to paste the reference, and then promptly forgot that I had to recopy the issue number to paste it... Anyway, the correct cross reference is to issue 968430. Thanks for keeping me accurate, again. (Hopefully I did the copy and paste right this time....:) |
|
|