Bug #15061

Document the relevant security aspects of the Tails Verification / Installation Assistant

Added by sajolida 2017-12-14 16:04:37 . Updated 2018-03-27 14:55:10 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Installation
Target version:
Start date:
2017-12-14
Due date:
% Done:

100%

Feature Branch:
contrib/15061-document-security-of-verification-extension
Type of work:
Contributors documentation
Blueprint:

Starter:
Affected tool:
Verification Extension
Deliverable for:

Description

https://mailman.boum.org/pipermail/tails-dev/2017-November/011916.html

« Once we’re done here, I’d like to see the security reasoning that
leads to the “it’s safe” conclusion documented somewhere in the
extension Git repo: we want our code to be friendly to auditors, and
mailing list archive are not an obvious place to find such doc.
@sajolida: I volunteer to take care of this if Uzair does not. »


Subtasks


Related issues

Related to Tails - Feature #14815: Update design doc to DAVE 2 Resolved 2017-10-09

History

#1 Updated by sajolida 2017-12-14 18:59:22

  • Feature Branch set to web/10640-assistant-design-doc

This could be done in the “Security inside the browser” section of /contribute/design/verification_extension.mdwn in web/10640-assistant-design-doc.

#2 Updated by sajolida 2017-12-17 16:34:38

  • Assignee changed from sajolida to intrigeri

I think you should do it :(

#3 Updated by intrigeri 2017-12-18 06:31:10

  • Target version set to Tails_3.5

#4 Updated by intrigeri 2017-12-22 08:41:13

  • Assignee changed from intrigeri to sajolida
  • QA Check set to Info Needed

sajolida wrote:
> This could be done in the “Security inside the browser” section of /contribute/design/verification_extension.mdwn in web/10640-assistant-design-doc.

Sure, I can give it a try even though I don’t really feel qualified.

Now it’s not clear to me where I should start from and when. After reading the “JavaScript security review for new verification extension” thread I see that:

  • You created a bunch of tickets about fixing various security-relevant issues → should I document the current state of things including these issues, or should I wait for them to be fixed? I’m concerned that if I do the former, it’ll be a waste of time because later we’ll have to update the doc I’ll have written to drop some text.
  • You wrote that you wanted to “finish understanding better these issues” before tasking other people with them: I guess this was done or is not relevant anymore, right?

#5 Updated by intrigeri 2017-12-22 08:42:06

  • Subject changed from Document the relevant security aspects of our code in the Git repo to Document the relevant security aspects of the Tails Verification / Installation Assistant

#6 Updated by sajolida 2017-12-22 17:51:21

  • QA Check deleted (Info Needed)

Yeah, we’re not in a hurry regarding that ticket so let’s wait until the code is finished before documenting what it does :)

I’ll keep this ticket assigned to me and reassign it to you once I think we’re ready.

#7 Updated by sajolida 2017-12-22 20:08:45

#8 Updated by anonym 2018-01-23 19:52:41

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

#9 Updated by sajolida 2018-03-02 15:37:14

  • Assignee changed from sajolida to intrigeri

Now all the issues raised by co6 are solved. I think that the only thing that is worth documenting is the messaging between the extension and the page.

Since you worked on the code Uzair removed the listening code on the extension side with 3a259b0.

Tell me if you still want to do that, otherwise I can do it myself.

#10 Updated by intrigeri 2018-03-03 17:36:06

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

#11 Updated by sajolida 2018-03-22 10:28:24

  • Assignee changed from intrigeri to sajolida

I’ll write a draft right now.

#12 Updated by sajolida 2018-03-22 11:43:30

  • Assignee changed from sajolida to intrigeri
  • QA Check set to Ready for QA
  • Feature Branch changed from web/10640-assistant-design-doc to contrib/15061-document-security-of-verification-extension

#13 Updated by intrigeri 2018-03-26 08:05:39

  • Status changed from Confirmed to In Progress
  • Assignee changed from intrigeri to sajolida
  • % Done changed from 0 to 50
  • QA Check changed from Ready for QA to Dev Needed

I think we should document this check:

  function receiveMessage(event) {
    if (event.source !== window

… that seems to be important security-wise; see discussion about it around https://mailman.boum.org/pipermail/tails-dev/2017-November/011916.html.

If the trade-off we made wrt. readAsBinaryString (Bug #15058) has any security implication, it should be mentioned.

Other than that, looks good to me!

#14 Updated by sajolida 2018-03-26 17:44:25

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

Thanks for the quick review!

I pushed a few more commits (up to f60a058ff1). It should be good now.

The trade-off with readAsBinaryString has no security implications. The only thing is that it’s not standard and might not work on all browsers, but it’s not even deprecated according to Mozilla.

#15 Updated by intrigeri 2018-03-27 14:52:51

  • % Done changed from 50 to 100
  • QA Check changed from Ready for QA to Pass

#16 Updated by intrigeri 2018-03-27 14:54:45

  • Status changed from In Progress to Resolved

Applied in changeset commit:7c9c179933ad71c7d38aa1c41440e2d808b97063.

#17 Updated by intrigeri 2018-03-27 14:55:10

  • Assignee deleted (intrigeri)