Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#11629 closed defect (implemented)

FTE breaks with latest obfsproxy (because of move to SOCKS5)

Reported by: asn Owned by: kpdyer
Priority: Medium Milestone:
Component: Obfuscation/FTE Version:
Severity: Keywords: MikePerry201405R
Cc: kpdyer, yawning Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Hello,

Arturo was trying to setup FTE in his OONI machine today. In his attempts, FTE would crash when Tor tried to spawn it.

Here is what Arturo told me during his debugging session:

16:29 < asn> 16:07 < hellais> I get a useful error message
16:29 < asn> 16:08 < hellais> AttributeError: 'module' object has no attribute 'SOCKSv4Factory'
16:29 < asn> 16:08 < hellais> could it be that I have an outdated version of obfsproxy?
16:29 < asn> 16:11 < hellais> oh no, I have a too new version of obfsproxy
16:29 < asn> 16:12 < hellais> I think that obfsproxy.network.socks.OBFSSOCKSv4Factor used to be called obfsproxy.network.socks.SOCKSv4Factor
16:29 < asn> 16:13 < hellais> actually all the SOCKSv4 stuff got removed in the last version of obfsproxy wereas they used to be present
16:29 < asn> 16:13 < hellais> so users of the old API will break
16:29 < asn> 16:14 < hellais> I would suggest you add into the obfsproxy.network.socks a subclass of OBFSSOCKSv5Factory called SOCKSv5Factory that writes a warning log stating that it's now deprecated

We recently moved from SOCKS4 to SOCSK5 on obfsproxy, and I forgot that FTE was borrowing code from obfsproxy, so we accidentally broke FTE. Sorry for that.

kpdyer, how would you like to fix this?

Child Tickets

Change History (18)

comment:1 Changed 4 years ago by kpdyer

Ideally, fteproxy should not have an external dependency upon obfsproxy.

As a short term solution I can include obfsproxy (internally) in fteproxy. Longer term I can resolve this issue within fteproxy.

asn - What's the best way to avoid obfsproxy-dependent code like:

https://github.com/kpdyer/fteproxy/blob/master/bin/fteproxy#L225
https://github.com/kpdyer/fteproxy/blob/master/bin/fteproxy#L275
https://github.com/kpdyer/fteproxy/blob/master/fteproxy/__init__.py#L391
and
https://github.com/kpdyer/fteproxy/blob/master/fteproxy/__init__.py#L471

I assume that there is a proper way to use pyptlib and my implementation is just a cheap hack?

-Kevin

comment:2 Changed 4 years ago by yawning

Status: newneeds_review

comment:3 in reply to:  1 ; Changed 4 years ago by yawning

Replying to kpdyer:

I assume that there is a proper way to use pyptlib and my implementation is just a cheap hack?

There's two correct ways to use it.

One would be to write your own networking code to provide the infrastructure being pulled in from obfsproxy (The equivalent of the obfsproxy.transports.base.BaseTransport code).

