Project

General

Profile

Feature #7873

Audit OnionShare

Added by u about 5 years ago. Updated over 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Start date:
Due date:
% Done:

100%

Feature Branch:
Type of work:
Security Audit
Blueprint:
Starter:
Affected tool:

Description

Before including it into Tails..

bandit.txt View (5.56 KB) hybridwipe, 09/07/2015 04:06 PM


Related issues

Blocks Tails - Feature #7870: Include OnionShare Resolved 12/07/2016
Blocked by Tails - Bug #8464: Wait for problems identified in initial audit of OnionShare to be fixed upstream Resolved 12/19/2014

History

#1 Updated by u about 5 years ago

Before/if.

#2 Updated by u about 5 years ago

  • Parent task deleted (#7870)

#3 Updated by u about 5 years ago

#4 Updated by sajolida about 5 years ago

  • Tracker changed from Bug to Feature
  • Status changed from New to Confirmed

#5 Updated by jvoisin almost 5 years ago

  • % Done changed from 0 to 20

I took a quick glance at the code, and sent some comments to micah.
It looks great so far.

#6 Updated by BitingBird almost 5 years ago

  • Status changed from Confirmed to In Progress

#7 Updated by u almost 5 years ago

<333

#8 Updated by jvoisin almost 5 years ago

I finished a quick audit, and I would recommend to not include OnionShare into Tails, for now.
But, I'll be happy to audit it (more in depth) again once some worrying issues are fixed, and a clear design documented drafted/published.

I'd like to emphasize the fact that this software is really well written, with safety in mind.

#9 Updated by u almost 5 years ago

  • Priority changed from Normal to Low

Thanks for doing this!

Will you keep track of changes in the code base?

I am changing the priority of this ticket to low until something happens on the upstream side of things.

#10 Updated by intrigeri almost 5 years ago

I am changing the priority of this ticket to low until something happens on the upstream side of things.

I don't think that low priority means what you want here. We have way better means to indicate that we're waiting for something to happen, e.g. create a subtask called "Wait for problems identified in initial audit of OnionShare to be fixed upstream", with type of work = Wait.

#11 Updated by u almost 5 years ago

  • Blocked by Bug #8464: Wait for problems identified in initial audit of OnionShare to be fixed upstream added

#12 Updated by u almost 5 years ago

  • Priority changed from Low to Normal

#13 Updated by BitingBird almost 5 years ago

  • Assignee deleted (jvoisin)

#14 Updated by BitingBird over 4 years ago

Code hasn't been updated yet.

#15 Updated by anarcat over 4 years ago

jvoisin wrote:

I finished a quick audit, and I would recommend to not include OnionShare into Tails, for now.
But, I'll be happy to audit it (more in depth) again once some worrying issues are fixed, and a clear design documented drafted/published.

I'd like to emphasize the fact that this software is really well written, with safety in mind.

This was 5 months ago. I don't see anything in the upstream issue tracker, nor any details here about specific problems with onionshare, which is already available in Debian.

Why isn't it part of tails? It seems like useful software.

I think responsible disclosure here implies sharing those issues with the public at some point, if upstream doesn't do anything (assuming that upstream was notified!).

#16 Updated by micahflee over 4 years ago

wrote:

This was 5 months ago. I don't see anything in the upstream issue tracker, nor any details here about specific problems with onionshare, which is already available in Debian.

Why isn't it part of tails? It seems like useful software.

I think responsible disclosure here implies sharing those issues with the public at some point, if upstream doesn't do anything (assuming that upstream was notified!).

Hey, upstream developer here! The issues that jvoisin found were all very minor, and I've simply been swamped and haven't fixed them upstream yet. I'm finally digging out some time to work on some onionshare bugfixes and release version 0.7, which will include all of the issues that jvoisin flagged. Hopefully I'll have 0.7 released next week, at which point I think it would be good to move forward with getting it in Tails.

Here's what I want to fix for 0.7:
https://github.com/micahflee/onionshare/milestones/0.7

These are the issues jvoisin found:
https://github.com/micahflee/onionshare/issues/168
https://github.com/micahflee/onionshare/issues/169
https://github.com/micahflee/onionshare/issues/170

In short, nothing serious at all. Although onionshare actually has a Tails-specific bug that started with a recent release of Tails:
https://github.com/micahflee/onionshare/issues/179

So I want to fix that for 0.7 as well.

#17 Updated by intrigeri over 4 years ago

Why isn't it part of tails?

Once the potential security issues are fixed, the integration / user story will need to be thought through. But that's for another ticket.

#18 Updated by intrigeri over 4 years ago

Although onionshare actually has a Tails-specific bug that started with a recent release of Tails: [...]

That doesn't look Tails-specific to me, but let's discuss that on tails-dev@ rather than on an unrelated ticket :)

