Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#3275 closed defect (fixed)

obfsproxy's SOCKS server should support more SOCKS failure replies

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

Description

The SOCKS5 standard defines:

o REP Reply field:

o X'00' succeeded
o X'01' general SOCKS server failure
o X'02' connection not allowed by ruleset
o X'03' Network unreachable
o X'04' Host unreachable
o X'05' Connection refused
o X'06' TTL expired
o X'07' Command not supported
o X'08' Address type not supported
o X'09' to X'FF' unassigned

our obfsproxy src/socks.c currently only supports X'00' and X'01'.
We should add support for the rest.

Child Tickets

Change History (6)

comment:1 Changed 9 years ago by asn

Status: newneeds_review

Check bug3275 branch in my obfsproxy repo.

I'm quite happy with the code, the only thing that bothers me is that I'm not sure if errno(3) stuff is cross-platform.

comment:2 Changed 9 years ago by nickm

The errno stuff isn't quite the same on Windows. There, you'll have to use the WSA* error codes listed in http://msdn.microsoft.com/en-us/library/ms740668(v=vs.85).aspx

You can define a macro to make this easier:

#ifdef WINDOWS
#define ERR(e) WSA##e
#else
#define ERR(e) e
#endif

and then say

 case ERR(EHOSTUNREACH): ... 

and so on.

Maybe the return values of socks5_handle_request want to turn into an enum? Having magic meanings for -2...1 seems like the reason enum was invented.

Instead of EVUTIL_SOCKET_ERROR(), you might want evutil_socket_geterror(): it can be more accurate about connect failures on windows.

Other than that, looks good!

comment:3 Changed 9 years ago by asn

Pushed fixes to your comments.

comment:4 Changed 9 years ago by asn

Windows code is untested.

comment:5 Changed 9 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

looks good; merging with a small patch.

comment:6 Changed 8 years ago by arma

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