The other would be to do what ScrambleSuit did and use obfsproxy for everything (See #11354). This will probably be easier from a development standpoint, but there is a licensing conflict since all the FTE code is GPLed, and I'm not sure how asn feels about adding a dependency to libfte that includes native code to obfsproxy.

comment:4 in reply to:  3 ; Changed 4 years ago by kpdyer

Replying to yawning:

Replying to kpdyer:

I assume that there is a proper way to use pyptlib and my implementation is just a cheap hack?

There's two correct ways to use it.

One would be to write your own networking code to provide the infrastructure being pulled in from obfsproxy (The equivalent of the obfsproxy.transports.base.BaseTransport code).

Was hoping to avoid that!

The other would be to do what ScrambleSuit did and use obfsproxy for everything (See #11354). This will probably be easier from a development standpoint, but there is a licensing conflict since all the FTE code is GPLed, and I'm not sure how asn feels about adding a dependency to libfte that includes native code to obfsproxy.

For now, it wouldn't make sense to include FTE in obfsproxy. Predominantly because fteproxy has some C++ that would complicate the obfsproxy deployment process, which is currently pure-python.

-Kevin

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

Replying to kpdyer:

Replying to yawning:

Replying to kpdyer:

I assume that there is a proper way to use pyptlib and my implementation is just a cheap hack?

There's two correct ways to use it.

Yeah, Yawning's suggestions would be the proper ways to fix this.

One would be to write your own networking code to provide the infrastructure being pulled in from obfsproxy (The equivalent of the obfsproxy.transports.base.BaseTransport code).

Was hoping to avoid that!

Yeah... Looking at the FTE code it seems to rely on obfsproxy quite much.

BTW, if you end up writing your own networking code, you can still rely on pyptlib to help you with some PT stuff (like the environment variable parsing, and CMETHOD stuff).

The other would be to do what ScrambleSuit did and use obfsproxy for everything (See #11354). This will probably be easier from a development standpoint, but there is a licensing conflict since all the FTE code is GPLed, and I'm not sure how asn feels about adding a dependency to libfte that includes native code to obfsproxy.

For now, it wouldn't make sense to include FTE in obfsproxy. Predominantly because fteproxy has some C++ that would complicate the obfsproxy deployment process, which is currently pure-python.

Hm, yeah that would complicate the deployment indeed. Still worth looking into I guess.

Also, check out the concept of #11354. If you find a way to fork obfsproxy that works for you, we can just add the fork in the TBB (obfsproxy is not very big so the code duplication will not hurt us much)

$ du -sh Tor/PluggableTransports/obfsproxy*
680K	Tor/PluggableTransports/obfsproxy
4.0K	Tor/PluggableTransports/obfsproxy.bin

comment:6 Changed 4 years ago by kpdyer

Let's leave this ticket open for now.

In the short term I'll pin a specific version of obfsproxy within fteproxy. I'll close this ticket when that's complete.

Long term we'll need a good solution, as I want fteproxy to be compatible with obfsproxy. Some long-term options: (1) I make a pure-python version of FTE that's included in obfsproxy and remove all tor-specific code from fteproxy or (2) we refactor common code in fteproxy/obfsproxy to a new project. It will take a bit of thinking to figure out which is best...

Was Arturo working from 0.2.13 or master?

-Kevin

comment:7 in reply to:  6 Changed 4 years ago by yawning

Replying to kpdyer:

In the short term I'll pin a specific version of obfsproxy within fteproxy. I'll close this ticket when that's complete.

The branch I provided should remove the short term need for this (since it will support both the old and new SOCKS code). If you'd rather pin the version that's fine too, but you'll miss out on IPv6 support.

AFAIK the only thing that could break this in the future is when #11134 gets fixed and when the PT arg passing mess gets cleaned up, but it will be a while before either happens I think.

comment:8 Changed 4 years ago by kpdyer

Ah, apologies Yawning.

https://github.com/Yawning/fteproxy/commit/0f7f68d01b133f7d61bd3ad239af89021af27076

That's a cute solution.

So, I guess there's an option three: leave things how they are, and just be aware this is a problem, but look towards a more stable long-term solution?

-Kevin

comment:9 in reply to:  8 Changed 4 years ago by yawning

Replying to kpdyer:

So, I guess there's an option three: leave things how they are, and just be aware this is a problem, but look towards a more stable long-term solution?

Yep. I'm really sorry about causing this mess in the first place, if I had known that the SOCKS4 code was directly used, I would have left it in instead of removing it when adding the SOCKS5 stuff.

comment:10 Changed 4 years ago by kpdyer

No worries!

comment:11 in reply to:  2 ; Changed 4 years ago by kpdyer

Replying to yawning:

Here, have a branch: https://github.com/Yawning/fteproxy/tree/bug11629

Can you turn this into a pull request so I can push it to kpdyer/fteproxy?

comment:12 in reply to:  11 Changed 4 years ago by yawning

Replying to kpdyer:

Replying to yawning:

Here, have a branch: https://github.com/Yawning/fteproxy/tree/bug11629

Can you turn this into a pull request so I can push it to kpdyer/fteproxy?

Done. (https://github.com/kpdyer/fteproxy/pull/134)

comment:14 Changed 4 years ago by kpdyer

I'll do a 0.2.14 release for fteproxy soon, and we can do a lock-step upgrade to obfsproxy 0.2.8 at the same time.

Will keep this open until 0.2.14 is ready.

comment:15 Changed 4 years ago by asn

Hello, any news with this? I see some 0.2.14 related commits in https://github.com/kpdyer/fteproxy/commits/master but I don't see the release in https://github.com/kpdyer/fteproxy/releases .

We are not in an insane hurry, but I think the TBB guys are thinking of doing another 3.6 release soon and it would be nice if we could have obfsproxy-0.2.8 in it so that we can test it out (and test scramblesuit too).

comment:16 Changed 4 years ago by kpdyer

The main hold up is that I've made a few structural changes to fteproxy under the hood, since 0.2.13. Specifically, I've partitioned the projects into two parts: fteproxy [1] and libfte [2].

All the changes are done and it seems stable, but there is a bit of work on the Tor Browser side to (1) include fte install in the build process and (2) test this change.

Anyways, I just need to make a decision about whether or not I want to include these changes in 0.2.14. In either case, I plan to have a release ready by next week.

-Kevin

[1] https://github.com/kpdyer/fteproxy
[2] https://github.com/kpdyer/libfte

comment:17 Changed 4 years ago by kpdyer

Keywords: MikePerry201405R added
Resolution: implemented
Status: needs_reviewclosed

comment:18 Changed 4 years ago by gk

Squashed and merged as 682f8f4d26d7df1b49a23c40dd7e7ca18f44ab90. I tweaked your changes slightly. Most notably, I used the respective commits instead of your tags due to the latter being not signed. If you start signing your tags let us know and we point to the tags instead and start verifying them. That said: thanks!

Note: See TracTickets for help on using tickets.