Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3474 closed defect (fixed)

obfsproxy managed proxies support

Reported by: asn Owned by: asn
Priority: Medium Milestone:
Component: Archived/Obfsproxy Version:
Severity: Keywords:
Cc: zwol Actual Points:
Parent ID: #3591 Points:
Reviewer: Sponsor:

Description

obfsproxy should support the managed proxies protocol defined in 180-pluggable-transport.txt

Child Tickets

Change History (9)

comment:1 Changed 8 years ago by asn

Okay, I pushed bug3474_prealpha in my obfsproxy repository [1].

I don't like the code that much, but I'm pushing it so that someone can test it with the tor part (#3472) that I pushed some minutes ago.

It implements most of the 180 spec.

[1]: git://gitorious.org/obfsproxy/obfsproxy.git

[I adopted the "0 on success, -1 on fail" return convention on this one (look at #3551)]

comment:2 Changed 8 years ago by asn

Parent ID: #3591

comment:3 Changed 8 years ago by nickm

In your obfs_strtok_r macro, you need to remember to parenthesize macro arguments when you're putting them in the macro definition.

In b5ed23e you left in a pointless "disable all optimization" change to Makefile.am

In b5ed23e also you move a big function,so I can't tell if you also change it. Please, when moving lots of code, do a commit that ONLY moves the code.

comment:4 Changed 8 years ago by nickm

Why have a separate notion of managed_proxy_params_t ? Why not have one configuration data structure that gets initialized from the command line if we're not in managed mode, and from the environment if we are?

comment:5 in reply to:  4 Changed 8 years ago by asn

Replying to nickm:

Why have a separate notion of managed_proxy_params_t ? Why not have one configuration data structure that gets initialized from the command line if we're not in managed mode, and from the environment if we are?

I'm still stuck on thinking on this.

I'm going to rebuild my branch to be based on Zack's more-protocols branch.
Zack's more-protocols branch introduces the config_t structure (instead of protocol_params_t), which contains the protocol_vtable and arbitrary hidden protocol-specific data.

Nick, this means that the stateful proxy-to-transport structure, you proposed in IRC some weeks ago, can't happen, since the transport structure is hidden in the abstraction of config_t.

Zooming out a bit, ideally, I would like the module writer to not care about managed or external proxies at all. I would like the module writer to define a (*config_create) which would do it for both managed and external proxies. The problem is that I can't find a nice way to do this.

Specifically, in the case of external proxies the whole argv slice has to be passed to the protocol. In the case of managed proxies, at least bindaddr and ORPort have to be passed to the protocol, so that it fills its *_params_t accordingly.
I can't see how separate config_create_managed() and config_create_external() can be avoided [0].

Additionally, an obfsproxy_params_t (or something) might be needed in both cases, so as to contain global obfsproxy information like the location to store transports state, tor's extended OR port, etc.

[0]:
config_t *(*config_create_external)(int n_options, const char *const *options);
config_t *(*config_create_managed)(int is_server, char *bindaddr, char *orport);
... or something.

comment:6 Changed 8 years ago by Sebastian

Cc: zwol added

comment:7 Changed 8 years ago by asn

Status: newneeds_review

Alright this is review/merge ready, please check branch '''bug3656''' @ git://gitorious.org/obfsproxy/obfsproxy.git.

comment:8 Changed 8 years ago by asn

Resolution: fixed
Status: needs_reviewclosed

merged

comment:9 Changed 8 years ago by arma

Component: Pluggable transportObfsproxy
Note: See TracTickets for help on using tickets.