classification
Title: email.Generator: no header wrapping for multipart/signed
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: barry, brian.curtin, collinwinter, gagern, georg.brandl, r.david.murray, srikanths
Priority: normal Keywords: easy, needs review, patch

Created on 2007-02-28 12:04 by gagern, last changed 2011-07-14 12:29 by srikanths. This issue is now closed.

Files
File name Uploaded Description Edit
968430_generator_norewrap.patch gagern, 2007-02-28 12:04 disable header wrapping for multipart/signed in email.Generator
test5.eml gagern, 2007-08-28 01:55
issue1670765a.py gagern, 2010-01-15 16:26 Self-contained test script
issue1670765_python2.patch gagern, 2010-01-15 18:44 Fix and test cases for Python 2
issue1670765_python3.patch gagern, 2010-01-16 09:15 Fix and test cases for Python 3
Messages (16)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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....:)
History
Date User Action Args
2011-07-14 12:29:25srikanthssetnosy: + srikanths
2010-01-18 19:07:25r.david.murraysetmessages: + msg98028
2010-01-18 16:49:31gagernsetmessages: + msg98026
2010-01-18 16:25:59r.david.murraysetmessages: + msg98024
2010-01-16 18:42:54r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg97895

stage: commit review -> resolved
2010-01-16 16:00:57gagernsetmessages: + msg97882
2010-01-16 15:00:34r.david.murraysetmessages: + msg97879
stage: patch review -> commit review
2010-01-16 09:15:19gagernsetfiles: + issue1670765_python3.patch

messages: + msg97866
2010-01-15 21:10:21r.david.murraysetassignee: barry -> r.david.murray
versions: + Python 2.7, Python 3.2

nosy: + r.david.murray
stage: test needed -> patch review
2010-01-15 18:44:17gagernsetfiles: + issue1670765_python2.patch

messages: + msg97831
2010-01-15 16:40:30brian.curtinsetnosy: + brian.curtin

messages: + msg97822
stage: patch review -> test needed
2010-01-15 16:33:04brian.curtinsetkeywords: + needs review
stage: test needed -> patch review
versions: + Python 3.1, - Python 3.0
2010-01-15 16:26:48gagernsetfiles: + issue1670765a.py

messages: + msg97819
2009-04-22 15:20:59ajaksu2setkeywords: + easy
2009-03-30 21:41:24ajaksu2setstage: test needed
type: behavior
versions: + Python 2.6, Python 3.0
2009-02-14 13:56:42ajaksu2linkissue968430 dependencies
2007-08-28 01:55:08gagernsetfiles: + test5.eml
messages: + msg55346
2007-08-28 00:37:59collinwintersetmessages: + msg55344
2007-08-28 00:32:37gagernsetmessages: + msg55343
2007-02-28 12:04:09gagerncreate