add socks proxy #255

philippe44 · 2019-05-02T04:43:31Z

This is just a proposal at this moment to add socks proxy to Async::HTTP connection and SimpleAsyncHTTP. I've tried to make as little change as possible and make sure they have no side effect.

The Slim::Networking::Async::HTTP::new can now receive a hash with the socks proxy information. As the new method was not overloaded so far, I've added a new that grab the socks information and then call the parents without the extra parameters, so there should be no impact.
The Slim::Networking::Async::HTTP::new_connect looks for these socks information and if found, it creates a new object of class Slim::Networking::Async::Socket::HTTPsocks which is a new super simple package
The Slim::Networking::SimpleAsyncHTTP has been modified to accept the same socks hash and transmit it to the Slim::Networking::Async::HTTP::new

The only caveat so far is that the socks negotiation has to be in blocking mode (as I think the SSL is) but after that all read/write are set back to non-blocking. The IO::Socket::Socks package must be added and is not in the pull request

I think using socks can be useful because it allows to create tunnel per each HTTP request, and not have to VPN all or nothing. I understand this can be seen as specific to my plugins, but I've tried to make that generic in hope that it could be beneficial for others

NB: whatever you decide, my plugins ave overloaded version of the above class, so that still works. It's just not very elegant of me to replace part of these packages system-wide

michaelherger

So... I understand that this would allow you to use a Socks proxy on a per connection basis? Could you please provide a minimalistic demo plugin (could be outside this PR) and/or some instructions how to use this?

Overall I'm ok with the minimalist approach of this change. I think it's very well isolated.

michaelherger · 2019-05-02T05:11:56Z

Slim/Networking/Async/HTTP.pm

@@ -43,6 +43,7 @@ use URI;
 use File::Spec::Functions qw(catdir);

 use Slim::Networking::Async::Socket::HTTP;
+use Slim::Networking::Async::Socket::HTTPsocks;


Now this is the most nitpicky detail I could find: I'd use HTTPSocks 😁.

Another small change: could please not use this module, but require it only when needed? Eg. inside the elsif ($self->socksAddr) ... conditional code.

I was hesitating on that (HTTPSocks), but I'll do as you say. I'll move to a require as well

michaelherger · 2019-05-02T05:12:52Z

Slim/Networking/Async/HTTP.pm

