Bug #7755: Migrate WhisperBack to Python 3

Bug #7755

Migrate WhisperBack to Python 3

Added by intrigeri 2014-08-07 09:11:35 . Updated 2015-08-17 06:44:25 .

Status:

Resolved

Priority:

Elevated

Assignee:

Category:

Target version:

Tails_2.0

Start date:

2014-09-13

Due date:

% Done:

100%

Feature Branch:

whisperback:feature/python3

Type of work:

Code

Blueprint:

Starter:

Affected tool:

WhisperBack

Deliverable for:

266

Description

python-gnupginterface won’t be part of Jessie, so we need to migrate it to some other GnuPG Python bindings, such as python-gnupg (or isis’ fork, that has been ITP’d, or python3-gnupg.

Subtasks

Feature #7891: Migrate WhisperBack to python3/GI

Resolved

Feature #7892: Migrate WhisperBack to SSL

Resolved

100

Feature #7893: Test updated WhisperBack

Resolved

100

Feature #7894: Migrate WhisperBack to python-gnupg

Resolved

100

Related issues

Related to Tails - ~~Bug #9412~~: Add SOCKS support to WhisperBack	Resolved	2015-05-16	2015-07-15
Related to Tails - ~~Bug #10577~~: WhisperBack does not start on Jessie with French locales	Resolved	2015-11-18
Has duplicate Tails - ~~Feature #7805~~: Migrate WhisperBack from python-gnupginterface to python-gnupg	Duplicate	2014-08-23
Blocks Tails - ~~Feature #7756~~: Reintroduce WhisperBack in Jessie ISOs	Resolved	2015-01-18
Blocked by Tails - ~~Feature #8513~~: Implement PGP/MIME in WhisperBack	Resolved	2015-01-02

History

#1 Updated by intrigeri 2014-08-07 09:14:53

Parent task set to ~~Feature #7756~~

#2 Updated by BitingBird 2014-09-12 22:26:14

has duplicate ~~Feature #7805~~: Migrate WhisperBack from python-gnupginterface to python-gnupg added

#3 Updated by intrigeri 2014-09-14 20:36:05

Subject changed from WhisperBack needs to be migrated away from python-gnupginterface to Migrate WhisperBack away from python-gnupginterface

#4 Updated by intrigeri 2014-09-14 20:39:08

Subject changed from Migrate WhisperBack away from python-gnupginterface to Migrate WhisperBack to Python 3

This task now has various subtasks that apparently have absolutely nothing to do with python-gnupginterface, so I’m retitling it to better match how it’s used. Alan, if I missed something, feel free to correct me.

#5 Updated by intrigeri 2014-09-14 20:41:58

Status changed from Confirmed to In Progress

#6 Updated by alant 2015-01-01 17:14:30

Feature Branch set to whisperback:feature/python3

#7 Updated by BitingBird 2015-01-02 20:12:33

Parent task deleted (~~~~Feature #7756~~~~)

#8 Updated by BitingBird 2015-01-02 20:13:52

blocks ~~Feature #7756~~: Reintroduce WhisperBack in Jessie ISOs added

#9 Updated by BitingBird 2015-01-02 20:16:04

Trying to set this ticket as a child from Feature #5958 fails with “Parent task is invalid”. The message doesn’t say why, so if somebody can debug that…

#10 Updated by BitingBird 2015-01-07 18:26:16

Affected tool set to WhisperBack

#11 Updated by alant 2015-01-12 12:18:20

related to Feature #6183: Use multiple PGP/MIME attachments for text snippets and OpenPGP public key included in Whisperback reports added

#12 Updated by alant 2015-01-18 18:28:22

related to deleted (~~Feature #6183: Use multiple PGP/MIME attachments for text snippets and OpenPGP public key included in Whisperback reports~~)

#13 Updated by alant 2015-01-18 18:36:45

We new use PGP/MIME (see ~~Feature #8513~~). The migration should be completed as whisperback is now able to send bugreports from Tails/Jessie.

#14 Updated by alant 2015-01-18 19:25:47

blocked by ~~Feature #8513~~: Implement PGP/MIME in WhisperBack added

#15 Updated by intrigeri 2015-01-18 23:15:42

What are the next steps here? Trying to guess:

get the updated code reviewed
update the packaging
build a Jessie-specific package

Right? Anything else?

#16 Updated by alant 2015-01-19 11:31:49

Assignee deleted (~~alant~~)
QA Check set to Ready for QA

This branch is ready for review. I don’t know wether we should merge it in a new “jessie” branch in whisperback repository or consider it to be the jessie branch once reviewed.

#17 Updated by intrigeri 2015-01-19 11:55:37

Assignee set to intrigeri

#18 Updated by alant 2015-01-19 13:36:00

Assignee deleted (~~intrigeri~~)

> - get the updated code reviewed

Yes, there is an email about that on tails-dev.

> - update the packaging

This is done in the branch.

> - build a Jessie-specific package

This is 7756 and its childern. Please see there.

#19 Updated by intrigeri 2015-01-19 14:28:34

>> - get the updated code reviewed

> Yes, there is an email about that on tails-dev.

That’s why I’ve assigned this ticket to me.

#20 Updated by intrigeri 2015-01-19 14:31:33

Assignee set to intrigeri

#21 Updated by alant 2015-01-19 15:53:57

Please consider also reviewing a forgotten commit I just pushed: c3a4096. Sorry for the inconvenience.

#22 Updated by intrigeri 2015-03-07 17:21:55

Assignee changed from intrigeri to alant
QA Check changed from Ready for QA to Dev Needed

Good job! Here’s an initial code review. I haven’t tested it yet.

I see gnupg.GPG(keyring=keyring): does this imply --no-default-keyring?
Any reason to set X-Python3-Version: >= 3.2 and not 3.4? Jessie has 3.4, so I suspect this code wasn’t tested on 3.2, so maybe better require 3.4 right away :)
Typo in “a list of recepients” (s/recepients/recipients/)
Typo in “message to be send” (s/send/sent)
I see a few set_charset('us-ascii') and can’t be bothered to understand if it will apply to decrypted content as well: was the PGP/MIME code tested with non-US-ascii cleartext?
Please include the copyright and license information for the code taken from http://www.physics.drexel.edu/~wking/code/python/send_pgp_mime, and then add it to debian/copyright too.

Perhaps more importantly, I see:

+    context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
+    context.load_verify_locations(cafile=tls_cafile)
+    context.verify_mode = ssl.CERT_REQUIRED
+    # disable compression to prevent CRIME attacks (OpenSSL 1.0+)
+    # this is copied from python's SSL module source
+    context.options |= ssl.OP_NO_COMPRESSION

Why do we bother doing this by hand, while ssl.create_default_context does the right thing wrt. supported protocols, CERT_REQUIRED (once purpose is set to SERVER_AUTH), and also configures a sane set of allowed ciphers?

My understanding is that using create_default_context gives us required protocol and ciphers upgrades for free, as opposed to:

protocols: having to maintain the list ourselves, which we’ll forget too often, and even if we don’t we can make mistakes: e.g. the current code does not enable TLS 1.1 and 1.2, for some reason;
ciphers: relying on SSLContext’s undocumented defaults, that may be weaker than what create_default_context does (I suspect these defaults are just OpenSSL’s ones).

#23 Updated by alant 2015-03-31 17:57:04

> * I see gnupg.GPG(keyring=keyring): does this imply --no-default-keyring?

_keyring (defaults to None)
If specified, the value is used as the name of the keyring file. The default keyring is not used. A list of paths to keyring files can also be specified._
(https://pythonhosted.org/python-gnupg/#getting-started)

> * Any reason to set X-Python3-Version: >= 3.2 and not 3.4? Jessie has 3.4, so I suspect this code wasn’t tested on 3.2, so maybe better require 3.4 right away :)

No, I pasted that from debian wiki I think. Fixed

> * Typo in “a list of recepients” (s/recepients/recipients/)

Fixed.

> * Typo in “message to be send” (s/send/sent)

Fixed.

> * I see a few set_charset('us-ascii') and can’t be bothered to understand if it will apply to decrypted content as well: was the PGP/MIME code tested with non-US-ascii cleartext?

This comes from http://www.physics.drexel.edu/~wking/code/python/send_pgp_mime, which seems to handle other charsets. I didn’t test though.

> * Please include the copyright and license information for the code taken from http://www.physics.drexel.edu/~wking/code/python/send_pgp_mime, and then add it to debian/copyright too.
>
Added to the header of encryption.py.

> Perhaps more importantly, I see:
>
>

+    context = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
+    context.load_verify_locations(cafile=tls_cafile)
+    context.verify_mode = ssl.CERT_REQUIRED
+    # disable compression to prevent CRIME attacks (OpenSSL 1.0+)
+    # this is copied from python's SSL module source
+    context.options |= ssl.OP_NO_COMPRESSION

>
> Why do we bother doing this by hand, while ssl.create_default_context does the right thing wrt. supported protocols, CERT_REQUIRED (once purpose is set to SERVER_AUTH), and also configures a sane set of allowed ciphers?
>
Right, that’s now the right way to do it. Fixed.

#24 Updated by alant 2015-03-31 17:58:34

Assignee changed from alant to intrigeri
QA Check changed from Dev Needed to Ready for QA

Please check debian package build. Thanks!

#25 Updated by intrigeri 2015-03-31 19:24:17

> Please check debian package build.

What do you mean exactly?

Also, please address the two remaining concerns from my previous review (updating debian/copyright, and actually testing encoding support) before asking for another one, thanks :)

#26 Updated by intrigeri 2015-03-31 19:25:06

Assignee changed from intrigeri to alant
QA Check changed from Ready for QA to Info Needed

#27 Updated by alant 2015-03-31 19:55:30

Assignee changed from alant to intrigeri

intrigeri wrote:
> > Please check debian package build.
>
> What do you mean exactly?
>
I’m not currently able to build the package. It would be very nice if you could handle this part. Else, I can do it, but not shortly.

> Also, please address the two remaining concerns from my previous review (updating debian/copyright,

I think this one was addressed already. What is missing?

> and actually testing encoding support) before asking for another one, thanks :)

I send a bug reports with a lot of accents and recieved it well. I don’t know how to test better, but I’m up to hear from you what else you’d like me to test.

#28 Updated by intrigeri 2015-03-31 20:45:58

> I’m not currently able to build the package. It would be very nice if you could handle this part. Else, I can do it, but not shortly.

I’m happy to do it, but I doubt I’ll have time to handle that shortly — who knows. In any case I’ll first do a review of the current state, so it’s fine to leave the ticket assigned to me.

>> Also, please address the two remaining concerns from my previous review (updating debian/copyright,

> I think this one was addressed already.

You wrote that you added it to encryption.py, without mentioning debian/copyright, and I didn’t notice that it also had been added there in a commit titled “Bump python version to the one actually targeted” that includes two changes that are fully unrelated to its explicit purpose.

> What is missing?

Apparently nothing :)

>> and actually testing encoding support) before asking for another one, thanks :)

