Opened 6 years ago

Closed 6 years ago

#11558 closed defect (fixed)

obfsproxy breaks with old versions of twisted

Reported by: yawning Owned by: asn
Priority: Medium Milestone:
Component: Archived/Obfsproxy Version:
Severity: Keywords: debian, twisted, ancient
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Debian apparently packages/ships Twisted 12.0.0 (presumably with patches), and I wrote a bunch of the recent code targeting 13.2.0.

Per the jenkins output:

  • twisted.internet.error.UnsupportedAddressFamily is missing, requires Twisted 12.1.0.
  • twisted.internet.endpoints.HostNameEndpoint is missing, requires Twisted 13.2.0.

Unrelated to this, the jenkins setup is also choking on missing pyyaml.

Thanks to weasel who brought it to my attention.

Child Tickets

Change History (8)

comment:1 Changed 6 years ago by asn

FWIW, Debian seems to be installing Twisted < 13.2 in stable and oldstable:
https://packages.debian.org/search?keywords=twisted

comment:2 Changed 6 years ago by yawning

Status: newneeds_review

Branch bug11558 in my repository has a fix for this. I tested it by installing Twisted-12.0.0 for pypi (I checked both before and after).

comment:3 Changed 6 years ago by yawning

Updated my branch to deal with Really Old Zope. Unfortunately this still isn't a full solution for people on Debian oldstable because txsocksx uses the same class decorator that I did.

Per the zope.interface documentation, support for the way that I was using the decorator was introduced in 3.6.0 (2010-04-29). Oldstable ships 3.5.3 (2009-12-08). The syntax I switched to is actually deprecated as of 4.0.0 (2012-05-16).

As it stands now, obfsproxy using the distribution provided zope/twisted packages will work on Debian stable and above. Making oldstable work would probably require selectively disabling the external socks5 support, but I'm not sure what the best way to do that is (catch the exception, and assume that it's a "your zope is ancient" issue?).

comment:4 Changed 6 years ago by asn

Please see my branch bug11558_optional_deps for an alternative approach.

The idea was to treat txsocksx as an optional dependency, that is imported only when proxies are used.
Also, if proxies are used but there is no txsocksx or twisted is outdated, then we throw an exception (or a PROXY-ERROR message).

I wanted to try this logic, because I'm not sure if kludgy code to make us compatible with ancient versions of twisted/zope is going to end well (for example, as Yawning said, his new patch still doesn't work in my Debian oldstable). Also, I don't like code in __init__.py.

Also, it's nice to not add new dependencies (since server-side obfsproxy will never need txsocksx anyway).

My branch is not ready (I still need to switch txsocksx to be an optional dependency), and I'm not sure if I actually like this approach.

comment:5 Changed 6 years ago by yawning

Hmm even if you make txsocksx optional there still needs to be code for handling Twisted versions that don't have twisted.internet.error.UnsupportedAddressFamily. Instead of in __init__.py this could be done in socks5.py since that's the only place that uses that exception.

Since it's just error handling, we could also change the code to sending a SOCKS5 General Failure error for EAFNOSUPPORT, but that seems less optimal for people that do have recent versions of Twisted.

comment:6 in reply to:  5 ; Changed 6 years ago by asn

Replying to yawning:

Hmm even if you make txsocksx optional there still needs to be code for handling Twisted versions that don't have twisted.internet.error.UnsupportedAddressFamily. Instead of in __init__.py this could be done in socks5.py since that's the only place that uses that exception.

Since it's just error handling, we could also change the code to sending a SOCKS5 General Failure error for EAFNOSUPPORT, but that seems less optimal for people that do have recent versions of Twisted.

Good point. That's a problem with the SOCKS server not the SOCKS client anymore.

Please check my branch bug11558_optional_deps again. I'm planning to test that branch later today or tomorrow.

comment:7 in reply to:  6 Changed 6 years ago by asn

Replying to asn:

Replying to yawning:

Hmm even if you make txsocksx optional there still needs to be code for handling Twisted versions that don't have twisted.internet.error.UnsupportedAddressFamily. Instead of in __init__.py this could be done in socks5.py since that's the only place that uses that exception.

Since it's just error handling, we could also change the code to sending a SOCKS5 General Failure error for EAFNOSUPPORT, but that seems less optimal for people that do have recent versions of Twisted.

Good point. That's a problem with the SOCKS server not the SOCKS client anymore.

Please check my branch bug11558_optional_deps again. I'm planning to test that branch later today or tomorrow.

UnsupportedAddressFamily test worked fine.

I started up Tor with obfsproxy in an oldstable system, and tried to connect to a non-existent bridge.
Without d7d7570 I get:
exceptions.AttributeError: 'module' object has no attribute 'UnsupportedAddressFamily'
With d7d7570, it seems to work fine.

comment:8 Changed 6 years ago by asn

Resolution: fixed
Status: needs_reviewclosed

Merged.

I also added txsocksx in the extras_require (optional deps), and moved it from being a mandatory dependency.

Note: See TracTickets for help on using tickets.