+sub new {
+	my ($class, $args) = @_;
+	my $self = $class->SUPER::new;
+	$self->socksAddr( $args->{socksAddr} ); 


Would it make sense to add some input value validation? Throw an error if socksAddr wasn't an IP or at least reasonable string?

I'll look into that, I'm not very good at these this but I'll find some examples

michaelherger · 2019-05-02T05:13:15Z

Slim/Networking/Async/HTTP.pm

+		my %args = @_;
+		main::DEBUGLOG && $log->debug("Using SOCKS proxy", $self->socksAddr, ":", $self->socksAddr);
+		# socks negotiation needs to be blocking or it terminates immediately on timeout
+		my $sock Slim::Networking::Async::Socket::HTTPsocks->new( @_, 


Isn't this missing the = sign for the assignment?

shame on me for bad process

michaelherger · 2019-05-02T05:14:32Z

Slim/Networking/Async/Socket/HTTPsocks.pm

+
+use strict;
+
+use base qw(IO::Socket::Socks Net::HTTP::Methods Slim::Networking::Async::Socket);


Is IO::Socket::Socks pure Perl? Could you please include it in the PR?

Yes it is pure Perl, no headache with compilation. I'll add it

philippe44 · 2019-05-02T05:27:59Z

One other thing - we should wait for the accepting the pull request, but I wanted to know before it you would accept the idea of the changes

philippe44 · 2019-05-03T20:20:35Z

I think I’ll finish the changes during the weekend. Do you prefer a clean new pull request or do you want me to continue with this one?

michaelherger

Minor changes. Most importantly don't try to load HTTPSSocks if IO::Socket::SSL wasn't available.

As for the PR: with a relatively small change like this I'd prefer to have a small change set without all the minor commits. Instead of creating a new branch and PR you could git rebase --interactively your work to squash everything in to one or two commits, then git push --force back to your origin. This would update this PR. Just one more way to do this :-).

michaelherger · 2019-05-04T04:31:55Z

Slim/Networking/Async/HTTP.pm

+
+	if ($self->socksAddr) {
+		require Slim::Networking::Async::Socket::HTTPSocks;
+		require Slim::Networking::Async::Socket::HTTPSSocks;


Please only load if hasSSL(). There still are systems without IO::Socket::SSL which would crash LMS here.

michaelherger · 2019-05-04T04:32:37Z

Slim/Networking/Async/HTTP.pm

+	if ($self->socksAddr) {
+		require Slim::Networking::Async::Socket::HTTPSocks;
+		require Slim::Networking::Async::Socket::HTTPSSocks;
+		%socks = ( 	ProxyAddr => $self->socksAddr,


The whitespace terror! Please use tabs for main indentation of the code, but spaces to align stuff in a block...

michaelherger · 2019-05-04T04:34:52Z

Slim/Networking/Async/HTTP.pm

+
+			if (%socks) {
+				$sock = Slim::Networking::Async::Socket::HTTPSSocks->new( %args, %socks );
+				$sock->blocking(0);


Couldn't this line be in ->new()? This would avoid having to remember to call it after each instantiation.

philippe44 · 2019-05-04T05:30:58Z

All done what you've requested (I think)

philippe44 · 2019-05-04T05:43:26Z

I've also created the following README

SSH tunnel with port forwarding allows per port TCP/UDP traffic to be 
"forwarded" from one end to the tunnel to the other and appear like it was 
initiated from the remote end of tunnel. For example all traffic send to 
port 5000 on the local machine can be forwared to a remote machine, on port
25000 and will appear to any client on the remote side as if it was locally 
coming from port 25000

SOCKS is a type of proxy server that works at the TCP level. It usually sits
on port 1080 and forward TCP traffic from there. Because it works at the TCP 
level (contrary to classical HTTP proxies), it can be coupled to SSH tunnels
to create very convenient firewall passthrough and geo-locked services unlocking. 
This tunneling is not a all-or-nothing tunnel an allow each TCP request to be 
selectively forwarding or not

You must first either use a public SOCKS/SSH server pair or create your own. 
On Linux openssh does everything, one local instance with dynamic port 
forwarding (-D) and a remote instance to a friend's network does the job. On 
Windows, you can use Bitvise Tunnelier & Server. There are plenty of internet
litterature on SOCKS/SSH that explain the concept much better than anything I 
could write :-)

To use socks proxy to your HTTP on LMS requests, simply use the folling keys

	- socksAddr : the address of your socks server
	- socksPort : the port of your socks server (1080 by default)
	
These keys can be passed as parameter to SimpleAsyncHTTP::new or Async::HTP::new
	
	my $http = Slim::Networking::SimpleAsyncHTTP->new(
		sub { # succes CB },
		sub { # error CB }, 
		{ socksAddr => '127.0.0.1',
		  socksPort => 1080
		}  
	);
		
	$http->get("http://www.google.com")

	- or -
	
	my $http = Slim::Networking::Async::HTTP->new( { 
									socksAddr => '127.0.0.1',
									socksPort => 1080
									} );
	$http ->send_request( {
							'request'     => HTTP::Request->new( GET => "http://www.google.com" ),
							'onHeaders'   => sub { },
							'onError'     => sub { },
							'passthrough' => [ $p1, $p2 ],
						} );									
						
Example assumes you have a SOCKS server located with your LMS machine, but it 
could be any machine on your local network if you have setup your own or it can
be any public SOCKS server

michaelherger · 2019-05-04T13:45:43Z

Where would you put that README? In a file's header? Or a separate file?

philippe44 · 2019-05-09T02:38:13Z

I've been back & forth on the idea of making SOCKS configuration settings a general LMS item and I finally decided it's a bad idea. The objective is for plugins to be able to leverage the newt HTTPSocks and HTTPSSocks classes and use socks to improve reachability, but in fact every plugin should be able to decide what SOCKS server it wants, it cannot be a global parameter

michaelherger

Overall this is looking good to me. See my comments for minor questions/issues.

michaelherger · 2019-05-09T07:14:55Z

Slim/Networking/Async/HTTP.pm

@@ -166,8 +206,9 @@ sub send_request {
 		$self->request->protocol( 'HTTP/1.0' );
 	}

+	# the used class is NET::HTTP::Method which now supports HTTP 1.1


What do you mean by "now supports 1.1"? That module is shipped with LMS. Would that support it?

Yes, the module shipped with LMS supports 1.1, AFAIK

michaelherger · 2019-05-09T07:15:53Z

Slim/Utils/Misc.pm

@@ -1435,6 +1436,28 @@ sub min {
  return $m;
 }

+=head2 obfuscate ( )


I think you can remove these, as we're not storing any credentials?

I was thinking of keeping them here as a general services (I use it in my plugins) and maybe if we can think of a better way to implement that in the future, that would a good thing to have a centralized service

I just don't like to keep code around which is not being used by LMS itself. It risks to be ripped out as "unused" at some point...

At least call it deobfuscate, and add a note that these are there as a service for plugins.

If it really bothers you, I can remove it and duplicate it in the plugins

grammar! bad process cleaning cleaning cleaning adding SSL through SOCKS as well cleaning socks4 by default, error management, put HTTPSSocks at the right place add SOCKS5 authentication better reuse IO::Socket::Socks + add obf/unobf remove obfuscate function set HTTP 1.0

mherger

Ready to merge?

philippe44 · 2019-05-10T05:08:24Z

If you are 👍 (cross fingers)

mherger · 2019-05-10T05:10:45Z

Thanks!

philippe44 · 2019-05-10T05:11:39Z

(scary - you you I have the habbit to propose crap paches ...)

michaelherger requested changes May 2, 2019

View reviewed changes

michaelherger requested changes May 4, 2019

View reviewed changes

michaelherger reviewed May 9, 2019

View reviewed changes

philippe44 added 2 commits May 9, 2019 18:58

enable HTTP 1.1

ecbc070

mherger approved these changes May 10, 2019

View reviewed changes

mherger merged commit 2a982e7 into LMS-Community:public/7.9 May 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add socks proxy #255

add socks proxy #255

philippe44 commented May 2, 2019 •

edited

michaelherger left a comment

michaelherger May 2, 2019

philippe44 May 2, 2019

michaelherger May 2, 2019

philippe44 May 2, 2019

michaelherger May 2, 2019

philippe44 May 2, 2019

michaelherger May 2, 2019

philippe44 May 2, 2019

philippe44 commented May 2, 2019

philippe44 commented May 3, 2019

michaelherger left a comment

michaelherger May 4, 2019

michaelherger May 4, 2019

michaelherger May 4, 2019

philippe44 commented May 4, 2019

philippe44 commented May 4, 2019

michaelherger commented May 4, 2019

philippe44 commented May 9, 2019

michaelherger left a comment

michaelherger May 9, 2019

philippe44 May 9, 2019

michaelherger May 9, 2019

philippe44 May 9, 2019

michaelherger May 9, 2019

philippe44 May 9, 2019

philippe44 May 10, 2019 •

edited

mherger left a comment

philippe44 commented May 10, 2019

mherger commented May 10, 2019

philippe44 commented May 10, 2019


		use strict;

		use base qw(IO::Socket::Socks Net::HTTP::Methods Slim::Networking::Async::Socket);

add socks proxy #255

add socks proxy #255

Conversation

philippe44 commented May 2, 2019 • edited

michaelherger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippe44 commented May 2, 2019

philippe44 commented May 3, 2019

michaelherger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippe44 commented May 4, 2019

philippe44 commented May 4, 2019

michaelherger commented May 4, 2019

philippe44 commented May 9, 2019

michaelherger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philippe44 May 10, 2019 • edited

Choose a reason for hiding this comment

mherger left a comment

Choose a reason for hiding this comment

philippe44 commented May 10, 2019

mherger commented May 10, 2019

philippe44 commented May 10, 2019

philippe44 commented May 2, 2019 •

edited

philippe44 May 10, 2019 •

edited