Opened 6 years ago

Closed 6 years ago

#9815 closed enhancement (fixed)

Pluggable transports should learn Tor's data directory

Reported by: phw Owned by: asn
Priority: Medium Milestone:
Component: Archived/Obfsproxy Version:
Severity: Keywords: pluggable transport, state location, persistent information
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by phw)

At least ScrambleSuit needs to store persistent information and Tor's data directory would be the best place to do that. Right now, however, pluggable transports have no way of learning the data directory.

A possible fix for that is in the branch pass_datadir (which itself is a branch of bug8979_draft) in:
https://github.com/NullHypothesis/obfsproxy

The patches pass the state location (Tor's data directory in managed mode) to the pluggable transport's constructor. For external mode, the patch adds another command line argument to pyobfsproxy.py which allows the user to do that manually.

On the plus side, it's an easy fix. On the minus side, it requires changing existing transports (even though it's just the constructor's method head). An alternative would be yet another method, transports would have to implement?

Please let me know if the patches make sense and if I missed anything. I could restructure it if you don't like the constructor way. Either way, it worked in my tests.

Child Tickets

Change History (9)

comment:1 Changed 6 years ago by phw

Status: newneeds_revision

comment:2 Changed 6 years ago by phw

Description: modified (diff)

comment:3 Changed 6 years ago by asn

Code looks good after review. I should merge soon after it gets some more testing from phw.

comment:4 Changed 6 years ago by asn

Hey phw,

following yesterday's discussion, and assuming that we will proceed with the construction-based approach, would you be interested in writing that TransportConfig (or whatever) class so that we can pass more options to transports in a smooth way? We can then refactor my #8979 patch to use the TransportConfig class too (it's currently using a method-based approach).

comment:5 in reply to:  4 Changed 6 years ago by phw

Replying to asn:

following yesterday's discussion, and assuming that we will proceed with the construction-based approach, would you be interested in writing that TransportConfig (or whatever) class so that we can pass more options to transports in a smooth way? We can then refactor my #8979 patch to use the TransportConfig class too (it's currently using a method-based approach).

Personally, I like the constructor-based approach as long as we have a TransportConfig. I'll come up with a patch for it.

comment:6 Changed 6 years ago by phw

I just committed an implementation of TransportConfig on top of the pass_datadir branch in my GitHub repository. At this point it's a simple container class with getters and setters. It should be easy to extend.

comment:7 Changed 6 years ago by asn

Hi phw,

new code (TransportConfig) looks good!

Two things:
a) Please don't base it on bf6522898bdab114db8863ecec3002813f8bb3f7. I should rewrite that commit to use the new TransportConfig class instead.
b) Can you also port the obfs2/obfs3 code to this new API? I guess the only change needed is to alter their constructor definition to accept the new argument.

comment:8 in reply to:  7 Changed 6 years ago by phw

Replying to asn:

Two things:
a) Please don't base it on bf6522898bdab114db8863ecec3002813f8bb3f7. I should rewrite that commit to use the new TransportConfig class instead.
b) Can you also port the obfs2/obfs3 code to this new API? I guess the only change needed is to alter their constructor definition to accept the new argument.

Thanks for having a look. It's now based on master and the constructors of dummy, b64, obfs2 and obfs3 are updated to match the new API. I pushed a new branch with all the changes to: https://github.com/NullHypothesis/obfsproxy/tree/bug_9815

comment:9 Changed 6 years ago by asn

Resolution: fixed
Status: needs_revisionclosed

Ding ding. Merged! Thanks for all the code. I need to fix #8979 now.

Note: See TracTickets for help on using tickets.