Moving towards 1.1.0 #15

acatton · 2015-07-30T03:36:20Z

I don't use this project, but I'll need to use in the future. And you seem to have a userbase.

This is my ongoing work to move towards 1.1.0.

Clean up the overuse of "pseudo"-private methods
Add the ability to override the GPG executable path
Don't modify os.environ
Get rid of assert is_encrypted(...) in the tests, use assert self.decrypt(encrypted_content) == content
Use a temporary directory as GPG --homedir during the tests
Fix Handle attachements (multipart) using PGP/MIME #2
Fix Add support for Python 3.4 #12
~~Fix Use setuptools for easy install? #3~~ See my comment on Use setuptools for easy install? #3
Include Fixed multiple recipient issue. #10

I don't have a setup with zeyple, can someone test my fixes on their setup?

* Set the protocol to the default protocol (it doesn't really matter) * Pass the home_dir to gpgme * Give the ability to override the gpg executable

(This is backward compatible with python 2)

infertux · 2015-07-30T14:42:31Z

Awesome work @acatton! I think we should leave #5 out of 1.1.0 though, this is already a lot of changes.

infertux · 2015-07-30T14:55:06Z

tests/test_zeyple.py

+        for m in emails:
+            assert m['X-Zeyple'] is not None
+            payload = self.decrypt(m.get_payload()).strip()
+            assert payload == six.b(content)


I think it'd be great to replace assert is_encrypted(...) with assert self.decrypt(...) == plain_text_content for all tests now that we have a way to decrypt messages. is_encrypted() was to make sure the email was actually encrypted but if we could both check that it is encrypted and properly decryptable, it's even better. Thoughts?

Right know, self.decrypt doesn't play well with non-ascii characters, and I haven't tried to figure it out yet. This is why the test I wrote use ascii characters for now. But yeah, that sounds like the right way to go. I added it to the todolist of this pull request.

infertux · 2015-07-30T15:14:39Z

This is great @acatton! I left some nitpicking comments on the diff.

I'll test your changes on my setup and let you know if everything works fine.

infertux · 2015-07-30T15:56:00Z

Just like #2 (comment), plain text emails work fine but multipart emails can't be decrypted.

Actual result:

Content-Type: multipart/mixed; boundary="BOKacYhQ+x31HxR3"
Content-Disposition: inline

-----BEGIN PGP MESSAGE-----
Version: GnuPG v1.4.12 (GNU/Linux)

[...]

I can decrypt [...] on the command line to:

Content-Type: multipart/mixed; boundary="===============6531299818164456294=="
MIME-Version: 1.0

--===============6531299818164456294==
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline


--===============6531299818164456294==
Content-Type: image/png
Content-Disposition: attachment; filename="image.png"
Content-Transfer-Encoding: base64

[base64 image]

Expected result using PGP/MIME (expected by Thunderbird at least):

Content-Type: multipart/encrypted;
 protocol="application/pgp-encrypted";
 boundary="tEksdshbSOMXJQgTtgeuSwKpLVMQ9INAl"

This is an OpenPGP/MIME encrypted message (RFC 4880 and 3156)
--tEksdshbSOMXJQgTtgeuSwKpLVMQ9INAl
Content-Type: application/pgp-encrypted
Content-Description: PGP/MIME version identification

Version: 1

--tEksdshbSOMXJQgTtgeuSwKpLVMQ9INAl
Content-Type: application/octet-stream; name="encrypted.asc"
Content-Description: OpenPGP encrypted message
Content-Disposition: inline; filename="encrypted.asc"

-----BEGIN PGP MESSAGE-----

[...]

[...] decrypts to:

Content-Type: multipart/mixed;
 boundary="------------020007060702010403070805"

This is a multi-part message in MIME format.
--------------020007060702010403070805
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable



--------------020007060702010403070805
Content-Type: image/png; name="image.png"
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="image.png"

[base64 image]

Basically, it looks like you need to wrap everything into a encrypted.asc file and use Content-Type: multipart/encrypted as the wrapping MIME type.

This is just my empirical findings. I haven't dug into the RFCs but I'll to it when I get a chance.

acatton · 2015-07-31T00:50:21Z

@infertux this is great. The multipart encryption is what I wanted you to test, and that what I need to work on then. And thank you @Nithanim for all the debugging information, this is very useful.

I'll try to address all the reported issues I soon as I can.

acatton · 2015-07-31T00:54:33Z

@infertux regarding #5, I think it's pretty important feature. I want to be able to generate keys for my servers, and make sure my servers actually sent these emails, and that nobody is spoofing them.

Moreover, I think it should be pretty straightforward to implement.

By forcing binary data, we don't have to worry about encoding. Everything is just bytes.

Fix #2

acatton · 2015-08-03T05:53:53Z

The tests should be totally broken right now. I need to fix them before merging, but this should behave according to the RFCs. Can someone try to send emails with and without attachment?

@infertux @Nithanim.

Thanks.

acatton · 2015-08-03T05:55:44Z

@infertux you are right. I think we should move #12 and #3 to another version. What do you think?

Nithanim · 2015-08-05T13:02:40Z

@acatton I tested your latest commit https://github.com/acatton/zeyple/commit/1720c0249763fb5f90227f477eb230551cc5e4e2 with mixed success.
Plaintext without attachment is broken now and won't display anything at all (although it is stating that it decrypted something). Plaintext with attachment works great! Html only without attachment is displayed somewhat plain only and then in html again after an < hr> but this is acceptable in some way but not perfect. Html with attachment on the other side looks now exactly as it should again.
You did an fantastic job there!

Here are some test-files again: https://www.dropbox.com/sh/awbjoff0naycmv3/AAB3SLvtLbSO_7uGpl81T3hva/2nd?dl=0%3Flst (link is the same as last time).

acatton · 2015-08-06T01:10:54Z

@Nithanim this is definitely not mergeable as is then. It supposed to work perfectly.

Thank you for the feedback, i'll be fixing that when I have time :) .

Nithanim · 2015-08-06T22:54:56Z

@acatton I don't know how useful it will be but I am trying to build a vagrant box to have a easy-to-setup test environment. It looks good so far. However it currently lacks the enigmail setup and the gpg keys.
It is very likely to have some problems. For example if you are on windows you need to "git config core.eol lf && git checkout-index --force --all" in a clean workingdir to workaround a python(?) bug in the guest system.
For the fearless: https://github.com/Nithanim/zeyple/tree/vagrant

PS: Got my first external html mail and it was successfully encrypted by zeyple! Exactly like in my simple test. That is so awesome!

infertux · 2015-08-14T15:10:06Z

Sorry for the delay @acatton. I tried https://github.com/acatton/zeyple/commit/1720c0249763fb5f90227f477eb230551cc5e4e2 and I'm experiencing the same issue described by @Nithanim.

I compared it with properly encrypted plain text emails and if I understand that right, you need to convert the plain text into a MIME payload here - maybe with this or that?

In other words, payload shouldn't be a mere hello world string but this:

Content-Type: multipart/mixed; boundary="MvkF3o3R8o27A6vwDL8mILdrsqfvHWfQC"

--MvkF3o3R8o27A6vwDL8mILdrsqfvHWfQC
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

hello world

--MvkF3o3R8o27A6vwDL8mILdrsqfvHWfQC--

Hope that makes sense.

infertux · 2015-08-14T15:23:12Z

Regarding the 1.1.0 release @acatton:

let's put off Add support for Python 3.4 #12 to a future releae since those are pretty recent Python versions anyway,
if you think sign outgoing Mails #5 is pretty straightforward to implement, then I'm okay to include it in 1.1.0 otherwise I think we should do it later since this release is already quite big in term of changes,
Use setuptools for easy install? #3 is a WONT FIX.

infertux · 2015-08-18T14:22:43Z

Relevant parts of the RFCs about the encrypted payload: http://tools.ietf.org/html/rfc3156#section-6.1 & http://tools.ietf.org/html/rfc1847#section-2.2

The contents of the body part to be protected is prepared according
to a local convention. The contents are then transformed into a
MIME body part in canonical MIME format, including an appropriate
set of MIME headers. [emphasis mine]

This seems to apply to any Content-Type, including text/plain.

/cc @acatton

infertux · 2015-08-18T15:37:36Z

Alright, I updated the tests to reflect what is expected by the RFCs in this branch. There is one failing test. If we manage to make it pass without breaking other tests, it should encrypt everything properly 😃

infertux · 2015-08-18T15:49:35Z

Just gave you push access to the repo @acatton so we can both work on the same branch. I opened a new PR with branch moving-towards-1.1.0 at #17.

acatton added 13 commits July 29, 2015 20:36

There's no problem with encrypt being public

48866fb

Make load_configuration() return config

86a80b4

Define the gpg context as a property

44ee5d6

* Set the protocol to the default protocol (it doesn't really matter) * Pass the home_dir to gpgme * Give the ability to override the gpg executable

Remove unused import

6644b1e

Fix #2

9701489

This is testing python, not zeyple

abf5ab4

Always use os.path.join instead of concatenation

b2669d5

Fix style: the PEP-0008 requires two space before an inline comment

70fba59

Make the tests use a temporary directory and temporary gpg homedir

db7c9ad

Use test keys (for decoding later)

f202c68

Octal literals needs to be lead by 0o in Python 3

d3793df

(This is backward compatible with python 2)

Propose a better alternative to #10

f378afa

Merge branch 'include-10' into moving-towards-1.1.0

398cde0

infertux reviewed Jul 30, 2015
View reviewed changes

acatton added 4 commits July 30, 2015 22:08

Fix bad abreviation fname -> filename

35cd5fd

Remove wrongfully committed test keys

ce71bff

Force all data to be binary and get rid of is_encrypted()

a019007

By forcing binary data, we don't have to worry about encoding. Everything is just bytes.

Behave according to the RFCs

1720c02

Fix #2

infertux mentioned this pull request Aug 18, 2015

Moving towards 1.1.0 #17

Merged

infertux closed this Aug 18, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving towards 1.1.0 #15

Moving towards 1.1.0 #15

acatton commented Jul 30, 2015

infertux commented Jul 30, 2015

infertux Jul 30, 2015

acatton Jul 31, 2015

infertux commented Jul 30, 2015

infertux commented Jul 30, 2015

acatton commented Jul 31, 2015

acatton commented Jul 31, 2015

acatton commented Aug 3, 2015

acatton commented Aug 3, 2015

Nithanim commented Aug 5, 2015

acatton commented Aug 6, 2015

Nithanim commented Aug 6, 2015

infertux commented Aug 14, 2015

infertux commented Aug 14, 2015

infertux commented Aug 18, 2015

infertux commented Aug 18, 2015

infertux commented Aug 18, 2015

Moving towards 1.1.0 #15

Moving towards 1.1.0 #15

Conversation

acatton commented Jul 30, 2015

infertux commented Jul 30, 2015

infertux Jul 30, 2015

Choose a reason for hiding this comment

acatton Jul 31, 2015

Choose a reason for hiding this comment

infertux commented Jul 30, 2015

infertux commented Jul 30, 2015

acatton commented Jul 31, 2015

acatton commented Jul 31, 2015

acatton commented Aug 3, 2015

acatton commented Aug 3, 2015

Nithanim commented Aug 5, 2015

acatton commented Aug 6, 2015

Nithanim commented Aug 6, 2015

infertux commented Aug 14, 2015

infertux commented Aug 14, 2015

infertux commented Aug 18, 2015

infertux commented Aug 18, 2015

infertux commented Aug 18, 2015