Feature #12226

Initial review of Tails Server implementation

Added by anonym 2017-02-13 20:12:13 . Updated 2019-07-18 09:16:35 .

Status:
In Progress
Priority:
Normal
Assignee:
Category:
Target version:
Start date:
2017-02-13
Due date:
% Done:

20%

Feature Branch:
Type of work:
Code
Blueprint:

Starter:
Affected tool:
Server
Deliverable for:

Description

Subtasks

History

#1 Updated by segfault 2017-02-14 15:46:30

#2 Updated by anonym 2017-03-05 16:23:20

  • Target version changed from Tails_2.11 to Tails_2.12

#3 Updated by anonym 2017-04-19 17:37:27

  • Target version changed from Tails_2.12 to Tails_3.0

#4 Updated by segfault 2017-04-25 19:33:06

#5 Updated by segfault 2017-04-25 19:33:20

#6 Updated by anonym 2017-04-27 14:24:48

  • Status changed from Confirmed to In Progress
  • % Done changed from 0 to 20

Currently Tails Server always starts the service before generating the onion address. Some services need the onion address for their configuration, so the order we do this (start the service, then generate the onion) won’t working then. We need to either:

  • Separate the onion address generation part into a step of its own that we run before starting the service.
  • Introduce an option that starts the service after the onion address has been generated and published.
  • Simply switch the order so the service always is starter after the onion has been published. I don’t really see a reason why this isn’t the case. Was there a reason for the current order?
# XXX: The connection string is user controlled, but because subprocess
# handles escaping and quoting of arguments, this should still be secure.

The way you invoke Popen means exec() will be used, not the shell, so there is no escaping and quoting to worry about. I.e. you can kill this comment.

sudo -u "$RUN_AS_USER" /usr/local/sbin/tails-server $@

You’ll want to quote $@ to retain the quoting of the parameters for the wrapped application.

@is_installed.setter
def is_installed(self, value):

It feels odd to have a setter with the is_-prefix. Not a blocker.

#7 Updated by anonym 2017-06-01 14:36:32

  • Target version changed from Tails_3.0 to Tails_3.1

#8 Updated by anonym 2017-08-04 10:18:01

  • Target version changed from Tails_3.1 to Tails_3.2

#9 Updated by intrigeri 2017-09-07 08:10:54

  • Affected tool set to Server

#10 Updated by intrigeri 2017-09-07 08:12:46

  • Target version changed from Tails_3.2 to Tails_3.3

#11 Updated by intrigeri 2017-11-06 15:43:31

  • Target version changed from Tails_3.3 to Tails_3.5

#12 Updated by intrigeri 2017-12-07 13:05:02

  • Target version changed from Tails_3.5 to Tails_3.6

#13 Updated by anonym 2018-03-06 14:19:47

I have started reviewing by committing my comments/suggested changes straight into Git in the anonym/review branches at:

#14 Updated by bertagaz 2018-03-14 11:32:06

  • Target version changed from Tails_3.6 to Tails_3.7

#15 Updated by intrigeri 2018-04-13 11:34:56

  • Target version deleted (Tails_3.7)

#16 Updated by intrigeri 2018-08-19 10:13:23

  • Assignee changed from anonym to segfault
  • QA Check set to Info Needed

At this point I think we should treat this review just like anything else => please ensure all the info for the review (pointers to the code, design, etc.) is on this ticket and the branch builds and works fine, and then I’ll happily find you a reviewer :)

#17 Updated by intrigeri 2019-06-02 15:29:30

  • QA Check deleted (Info Needed)

#18 Updated by segfault 2019-07-18 09:16:35

  • Assignee deleted (segfault)