Feature #13581

Update AppArmor profile for tor/obfs4proxy

Added by groente 2017-08-04 22:09:17 . Updated 2017-09-05 10:28:24 .

Status:
Resolved
Priority:
Elevated
Assignee:
groente
Category:
Infrastructure
Target version:
Tails_3.2
Start date:
2017-08-04
Due date:
% Done:

100%

Feature Branch:
puppet-lizard-tails:bugfix/13581-apparmor-profile-tor-obfs4proxy
Type of work:
Sysadmin
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867342

Subtasks

Related issues

Related to Tails - Bug #13582: Monitoring bridge Duplicate 2017-08-04
Related to Tails - Bug #14603: Remove custom apparmor profile for tor Resolved 2017-09-05
Blocks Tails - Feature #13233: Core work 2017Q3: Sysadmin (Maintain our already existing services) Resolved 2017-06-29

History

#1 Updated by groente 2017-08-04 22:18:39

  • Assignee changed from groente to intrigeri
  • QA Check set to Ready for QA
  • Feature Branch set to puppet-lizard-tails:bugfix/13581-apparmor-profile-tor-obfs4proxy

Updated the apparmor profile as suggested in the debian bug and included the net_admin capability, as I noticed that coming by while running the profile in complain mode. Commit: 2253a13c4fae0e4089db509e64e37f22b065b998

#2 Updated by bertagaz 2017-08-05 12:09:40

  • Assignee changed from intrigeri to bertagaz

groente wrote:
> Updated the apparmor profile as suggested in the debian bug and included the net_admin capability, as I noticed that coming by while running the profile in complain mode. Commit: 2253a13c4fae0e4089db509e64e37f22b065b998

Taking over, after all I’m supposed to be this shift companion. May be able to do that tomorrow. :)

#3 Updated by intrigeri 2017-08-05 12:34:45

  • blocks Feature #13233: Core work 2017Q3: Sysadmin (Maintain our already existing services) added

#4 Updated by intrigeri 2017-08-05 12:35:16

  • Status changed from New to In Progress

I see no reason why this ticket should be private. Please explain or make it public :)

#5 Updated by intrigeri 2017-08-05 12:36:59

#6 Updated by groente 2017-08-05 12:59:50

I flagged it private because it mentions a private repo, feel free to make it public if you feel this is overzealous.

#7 Updated by intrigeri 2017-08-05 13:39:42

  • Private changed from Yes to No

#8 Updated by bertagaz 2017-08-09 14:29:26

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

#9 Updated by bertagaz 2017-08-12 13:23:14

  • Assignee changed from bertagaz to groente
  • % Done changed from 0 to 20
  • QA Check changed from Ready for QA to Dev Needed

Sorry for the lag, been busy with 3.1. I took time to have a better look at this.

I never noticed we had a custom apparmor profile for Tor on bridge.lizard! Seems to be a leftover we should definitely get rid of and get everything fixed in Debian. Good news is that intrigeri seems to be up on it on the Debian side, so it’s in progress.

groente wrote:
> Updated the apparmor profile as suggested in the debian bug and included the net_admin capability, as I noticed that coming by while running the profile in complain mode. Commit: 2253a13c4fae0e4089db509e64e37f22b065b998

I can’t find such a commit, but I have 06a7a43. About the net_admin cap, I’m not much convinced. That would be another deviation compared to what the next Debian package upload will ship, so we’d have to keep this custom apparmor profile. So either we don’t apply this new cap change, either we report it on the Debian bug so that it gets integrated “upstream”.

Also in the commit I mention, you left the dac_override cap while it’s proposed to be removed in the debian bug tracker, and reported to work fine without. We should probably do that too.

When this two points will be fixed, I think we can merge that and deploy. Do you want to do that yourself? We can set up an appointement so that I’ll be reachable for guidance.

Then maybe we’d better keep this ticket open (or create another one) to remember to remove this custom apparmor profile at the next upgrade.

#10 Updated by intrigeri 2017-08-22 13:52:16

groente wrote:
> I flagged it private because it mentions a private repo, feel free to make it public if you feel this is overzealous.

OK, thanks for being careful by default about it :)

We generally use public tickets for sysadmin unless they contain sensitive/secret info, which thankfully is pretty rare in practice. In this case IMO it’s a bug that this code & files are in our private manifests repo (not sure why I did it this way
initially); there’s no secret involved, so I’ve made this ticket public.

#11 Updated by intrigeri 2017-08-26 06:33:32

Meta: I notice that /etc/apparmor.d/abstractions/tor has been edited manually on bridge.lizard. We don’t do this on our infrastructure (Puppet, infrastructure as code, you know the drill :) Now, fancy theory put aside, I acknowledge there will probably be exceptions in practice until you’re comfortable with Puppet, e.g. if a mission-critical service is broken, you’re on-call, and nobody else is around to Puppet-ize your proposed fix. Let’s keep the scope of these exceptions as small as possible. In the case at hand, our Tor bridge is not critical. This being clarified, are we on the same page? (If not, let’s discuss this on the sysadmins mailing list.)

Besides, /etc/apparmor.d/usr.bin.obfsproxy has been put into complain mode. I have no clue why, since obfsproxy is not even installed on that system. Please revert this change or document why it’s needed + the fact it needs to be Puppet-ized.

#12 Updated by intrigeri 2017-08-26 06:34:35

  • Subject changed from update apparmor profile for tor/obfs4proxy to Update AppArmor profile for tor/obfs4proxy

#13 Updated by groente 2017-09-04 10:44:30

  • Assignee changed from groente to intrigeri
  • QA Check changed from Dev Needed to Ready for QA

The change has been reverted. The two changes proposed by bertagaz have been addressed in commit 1818134ffc0a7fc5ad1c1ecd5b48857ff6e9c7ba. Time to merge?

#14 Updated by intrigeri 2017-09-04 13:13:23

  • Assignee changed from intrigeri to groente
  • % Done changed from 20 to 50
  • QA Check changed from Ready for QA to Dev Needed

groente wrote:
> The change has been reverted. The two changes proposed by bertagaz have been addressed in commit 1818134ffc0a7fc5ad1c1ecd5b48857ff6e9c7ba. Time to merge?

Yes, please go ahead after improving the commit message for 1818134 so that it explains why we do that (instead of merely paraphrasing the diff, which hasn’t a good s/n ratio IMO :)

And then, before closing this ticket we’ll need another one about removing that custom abstration once it’s not needed. I’ll let you check when you think it can be removed, and I’ll review your reasoning & conclusions on that new ticket.

Thanks!

#15 Updated by groente 2017-09-05 09:54:29

  • Status changed from In Progress to Resolved
  • % Done changed from 50 to 100
  • QA Check changed from Dev Needed to Pass

#16 Updated by intrigeri 2017-09-05 10:27:17

  • related to Bug #14603: Remove custom apparmor profile for tor added

#17 Updated by intrigeri 2017-09-05 10:28:24

Yeah :)