Re: MEDIUM: Adding upstream socks4 proxy support
On Thu, Jun 6, 2019 at 12:27 PM Igor Pav wrote: > Sorry to ask a not so related question here, I have a Linux gateway to > redirect user's TCP traffic by using iptables like `iptables -t nat -A > PREROUTING -p tcp dst -j REDIRECT --to-ports 1000`, port 1000 is > redsocks transparent tcp-to-socks proxy, > since we have Alec's patch here, I wonder if that easy to modify the > patch to meet my weird requirement like: > ``` > listen tcps > bind: 1000 transparent > server x.x.x.x:2000 ssl > ``` > x.x.x.x:2000 is a remote socks server with ssl wrap. I want user TCP > traffic from port 1000 to redirect to remote socks server. I'm not so > familiar in haproxy core architecture, can you guys point me which > part in the source should I look at? Hello Igor! This is exactly the same use-case I've proposed (along others that would fit in the same "pattern"), which was spawned into a different thread with the subject: Discussion about "Upstream socks proxy support #82" (Thus I would suggest to switch to that thread.) At the moment, it is still debated if such a feature would be useful, and unfortunately nobody has stepped-up to write a patch. :) Ciprian.
Re: MEDIUM: Adding upstream socks4 proxy support
Hi, Alec, Willy Sorry to ask a not so related question here, I have a Linux gateway to redirect user's TCP traffic by using iptables like `iptables -t nat -A PREROUTING -p tcp dst -j REDIRECT --to-ports 1000`, port 1000 is redsocks transparent tcp-to-socks proxy, since we have Alec's patch here, I wonder if that easy to modify the patch to meet my weird requirement like: ``` listen tcps bind: 1000 transparent server x.x.x.x:2000 ssl ``` x.x.x.x:2000 is a remote socks server with ssl wrap. I want user TCP traffic from port 1000 to redirect to remote socks server. I'm not so familiar in haproxy core architecture, can you guys point me which part in the source should I look at? Thanks! On Mon, Jun 3, 2019 at 5:45 PM Alec Liu wrote: > > Hi Willy, > > Totally understand. > Thank you for the kindness! > > Regards, > Alexander Liu > > On Mon, Jun 3, 2019 at 5:39 PM Willy Tarreau wrote: > > > > On Mon, Jun 03, 2019 at 04:50:57PM +0800, Alec Liu wrote: > > > Hi Willy, > > > > > > Good to know it was fixed. > > > I will try to do some more testing as well. > > > > OK. > > > > > btw: do you mind if I am trying to have the SOCKS4 support backported > > > to 1.9 and 1.8? > > > > We normally don't backport new features into stable versions, as > > experience told us that while tempting, over the long term it causes > > quite some issues. It may exceptionally happen very early after a > > release because something doesn't work as expected or is incomplete, > > but we've long past this stage for both 1.8 and 1.9. However if you > > are willing to keep your own patch and apply it to your local tree, > > and just need some help to review it, I can help here. But I'd still > > recommend you to migrate to 2.0 ASAP, this will be less trouble for > > you than to maintain your patch set. > > > > Cheers, > > Willy >
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy, Totally understand. Thank you for the kindness! Regards, Alexander Liu On Mon, Jun 3, 2019 at 5:39 PM Willy Tarreau wrote: > > On Mon, Jun 03, 2019 at 04:50:57PM +0800, Alec Liu wrote: > > Hi Willy, > > > > Good to know it was fixed. > > I will try to do some more testing as well. > > OK. > > > btw: do you mind if I am trying to have the SOCKS4 support backported > > to 1.9 and 1.8? > > We normally don't backport new features into stable versions, as > experience told us that while tempting, over the long term it causes > quite some issues. It may exceptionally happen very early after a > release because something doesn't work as expected or is incomplete, > but we've long past this stage for both 1.8 and 1.9. However if you > are willing to keep your own patch and apply it to your local tree, > and just need some help to review it, I can help here. But I'd still > recommend you to migrate to 2.0 ASAP, this will be less trouble for > you than to maintain your patch set. > > Cheers, > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Mon, Jun 03, 2019 at 04:50:57PM +0800, Alec Liu wrote: > Hi Willy, > > Good to know it was fixed. > I will try to do some more testing as well. OK. > btw: do you mind if I am trying to have the SOCKS4 support backported > to 1.9 and 1.8? We normally don't backport new features into stable versions, as experience told us that while tempting, over the long term it causes quite some issues. It may exceptionally happen very early after a release because something doesn't work as expected or is incomplete, but we've long past this stage for both 1.8 and 1.9. However if you are willing to keep your own patch and apply it to your local tree, and just need some help to review it, I can help here. But I'd still recommend you to migrate to 2.0 ASAP, this will be less trouble for you than to maintain your patch set. Cheers, Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy, Good to know it was fixed. I will try to do some more testing as well. btw: do you mind if I am trying to have the SOCKS4 support backported to 1.9 and 1.8? Thank you. Regards, Alexander Liu On Mon, Jun 3, 2019 at 2:42 PM Willy Tarreau wrote: > > On Mon, Jun 03, 2019 at 07:19:46AM +0200, Willy Tarreau wrote: > > On Mon, Jun 03, 2019 at 01:40:28AM +0800, Alec Liu wrote: > > > Hi Willy, > > > > > > Your configuration working for me too, but once I have the "mode tcp" > > > changed to "mode http", it was failed. > > > Can you give it a try by changing your testing configuration mode from > > > "mode tcp" to "mode http"? > > > > OK I can now reproduce it this way. And I'm seeing the same problem > > you have regarding epoll_ctl(DEL). I'm anticipating something ugly > > in the lower layers now... > > So as I feared it, it was ugly, but not that much in the end. The problem > stemmed from long ago due to handshake handlers partially dealing for their > events and for others', leaving the connections in a pseudo-random polling > state. And the reason it failed in HTTP mode was because mux-h1 immediately > tries to recv() after a connect(), fails, then subscribes the connection to > polling marking it not ready, then the handshake handlers use this not-ready > mark to stop processing, and did not even enable polling for recv when > leaving, which explains the EPOLL_CTL_DEL you've seen. > > I've fixed the connection issues and also have a working fix for mux-h1 that > I'll ask Christopher confirmation before merging it. > > Now everything works for me, tcp/http, proxy v1/v2/none, with or without > socks4, with or without checks. > > Cheers, > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Mon, Jun 03, 2019 at 07:19:46AM +0200, Willy Tarreau wrote: > On Mon, Jun 03, 2019 at 01:40:28AM +0800, Alec Liu wrote: > > Hi Willy, > > > > Your configuration working for me too, but once I have the "mode tcp" > > changed to "mode http", it was failed. > > Can you give it a try by changing your testing configuration mode from > > "mode tcp" to "mode http"? > > OK I can now reproduce it this way. And I'm seeing the same problem > you have regarding epoll_ctl(DEL). I'm anticipating something ugly > in the lower layers now... So as I feared it, it was ugly, but not that much in the end. The problem stemmed from long ago due to handshake handlers partially dealing for their events and for others', leaving the connections in a pseudo-random polling state. And the reason it failed in HTTP mode was because mux-h1 immediately tries to recv() after a connect(), fails, then subscribes the connection to polling marking it not ready, then the handshake handlers use this not-ready mark to stop processing, and did not even enable polling for recv when leaving, which explains the EPOLL_CTL_DEL you've seen. I've fixed the connection issues and also have a working fix for mux-h1 that I'll ask Christopher confirmation before merging it. Now everything works for me, tcp/http, proxy v1/v2/none, with or without socks4, with or without checks. Cheers, Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Mon, Jun 03, 2019 at 01:40:28AM +0800, Alec Liu wrote: > Hi Willy, > > Your configuration working for me too, but once I have the "mode tcp" > changed to "mode http", it was failed. > Can you give it a try by changing your testing configuration mode from > "mode tcp" to "mode http"? OK I can now reproduce it this way. And I'm seeing the same problem you have regarding epoll_ctl(DEL). I'm anticipating something ugly in the lower layers now... Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Something strange noticed.
Here is the system calls:
write(2, "SOCKS PROXY HS FD[000A], Request"..., 60SOCKS PROXY HS
FD[000A], Request, conn->flags [0x11432776]) = 60
sendto(10, "\4\1'`\177\0\0\1HAProxy\0", 16,
MSG_DONTWAIT|MSG_NOSIGNAL|MSG_MORE, NULL, 0) = 16
write(2, "SOCKS PROXY HS FD[000A]: Before "..., 63SOCKS PROXY HS
FD[000A]: Before send remain is [16], sent [16]) = 63
epoll_ctl(3, EPOLL_CTL_DEL, 10, 0x7fc2056cf7b0) = 0
Looks like the socks4 FD get removed from the poll just after sending
the socks4 request.
Please have a look into it, thank you.
Regards,
Alexander Liu
On Mon, Jun 3, 2019 at 1:40 AM Alec Liu wrote:
>
> Hi Willy,
>
> Your configuration working for me too, but once I have the "mode tcp"
> changed to "mode http", it was failed.
> Can you give it a try by changing your testing configuration mode from
> "mode tcp" to "mode http"?
>
> Thank you.
>
> Regards,
> Alexander Liu
>
> On Sun, Jun 2, 2019 at 6:56 PM Willy Tarreau wrote:
> >
> > On Sun, Jun 02, 2019 at 06:49:18PM +0800, Alec Liu wrote:
> > > Hi Willy,
> > >
> > > Can you send me your testing configuration for this? Thank you.
> >
> > Sure, here it comes. Please note that I'm done with typing the release
> > message, so I'm releasing -dev5 now, but nothing prevents us from fixing
> > this problem of course!
> >
> > global
> > log stdout daemon
> >
> > listen 20080
> > bind 0.0.0.0:20080
> > mode tcp
> > log global
> > option tcplog
> > timeout connect 5s
> > timeout client 5s
> > timeout server 5s
> > #option tcp-check
> > #option httpchk
> > #server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy-v2
> > #check check-via-socks4
> > server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy #check
> > check-via-socks4
> >
> > listen 10080
> > bind :10080 accept-proxy
> > bind :10081
> > mode http
> > log global
> > option httplog
> > ### below just to create a pause (1s response time)
> > #no option http-use-htx
> > #tcp-request content accept if { req.len gt 1 }
> > #tcp-request inspect-delay 1s
> > redirect location /PP
> >
> > Cheers,
> > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Your configuration working for me too, but once I have the "mode tcp"
changed to "mode http", it was failed.
Can you give it a try by changing your testing configuration mode from
"mode tcp" to "mode http"?
Thank you.
Regards,
Alexander Liu
On Sun, Jun 2, 2019 at 6:56 PM Willy Tarreau wrote:
>
> On Sun, Jun 02, 2019 at 06:49:18PM +0800, Alec Liu wrote:
> > Hi Willy,
> >
> > Can you send me your testing configuration for this? Thank you.
>
> Sure, here it comes. Please note that I'm done with typing the release
> message, so I'm releasing -dev5 now, but nothing prevents us from fixing
> this problem of course!
>
> global
> log stdout daemon
>
> listen 20080
> bind 0.0.0.0:20080
> mode tcp
> log global
> option tcplog
> timeout connect 5s
> timeout client 5s
> timeout server 5s
> #option tcp-check
> #option httpchk
> #server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy-v2
> #check check-via-socks4
> server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy #check
> check-via-socks4
>
> listen 10080
> bind :10080 accept-proxy
> bind :10081
> mode http
> log global
> option httplog
> ### below just to create a pause (1s response time)
> #no option http-use-htx
> #tcp-request content accept if { req.len gt 1 }
> #tcp-request inspect-delay 1s
> redirect location /PP
>
> Cheers,
> Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Sun, Jun 02, 2019 at 06:49:18PM +0800, Alec Liu wrote:
> Hi Willy,
>
> Can you send me your testing configuration for this? Thank you.
Sure, here it comes. Please note that I'm done with typing the release
message, so I'm releasing -dev5 now, but nothing prevents us from fixing
this problem of course!
global
log stdout daemon
listen 20080
bind 0.0.0.0:20080
mode tcp
log global
option tcplog
timeout connect 5s
timeout client 5s
timeout server 5s
#option tcp-check
#option httpchk
#server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy-v2 #check
check-via-socks4
server srv 127.0.0.1:10080 socks4 127.0.0.1:1080 send-proxy #check
check-via-socks4
listen 10080
bind :10080 accept-proxy
bind :10081
mode http
log global
option httplog
### below just to create a pause (1s response time)
#no option http-use-htx
#tcp-request content accept if { req.len gt 1 }
#tcp-request inspect-delay 1s
redirect location /PP
Cheers,
Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Can you send me your testing configuration for this? Thank you.
Regards,
Alexander Liu
On Sun, Jun 2, 2019 at 5:57 PM Willy Tarreau wrote:
>
> On Sun, Jun 02, 2019 at 05:30:05PM +0800, Alec Liu wrote:
> > Hi Willy,
> >
> > Here is the full conf:
> (...)
> > listen HTTPS-20443
> > bind 0.0.0.0:20443
> > mode http
> > option tcp-check
> > server Apache2_Via_SocksProxy1_HTTP_SP 192.168.101.227:49980
> > send-proxy verify none socks4 127.0.0.1:1080 check-send-proxy
> > check-via-socks4 check inter 3 fastinter 1000
>
> Even with this I can't get it to fail here using the SSH trick you
> sent me. I can send requests which are perfectly forwarded and get
> the response back.
>
> My config receives HTTP requests on port 20080 (TCP mode), forwards
> them to my ssh-based socks server (port 1080), with proxy-proto
> encapsulation inside. It then forwards that to a second instance
> on port 10080, which decapsulates the PP header, receives the HTTP
> requests, processes it, sends a redirect back.
>
> It gives me this (just relevant lines) :
>
> # curl connects to haproxy 1st stage:
> 11:50:41.844303 accept4(4, {sa_family=AF_INET, sin_port=htons(53894),
> sin_addr=inet_addr("127.0.0.1")}, [16], SOCK_NONBLOCK) = 9
>
> # haproxy connects to socks server:
> 11:50:41.844406 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 10
> 11:50:41.844433 connect(10, {sa_family=AF_INET, sin_port=htons(1080),
> sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in
> progress)
> 11:50:41.844496 sendto(10, "\4\1'`\177\0\0\1HAProxy\0", 16,
> MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 16
> 11:50:41.844512 recvfrom(10, 0x7ffd7c330f68, 8, MSG_PEEK, NULL, NULL) = -1
> EAGAIN (Resource temporarily unavailable)
> 11:50:41.844524 epoll_ctl(3, EPOLL_CTL_ADD, 10, {EPOLLIN|EPOLLRDHUP, {u32=10,
> u64=10}}) = 0
>
> # socks server connects to second stage:
> 11:50:41.844613 accept4(5, {sa_family=AF_INET, sin_port=htons(41026),
> sin_addr=inet_addr("127.0.0.1")}, [16], SOCK_NONBLOCK) = 11
> 11:50:41.844647 recvfrom(11, 0x19082b0, 16384, MSG_PEEK, NULL, NULL) = -1
> EAGAIN (Resource temporarily unavailable)
> 11:50:41.844656 epoll_ctl(3, EPOLL_CTL_ADD, 11, {EPOLLIN|EPOLLRDHUP, {u32=11,
> u64=11}}) = 0
>
> # socks server responds with SOCKS4 header:
> 11:50:41.844691 recvfrom(10, "\0Z\0\0\0\0\0\0", 8, MSG_PEEK, NULL, NULL) = 8
> 11:50:41.844700 recvfrom(10, "\0Z\0\0\0\0\0\0", 8, 0, NULL, NULL) = 8
>
> # haproxy sends the proxy protocol header:
> 11:50:41.844711 sendto(10, "PROXY TCP4 127.0.0.1 127.0.0.1 53894 20080\r\n",
> 44, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 44
> 11:50:41.844726 recvfrom(10, 0x196f480, 16384, 0, NULL, NULL) = -1 EAGAIN
> (Resource temporarily unavailable)
> 11:50:41.844736 epoll_ctl(3, EPOLL_CTL_MOD, 10, {EPOLLIN|EPOLLRDHUP, {u32=10,
> u64=10}}) = 0
>
> # SOCKS server forwards it to second layer:
> 11:50:41.844812 recvfrom(11, "PROXY TCP4 127.0.0.1 127.0.0.1 53894
> 20080\r\n", 16384, MSG_PEEK, NULL, NULL) = 44
> 11:50:41.844822 recvfrom(11, "PROXY TCP4 127.0.0.1 127.0.0.1 53894
> 20080\r\n", 44, 0, NULL, NULL) = 44
>
> # curl sends its HTTP request to first stage:
> 11:50:41.844897 recvfrom(9, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
> curl/7.57.0\r\nAccept: */*\r\n\r\n", 16384, 0, NULL, NULL) = 71
>
> # haproxy forwards it to socks server:
> 11:50:41.844915 sendto(10, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
> curl/7.57.0\r\nAccept: */*\r\n\r\n", 71, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0)
> = 71
>
> # which in turn forwards it to second stage:
> 11:50:41.844995 recvfrom(11, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
> curl/7.57.0\r\nAccept: */*\r\n\r\n", 16328, 0, NULL, NULL) = 71
>
> # second stage responds to socks server:
> 11:50:41.845091 sendto(11, "HTTP/1.1 302 Found\r\ncontent-length:
> 0\r\nlocation: /PP\r\ncache-control: no-cache\r\n\r\n", 81,
> MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 81
>
> # socks server forwards the response to first stage:
> 11:50:41.845222 recvfrom(10, "HTTP/1.1 302 Found\r\ncontent-length:
> 0\r\nlocation: /PP\r\ncache-control: no-cache\r\n\r\n", 16384, 0, NULL, NULL)
> = 81
>
> # which sends it back to curl:
> 11:50:41.845241 sendto(9, "HTTP/1.1 302 Found\r\ncontent-length:
> 0\r\nlocation: /PP\r\ncache-control: no-cache\r\n\r\n", 81,
> MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 81
>
> So at this point I can't reproduce the behaviour :-( Given that changing
> the code changes something for you it must be a race somewhere.
>
> Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Sun, Jun 2, 2019 at 5:34 PM Willy Tarreau wrote:
>
> On Sun, Jun 02, 2019 at 04:57:13PM +0800, Alec Liu wrote:
> > Hi Willy,
> >
> > Just get it tested, nothing to do with check-send-proxy, still the
> > same no response.
>
> Hmmm that's annoying because I have one here. Thus probably it's a matter
> of polling issue (wait vs immediate response).
>
> > I have some change as below to make it work again.
> >
> > =
> > diff --git a/src/connection.c b/src/connection.c
> > index f0ca2bb16..c8ebe9da4 100644
> > --- a/src/connection.c
> > +++ b/src/connection.c
> > @@ -1017,17 +1017,12 @@ int conn_send_socks4_proxy_request(struct
> > connection *conn)
> > if (conn->send_proxy_ofs != 0) {
> > goto out_wait;
> > }
> > -}
> > -
> > -/* OK we've the whole request sent */
> > -conn->flags &= ~CO_FL_SOCKS4_SEND;
> > -__conn_sock_stop_send(conn);
> > +
> > +/* OK we've the whole request sent */
> > +conn->flags &= ~CO_FL_SOCKS4_SEND;
>
> Please be careful not to reindent the code in your changes, as that
> complicates the review. So what I'm seeing is that you only removed
> the conn_sock_stop_send() call above, right ?
Actually the code looks like this.
Th Master Version:
if (conn->send_proxy_ofs < 0) {
...
}
/* OK we've the whole request sent */
conn->flags &= ~CO_FL_SOCKS4_SEND;
__conn_sock_stop_send(conn);
My version:
if (conn->send_proxy_ofs < 0) {
...
/* OK we've the whole request sent */
conn->flags &= ~CO_FL_SOCKS4_SEND;
__conn_sock_want_recv(conn);
}
>
> > -/* The connection is ready now, simply return and let the connection
> > - * handler notify upper layers if needed.
> > - */
> > -if (conn->flags & CO_FL_WAIT_L4_CONN)
> > -conn->flags &= ~CO_FL_WAIT_L4_CONN;
> > +__conn_sock_want_recv(conn);
> > +}
>
> I disagree with adjusting the recv polling when sending. It should be
> set (if needed) when entering the socks4 recv handler. Also I don't
> see how that can fix the problem given that if I understand well, in
> your case it's the server's response which fails, thus it's unrelated
> to conn_sock (or we have a deeper issue here).
I do not have a good explanation for it yet, but actually this is the codes
fix the issue on my testing env, not others.
>
> > if (conn->flags & CO_FL_SEND_PROXY) {
> > /*
> > @@ -1157,6 +1152,16 @@ int conn_recv_socks4_proxy_response(struct
> > connection *conn)
> > } while (0);
> >
> > conn->flags &= ~CO_FL_SOCKS4_RECV;
> > +
> > +if (conn->flags & CO_FL_WAIT_L4_CONN) {
> > +if (!(conn->flags & CO_FL_HANDSHAKE & ~CO_FL_SOCKS4)) {
> > +/* No more handshake needed, the connection is ready now,
> > + * simply return and let the connection handler notify
> > + * upper layers if needed.
> > + */
> > +conn->flags &= ~CO_FL_WAIT_L4_CONN;
> > +}
> > +}
>
> Wow, I didn't notice this one was missing, this can actually be the
> real cause. I'm going to run more tests as well. I was about to issue
> dev5 but it can wait a few more minutes, I'll delete my tag.
>
> Willy
The change help on my testing env.
--- a/src/connection.c
+++ b/src/connection.c
@@ -1017,18 +1017,12 @@ int conn_send_socks4_proxy_request(struct
connection *conn)
if (conn->send_proxy_ofs != 0) {
goto out_wait;
}
+
+ /* OK we've the whole request sent */
+ conn->flags &= ~CO_FL_SOCKS4_SEND;
+ __conn_sock_want_recv(conn);
}
- /* OK we've the whole request sent */
- conn->flags &= ~CO_FL_SOCKS4_SEND;
- __conn_sock_stop_send(conn);
-
- /* The connection is ready now, simply return and let the connection
-* handler notify upper layers if needed.
-*/
- if (conn->flags & CO_FL_WAIT_L4_CONN)
- conn->flags &= ~CO_FL_WAIT_L4_CONN;
-
if (conn->flags & CO_FL_SEND_PROXY) {
/*
* Get the send_proxy_ofs ready for the send_proxy due to we are
==
I will try to look into it, why it is not able to reproduce on your side.
Thank you.
Regards,
Alexander Liu
Re: MEDIUM: Adding upstream socks4 proxy support
On Sun, Jun 02, 2019 at 05:30:05PM +0800, Alec Liu wrote:
> Hi Willy,
>
> Here is the full conf:
(...)
> listen HTTPS-20443
> bind 0.0.0.0:20443
> mode http
> option tcp-check
> server Apache2_Via_SocksProxy1_HTTP_SP 192.168.101.227:49980
> send-proxy verify none socks4 127.0.0.1:1080 check-send-proxy
> check-via-socks4 check inter 3 fastinter 1000
Even with this I can't get it to fail here using the SSH trick you
sent me. I can send requests which are perfectly forwarded and get
the response back.
My config receives HTTP requests on port 20080 (TCP mode), forwards
them to my ssh-based socks server (port 1080), with proxy-proto
encapsulation inside. It then forwards that to a second instance
on port 10080, which decapsulates the PP header, receives the HTTP
requests, processes it, sends a redirect back.
It gives me this (just relevant lines) :
# curl connects to haproxy 1st stage:
11:50:41.844303 accept4(4, {sa_family=AF_INET, sin_port=htons(53894),
sin_addr=inet_addr("127.0.0.1")}, [16], SOCK_NONBLOCK) = 9
# haproxy connects to socks server:
11:50:41.844406 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 10
11:50:41.844433 connect(10, {sa_family=AF_INET, sin_port=htons(1080),
sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in
progress)
11:50:41.844496 sendto(10, "\4\1'`\177\0\0\1HAProxy\0", 16,
MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 16
11:50:41.844512 recvfrom(10, 0x7ffd7c330f68, 8, MSG_PEEK, NULL, NULL) = -1
EAGAIN (Resource temporarily unavailable)
11:50:41.844524 epoll_ctl(3, EPOLL_CTL_ADD, 10, {EPOLLIN|EPOLLRDHUP, {u32=10,
u64=10}}) = 0
# socks server connects to second stage:
11:50:41.844613 accept4(5, {sa_family=AF_INET, sin_port=htons(41026),
sin_addr=inet_addr("127.0.0.1")}, [16], SOCK_NONBLOCK) = 11
11:50:41.844647 recvfrom(11, 0x19082b0, 16384, MSG_PEEK, NULL, NULL) = -1
EAGAIN (Resource temporarily unavailable)
11:50:41.844656 epoll_ctl(3, EPOLL_CTL_ADD, 11, {EPOLLIN|EPOLLRDHUP, {u32=11,
u64=11}}) = 0
# socks server responds with SOCKS4 header:
11:50:41.844691 recvfrom(10, "\0Z\0\0\0\0\0\0", 8, MSG_PEEK, NULL, NULL) = 8
11:50:41.844700 recvfrom(10, "\0Z\0\0\0\0\0\0", 8, 0, NULL, NULL) = 8
# haproxy sends the proxy protocol header:
11:50:41.844711 sendto(10, "PROXY TCP4 127.0.0.1 127.0.0.1 53894 20080\r\n",
44, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 44
11:50:41.844726 recvfrom(10, 0x196f480, 16384, 0, NULL, NULL) = -1 EAGAIN
(Resource temporarily unavailable)
11:50:41.844736 epoll_ctl(3, EPOLL_CTL_MOD, 10, {EPOLLIN|EPOLLRDHUP, {u32=10,
u64=10}}) = 0
# SOCKS server forwards it to second layer:
11:50:41.844812 recvfrom(11, "PROXY TCP4 127.0.0.1 127.0.0.1 53894 20080\r\n",
16384, MSG_PEEK, NULL, NULL) = 44
11:50:41.844822 recvfrom(11, "PROXY TCP4 127.0.0.1 127.0.0.1 53894 20080\r\n",
44, 0, NULL, NULL) = 44
# curl sends its HTTP request to first stage:
11:50:41.844897 recvfrom(9, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
curl/7.57.0\r\nAccept: */*\r\n\r\n", 16384, 0, NULL, NULL) = 71
# haproxy forwards it to socks server:
11:50:41.844915 sendto(10, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
curl/7.57.0\r\nAccept: */*\r\n\r\n", 71, MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) =
71
# which in turn forwards it to second stage:
11:50:41.844995 recvfrom(11, "GET / HTTP/1.1\r\nHost: 0:20080\r\nUser-Agent:
curl/7.57.0\r\nAccept: */*\r\n\r\n", 16328, 0, NULL, NULL) = 71
# second stage responds to socks server:
11:50:41.845091 sendto(11, "HTTP/1.1 302 Found\r\ncontent-length:
0\r\nlocation: /PP\r\ncache-control: no-cache\r\n\r\n", 81,
MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 81
# socks server forwards the response to first stage:
11:50:41.845222 recvfrom(10, "HTTP/1.1 302 Found\r\ncontent-length:
0\r\nlocation: /PP\r\ncache-control: no-cache\r\n\r\n", 16384, 0, NULL, NULL) =
81
# which sends it back to curl:
11:50:41.845241 sendto(9, "HTTP/1.1 302 Found\r\ncontent-length: 0\r\nlocation:
/PP\r\ncache-control: no-cache\r\n\r\n", 81, MSG_DONTWAIT|MSG_NOSIGNAL, NULL,
0) = 81
So at this point I can't reproduce the behaviour :-( Given that changing
the code changes something for you it must be a race somewhere.
Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Sun, Jun 02, 2019 at 04:57:13PM +0800, Alec Liu wrote:
> Hi Willy,
>
> Just get it tested, nothing to do with check-send-proxy, still the
> same no response.
Hmmm that's annoying because I have one here. Thus probably it's a matter
of polling issue (wait vs immediate response).
> I have some change as below to make it work again.
>
> =
> diff --git a/src/connection.c b/src/connection.c
> index f0ca2bb16..c8ebe9da4 100644
> --- a/src/connection.c
> +++ b/src/connection.c
> @@ -1017,17 +1017,12 @@ int conn_send_socks4_proxy_request(struct
> connection *conn)
> if (conn->send_proxy_ofs != 0) {
> goto out_wait;
> }
> -}
> -
> -/* OK we've the whole request sent */
> -conn->flags &= ~CO_FL_SOCKS4_SEND;
> -__conn_sock_stop_send(conn);
> +
> +/* OK we've the whole request sent */
> +conn->flags &= ~CO_FL_SOCKS4_SEND;
Please be careful not to reindent the code in your changes, as that
complicates the review. So what I'm seeing is that you only removed
the conn_sock_stop_send() call above, right ?
> -/* The connection is ready now, simply return and let the connection
> - * handler notify upper layers if needed.
> - */
> -if (conn->flags & CO_FL_WAIT_L4_CONN)
> -conn->flags &= ~CO_FL_WAIT_L4_CONN;
> +__conn_sock_want_recv(conn);
> +}
I disagree with adjusting the recv polling when sending. It should be
set (if needed) when entering the socks4 recv handler. Also I don't
see how that can fix the problem given that if I understand well, in
your case it's the server's response which fails, thus it's unrelated
to conn_sock (or we have a deeper issue here).
> if (conn->flags & CO_FL_SEND_PROXY) {
> /*
> @@ -1157,6 +1152,16 @@ int conn_recv_socks4_proxy_response(struct
> connection *conn)
> } while (0);
>
> conn->flags &= ~CO_FL_SOCKS4_RECV;
> +
> +if (conn->flags & CO_FL_WAIT_L4_CONN) {
> +if (!(conn->flags & CO_FL_HANDSHAKE & ~CO_FL_SOCKS4)) {
> +/* No more handshake needed, the connection is ready now,
> + * simply return and let the connection handler notify
> + * upper layers if needed.
> + */
> +conn->flags &= ~CO_FL_WAIT_L4_CONN;
> +}
> +}
Wow, I didn't notice this one was missing, this can actually be the
real cause. I'm going to run more tests as well. I was about to issue
dev5 but it can wait a few more minutes, I'll delete my tag.
Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy, btw: I have the apache http server setup with PP support on the port "192.168.101.227:49980" already, it is a working one. Regards, Alexander Liu On Sun, Jun 2, 2019 at 5:26 PM Willy Tarreau wrote: > > On Sun, Jun 02, 2019 at 04:37:25PM +0800, Alec Liu wrote: > > Hi Willy, > > > > The test case which I used was on purpose, to combine proxy protocol > > with socks4. > > Due to I am not doing any special checking, it should be fine without > > check-send-proxy. > > It's just that your check is inconsistent then : if your server is > configured to receive the proxy protocol, you're sending half a handshake. > Well, I'm well aware that the PP doesn't return a response but if the server > receives something invalid (e.g. HTTP) it will immediately close, making > the handshake appear as failed. I don't have access to the rest of your > config so I don't know what else is configured. If by "not doing any special > checking" you mean there's no http-checks, then it should not affect it. > > > Actually the check part should be fine with my testing configuration, > > at least it shows as up in the HAProxy. > > I will do some more testing with check-send-proxy added too, let you > > know in short. > > > > btw: for SOCKS server you can just use the "ssh -D 1080 ...". > > Ah good to know, thanks. > > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy, Here is the full conf: global log /dev/log local0 log /dev/log local1 notice stats timeout 30s tune.ssl.default-dh-param 2048 defaults log global mode http option httplog option dontlognull timeout connect 5000 timeout client 5 timeout server 5 listen HTTPS-20443 bind 0.0.0.0:20443 mode http option tcp-check server Apache2_Via_SocksProxy1_HTTP_SP 192.168.101.227:49980 send-proxy verify none socks4 127.0.0.1:1080 check-send-proxy check-via-socks4 check inter 3 fastinter 1000 # HAProxy web ui listen stats bind 0.0.0.0:20936 mode http log global maxconn 10 timeout client 100s timeout server 100s timeout connect 100s timeout queue 100s stats enable stats uri /haproxy?stats stats realm HAProxy\ Statistics stats admin if TRUE stats show-node - Regards, Alexander Liu On Sun, Jun 2, 2019 at 5:26 PM Willy Tarreau wrote: > > On Sun, Jun 02, 2019 at 04:37:25PM +0800, Alec Liu wrote: > > Hi Willy, > > > > The test case which I used was on purpose, to combine proxy protocol > > with socks4. > > Due to I am not doing any special checking, it should be fine without > > check-send-proxy. > > It's just that your check is inconsistent then : if your server is > configured to receive the proxy protocol, you're sending half a handshake. > Well, I'm well aware that the PP doesn't return a response but if the server > receives something invalid (e.g. HTTP) it will immediately close, making > the handshake appear as failed. I don't have access to the rest of your > config so I don't know what else is configured. If by "not doing any special > checking" you mean there's no http-checks, then it should not affect it. > > > Actually the check part should be fine with my testing configuration, > > at least it shows as up in the HAProxy. > > I will do some more testing with check-send-proxy added too, let you > > know in short. > > > > btw: for SOCKS server you can just use the "ssh -D 1080 ...". > > Ah good to know, thanks. > > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Sun, Jun 02, 2019 at 04:37:25PM +0800, Alec Liu wrote: > Hi Willy, > > The test case which I used was on purpose, to combine proxy protocol > with socks4. > Due to I am not doing any special checking, it should be fine without > check-send-proxy. It's just that your check is inconsistent then : if your server is configured to receive the proxy protocol, you're sending half a handshake. Well, I'm well aware that the PP doesn't return a response but if the server receives something invalid (e.g. HTTP) it will immediately close, making the handshake appear as failed. I don't have access to the rest of your config so I don't know what else is configured. If by "not doing any special checking" you mean there's no http-checks, then it should not affect it. > Actually the check part should be fine with my testing configuration, > at least it shows as up in the HAProxy. > I will do some more testing with check-send-proxy added too, let you > know in short. > > btw: for SOCKS server you can just use the "ssh -D 1080 ...". Ah good to know, thanks. Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Just get it tested, nothing to do with check-send-proxy, still the
same no response.
I have some change as below to make it work again.
=
diff --git a/src/connection.c b/src/connection.c
index f0ca2bb16..c8ebe9da4 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -1017,17 +1017,12 @@ int conn_send_socks4_proxy_request(struct
connection *conn)
if (conn->send_proxy_ofs != 0) {
goto out_wait;
}
-}
-
-/* OK we've the whole request sent */
-conn->flags &= ~CO_FL_SOCKS4_SEND;
-__conn_sock_stop_send(conn);
+
+/* OK we've the whole request sent */
+conn->flags &= ~CO_FL_SOCKS4_SEND;
-/* The connection is ready now, simply return and let the connection
- * handler notify upper layers if needed.
- */
-if (conn->flags & CO_FL_WAIT_L4_CONN)
-conn->flags &= ~CO_FL_WAIT_L4_CONN;
+__conn_sock_want_recv(conn);
+}
if (conn->flags & CO_FL_SEND_PROXY) {
/*
@@ -1157,6 +1152,16 @@ int conn_recv_socks4_proxy_response(struct
connection *conn)
} while (0);
conn->flags &= ~CO_FL_SOCKS4_RECV;
+
+if (conn->flags & CO_FL_WAIT_L4_CONN) {
+if (!(conn->flags & CO_FL_HANDSHAKE & ~CO_FL_SOCKS4)) {
+/* No more handshake needed, the connection is ready now,
+ * simply return and let the connection handler notify
+ * upper layers if needed.
+ */
+conn->flags &= ~CO_FL_WAIT_L4_CONN;
+}
+}
return 1;
recv_abort:
=
Regards,
Alexander Liu
On Sun, Jun 2, 2019 at 4:37 PM Alec Liu wrote:
>
> Hi Willy,
>
> The test case which I used was on purpose, to combine proxy protocol
> with socks4.
> Due to I am not doing any special checking, it should be fine without
> check-send-proxy.
>
> Actually the check part should be fine with my testing configuration,
> at least it shows as up in the HAProxy.
> I will do some more testing with check-send-proxy added too, let you
> know in short.
>
> btw: for SOCKS server you can just use the "ssh -D 1080 ...".
>
> Regards,
> Alexander Liu
>
> On Sun, Jun 2, 2019 at 4:29 PM Willy Tarreau wrote:
> >
> > Hi Alec,
> >
> > On Sat, Jun 01, 2019 at 05:39:30AM +0800, Alec Liu wrote:
> > > Hi Willy,
> > >
> > > Thank you for merging it !
> > >
> > > I have the merged code pull, and rerun some basic test, ends up failed.
> > > here is the testing configuration:
> > > * server Apache2_Via_SocksProxy_HTTP_SP 192.168.101.227:49980
> > > send-proxy verify none socks4 127.0.0.1:1080 check-via-socks4 check
> > > inter 3 fastinter 1000
> > >
> > > Looks like it is something related to the send and recv you have
> > > mentioned.
> > > Should we just leave it in this way for now, and wait for the issue to
> > > be fixed?
> >
> > So I checked but I can't get it to fail here. In the trace below,
> > my SOCKS server (netcat+script) is on 127.0.0.1:9000 :
> >
> > 10:22:57.486347 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 8
> > 10:22:57.486401 connect(8, {sa_family=AF_INET, sin_port=htons(9000),
> > sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in
> > progress)
> > 10:22:57.486462 sendto(8, "\4\1\0P>\324r > MSG_DONTWAIT|MSG_NOSIGNAL|MSG_MORE, NULL, 0) = 16
> > 10:22:57.486481 recvfrom(8, 0x7ffd5106b7f8, 8, MSG_PEEK, NULL, NULL) = -1
> > EAGAIN (Resource temporarily unavailable)
> > 10:22:57.486493 epoll_ctl(3, EPOLL_CTL_ADD, 8, {EPOLLIN|EPOLLRDHUP, {u32=8,
> > u64=8}}) = 0
> > 10:22:57.692857 recvfrom(8, "\0Z\0\0\0\0\0\0", 8, MSG_PEEK, NULL, NULL) = 8
> > 10:22:57.692948 recvfrom(8, "\0Z\0\0\0\0\0\0", 8, 0, NULL, NULL) = 8
> > 10:22:57.693028 sendto(8, "OPTIONS / HTTP/1.0\r\n\r\n", 22,
> > MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 22
> > 10:22:57.693142 recvfrom(8, 0x1df9bd0, 16384, 0, NULL, NULL) = -1 EAGAIN
> > (Resource temporarily unavailable)
> > 10:22:57.693197 epoll_ctl(3, EPOLL_CTL_MOD, 8, {EPOLLIN|EPOLLRDHUP, {u32=8,
> > u64=8}}) = 0
> > 10:22:57.693549 recvfrom(8, "HTTP/1.1 200 OK\r\n", 16384, 0, NULL, NULL) =
> > 17
> > 10:22:57.693631 recvfrom(8, 0x1df9be1, 16367, 0, NULL, NULL) = -1 EAGAIN
> > (Resource temporarily unavailable)
> > 10:22:57.693755 epoll_ctl(3, EPOLL_CTL_DEL, 8, 0x7fdcbd2c97e0) = 0
> > 10:22:57.693921 close(8)= 0
> > [WARNING] 152/102257 (29489) : Server 20080/srv is UP, reason: Layer7 check
> > passed, code: 200, info: "OK", check duration: 207ms. 1 active and 0 backup
> > servers online. 0 sessions requeued, 0 total in queue.
> > <29>Jun 2 10:22:57 haproxy[29489]: Server 20080/srv is UP, reason: Layer7
> > check passed, code: 200, info: "OK", check duration: 207ms. 1 active and 0
> > backup servers online. 0 sessions requeued, 0 total in queue.
> >
> > It consumes the handshake, responds with the code you're expecting then
> > receives the reqeust and sends the response.
> >
> > Oh I'm seeing that you're combining send-proxy with it so I guess you're
> > using the proxy protocol
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
The test case which I used was on purpose, to combine proxy protocol
with socks4.
Due to I am not doing any special checking, it should be fine without
check-send-proxy.
Actually the check part should be fine with my testing configuration,
at least it shows as up in the HAProxy.
I will do some more testing with check-send-proxy added too, let you
know in short.
btw: for SOCKS server you can just use the "ssh -D 1080 ...".
Regards,
Alexander Liu
On Sun, Jun 2, 2019 at 4:29 PM Willy Tarreau wrote:
>
> Hi Alec,
>
> On Sat, Jun 01, 2019 at 05:39:30AM +0800, Alec Liu wrote:
> > Hi Willy,
> >
> > Thank you for merging it !
> >
> > I have the merged code pull, and rerun some basic test, ends up failed.
> > here is the testing configuration:
> > * server Apache2_Via_SocksProxy_HTTP_SP 192.168.101.227:49980
> > send-proxy verify none socks4 127.0.0.1:1080 check-via-socks4 check
> > inter 3 fastinter 1000
> >
> > Looks like it is something related to the send and recv you have mentioned.
> > Should we just leave it in this way for now, and wait for the issue to
> > be fixed?
>
> So I checked but I can't get it to fail here. In the trace below,
> my SOCKS server (netcat+script) is on 127.0.0.1:9000 :
>
> 10:22:57.486347 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 8
> 10:22:57.486401 connect(8, {sa_family=AF_INET, sin_port=htons(9000),
> sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in
> progress)
> 10:22:57.486462 sendto(8, "\4\1\0P>\324r MSG_DONTWAIT|MSG_NOSIGNAL|MSG_MORE, NULL, 0) = 16
> 10:22:57.486481 recvfrom(8, 0x7ffd5106b7f8, 8, MSG_PEEK, NULL, NULL) = -1
> EAGAIN (Resource temporarily unavailable)
> 10:22:57.486493 epoll_ctl(3, EPOLL_CTL_ADD, 8, {EPOLLIN|EPOLLRDHUP, {u32=8,
> u64=8}}) = 0
> 10:22:57.692857 recvfrom(8, "\0Z\0\0\0\0\0\0", 8, MSG_PEEK, NULL, NULL) = 8
> 10:22:57.692948 recvfrom(8, "\0Z\0\0\0\0\0\0", 8, 0, NULL, NULL) = 8
> 10:22:57.693028 sendto(8, "OPTIONS / HTTP/1.0\r\n\r\n", 22,
> MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 22
> 10:22:57.693142 recvfrom(8, 0x1df9bd0, 16384, 0, NULL, NULL) = -1 EAGAIN
> (Resource temporarily unavailable)
> 10:22:57.693197 epoll_ctl(3, EPOLL_CTL_MOD, 8, {EPOLLIN|EPOLLRDHUP, {u32=8,
> u64=8}}) = 0
> 10:22:57.693549 recvfrom(8, "HTTP/1.1 200 OK\r\n", 16384, 0, NULL, NULL) = 17
> 10:22:57.693631 recvfrom(8, 0x1df9be1, 16367, 0, NULL, NULL) = -1 EAGAIN
> (Resource temporarily unavailable)
> 10:22:57.693755 epoll_ctl(3, EPOLL_CTL_DEL, 8, 0x7fdcbd2c97e0) = 0
> 10:22:57.693921 close(8)= 0
> [WARNING] 152/102257 (29489) : Server 20080/srv is UP, reason: Layer7 check
> passed, code: 200, info: "OK", check duration: 207ms. 1 active and 0 backup
> servers online. 0 sessions requeued, 0 total in queue.
> <29>Jun 2 10:22:57 haproxy[29489]: Server 20080/srv is UP, reason: Layer7
> check passed, code: 200, info: "OK", check duration: 207ms. 1 active and 0
> backup servers online. 0 sessions requeued, 0 total in queue.
>
> It consumes the handshake, responds with the code you're expecting then
> receives the reqeust and sends the response.
>
> Oh I'm seeing that you're combining send-proxy with it so I guess you're
> using the proxy protocol on top of the SOCKS connection, but don't have
> check-send-proxy, thus there is no way the server will be able to respond.
> Are you sure this isn't the cause of your problem ?
>
> Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec,
On Sat, Jun 01, 2019 at 05:39:30AM +0800, Alec Liu wrote:
> Hi Willy,
>
> Thank you for merging it !
>
> I have the merged code pull, and rerun some basic test, ends up failed.
> here is the testing configuration:
> * server Apache2_Via_SocksProxy_HTTP_SP 192.168.101.227:49980
> send-proxy verify none socks4 127.0.0.1:1080 check-via-socks4 check
> inter 3 fastinter 1000
>
> Looks like it is something related to the send and recv you have mentioned.
> Should we just leave it in this way for now, and wait for the issue to
> be fixed?
So I checked but I can't get it to fail here. In the trace below,
my SOCKS server (netcat+script) is on 127.0.0.1:9000 :
10:22:57.486347 socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 8
10:22:57.486401 connect(8, {sa_family=AF_INET, sin_port=htons(9000),
sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in
progress)
10:22:57.486462 sendto(8, "\4\1\0P>\324rJun 2 10:22:57 haproxy[29489]: Server 20080/srv is UP, reason: Layer7
check passed, code: 200, info: "OK", check duration: 207ms. 1 active and 0
backup servers online. 0 sessions requeued, 0 total in queue.
It consumes the handshake, responds with the code you're expecting then
receives the reqeust and sends the response.
Oh I'm seeing that you're combining send-proxy with it so I guess you're
using the proxy protocol on top of the SOCKS connection, but don't have
check-send-proxy, thus there is no way the server will be able to respond.
Are you sure this isn't the cause of your problem ?
Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Sat, Jun 01, 2019 at 05:39:30AM +0800, Alec Liu wrote: > I have the merged code pull, and rerun some basic test, ends up failed. > here is the testing configuration: > * server Apache2_Via_SocksProxy_HTTP_SP 192.168.101.227:49980 > send-proxy verify none socks4 127.0.0.1:1080 check-via-socks4 check > inter 3 fastinter 1000 > > Looks like it is something related to the send and recv you have mentioned. Ah, bad :-( One of the problems I faced after integrating it was that conn_fd_handler() was called in loops until recv() succeeded. I found that fd_cant_recv() was initially not expected to be called from there (eventhough all other handshakes do it as well) and that it was later cancelled by with the conn_sock_* calls done in conn_fd_handler(). I managed to work around this by using both conn_sock* and fd_cant*, but I'm not very proud of this. > Should we just leave it in this way for now, and wait for the issue to > be fixed? I'll have a look at it. If it ends up being a limitation of the current polling mechanism that's going to be dropped by Olivier's update, I won't insist too much but I'd rather try to fix it anyway. Thanks for the report! Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy, Thank you for merging it ! I have the merged code pull, and rerun some basic test, ends up failed. here is the testing configuration: * server Apache2_Via_SocksProxy_HTTP_SP 192.168.101.227:49980 send-proxy verify none socks4 127.0.0.1:1080 check-via-socks4 check inter 3 fastinter 1000 Looks like it is something related to the send and recv you have mentioned. Should we just leave it in this way for now, and wait for the issue to be fixed? Regards, Alexander Liu On Fri, May 31, 2019 at 11:30 PM Willy Tarreau wrote: > > On Fri, May 31, 2019 at 03:50:41PM +0200, Willy Tarreau wrote: > > I think that once I've figured what causes the strange things I've seen > > I'll simply adjust it and merge it. > > OK now adjusted and merged. There's in fact an issue in the connection > layer related to switching between send and recv, which didn't exist in > the past. I simply worked around it since the next changes will make > this an old story. > > Thanks! > Willy
Re: MEDIUM: Adding upstream socks4 proxy support
On Fri, May 31, 2019 at 03:50:41PM +0200, Willy Tarreau wrote: > I think that once I've figured what causes the strange things I've seen > I'll simply adjust it and merge it. OK now adjusted and merged. There's in fact an issue in the connection layer related to switching between send and recv, which didn't exist in the past. I simply worked around it since the next changes will make this an old story. Thanks! Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec, On Thu, May 30, 2019 at 07:59:22PM +0800, Alec Liu wrote: > Hi Willy, > > Here is the updated patch, please help to look into it, thank you! Thanks. So I've had a look and am seeing something very strange : the connection establishes to the server, the SOCK4 handshake is exchanged, then a connect() is made again from the same connection, I guess that it's just in order to validate that the connection properly established, and from there it works. A few times I saw epoll_wait() wake up in loops with an uncaught write event. I'm currently having a look into this as I suspect it's the same problem and that there is a place where it's not clear that we've completed the handshake. But overall it looks good. I've discussed with Olivier regarding the possible conflicts, and he said that we can merge your patch before his, and that it will not be too much work to adapt so I'll do this. I think that once I've figured what causes the strange things I've seen I'll simply adjust it and merge it. Thanks! Willy
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Here is the updated patch, please help to look into it, thank you!
Regards,
Alexander Liu
On Thu, May 30, 2019 at 6:20 PM Alec Liu wrote:
>
> Hi Willy,
>
> I am still doing some testing. Once finished, I will try to submit the
> new patch ASAP.
> Hope can get it done before Olivier's code merge, to save you some work load.
>
> On Thu, May 30, 2019 at 4:18 PM Willy Tarreau wrote:
> >
> > Hi Alec,
> >
> > On Thu, May 30, 2019 at 01:15:47AM +0800, Alec Liu wrote:
> > > Hi Willy,
> > >
> > > Thank you once again for all the detail reply.
> >
> > You're welcome.
> >
> > > > Just so that you know, Olivier is finishing his changes in this area to
> > > > make SSL totally self-sustaining (will be needed for QUIC) so at some
> > > > point we'll have a conflict here. He'll probably know how to adapt this
> > > > however.
> > >
> > > I will try to read the code from Olivier for the SSL part , to learn
> > > more about it.
> >
> > OK so I discussed about this with Olivier, as I'm about to merge his
> > cleanups. What I'll propose you is to only address your issues WITHOUT
> > rebasing your code, then send the updated patch. We'll manually rebase it
> > on the modified code then so that you don't have to waste your time trying
> > to figure where things moved.
> >
> > > > > + /* OK we've the whole request sent */
> > > > > + conn->flags &= (~CO_FL_SOCKS4_SEND);
> > > >^ ^
> > > > Please avoid these useless parenthesis above, I noticed them at a few
> > > > places and they are even inconsistent (you don't have them below for
> > > > example).
> > > >
> > > I just want to make it easier to read, "LEFTPART &= RIGHTPART", both
> > > left and right will be a whole one, but
> > > "LEFTPART &= RIGHTPART1 x RIGHTPART2 x RIGHTPART3".
> >
> > For me it's the opposite, since "&=" is an assignment, I don't see why
> > the right part has to be enclosed and since this is not natural I visually
> > flag it as suspicious.
> Ok, I will try to follow it in the project.
>
> >
> > > > > + /* Turn to receiving response */
> > > > > + conn->flags |= CO_FL_SOCKS4_RECV;
> > > >
> > > > This one should be set where you set CO_FL_SOCKS4_SEND because when
> > > > you decide you'll send it, you also know you want to wait for the
> > > > response.
> > > >
> > > Yep you are right, when it sending the socks4 request, it is waiting for
> > > reply.
> > > But if the sending part get failed, it won't going to expect for the
> > > response, and
> > > we should not getting anything before sending the request, if we do,
> > > it should be something wrong.
> >
> > It's the same for all other handshake flags in fact. Whenever you face an
> > error in any of them, the error is immediately raised and the other ones
> > are not executed.
> >
> > > May be it should be ok to have both flags set at the same time for most
> > > cases, but I think setting it separately will be fine too.
> >
> > It is trickier to figure which ones are expected when they are set
> > dynamically. For example, during a connect() we could decide about some
> > options depending on whether or not we expect to send only or to
> > send+receive. That's why it's better to have them set right as soon
> > as possible.
> What will happen if the server just send you the correct response
> without your request?
> In the SOCKS4 proxy case, you get the 0x5A response, but you have not
> send the request yet.
> How can we identify it, and reject the response, mark it as error, and close
> it?
>
> >
> > > > > + if (line[1] != 0x5A) {
> > > >
> > > > Please either use #define/enums if this corresponds to something
> > > > particular,
> > > > or at least indicate in a comment what you're trying to verify (what's
> > > > in
> > > > line[1] and what 0x5A corresponds to).
> > > Actually I have comment for it at " /* SOCKS4 Proxy request granted
> > > server response, 0x00 | 0x5A | 0x00 0x00 | 0x00 0x00 0x00 0x00".
> > > Anyway, I will make thing clearer here.
> >
> > Thanks :-)
> >
> > > > > flags |= CONNECT_HAS_DATA;
> > > > >
> > > > > if (global.tune.server_sndbuf)
> > > > > @@ -315,7 +322,7 @@ static int sockpair_connect_server(struct
> > > > > connection *conn, int flags)
> > > > > conn->flags |= CO_FL_ADDR_TO_SET;
> > > > >
> > > > > /* Prepare to send a few handshakes related to the on-wire
> > > > > protocol. */
> > > > > - if (conn->send_proxy_ofs)
> > > > > + if (srv && srv->pp_opts)
> > > > > conn->flags |= CO_FL_SEND_PROXY;
> > > >
> > > > And here, we should just move the CO_FL_SEND_PROXY flag setting to the
> > > > place where send_proxy_ofs is set and remove this test. It looks like
> > > > checks already do it correctly and that it's only needed in backend.c.
> > > > Then with this it becomes clean : the one who wants to emit a header
> > > > just sets his CO_FL_SEND_XXX flag with send_proxy_ofs=1, then each other
>
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
I am still doing some testing. Once finished, I will try to submit the
new patch ASAP.
Hope can get it done before Olivier's code merge, to save you some work load.
On Thu, May 30, 2019 at 4:18 PM Willy Tarreau wrote:
>
> Hi Alec,
>
> On Thu, May 30, 2019 at 01:15:47AM +0800, Alec Liu wrote:
> > Hi Willy,
> >
> > Thank you once again for all the detail reply.
>
> You're welcome.
>
> > > Just so that you know, Olivier is finishing his changes in this area to
> > > make SSL totally self-sustaining (will be needed for QUIC) so at some
> > > point we'll have a conflict here. He'll probably know how to adapt this
> > > however.
> >
> > I will try to read the code from Olivier for the SSL part , to learn
> > more about it.
>
> OK so I discussed about this with Olivier, as I'm about to merge his
> cleanups. What I'll propose you is to only address your issues WITHOUT
> rebasing your code, then send the updated patch. We'll manually rebase it
> on the modified code then so that you don't have to waste your time trying
> to figure where things moved.
>
> > > > + /* OK we've the whole request sent */
> > > > + conn->flags &= (~CO_FL_SOCKS4_SEND);
> > >^ ^
> > > Please avoid these useless parenthesis above, I noticed them at a few
> > > places and they are even inconsistent (you don't have them below for
> > > example).
> > >
> > I just want to make it easier to read, "LEFTPART &= RIGHTPART", both
> > left and right will be a whole one, but
> > "LEFTPART &= RIGHTPART1 x RIGHTPART2 x RIGHTPART3".
>
> For me it's the opposite, since "&=" is an assignment, I don't see why
> the right part has to be enclosed and since this is not natural I visually
> flag it as suspicious.
Ok, I will try to follow it in the project.
>
> > > > + /* Turn to receiving response */
> > > > + conn->flags |= CO_FL_SOCKS4_RECV;
> > >
> > > This one should be set where you set CO_FL_SOCKS4_SEND because when
> > > you decide you'll send it, you also know you want to wait for the
> > > response.
> > >
> > Yep you are right, when it sending the socks4 request, it is waiting for
> > reply.
> > But if the sending part get failed, it won't going to expect for the
> > response, and
> > we should not getting anything before sending the request, if we do,
> > it should be something wrong.
>
> It's the same for all other handshake flags in fact. Whenever you face an
> error in any of them, the error is immediately raised and the other ones
> are not executed.
>
> > May be it should be ok to have both flags set at the same time for most
> > cases, but I think setting it separately will be fine too.
>
> It is trickier to figure which ones are expected when they are set
> dynamically. For example, during a connect() we could decide about some
> options depending on whether or not we expect to send only or to
> send+receive. That's why it's better to have them set right as soon
> as possible.
What will happen if the server just send you the correct response
without your request?
In the SOCKS4 proxy case, you get the 0x5A response, but you have not
send the request yet.
How can we identify it, and reject the response, mark it as error, and close it?
>
> > > > + if (line[1] != 0x5A) {
> > >
> > > Please either use #define/enums if this corresponds to something
> > > particular,
> > > or at least indicate in a comment what you're trying to verify (what's in
> > > line[1] and what 0x5A corresponds to).
> > Actually I have comment for it at " /* SOCKS4 Proxy request granted
> > server response, 0x00 | 0x5A | 0x00 0x00 | 0x00 0x00 0x00 0x00".
> > Anyway, I will make thing clearer here.
>
> Thanks :-)
>
> > > > flags |= CONNECT_HAS_DATA;
> > > >
> > > > if (global.tune.server_sndbuf)
> > > > @@ -315,7 +322,7 @@ static int sockpair_connect_server(struct
> > > > connection *conn, int flags)
> > > > conn->flags |= CO_FL_ADDR_TO_SET;
> > > >
> > > > /* Prepare to send a few handshakes related to the on-wire
> > > > protocol. */
> > > > - if (conn->send_proxy_ofs)
> > > > + if (srv && srv->pp_opts)
> > > > conn->flags |= CO_FL_SEND_PROXY;
> > >
> > > And here, we should just move the CO_FL_SEND_PROXY flag setting to the
> > > place where send_proxy_ofs is set and remove this test. It looks like
> > > checks already do it correctly and that it's only needed in backend.c.
> > > Then with this it becomes clean : the one who wants to emit a header
> > > just sets his CO_FL_SEND_XXX flag with send_proxy_ofs=1, then each other
> > > one in turn will simply have to recheck this. We could even go slightly
> > > further and let the connect() functions set send_proxy_ofs to 1 themselves
> > > when any of the SEND flags are set so that the upper layer only has to
> > > focus on setting the appropriate flags.
> > >
> > > Please do that in a separate preliminary patch, which will allow to
> > > bisect later if
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec,
On Thu, May 30, 2019 at 01:15:47AM +0800, Alec Liu wrote:
> Hi Willy,
>
> Thank you once again for all the detail reply.
You're welcome.
> > Just so that you know, Olivier is finishing his changes in this area to
> > make SSL totally self-sustaining (will be needed for QUIC) so at some
> > point we'll have a conflict here. He'll probably know how to adapt this
> > however.
>
> I will try to read the code from Olivier for the SSL part , to learn
> more about it.
OK so I discussed about this with Olivier, as I'm about to merge his
cleanups. What I'll propose you is to only address your issues WITHOUT
rebasing your code, then send the updated patch. We'll manually rebase it
on the modified code then so that you don't have to waste your time trying
to figure where things moved.
> > > + /* OK we've the whole request sent */
> > > + conn->flags &= (~CO_FL_SOCKS4_SEND);
> >^ ^
> > Please avoid these useless parenthesis above, I noticed them at a few
> > places and they are even inconsistent (you don't have them below for
> > example).
> >
> I just want to make it easier to read, "LEFTPART &= RIGHTPART", both
> left and right will be a whole one, but
> "LEFTPART &= RIGHTPART1 x RIGHTPART2 x RIGHTPART3".
For me it's the opposite, since "&=" is an assignment, I don't see why
the right part has to be enclosed and since this is not natural I visually
flag it as suspicious.
> > > + /* Turn to receiving response */
> > > + conn->flags |= CO_FL_SOCKS4_RECV;
> >
> > This one should be set where you set CO_FL_SOCKS4_SEND because when
> > you decide you'll send it, you also know you want to wait for the
> > response.
> >
> Yep you are right, when it sending the socks4 request, it is waiting for
> reply.
> But if the sending part get failed, it won't going to expect for the
> response, and
> we should not getting anything before sending the request, if we do,
> it should be something wrong.
It's the same for all other handshake flags in fact. Whenever you face an
error in any of them, the error is immediately raised and the other ones
are not executed.
> May be it should be ok to have both flags set at the same time for most
> cases, but I think setting it separately will be fine too.
It is trickier to figure which ones are expected when they are set
dynamically. For example, during a connect() we could decide about some
options depending on whether or not we expect to send only or to
send+receive. That's why it's better to have them set right as soon
as possible.
> > > + if (line[1] != 0x5A) {
> >
> > Please either use #define/enums if this corresponds to something particular,
> > or at least indicate in a comment what you're trying to verify (what's in
> > line[1] and what 0x5A corresponds to).
> Actually I have comment for it at " /* SOCKS4 Proxy request granted
> server response, 0x00 | 0x5A | 0x00 0x00 | 0x00 0x00 0x00 0x00".
> Anyway, I will make thing clearer here.
Thanks :-)
> > > flags |= CONNECT_HAS_DATA;
> > >
> > > if (global.tune.server_sndbuf)
> > > @@ -315,7 +322,7 @@ static int sockpair_connect_server(struct connection
> > > *conn, int flags)
> > > conn->flags |= CO_FL_ADDR_TO_SET;
> > >
> > > /* Prepare to send a few handshakes related to the on-wire
> > > protocol. */
> > > - if (conn->send_proxy_ofs)
> > > + if (srv && srv->pp_opts)
> > > conn->flags |= CO_FL_SEND_PROXY;
> >
> > And here, we should just move the CO_FL_SEND_PROXY flag setting to the
> > place where send_proxy_ofs is set and remove this test. It looks like
> > checks already do it correctly and that it's only needed in backend.c.
> > Then with this it becomes clean : the one who wants to emit a header
> > just sets his CO_FL_SEND_XXX flag with send_proxy_ofs=1, then each other
> > one in turn will simply have to recheck this. We could even go slightly
> > further and let the connect() functions set send_proxy_ofs to 1 themselves
> > when any of the SEND flags are set so that the upper layer only has to
> > focus on setting the appropriate flags.
> >
> > Please do that in a separate preliminary patch, which will allow to
> > bisect later if some problems are reported after your patches are merged.
> >
> I will keep it this way first, and then try to submit another patch for the
> idea
> to make the "send_proxy_ofs" clearer.
OK. That's something I can handle once Olivier's patchset is merged anyway.
> > > @@ -566,7 +566,7 @@ static int uxst_connect_server(struct connection
> > > *conn, int flags)
> > > conn->flags |= CO_FL_ADDR_TO_SET;
> > >
> > > /* Prepare to send a few handshakes related to the on-wire
> > > protocol. */
> > > - if (conn->send_proxy_ofs)
> > > + if (srv && srv->pp_opts)
> > > conn->flags |= CO_FL_SEND_PROXY;
> >
> > Same comments here. You didn't implement SOCKS4 over unix sockets ?
> > Is there any
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Thank you once again for all the detail reply.
On Wed, May 29, 2019 at 1:49 PM Willy Tarreau wrote:
>
> Hi Alec,
>
> On Thu, May 23, 2019 at 12:43:54AM +0800, Alec Liu wrote:
> > Finally got sometime to have the patch revised.
>
> And finally got some time to review it :-)
>
> > I am reusing the "send_proxy_ofs" right now.
>
> OK.
>
> > Due to it was used as some kind of flag for send Proxy Protocol in some
> > places.
> > I have to replace those usage to check the srv->pp_opts.
>
> Hmmm this will break health checks then, since you don't know whether a
> connection is made for data traffic or for a check. That makes me think
> that we've been quite annoyed for a while with these problems of finding
> all relevant connection parameters, and that maybe it could be useful to
> think about defining a "connection settings" structure with a lot of
> stuff that we want to use in various situations. A server would then
> have 2 or 3 of these (one for regular traffic or dataplane, one for
> health checks, one for agent checks). We could then use this for the
> source bindings, TLS/PP options etc. When setting up a connection, we'd
> just have to pass a pointer to the desired structure to provide all the
> settings. This definitely is something to think about for 2.1.
>
That should be able to make things more clear and easier to understand
for someone like
me new to the project. But that, I will try to read the code more
carefully to understand
how it works, and will have those changes reversed according to your comments.
> > From 6767c73a40a9d242cce061cc7feaf480ad416314 Mon Sep 17 00:00:00 2001
> > From: Alexander Liu
> > Date: Wed, 22 May 2019 19:44:48 +0800
> > Subject: [PATCH] MEDIUM: connection: Upstream SOCKS4 proxy support
> >
> > Have "socks4" and "check-via-socks4" server keyword added.
> > Implement handshake with SOCKS4 proxy server for tcp stream connection.
> > See issue #82.
> >
> > I have the "SOCKS: A protocol for TCP proxy across firewalls" doc found
> > at openssh's website under OpenSSH Sprcifications along with other RFCs,
> > at "https://www.openssh.com/specs.html;,
> > and "https://www.openssh.com/txt/socks4.protocol;.
> >
> > OpenSSH is using GPL, so I believe it should be fine to have the doc here.
> ^
>
> It's written exactly the opposite on their site and in the license file :-)
>
> LICENSE:
> "OpenSSH contains no GPL code."
>
> Well, I think you'll have to find the license covering the doc (probably
> BSD), include it in the doc/ directory and mention in the LICENSE file that
> this doc is provided under that license. Or we consider that it's too much
> hassle for just a protocol specification and you simply put a link to a
> stable URL in the commit message, which will be much simpler (and will also
> support fixes to the spec, that a copy doesn't provide).
>
Sorry for missing the license.
Due to it is a very old specification, most link just not working
well, that's why I try
to have the full text included at first place, and I have been try to
send email to the
author, but those email address just not working anymore, any way I
will leave the url
"https://www.openssh.com/txt/socks4.protocol; first but the full text.
> > diff --git a/include/proto/connection.h b/include/proto/connection.h
> > index ede372638..915be87bf 100644
> > --- a/include/proto/connection.h
> > +++ b/include/proto/connection.h
> > @@ -60,6 +60,10 @@ int conn_sock_send(struct connection *conn, const void
> > *buf, int len, int flags)
> > /* drains any pending bytes from the socket */
> > int conn_sock_drain(struct connection *conn);
> >
> > +/* scoks4 proxy handshake */
> > +int conn_send_socks4_proxy_request(struct connection *conn);
> > +int conn_recv_socks4_proxy_response(struct connection *conn);
> > +
> > /* returns true is the transport layer is ready */
> > static inline int conn_xprt_ready(const struct connection *conn)
> > {
> > @@ -889,6 +893,11 @@ static inline const char *conn_err_code_str(struct
> > connection *c)
> > case CO_ER_SSL_HANDSHAKE_HB: return "SSL handshake failure after
> > heartbeat";
> > case CO_ER_SSL_KILLED_HB: return "Stopped a TLSv1 heartbeat attack
> > (CVE-2014-0160)";
> > case CO_ER_SSL_NO_TARGET: return "Attempt to use SSL on an unknown
> > target (internal error)";
> > +
> > + case CO_ER_SOCKS4_SEND:return "SOCKS4 Proxy write error during
> > handshake";
> > + case CO_ER_SOCKS4_RECV:return "SOCKS4 Proxy read error during
> > handshake";
> > + case CO_ER_SOCKS4_DENY:return "SOCKS4 Proxy deny the request";
> denied
>
> > + case CO_ER_SOCKS4_ABORT: return "SOCKS4 Proxy handshake aborted by
> > server";
> > }
> > return NULL;
> > }
> > diff --git a/include/types/connection.h b/include/types/connection.h
> > index be590f098..9a362f05d 100644
> > --- a/include/types/connection.h
> > +++
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec,
On Thu, May 23, 2019 at 12:43:54AM +0800, Alec Liu wrote:
> Finally got sometime to have the patch revised.
And finally got some time to review it :-)
> I am reusing the "send_proxy_ofs" right now.
OK.
> Due to it was used as some kind of flag for send Proxy Protocol in some
> places.
> I have to replace those usage to check the srv->pp_opts.
Hmmm this will break health checks then, since you don't know whether a
connection is made for data traffic or for a check. That makes me think
that we've been quite annoyed for a while with these problems of finding
all relevant connection parameters, and that maybe it could be useful to
think about defining a "connection settings" structure with a lot of
stuff that we want to use in various situations. A server would then
have 2 or 3 of these (one for regular traffic or dataplane, one for
health checks, one for agent checks). We could then use this for the
source bindings, TLS/PP options etc. When setting up a connection, we'd
just have to pass a pointer to the desired structure to provide all the
settings. This definitely is something to think about for 2.1.
> From 6767c73a40a9d242cce061cc7feaf480ad416314 Mon Sep 17 00:00:00 2001
> From: Alexander Liu
> Date: Wed, 22 May 2019 19:44:48 +0800
> Subject: [PATCH] MEDIUM: connection: Upstream SOCKS4 proxy support
>
> Have "socks4" and "check-via-socks4" server keyword added.
> Implement handshake with SOCKS4 proxy server for tcp stream connection.
> See issue #82.
>
> I have the "SOCKS: A protocol for TCP proxy across firewalls" doc found
> at openssh's website under OpenSSH Sprcifications along with other RFCs,
> at "https://www.openssh.com/specs.html;,
> and "https://www.openssh.com/txt/socks4.protocol;.
>
> OpenSSH is using GPL, so I believe it should be fine to have the doc here.
^
It's written exactly the opposite on their site and in the license file :-)
LICENSE:
"OpenSSH contains no GPL code."
Well, I think you'll have to find the license covering the doc (probably
BSD), include it in the doc/ directory and mention in the LICENSE file that
this doc is provided under that license. Or we consider that it's too much
hassle for just a protocol specification and you simply put a link to a
stable URL in the commit message, which will be much simpler (and will also
support fixes to the spec, that a copy doesn't provide).
> diff --git a/include/proto/connection.h b/include/proto/connection.h
> index ede372638..915be87bf 100644
> --- a/include/proto/connection.h
> +++ b/include/proto/connection.h
> @@ -60,6 +60,10 @@ int conn_sock_send(struct connection *conn, const void
> *buf, int len, int flags)
> /* drains any pending bytes from the socket */
> int conn_sock_drain(struct connection *conn);
>
> +/* scoks4 proxy handshake */
> +int conn_send_socks4_proxy_request(struct connection *conn);
> +int conn_recv_socks4_proxy_response(struct connection *conn);
> +
> /* returns true is the transport layer is ready */
> static inline int conn_xprt_ready(const struct connection *conn)
> {
> @@ -889,6 +893,11 @@ static inline const char *conn_err_code_str(struct
> connection *c)
> case CO_ER_SSL_HANDSHAKE_HB: return "SSL handshake failure after
> heartbeat";
> case CO_ER_SSL_KILLED_HB: return "Stopped a TLSv1 heartbeat attack
> (CVE-2014-0160)";
> case CO_ER_SSL_NO_TARGET: return "Attempt to use SSL on an unknown
> target (internal error)";
> +
> + case CO_ER_SOCKS4_SEND:return "SOCKS4 Proxy write error during
> handshake";
> + case CO_ER_SOCKS4_RECV:return "SOCKS4 Proxy read error during
> handshake";
> + case CO_ER_SOCKS4_DENY:return "SOCKS4 Proxy deny the request";
denied
> + case CO_ER_SOCKS4_ABORT: return "SOCKS4 Proxy handshake aborted by
> server";
> }
> return NULL;
> }
> diff --git a/include/types/connection.h b/include/types/connection.h
> index be590f098..9a362f05d 100644
> --- a/include/types/connection.h
> +++ b/include/types/connection.h
> @@ -47,6 +47,15 @@ struct server;
> struct session;
> struct pipe;
>
> +/* socks4 upstream proxy definitions */
> +struct socks4_request {
> + uint8_t version;/* SOCKS version number, 1 byte, must be 0x04
> for this version */
> + uint8_t command;/* 0x01 = establish a TCP/IP stream connection
> */
> + uint16_t port; /* port number, 2 bytes (in network byte order)
> */
> + uint32_t ip;/* IP address, 4 bytes (in network byte order)
> */
> + char user_id[8];/* the user ID string, variable length,
> terminated with a null (0x00); Using "HAProxy\0" */
> +};
> +
> /* Note: subscribing to these events is only valid after the caller has
> really
> * attempted to perform the operation, and failed to proceed or complete.
> */
> @@ -155,8 +164,8 @@ enum {
>
> CO_FL_EARLY_SSL_HS = 0x4000, /* We have early data pending,
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Finally got sometime to have the patch revised.
I am reusing the "send_proxy_ofs" right now.
Due to it was used as some kind of flag for send Proxy Protocol in some places.
I have to replace those usage to check the srv->pp_opts.
Please help to have a look into it, thank you!
Regards,
Alexander Liu
On Mon, May 6, 2019 at 2:32 PM Willy Tarreau wrote:
>
> Hi Alec,
>
> On Tue, Apr 30, 2019 at 07:59:16PM +0800, Alec Liu wrote:
> > > I'm seeing that you copied a doc retrieved from somewhere else that is
> > > found at various places on the net. Have you checked the license for
> > > this doc to be sure we can copy and distribute it like this ? It might
> > > be perfectly fine but we need to be sure. In any case we'll have to
> > > mention the license if it differs from the other ones.
> >
> > I have the "SOCKS: A protocol for TCP proxy across firewalls" doc
> > found at openssh's website
> > under OpenSSH Sprcifications along with those RFCs.
> > "https://www.openssh.com/specs.html;,
> > "https://www.openssh.com/txt/socks4.protocol;
> > And OpenSSH is using GPL, so I believe it should be fine.
>
> OK. Please at least mention it in the commit message so that if anybody
> complains, there is no doubt about the origin and we can even take
> appropriate actions such as removing and replacing it with a link or
> anything else. Similarly if this doc ever were updated, someone could
> propose us some updates.
>
> > > > struct check_status {
> > > > diff --git a/include/types/connection.h b/include/types/connection.h
> > > > index 308be7d7..2a1b23b4 100644
> > > > --- a/include/types/connection.h
> > > > +++ b/include/types/connection.h
> > > > @@ -47,6 +47,15 @@ struct server;
> > > > struct session;
> > > > struct pipe;
> > > >
> > > > +/* socks4 upstream proxy definitions */
> > > > +struct socks4_request {
> > > > + uint8_t version;/* SOCKS version number, 1 byte, must be
> > > > 0x04 for this version */
> > > > + uint8_t command;/* 0x01 = establish a TCP/IP stream
> > > > connection */
> > > > + uint16_t port; /* port number, 2 bytes (in network byte
> > > > order) */
> > > > + uint32_t ip;/* IP address, 4 bytes (in network byte
> > > > order) */
> > > > + char user_id[8];/* the user ID string, variable length,
> > > > terminated with a null (0x00); Using "HAProxy\0" */
> > > > +};
> > > > +
> > > > /* Note: subscribing to these events is only valid after the caller
> > > > has really
> > > > * attempted to perform the operation, and failed to proceed or
> > > > complete.
> > > > */
> > > > @@ -156,7 +165,7 @@ enum {
> > > >
> > > > CO_FL_EARLY_SSL_HS = 0x4000, /* We have early data pending,
> > > > don't start SSL handshake yet */
> > > > CO_FL_EARLY_DATA= 0x8000, /* At least some of the data
> > > > are early data */
> > > > - /* unused : 0x0001 */
> > > > + CO_FL_SOCKS_HS = 0x0001, /* handshaking with upstream
> > > > SOCKS proxy */
> > > > /* unused : 0x0002 */
> > >
> > > From what I'm seeing later in the code, it's not exactly a handshake,
> > > you have to send a header and to wait for the response header to arrive.
> > > I'd rather have one flag per direction which save from having to deal
> > > with state management.
> >
> > Actually, I have the same idea as you, using two flags only. But ends up
> > with
> > this design later on. The reason I am doing it in this way, that's because:
> > 1) I might want to add the socks5 protocol support onto it later on, which
> > will become a little more complex, not just one request and response.
> > We will get something like authentication method handshake, login,
> > connection request, etc.
>
> Then this is exactly what muxes are made for. They live their own life
> and encapsulate application data in a way that is transparent for the
> application layer. Ultimately we should be able to stack multiple such
> transformations much easier than what it is today. The SSL layer is
> currently being modified for this for example. I'd suggest that you
> stick to two individual headers for SOCKS4 and that we see later how
> SOCKS5 has to be done (if ever done by the way).
>
> > 2) Looks like do not have much unused bits leftover within CO_FL_*.
>
> That's not *your* problem but the one for the person having something
> else to implement after you. Bits are like desserts in restaurants :
> first come, first served.
>
> > > > /* possible connection error codes */
> > > > enum {
> > > > @@ -451,6 +469,13 @@ struct connection {
> > > > struct {
> > > > struct sockaddr_storage from; /* client address, or
> > > > address to spoof when connecting to the server */
> > > > struct sockaddr_storage to; /* address reached by the
> > > > client, or address to connect to */
> > > > + struct {
> > > > + unsigned char use; /* socks4
>
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec,
On Tue, Apr 30, 2019 at 07:59:16PM +0800, Alec Liu wrote:
> > I'm seeing that you copied a doc retrieved from somewhere else that is
> > found at various places on the net. Have you checked the license for
> > this doc to be sure we can copy and distribute it like this ? It might
> > be perfectly fine but we need to be sure. In any case we'll have to
> > mention the license if it differs from the other ones.
>
> I have the "SOCKS: A protocol for TCP proxy across firewalls" doc
> found at openssh's website
> under OpenSSH Sprcifications along with those RFCs.
> "https://www.openssh.com/specs.html;,
> "https://www.openssh.com/txt/socks4.protocol;
> And OpenSSH is using GPL, so I believe it should be fine.
OK. Please at least mention it in the commit message so that if anybody
complains, there is no doubt about the origin and we can even take
appropriate actions such as removing and replacing it with a link or
anything else. Similarly if this doc ever were updated, someone could
propose us some updates.
> > > struct check_status {
> > > diff --git a/include/types/connection.h b/include/types/connection.h
> > > index 308be7d7..2a1b23b4 100644
> > > --- a/include/types/connection.h
> > > +++ b/include/types/connection.h
> > > @@ -47,6 +47,15 @@ struct server;
> > > struct session;
> > > struct pipe;
> > >
> > > +/* socks4 upstream proxy definitions */
> > > +struct socks4_request {
> > > + uint8_t version;/* SOCKS version number, 1 byte, must be
> > > 0x04 for this version */
> > > + uint8_t command;/* 0x01 = establish a TCP/IP stream
> > > connection */
> > > + uint16_t port; /* port number, 2 bytes (in network byte
> > > order) */
> > > + uint32_t ip;/* IP address, 4 bytes (in network byte
> > > order) */
> > > + char user_id[8];/* the user ID string, variable length,
> > > terminated with a null (0x00); Using "HAProxy\0" */
> > > +};
> > > +
> > > /* Note: subscribing to these events is only valid after the caller has
> > > really
> > > * attempted to perform the operation, and failed to proceed or complete.
> > > */
> > > @@ -156,7 +165,7 @@ enum {
> > >
> > > CO_FL_EARLY_SSL_HS = 0x4000, /* We have early data pending,
> > > don't start SSL handshake yet */
> > > CO_FL_EARLY_DATA= 0x8000, /* At least some of the data are
> > > early data */
> > > - /* unused : 0x0001 */
> > > + CO_FL_SOCKS_HS = 0x0001, /* handshaking with upstream
> > > SOCKS proxy */
> > > /* unused : 0x0002 */
> >
> > From what I'm seeing later in the code, it's not exactly a handshake,
> > you have to send a header and to wait for the response header to arrive.
> > I'd rather have one flag per direction which save from having to deal
> > with state management.
>
> Actually, I have the same idea as you, using two flags only. But ends up with
> this design later on. The reason I am doing it in this way, that's because:
> 1) I might want to add the socks5 protocol support onto it later on, which
> will become a little more complex, not just one request and response.
> We will get something like authentication method handshake, login,
> connection request, etc.
Then this is exactly what muxes are made for. They live their own life
and encapsulate application data in a way that is transparent for the
application layer. Ultimately we should be able to stack multiple such
transformations much easier than what it is today. The SSL layer is
currently being modified for this for example. I'd suggest that you
stick to two individual headers for SOCKS4 and that we see later how
SOCKS5 has to be done (if ever done by the way).
> 2) Looks like do not have much unused bits leftover within CO_FL_*.
That's not *your* problem but the one for the person having something
else to implement after you. Bits are like desserts in restaurants :
first come, first served.
> > > /* possible connection error codes */
> > > enum {
> > > @@ -451,6 +469,13 @@ struct connection {
> > > struct {
> > > struct sockaddr_storage from; /* client address, or
> > > address to spoof when connecting to the server */
> > > struct sockaddr_storage to; /* address reached by the
> > > client, or address to connect to */
> > > + struct {
> > > + unsigned char use; /* socks4
> > > proxy enabled */
> > > + struct sockaddr_storage addr; /* the
> > > address of the socks4 proxy */
> > > + struct socks4_request req_line; /* the info
> > > send to socks4 proxy */
> > > + unsigned int status;/*
> > > CO_SOCKS_* */
> > > + signed short req_remain_len;/* the
> > > length remain to sent */
> > > + } socks_proxy;
> >
> > It is extremely huge for the struct connection unfortunately. We're
> >
Re: MEDIUM: Adding upstream socks4 proxy support
Alec,
Am 30.04.19 um 13:59 schrieb Alec Liu:
> What if we just have a reference here, and using malloc to alloc some memory
> for those connections which are going to use socks proxy only. It will be
> NULL for those connections which won't need it. When the socks proxy handshake
> finished, we can have this part of memory freed, before turn it over
> for data usage.
>
> I will try to look into it, if we can reuse the "proxy protocol
> offset", if we want to keep the ability to
> allow using PROXY Protocol and socks proxy at the same time.
>
I can't comment on whether using a pointer here makes sense or not, but
please note that HAProxy includes a memory pool implementation in
include/common/memory.h ("REGISTER_POOL"). I believe all your
allocations will have the same size which fits and you don't have to
annoy the system libc malloc all the time.
Best regards
Tim Düsterhus
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Willy,
Thank for the look into it.
Here are some of my thoughts about your comments, pls check below.
> I do however have some comments, mainly about the way used to retrieve
> configuration elements as at the moment it doubles the memory consumption
> of each connection, but there are solutions to fix this that I describe
> below.
>
> > diff --git a/doc/SOCKS4.protocol.txt b/doc/SOCKS4.protocol.txt
> > new file mode 100644
> > index ..16483dee
> > --- /dev/null
> > +++ b/doc/SOCKS4.protocol.txt
> > @@ -0,0 +1,150 @@
> > + SOCKS: A protocol for TCP proxy across firewalls
> > +
> > + Ying-Da Lee
> > + yin...@best.com or yin...@esd.sgi.com
>
> (...)
>
> I'm seeing that you copied a doc retrieved from somewhere else that is
> found at various places on the net. Have you checked the license for
> this doc to be sure we can copy and distribute it like this ? It might
> be perfectly fine but we need to be sure. In any case we'll have to
> mention the license if it differs from the other ones.
I have the "SOCKS: A protocol for TCP proxy across firewalls" doc
found at openssh's website
under OpenSSH Sprcifications along with those RFCs.
"https://www.openssh.com/specs.html;,
"https://www.openssh.com/txt/socks4.protocol;
And OpenSSH is using GPL, so I believe it should be fine.
>
> > diff --git a/examples/socks4.cfg b/examples/socks4.cfg
> > new file mode 100644
> > index ..22cbad44
> > --- /dev/null
> > +++ b/examples/socks4.cfg
> > (...)
> > + server SMTPS1_Via_SocksProxy1 169.38.103.42:25 check inter 3
> > fastinter 1000
> > + server SMTPS2_Via_SocksProxy2 161.202.148.179:25 socks4
> > 127.0.0.1:1080 check-via-socks check inter 3 fastinter 1000 backup
>
> These ones are real mail servers which do respond on the net, it's not
> really friendly nor recommended to put real server addresses there, you
> can use the 192.0.2.0/24 range instead, which is reserved for
> documentation.
Will have it changed.
>
> > diff --git a/include/types/checks.h b/include/types/checks.h
> > index f89abcba..b09b5f89 100644
> > --- a/include/types/checks.h
> > +++ b/include/types/checks.h
> > @@ -189,6 +189,8 @@ struct check {
> > char *sni; /* Server name */
> > char *alpn_str; /* ALPN to use for checks */
> > int alpn_len; /* ALPN string length */
> > +
> > + int via_socks; /* check the connection via
> > socks proxy */
> > };
>
> At some point we'll have to think about creating a bit field for all
> these booleans because we're progressively inflating the struct by
> 32 bits every time we need a new flag. But let's do this later and
> not annoy you with this for now.
Totally agree, thx.
>
> > struct check_status {
> > diff --git a/include/types/connection.h b/include/types/connection.h
> > index 308be7d7..2a1b23b4 100644
> > --- a/include/types/connection.h
> > +++ b/include/types/connection.h
> > @@ -47,6 +47,15 @@ struct server;
> > struct session;
> > struct pipe;
> >
> > +/* socks4 upstream proxy definitions */
> > +struct socks4_request {
> > + uint8_t version;/* SOCKS version number, 1 byte, must be 0x04
> > for this version */
> > + uint8_t command;/* 0x01 = establish a TCP/IP stream
> > connection */
> > + uint16_t port; /* port number, 2 bytes (in network byte
> > order) */
> > + uint32_t ip;/* IP address, 4 bytes (in network byte
> > order) */
> > + char user_id[8];/* the user ID string, variable length,
> > terminated with a null (0x00); Using "HAProxy\0" */
> > +};
> > +
> > /* Note: subscribing to these events is only valid after the caller has
> > really
> > * attempted to perform the operation, and failed to proceed or complete.
> > */
> > @@ -156,7 +165,7 @@ enum {
> >
> > CO_FL_EARLY_SSL_HS = 0x4000, /* We have early data pending,
> > don't start SSL handshake yet */
> > CO_FL_EARLY_DATA= 0x8000, /* At least some of the data are
> > early data */
> > - /* unused : 0x0001 */
> > + CO_FL_SOCKS_HS = 0x0001, /* handshaking with upstream SOCKS
> > proxy */
> > /* unused : 0x0002 */
>
> From what I'm seeing later in the code, it's not exactly a handshake,
> you have to send a header and to wait for the response header to arrive.
> I'd rather have one flag per direction which save from having to deal
> with state management.
Actually, I have the same idea as you, using two flags only. But ends up with
this design later on. The reason I am doing it in this way, that's because:
1) I might want to add the socks5 protocol support onto it later on, which
will become a little more complex, not just one request and response.
We will get something like authentication method handshake, login,
connection request, etc.
2) Looks like do not have much unused bits leftover within CO_FL_*.
>
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Alec,
Overall it looks nice. I didn't know that SOCKS4 only included a header
in each direction, thus of course your approach is simpler than creating
a new mux.
I do however have some comments, mainly about the way used to retrieve
configuration elements as at the moment it doubles the memory consumption
of each connection, but there are solutions to fix this that I describe
below.
> diff --git a/doc/SOCKS4.protocol.txt b/doc/SOCKS4.protocol.txt
> new file mode 100644
> index ..16483dee
> --- /dev/null
> +++ b/doc/SOCKS4.protocol.txt
> @@ -0,0 +1,150 @@
> + SOCKS: A protocol for TCP proxy across firewalls
> +
> + Ying-Da Lee
> + yin...@best.com or yin...@esd.sgi.com
(...)
I'm seeing that you copied a doc retrieved from somewhere else that is
found at various places on the net. Have you checked the license for
this doc to be sure we can copy and distribute it like this ? It might
be perfectly fine but we need to be sure. In any case we'll have to
mention the license if it differs from the other ones.
> diff --git a/examples/socks4.cfg b/examples/socks4.cfg
> new file mode 100644
> index ..22cbad44
> --- /dev/null
> +++ b/examples/socks4.cfg
> (...)
> + server SMTPS1_Via_SocksProxy1 169.38.103.42:25 check inter 3
> fastinter 1000
> + server SMTPS2_Via_SocksProxy2 161.202.148.179:25 socks4 127.0.0.1:1080
> check-via-socks check inter 3 fastinter 1000 backup
These ones are real mail servers which do respond on the net, it's not
really friendly nor recommended to put real server addresses there, you
can use the 192.0.2.0/24 range instead, which is reserved for
documentation.
> diff --git a/include/types/checks.h b/include/types/checks.h
> index f89abcba..b09b5f89 100644
> --- a/include/types/checks.h
> +++ b/include/types/checks.h
> @@ -189,6 +189,8 @@ struct check {
> char *sni; /* Server name */
> char *alpn_str; /* ALPN to use for checks */
> int alpn_len; /* ALPN string length */
> +
> + int via_socks; /* check the connection via
> socks proxy */
> };
At some point we'll have to think about creating a bit field for all
these booleans because we're progressively inflating the struct by
32 bits every time we need a new flag. But let's do this later and
not annoy you with this for now.
> struct check_status {
> diff --git a/include/types/connection.h b/include/types/connection.h
> index 308be7d7..2a1b23b4 100644
> --- a/include/types/connection.h
> +++ b/include/types/connection.h
> @@ -47,6 +47,15 @@ struct server;
> struct session;
> struct pipe;
>
> +/* socks4 upstream proxy definitions */
> +struct socks4_request {
> + uint8_t version;/* SOCKS version number, 1 byte, must be 0x04
> for this version */
> + uint8_t command;/* 0x01 = establish a TCP/IP stream connection
> */
> + uint16_t port; /* port number, 2 bytes (in network byte order)
> */
> + uint32_t ip;/* IP address, 4 bytes (in network byte order)
> */
> + char user_id[8];/* the user ID string, variable length,
> terminated with a null (0x00); Using "HAProxy\0" */
> +};
> +
> /* Note: subscribing to these events is only valid after the caller has
> really
> * attempted to perform the operation, and failed to proceed or complete.
> */
> @@ -156,7 +165,7 @@ enum {
>
> CO_FL_EARLY_SSL_HS = 0x4000, /* We have early data pending, don't
> start SSL handshake yet */
> CO_FL_EARLY_DATA= 0x8000, /* At least some of the data are
> early data */
> - /* unused : 0x0001 */
> + CO_FL_SOCKS_HS = 0x0001, /* handshaking with upstream SOCKS
> proxy */
> /* unused : 0x0002 */
>From what I'm seeing later in the code, it's not exactly a handshake,
you have to send a header and to wait for the response header to arrive.
I'd rather have one flag per direction which save from having to deal
with state management.
> +/* flags for use in connection->socks_proxy.status */
> +enum {
> + CO_SOCKS_NONE,
> + CO_SOCKS_SENDING,
> + CO_SOCKS_SENT,
> + CO_SOCKS_RECEIVING,
> + CO_SOCKS_READY,
> + CO_SOCKS_DENY
> +};
In my opinion these ones are easily addressed with 2 bits as mentioned
above. The error status can be used for the deny case upon receipt.
> /* possible connection error codes */
> enum {
> @@ -451,6 +469,13 @@ struct connection {
> struct {
> struct sockaddr_storage from; /* client address, or address
> to spoof when connecting to the server */
> struct sockaddr_storage to; /* address reached by the
> client, or address to connect to */
> + struct {
> + unsigned char use; /* socks4 proxy
> enabled */
> + struct sockaddr_storage addr; /* the address
> of the
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Tim, Ic, let me resent it here, thank you. Hi List, Willy, I have the updated patch attached to this email, please help to review it. If you want to give it a try, you can use "ssh -D 1080 ..." to setup a quick socks4 tunnel proxy for testing. Thank you. Regards, Alexander Liu On Sat, Apr 27, 2019 at 5:55 PM Tim Düsterhus wrote: > > Alec, > > Am 27.04.19 um 11:39 schrieb Alec Liu: > > Got it, thank you. > > btw: I have the patch sent to the mail loop of the issue #82 already. > > please keep it on the list (in this specific thread). GitHub strips the > attachments and generally does not forward the emails unmodified. You > can see the result here: > https://github.com/haproxy/haproxy/issues/82#issuecomment-487271175 > > Best regards > Tim Düsterhus 0001-MEDIUM-connection-issue-82-Adding-upstream-socks4-pr.patch Description: Binary data
Re: MEDIUM: Adding upstream socks4 proxy support
Alec, Am 27.04.19 um 11:39 schrieb Alec Liu: > Got it, thank you. > btw: I have the patch sent to the mail loop of the issue #82 already. please keep it on the list (in this specific thread). GitHub strips the attachments and generally does not forward the emails unmodified. You can see the result here: https://github.com/haproxy/haproxy/issues/82#issuecomment-487271175 Best regards Tim Düsterhus
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Tim, Got it, thank you. btw: I have the patch sent to the mail loop of the issue #82 already. Regards, Alexander Liu On Sat, Apr 27, 2019 at 5:21 PM Tim Düsterhus wrote: > > Alec, > > Am 27.04.19 um 08:54 schrieb Alec Liu: > > Thank you for the comment. I will have them fix. > > But I have a question, I am not sure which subsystem should I put it > > in? Looks like I have more than one subsystem touched. > > > > In my previous reply I suggested 'connection' which seems fitting. But > in the end I'm not the person applying the patch. Don't sweat it too > much though, Willy usually is happy to adjust small things if the patch > is fine in general. > > Best regards > Tim Düsterhus
Re: MEDIUM: Adding upstream socks4 proxy support
Hi Tim, Thank you for the comment. I will have them fix. But I have a question, I am not sure which subsystem should I put it in? Looks like I have more than one subsystem touched. Thank you. Regards, Alexander Liu On Sat, Apr 27, 2019 at 7:57 AM Tim Düsterhus wrote: > > Alec, > > Am 26.04.19 um 22:10 schrieb Alec Liu: > > Here is the patch for adding upstream socks4 proxy support. > > Please help to review it, thank you. > > > > I can't comment on the technical side, but a few comments on the > 'process' side: > > 1. You did not update the documentation yet (doc/configuration.txt) > 2. The subsystem is missing from your commit message. It probably should > be: MEDIUM: connection: Add socks4 support for upstream connections > 3. The two commits should be merged into a single one. > 4. Please mention the issue number in the commit message: 'See issue #82' > > Best regards > Tim Düsterhus
Re: MEDIUM: Adding upstream socks4 proxy support
Alec, Am 26.04.19 um 22:10 schrieb Alec Liu: > Here is the patch for adding upstream socks4 proxy support. > Please help to review it, thank you. > I can't comment on the technical side, but a few comments on the 'process' side: 1. You did not update the documentation yet (doc/configuration.txt) 2. The subsystem is missing from your commit message. It probably should be: MEDIUM: connection: Add socks4 support for upstream connections 3. The two commits should be merged into a single one. 4. Please mention the issue number in the commit message: 'See issue #82' Best regards Tim Düsterhus