> I send a bug reports with a lot of accents and recieved it well.

Thanks, that’s good enough for me! :)

#29 Updated by BitingBird 2015-04-01 12:31:47

QA Check deleted (~~Info Needed~~)

#30 Updated by intrigeri 2015-04-01 13:04:17

QA Check set to Ready for QA

#31 Updated by intrigeri 2015-05-16 09:06:38

Code review passes. Will now deal with the Debian packaging, then will test it.

#32 Updated by intrigeri 2015-05-16 10:09:52

Status changed from In Progress to Resolved

Applied in changeset commit:a716e575e99b7ecd4d39489d9465808a322d3454.

#33 Updated by intrigeri 2015-05-16 10:12:52

Assignee deleted (~~intrigeri~~)
QA Check changed from Ready for QA to Pass

Pass, except it doesn’t start under torsocks => merged to ease future work on this anyway.

#34 Updated by intrigeri 2015-05-16 10:13:34

related to ~~Bug #9412~~: Add SOCKS support to WhisperBack added

#35 Updated by sajolida 2015-08-05 10:32:21

related to #8668 added

#36 Updated by sajolida 2015-08-06 07:30:29

related to deleted (~~#8668~~)

#37 Updated by sajolida 2015-08-06 07:30:36

blocks #8668 added

#38 Updated by romeopapa 2015-08-10 04:59:51

On the latest jessie build, I’m getting the following error when trying to send a report:

Exception in thread Thread-6:
Traceback (most recent call last):
File “/usr/lib/python3.4/threading.py”, line 920, in _bootstrap_inner
self.run()
File “/usr/lib/python3.4/threading.py”, line 868, in run
self._target(self._args,*self._kwargs)
File “/usr/lib/python3/dist-packages/whisperBack/whisperback.py”, line 193, in save_exception
func(*args)
File “/usr/lib/python3/dist-packages/whisperBack/mail.py”, line 48, in send_message_tls
ssl_context = ssl.create_defaut_context(purpose=ssl.Purpose.SERVER_AUTH,
AttributeError: ‘module’ object has no attribute ‘create_defaut_context’

Python3 on latest jessie build is Python 3.4.2

#39 Updated by intrigeri 2015-08-11 02:01:40

romeopapa wrote:
> On the latest jessie build, I’m getting the following error when trying to send a report:

Confirmed. That’s a typo, hopefully fixed (108fdd1), untested, pushed to the topic branch. This makes me “slightly” doubtful that this code path was ever tested..

#40 Updated by intrigeri 2015-08-11 02:12:02

intrigeri wrote:
> romeopapa wrote:
> > On the latest jessie build, I’m getting the following error when trying to send a report:
>
> Confirmed. That’s a typo, hopefully fixed (108fdd1), untested, pushed to the topic branch. This makes me “slightly” doubtful that this code path was ever tested..

Uploaded 1.6.29+jessie.1 to the feature-jessie APT suite => next builds of feature/jessie should have the fix.

#41 Updated by romeopapa 2015-08-11 08:08:23

OK seems to work, I’ve been able to successfully send a whisperback report. Nice catch on the typo, I really hadn’t noticed it!

#42 Updated by alant 2015-08-17 06:44:25

Worked for me too.

#43 Updated by intrigeri 2015-11-18 07:20:10

related to ~~Bug #10577~~: WhisperBack does not start on Jessie with French locales added