#19 Updated by micahflee over 4 years ago

I just released OnionShare 0.7, and all of the issues related to the audit have now been resolved. So I think this issue is actually done.

However, OnionShare in Tails is now blocked by a new issue:
https://github.com/micahflee/onionshare/issues/179
https://trac.torproject.org/projects/tor/ticket/16106

#20 Updated by intrigeri over 4 years ago

  • Assignee set to micahflee
  • QA Check set to Info Needed

I just released OnionShare 0.7, and all of the issues related to the audit have now been resolved.

Great, thanks! The results of this initial audit are being tracked by #8464, so I've asked jvoisin to confirm your claims there.

So I think this issue is actually done.

Not exactly: after his initial audit, jvoisin wrote "I'll be happy to audit it (more in depth) again once some worrying issues are fixed, and a clear design documented drafted/published", so the next action on this ticket is blocked by:

  • #8464 (as said above, and as expressed by the relationship between these two tickets already);
  • the lack of some design doc: is there any? I could not find it.

#21 Updated by micahflee over 4 years ago

There isn't a design document yet, but I opened an issue to write one awhile ago: https://github.com/micahflee/onionshare/issues/167

I'll get on that.

#22 Updated by micahflee over 4 years ago

I've written a security design document: https://github.com/micahflee/onionshare/blob/master/SECURITY.md

It will be included with the source code in the next release.

#23 Updated by BitingBird over 4 years ago

Wouah, bonus points for the design doc being human-readable, congrats!

#24 Updated by intrigeri over 4 years ago

  • Assignee changed from micahflee to jvoisin
  • QA Check changed from Info Needed to Dev Needed

I've written a security design document: https://github.com/micahflee/onionshare/blob/master/SECURITY.md

Yay :) => reassigning to jvoisin for the next auditing steps he wanted to do.

#25 Updated by hybridwipe about 4 years ago

I ran the bandit static analyzer on it, thought I'd share it here.

#26 Updated by bertagaz about 4 years ago

Thanks a lot! It seems to raise interesting issues, in particular the predictable tmp directory usage. Did you send it upstream?

#27 Updated by hybridwipe about 4 years ago

bertagaz wrote:

Thanks a lot! It seems to raise interesting issues, in particular the predictable tmp directory usage. Did you send it upstream?

I have not, please cc me if you do.

#28 Updated by bertagaz about 4 years ago

  • Assignee changed from jvoisin to micahflee

Assigning to micah then, for him to have a look at it. Please set the assignee back to jvoisin once done.

#29 Updated by intrigeri almost 4 years ago

  • Assignee changed from micahflee to jvoisin
  • % Done changed from 0 to 30

intrigeri wrote:

I've written a security design document: https://github.com/micahflee/onionshare/blob/master/SECURITY.md

Yay :) => reassigning to jvoisin for the next auditing steps he wanted to do.

@jvoisin: what's the status on this one? Is there any remaining issue you have identified, and that is not fixed yet? If so, it would be awesome to list them here / point to GitHub tickets, except of course in cases when less rapid disclosure is preferred :)

@bertagaz + @hybridwipe: this ticket is about us auditing OnionShare, to make sure it satisfies our own security requirements. The idea is to the upstream author to specific problems, if we can find any. I don't think it's fair to send him the raw output of static code analyzers, and expect he'll analyze the output himself: he never committed to do any such thing, while we did.

#30 Updated by jvoisin almost 4 years ago

It seems that onionshare is under heavy refactoring to use ephemeral hidden services. There is little point in doing a security audit now.

#31 Updated by intrigeri over 3 years ago

  • Type of work changed from Audit to Security Audit

#32 Updated by jvoisin over 3 years ago

  • % Done changed from 30 to 100
  • QA Check deleted (Dev Needed)

I just did a review of the code, and found nothing alarming.
Great job michaflee!

#33 Updated by intrigeri over 3 years ago

  • Status changed from In Progress to Resolved
  • Assignee deleted (jvoisin)

Thanks!

Also available in: Atom PDF